linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN as paged
@ 2019-04-16 18:36 Ruslan Babayev
  2019-04-16 19:40 ` Guenter Roeck
  0 siblings, 1 reply; 4+ messages in thread
From: Ruslan Babayev @ 2019-04-16 18:36 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: xe-linux-external, Jean Delvare, linux-hwmon, linux-kernel

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;
 			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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN as paged
  2019-04-16 18:36 [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN as paged Ruslan Babayev
@ 2019-04-16 19:40 ` Guenter Roeck
  2019-04-16 21:30   ` Guenter Roeck
  0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2019-04-16 19:40 UTC (permalink / raw)
  To: Ruslan Babayev; +Cc: xe-linux-external, Jean Delvare, linux-hwmon, linux-kernel

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.

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN as paged
  2019-04-16 19:40 ` Guenter Roeck
@ 2019-04-16 21:30   ` Guenter Roeck
  0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2019-04-16 21:30 UTC (permalink / raw)
  To: Ruslan Babayev; +Cc: xe-linux-external, Jean Delvare, linux-hwmon, linux-kernel

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
> > 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN as paged
@ 2019-04-16 18:45 Ruslan Babayev
  0 siblings, 0 replies; 4+ messages in thread
From: Ruslan Babayev @ 2019-04-16 18:45 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Ruslan Babayev, xe-linux-external, Jean Delvare, linux-hwmon,
	linux-kernel

From: Ruslan Babayev <ruslan@babayev.com>

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;
 			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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-04-16 21:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 18:36 [PATCH 2/4] hwmon: (pmbus) mark registers READ_VIN and READ_IIN as paged Ruslan Babayev
2019-04-16 19:40 ` Guenter Roeck
2019-04-16 21:30   ` Guenter Roeck
2019-04-16 18:45 Ruslan Babayev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).