Re: [PATCH v2] rtc: ftrtc010: fix cast

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rtc: ftrtc010: 64-bit promotion breaks negative offset wraparound
--

commit 2ccc1a04cd44cfcc28a0e0150aa880f011201f5c
Author: Alexandre Belloni <alexandre.belloni@bootlin.com>

rtc: ftrtc010: fix cast

The hour and minute registers will rollover before the multiplication to
convert to a number of seconds can exceed an 32bit integer, there is no
need to cast them.

Link: https://patch.msgid.link/20261002105946.8rdteIpO7uWGEcTlS65F8iSi4gsRoXi2kgq8FTSaDPA@z/

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> index b29c96be40f4a..c66d4a85f23c1 100644
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -72,7 +72,7 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>  	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
>  
> -	time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);
> +	time = offset + (timeu64_t)days * 86400 + hour * 3600 + min * 60 + sec;

[Severity: High]
Does this 64-bit promotion break the negative offset wraparound?

In ftrtc010_rtc_set_time(), if the target time is set backwards to a value
smaller than the elapsed hardware counter (time < hw_counter), the 64-bit
subtraction yields a negative offset that is truncated and stored in the
32-bit offset register as a two's complement value (e.g., 0xFFFFFC18).

The previous implementation explicitly performed offset + hw_counter in
32-bit arithmetic, allowing the negative offset to correctly wrap around
modulo 2^32 and yield the correct time.

By removing the outer grouping parentheses and casting days to timeu64_t,
the entire addition is promoted to 64-bit arithmetic. Because offset is an
unsigned 32-bit integer, it is zero-extended to 64-bit rather than
sign-extended, which prevents the necessary 32-bit wraparound.

As a result, the negative offset is added as a massive positive value
(e.g., 4,294,967,295), causing the RTC read time to leap about 136 years
into the future. Setting the clock backwards triggers this logic error,
resulting in corrupted time reading.

[ ... ]
> @@ -92,8 +92,7 @@ static int ftrtc010_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
>  	day = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>  
> -	offset = time - ((timeu64_t)day * 86400 + (timeu64_t)hour * 3600 +
> -			 (timeu64_t)min * 60 + (timeu64_t)sec);
> +	offset = time - ((timeu64_t)day * 86400 + hour * 3600 + min * 60 + sec);

[Severity: High]
Does setting the time to a value smaller than the elapsed hardware counter
generate a two's complement negative offset in this 32-bit unsigned register?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005111502.159855-1-alexandre.belloni@bootlin.com?part=1