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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The RTC time read retry loop in `p1_rtc_read_time` is vulnerable to a torn read (time corruption) if a time roll-over occurs during the register read.
--

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

rtc: spacemit: handle regmap_test_bits() error return

p1_rtc_read_time() called if (!regmap_test_bits(regmap, RTC_CTRL, RTC_EN))
to check if the RTC was enabled. This evaluates negative error codes as
boolean false, causing read failures to be ignored. Fix this by capturing
the return value of regmap_test_bits() and returning the error code.

> diff --git a/drivers/rtc/rtc-spacemit-p1.c b/drivers/rtc/rtc-spacemit-p1.c
> index 43ab62494bb4b..1de7bd995d296 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 ?: -EINVAL;	/* RTC is disabled or error */
>  
>  	ret = regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
>  	if (ret)
>  		return ret;
>  
>  	do {
>  		seconds = time[0];
>  		ret = regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
>  		if (ret)
>  			return ret;
>  	} while (time[0] != seconds && --count);

[Severity: High]
This is a pre-existing issue, but does this loop correctly detect time
roll-overs that happen during the read?

If a time roll-over occurs during the second regmap_bulk_read() in
p1_rtc_read_time(), specifically after reading time[0] (seconds) but before
reading higher units, time[0] will match the seconds from the previous read.

This would cause the loop to terminate early and accept the newly read time
array.

Could this return a torn time from the RTC (for example, 14:00:59 instead of
13:59:59 or 14:00:00)?

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