cpu/sam0_common: RTC: ensure RTC alarm alignment for SAM D2x #16078

benpicco · 2021-02-23T15:51:43Z

Contribution description

There is an inconsistency with the behavior of the RTC on the sam0 family.
The RTC alarm on SAM D1x/2x is triggered at the end of the alarm second, on SAM D5x/E5x it's triggered at the start of the set alarm second.

The later behavior is closer to what one would expect IMHO.

The Datasheet read

When an alarm match occurs, the Alarm 0 Interrupt flag in the Interrupt Flag Status and Clear registers (INTFLAG.ALARMn0) is set on the next 0-to-1 transition of CLK_RTC_CNT. E.g. For a 1Hz clock counter, it means the Alarm 0 Interrupt flag is set with a delay of 1s after the occurrence of alarm match.

for all members, but I could not observe this behavior on same54-xpro, but only on samd10-xmini and samr21-xpro.

I modified tests/periph_rtc to take the current RTC time into account when setting the alarm instead of just incrementing the alarm time by 2.
This reveals the inconsistent behavior (interval between alarms is now 3s instead of 2s without the RTC patch).

Testing procedure

Run tests/periph_rtc on all supported familys

SAM D1x/D2x
SAM L1x
SAM L2x
SAM D5x/E5x

It would be interesting to know if other CPUs have this problem too, the RTC test has been pretty lax about the second alignment so far.

Issues/PRs references

discovered in #15949

Due to the nature of the RTC, the alarm will only trigger at the *end* of the set alarm second. This introduces a 1s offset to the alarm time. Compensate for this by subtracting 1s from the alarm time.

kaspar030 · 2021-02-23T15:54:43Z

The RTC alarm on SAM D1x/2x is triggered at the end of the alarm second, on SAM D5x/E5x it's triggered at the start of the set alarm second.

nice catch...

fjmolinas · 2021-02-23T16:34:31Z

Sadly I do not have any of the missing families to test, but I'm pretty sure @dylad does :D

benpicco · 2021-02-23T16:38:01Z

But IIRC you have a nice STM32 collection - do those agree on when to generate the alarm?

fjmolinas · 2021-02-23T16:40:07Z

But IIRC you have a nice STM32 collection - do those agree on when to generate the alarm?

I'm running this PR on them ATM

fjmolinas · 2021-02-23T16:46:35Z

I think this inconsistencies are showing up elsewhere as well like the clearalarm test, on some boards:

2021-02-23 17:43:05,548 # RIOT RTC low-level driver test
2021-02-23 17:43:05,554 # This test will display 'Alarm!' every 2 seconds for 4 times
2021-02-23 17:43:05,559 #   Setting clock to   2020-02-28 23:59:57
2021-02-23 17:43:05,560 # Clock value is now   2020-02-28 23:59:57
2021-02-23 17:43:05,565 #   Setting alarm to   2020-02-28 23:59:59
2021-02-23 17:43:05,567 #    Alarm is set to   2020-02-28 23:59:59
2021-02-23 17:43:05,572 #   Alarm cleared at   2020-02-28 23:59:57
2021-02-23 17:43:07,588 #        No alarm at   2020-02-28 23:59:58
2021-02-23 17:43:07,594 #   Setting alarm to   2020-02-29 00:00:00

fjmolinas · 2021-02-23T16:47:46Z

I think this inconsistencies are showing up elsewhere as well like the clearalarm test, on some boards:

2021-02-23 17:43:05,548 # RIOT RTC low-level driver test
2021-02-23 17:43:05,554 # This test will display 'Alarm!' every 2 seconds for 4 times
2021-02-23 17:43:05,559 #   Setting clock to   2020-02-28 23:59:57
2021-02-23 17:43:05,560 # Clock value is now   2020-02-28 23:59:57
2021-02-23 17:43:05,565 #   Setting alarm to   2020-02-28 23:59:59
2021-02-23 17:43:05,567 #    Alarm is set to   2020-02-28 23:59:59
2021-02-23 17:43:05,572 #   Alarm cleared at   2020-02-28 23:59:57
2021-02-23 17:43:07,588 #        No alarm at   2020-02-28 23:59:58
2021-02-23 17:43:07,594 #   Setting alarm to   2020-02-29 00:00:00

Hmm no this seems to be just jitter.

fjmolinas · 2021-02-23T18:04:41Z

The test is passing In most cases for the BOARD's I have, issues arise because of #15628 on a couple of boards, sometimes I get the following:

RIOT RTC low-level driver test
This test will display 'Alarm!' every 2 seconds for 4 times
  Setting clock to   2020-02-28 23:59:57
Clock value is now   2020-02-28 23:59:57
  Setting alarm to   2020-02-28 23:59:59
   Alarm is set to   2020-02-28 23:59:58

