periph/rtc: move init from auto_init to periph_init#6900
periph/rtc: move init from auto_init to periph_init#6900dylad merged 5 commits intoRIOT-OS:masterfrom
Conversation
|
@vincent-d can you elaborate why (and when) this is an issue? Currently RTC is initialized by I'm unsure if moving this to periph_init is the right solution, maybe @haukepetersen can enlighten us: what's the idea of periph_init vs auto_init? |
|
I was playing around with my rtc and I discovered that In fact, MODULE_RTC is never defined, rtc does not exist as a module nor a pseudomodule. Then we could replace the |
|
ah |
|
Actually, moving the init to The guard should in a future case be something like |
|
@vincent-d I just tried |
|
These are the perfect counterexamples, because these both app call explicitly But I think |
|
I am with @vincent-d here, Maybe a |
Damn, my bad ... oversaw obvious calls 😞 But then, at least for |
|
just tested the following: I removed all Questions is if that should be default behavior and whether we need an option to disable auto_init of RTC somehow? |
Why? We already do that for SPI, don't we? But this is not completely clear what the |
|
yes currently it looks like SPI is auto_init if available ... but that doesn't mean that's a good choice in all cases. And that's why I asked:
If I got @haukepetersen correctly, we agree that we should have switches (macros) to either opt-out or opt-in auto-init of peripheral drivers - the default could be auto-init ON (as now). |
| */ | ||
|
|
||
| #include "periph/spi.h" | ||
| #include "periph/rtc.h" |
There was a problem hiding this comment.
this should be guarded with #if RTC_NUMOF as well.
and in theory the same for the spi header above, too - see #7079
|
It does not work on sam because RTT and RTC are the same periph. The linker complains as soon as you use the RTT. Maybe using a pseudomodule as @haukepetersen suggested is the best way to go. We definitely need to find a way to compile periph drivers independently! |
3d8fb07 to
f2466fb
Compare
|
Since #7421 is merged, I guess it becomes as trivial as that! |
|
Added shell commands fix and removed rtc init from examples/default (not needed anymore). |
haukepetersen
left a comment
There was a problem hiding this comment.
now it looks nice: ACK
|
side question, possibly to be solved somewhere else: should we then also remove the |
dylad
left a comment
There was a problem hiding this comment.
rtc_init function & FEATURE_PERIPH_RTC are still present in the following files :
sys/shell/commands/sc_rtc.c
tests/periph_rtc/main.c
tests/lwmac/main.c
tests/pkg_fatfs/main.c
|
@haukepetersen that is what I was thinking. |
|
notice: |
|
|
I think we're good here. I tested this PR on saml21-xpro using tests/periph_rtc and it's work as expected. |
It seems that the auto_init code used to initialize rtc was not working.
Using the periph_init seems better.