| Message ID | 20200224215031.24729-1-vadimp@mellanox.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [hwmon-next,v2] hwmon: (pmbus/xdpe12284) Add callback for vout limits conversion | expand |
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@mellanox.com> > --- > 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); 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 >
> -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Tuesday, February 25, 2020 12:13 AM > To: Vadim Pasternak <vadimp@mellanox.com> > Cc: linux-hwmon@vger.kernel.org > 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@mellanox.com> > > --- > > 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 > >
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); + case vr12: + if (val >= 250) + return 1 + DIV_ROUND_CLOSEST(val - 250, 5); + case imvp9: + if (val >= 200) + return 1 + DIV_ROUND_CLOSEST(val - 200, 10); + case amd625mv: + if (val >= 200 && val <= 968750) + return DIV_ROUND_CLOSEST((1550 - val) * 100, + 625); + 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,
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@mellanox.com> --- 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(+)