Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/sam0_common/periph: fix rtt reset after hibernation #20842

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

mariemC
Copy link
Contributor

@mariemC mariemC commented Aug 28, 2024

Contribution description

This PR fixes the issue with rtt reset after a node hibernation
You may notice an issue when using the rtt_rtc module in any RIOT OS example. Specifically, if you use the rtc_gettime() function to get the current time, then set an alarm, put the node into hibernation, and finally retrieve the time again after the node wakes up, you will find that the time is incorrect after hibernation.

I encountered this issue while working with the gcoap example on the same54-xpro board. After adding the rtt_rtc module, I followed the steps mentioned above and observed that the time returned by rtc_gettime() after waking up from hibernation did not match the expected value.

Testing procedure

The output before the fix, you can notice the time after hibernation is wrong

rtc gettime
2024-08-28 17:48:10,774 # rtc gettime
2024-08-28 17:48:10,779 # in _rtc_now: now 151613 and last_alarm 0 and rtc_now 0
2024-08-28 17:48:10,780 # tmp 4s
2024-08-28 17:48:10,782 # 2020-01-01 00:00:04
> rtc setalarm 2020-01-01 00:01:04
2024-08-28 17:48:25,766 # rtc setalarm 2020-01-01 00:01:04
2024-08-28 17:48:25,767 # alarm_time 64
2024-08-28 17:48:25,772 # in _rtc_now: now 642893 and last_alarm 0 and rtc_now 0
2024-08-28 17:48:25,774 # rtc_now in set alarm 19s
2024-08-28 17:48:25,776 # RTT_SECONDS 32768
2024-08-28 17:48:25,777 # RTT_MAX_VALUE -1
2024-08-28 17:48:25,781 # last_alarm in update alarm 622592s and rtc_now 19
2024-08-28 17:48:25,783 # next alarm is 45
> pm set 1
2024-08-28 17:48:31,014 # pm set 1
2024-08-28 17:48:31,017 # CPU is entering power mode 1.
2024-08-28 17:48:31,020 # Now waiting for a wakeup event...
2024-08-28 17:49:10,789 # main(): This is RIOT! (Version: 2024.10-devel-132-gb05606)
2024-08-28 17:49:10,791 # gcoap example app
2024-08-28 17:49:10,793 # All up, running the shell now
rtc gettime
2024-08-28 17:49:15,846 # rtc gettime
2024-08-28 17:49:15,851 # in _rtc_now: now 165913 and last_alarm 622592 and rtc_now 19
2024-08-28 17:49:15,852 # tmp 131077s
2024-08-28 17:49:15,854 # 2020-01-02 12:24:37

This is the output after the fix:

rtc gettime
2024-08-28 18:14:47,512 # rtc gettime
2024-08-28 18:14:47,517 # in _rtc_now: now 83033 and last_alarm 0 and rtc_now 0
2024-08-28 18:14:47,517 # tmp 2s
2024-08-28 18:14:47,519 # 2020-01-01 00:00:02
rtc setalarm 2020-01-01 00:01:00
2024-08-28 18:14:55,528 # rtc setalarm 2020-01-01 00:01:00
2024-08-28 18:14:55,530 # alarm_time 60
2024-08-28 18:14:55,534 # in _rtc_now: now 345737 and last_alarm 0 and rtc_now 0
2024-08-28 18:14:55,536 # rtc_now in set alarm 10s
2024-08-28 18:14:55,538 # RTT_SECONDS 32768
2024-08-28 18:14:55,540 # RTT_MAX_VALUE -1
2024-08-28 18:14:55,544 # last_alarm in update alarm 327680s and rtc_now 10
2024-08-28 18:14:55,545 # next alarm is 50
> pm set 1
2024-08-28 18:15:01,015 # pm set 1
2024-08-28 18:15:01,018 # CPU is entering power mode 1.
2024-08-28 18:15:01,021 # Now waiting for a wakeup event...
2024-08-28 18:15:45,552 # main(): This is RIOT! (Version: 2024.10-devel-132-gb05606)
2024-08-28 18:15:45,553 # gcoap example app
2024-08-28 18:15:45,556 # All up, running the shell now
> rtc gettime
2024-08-28 18:15:51,254 # rtc gettime
2024-08-28 18:15:51,260 # in _rtc_now: now 2171904 and last_alarm 327680 and rtc_now 10
2024-08-28 18:15:51,260 # tmp 66s
2024-08-28 18:15:51,262 # 2020-01-01 00:01:06

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Aug 28, 2024
@dylad
Copy link
Member

dylad commented Aug 29, 2024

I'm a bit confuse here. I understand the issue and what the fix is trying to do but you're fixing the RTT part of the driver while you use the RTC part.

So far I was not able to reproduce the issue on SAME54 with tests/periph/pm.

@mariemC mariemC force-pushed the fix/rtt_reset_after_hibernation branch from d8037a6 to 6cc37b7 Compare August 29, 2024 08:05
@mariemC
Copy link
Contributor Author

mariemC commented Aug 29, 2024

I'm a bit confuse here. I understand the issue and what the fix is trying to do but you're fixing the RTT part of the driver while you use the RTC part.

So far I was not able to reproduce the issue on SAME54 with tests/periph/pm.

The issue is on rtt, so in order to see it you need to include the module rtt_rtc. I believe the tests/periph/pm is using periph_rtc?

Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the right module :)

@dylad dylad added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 29, 2024
@riot-ci
Copy link

riot-ci commented Aug 29, 2024

Murdock results

✔️ PASSED

7df7886 cpu/sam0_common/periph: fix rtt reset after hibernation

Success Failures Total Runtime
10192 0 10193 14m:52s

Artifacts

@dylad
Copy link
Member

dylad commented Aug 29, 2024

I think this PR needs a rebase so static-test will be happy.

@mariemC mariemC force-pushed the fix/rtt_reset_after_hibernation branch from 6cc37b7 to 7df7886 Compare August 29, 2024 15:09
@dylad
Copy link
Member

dylad commented Aug 29, 2024

Here we go !

@dylad dylad added this pull request to the merge queue Aug 29, 2024
Merged via the queue into RIOT-OS:master with commit 481b580 Aug 29, 2024
25 checks passed
@dylad
Copy link
Member

dylad commented Aug 29, 2024

Thanks for the fix !

@benpicco benpicco added this to the Release 2024.10 milestone Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants