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

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 se=
conds byte between reads.
--

--- Patch [1]: [PATCH] rtc: spacemit: handle regmap_test_bits() error retur=
n ---
commit 8fa251c335a9b68b25f44d335129e65501d1e159
Author: Surendra Singh Chouhan <[email protected]>

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;
> =20
> -	if (!regmap_test_bits(regmap, RTC_CTRL, RTC_EN))
> -		return -EINVAL;		/* RTC is disabled */
> +	ret =3D regmap_test_bits(regmap, RTC_CTRL, RTC_EN);
> +	if (ret <=3D 0)
> +		return ret ? ret : -EINVAL;	/* RTC is disabled or error */
> =20
>  	ret =3D 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 =3D time[0];
		ret =3D regmap_bulk_read(regmap, RTC_TIME, time, sizeof(time));
		if (ret)
			return ret;
	} while (time[0] !=3D 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.=20

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?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260724024220.1011=
[email protected]?part=3D1