Hi Alexandre Belloni,
> Subject: Re: [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument
>
> Hello,
>
> On 06/07/2022 13:07:56+0100, Biju Das wrote:
> > This patch fixes the issue RTC_RD_TIME: Invalid argument with the
> > below use case.
> >
> > Steps to reproduce:
> > ------------------
> > date -s "2022-12-31 23:59:55";hwclock -w hwclock; sleep 10; hwclock
> >
> > Original result:
> > ---------------
> > Sat Dec 31 23:59:55 UTC 2022
> > Sat Dec 31 23:59:56 2022 0.000000 seconds
> > hwclock: RTC_RD_TIME: Invalid argument
> >
> > Result after the fix:
> > --------------------
> > Sat Dec 31 23:59:55 UTC 2022
> > Sat Dec 31 23:59:56 2022 0.000000 seconds Sun Jan 1 00:00:06 2023
> > 0.000000 seconds
> >
> > Fixes: deeb4b5393e1 ("rtc: rzn1: Add new RTC driver")
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > This patch fix is based on [1]
> > [1]
> > ---
> > drivers/rtc/rtc-rzn1.c | 47
> > ++++++++++++++++++++++++++++--------------
> > 1 file changed, 32 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c index
> > ac788799c8e3..0f99b4fd3c4b 100644
> > --- a/drivers/rtc/rtc-rzn1.c
> > +++ b/drivers/rtc/rtc-rzn1.c
> > @@ -88,6 +88,35 @@ static unsigned int rzn1_rtc_tm_to_wday(struct
> rtc_time *tm)
> > return (days + 4) % 7;
> > }
> >
> > +static void bcd2tm(struct rtc_time *tm) {
> > + tm->tm_sec = bcd2bin(tm->tm_sec);
> > + tm->tm_min = bcd2bin(tm->tm_min);
> > + tm->tm_hour = bcd2bin(tm->tm_hour);
> > + tm->tm_wday = bcd2bin(tm->tm_wday);
> > + tm->tm_mday = bcd2bin(tm->tm_mday);
> > + tm->tm_mon = bcd2bin(tm->tm_mon) - 1;
>
> I guess this is the actual fix, I'm not sure creating the bcd2tm and
> tm2bcd functions is useful, it obfuscates more than it helps.
>
>
> > + /* epoch == 1900 */
> > + tm->tm_year = bcd2bin(tm->tm_year) + 100;
>
> Is it really the epoch of the RTC?
I guess so When I debugged last time, With the current code,
HW registers are entering into a wrong state where day =32, when it reaches 23:59:59
and month and year are not propagated to next level.
With the patch I submitted, it never entered to this wrong state.
>
> > +}
> > +
> > +static int tm2bcd(struct rtc_time *tm) {
> > + if (rtc_valid_tm(tm) != 0)
> > + return -EINVAL;
> > +
>
> This will never fail, I'm not sure why you need to check here.
Currently I don't have a board to recheck this.
Cheers,
Biju
>
> > + tm->tm_sec = bin2bcd(tm->tm_sec);
> > + tm->tm_min = bin2bcd(tm->tm_min);
> > + tm->tm_hour = bin2bcd(tm->tm_hour);
> > + tm->tm_wday = bin2bcd(rzn1_rtc_tm_to_wday(tm));
> > + tm->tm_mday = bin2bcd(tm->tm_mday);
> > + tm->tm_mon = bin2bcd(tm->tm_mon + 1);
> > + /* epoch == 1900 */
> > + tm->tm_year = bin2bcd(tm->tm_year - 100);
> > +
> > + return 0;
> > +}
> > +
> > static int rzn1_rtc_read_time(struct device *dev, struct rtc_time
> > *tm) {
> > struct rzn1_rtc *rtc = dev_get_drvdata(dev); @@ -106,14 +135,7 @@
> > static int rzn1_rtc_read_time(struct device *dev, struct rtc_time *tm)
> > if (tm->tm_sec != secs)
> > rzn1_rtc_get_time_snapshot(rtc, tm);
> >
> > - tm->tm_sec = bcd2bin(tm->tm_sec);
> > - tm->tm_min = bcd2bin(tm->tm_min);
> > - tm->tm_hour = bcd2bin(tm->tm_hour);
> > - tm->tm_wday = bcd2bin(tm->tm_wday);
> > - tm->tm_mday = bcd2bin(tm->tm_mday);
> > - tm->tm_mon = bcd2bin(tm->tm_mon);
> > - tm->tm_year = bcd2bin(tm->tm_year);
> > -
> > + bcd2tm(tm);
> > return 0;
> > }
> >
> > @@ -123,13 +145,8 @@ static int rzn1_rtc_set_time(struct device *dev,
> struct rtc_time *tm)
> > u32 val;
> > int ret;
> >
> > - tm->tm_sec = bin2bcd(tm->tm_sec);
> > - tm->tm_min = bin2bcd(tm->tm_min);
> > - tm->tm_hour = bin2bcd(tm->tm_hour);
> > - tm->tm_wday = bin2bcd(rzn1_rtc_tm_to_wday(tm));
> > - tm->tm_mday = bin2bcd(tm->tm_mday);
> > - tm->tm_mon = bin2bcd(tm->tm_mon);
> > - tm->tm_year = bin2bcd(tm->tm_year);
> > + if (tm2bcd(tm) < 0)
> > + return -EINVAL;
> >
> > val = readl(rtc->base + RZN1_RTC_CTL2);
> > if (!(val & RZN1_RTC_CTL2_STOPPED)) {
> > --
> > 2.25.1
> >
>
> --
