> -----Original Message-----
> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> Sent: Tuesday, February 25, 2020 12:13 AM
> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> Cc: linux-hwmon@xxxxxxxxxxxxxxx
> Subject: Re: [hwmon-next v2] hwmon: (pmbus/xdpe12284) Add callback for vout
> limits conversion
>
> On Mon, Feb 24, 2020 at 11:50:31PM +0200, Vadim Pasternak wrote:
> > Provide read_word_data() callback for overvoltage and undervoltage
> > output readouts conversion. These registers are presented in
> > 'slinear11' format, while default conversion for 'vout' class for the
> > devices is 'vid'. It is resulted in wrong conversion in
> > pmbus_reg2data() for in{3-4}_lcrit and in{3-4}_crit attributes.
> > )
> > Fixes: aaafb7c8eb1c ("hwmon: (pmbus) Add support for Infineon
> > Multi-phase xdpe122 family controllers")
> > Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> > ---
> > v1->v2:
> > Comments pointed out by Guenter:
> > - Drop reg2data() callback, provide conversion through
> > read_word_data() callback instead.
> > ---
> > drivers/hwmon/pmbus/xdpe12284.c | 51
> > +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > diff --git a/drivers/hwmon/pmbus/xdpe12284.c
> > b/drivers/hwmon/pmbus/xdpe12284.c index ecd9b65627ec..44b737d0bc24
> > 100644
> > --- a/drivers/hwmon/pmbus/xdpe12284.c
> > +++ b/drivers/hwmon/pmbus/xdpe12284.c
> > @@ -18,6 +18,56 @@
> > #define XDPE122_AMD_625MV 0x10 /* AMD mode 6.25mV */
> > #define XDPE122_PAGE_NUM 2
> >
> > +static int xdpe122_read_word_data(struct i2c_client *client, int page,
> > + int phase, int reg)
> > +{
> > + const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
> > + long val;
> > + s16 exponent;
> > + s32 mantissa;
> > + int ret;
> > +
> > + switch (reg) {
> > + case PMBUS_VOUT_OV_FAULT_LIMIT:
> > + case PMBUS_VOUT_UV_FAULT_LIMIT:
> > + ret = pmbus_read_word_data(client, page, phase, reg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Convert register value to LINEAR11 data. */
> > + exponent = ((s16)ret) >> 11;
> > + mantissa = ((s16)((ret & GENMASK(10, 0)) << 5)) >> 5;
> > + val = mantissa * 1000L;
> > + if (exponent >= 0)
> > + val <<= exponent;
> > + else
> > + val >>= -exponent;
> > +
> > + /* Convert data to VID register. */
> > + switch (info->vrm_version[page]) {
> > + case vr13:
> > + if (val >= 500)
> > + return 1 + DIV_ROUND_CLOSEST(val - 500, 10);
>
> return 0; ?
>
> > + case vr12:
> > + if (val >= 250)
> > + return 1 + DIV_ROUND_CLOSEST(val - 250, 5);
>
> return 0; ?
>
> > + case imvp9:
> > + if (val >= 200)
> > + return 1 + DIV_ROUND_CLOSEST(val - 200, 10);
>
> return 0; ?
> > + case amd625mv:
> > + if (val >= 200 && val <= 968750)
> > + return DIV_ROUND_CLOSEST((1550 - val) *
> 100,
> > + 625);
Oh, it should be 1550.
(155000 - 0 * 625) / 100 = 1550.
Sorry.
> return 0; ?
>
> Also, are you sure that this calculation is correct ?
> 1550 - val is almost always negative.
>
> > + default:
> > + return -EINVAL;
> > + }
> > + default:
> > + return -ENODATA;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int xdpe122_identify(struct i2c_client *client,
> > struct pmbus_driver_info *info) { @@ -70,6 +120,7
> @@ static
> > struct pmbus_driver_info xdpe122_info = {
> > PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
> > PMBUS_HAVE_POUT | PMBUS_HAVE_PIN |
> PMBUS_HAVE_STATUS_INPUT,
> > .identify = xdpe122_identify,
> > + .read_word_data = xdpe122_read_word_data,
> > };
> >
> > static int xdpe122_probe(struct i2c_client *client,
> > --
> > 2.11.0
> >
