[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240617072551acf731aa@mail.local>
Date: Mon, 17 Jun 2024 09:25:51 +0200
From: Alexandre Belloni <alexandre.belloni@...tlin.com>
To: claudiu beznea <claudiu.beznea@...on.dev>
Cc: geert+renesas@...der.be, mturquette@...libre.com, sboyd@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
lee@...nel.org, magnus.damm@...il.com,
linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-rtc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH 06/12] rtc: renesas-rtca3: Add driver for RTCA-3
available on Renesas RZ/G3S SoC
On 14/06/2024 14:06:38+0300, claudiu beznea wrote:
> >> + /*
> >> + * Stop the RTC and set to 12 hours mode and calendar count mode.
> >> + * RCR2.START initial value is undefined so we need to stop here
> >> + * all the time.
> >> + */
> >
> > Certainly not, if you stop the RTC on probe, you lose the time
> > information, this must only be done when the RTC has never been
> > initialised. The whole goal of the RTC is the keep time across reboots,
> > its lifecycle is longer than the system.
>
> This was also my first thought when I read the HW manual.
>
> It has been done like this to follow the HW manual. According to HW manual
> [1], chapter 22.3.19 RTC Control Register 2 (RCR2), initial value of START
> bit is undefined.
>
> If it's 1 while probing but it has never been initialized, we can falsely
> detect that RTC is started and skip the rest of the initialization steps.
> W/o initialization configuration, the RTC will not be able to work.
>
> Even with this implementation we don't loose the time b/w reboots. Here is
> the output on my board [2]. The steps I did were the following:
> 1/ remove the power to the board (I don't have a battery for RTC installed
> at the moment)
> 2/ boot the board and issue hwclock -w
> 3/ reboot
> 4/ check the systime and rtc time
> 5/ poweroff
> 6/ poweron
> 7/ boot and check systime and RTC time
>
> As you can see the time is not lost but continue to increment. I presume
> the hardware takes into account that time needs to increment when initial
> configuration is executed.
I don't think so, I guess it stops ticking but the registers are not
reset so when ts starts ticking again, you are not too far from the time
that it should report.
>
> [1]
> https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzg3s-general-purpose-microprocessors-single-core-arm-cortex-a55-11-ghz-cpu-and-dual-core-cortex-m33-250
> [2] https://p.fr33tux.org/585cd6
>
> >
> > Also, why do you insist on 12H-mode? The proper thing to do is to support
> > 12H-mode on read but always use 24H-mode when setting the time.
>
> OK, I wasn't aware of this. I think I followed this approach as it looked
> to me the number of operation to update the hardware registers was lower
> for 12h mode.
How come, using 12H-mode implies testing for the AM/PM bit and doing and
addition while 24H-mode would simply be a read?
> >> + priv->rtc_dev->ops = &rtca3_ops;
> >> + priv->rtc_dev->max_user_freq = 256;
> >> + priv->rtc_dev->range_min = mktime64(1999, 1, 1, 0, 0, 0);
> >> + priv->rtc_dev->range_max = mktime64(2098, 12, 31, 23, 59, 59);
> >
> > This very much looks like the range should be 2000 to 2099, why do you
> > want to shift it?
>
> 2000-2099 was my first option for this but then I saw one of your old
> commits on this topic and, since I'm not very familiar with RTC,
> I took it as example. I'll adjust as you proposed.
>
> commit beee05dfbead
> Author: Alexandre Belloni <alexandre.belloni@...tlin.com>
> Date: Wed Mar 20 12:30:10 2019 +0100
>
> rtc: sh: set range
>
> The SH RTC is a BCD RTC with some version having 4 digits for the year.
>
> The range for the RTCs with only 2 digits for the year was unfortunately
> shifted to handle 1999 to 2098.
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@...tlin.com>
This is because the sh driver predated the introduction of the range and
was already shifting it.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists