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

Skip /32 AHB prescaler #228

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Conversation

knoellle
Copy link
Contributor

Fixes #225

There is no /32 AHB prescaler, but the old calculation code assumed there was, causing all calculated clock frequencies with an AHB prescaler > 16 to be off by a factor of 2.

Note that this only affects the frequencies stored in the Clocks struct, not the ones actually applied to the AHB frequency.
Page 194 of the reference manual shows mapping of register bits to prescaler factors. As far as I can see, those are implemented correctly.

@knoellle knoellle requested a review from a team as a code owner March 30, 2023 16:33
Copy link
Contributor

@jamwaffles jamwaffles left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I left a question inline, but on top of that could you also please add a changelog entry under the Fixed section?

src/rcc.rs Show resolved Hide resolved
@jamwaffles
Copy link
Contributor

This is good to go aside from:

on top of that could you also please add a changelog entry under the Fixed section?

If you missed it :)

@knoellle
Copy link
Contributor Author

knoellle commented Apr 3, 2023

Added a changelog entry.
However, at least on the preview in my fork, the PR link doesn't seem to work. Is that because this PR was created after the fork? All the other links seem to correctly reference PRs in the upstream repository. Will that fix itself once the change is merged?

Also, in the (now obsolete) workflow run, the clippy check failed. Do you want me to fix that in this PR even though it's not a related change?

@jamwaffles
Copy link
Contributor

You need to add a line at the bottom of the file like this, then the link should work.

@jamwaffles
Copy link
Contributor

Also, in the (now obsolete) workflow run, the clippy check failed. Do you want me to fix that in this PR even though it's not a related change?

Yeah that's fine, thanks for bringing it up :)

@knoellle
Copy link
Contributor Author

knoellle commented Apr 4, 2023

I'm confused... why did the clippy check succeed this time? 👀
Rustup reportedly installed the same versions in both runs.

Edit: Ah, apparently this was already fixed here and I hadn't rebased yet.

@knoellle
Copy link
Contributor Author

knoellle commented Apr 4, 2023

Fixed the changelog link, feel free to merge when you're happy.

@jamwaffles
Copy link
Contributor

Thanks!

@jamwaffles jamwaffles merged commit 8de7c8d into stm32-rs:master Apr 5, 2023
@knoellle knoellle deleted the fixAHBPrescaler branch April 5, 2023 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect AHB prescaler calculation
2 participants