cpu: RTC implementation for STM32F1 #11258

Former · 2019-03-25T08:52:19Z

Implement real time clock for stm32f1.

Tested on bluepill board.
Use tests: /tests/periph_rtc and /tests/periph_pm
Alarm can wake up board with command "unblock_rtc 0 30" after command "off" or "set 0".
set_alarm work fine

maribu

Thanks for your pull request. There are some things that need to be addressed. I did only check the fundamental stuff and didn't read the datasheet/documentation required for a full review. So there could be more issues that need to be worked out that I didn't spot during the brief look I had at this PR. I will look into this in more detail when the current issues are addressed.

cpu/stm32_common/periph/rtc.c

boards/common/stm32f103c8/Makefile.features

cpu/stm32_common/periph/rtc.c

maribu · 2019-03-27T10:16:30Z

Oh, I just noticed that the commit messages do not have the format <area>: <summary>. E.g. the could be something like these:

cpu: RTC implementation for STM32F1
boards/bluepill: FEATURES_PROVIDED += periph_rtc

cpu/stm32_common/periph/rtc.c

maribu · 2019-04-09T08:41:50Z

Tested on the blue pill using examples/default

Setting/getting current time works as expected
Setting an alarm in the future does work, the callback is executed at the correct point in time (message The alarm rang is displayed)
Setting an alarm in the future and clearing it before it is triggered does work, the callback is not executed
Setting an alarm in the past does work. The documentation is not clear if this should be an error or not. (rtc_set_alarm() should return -2 for an invalid time passed as struct tm, but not what to do on a valid date in the past.)

cpu/stm32_common/periph/rtc.c

fjmolinas · 2019-05-22T12:37:28Z

@Former

Since rtt.c and rtc.c both use the same underlying RTC in STM32F1 I think you should add:

FEATURES_CONFLICT += periph_rtc:periph_rtt
FEATURES_CONFLICT_MSG += "On the STM32F1, the RTC and RTT map to the same hardware peripheral."

under cpu/stm32f1/Makefile.features

fjmolinas · 2019-05-22T12:38:38Z

Tested on iotlab-m3, it works except in sleep modes where the rtc alarm FAILS to wake up the device.

fjmolinas · 2019-09-19T06:34:40Z

Tested on bluepill board.
Use tests: /tests/periph_rtc and /tests/periph_pm
Alarm can wake up board with command "unblock_rtc 0 30" after command "off" or "set 0".
set_alarm work fine

I'm able to wake up from stand-by:

2019-09-19 08:30:02,963 - INFO # help
2019-09-19 08:30:02,966 - INFO # Command              Description
2019-09-19 08:30:02,969 - INFO # ---------------------------------------
2019-09-19 08:30:02,972 - INFO # off                  turn off
2019-09-19 08:30:02,975 - INFO # reboot               reboot
2019-09-19 08:30:02,978 - INFO # block                block power mode
2019-09-19 08:30:02,981 - INFO # set                  set power mode
2019-09-19 08:30:02,985 - INFO # unblock              unblock power mode
2019-09-19 08:30:02,989 - INFO # unblock_rtc          temporary unblock power mode
> unblock 0
2019-09-19 08:30:06,854 - INFO #  unblock 0
2019-09-19 08:30:06,856 - INFO # Unblocking power mode 0.
> unblock 1
2019-09-19 08:30:09,006 - INFO #  unblock 1
2019-09-19 08:30:09,008 - INFO # Unblocking power mode 1.
> unblock_rtc 1 3
2019-09-19 08:30:14,429 - INFO #  unblock_rtc 1 3
2019-09-19 08:30:14,432 - INFO # Unblocking power mode 1 for 3 seconds.
> help2019-09-19 08:30:18,961 - INFO #  �main(): This is RIOT! (Version: 2019.10-devel-588-g3043a-pr-11258)
2019-09-19 08:30:18,966 - INFO # This application allows you to test the CPU power management.
2019-09-19 08:30:18,972 - INFO # The available power modes are 0 - 1. Lower-numbered power modes
2019-09-19 08:30:18,977 - INFO # save more power, but may require an event/interrupt to wake up
2019-09-19 08:30:18,980 - INFO # the CPU. Reset the CPU if needed.

