| Message ID | 20220830192212.28570-13-farbere@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Variety of fixes and new features for mr75203 driver | expand |
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.
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
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:
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:
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 <farbere@amazon.com> --- 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(-)