Re: [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN as paged

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Apr 16, 2019 at 12:40:33PM -0700, Guenter Roeck wrote:
> On Tue, Apr 16, 2019 at 11:36:17AM -0700, Ruslan Babayev wrote:
> > On some devices (like IR35215) READ_VIN and READ_IIN registers are
> > paged.
> > 
> > For devices where these registers are not paged the extra check
> > ensures we expose only the registers that are actually present.
> > 
> > Cc: xe-linux-external@cisco.com
> > Signed-off-by: Ruslan Babayev <ruslan@babayev.com>
> > ---
> >  drivers/hwmon/pmbus/pmbus_core.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> > index f35b239961e3..fac966967816 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -1379,6 +1379,9 @@ static int pmbus_add_sensor_attrs(struct i2c_client *client,
> >  		for (page = 0; page < pages; page++) {
> >  			if (!(info->func[page] & attrs->func))
> >  				continue;
> > +			if (!pmbus_check_word_register(client, page,
> > +						       attrs->reg))
> > +				continue;
> 
> This won't work. Most other chips will just report the values from page 0
> when trying to read VIN/IIN from other pages.
> 
Another problem is that this would change the label for non-pages devices
to vin[0...]. I understand the need, but we'll need some means to tell the
core that the input (voltage, current, power, ..) registers are paged.

Another option, to specifically address this case, might be to report the
second voltage as vmon or vcap (and maybe add imon / icap), but that would
be a bit clumsy unless it is not really an "input" voltage but, for example,
the chip's VCC.

It is quite unfortunate that the datasheet is not public. It would be quite
useful to know what exactly is measured with those paged input sensors.

Guenter

> Guenter
> 
> >  			ret = pmbus_add_sensor_attrs_one(client, data, info,
> >  							 name, index, page,
> >  							 attrs);
> > @@ -1498,6 +1501,7 @@ static const struct pmbus_sensor_attr voltage_attributes[] = {
> >  		.reg = PMBUS_READ_VIN,
> >  		.class = PSC_VOLTAGE_IN,
> >  		.label = "vin",
> > +		.paged = true,
> >  		.func = PMBUS_HAVE_VIN,
> >  		.sfunc = PMBUS_HAVE_STATUS_INPUT,
> >  		.sbase = PB_STATUS_INPUT_BASE,
> > @@ -1602,6 +1606,7 @@ static const struct pmbus_sensor_attr current_attributes[] = {
> >  		.reg = PMBUS_READ_IIN,
> >  		.class = PSC_CURRENT_IN,
> >  		.label = "iin",
> > +		.paged = true,
> >  		.func = PMBUS_HAVE_IIN,
> >  		.sfunc = PMBUS_HAVE_STATUS_INPUT,
> >  		.sbase = PB_STATUS_INPUT_BASE,
> > -- 
> > 2.17.1
> >