Traceback (most recent call last):
  File "/builds/workspace/RIOT.git.jk/tests/periph_rtc/tests/01-run.py", line 45, in <module>
    sys.exit(run(testfunc))
  File "/builds/workspace/RIOT.git.jk/dist/pythonlibs/testrunner/__init__.py", line 30, in run
    testfunc(child)
  File "/builds/workspace/RIOT.git.jk/tests/periph_rtc/tests/01-run.py", line 32, in testfunc
    assert alarm_set == alarm_value

dylad · 2021-02-24T08:37:51Z

I'll run the test on SAMLxx by the end of the week.

dylad · 2021-02-25T18:12:05Z

Test results:

SAML11

2021-02-25 19:03:44,320 # Help: Press s to start test, r to print it is ready
s
2021-02-25 19:03:46,746 # START
2021-02-25 19:03:46,754 # main(): This is RIOT! (Version: 2021.04-devel-725-g91f45-cpu/sam0_common-rtc_alarm_alignment)
2021-02-25 19:03:46,754 # 
2021-02-25 19:03:46,757 # RIOT RTC low-level driver test
2021-02-25 19:03:46,762 # This test will display 'Alarm!' every 2 seconds for 4 times
2021-02-25 19:03:46,780 #   Setting clock to   2020-02-28 23:59:57
2021-02-25 19:03:46,787 # Clock value is now   2020-02-28 23:59:57
2021-02-25 19:03:46,791 #   Setting alarm to   2020-02-28 23:59:59
2021-02-25 19:03:46,798 #    Alarm is set to   2020-02-28 23:59:59
2021-02-25 19:03:46,802 #   Alarm cleared at   2020-02-28 23:59:57
2021-02-25 19:03:48,821 #        No alarm at   2020-02-28 23:59:59
2021-02-25 19:03:48,828 #   Setting alarm to   2020-02-29 00:00:01
2021-02-25 19:03:48,828 # 
2021-02-25 19:03:51,773 # Alarm!
2021-02-25 19:03:53,773 # Alarm!
2021-02-25 19:03:55,773 # Alarm!
2021-02-25 19:03:57,773 # Alarm!

SAML10


2021-02-25 19:09:34,242 # START
2021-02-25 19:09:34,250 # main(): This is RIOT! (Version: 2021.04-devel-725-g91f45-cpu/sam0_common-rtc_alarm_alignment)
2021-02-25 19:09:34,250 # 
2021-02-25 19:09:34,253 # RIOT RTC low-level driver test
2021-02-25 19:09:34,259 # This test will display 'Alarm!' every 2 seconds for 4 times
2021-02-25 19:09:34,276 #   Setting clock to   2020-02-28 23:59:57
2021-02-25 19:09:34,282 # Clock value is now   2020-02-28 23:59:57
2021-02-25 19:09:34,287 #   Setting alarm to   2020-02-28 23:59:59
2021-02-25 19:09:34,293 #    Alarm is set to   2020-02-28 23:59:59
2021-02-25 19:09:34,298 #   Alarm cleared at   2020-02-28 23:59:57
2021-02-25 19:09:36,320 #        No alarm at   2020-02-28 23:59:59
2021-02-25 19:09:36,328 #   Setting alarm to   2020-02-29 00:00:01
2021-02-25 19:09:36,329 # 
2021-02-25 19:09:39,269 # Alarm!
2021-02-25 19:09:41,269 # Alarm!
2021-02-25 19:09:43,269 # Alarm!
2021-02-25 19:09:45,269 # Alarm!

SAML21

2021-02-25 19:11:13,508 # main(): This is RIOT! (Version: 2021.04-devel-725-g91f45-cpu/sam0_common-rtc_alarm_alignment)
2021-02-25 19:11:13,508 # 
2021-02-25 19:11:13,510 # RIOT RTC low-level driver test
2021-02-25 19:11:13,516 # This test will display 'Alarm!' every 2 seconds for 4 times
2021-02-25 19:11:13,535 #   Setting clock to   2020-02-28 23:59:57
2021-02-25 19:11:13,536 # Clock value is now   2020-02-28 23:59:57
2021-02-25 19:11:13,539 #   Setting alarm to   2020-02-28 23:59:59
2021-02-25 19:11:13,546 #    Alarm is set to   2020-02-28 23:59:59
2021-02-25 19:11:13,548 #   Alarm cleared at   2020-02-28 23:59:57
2021-02-25 19:11:15,565 #        No alarm at   2020-02-28 23:59:59
2021-02-25 19:11:15,571 #   Setting alarm to   2020-02-29 00:00:01
2021-02-25 19:11:15,571 # 
2021-02-25 19:11:18,526 # Alarm!
2021-02-25 19:11:20,526 # Alarm!
2021-02-25 19:11:22,526 # Alarm!
2021-02-25 19:11:24,526 # Alarm!

This looks good to me.

