Re: [PATCH] hwmon: (lm70) Fix rounding of negative temperatures
From: Christian Lamparter
Date: Sun Sep 27 2026 - 07:10:46 EST
On 9/27/26 8:31 AM, David Laight wrote:
> > But you'll also get flagged because C doesn't define right shifts
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...
A compiler will convert an unsigned divide to a shift, but can't
do so for signed divides.
for negative values.
Hmm... For at least x86, any compiler should see that the is an integer type
and use the Shift Arithmetic Right (SAR) instead of the SHR instruction. From
what I know that SAR instructions is very old, like 8086 with improvements
some major improvements on the 286... And linux started with the 386 so, this
should work even into the past. (But please correct if this is wrong).
I can't tell what clang would do, but GCC says this (second last bullet point):
<https://gcc.gnu.org/onlinedocs/gcc/Integers-implementation.html>
|Bitwise operators act on the representation of the value including both
|the sign and value bits, where the sign bit is considered immediately above
|the highest-value value bit. Signed ‘>>’ acts on negative numbers by sign extension.
|
|As an extension to the C language, GCC does not use the latitude given in C99 and
|later to treat certain aspects of signed ‘<<’ as undefined.
|However, -fsanitize=shift (and -fsanitize=undefined) will diagnose such cases.
|They are also diagnosed where constant expressions are required.
So, the << (shift left) could indeed be a problem/undefined.
Cheers,
Christian