diff mbox series

[hwmon-next,v1,2/2] hwmon: (pmbus/xdpe12284) Add custom format for vout limits conversion

Message ID 20200224131257.28176-3-vadimp@mellanox.com (mailing list archive)
State Changes Requested
Headers show
Series hwmon: (pmbus) Add support for custom register to data conversion | expand

Commit Message

Vadim Pasternak Feb. 24, 2020, 1:12 p.m. UTC 
Provide 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.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 drivers/hwmon/pmbus/xdpe12284.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Guenter Roeck Feb. 24, 2020, 2:27 p.m. UTC | #1 
On 2/24/20 5:12 AM, Vadim Pasternak wrote:
> Provide 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.
> 
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
>   drivers/hwmon/pmbus/xdpe12284.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/xdpe12284.c b/drivers/hwmon/pmbus/xdpe12284.c
> index ecd9b65627ec..751c8e18c881 100644
> --- a/drivers/hwmon/pmbus/xdpe12284.c
> +++ b/drivers/hwmon/pmbus/xdpe12284.c
> @@ -18,6 +18,28 @@
>   #define XDPE122_AMD_625MV		0x10 /* AMD mode 6.25mV */
>   #define XDPE122_PAGE_NUM		2
>   
> +static int xdpe122_reg2data(u16 reg, int data, long *val)
> +{
> +	s16 exponent;
> +	s32 mantissa;
> +
> +	switch (reg) {
> +	case PMBUS_VOUT_OV_FAULT_LIMIT:
> +	case PMBUS_VOUT_UV_FAULT_LIMIT:

Another situation where having a datasheet would be quite valuable.

I would suggest to implement reading those two registers locally
and convert them to the expected data format. That seems to be
more straightforward than re-implementing slinear conversion.

Thanks,
Guenter

> +		/* Convert to LINEAR11. */
> +		exponent = ((s16)data) >> 11;
> +		mantissa = ((s16)((data & GENMASK(10, 0)) << 5)) >> 5;
> +		*val = mantissa * 1000L;
> +		if (exponent >= 0)
> +			*val <<= exponent;
> +		else
> +			*val >>= -exponent;
> +		return 0;
> +	default:
> +		return -ENODATA;
> +	}
> +}
> +
>   static int xdpe122_identify(struct i2c_client *client,
>   			    struct pmbus_driver_info *info)
>   {
> @@ -70,6 +92,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,
> +	.reg2data = xdpe122_reg2data,
>   };
>   
>   static int xdpe122_probe(struct i2c_client *client,
>
Vadim Pasternak Feb. 24, 2020, 3 p.m. UTC | #2 
> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Monday, February 24, 2020 4:27 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: linux-hwmon@vger.kernel.org
> Subject: Re: [hwmon-next v1 2/2] hwmon: (pmbus/xdpe12284) Add custom
> format for vout limits conversion
> 
> On 2/24/20 5:12 AM, Vadim Pasternak wrote:
> > Provide 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.
> >
> > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > ---
> >   drivers/hwmon/pmbus/xdpe12284.c | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/hwmon/pmbus/xdpe12284.c
> > b/drivers/hwmon/pmbus/xdpe12284.c index ecd9b65627ec..751c8e18c881
> > 100644
> > --- a/drivers/hwmon/pmbus/xdpe12284.c
> > +++ b/drivers/hwmon/pmbus/xdpe12284.c
> > @@ -18,6 +18,28 @@
> >   #define XDPE122_AMD_625MV		0x10 /* AMD mode 6.25mV */
> >   #define XDPE122_PAGE_NUM		2
> >
> > +static int xdpe122_reg2data(u16 reg, int data, long *val) {
> > +	s16 exponent;
> > +	s32 mantissa;
> > +
> > +	switch (reg) {
> > +	case PMBUS_VOUT_OV_FAULT_LIMIT:
> > +	case PMBUS_VOUT_UV_FAULT_LIMIT:
> 
> Another situation where having a datasheet would be quite valuable.
> 
> I would suggest to implement reading those two registers locally and convert
> them to the expected data format. That seems to be more straightforward than
> re-implementing slinear conversion.

Hi Guenter,

Do you mean to implement them locally through the virtual registers?
	case PMBUS_VIRT_VMON_OV_FAULT_LIMIT:
	case PMBUS_VIRT_VMON_UV_FAULT_LIMIT:

Thanks,
Vadim.

> 
> Thanks,
> Guenter
> 
> > +		/* Convert to LINEAR11. */
> > +		exponent = ((s16)data) >> 11;
> > +		mantissa = ((s16)((data & GENMASK(10, 0)) << 5)) >> 5;
> > +		*val = mantissa * 1000L;
> > +		if (exponent >= 0)
> > +			*val <<= exponent;
> > +		else
> > +			*val >>= -exponent;
> > +		return 0;
> > +	default:
> > +		return -ENODATA;
> > +	}
> > +}
> > +
> >   static int xdpe122_identify(struct i2c_client *client,
> >   			    struct pmbus_driver_info *info)
> >   {
> > @@ -70,6 +92,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,
> > +	.reg2data = xdpe122_reg2data,
> >   };
> >
> >   static int xdpe122_probe(struct i2c_client *client,
> >
Guenter Roeck Feb. 24, 2020, 3:08 p.m. UTC | #3 
On 2/24/20 7:00 AM, Vadim Pasternak wrote:
> 
> 
>> -----Original Message-----
>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>> Sent: Monday, February 24, 2020 4:27 PM
>> To: Vadim Pasternak <vadimp@mellanox.com>
>> Cc: linux-hwmon@vger.kernel.org
>> Subject: Re: [hwmon-next v1 2/2] hwmon: (pmbus/xdpe12284) Add custom
>> format for vout limits conversion
>>
>> On 2/24/20 5:12 AM, Vadim Pasternak wrote:
>>> Provide 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.
>>>
>>> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
>>> ---
>>>    drivers/hwmon/pmbus/xdpe12284.c | 23 +++++++++++++++++++++++
>>>    1 file changed, 23 insertions(+)
>>>
>>> diff --git a/drivers/hwmon/pmbus/xdpe12284.c
>>> b/drivers/hwmon/pmbus/xdpe12284.c index ecd9b65627ec..751c8e18c881
>>> 100644
>>> --- a/drivers/hwmon/pmbus/xdpe12284.c
>>> +++ b/drivers/hwmon/pmbus/xdpe12284.c
>>> @@ -18,6 +18,28 @@
>>>    #define XDPE122_AMD_625MV		0x10 /* AMD mode 6.25mV */
>>>    #define XDPE122_PAGE_NUM		2
>>>
>>> +static int xdpe122_reg2data(u16 reg, int data, long *val) {
>>> +	s16 exponent;
>>> +	s32 mantissa;
>>> +
>>> +	switch (reg) {
>>> +	case PMBUS_VOUT_OV_FAULT_LIMIT:
>>> +	case PMBUS_VOUT_UV_FAULT_LIMIT:
>>
>> Another situation where having a datasheet would be quite valuable.
>>
>> I would suggest to implement reading those two registers locally and convert
>> them to the expected data format. That seems to be more straightforward than
>> re-implementing slinear conversion.
> 
> Hi Guenter,
> 
> Do you mean to implement them locally through the virtual registers?
> 	case PMBUS_VIRT_VMON_OV_FAULT_LIMIT:
> 	case PMBUS_VIRT_VMON_UV_FAULT_LIMIT:
> 

No, just implement reading PMBUS_VOUT_OV_FAULT_LIMIT and PMBUS_VOUT_UV_FAULT_LIMIT
locally, or in other words intercept it in a read_word_data function.

Thanks,
Guenter

> Thanks,
> Vadim.
> 
>>
>> Thanks,
>> Guenter
>>
>>> +		/* Convert to LINEAR11. */
>>> +		exponent = ((s16)data) >> 11;
>>> +		mantissa = ((s16)((data & GENMASK(10, 0)) << 5)) >> 5;
>>> +		*val = mantissa * 1000L;
>>> +		if (exponent >= 0)
>>> +			*val <<= exponent;
>>> +		else
>>> +			*val >>= -exponent;
>>> +		return 0;
>>> +	default:
>>> +		return -ENODATA;
>>> +	}
>>> +}
>>> +
>>>    static int xdpe122_identify(struct i2c_client *client,
>>>    			    struct pmbus_driver_info *info)
>>>    {
>>> @@ -70,6 +92,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,
>>> +	.reg2data = xdpe122_reg2data,
>>>    };
>>>
>>>    static int xdpe122_probe(struct i2c_client *client,
>>>
>
diff mbox series

Patch

diff --git a/drivers/hwmon/pmbus/xdpe12284.c b/drivers/hwmon/pmbus/xdpe12284.c
index ecd9b65627ec..751c8e18c881 100644
--- a/drivers/hwmon/pmbus/xdpe12284.c
+++ b/drivers/hwmon/pmbus/xdpe12284.c
@@ -18,6 +18,28 @@ 
 #define XDPE122_AMD_625MV		0x10 /* AMD mode 6.25mV */
 #define XDPE122_PAGE_NUM		2
 
+static int xdpe122_reg2data(u16 reg, int data, long *val)
+{
+	s16 exponent;
+	s32 mantissa;
+
+	switch (reg) {
+	case PMBUS_VOUT_OV_FAULT_LIMIT:
+	case PMBUS_VOUT_UV_FAULT_LIMIT:
+		/* Convert to LINEAR11. */
+		exponent = ((s16)data) >> 11;
+		mantissa = ((s16)((data & GENMASK(10, 0)) << 5)) >> 5;
+		*val = mantissa * 1000L;
+		if (exponent >= 0)
+			*val <<= exponent;
+		else
+			*val >>= -exponent;
+		return 0;
+	default:
+		return -ENODATA;
+	}
+}
+
 static int xdpe122_identify(struct i2c_client *client,
 			    struct pmbus_driver_info *info)
 {
@@ -70,6 +92,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,
+	.reg2data = xdpe122_reg2data,
 };
 
 static int xdpe122_probe(struct i2c_client *client,