Re: [PATCH v3 4/5] rtc: mt6397: Remove start time parameters

[Uwe Kleine-König]

Hello Alex,

On Mon, Apr 14, 2025 at 03:56:11PM +0200, Alexandre Mergnat wrote:
> On 14/04/2025 13:09, AngeloGioacchino Del Regno wrote:
> > Il 11/04/25 15:39, Alexandre Belloni ha scritto:
> > > On 11/04/2025 15:36:12+0200, Alexandre Belloni wrote:
> > > > On 11/04/2025 14:35:57+0200, Alexandre Mergnat wrote:
> > > > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> > > > > index 692c00ff544b2..d47626d47602f 100644
> > > > > --- a/drivers/rtc/rtc-mt6397.c
> > > > > +++ b/drivers/rtc/rtc-mt6397.c
> > > > > @@ -291,8 +291,6 @@ static int mtk_rtc_probe(struct platform_device *pdev)
> > > > >       rtc->rtc_dev->ops = &mtk_rtc_ops;
> > > > >       rtc->rtc_dev->range_min = RTC_TIMESTAMP_BEGIN_1900;
> > > > >       rtc->rtc_dev->range_max = mktime64(2027, 12, 31, 23, 59, 59);
> > > > > -    rtc->rtc_dev->start_secs = mktime64(1968, 1, 2, 0, 0, 0);
> > > > > -    rtc->rtc_dev->set_start_time = true;
> > > > 
> > > > This is going to break the time for people upgrading their kernel, you
> > > > are unfortunately stuck with this.
> > > 
> > > To be clear, the breakage will happen when upgrading the kernel but not
> > > the device tree with 5/5
> >
> > Yes, you're stuck with this. Devicetree has to be retrocompatible.
> > 
> > Besides, this start_secs is what gets used by default, and the start-year
> > devicetree property should take precedence and effectively override the
> > start_secs default.
> > 
> > Just keep it there.... :-)

It would work to keep setting start_secs but allow overwriting that
value in the device tree. But see below.
 
> When you boot your board for the first time, is the date January 2nd 1968 ?
> If not, that mean it is used as a finetune offset year.
> IMHO, mktime64(1968, 1, 2, 0, 0, 0) is a workaround for the rtc framework
> issue we try to solve in this serie because start_secs is negative (1968 <
> 1970). Now framework handle the negative value properly, even if you keep
> mktime64(1968, 1, 2, 0, 0, 0) , the device time will change. I prefer to
> notify you.  :)

I don't understand everything you wrote here, but as far as I see it,
rtc_time64_to_tm() not being able to handle dates before 1970 is the
main issue here. This is of course only relevant, because your hardware
occasionally contains such a date. The technically right fix is to
extend rtc_time64_to_tm() to work for dates >= 1900-01-01. (An
alternative would be to assume that a hardware read returning a date
before 1970 is invalid. If you refuse to write dates before 1970 that
should give a consistent behaviour. But the original approach is the
nicer one.)

> TBH, it's hard to follow the logic, so I've a question:
> If I push in my V4 a framework fix that drivers using year < 1970 will need
> to have a new start_secs or start-year value to stay aligned with there
> previous value, do you will accept it ?

Doesn't the need to shift the start year simply goes away once
rtc_time64_to_tm() is fixed for negative time values?

So I would expect that going forward with just patches #1 and #2 should
result in a fixed driver regarding the breakage you're seeing. (I'm
unsure about patch #3, I'll address that in a reply to the respective
mail.)

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature