Re: [PATCH 1/5] hwmon: (pmbus/core) Introduce pmbus_read_smbus_i2c_block_data()

["Nuno Sá" <noname.nuno@gmail.com>]

On Mon, Aug 03, 2026 at 12:35:31PM -0700, Guenter Roeck wrote:
> PMBus drivers need support for reading SMBus block data. Unfortunately,
> not all i2C controllers support this command.
> 
> Implement pmbus_read_smbus_i2c_block_data() which first tries to use
> i2c_smbus_read_block_data(). If not supported, try to emulate it by calling
> i2c_smbus_read_i2c_block_data(). Export the new function for use in PMBus
> drivers.
> 
> Cc: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> Cc: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/pmbus/pmbus.h      |   1 +
>  drivers/hwmon/pmbus/pmbus_core.c | 102 ++++++++++++++++++++++++++++---
>  2 files changed, 95 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index 5fe2c415eada..2cd3216b3cd9 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -561,6 +561,7 @@ void pmbus_set_update(struct i2c_client *client, u8 reg, bool update);
>  void pmbus_wait(struct i2c_client *client);
>  void pmbus_update_ts(struct i2c_client *client, int op);
>  int pmbus_set_page(struct i2c_client *client, int page, int phase);
> +int pmbus_read_smbus_i2c_block_data(struct i2c_client *client, u8 reg, char *data_buf);
>  int pmbus_read_word_data(struct i2c_client *client, int page, int phase,
>  			 u8 reg);

...

> +int pmbus_read_smbus_i2c_block_data(struct i2c_client *client, u8 reg, char *data_buf)
> +{
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	int blen, len, ret;
> +
> +	if (i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
> +		pmbus_wait(client);
> +		ret = i2c_smbus_read_block_data(client, reg, data_buf);
> +		pmbus_update_ts(client, 0);
> +		return ret;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		dev_err_once(&client->dev, "I2C adapter does not support I2C_FUNC_SMBUS_READ_I2C_BLOCK\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	/*
> +	 * If the returned data is valid SMBus block data, the first byte
> +	 * must be the data length.
> +	 *
> +	 * i2c_smbus_read_i2c_block_data() may return an error if the chip
> +	 * sends NACK before the number of requested bytes is received.
> +	 * Handle this by reading the data length first, then reading the
> +	 * entire message up to I2C_SMBUS_BLOCK_MAX bytes. This ensures
> +	 * that requested number of bytes never exceeds the number of
> +	 * bytes sent by the chip.
> +	 */
> +	pmbus_wait(client);
> +	ret = i2c_smbus_read_i2c_block_data(client, reg, 1, buf);
> +	pmbus_update_ts(client, 0);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +

I think the above is ret != -1 is not really necessary (same for the
below check. i2c_smbus_read_i2c_block_data() always returns the length passed to it.
The only case where it doesn't is if we pass something >
I2C_SMBUS_BLOCK_MAX which would always fail anyways.

- Nuno Sá

> +	len = buf[0];
> +	if (len == 0)
> +		return 0;
> +	blen = len;
> +	if (len >= I2C_SMBUS_BLOCK_MAX)
> +		len = I2C_SMBUS_BLOCK_MAX - 1;
> +	pmbus_wait(client);
> +	ret = i2c_smbus_read_i2c_block_data(client, reg, len + 1, buf);
> +	pmbus_update_ts(client, 0);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != len + 1)
> +		return -EIO;
> +	if (buf[0] != blen)
> +		return -EIO;
> +	memcpy(data_buf, buf + 1, len);
> +	return len;
> +}
> +EXPORT_SYMBOL_NS_GPL(pmbus_read_smbus_i2c_block_data, "PMBUS");
> +
>  static int pmbus_read_block_data(struct i2c_client *client, int page, u8 reg,
>  				 char *data_buf)
>  {
> @@ -527,11 +617,7 @@ static int pmbus_read_block_data(struct i2c_client *client, int page, u8 reg,
>  	if (rv < 0)
>  		return rv;
>  
> -	pmbus_wait(client);
> -	rv = i2c_smbus_read_block_data(client, reg, data_buf);
> -	pmbus_update_ts(client, 0);
> -
> -	return rv;
> +	return pmbus_read_smbus_i2c_block_data(client, reg, data_buf);
>  }
>  
>  static struct pmbus_sensor *pmbus_find_sensor(struct pmbus_data *data, int page,
> -- 
> 2.45.2
>