[stm32] Adding Real Time Clock (RTC) for STM32G4

[rasmuskleist]

This is still work in progress, but I believe that I need a bit of help to finish it up! Currently, I intend to use RTC solely for the for getting timestamps used for logging to a flash chip. This PR should resolve some of the points discussed in #730.

I have left a lot of TODOs in code which I will try to resolve within the near future. However, I would like your input on the following points:

1. With the current implmenentation of the lbuild script for the rcc, the enable peripheral function does not enable the the APB1 peripheral clock for RTC (at least for the G4 family). The function does however enable the RTC with RCC->BDCR |= RCC_BDCR_RTCEN;. I believe that both should be enabled within this function, or what do you think?
2. Currently I have added my own struct for date/time information. I am well-aware that std::tm exists, and I am debating if I should use this or some other standard library struct. I have not implemented the driver with std::tm yet because it counts years since 1900 and weekday starting from 0. This does not align with the STM32G4 RTC which only support two digit years and where Monday = 1 and Sunday = 7. Of course, I can make the conversion, but then this would have to be carried out everywhere in the RTC code. What is your opinion on this?
3. I have implemented functions for converting to and from BCD (these should probably be unrolled). However, I am considering if these functions deserve a place in math::utils or somewhere a like? Afterall there are other drivers also converting to and from BCD in modm.

Finally, I have not yet devised a novel test for verifying that the synchronize function is actually working. From looking at the documentation I suppose it should be working, but my experience is that there is usually, some bug lurking in my untested code. Have you got any idea for such a test? My initial thought was to test if I could (approximately) zero the subsecond counter.

[chris-durand]

1. Probably yes. The current RCC implementation assumes there is only one enable bit for each peripheral.

2. Have a look at std::chrono::year_month_weekday to see if that's usable. std::chrono would be preferable to the C time API. I'd like to have a standard API for the RTC independent of the hardware representation. I'm curious what using the standard calendar APIs does to the flash size. There are big things it could accidentally pull in, like the timezone data, if we are unlucky. You'd have to try to see if it's practical or not.

3. Yes, that's a good idea. You can put them into the math:utils module and ideally add a unit test in modm/test/modm/math/utils.

[rasmuskleist]

It occured to me that RCC_BDCR_RTCEN is being set in both Rcc::enableRealTimeClock and in Rcc::enable with the upstream build script. I have thus changed the build script to generate code that sets RCC_APB1ENR1_RTCAPBEN in the Rcc::enable function. I have done this by not overwriting entries in the rcc_enable dict. I am unware if this causese unwanted behaviour for other devices than STM32G4, but I do find it vulnerable that the code generated is dependent on the sorting of rcc_map or what do you think?

I am also debating if the Rtc class should take the asynchronous and synchronous prescalars as template paramters. This will allow me to define using resolution = std::ratio<1, Synchronous + 1> and using duration = std::duration<int32_t, resolution> inside Rtc, which will be convenient for conversions for example in synchronize and simplify the code in other places. I also think it will possibly be able to statically assert that the RTC clock frequency is less than the APB1 clock frequency and use if consexpr in other places involving the RTC clock frequency. What do you think?

I pressume that the ideal interface for the Rtc would be similar to std::chrono::steady_clock, basically that it implements now which returns std::chrono::time_point with epoch being the zero of the RTC? Here std::time_point could use std::chrono::seconds, std::chrono::milliseconds or resolution from above, each presenting their own problems. For my own use case, I need better precision than seconds, but with a uint32_t the milliseconds will wrap around in 49.7 days. Meanwhile, the seconds will wrap around in 136 years, which is more than enough for the RTC. The resolution will have the best attainable precision and depending on the choice of synchronous prescaler an intermediate range. Specifically, if PREDIV_S = 0xFF (default) the range is approximately half a year. What are your thoughts on this?

For initialization something I think std::chrono::year_month_day could be suiteable suitable, without knowning the exact details. Presumably, time since midnight would then be passed on with std::chrono::seconds?

[salkinium]

I have done this by not overwriting entries in the rcc_enable dict.

Seems fine. Honestly the whole remap thing is quite a hack anyways…

I am also debating if the Rtc class should take the asynchronous and synchronous prescalars as template paramters.

No, try to stick to the initialize<SystemClock>() convention.

I presume that the ideal interface for the Rtc would be similar to std::chrono::steady_clock

Yes, but I think the system_clock may fit better.
I would use int64_t, as this is also used in std::chrono, and makes it possible to use all the library functions directly. The uint32_t modm::Clock is just optimizations for the Timers.

For initialization something I think std::chrono::year_month_day

Maybe? Would be nice not to have to do calendar calculations in software when it's already done in hardware, but dunno if the hardware supports that.

[salkinium]

