diff mbox series

[2/3] hwmon: (tmp513) Fix Current Register value interpretation

Message ID 20241216173648.526-3-m.masimov@maxima.ru (mailing list archive)
State Accepted
Headers show
Series hwmon: (tmp513) Fix interpretation of values of TMP513 registers | expand

Commit Message

Murad Masimov Dec. 16, 2024, 5:36 p.m. UTC
The value returned by the driver after processing the contents of the Current
Register does not correspond to the TMP512/TMP513 specifications. A raw
register value is converted to a signed integer value by a sign extension in
accordance with the algorithm provided in the specification, but due to the
off-by-one error in the sign bit index, the result is incorrect. Moreover,
negative values will be reported as large positive due to missing sign
extension from u32 to long.

According to the TMP512 and TMP513 datasheets, the Current Register (07h) is a
16-bit two's complement integer value. E.g., if regval = 1000 0011 0000 0000,
then the value must be (-32000 * lsb), but the driver will return (33536 * lsb).

Fix off-by-one bug, and also cast data->curr_lsb_ua (which is of type u32) to
long to prevent incorrect cast for negative values.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 59dfa75e5d82 ("hwmon: Add driver for Texas Instruments TMP512/513 sensor chips.")
Signed-off-by: Murad Masimov <m.masimov@maxima.ru>
---
 drivers/hwmon/tmp513.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.39.2

Comments

Guenter Roeck Dec. 16, 2024, 11:55 p.m. UTC | #1
On Mon, Dec 16, 2024 at 08:36:47PM +0300, Murad Masimov wrote:
> The value returned by the driver after processing the contents of the Current
> Register does not correspond to the TMP512/TMP513 specifications. A raw
> register value is converted to a signed integer value by a sign extension in
> accordance with the algorithm provided in the specification, but due to the
> off-by-one error in the sign bit index, the result is incorrect. Moreover,
> negative values will be reported as large positive due to missing sign
> extension from u32 to long.
> 
> According to the TMP512 and TMP513 datasheets, the Current Register (07h) is a
> 16-bit two's complement integer value. E.g., if regval = 1000 0011 0000 0000,
> then the value must be (-32000 * lsb), but the driver will return (33536 * lsb).
> 
> Fix off-by-one bug, and also cast data->curr_lsb_ua (which is of type u32) to
> long to prevent incorrect cast for negative values.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 59dfa75e5d82 ("hwmon: Add driver for Texas Instruments TMP512/513 sensor chips.")
> Signed-off-by: Murad Masimov <m.masimov@maxima.ru>

Applied, after fixing checkpatch complaints.

Guenter
diff mbox series

Patch

diff --git a/drivers/hwmon/tmp513.c b/drivers/hwmon/tmp513.c
index d401cb55de14..dacce7417bfd 100644
--- a/drivers/hwmon/tmp513.c
+++ b/drivers/hwmon/tmp513.c
@@ -222,7 +222,7 @@  static int tmp51x_get_value(struct tmp51x_data *data, u8 reg, u8 pos,
 		break;
 	case TMP51X_BUS_CURRENT_RESULT:
 		// Current = (ShuntVoltage * CalibrationRegister) / 4096
-		*val = sign_extend32(regval, 16) * data->curr_lsb_ua;
+		*val = sign_extend32(regval, 15) * (long)data->curr_lsb_ua;
 		*val = DIV_ROUND_CLOSEST(*val, MILLI);
 		break;
 	case TMP51X_LOCAL_TEMP_RESULT: