linux-kernel - Re: [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190416213048.GA29411@roeck-us.net>
Date:   Tue, 16 Apr 2019 14:30:48 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Ruslan Babayev <ruslan@...ayev.com>
Cc:     xe-linux-external@...co.com, Jean Delvare <jdelvare@...e.com>,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN
 as paged

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@...co.com
> > Signed-off-by: Ruslan Babayev <ruslan@...ayev.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
> >