[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).
>