Messages in this thread |  | | Subject | Re: [PATCH v5 3/4] pmbus (core): Add virtual page config bit | From | Guenter Roeck <> | Date | Sun, 19 Nov 2017 08:49:19 -0800 |
| |
On 11/17/2017 07:26 PM, Andrew Jeffery wrote: > Some circumstances call for virtual pages, to expose multiple values > packed into an extended PMBus register in a manner non-compliant with > the PMBus standard. An example of this is the Maxim MAX31785 controller, which > extends the READ_FAN_SPEED_1 PMBus register from two to four bytes to support > tach readings for both rotors of a dual rotor fan. This extended register > contains two word-sized values, one reporting the rate of the fastest rotor, > the other the rate of the slowest. The concept of virtual pages aids this > situation by mapping the page number onto the value to be selected from the > vectored result. > > We should not try to set virtual pages on the device as such a page explicitly > doesn't exist; add a flag so we can avoid doing so. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > drivers/hwmon/pmbus/pmbus.h | 2 ++ > drivers/hwmon/pmbus/pmbus_core.c | 10 +++++++--- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h > index b54d7604d3ef..d39d506aa63e 100644 > --- a/drivers/hwmon/pmbus/pmbus.h > +++ b/drivers/hwmon/pmbus/pmbus.h > @@ -372,6 +372,8 @@ enum pmbus_sensor_classes { > #define PMBUS_HAVE_PWM12 BIT(20) > #define PMBUS_HAVE_PWM34 BIT(21) > > +#define PMBUS_PAGE_VIRTUAL BIT(31) > + > enum pmbus_data_format { linear = 0, direct, vid }; > enum vrm_version { vr11 = 0, vr12, vr13 }; > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > index 631db88526b6..ba97230fcde7 100644 > --- a/drivers/hwmon/pmbus/pmbus_core.c > +++ b/drivers/hwmon/pmbus/pmbus_core.c > @@ -164,14 +164,18 @@ int pmbus_set_page(struct i2c_client *client, int page) > int rv = 0; > int newpage; > > - if (page >= 0 && page != data->currpage) { > + if (page < 0 || page == data->currpage) > + return 0; > + > + if (!(data->info->func[page] & PMBUS_PAGE_VIRTUAL)) { > rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); > newpage = i2c_smbus_read_byte_data(client, PMBUS_PAGE); > if (newpage != page) > rv = -EIO;
This should probably be something like rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page); if (rv < 0) return rv; newpage = i2c_smbus_read_byte_data(client, PMBUS_PAGE); if (newpage < 0) return newpage; if (newpage != page) return -EIO;
We can actually drop 'newpage' and just use 'rv'.
> - else > - data->currpage = page; > } > + > + data->currpage = page; > +
... otherwise currpage is set even on error.
> return rv;
this can then be return 0;
> } > EXPORT_SYMBOL_GPL(pmbus_set_page); >
|  |