Re: [PATCH v3 2/2] hwmon: (pmbus/max20830) add driver for max20830

[Guenter Roeck]

,

On Fri, Apr 17, 2026 at 04:27:14PM +0800, Alexis Czezar Torreno wrote:
> Add support for MAX20830 step-down DC-DC switching regulator with
> PMBus interface. It allows monitoring of input/output voltage,
> output current and temperature through the PMBus serial interface.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@xxxxxxxxxx>
> ---
...
> +
> +	/*
> +	 * Use i2c_smbus_read_i2c_block_data() instead of
> +	 * i2c_smbus_read_block_data() to support I2C controllers
> +	 * which do not support SMBus block reads.
> +	 */
> +	ret = i2c_smbus_read_i2c_block_data(client, PMBUS_IC_DEVICE_ID,
> +					    I2C_SMBUS_BLOCK_MAX, buf);
> +	if (ret < 0)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to read IC_DEVICE_ID\n");
> +
> +	/* First byte is the block length (including itself). */
> +	len = buf[0];
> +	if (len != 9 || ret < len)
> +		return dev_err_probe(&client->dev, -ENODEV,
> +				     "IC_DEVICE_ID length mismatch: reported %u, read %d\n",
> +				     len, ret);
> +
> +	/* Data is at buf[1..8], so null terminator goes at buf[9]. */

I ended up checking the kernel code. As it turns out,
i2c_smbus_read_i2c_block_data does _not_ return the length in byte 0.
It returns the first byte of the actual data, and the length as return
value. See i2c_smbus_read_i2c_block_data() in drivers/i2c/i2c-core-smbus.c.

So this can not work as written. Something like

        if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BLOCK_DATA))
                ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, data_buf);
        else
                ret = i2c_smbus_read_i2c_block_data(client, PMBUS_IC_DEVICE_ID,
						    I2C_SMBUS_BLOCK_MAX, buf);

should do, assuming that support for I2C_FUNC_SMBUS_BLOCK_DATA and/or
I2C_FUNC_SMBUS_READ_I2C_BLOCK was checked before.

Thanks,
Guenter