I'm unable to wake up form stop-mode:

make: Entering directory '/builds/tmp/RIOT/tests/periph_pm'
/builds/tmp/RIOT/dist/tools/pyterm/pyterm -p "/dev/riot/tty-nucleo-f103rb" -b "115200" 
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2019-09-19 08:29:15,103 - INFO # Connect to serial port /dev/riot/tty-nucleo-f103rb
helpWelcome to pyterm!
Type '/exit' to exit.
help
2019-09-19 08:29:16,529 - INFO # help
2019-09-19 08:29:16,532 - INFO # Command              Description
2019-09-19 08:29:16,536 - INFO # ---------------------------------------
2019-09-19 08:29:16,538 - INFO # off                  turn off
2019-09-19 08:29:16,541 - INFO # reboot               reboot
2019-09-19 08:29:16,544 - INFO # block                block power mode
2019-09-19 08:29:16,548 - INFO # set                  set power mode
2019-09-19 08:29:16,551 - INFO # unblock              unblock power mode
2019-09-19 08:29:16,556 - INFO # unblock_rtc          temporary unblock power mode
> unblock 1
2019-09-19 08:29:21,751 - INFO #  unblock 1
2019-09-19 08:29:21,753 - INFO # Unblocking power mode 1.
> unblock_rtc 1 3
2019-09-19 08:29:26,963 - INFO #  unblock_rtc 1 3
2019-09-19 08:29:26,966 - INFO # Unblocking power mode 1 for 3 seconds.
> help
help
help
help
help
help

EDUT: this has nothing to since we are not using LPTIM here
I didn't work for me when rebasing on #11286 either.

fjmolinas · 2019-09-19T06:38:34Z

Might it be missing the EXTI configuration?

benpicco · 2019-11-28T17:14:09Z

This needs a rebase.
I think this PR should be good, stop-mode might be a separate issue.

Former · 2019-11-28T18:54:33Z

I rebase branch and fix conflicts.

benpicco

Looks good overall, will give it a whirl on the blackpill.

boards/common/arduino-atmega/Makefile.features

cpu/stm32f1/periph/rtc.c

benpicco · 2019-11-28T21:50:01Z

When I run tests/periph_rtc I only see one alarm:

2019-11-28 22:49:22,690 # main(): This is RIOT! (Version: 2020.01-devel-1083-g37dc6)
2019-11-28 22:49:22,691 # 
2019-11-28 22:49:22,693 # RIOT RTC low-level driver test
2019-11-28 22:49:22,698 # This test will display 'Alarm!' every 2 seconds for 4 times
2019-11-28 22:49:22,702 #   Setting clock to   2011-12-13 14:15:57
2019-11-28 22:49:22,705 # Clock value is now   2011-12-13 14:15:57
2019-11-28 22:49:22,709 #   Setting alarm to   2011-12-13 14:15:59
2019-11-28 22:49:22,713 #    Alarm is set to   2011-12-13 14:15:59
2019-11-28 22:49:22,714 # 
2019-11-28 22:49:25,703 # Alarm!

benpicco · 2019-12-05T15:46:44Z

Murdock found some minor issues but tests/periph_rtc is not working as expected.

Former · 2019-12-05T17:02:51Z

Murdock found some minor issues but tests/periph_rtc is not working as expected.

Fix this on 5d5357b. Use PRIu32.
tests/periph_rtc fix in 7e3921f

cpu/stm32f1/periph/rtc.c

Works get_time, set_time, alarm and wakeup after set power mode STM32_PM_STOP

benpicco · 2019-12-06T09:57:39Z

cpu/stm32f1/periph/rtc.c

+    _rtc_disable_alarm();
+    _rtc_set_alarm_time(0);
+
+    _rtc_clear_isr_ctx();


I think you don't need this here either.

_rtc_clear_isr_ctx?
On other processor efm32/rtc, esp32/rtc and other callback is cleared.
I think clear need.

just "because others are doing it" is never good reasoning 😉

The alarm can only be enabled again by rtc_set_alarm() where those are overwritten anyway.
It does no harm, but it's a waste of instructions and adds needless clutter to the code.

You can drop the _rtc_set_alarm_time(0) too.

