[PATCH] hwmon: (lm70) Fix rounding of negative temperatures

[PATCH] hwmon: (lm70) Fix rounding of negative temperatures

[Ridham Khurana]

temp1_input_show() drops the low bits of the temperature register by
dividing the raw value (raw / 32 on the LM70). These bits are not part
of the temperature: the LM70, LM71, LM74 and TMP122/TMP124 always
return some of them as 1, and the TMP125 fills them with copies of the
temperature LSB. Division rounds toward zero, so a negative temperature
with any of these bits set is reported one LSB too high.

For example, the TMP122 returns 0xffff for -0.0625 degrees C, which the
driver reports as 0. The LM70 returns 0xf39f for -25 degrees C, which
the driver reports as -24750.

Use an arithmetic right shift instead, which drops the low bits and
rounds down. tmp421 had a similar problem with negative values, fixed
by commit 724e8af85854 ("hwmon: (tmp421) fix rounding for negative
values").

Fixes: e1a8e913f97e ("[PATCH] lm70: New hardware monitoring driver")
Fixes: a86e94dc946d ("hwmon: (lm70) Add support for LM71 and LM74")
Fixes: cd929672a9ef ("hwmon: (lm70) Add ti,tmp125 support")
Signed-off-by: Ridham Khurana <khurana.ridham222@gmail.com>
---
Build-tested on arm64 and arm (ep93xx_defconfig) with CONFIG_SENSORS_LM70=m. Tested in QEMU (arm64 and arm) with a stub SPI controller returning the datasheet register values; not tested on hardware.

 drivers/hwmon/lm70.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c
index 8b525725fef4..b29bac7a4cc1 100644
--- a/drivers/hwmon/lm70.c
+++ b/drivers/hwmon/lm70.c
@@ -96,21 +96,21 @@ static ssize_t temp1_input_show(struct device *dev,
 	 */
 	switch (p_lm70->chip) {
 	case LM70_CHIP_LM70:
-		val = ((int)raw / 32) * 250;
+		val = ((int)raw >> 5) * 250;
 		break;
 
 	case LM70_CHIP_TMP121:
 	case LM70_CHIP_TMP122:
 	case LM70_CHIP_LM74:
-		val = ((int)raw / 8) * 625 / 10;
+		val = ((int)raw >> 3) * 625 / 10;
 		break;
 
 	case LM70_CHIP_LM71:
-		val = ((int)raw / 4) * 3125 / 100;
+		val = ((int)raw >> 2) * 3125 / 100;
 		break;
 
 	case LM70_CHIP_TMP125:
-		val = (sign_extend32(raw, 14) / 32) * 250;
+		val = (sign_extend32(raw, 14) >> 5) * 250;
 		break;
 	}
 
-- 
2.47.3

Re: [PATCH] hwmon: (lm70) Fix rounding of negative temperatures

[Guenter Roeck]

On Thu, Sep 24, 2026 at 08:53:23PM +0000, Ridham Khurana wrote:
> temp1_input_show() drops the low bits of the temperature register by
> dividing the raw value (raw / 32 on the LM70). These bits are not part
> of the temperature: the LM70, LM71, LM74 and TMP122/TMP124 always
> return some of them as 1, and the TMP125 fills them with copies of the
> temperature LSB. Division rounds toward zero, so a negative temperature
> with any of these bits set is reported one LSB too high.
> 
> For example, the TMP122 returns 0xffff for -0.0625 degrees C, which the
> driver reports as 0. The LM70 returns 0xf39f for -25 degrees C, which
> the driver reports as -24750.
> 
> Use an arithmetic right shift instead, which drops the low bits and
> rounds down. tmp421 had a similar problem with negative values, fixed
> by commit 724e8af85854 ("hwmon: (tmp421) fix rounding for negative
> values").

Please stop providing those "had a similar problem" messages (or tell your
AI to do it). This is irrelevant for this fix.

> 
> Fixes: e1a8e913f97e ("[PATCH] lm70: New hardware monitoring driver")
> Fixes: a86e94dc946d ("hwmon: (lm70) Add support for LM71 and LM74")
> Fixes: cd929672a9ef ("hwmon: (lm70) Add ti,tmp125 support")
> Signed-off-by: Ridham Khurana <khurana.ridham222@gmail.com>

Applied.

Thanks,
Guenter

Re: [PATCH] hwmon: (lm70) Fix rounding of negative temperatures

[Christian Lamparter]

Hi,

On 9/24/26 10:53 PM, Ridham Khurana wrote:
> temp1_input_show() drops the low bits of the temperature register by
> dividing the raw value (raw / 32 on the LM70). These bits are not part
> of the temperature: the LM70, LM71, LM74 and TMP122/TMP124 always
> return some of them as 1, and the TMP125 fills them with copies of the
> temperature LSB. Division rounds toward zero, so a negative temperature
> with any of these bits set is reported one LSB too high.
> 
> For example, the TMP122 returns 0xffff for -0.0625 degrees C, which the
> driver reports as 0. The LM70 returns 0xf39f for -25 degrees C, which
> the driver reports as -24750.
> 
> Use an arithmetic right shift instead, which drops the low bits and
> rounds down. tmp421 had a similar problem with negative values, fixed
> by commit 724e8af85854 ("hwmon: (tmp421) fix rounding for negative
> values").

For the TMP125 yes:

Reviewed-by: Christian Lamparter <chunkeey@gmail.com>


> Fixes: e1a8e913f97e ("[PATCH] lm70: New hardware monitoring driver")
> Fixes: a86e94dc946d ("hwmon: (lm70) Add support for LM71 and LM74")
> Fixes: cd929672a9ef ("hwmon: (lm70) Add ti,tmp125 support")
> Signed-off-by: Ridham Khurana <khurana.ridham222@gmail.com>
> ----
I wrote a little test program by hand (see below) and ran it on x64.
No idea if different archs behave differently or if
a clever unsafe math optimizing compiler flag will
convert the  "/ 32" to a " >> 5" but I doubt that...

---
tmp125: raw:7ec0 tmp125_proposed: -2500 tmp125_now: -2500
tmp125: raw:7eff tmp125_proposed: -2250 tmp125_now: -2000  <--- loss of percision
tmp125: raw:7f00 tmp125_proposed: -2000 tmp125_now: -2000
tmp125: raw:7f3f tmp125_proposed: -1750 tmp125_now: -1500  <--- more
tmp125: raw:7f40 tmp125_proposed: -1500 tmp125_now: -1500
tmp125: raw:7f7f tmp125_proposed: -1250 tmp125_now: -1000  <--- and this
tmp125: raw:7f80 tmp125_proposed: -1000 tmp125_now: -1000
tmp125: raw:7fbf tmp125_proposed:  -750 tmp125_now:  -500  <--- also bad
tmp125: raw:7fc0 tmp125_proposed:  -500 tmp125_now:  -500
tmp125: raw:7fff tmp125_proposed:  -250 tmp125_now:     0  <--- ouch
tmp125: raw:   0 tmp125_proposed:     0 tmp125_now:     0
tmp125: raw:  3f tmp125_proposed:   250 tmp125_now:   250
tmp125: raw:  40 tmp125_proposed:   500 tmp125_now:   500
tmp125: raw:  7f tmp125_proposed:   750 tmp125_now:   750
tmp125: raw:  80 tmp125_proposed:  1000 tmp125_now:  1000
tmp125: raw:  bf tmp125_proposed:  1250 tmp125_now:  1250
tmp125: raw:  c0 tmp125_proposed:  1500 tmp125_now:  1500
tmp125: raw:  ff tmp125_proposed:  1750 tmp125_now:  1750
tmp125: raw: 100 tmp125_proposed:  2000 tmp125_now:  2000
tmp125: raw: 13f tmp125_proposed:  2250 tmp125_now:  2250
tmp125: raw: 140 tmp125_proposed:  2500 tmp125_now:  2500

(the positive values all look good.)

--- SNIP test.c program

#just name the file test.c and run make test (in a directory without any other makefile)
#include <stdio.h>
#include <asm-generic/int-l64.h>

static __s32 sign_extend32(__u32 value, int index)
{
         __u8 shift = 31 - index;
         return (__s32)(value << shift) >> shift;
}

static int tmp125_now(__s16 raw)
{
         return (sign_extend32(raw, 14) / 32) * 250;

}

static int tmp125_proposed(__s16 raw)
{
         return (sign_extend32(raw, 14) >> 5) * 250;

}

int main(int argc, char **args)
{
         for (__s16 raw = -320; raw <= 320; raw+=32) {
                 __s16 tmp=raw & 0x7fe0 | ((raw & 32) >> 1) | ((raw & 32) >> 2) | ((raw & 32) >> 3) | ((raw & 32) >> 4) | ((raw & 32) >> 5);
                 printf("tmp125: raw:%4x tmp125_proposed:%6d tmp125_now:%6d\n", tmp, tmp125_proposed(tmp), tmp125_now(tmp));
         }

         return 0;
}

--- SNAP

FYI: TMP125 datasheet says:
"The  Temperature  Register  of  the  TMP125  is  a  16-bit, read-only register that stores
the output of the most recentconversion. However, temperature is represented by only10-bits,
which are in signed two’s complement format. Thefirst bit of the Temperature Register, D15,
is a leading zero. Bits D14 and [sic] D5 are used to indicate temperature. Bits D4 to D0
are the same as D5 (see Table 1)."

(They probably meant Bits D14 >>to<< D5 are used to indicate temperature).

Re: [PATCH] hwmon: (lm70) Fix rounding of negative temperatures

[Guenter Roeck]

On Fri, Sep 25, 2026 at 08:03:06PM +0200, Christian Lamparter wrote:
> Hi,
> 
> On 9/24/26 10:53 PM, Ridham Khurana wrote:
> > temp1_input_show() drops the low bits of the temperature register by
> > dividing the raw value (raw / 32 on the LM70). These bits are not part
> > of the temperature: the LM70, LM71, LM74 and TMP122/TMP124 always
> > return some of them as 1, and the TMP125 fills them with copies of the
> > temperature LSB. Division rounds toward zero, so a negative temperature
> > with any of these bits set is reported one LSB too high.
> > 
> > For example, the TMP122 returns 0xffff for -0.0625 degrees C, which the
> > driver reports as 0. The LM70 returns 0xf39f for -25 degrees C, which
> > the driver reports as -24750.
> > 
> > Use an arithmetic right shift instead, which drops the low bits and
> > rounds down. tmp421 had a similar problem with negative values, fixed
> > by commit 724e8af85854 ("hwmon: (tmp421) fix rounding for negative
> > values").
> 
> For the TMP125 yes:
> 
> Reviewed-by: Christian Lamparter <chunkeey@gmail.com>
> 
> 
> > Fixes: e1a8e913f97e ("[PATCH] lm70: New hardware monitoring driver")
> > Fixes: a86e94dc946d ("hwmon: (lm70) Add support for LM71 and LM74")
> > Fixes: cd929672a9ef ("hwmon: (lm70) Add ti,tmp125 support")
> > Signed-off-by: Ridham Khurana <khurana.ridham222@gmail.com>
> > ----
> I wrote a little test program by hand (see below) and ran it on x64.
> No idea if different archs behave differently or if
> a clever unsafe math optimizing compiler flag will
> convert the  "/ 32" to a " >> 5" but I doubt that...
> 
> ---
> tmp125: raw:7ec0 tmp125_proposed: -2500 tmp125_now: -2500
> tmp125: raw:7eff tmp125_proposed: -2250 tmp125_now: -2000  <--- loss of percision
> tmp125: raw:7f00 tmp125_proposed: -2000 tmp125_now: -2000
> tmp125: raw:7f3f tmp125_proposed: -1750 tmp125_now: -1500  <--- more
> tmp125: raw:7f40 tmp125_proposed: -1500 tmp125_now: -1500
> tmp125: raw:7f7f tmp125_proposed: -1250 tmp125_now: -1000  <--- and this
> tmp125: raw:7f80 tmp125_proposed: -1000 tmp125_now: -1000
> tmp125: raw:7fbf tmp125_proposed:  -750 tmp125_now:  -500  <--- also bad
> tmp125: raw:7fc0 tmp125_proposed:  -500 tmp125_now:  -500
> tmp125: raw:7fff tmp125_proposed:  -250 tmp125_now:     0  <--- ouch
> tmp125: raw:   0 tmp125_proposed:     0 tmp125_now:     0
> tmp125: raw:  3f tmp125_proposed:   250 tmp125_now:   250
> tmp125: raw:  40 tmp125_proposed:   500 tmp125_now:   500
> tmp125: raw:  7f tmp125_proposed:   750 tmp125_now:   750
> tmp125: raw:  80 tmp125_proposed:  1000 tmp125_now:  1000
> tmp125: raw:  bf tmp125_proposed:  1250 tmp125_now:  1250
> tmp125: raw:  c0 tmp125_proposed:  1500 tmp125_now:  1500
> tmp125: raw:  ff tmp125_proposed:  1750 tmp125_now:  1750
> tmp125: raw: 100 tmp125_proposed:  2000 tmp125_now:  2000
> tmp125: raw: 13f tmp125_proposed:  2250 tmp125_now:  2250
> tmp125: raw: 140 tmp125_proposed:  2500 tmp125_now:  2500
> 

Sorry, you lost me with the above. Are you suggesting that the patch is
correct, that it is only correct for TMP125, or something else ?

Guenter

> (the positive values all look good.)
> 
> --- SNIP test.c program
> 
> #just name the file test.c and run make test (in a directory without any other makefile)
> #include <stdio.h>
> #include <asm-generic/int-l64.h>
> 
> static __s32 sign_extend32(__u32 value, int index)
> {
>         __u8 shift = 31 - index;
>         return (__s32)(value << shift) >> shift;
> }
> 
> static int tmp125_now(__s16 raw)
> {
>         return (sign_extend32(raw, 14) / 32) * 250;
> 
> }
> 
> static int tmp125_proposed(__s16 raw)
> {
>         return (sign_extend32(raw, 14) >> 5) * 250;
> 
> }
> 
> int main(int argc, char **args)
> {
>         for (__s16 raw = -320; raw <= 320; raw+=32) {
>                 __s16 tmp=raw & 0x7fe0 | ((raw & 32) >> 1) | ((raw & 32) >> 2) | ((raw & 32) >> 3) | ((raw & 32) >> 4) | ((raw & 32) >> 5);
>                 printf("tmp125: raw:%4x tmp125_proposed:%6d tmp125_now:%6d\n", tmp, tmp125_proposed(tmp), tmp125_now(tmp));
>         }
> 
>         return 0;
> }
> 
> --- SNAP
> 
> FYI: TMP125 datasheet says:
> "The  Temperature  Register  of  the  TMP125  is  a  16-bit, read-only register that stores
> the output of the most recentconversion. However, temperature is represented by only10-bits,
> which are in signed two’s complement format. Thefirst bit of the Temperature Register, D15,
> is a leading zero. Bits D14 and [sic] D5 are used to indicate temperature. Bits D4 to D0
> are the same as D5 (see Table 1)."
> 
> (They probably meant Bits D14 >>to<< D5 are used to indicate temperature).
> 

Re: [PATCH] hwmon: (lm70) Fix rounding of negative temperatures

[Christian Lamparter]

Hi,

On 9/25/26 11:19 PM, Guenter Roeck wrote:
> On Fri, Sep 25, 2026 at 08:03:06PM +0200, Christian Lamparter wrote:
>> On 9/24/26 10:53 PM, Ridham Khurana wrote:
>>> temp1_input_show() drops the low bits of the temperature register by
>>> dividing the raw value (raw / 32 on the LM70). These bits are not part
>>> of the temperature: the LM70, LM71, LM74 and TMP122/TMP124 always
>>> return some of them as 1, and the TMP125 fills them with copies of the
>>> temperature LSB. Division rounds toward zero, so a negative temperature
>>> with any of these bits set is reported one LSB too high.
>>>
>>> For example, the TMP122 returns 0xffff for -0.0625 degrees C, which the
>>> driver reports as 0. The LM70 returns 0xf39f for -25 degrees C, which
>>> the driver reports as -24750.
>>>
>>> Use an arithmetic right shift instead, which drops the low bits and
>>> rounds down. tmp421 had a similar problem with negative values, fixed
>>> by commit 724e8af85854 ("hwmon: (tmp421) fix rounding for negative
>>> values").
>>
>> For the TMP125 yes:
>>
>> Reviewed-by: Christian Lamparter <chunkeey@gmail.com>
>>
>>
>>> Fixes: e1a8e913f97e ("[PATCH] lm70: New hardware monitoring driver")
>>> Fixes: a86e94dc946d ("hwmon: (lm70) Add support for LM71 and LM74")
>>> Fixes: cd929672a9ef ("hwmon: (lm70) Add ti,tmp125 support")
>>> Signed-off-by: Ridham Khurana <khurana.ridham222@gmail.com>
>>> ----
>> I wrote a little test program by hand (see below) and ran it on x64.
>> No idea if different archs behave differently or if
>> a clever unsafe math optimizing compiler flag will
>> convert the  "/ 32" to a " >> 5" but I doubt that...
>>
>> ---
>> tmp125: raw:7ec0 tmp125_proposed: -2500 tmp125_now: -2500
>> tmp125: raw:7eff tmp125_proposed: -2250 tmp125_now: -2000  <--- loss of percision
>> tmp125: raw:7f00 tmp125_proposed: -2000 tmp125_now: -2000
>> tmp125: raw:7f3f tmp125_proposed: -1750 tmp125_now: -1500  <--- more
>> tmp125: raw:7f40 tmp125_proposed: -1500 tmp125_now: -1500
>> tmp125: raw:7f7f tmp125_proposed: -1250 tmp125_now: -1000  <--- and this
>> tmp125: raw:7f80 tmp125_proposed: -1000 tmp125_now: -1000
>> tmp125: raw:7fbf tmp125_proposed:  -750 tmp125_now:  -500  <--- also bad
>> tmp125: raw:7fc0 tmp125_proposed:  -500 tmp125_now:  -500
>> tmp125: raw:7fff tmp125_proposed:  -250 tmp125_now:     0  <--- ouch
>> tmp125: raw:   0 tmp125_proposed:     0 tmp125_now:     0
>> tmp125: raw:  3f tmp125_proposed:   250 tmp125_now:   250
>> tmp125: raw:  40 tmp125_proposed:   500 tmp125_now:   500
>> tmp125: raw:  7f tmp125_proposed:   750 tmp125_now:   750
>> tmp125: raw:  80 tmp125_proposed:  1000 tmp125_now:  1000
>> tmp125: raw:  bf tmp125_proposed:  1250 tmp125_now:  1250
>> tmp125: raw:  c0 tmp125_proposed:  1500 tmp125_now:  1500
>> tmp125: raw:  ff tmp125_proposed:  1750 tmp125_now:  1750
>> tmp125: raw: 100 tmp125_proposed:  2000 tmp125_now:  2000
>> tmp125: raw: 13f tmp125_proposed:  2250 tmp125_now:  2250
>> tmp125: raw: 140 tmp125_proposed:  2500 tmp125_now:  2500
>>
> 
> Sorry, you lost me with the above. Are you suggesting that the patch is
> correct, that it is only correct for TMP125, or something else ?
> 
> Guenter

Ok, I guess the same thing happend to me now too?

Is there something specific you want to hear? My reasoning is that I wrote that
cd929672a9ef ("hwmon: (lm70) Add ti,tmp125 support")

I definitely wasn't aware that / 32 screws the with the rouding result for negative
integers and >> 5 wasn't. For unsigned compilers actually replace the divison when
the divisor is a 2^x constant on their own. You can find so many threads/topics about
that explaining why (basically pick your favourite).

https://softwareengineering.stackexchange.com/questions/209678/why-does-division-and-multiplication-by-2-use-the-shift-operator-rather-than-div
https://stackoverflow.com/a/27052650
...

It's been too long to remember what went through my head back then, but I knew that
replacing / 32 with >> 5 is a common optimization because the bitshift operation is
faster than spooling up the hardware dividers. But it would have looked funny when
all the other conversion codes (especially the LM70s. Because it's almost identical...
except the LM70 has the sign-bit at D15 and hence the raw value can be directly cast
to a s16 type). So definitly that >> 5 was on my mind.

Now, I was surprised to hear that / 32 and >> 5 differ for negative numbers and I wanted
to find out and the experiment tells me: yes, it's true.

Do you get the same result? What if you replace the function with the other
LM/TMP implementation?

But I have the suspicion, this doesn't answer your question, or does it?

Cheers,
Christian