cpu/stm32f1: add unified rtt configuration by fjmolinas · Pull Request #13907 · RIOT-OS/RIOT

fjmolinas · 2020-04-20T10:33:30Z

Contribution description

This PR is split from #13874. It unifies rtt configuration for stm32f1 and make the RTT_FREQYENCY configurable.

As in #13874 max and min values for the RTT are documented.

Testing procedure

tests/periph_rtt should still pass on stm32f1 boards with different frequencies

BOARD=iotlab-m3 make -C tests/periph_rtt flash test --no-print-directory -j3

main(): This is RIOT! (Version: 2020.07-devel-56-g42700-pr_rtt_enhancement)

RIOT RTT low-level driver test
This test will display 'Hello' every 5 seconds

Initializing the RTT driver
RTT now: 0
Setting initial alarm to now + 5 s (81920)
Done setting up the RTT, wait for many Hellos
Hello
Hello
Hello
Hello
Hello

CFLAGS+=-DRTT_FREQUENCY=1 BOARD=iotlab-m3 make -C tests/periph_rtt clean flash test --no-print-directory -j3

main(): This is RIOT! (Version: 2020.07-devel-56-g42700-pr_rtt_enhancement)

RIOT RTT low-level driver test
This test will display 'Hello' every 5 seconds

Initializing the RTT driver
RTT now: 0
Setting initial alarm to now + 5 s (5)
Done setting up the RTT, wait for many Hellos
Hello
Hello
Hello
Hello
Hello

Issues/PRs references

Split from #13874

boards/common/iotlab/include/periph_conf_common.h

boards/fox/include/periph_conf.h

fjmolinas · 2020-04-22T08:33:40Z

Re-worked to leave only RTT_FREQUENCY definition in periph_conf.h since it is a per board configuration. I kept the same configurations as before, but you can eventually override the default RTT_FREQUENCY at application level.

benpicco

Looks good to me, the math works out the same as before.

fjmolinas · 2020-04-22T12:00:46Z

Seems there is a naming conflict..

fjmolinas · 2020-04-22T12:04:22Z

Seems there is a naming conflict..

Oh no, its because those BOARDs do not have rtt feature but are still defining RTT_MAX_VALUE with this change.

fjmolinas · 2020-04-22T12:09:38Z

Oh no, its because those BOARDs do not have rtt feature but are still defining RTT_MAX_VALUE with this change.

Seem like they both have a LSE oscilator. But I'll guard the definition against MODULE_PERIPH_RTT anyway.

fjmolinas · 2020-04-22T12:10:57Z

Seem like they both have a LSE oscilator. But I'll guard the definition against MODULE_PERIPH_RTT anyway.

And add rtt in a follow-up.

fjmolinas · 2020-04-22T12:14:09Z

And add rtt in a follow-up.

Actually, does it make sense enabling right away? the bluepill config already has CLOCK_LSE set to 1, and periph_rtc is already provideed by default for all stm32f1.

fjmolinas · 2020-04-22T12:14:32Z

Actually, does it make sense enabling right away? the bluepill config already has CLOCK_LSE set to 1, and periph_rtc is already provideed by default for all stm32f1.

I wouldn't have any board to test though.

fjmolinas · 2020-04-22T12:22:13Z

@MrKevinWeiss do you have some blxxxpill?

jue89 · 2020-04-23T10:13:17Z

Actually, does it make sense enabling right away? the bluepill config already has CLOCK_LSE set to 1, and periph_rtc is already provideed by default for all stm32f1.

I wouldn't have any board to test though.

I actually had a PR in the pipe line enabling periph_rtt on the blxxxpill. I would happily test your changes on the bluepill.

fjmolinas · 2020-04-23T10:26:08Z

I actually had a PR in the pipe line enabling periph_rtt on the blxxxpill. I would happily test your changes on the bluepill.

I added RTT_FREQUENCY for the bluepill and enabled periph_rtt for all stm32f1, just as for periph_rtc, in that commit I removed FEATURES_PROVIDED += periph_rtt from stm32f1 boards, since its provided by the cpu.

fjmolinas · 2020-04-23T14:01:53Z

I added RTT_FREQUENCY for the bluepill and enabled periph_rtt for all stm32f1, just as for periph_rtc, in that commit I removed FEATURES_PROVIDED += periph_rtt from stm32f1 boards, since its provided by the cpu.

Seem that with the github issues, my comment got lost. I wanted to provide periph_rtt at cpu level, but then realized that at least 3 stm32f1 boards do not have CLOCK_LSE, so I just kept the original commit and split the blxxxchange into #13929.

fjmolinas · 2020-04-23T14:14:45Z

Seems github issues got in the way here.

