2022-08-30 19:24:31

by Farber, Eliav

[permalink] [raw]
Subject: [PATCH v3 12/19] hwmon: (mr75203) fix voltage equation for negative source input

According to Moortec Embedded Voltage Monitor (MEVM) series 3 data sheet,
the minimum input signal is -100mv and maximum input signal is +1000mv.
When n was small enough, such that PVT_N_CONST * n < PVT_R_CONST it
resulted in n overflowing to a very large number (since n is u32 type).

This change fixes the problem by casting n to long and replacing shift
right with div operation.

Signed-off-by: Eliav Farber <[email protected]>
---
V3 -> V2:
- Fix equation to support negative values instead of limiting value to
zero.

drivers/hwmon/mr75203.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 1cd5ff6eacce..d1f090a9baac 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -222,10 +222,11 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
return ret;

n &= SAMPLE_DATA_MSK;
+
/* Convert the N bitstream count into voltage */
pre_scaler = pvt->vd[channel].pre_scaler;
- *val = pre_scaler * (PVT_N_CONST * n - PVT_R_CONST) >>
- PVT_CONV_BITS;
+ *val = pre_scaler * (PVT_N_CONST * (long)n - PVT_R_CONST) /
+ (1 << PVT_CONV_BITS);

return 0;
default:
--
2.37.1


2022-08-31 12:33:17

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 12/19] hwmon: (mr75203) fix voltage equation for negative source input

On Tue, Aug 30, 2022 at 07:22:05PM +0000, Eliav Farber wrote:
> According to Moortec Embedded Voltage Monitor (MEVM) series 3 data sheet,
> the minimum input signal is -100mv and maximum input signal is +1000mv.
> When n was small enough, such that PVT_N_CONST * n < PVT_R_CONST it
> resulted in n overflowing to a very large number (since n is u32 type).
>
> This change fixes the problem by casting n to long and replacing shift
> right with div operation.

Fixes tag?

...

> n &= SAMPLE_DATA_MSK;
> +

Unrelated change.

--
With Best Regards,
Andy Shevchenko


2022-09-01 13:14:02

by Farber, Eliav

[permalink] [raw]
Subject: Re: [PATCH v3 12/19] hwmon: (mr75203) fix voltage equation for negative source input

On 8/31/2022 3:04 PM, Andy Shevchenko wrote:
> On Tue, Aug 30, 2022 at 07:22:05PM +0000, Eliav Farber wrote:
>> According to Moortec Embedded Voltage Monitor (MEVM) series 3 data
>> sheet,
>> the minimum input signal is -100mv and maximum input signal is +1000mv.
>> When n was small enough, such that PVT_N_CONST * n < PVT_R_CONST it
>> resulted in n overflowing to a very large number (since n is u32 type).
>>
>> This change fixes the problem by casting n to long and replacing shift
>> right with div operation.
>
> Fixes tag?

For v4 I modified the commit message to (hopefully) be more
understandable:

"
According to Moortec Embedded Voltage Monitor (MEVM) series 3 data
sheet, the minimum input signal is -100mv and maximum input signal
is +1000mv.

On 64 bit machines sizeof(u32) = 4 and sizeof(long) = 8.
So when measuring a negative input and n is small enough, such that
PVT_N_CONST * n < PVT_R_CONST, it results in n overflowing to a very
large number which is not negative (because 4 MSB bytes of val are 0).

This change fixes the sign problem and supports negative values by
casting n to long and replacing shift right with div operation.
"


> ...
>
>>               n &= SAMPLE_DATA_MSK;
>> +
>
> Unrelated change.

Removed.

--
Thanks, Eliav

2022-09-01 20:50:20

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 12/19] hwmon: (mr75203) fix voltage equation for negative source input

On Thu, Sep 01, 2022 at 03:47:39PM +0300, Farber, Eliav wrote:
> On 8/31/2022 3:04 PM, Andy Shevchenko wrote:
> > On Tue, Aug 30, 2022 at 07:22:05PM +0000, Eliav Farber wrote:
> > > According to Moortec Embedded Voltage Monitor (MEVM) series 3 data
> > > sheet,
> > > the minimum input signal is -100mv and maximum input signal is +1000mv.
> > > When n was small enough, such that PVT_N_CONST * n < PVT_R_CONST it
> > > resulted in n overflowing to a very large number (since n is u32 type).
> > >
> > > This change fixes the problem by casting n to long and replacing shift
> > > right with div operation.
> >
> > Fixes tag?
>
> For v4 I modified the commit message to (hopefully) be more
> understandable:
>
> "
> According to Moortec Embedded Voltage Monitor (MEVM) series 3 data
> sheet, the minimum input signal is -100mv and maximum input signal
> is +1000mv.
>
> On 64 bit machines sizeof(u32) = 4 and sizeof(long) = 8.
> So when measuring a negative input and n is small enough, such that
> PVT_N_CONST * n < PVT_R_CONST, it results in n overflowing to a very
> large number which is not negative (because 4 MSB bytes of val are 0).
>
> This change fixes the sign problem and supports negative values by
> casting n to long and replacing shift right with div operation.
> "

What I meant is to add the tag of the commit which this one is fixing.
We have Fixes tag format for that. You may see how it's done by looking
into Git history: git log --grep Fixes:

--
With Best Regards,
Andy Shevchenko