Re: [PATCH 3/5] hwmon: (pmbus/core) Add mapping function to pmbus_read_block_data()

On Tue, Jul 28, 2026 at 02:05:53PM -0700, Guenter Roeck wrote:
> On 7/28/26 09:03, Nuno Sá via B4 Relay wrote:
> > From: Nuno Sá <[email protected]>
> > 
> > This is in preparation for adding support to a device which needs to
> > use it's own read_block implementation.
> > 
> 
> The chip-specific implementation calls i2c_smbus_read_i2c_block_data().
> I'll need to know if this is a chip limit or a controller limit.
> If it is a controller limit, a chip specific override would be
> inappropriate.

I'll reply from top of my head (did not looked at the driver again).
IIRC, the biggest reason we need the chip-specific implementation is because of
the RAIL selection logic (mainly when not in page mode).

- Nuno Sá

> 
> Guenter
> 
> > Signed-off-by: Nuno Sá <[email protected]>
> > ---
> >   drivers/hwmon/pmbus/pmbus.h      |  3 +++
> >   drivers/hwmon/pmbus/pmbus_core.c | 24 ++++++++++++++++++++++--
> >   2 files changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> > index 3d5586c67f84..d697939ec892 100644
> > --- a/drivers/hwmon/pmbus/pmbus.h
> > +++ b/drivers/hwmon/pmbus/pmbus.h
> > @@ -459,6 +459,9 @@ struct pmbus_driver_info {
> >   	int (*read_byte_data)(struct i2c_client *client, int page, int reg);
> >   	int (*read_word_data)(struct i2c_client *client, int page, int phase,
> >   			      int reg);
> > +	/* size of data_buf is I2C_SMBUS_BLOCK_MAX + 2 */
> > +	int (*read_block_data)(struct i2c_client *client, int page, u8 reg,
> > +			       char *data_buf);
> >   	int (*write_byte_data)(struct i2c_client *client, int page, int reg,
> >   			      u8 byte);
> >   	int (*write_word_data)(struct i2c_client *client, int page, int reg,
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> > index 7b58f7198574..ff4572c473b7 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -533,6 +533,26 @@ static int pmbus_read_block_data(struct i2c_client *client, int page, u8 reg,
> >   	return rv;
> >   }
> > +/*
> > + * _pmbus_read_block_data() is similar to pmbus_read_block_data(), but checks if
> > + * a device specific mapping function exists and calls it if necessary.
> > + */
> > +static int _pmbus_read_block_data(struct i2c_client *client, int page, u8 reg,
> > +				  char *data_buf)
> > +{
> > +	struct pmbus_data *data = i2c_get_clientdata(client);
> > +	const struct pmbus_driver_info *info = data->info;
> > +	int status;
> > +
> > +	if (info->read_block_data) {
> > +		status = info->read_block_data(client, page, reg, data_buf);
> > +		if (status != -ENODATA)
> > +			return status;
> > +	}
> > +
> > +	return pmbus_read_block_data(client, page, reg, data_buf);
> > +}
> > +
> >   static struct pmbus_sensor *pmbus_find_sensor(struct pmbus_data *data, int page,
> >   					      int reg)
> >   {
> > @@ -678,7 +698,7 @@ static bool __maybe_unused pmbus_check_block_register(struct i2c_client *client,
> >   	struct pmbus_data *data = i2c_get_clientdata(client);
> >   	char data_buf[I2C_SMBUS_BLOCK_MAX + 2];
> > -	rv = pmbus_read_block_data(client, page, reg, data_buf);
> > +	rv = _pmbus_read_block_data(client, page, reg, data_buf);
> >   	if (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))
> >   		rv = pmbus_check_status_cml(client);
> >   	if (rv < 0 && (data->flags & PMBUS_READ_STATUS_AFTER_FAILED_CHECK))
> > @@ -3564,7 +3584,7 @@ static ssize_t pmbus_debugfs_block_read(struct file *file, char __user *buf,
> >   	char data[I2C_SMBUS_BLOCK_MAX + 2] = { 0 };
> >   	scoped_guard(pmbus_lock, client) {
> > -		rc = pmbus_read_block_data(client, entry->page, entry->reg, data);
> > +		rc = _pmbus_read_block_data(client, entry->page, entry->reg, data);
> >   		if (rc < 0)
> >   			return rc;
> >   	}
> > 
>