See other rtc_clear_alarm(void). All contain callback clear.

lpc2387 and cc430 do not…
But yea there seems to be some strange cargo cult going on.

Well keep it if you want.
But the _rtc_set_alarm_time(0) won't help you if the counter wraps around and _rtc_disable_alarm() was not working for some reason. (Which would be the only reason why you would want to delete the callback)

I found case.
We set time to 15:20:59, and set alarm 15:21:10. Alarm called.
At cur time 15:21:20 - set time 15:20:59.
Now we have: time 15:20:59. Alarm 15:21:10 (rtc_get_alarm return). But alarm not called (is disable)
I think that after callback we need clear alarm time!

just "because others are doing it" is never good reasoning wink

The alarm can only be enabled again by rtc_set_alarm() where those are overwritten anyway.
It does no harm, but it's a waste of instructions and adds needless clutter to the code.

Ok. I can argee with clear_isr_ctx. isr_ctx not visible outside. But alarm time visible on rtc_get_alarm.

Hm I would have expected rtc_get_alarm() to return -1 if no alarm is set, but this doesn't seem to be the case.

So returning an alarm in the past might be the next best thing.
IMHO the RTC API needs some overhaul…

Hm I would have expected rtc_get_alarm() to return -1 if no alarm is set, but this doesn't seem to be the case.

So returning an alarm in the past might be the next best thing.
IMHO the RTC API needs some overhaul…

If clear alarm on callback, then test periph_rtc work wrong:
puts("Alarm!");

if (++cnt < REPEAT) {
struct tm time;
rtc_get_alarm(&time);
inc_secs(&time, PERIOD);
rtc_set_alarm(&time, cb, NULL);
}
rtc_get_alarm - return zero time and next 3 alarm not run.
If rtc_get_alarm return -1 when alarm is disable (on callback too) - same problem. Alarm run only 1 times. Returned value rtc_get_alarm not check in test. I think replace rtc_get_alarm to rtc_get_time - good things.

Yea like I said, I think the API needs a bit of rework to be less ambiguous.
Anyway, your implementation works and should not be help up over different interpretations of the unclear RTC API.

benpicco

Tested on blackpill board, code looks good.

The semantics of how rtc_clear_alarm() <-> rtc_get_alarm() should behave are ill defined, that shouldn't block this PR though.

Former force-pushed the stm32f1_rtc branch from 1d89144 to 96c3431 Compare March 25, 2019 09:44

maribu requested changes Mar 25, 2019

View reviewed changes

maribu added Area: cpu Area: CPU/MCU ports Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Mar 25, 2019

Former force-pushed the stm32f1_rtc branch 2 times, most recently from 63f8b3f to 2a04006 Compare March 27, 2019 09:27

Former force-pushed the stm32f1_rtc branch from 2a04006 to c58a5f3 Compare March 27, 2019 10:42

Former changed the title ~~RTC for stm32f1~~ cpu: RTC implementation for STM32F1 Mar 27, 2019

smlng requested a review from MrKevinWeiss March 29, 2019 19:23

maribu previously requested changes Apr 9, 2019

View reviewed changes

cpu/stm32_common/periph/rtc.c Outdated Show resolved Hide resolved

cpu/stm32_common/periph/rtc.c Outdated Show resolved Hide resolved

maribu added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines labels Apr 9, 2019

maribu reviewed Apr 9, 2019

View reviewed changes

cpu/stm32_common/periph/rtc.c Outdated Show resolved Hide resolved

cpu/stm32_common/periph/rtc.c Outdated Show resolved Hide resolved

aabadie reviewed Apr 9, 2019

View reviewed changes

cpu/stm32_common/periph/rtc.c Outdated Show resolved Hide resolved

Former force-pushed the stm32f1_rtc branch from c58a5f3 to 60cb42e Compare April 10, 2019 10:16

kaspar030 mentioned this pull request Apr 10, 2019

WIP: introduce periph_sleepmem #11369

Closed

Former force-pushed the stm32f1_rtc branch from 60cb42e to 199feab Compare June 17, 2019 15:32

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 17, 2019

benpicco requested a review from fjmolinas September 17, 2019 22:26

