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.

Sign up for GitHub

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

Jump to bottom

sys/ztimer: Simplify ztimer backend selection #19684

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

sys/ztimer: Simplify ztimer backend selection #19684

wants to merge 5 commits into from
+264 −17

Conversation

MrKevinWeiss
Copy link
Contributor

Contribution description

I added a test (with an informative README) that exposes a problem with the ztimer backend selection in both make and Kconfig.

This documents it, provides a test to show it, and provides a solution.

The potential issue is that some application that may have used ztimer_periph_rtt as a backend could have changed to ztimer_periph_timer.

Testing procedure

Green murdock...

Issues/PRs references

Preventing #18005 from being merged...

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework Area: timers Area: timer subsystems Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms labels May 30, 2023
@MrKevinWeiss MrKevinWeiss removed Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: doc Area: Documentation Area: tests Area: tests and testing framework Platform: AVR Platform: This PR/issue effects AVR-based platforms Area: timers Area: timer subsystems Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports Area: sys Area: System Area: Kconfig Area: Kconfig integration labels May 30, 2023
@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 30, 2023
@riot-ci
Copy link

riot-ci commented May 30, 2023 •
edited
Loading

Murdock results

❌ FAILED

2295306 fixup! sys/ztimer/Kconfig: Improve backend selection

Success Failures Total Runtime
1297 0 6947 02m:47s

Artifacts

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework Area: timers Area: timer subsystems Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms labels May 31, 2023
@MrKevinWeiss
Copy link
Contributor Author

Seems unrelated?

@MrKevinWeiss
Copy link
Contributor Author

MrKevinWeiss commented Jun 1, 2023 •
edited
Loading

Interesting for the examples/twr_aloha failure, it is because the stk3200 used to use rtt but now uses periph_timer. As a result OS_CPUTIME_FREQ_HIGH gets set to CONFIG_ZTIMER_MSEC_BASE_FREQ (1000000) instead of RTT_FREQUENCY.
Though the error says:

#error "Invalid OS_CPUTIME_FREQ value.  Value must be one of a) a power of 2" \
       ">= 256Hz, or b) any value >= 1MHz"

The actual condition for the error is #elif MYNEWT_VAL(OS_CPUTIME_FREQ) > 1000000, which is > than not >=.

@MrKevinWeiss
Copy link
Contributor Author

Hmm, this error points out that we are giving up using rtt on any boards with rtc conflicts, even if the rtc is not used...

@MrKevinWeiss
Copy link
Contributor Author

It is actually 250000 for that board... Something due to XTIMER_HZ overriding it or something. So it should be failing.

@MrKevinWeiss
Copy link
Contributor Author

I think I need some feedback on how to prevent this issue... Maybe something with default modules would be better...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@smlng smlng Awaiting requested review from smlng

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

@gschorcht gschorcht Awaiting requested review from gschorcht gschorcht is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework Area: timers Area: timer subsystems 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 Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MrKevinWeiss @riot-ci