LM77: correction of "register to

PATCH New Jean Delvare 1 participant 1 message Dec 20, 2005 Hardware monitoring

lore

Share & tools

lore

Jean Delvare

author

2005-12-20 19:52 UTC #0 lore raw

PATCH 1/1 LM77: correction of "register to

Hi Michael,

Quoted 8 lines

Temperature conversion in lm77.c is currently broken for negative temperatures, as discovered in
http://list.voyage.hk/pipermail/voyage-linux/2005-December/000540.html
National uses the upper 4 bits of the temperature registers to represent the sign (all these are either set or unset at the same time), while the lower 3 bits are either unused or represent the alarm status (depending on which of the temperature registers you look at).

You seem to think that using more than one bit for sign is odd from National. This is actually the standard way of doing though. Think about it. If you store signed values on 8 bit, you'll be able to store values from -128 to +127. If for some reason, you need the range of values to be shorter than that, say, -32 to +31, then all valid negative values will start with three "1" bits, and all valid positive values will start with three "0" bits. It's the simple consequence of the range limitation, not a design decision.

Quoted 10 lines

The "register to temperature" conversion doesn't properly consider these sign bits. In the result they are used as part of the absolute temperature value, so that negative temperatures are incorrectly converted to huge positive values instead.
(Side note: the "temperature to register" conversion works fine with positive and negative values.)
The attached patch (which is against kernel 2.6.14.4) solves this issue. It has been tested against all the sample values given in the LM77 datasheet (section 1.4) to proof it is working correctly.

Your patch is probably correct but also needlessly complex. All we really have here is a unsigned vs. signed issue. What about this more simple fix instead?

Fix negative temperature readings in lm77 driver.

Signed-off-by Jean Delvare <khali at linux-fr.org>

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

drivers/hwmon/lm77.c +4 -4

@@ -87,15 +87,15 @@
 
 /* In the temperature registers, the low 3 bits are not part of the
    temperature values; they are the status bits. */
-static inline u16 LM77_TEMP_TO_REG(int temp)
+static inline s16 LM77_TEMP_TO_REG(int temp)
 {
 	int ntemp = SENSORS_LIMIT(temp, LM77_TEMP_MIN, LM77_TEMP_MAX);
-	return (u16)((ntemp / 500) * 8);
+	return (ntemp / 500) * 8;
 }
 
-static inline int LM77_TEMP_FROM_REG(u16 reg)
+static inline int LM77_TEMP_FROM_REG(s16 reg)
 {
-	return ((int)reg / 8) * 500;
+	return (reg / 8) * 500;
 }
 
 /* sysfs stuff */

Signature · 1 line

Jean Delvare