8000 cpu/native: fix bug in periph_timer [backport 2023.10] by MrKevinWeiss · Pull Request #20049 · RIOT-OS/RIOT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

cpu/native: fix bug in periph_timer [backport 2023.10] #20049

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
Merged
merged 2 commits into from
Nov 15, 2023
Merged

cpu/native: fix bug in periph_timer [backport 2023.10] #20049

merged 2 commits into from
Nov 15, 2023

Conversation

MrKevinWeiss
Copy link
Contributor

Backport of #20009

Contribution description

While debugging #18977 (comment) it became obvious that the periph_timer in native is broken and issues early IRQs. This replaces the use of setitimer that cannot use a monotonic clock source with timer_settime().

Testing procedure

I have some non-publishable code that tests if the time an ISR fires in terms of timer_read() is no earlier than the time expected. This occasionally triggered with master, but I didn't see any of these issues anymore with this PR. I guess I should revive my PR to spice up the periph timer tests and add a polished version of this and let this run for an hour or two.

The tests ins tests/periph/timer* should still succeed on native. (They do for me in a container running riot/riotbuild).

Issues/PRs references

Found while debugging #18977 (comment)

@MrKevinWeiss MrKevinWeiss added 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 CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train Platform: native Platform: This PR/issue effects the native platform Process: release backport Integration Process: The PR is a release backport of a change previously provided to master labels Nov 6, 2023
@riot-ci
Copy link
riot-ci commented Nov 6, 2023 •
edited
Loading

Murdock results

✔️ PASSED

273f9e0 cpu/native: drop unused real_setitimer

Success Failures Total Runtime
7937 0 7937 11m:35s

Artifacts

@maribu @MrKevinWeiss
cpu/native: fix bug in periph_timer
2d50089 
Also use `CLOCK_MONOTONIC` for the timeouts, not just for
`timer_read()`. This fixes mismatches between when a timeout
occurs and what is expected in the context of the values returned by
`timer_read()`.

(cherry picked from commit cea7fce)
@maribu @MrKevinWeiss
cpu/native: drop unused real_setitimer
273f9e0 
(cherry picked from commit 50b841e)
@MrKevinWeiss MrKevinWeiss force-pushed the backport/2023.10/native_timer_bug_work_around branch from 1d1efba to 273f9e0 Compare November 14, 2023 08:20
@MrKevinWeiss
Copy link
Contributor Author

rebased

@MrKevinWeiss
Copy link
Contributor Author

Still waiting on this...

@MrKevinWeiss MrKevinWeiss added this pull request to the merge queue Nov 15, 2023
Merged via the queue into RIOT-OS:2023.10-branch with commit 4bf2864 Nov 15, 2023
@MrKevinWeiss MrKevinWeiss deleted the backport/2023.10/native_timer_bug_work_around branch November 15, 2023 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maribu maribu maribu approved these changes

@benpicco benpicco Awaiting requested review from benpicco

Assignees
No one assigned
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 CI: ready for merge train 🚃 PR is ready to be merged and awaiting the next merge train Platform: native Platform: This PR/issue effects the native platform Process: release backport Integration Process: The PR is a release backport of a change previously provided to master
Projects
None yet
Milestone
No milestone
Development

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