Staging/max77958 - #3541
Staging/max77958#3541sean-park-pub wants to merge 3 commits into
Conversation
Add MAX77958 to KERNEL_ALL_ADI_DRIVERS config to enable
building the MAX77958 usb/typec driver with the ADI driver collection.
Signed-off-by: Park, Sean <Sean.Park@analog.com>
Signed-off-by: Park, Sean <Sean.Park@analog.com>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Park, Sean <Sean.Park@analog.com>
db2aa28 to
902c7a4
Compare
| max77958_get_version_info(usbc_data); | ||
| max77958_init_irq_handler(usbc_data); | ||
|
|
||
| max77958_cc_init(usbc_data); | ||
| max77958_bc12_init(usbc_data); | ||
| max77958_pd_init(usbc_data); |
There was a problem hiding this comment.
Like in the other prs, all may return errors and early, and no proper clean-up is being done; please use devm_* wrappers
| } | ||
| INIT_WORK(&max77958->wait_apcmd_resp_work, wait_apcmd_resp_work_func); | ||
|
|
||
| ret = request_firmware(&fw, MAX77958_FIRMWARE_FILE, &max77958->i2c->dev); |
There was a problem hiding this comment.
firmware file size is not checked before accessing fw_bin[4] and fw_bin[5], out-of-boundary memory bug if the file is truncated.
| dev_err(&max77958->i2c->dev, "Failed to load firmware '%s' (err=%d)\n", MAX77958_FIRMWARE_FILE, ret); | ||
| } else { | ||
| dev_info(&max77958->i2c->dev, "Firmware '%s' loaded as %zu bytes\n", MAX77958_FIRMWARE_FILE, fw->size); | ||
| ret = max77958_usbc_fw_update(max77958, fw->data, fw->size, 0); |
There was a problem hiding this comment.
ret is never read, what if max77958_usbc_fw_update fails?
| err = i2c_smbus_write_i2c_block_data(i2c, addr, len, pbuf); | ||
| if (err) { | ||
| pr_err("%s : failed(%d) to i2c_smbus_write_i2c_block_data\n", __func__, err); | ||
| break; |
There was a problem hiding this comment.
you break the case, not the outer loop, overwriting the err code.
| MAX77958_I2C_BULK_DEBUG(KERN_INFO, strdump, DUMP_PREFIX_NONE, 8, 1, (const void *)pbuf, len, false); | ||
| err = i2c_smbus_write_i2c_block_data(i2c, addr, len, pbuf); | ||
| if (err) { | ||
| pr_err("%s : failed(%d) to i2c_smbus_write_i2c_block_data\n", __func__, err); |
There was a problem hiding this comment.
Use dev_err, dev_err_probe for device errors, no __func__, there are tracing symbols that do the same.
the dynamic debug with function name is
echo 'module <MODULE> +pf' > /sys/kernel/debug/dynamic_debug/control
see flags section at
https://docs.kernel.org/admin-guide/dynamic-debug-howto.html
| g_usbc_data = usbc_data; | ||
| usbc_data->dev = max77958->dev; | ||
| usbc_data->max77958 = max77958; |
There was a problem hiding this comment.
What if we have two max77958 in the devicetree? the second will overwrite the first. Properly allocate per device probe, be careful with shared global state.
max77958_dev is per device probe, a better location to store state.
| return ret; | ||
| } | ||
|
|
||
| void max77958_usbc_deinit(struct max77958_usbc_platform_data *usbc_data) |
There was a problem hiding this comment.
delayed work is never cleared. see devm_delayed_work_autocancel to auto cancel
This branch has not been deployed
3 participants
PR Description
necessary to understand them. List any dependencies required for this change.
any space), or simply check them after publishing the PR.
description and try to push all related PRs simultaneously.
PR Type
PR Checklist