cpu/sam0_common/periph/rtc_rtt.c

maribu · 2021-04-20T13:47:47Z

I don't think this needs to fully retested. Please squash right away. I can retest with a my wemos-zero, which has a SAMD21 MCU. The code change doesn't effect other MCUs anyway.

cpu/sam0_common/periph/rtc_rtt.c

…M D2x

benpicco · 2021-04-25T22:04:35Z

I extended rtc_tm_normalize() to also handle negative values.
I wonder a bit if it makes sense to split the function, so there is a 'simple' version that only deals with positive overflow (as used by rtt_rtc), this would save us about 112 byte in that case.

But if we always want to guarantee that struct tm might as well contain negative elements, we can keep it in the common implementation.

MrKevinWeiss · 2021-06-28T12:59:26Z

tested on the samr21-xpro

BOARD=samr21-xpro make test -C tests/periph_rtc
make: Entering directory '/home/weiss/wd/RIOT/tests/periph_rtc'
r
/home/weiss/wd/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" --no-reconnect --noprefix --no-repeat-command-on-empty-line 
Twisted not available, please install it if you want to use pyterm's JSON capabilities
Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
READY
s
START
main(): This is RIOT! (Version: 2021.04-devel-728-g386e4e-cpu/sam0_common-rtc_alarm_alignment)

RIOT RTC low-level driver test
This test will display 'Alarm!' every 2 seconds for 4 times
  Setting clock to   2020-02-28 23:59:57
Clock value is now   2020-02-28 23:59:57
  Setting alarm to   2020-02-28 23:59:59
   Alarm is set to   2020-02-28 23:59:59
  Alarm cleared at   2020-02-28 23:59:57
       No alarm at   2020-02-28 23:59:59
  Setting alarm to   2020-02-29 00:00:00

Alarm!
Alarm!
Alarm!
Alarm!

saml10

BOARD=saml10-xpro make flash test -C tests/periph_rtc
make: Entering directory '/home/weiss/wd/RIOT/tests/periph_rtc'
Building application "tests_periph_rtc" for "saml10-xpro" with MCU "saml1x".

"make" -C /home/weiss/wd/RIOT/boards/saml10-xpro
"make" -C /home/weiss/wd/RIOT/boards/common/saml1x
"make" -C /home/weiss/wd/RIOT/core
"make" -C /home/weiss/wd/RIOT/cpu/saml1x
"make" -C /home/weiss/wd/RIOT/cpu/cortexm_common
"make" -C /home/weiss/wd/RIOT/cpu/cortexm_common/periph
"make" -C /home/weiss/wd/RIOT/cpu/sam0_common
"make" -C /home/weiss/wd/RIOT/cpu/sam0_common/periph
"make" -C /home/weiss/wd/RIOT/cpu/saml1x/periph
"make" -C /home/weiss/wd/RIOT/drivers
"make" -C /home/weiss/wd/RIOT/drivers/periph_common
"make" -C /home/weiss/wd/RIOT/sys
"make" -C /home/weiss/wd/RIOT/sys/auto_init
"make" -C /home/weiss/wd/RIOT/sys/div
"make" -C /home/weiss/wd/RIOT/sys/isrpipe
"make" -C /home/weiss/wd/RIOT/sys/malloc_thread_safe
"make" -C /home/weiss/wd/RIOT/sys/newlib_syscalls_default
"make" -C /home/weiss/wd/RIOT/sys/pm_layered
"make" -C /home/weiss/wd/RIOT/sys/stdio_uart
"make" -C /home/weiss/wd/RIOT/sys/test_utils/interactive_sync
"make" -C /home/weiss/wd/RIOT/sys/tsrb
"make" -C /home/weiss/wd/RIOT/sys/xtimer
   text	   data	    bss	    dec	    hex	filename
  15476	    136	   2416	  18028	   466c	/home/weiss/wd/RIOT/tests/periph_rtc/bin/saml10-xpro/tests_periph_rtc.elf
[INFO] edbg binary not found - building it from source now
CC= CFLAGS= make -C /home/weiss/wd/RIOT/dist/tools/edbg
[INFO] updating edbg /home/weiss/wd/RIOT/dist/tools/edbg/bin/.pkg-state.git-downloaded
echo 99d15460fcff723f73b16c29c8ca14bff4b33b20 > /home/weiss/wd/RIOT/dist/tools/edbg/bin/.pkg-state.git-downloaded
[INFO] patch edbg
[INFO] edbg binary successfully built!
/home/weiss/wd/RIOT/dist/tools/edbg/edbg.sh flash /home/weiss/wd/RIOT/tests/periph_rtc/bin/saml10-xpro/tests_periph_rtc.bin
### Flashing Target ###
Debugger: ATMEL EDBG CMSIS-DAP ATML2769041800000967 03.25.01B6 (S)
Clock frequency: 16.0 MHz
Target: SAM L10E16A (Rev B)
Verification...
at address 0x48 expected 0x11, read 0x91
Error: verification failed
Debugger: ATMEL EDBG CMSIS-DAP ATML2769041800000967 03.25.01B6 (S)
Clock frequency: 16.0 MHz
Target: SAM L10E16A (Rev B)
Programming................................................................ done.
Verification................................................................ done.
Done flashing
r
/home/weiss/wd/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" --no-reconnect --noprefix --no-repeat-command-on-empty-line 
Twisted not available, please install it if you want to use pyterm's JSON capabilities
Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
READY
s
START
main(): This is RIOT! (Version: 2021.04-devel-728-g386e4e-cpu/sam0_common-rtc_alarm_alignment)

