Re: [PATCH] hwmon: (lm70) Fix rounding of negative temperatures
From: Christian Lamparter
Date: Fri Sep 25 2026 - 14:03:49 EST
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@xxxxxxxxx>
Fixes: e1a8e913f97e ("[PATCH] lm70: New hardware monitoring driver")I wrote a little test program by hand (see below) and ran it on x64.
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@xxxxxxxxx>
----
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).