On 17.06.2024 10:25, Alexandre Belloni wrote: > 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. I'll double check with hardware team on this and return with an answer. Thank you for your review, Claudiu Beznea > >> >> [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. > >
