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

drivers/periph_timer: add periph_timer_freq feature #20131

Closed
wants to merge 4 commits into from

Conversation

maribu
Copy link
Member

@maribu maribu commented Dec 1, 2023

Contribution description

The optional feature allows to query the frequency a timer is actually running at. This is especially useful when the MCU is running of an integrated in-silicon RC oscillator, which may run at an actual frequency that differs a lot from the nominal frequency.

Testing procedure

Some PRs to add this feature will be added soonish. The test app has been extended to print the actual frequency.

Issues/PRs references

None

The optional feature allows to query the frequency a timer is actually
running at. This is especially useful when the MCU is running of an
integrated in-silicon RC oscillator, which may run at an actual
frequency that differs a lot from the nominal frequency.
@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration labels Dec 1, 2023
@MrKevinWeiss
Copy link
Contributor

Would you mind adding a single implementation so I can test it. Other than that it makes sense!

@github-actions github-actions bot added Area: boards Area: Board ports Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Area: cpu Area: CPU/MCU ports labels Dec 5, 2023
@maribu
Copy link
Member Author

maribu commented Dec 5, 2023

Would you mind adding a single implementation so I can test it. Other than that it makes sense!

Did so now for the fe310. Sadly, that is a pretty boring timer :-/

@maribu
Copy link
Member Author

maribu commented Dec 5, 2023

I'll also squash the fixup that adds a missing ")" in message printed in the test.

The FE310 has only a 32.678 kHz RTT that is either clocked by an
external watch crystal or an internal oscillator. Since this timer
is always present and requires no board configuration, we can add this
feature at CPU level.
@maribu maribu force-pushed the drivers/periph_timer/timer_freq branch from 512d610 to 0f39866 Compare December 5, 2023 08:49
@maribu maribu added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Dec 5, 2023
@maribu
Copy link
Member Author

maribu commented Dec 5, 2023

Let's put this on a hold. With #16349 finally on its way in, I'm not sure this is a good idea anymore.

With #16349 one can just query prior to timer_init() what frequencies are supported. If one picks one of the actually supported frequencies, one can expect that the timer is actually running at the requested frequency.

I do agree that this API can be easier to use. But as this would need to be implemented for each and every peripheral, I'm not sure if the effort to maintain it is worth the convenience.

@maribu
Copy link
Member Author

maribu commented Dec 5, 2023

After a (short) discussion in the matrix room, I close this in favor of #16349

@maribu maribu closed this Dec 5, 2023
@maribu maribu deleted the drivers/periph_timer/timer_freq branch March 1, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants