Re: [PATCH 2/2] rtc: mt6359: fix year issue

[AngeloGioacchino Del Regno]

Il 09/01/25 16:46, Alexandre Belloni ha scritto:
> On 09/01/2025 16:29:52+0100, Alexandre Mergnat wrote:
> > Removing the RTC_MIN_YEAR_OFFSET addition and subtraction has
> > introduced a regression.
> >
> > ~# hwclock -r --verbose
> > hwclock from util-linux 2.37.4
> > System Time: 1262312013.143552
> > Trying to open: /dev/rtc0
> > Using the rtc interface to the clock.
> > Assuming hardware clock is kept in UTC time.
> > Waiting for clock tick...
> > hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> > ...synchronization failed
> >
> > Bring back the RTC_MIN_YEAR_OFFSET to fix the RTC.
>
> NAK, you'd have to investigate a bit more, I want to get rid of the
> RTC_MIN_YEAR_OFFSET insanity.

If literally all currently supported MediaTek PMICs RTC are working fine (and
it's not just one), but the one that is introduced here has issues, clearly
the problem is *not* about the min_year_offset not being there, I'd say!

Btw:
"hwclock: select() to /dev/rtc0 to wait for clock tick timed out"
...is the WRTGR write failing? :-)

And no, Alexandre M, don't trust the regmap_read_poll_timeout() to return an
error, I'm not sure that the CBUSY gets set to zero for *literally all* failures...

In particular.... some RTCs are *locked* and need to be *unlocked*, and in that
case I'm not sure if the write just goes through but gets ignored or if CBUSY
stays set.

Anyway, check the RTC_PROT register for the unlocking mechanism :-)

Cheers,
Angelo

> > Fixes: 34bbdc12d04e ("rtc: mt6359: Add RTC hardware range and add support for start-year")
> > Signed-off-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> > ---
> >   drivers/rtc/rtc-mt6397.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > index 55e75712edd4..9930b6bdb6ca 100644
> > --- a/drivers/rtc/rtc-mt6397.c
> > +++ b/drivers/rtc/rtc-mt6397.c
> > @@ -96,6 +96,12 @@ static int mtk_rtc_read_time(struct device *dev, struct rtc_time *tm)
> >   			goto exit;
> >   	} while (sec < tm->tm_sec);
> >   
> > +	/* HW register use 7 bits to store year data, minus
> > +	 * RTC_MIN_YEAR_OFFSET before write year data to register, and plus
> > +	 * RTC_MIN_YEAR_OFFSET back after read year from register
> > +	 */
> > +	tm->tm_year += RTC_MIN_YEAR_OFFSET;
> > +
> >   	/* HW register start mon/wday from one, but tm_mon/tm_wday start from zero. */
> >   	tm->tm_mon--;
> >   	tm->tm_wday--;
> > @@ -110,6 +116,7 @@ static int mtk_rtc_set_time(struct device *dev, struct rtc_time *tm)
> >   	int ret;
> >   	u16 data[RTC_OFFSET_COUNT];
> >   
> > +	tm->tm_year -= RTC_MIN_YEAR_OFFSET;
> >   	tm->tm_mon++;
> >   	tm->tm_wday++;
> >   
> > @@ -167,6 +174,7 @@ static int mtk_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
> >   	tm->tm_mon = data[RTC_OFFSET_MTH] & RTC_AL_MTH_MASK;
> >   	tm->tm_year = data[RTC_OFFSET_YEAR] & RTC_AL_YEA_MASK;
> >   
> > +	tm->tm_year += RTC_MIN_YEAR_OFFSET;
> >   	tm->tm_mon--;
> >   
> >   	return 0;
> > @@ -182,6 +190,7 @@ static int mtk_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
> >   	int ret;
> >   	u16 data[RTC_OFFSET_COUNT];
> >   
> > +	tm->tm_year -= RTC_MIN_YEAR_OFFSET;
> >   	tm->tm_mon++;
> >   
> >   	mutex_lock(&rtc->lock);
> >
> > --
> > 2.25.1