Description

In the current implementation rtc_unlock() doesn't, like the name indicates, only unlocks the rtc but also enters init mode. Looking at the datasheet this is not a really good idea. Refering to "Calendar initialization and configuration":

1. Set INIT bit to 1 in the RTC_ISR register to enter initialization mode. In this mode, the
calendar counter is stopped and its value can be updated.
(...)
5. (...) Exit the initialization mode by clearing the INIT bit. The actual calendar counter value is
then automatically loaded and the counting restarts after 4 RTCCLK clock cycles.

So every operation that calls rtc_unlock() will delay the rtc by the time the operation itself takes, plus at minimum the 4 RTCCLK cycles. This is for example true for rtc_set_alarm(). So if you happen to use the alarm feature to wake up every few seconds your time reference will become borked up pretty quickly.

The next problem comes up when using the rtc in combination with pm. This is only visible when your application makes use of power modes that issue a system reset when exiting this mode (namely Shutdown mode in my case). On reset rtc_init(), which is only conditioned by MODULE_PERIPH_RTC, is always called if the rtc is available. Since this is part of periph_init() it also doesn't help to disable auto_init.

One thing that was really nasty to debug here, came from the fact that rtc_init() isn't actually resetting the RTC_DR and RTC_TR registers. So testing if the rtc works together with pm looks "working ok" if you only observe the system over a short period of time, because the rtc_init() will effectively only loose the SSR (sub second regiser).
What do you think about using the INITS flag in RTC_ISR to only init the rtc if it wasn't already initialized before?

The fact that the current tests/periph_rtc doesn't highlight any of the problems brings me to the conclusion that the test needs improvement.

I'd like to exchange opinions on the following thoughts:
IMO It's absolutely necessary to add some quantifiable output on the accuracy. For example sntp could be used as a reference here.
This should also be tested with pm&rtc - particularly since rtc is a feature that will often be combined with low power modes.
Another thing that came to my mind: is it intentional that the rtc api doesn't provide functions to perform calibration?

Steps to reproduce the issue

taking tests/periph_rtc/ as starting point:
add USEMODULE += pm_layered to tests/periph_rtc/Makefile
add #include "pm_layered.h" to main.c
replace main() with this:

int main(void)
{
    struct tm time;
    rtc_get_time(&time);

    print_time("\nwoke up at: ", &time);

    time_t time_sec = mktime(&time);
    time_sec += 1;
    struct tm *alarm_time = gmtime(&time_sec);

    rtc_set_alarm(alarm_time, cb, NULL);

    print_time("setting alarm to: ", alarm_time);

    pm_unblock(0);
    pm_unblock(1);
    pm_unblock(2);
    pm_unblock(3);

    return 0;
}

Expected results

Accuracy near the oscillator specifications (e.g. <20 ppm on nucleo-l476 in my case).

Actual results

Accuracy dependent on application beaviour (duty cycle etc.) and for some use cases in fact abysmal.
Watching pyterms time stamps for the above code gives an error of ~220ms per minute. This translates to 3666 ppm.
Fun fact: this is more than seven times greater than the maximum this chips calibration feature could compensate, if it were implemented ;)

Versions

current master