best to change your editor to add a newline at the end of file, otherwise there'll be a lot of these changes in the modm code base ;-P

[salkinium]

You would typically pass the SystemClock as a template (function) parameter here to figure out which bus the RTC is clocked from, and then use the modm::Prescaler static methods (or a custom constexpr computation that you can also unit test separately) to compute your prescalers for the right running rate (1Hz?) and assert their required tolerance.

This architecture enables that you can switch to different SlowSystemClock::enable() configurations at runtime and call the initialize<SlowSystemClock, ...>() functions without too much code duplication. At least in theory, I've only had prototypes of this working, using different clocks is only useful for power saving, which modm… doesn't support anyways.

[salkinium]

You would write this information to a bool member in initialize() and query it here. it's a bit inelegant, but kinda how we do things here.

[salkinium]

Should there perhaps also be a setDateTime, if the hardware support that? Would be useful for stuff like proper FAT file time etc.

[salkinium]

maybe only provide the system_clock::now() interface instead of this function? Hm, at least not to float ;-P

[salkinium]

what does this do?

[rasmuskleist]

Thanks for the nice comments! I have tried to resolve most of the issues and will solve the remaining hopefully this week. In resolving these issues I have encountered the following problems that I would like your take on:

1. Currently, I have also given initialize a third template parameter namely RTCCLK. This can vary significantly depending on the clock source used for the RTC. Typically a 32.768 kHz LSE, will be used, but HSE can also be used. Therefore I think the most flexible way of getting RTCCLK into initialize is to pass it manually. I choose to pass it as a template parameter, so I can statically assert that it is not larger than the APB1 frequency.
2. How to calculate prescalers? The RTC is somewhat different from the other peripherals like SysTickTimer because it has two prescalers. These should be chosen to generate ck_spre of 1 Hz (usually). Hence there can be multiple good choices of prescalers dependning on the objective. I suggest the objective will be to choose PREDIV_A as large as possible (default) and choose PREDIV_S accordingly such that PREDIV_A * PREDIV_S = RTCCLK. This decreases the RTC dynamic consumption as much as possible. However, this is a small optimization problem, which may be difficult to solve. Do you have other suggestions to the choice of prescalers?
3. According to the documentation (RM0440 35.3.8 RTC initialization and configuration) the prescalers are set followed by calender initialization - both must be done when in initialization mode. Furthermore, it is my understanding that the RTC counter is the calender? Therefore it is difficult for me to see how you will avoid doing the calender calculations in software. That is unless you somehow setup an interrupt to occur on every ck_spre cycle similar to SysTickTime.
4. In case of a power cycle it is critical that the members ck_apre and ck_spre are re-initialized on startup. However, in that case the calender and prescalers will most likely not have to be initialized again. For my own application this is mostly in the unlikely event of a short power loss. Therefore it may make sense to implement multiple initialize functions for example setDateTime? However, this is not a major concern from my part at least with this iteration.
5. As you write I should probably only implement now which returns a time_point. However, the value of the time_point should be calculated from the value of RTC_SSR, RTC_TR and RTC_DR?

Thanks in advance for your help!

[salkinium]

Currently, I have also given initialize a third template parameter namely RTCCLK.

No, that informatio should come from SystemClock, which should contain the full clock configuration.
There should be a member SystemClock::Rtcclk with the right frequency or whatever is the most canonical form.
If the HSE is used, then the SystemClock needs to reflect that (and SystemClock::enable() needs to configure that).

Do you have other suggestions to the choice of prescalers?

Prescaler A is only 7-bit so you can linearly iterate 1-128 and find a suitable prescaler S that fits the tolerance. Maybe a prescaler s ≥1000 ticks so that you get millisecond resolution? You can do it at compile time with constexpr, then it's easy to code and costs almost nothing in compile time and you can unit test it separately.
Perhaps add the algorithm to the Prescaler class, it may be used by other RTCs or at least be similar.

Perhaps you should pass a frequency parameter so that people can specify what subsecond frequency they require.
That would probably be the cleanest abstraction, as it also allows for "weird" frequencies to work correctly.
So Rtc::initialize<Board::SystemClock, 1_kHz>(); for the prescaler s ≥1000.

Therefore it is difficult for me to see how you will avoid doing the calender calculations in software.

Ok, that's fine, just may have been nice.

However, in that case the calender and prescalers will most likely not have to be initialized again.

modm does not yet have the concept for brown out detection. Just reinitialize everything.

As you write I should probably only implement now which returns a time_point. However, the value of the time_point should be calculated from the value of RTC_SSR, RTC_TR and RTC_DR

Ugh, I was hoping there'd just be a uint32_t unix timestamp in there.
Maybe there is a good third party library for converting between the unix timestamp since 1970 and RTC?
Otherwise this might not be worth the effort, you might as well use the modm::Clock then…