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.

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

cpu/sam0_common/periph_timer: implement timer_set() #19036

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maribu
Copy link
Member

@maribu maribu commented Dec 9, 2022

Contribution description

The fallback implementation of timer_set() in drivers/periph_common is known to fail on short relative sets. This adds a robust implementation.

Testing procedure

Run tests/periph_timer_short_relative_set at least a few dozen times (or use #19030 to have a few dozen repetitions of the test case in a single run of the test application). It should now succeed.

Issues/PRs references

None

The fallback implementation of timer_set() in `drivers/periph_common`
is known to fail on short relative sets. This adds a robust
implementation.
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: cpu Area: CPU/MCU ports labels Dec 9, 2022
@github-actions github-actions bot added the Platform: ARM Platform: This PR/issue effects ARM-based platforms label Dec 9, 2022
* afterwards. We add one to risk rather jumping over one tick
* rather than having the clock going backwards by one time */
unsigned now = timer_read(tim);
timer_stop(tim);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Does this have nasty side affects like loosing the IRQ mask or the CC reg of the other channel?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this TODO required to be done for this PR ?
If not, I can have a look at it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I would just extend the periph_timer test so that such side effects would be directly detected. I'll try to allocate some time for that, but currently my work load is quite high :-/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel your pain, I'm somehow in the same position :)
So should we wait a bit for this PR ?

@riot-ci
Copy link

riot-ci commented Dec 9, 2022

Murdock results

✔️ PASSED

05bc531 cpu/sam0_common/periph_timer: implement timer_set()

Success Failures Total Runtime
2006 0 2006 04m:13s

Artifacts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants