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@xxxxxxxxxxx> > 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@xxxxxxxxx> > Signed-off-by: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> 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