RIOT RTC low-level driver test
This test will display 'Alarm!' every 2 seconds for 4 times
  Setting clock to   2020-02-28 23:59:57
Clock value is now   2020-02-28 23:59:57
  Setting alarm to   2020-02-28 23:59:59
   Alarm is set to   2020-02-28 23:59:59
  Alarm cleared at   2020-02-28 23:59:57
       No alarm at   2020-02-28 23:59:59
  Setting alarm to   2020-02-29 00:00:01

Alarm!
Alarm!
Alarm!
Alarm!
make: Leaving directory '/home/weiss/wd/RIOT/tests/periph_rtc'

Can you fix the murdock errors (I think some insufficient memory issue for examples/default for samd10-xmin?

benpicco added 2 commits February 23, 2021 16:42

cpu/sam0_common: RTC: ensure RTC alarm alignment for SAM D2x

e60e04a

Due to the nature of the RTC, the alarm will only trigger at the *end* of the set alarm second. This introduces a 1s offset to the alarm time. Compensate for this by subtracting 1s from the alarm time.

tests/periph_rtc: calculate alarm time from current time

91f45fa

benpicco requested a review from MrKevinWeiss as a code owner February 23, 2021 15:51

benpicco requested review from fjmolinas and dylad February 23, 2021 15:51

benpicco added Area: cpu Area: CPU/MCU ports Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 23, 2021

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 25, 2021

benpicco requested a review from jue89 February 28, 2021 14:34

benpicco requested a review from maribu April 19, 2021 08:35

maribu reviewed Apr 20, 2021

View reviewed changes

cpu/sam0_common/periph/rtc_rtt.c Show resolved Hide resolved

fixup! cpu/sam0_common: RTC: ensure RTC alarm alignment for SAM D2x

a10e230

benpicco requested review from aabadie, keestux, leandrolanzieri, MichelRottleuthner and smlng as code owners April 20, 2021 11:33

maribu reviewed Apr 20, 2021

View reviewed changes

cpu/sam0_common/periph/rtc_rtt.c Show resolved Hide resolved

maribu added the Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines label Apr 20, 2021

benpicco commented Apr 20, 2021

View reviewed changes

cpu/sam0_common/periph/rtc_rtt.c Show resolved Hide resolved

benpicco added 2 commits April 25, 2021 23:58

drivers/periph_common: Handle negative values in rtc_tm_normalize()

51c24a3

fixup! fixup! cpu/sam0_common: RTC: ensure RTC alarm alignment for SA…

386e4ec

…M D2x

benpicco requested a review from miri64 as a code owner April 25, 2021 22:00

MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cpu/sam0_common: RTC: ensure RTC alarm alignment for SAM D2x #16078

cpu/sam0_common: RTC: ensure RTC alarm alignment for SAM D2x #16078

benpicco commented Feb 23, 2021 •

edited

Loading

kaspar030 commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

benpicco commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

fjmolinas commented Feb 23, 2021 •

edited

Loading

dylad commented Feb 24, 2021

dylad commented Feb 25, 2021

maribu commented Apr 20, 2021

benpicco commented Apr 25, 2021

MrKevinWeiss commented Jun 28, 2021 •

edited

Loading

cpu/sam0_common: RTC: ensure RTC alarm alignment for SAM D2x #16078

Are you sure you want to change the base?

cpu/sam0_common: RTC: ensure RTC alarm alignment for SAM D2x #16078

Conversation

benpicco commented Feb 23, 2021 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

kaspar030 commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

benpicco commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

fjmolinas commented Feb 23, 2021

fjmolinas commented Feb 23, 2021 • edited Loading

dylad commented Feb 24, 2021

dylad commented Feb 25, 2021

maribu commented Apr 20, 2021

benpicco commented Apr 25, 2021

MrKevinWeiss commented Jun 28, 2021 • edited Loading

benpicco commented Feb 23, 2021 •

edited

Loading

fjmolinas commented Feb 23, 2021 •

edited

Loading

MrKevinWeiss commented Jun 28, 2021 •

edited

Loading