Re: [PATCH] rtc: ab-eoz9: fix negative temperature readings

Re: [PATCH] rtc: ab-eoz9: fix negative temperature readings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] rtc: ab-eoz9: truncated opening sentence in commit message
--

commit 672667a84ca874a5ecdaf588ad6693378fe916e0
Author: Yuehong Wu <wuyuehong@cmss.chinamobile.com>

rtc: ab-eoz9: fix negative temperature readings

This commit casts the unsigned raw register reading in
abeoz9z3_temp_read() to int before adding ABEOZ953_TEMP_MIN so negative
temperatures do not wrap around as unsigned integers.

> (-60 degrees C), so the driver computes "raw + ABEOZ953_TEMP_MIN".

[Severity: Low]
The opening line of the commit message is truncated, likely because a
preceding line starting with "#" (such as #define ABEOZ953_TEMP_MIN) was
stripped as a comment by git commit.  Can the missing opening clause be
restored?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-rtc-ab-eoz9-neg-temp-1-yuehongwu@cmss.chinamobile.com?part=1

[PATCH] rtc: ab-eoz9: fix negative temperature readings

[Yuehong Wu]

(-60 degrees C), so the driver computes "raw + ABEOZ953_TEMP_MIN".
regmap_read() hands the raw value back in an unsigned int, which makes
that addition - and the multiplication by 1000 that follows - unsigned as
well: -60 becomes 0xffffffc4 and 1000 * (raw - 60) wraps around for every
raw value below 60.

On 64-bit systems the wrapped unsigned int is then zero-extended into the
long hwmon attribute, so every temperature below 0 degrees C is reported
as a huge positive value: raw 0 (the -60 degrees C code) becomes
4294907296 instead of -60000, raw 30 (-30 degrees C) becomes 4294937296
instead of -30000.  This also contradicts temp1_min, which the same
driver reports as -60000, and consumers that only look for negative
values (thermal governors, monitoring daemons) do not discard the bogus
value.

Perform the arithmetic on a signed value.

Fixes: 67075b63cce2 ("rtc: add AB-RTCMC-32.768kHz-EOZ9 RTC support")
Cc: <stable@vger.kernel.org>
Signed-off-by: Yuehong Wu <wuyuehong@cmss.chinamobile.com>
---
The two expressions were checked with a small user space model of the
same types: raw 0 gives 4294907296 with the old expression and -60000
with this fix, raw 30 gives 4294937296 and -30000.

Compile-tested with gcc and W=1 (CONFIG_RTC_DRV_ABEOZ9=m); there is no
AB-RTCMC-32.768kHz-EOZ9 hardware here, so this was not exercised on a
device.

 drivers/rtc/rtc-ab-eoz9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
--- a/drivers/rtc/rtc-ab-eoz9.c
+++ b/drivers/rtc/rtc-ab-eoz9.c
@@ -399,7 +399,7 @@ static int abeoz9z3_temp_read(struct device *dev,
 		ret = regmap_read(regmap, ABEOZ9_REG_REG_TEMP, &val);
 		if (ret < 0)
 			return ret;
-		*temp = 1000 * (val + ABEOZ953_TEMP_MIN);
+		*temp = 1000 * ((int)val + ABEOZ953_TEMP_MIN);
 		return 0;
 	case hwmon_temp_max:
 		*temp = 1000 * ABEOZ953_TEMP_MAX;

Re: [PATCH v2] rtc: ab-eoz9: fix negative temperature readings

[sashiko-bot]

> The temperature register is coded with an offset of ABEOZ953_TEMP_MIN
> (-60 degrees C), so the driver computes "raw + ABEOZ953_TEMP_MIN".
> regmap_read() hands the raw value back in an unsigned int, which makes
> that addition - and the multiplication by 1000 that follows - unsigned as
> well: -60 becomes 0xffffffc4 and 1000 * (raw - 60) wraps around for every
> raw value below 60.
> 
> On 64-bit systems the wrapped unsigned int is then zero-extended into the
> long hwmon attribute, so every temperature below 0 degrees C is reported
> as a huge positive value: raw 0 (the -60 degrees C code) becomes
> 4294907296 instead of -60000, raw 30 (-30 degrees C) becomes 4294937296
> instead of -30000.  This also contradicts temp1_min, which the same
> driver reports as -60000, and consumers that only look for negative
> values (thermal governors, monitoring daemons) do not discard the bogus
> value.
> 
> Perform the arithmetic on a signed value.
> 
> Fixes: 67075b63cce2 ("rtc: add AB-RTCMC-32.768kHz-EOZ9 RTC support")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Yuehong Wu <wuyuehong@cmss.chinamobile.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-rtc-ab-eoz9-neg-temp-v2-1-yuehongwu@cmss.chinamobile.com?part=1

[PATCH v2] rtc: ab-eoz9: fix negative temperature readings

[Yuehong Wu]

The temperature register is coded with an offset of ABEOZ953_TEMP_MIN
(-60 degrees C), so the driver computes "raw + ABEOZ953_TEMP_MIN".
regmap_read() hands the raw value back in an unsigned int, which makes
that addition - and the multiplication by 1000 that follows - unsigned as
well: -60 becomes 0xffffffc4 and 1000 * (raw - 60) wraps around for every
raw value below 60.

On 64-bit systems the wrapped unsigned int is then zero-extended into the
long hwmon attribute, so every temperature below 0 degrees C is reported
as a huge positive value: raw 0 (the -60 degrees C code) becomes
4294907296 instead of -60000, raw 30 (-30 degrees C) becomes 4294937296
instead of -30000.  This also contradicts temp1_min, which the same
driver reports as -60000, and consumers that only look for negative
values (thermal governors, monitoring daemons) do not discard the bogus
value.

Perform the arithmetic on a signed value.

Fixes: 67075b63cce2 ("rtc: add AB-RTCMC-32.768kHz-EOZ9 RTC support")
Cc: <stable@vger.kernel.org>
Signed-off-by: Yuehong Wu <wuyuehong@cmss.chinamobile.com>
---
v2: no code change; restore the first line of the commit message, which
was lost when v1 was converted to plain text.

The two expressions were checked with a small user space model of the
same types: raw 0 gives 4294907296 with the old expression and -60000
with this fix, raw 30 gives 4294937296 and -30000.

Compile-tested with gcc and W=1 (CONFIG_RTC_DRV_ABEOZ9=m); there is no
AB-RTCMC-32.768kHz-EOZ9 hardware here, so this was not exercised on a
device.

 drivers/rtc/rtc-ab-eoz9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ab-eoz9.c b/drivers/rtc/rtc-ab-eoz9.c
--- a/drivers/rtc/rtc-ab-eoz9.c
+++ b/drivers/rtc/rtc-ab-eoz9.c
@@ -399,7 +399,7 @@ static int abeoz9z3_temp_read(struct device *dev,
 		ret = regmap_read(regmap, ABEOZ9_REG_REG_TEMP, &val);
 		if (ret < 0)
 			return ret;
-		*temp = 1000 * (val + ABEOZ953_TEMP_MIN);
+		*temp = 1000 * ((int)val + ABEOZ953_TEMP_MIN);
 		return 0;
 	case hwmon_temp_max:
 		*temp = 1000 * ABEOZ953_TEMP_MAX;