Re: [PATCH v2 2/2] rtc: mt6397: Fix mt6357 RTC year offset handling for hwclock commands

[Alexandre Mergnat]

On 02/04/2025 15:03, AngeloGioacchino Del Regno wrote:
> Il 02/04/25 12:51, Alexandre Mergnat ha scritto:
> > The mt6357 RTC was failing when using the `hwclock -r --verbose` command,
> > despite reading correctly through sysfs. There is high chance for other
> > platform to be impacted by the year offset handling issue.
> >
> > The hardware RTC registers store years relative to 1968, but the driver
> > wasn't consistently applying the offset when converting between
> > hardware and Linux time representation.
> >
> > This inconsistency caused alarm rollover failures during device
> > registration, with the error "alarm rollover not handled -22" in the
> > logs, causing hwclock commands to fail.
> >
> > The ioctl interface used by the hwclock command requires proper time
> > range validation that wasn't happening with the inconsistent year
> > offsets.
> >
> > Fixes the issue by applying the year offset in all operations:
> >     - Adding (RTC_MIN_YEAR - RTC_BASE_YEAR) when reading from hardware
> >     - Subtracting the same offset when writing to hardware
> >     - Using the same logic for both regular time and alarm operations
> >
> > With these changes, the hwclock command works correctly and time
> > values are consistently handled across all interfaces.
> >
> > Signed-off-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> > ---
> >   drivers/rtc/rtc-mt6397.c | 13 ++++++++-----
> >   1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index 692c00ff544b2..ba52e225dc8fa 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -77,7 +77,8 @@ static int __mtk_rtc_read_time(struct mt6397_rtc *rtc,
> >       tm->tm_mday = data[RTC_OFFSET_DOM];
> >       tm->tm_wday = data[RTC_OFFSET_DOW];
> >       tm->tm_mon = data[RTC_OFFSET_MTH] & RTC_TC_MTH_MASK;
> > -    tm->tm_year = data[RTC_OFFSET_YEAR];
> > +    /* The RTC registers store years since 1968 (hardware's base year) */
> > +    tm->tm_year = data[RTC_OFFSET_YEAR] + (RTC_MIN_YEAR - RTC_BASE_YEAR);
>
> This patch received a NACK because of RTC_MIN_YEAR_OFFSET.
>
> What you're doing here is avoiding to use the "RTC_MIN_YEAR_OFFSET" definition name
> but otherwise doing the very same thing that was NACKed before.

You're right sorry. In my mind, the rtc framework was working well, then I try-hard to
fix the issue in this driver... but I was wrong. :(
The RTC framework have issues so fixes should be in the framework directly. My next suggestion:
https://lore.kernel.org/r/20250109-enable-rtc-v3-0-f003e8144419@xxxxxxxxxxxx


--
Regards,
Alexandre