-- github reports HEAD of master as 341916cd307bb9fcacf8a457f1d5988528c6ca5c
fatal: unable to access 'https://github.com/RIOT-OS/RIOT.git/': The requested URL returned error: 504
--- creating merge commit ...
-- merging c0d1884813e1816b5cf445b21464fa5d7134da9f into 341916cd307bb9fcacf8a457f1d5988528c6ca5c
--- cloning base repo
git-cache: cloning from cache. tag=commit341916cd307bb9fcacf8a457f1d5988528c6ca5c
--- adding remotes
--- checking out merge branch
Switched to a new branch 'pull/341916cd307bb9fcacf8a457f1d5988528c6ca5c/c0d1884813e1816b5cf445b21464fa5d7134da9f'
--- fetching pr_stm32f1_rtt
remote: 
remote: 
remote: 
remote: 
502 Bad Gateway

fjmolinas · 2020-04-23T15:33:04Z

Ajjj is starting to be more annoying than I though, because periph_cpu is included before periph_conf I can't have a default value there which is overridden per BOARD. Providing them only per BOARD is ok, but now rtc_rtt unit tests is complaining about RTT_MAX_VALUE missing, anyway this last one should make murdock happy, sorry for the force pushing.

boards/common/iotlab/include/periph_conf_common.h

fjmolinas · 2020-04-24T08:07:22Z

@benpicco does your ACK still stand?

aabadie

Looks good to me now.

ACK

fjmolinas · 2020-04-24T08:43:21Z

Thanks for the review!

fjmolinas added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers labels Apr 20, 2020

fjmolinas requested a review from aabadie April 20, 2020 10:33

fjmolinas requested review from DipSwitch, MrKevinWeiss and vincent-d as code owners April 20, 2020 10:33

fjmolinas mentioned this pull request Apr 20, 2020

pkg/openwsn: re-integrate the network stack as a package #13824

Merged

11 tasks

benpicco reviewed Apr 20, 2020

View reviewed changes

boards/common/iotlab/include/periph_conf_common.h Show resolved Hide resolved

benpicco reviewed Apr 20, 2020

View reviewed changes

boards/fox/include/periph_conf.h Show resolved Hide resolved

fjmolinas force-pushed the pr_stm32f1_rtt branch 4 times, most recently from 99b4f22 to f1deca1 Compare April 22, 2020 08:30

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 22, 2020

benpicco approved these changes Apr 22, 2020

View reviewed changes

fjmolinas force-pushed the pr_stm32f1_rtt branch from f1deca1 to 45f2c86 Compare April 22, 2020 12:11

fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 22, 2020

fjmolinas force-pushed the pr_stm32f1_rtt branch from 45f2c86 to 07ff619 Compare April 23, 2020 10:24

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 23, 2020

fjmolinas force-pushed the pr_stm32f1_rtt branch from 07ff619 to 9d74173 Compare April 23, 2020 13:31

fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 23, 2020

fjmolinas force-pushed the pr_stm32f1_rtt branch from 9d74173 to c0d1884 Compare April 23, 2020 13:40

fjmolinas mentioned this pull request Apr 23, 2020

boards/common/blxxxpill: provide periph_rtt #13929

Merged

fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 23, 2020

fjmolinas force-pushed the pr_stm32f1_rtt branch from c0d1884 to e07a862 Compare April 23, 2020 15:32

fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 23, 2020

fjmolinas force-pushed the pr_stm32f1_rtt branch from e07a862 to c081ab7 Compare April 23, 2020 15:34

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 23, 2020

aabadie reviewed Apr 23, 2020

View reviewed changes

boards/common/iotlab/include/periph_conf_common.h Outdated Show resolved Hide resolved

fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 23, 2020

cpu/stm32f1: add unified rtt configuration

4d398ab

fjmolinas force-pushed the pr_stm32f1_rtt branch from c081ab7 to 4d398ab Compare April 24, 2020 06:57

fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 24, 2020

aabadie approved these changes Apr 24, 2020

View reviewed changes

aabadie merged commit 5b814db into RIOT-OS:master Apr 24, 2020

fjmolinas deleted the pr_stm32f1_rtt branch April 24, 2020 08:43

miri64 added this to the Release 2020.07 milestone Jun 24, 2020

Conversation

fjmolinas commented Apr 20, 2020

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

Uh oh!

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

benpicco left a comment

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

fjmolinas commented Apr 22, 2020

Uh oh!

jue89 commented Apr 23, 2020

Uh oh!

fjmolinas commented Apr 23, 2020

Uh oh!

fjmolinas commented Apr 23, 2020

Uh oh!

fjmolinas commented Apr 23, 2020

Uh oh!

fjmolinas commented Apr 23, 2020

Uh oh!

Uh oh!

fjmolinas commented Apr 24, 2020

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

fjmolinas commented Apr 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants