On 6/4/24 6:33 PM, Sebastian Reichel wrote:
[...]
+static int fusb302_i2c_block_write(struct udevice *dev, u8 address,
u8 length, const u8 *data)+{
- int ret = 0;
This initialization of ret = 0 is probably unnecessary, see right below.
- if (length <= 0)
return ret;
Should this return -EINVAL if length < 0 ?
- ret = dm_i2c_write(dev, address, data, length);
- if (ret)
dev_err(dev, "cannot block write 0x%02x, len=%d, ret=%d\n",address, length, ret);- return ret;
+}
+static int fusb302_i2c_read(struct udevice *dev, u8 address, u8 *data) +{
- int ret = 0, retries;
Plain 'int ret;' is enough here , ret is always assigned in the loop below.
- for (retries = 0; retries < 3; retries++) {
ret = dm_i2c_read(dev, address, data, 1);if (ret == 0)return ret;dev_err(dev, "cannot read %02x, ret=%d\n", address, ret);- }
- return ret;
+}
+static int fusb302_i2c_block_read(struct udevice *dev, u8 address,
u8 length, u8 *data)+{
- int ret = 0;
int ret;
- if (length <= 0)
return ret;
return -EINVAL;
- ret = dm_i2c_read(dev, address, data, length);
- if (ret)
dev_err(dev, "cannot block read 0x%02x, len=%d, ret=%d\n",address, length, ret);- return ret;
+}
+static int fusb302_i2c_mask_write(struct udevice *dev, u8 address,
u8 mask, u8 value)+{
- int ret = 0;
int ret;
- u8 data;
- ret = fusb302_i2c_read(dev, address, &data);
- if (ret)
return ret;- data &= ~mask;
- data |= value;
- ret = fusb302_i2c_write(dev, address, data);
- if (ret)
return ret;- return ret;
+}
+static int fusb302_i2c_set_bits(struct udevice *dev, u8 address, u8 set_bits) +{
- return fusb302_i2c_mask_write(dev, address, 0x00, set_bits);
+}
+static int fusb302_i2c_clear_bits(struct udevice *dev, u8 address, u8 clear_bits) +{
- return fusb302_i2c_mask_write(dev, address, clear_bits, 0x00);
+}
+static int fusb302_sw_reset(struct udevice *dev) +{
- int ret = fusb302_i2c_write(dev, FUSB_REG_RESET, FUSB_REG_RESET_SW_RESET);
- if (ret)
dev_err(dev, "cannot sw reset the fusb302: %d\n", ret);- return ret;
+}
+static int fusb302_enable_tx_auto_retries(struct udevice *dev, u8 retry_count) +{
- int ret = 0;
int ret; , please fix globally
- ret = fusb302_i2c_set_bits(dev, FUSB_REG_CONTROL3, retry_count |
FUSB_REG_CONTROL3_AUTO_RETRY);- return ret;
+}
[...]
+static int fusb302_set_cc(struct udevice *dev, enum typec_cc_status cc) +{
- struct fusb302_chip *chip = dev_get_priv(dev);
- u8 switches0_mask = FUSB_REG_SWITCHES0_CC1_PU_EN |
FUSB_REG_SWITCHES0_CC2_PU_EN |FUSB_REG_SWITCHES0_CC1_PD_EN |FUSB_REG_SWITCHES0_CC2_PD_EN;- u8 rd_mda, switches0_data = 0x00;
The masks can be const u8 .
- int ret = 0;
[...]
+static int fusb302_set_polarity(struct udevice *dev,
enum typec_cc_polarity polarity)+{
- return 0;
The uclass should check if callback is available and not call it if it is not.
+}
[...]
+static int fusb302_pd_send_message(struct udevice *dev,
const struct pd_message *msg)+{
- int ret = 0;
- u8 buf[40];
- u8 pos = 0;
- int len;
- /* SOP tokens */
- buf[pos++] = FUSB302_TKN_SYNC1;
- buf[pos++] = FUSB302_TKN_SYNC1;
- buf[pos++] = FUSB302_TKN_SYNC1;
- buf[pos++] = FUSB302_TKN_SYNC2;
Why not do this ?
u8 buf[40] = { FUSB302_TKN_SYNC1, FUSB302_TKN_SYNC1, FUSB302_TKN_SYNC1, FUSB302_TKN_SYNC2 }; u8 pos = 4;
- len = pd_header_cnt_le(msg->header) * 4;
- /* plug 2 for header */
- len += 2;
- if (len > 0x1F) {
Magic value should be defined in some macro.
dev_err(dev, "PD message too long %d (incl. header)", len);return -EINVAL;- }
[...]