[PATCH] rtc: spacemit: handle regmap_test_bits() error return

[PATCH] rtc: spacemit: handle regmap_test_bits() error return

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

p1_rtc_read_time() called if (!regmap_test_bits(regmap, RTC_CTRL, RTC_EN))
to check if the RTC was enabled.

regmap_test_bits() returns 1 if the bit is set, 0 if not set, and a
negative error code (e.g. -EIO) if reading the control register fails.
Using !regmap_test_bits(...) evaluates a negative error code as boolean
false, causing I2C/regmap read failures to be ignored and incorrectly
proceeding to read time registers from a failing device.

Fix this by capturing the return value of regmap_test_bits() and returning
the error code if negative, or -EINVAL if the RTC is disabled.

Fixes: a6de182daa2b ("rtc: spacemit: support the SpacemiT P1 RTC")
Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/rtc/rtc-spacemit-p1.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-spacemit-p1.c b/drivers/rtc/rtc-spacemit-p1.c
index 43ab62494bb4..c1820c920ba3 100644
--- a/drivers/rtc/rtc-spacemit-p1.c
+++ b/drivers/rtc/rtc-spacemit-p1.c
@@ -57,8 +57,9 @@ static int p1_rtc_read_time(struct device *dev, struct rtc_time *t)
 	u8 time[6];
 	int ret;
 
-	if (!regmap_test_bits(regmap, RTC_CTRL, RTC_EN))
-		return -EINVAL;		/* RTC is disabled */
+	ret = regmap_test_bits(regmap, RTC_CTRL, RTC_EN);
+	if (ret <= 0)
+		return ret ? ret : -EINVAL;	/* RTC is disabled or error */
 
 	ret = regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
 	if (ret)
-- 
2.55.0

Re: [PATCH] rtc: spacemit: handle regmap_test_bits() error return

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The anti-tearing retry loop in `p1_rtc_read_time()` is flawed and can silently accept corrupted time readings because it only compares the seconds byte between reads.
--

--- Patch [1]: [PATCH] rtc: spacemit: handle regmap_test_bits() error return ---
commit 8fa251c335a9b68b25f44d335129e65501d1e159
Author: Surendra Singh Chouhan <kr494167@gmail.com>

rtc: spacemit: handle regmap_test_bits() error return

This commit fixes the error handling when reading the RTC control register.
It ensures that if regmap_test_bits() returns a negative error code, that
error is propagated instead of being incorrectly treated as boolean false.

> diff --git a/drivers/rtc/rtc-spacemit-p1.c b/drivers/rtc/rtc-spacemit-p1.c
> index 43ab62494bb4b..c1820c920ba3d 100644
> --- a/drivers/rtc/rtc-spacemit-p1.c
> +++ b/drivers/rtc/rtc-spacemit-p1.c
> @@ -57,8 +57,9 @@ static int p1_rtc_read_time(struct device *dev, struct rtc_time *t)
>  	u8 time[6];
>  	int ret;
>  
> -	if (!regmap_test_bits(regmap, RTC_CTRL, RTC_EN))
> -		return -EINVAL;		/* RTC is disabled */
> +	ret = regmap_test_bits(regmap, RTC_CTRL, RTC_EN);
> +	if (ret <= 0)
> +		return ret ? ret : -EINVAL;	/* RTC is disabled or error */
>  
>  	ret = regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
>  	if (ret)

[Severity: High]
This is a pre-existing issue, but does the anti-tearing retry loop in
p1_rtc_read_time() fully protect against corrupted time readings?

Looking just below this diff in p1_rtc_read_time(), the loop checks for time
tearing by only comparing the seconds byte:

	do {
		seconds = time[0];
		ret = regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
		if (ret)
			return ret;
	} while (time[0] != seconds && --count);

If a hardware time rollover occurs exactly after the seconds byte (time[0])
is read during the second regmap_bulk_read(), the read could return the
pre-rollover seconds but post-rollover minutes, hours, or days. 

Since the seconds byte matches the previous read, the loop would incorrectly
terminate and accept the torn time array. Can this lead to corrupted time
being returned to the RTC subsystem?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724024220.10113-1-kr494167@gmail.com?part=1