Former force-pushed the stm32f1_rtc branch from 199feab to 0206950 Compare November 28, 2019 18:48

benpicco reviewed Nov 28, 2019

View reviewed changes

boards/common/arduino-atmega/Makefile.features Outdated Show resolved Hide resolved

cpu/stm32f1/periph/rtc.c Show resolved Hide resolved

Former force-pushed the stm32f1_rtc branch from 0206950 to c1fd3b8 Compare December 5, 2019 09:26

Former force-pushed the stm32f1_rtc branch from c1fd3b8 to 7e3921f Compare December 5, 2019 16:57

benpicco reviewed Dec 5, 2019

View reviewed changes

cpu/stm32f1/periph/rtc.c Show resolved Hide resolved

benpicco reviewed Dec 5, 2019

View reviewed changes

cpu/stm32f1/periph/rtc.c Outdated Show resolved Hide resolved

Former force-pushed the stm32f1_rtc branch 2 times, most recently from b443f35 to 74ebf14 Compare December 5, 2019 18:49

benpicco reviewed Dec 5, 2019

View reviewed changes

cpu/stm32f1/periph/rtc.c Outdated Show resolved Hide resolved

benpicco reviewed Dec 5, 2019

View reviewed changes

cpu/stm32f1/periph/rtc.c Outdated Show resolved Hide resolved

Alexei Bezborodov added 2 commits December 6, 2019 12:47

cpu: RTC implementation for STM32F1

239fc2b

Works get_time, set_time, alarm and wakeup after set power mode STM32_PM_STOP

cpu/stm32f1: FEATURES_PROVIDED += periph_rtc

88c429a

Former force-pushed the stm32f1_rtc branch from 74ebf14 to 88c429a Compare December 6, 2019 09:50

benpicco reviewed Dec 6, 2019

View reviewed changes

benpicco approved these changes Dec 8, 2019

View reviewed changes

benpicco merged commit 3496300 into RIOT-OS:master Dec 8, 2019

fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019

Hoernchen20 mentioned this pull request Dec 29, 2019

cpu/stm32f1: improve rtc irq #13013

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cpu: RTC implementation for STM32F1 #11258

cpu: RTC implementation for STM32F1 #11258

Former commented Mar 25, 2019 •

edited

Loading

maribu left a comment

maribu commented Mar 27, 2019 •

edited

Loading

maribu commented Apr 9, 2019

fjmolinas commented May 22, 2019

fjmolinas commented May 22, 2019

fjmolinas commented Sep 19, 2019 •

edited

Loading

fjmolinas commented Sep 19, 2019

benpicco commented Nov 28, 2019

Former commented Nov 28, 2019

benpicco left a comment

benpicco commented Nov 28, 2019

benpicco commented Dec 5, 2019

Former commented Dec 5, 2019

benpicco Dec 6, 2019

Former Dec 6, 2019

benpicco Dec 6, 2019

benpicco Dec 6, 2019

Former Dec 6, 2019

Former Dec 6, 2019

Former Dec 6, 2019

benpicco Dec 7, 2019

Former Dec 8, 2019

benpicco Dec 8, 2019

benpicco left a comment •

edited

Loading

cpu: RTC implementation for STM32F1 #11258

cpu: RTC implementation for STM32F1 #11258

Conversation

Former commented Mar 25, 2019 • edited Loading

maribu left a comment

Choose a reason for hiding this comment

maribu commented Mar 27, 2019 • edited Loading

maribu commented Apr 9, 2019

fjmolinas commented May 22, 2019

fjmolinas commented May 22, 2019

fjmolinas commented Sep 19, 2019 • edited Loading

fjmolinas commented Sep 19, 2019

benpicco commented Nov 28, 2019

Former commented Nov 28, 2019

benpicco left a comment

Choose a reason for hiding this comment

benpicco commented Nov 28, 2019

benpicco commented Dec 5, 2019

Former commented Dec 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benpicco left a comment • edited Loading

Choose a reason for hiding this comment

Former commented Mar 25, 2019 •

edited

Loading

maribu commented Mar 27, 2019 •

edited

Loading

fjmolinas commented Sep 19, 2019 •

edited

Loading

benpicco left a comment •

edited

Loading