Re: [PATCH v3 03/12] rtc: rzn1: fix weekday underflow when alarm crosses month boundary

Hi Wolfram,

Thank you for the review.

On Tue, Aug 18, 2026 at 11:10 AM Wolfram Sang
<[email protected]> wrote:
>
> On Mon, Jul 06, 2026 at 06:51:29PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > In rzn1_rtc_set_alarm(), the driver attempts to calculate the weekday
> > for an alarm by computing the day delta between the alarm time and the
> > current time:
> >
> >     days_ahead = tm->tm_mday - tm_now.tm_mday;
> >     wday = (tm_now.tm_wday + days_ahead) % 7;
> >
> > However, if an alarm is scheduled for the beginning of the next month
> > while the current time is at the end of the month (e.g., current day is
> > 31, alarm day is 1), `tm->tm_mday - tm_now.tm_mday` results in a negative
> > value (-30). Since `days_ahead` is an unsigned int, this underflows to a
> > large positive number, leading to an incorrect `wday` being written to
> > the RZN1_RTC_ALW register. As a result, the alarm fails to fire.
> >
> > Fix this by utilizing the already computed `alarm` time64_t timestamp.
> > Convert it back into an rtc_time struct via rtc_time64_to_tm(), which
> > automatically handles month boundaries and correctly populates the
> > `tm_wday` field.
> >
> > Fixes: b5ad1bf00d2c4 ("rtc: rzn1: Add alarm support")
> > Cc: [email protected]
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> These messages look a bit LLM-generated. Is this true? What about the
> code?
>
Yes the commit message was fine tuned by an LLM, but not the code.

> > -     days_ahead = tm->tm_mday - tm_now.tm_mday;
> > -     wday = (tm_now.tm_wday + days_ahead) % 7;
> > +     rtc_time64_to_tm(alarm, &alarm_tm);
>
> This conversion is superfluous, we already have 'tm'? So, I think we can
> skip the whole conversion block here and use?
>
>         writel(BIT(tm->tm_wday), rtc->base + RZN1_RTC_ALW);
>
> Only lightly tested, please double check.
>
Yes it does the trick. I tested with the below script:

#!/bin/sh
set -x
DEV=/dev/rtc0

for d in "2026-08-31 23:58:00" "2026-04-30 23:58:00" "2028-02-28
23:58:00" "2027-02-27 23:58:00"; do
    echo "=== Testing boundary at: $d ==="
    hwclock --set --date="$d" -f $DEV
    hwclock -f $DEV --hctosys

    # -m mem/standby would actually suspend the board, which is disruptive
    # in an automated test loop. Instead, block on the alarm IRQ directly
    # via select(2) on the rtc fd -- this is what confirms the alarm
    # actually asserts, not just that it was armed.
    timeout 150 rtcwake -m on -d $DEV -s 130
    echo "exit: $?"
done

Without the fix I get timeouts as expected.

Cheers,
Prabhakar