Use fixed-point u64 math to divide the PLL values #212
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This avoids the need for almost 1K of code to implement division for u64 values (which the Cortex-M7 can't do in hardware). It can multiply u64 values in hardware, so instead of dividing by (integer) x, we find 1/x in fixed-point, multiply, and shift the fixed-point offset back out. Finding 1/x can be done in a u32 since our shift is small enough, and that division *can* be done in hardware on this CPU. Fixes stm32-rs#211
|
Hmm, I seem to be failing a bunch of rcc tests. Let's fix that up... |
This requires shifting the input frequencies down by 4 bits in order to fit both the max base_clk*n value, and the 30-bit fractional part, into a u64. (base_clk*n is up to 38 bits wide.) This is OK as long as all input frequencies are a multiple of 250kHz, which has the bottom 4 bits clear.
"cargo fmt" does not work because it requires both test mode and the #![no_std] attribute, which the crate lib.rs doesn't enable in test mode. But manually running rustfmt does.
|
For people not looking at #211, the issue with the tests was that we lost too much precision using only 26 bits for the fractional part of the fixed-point math. It needed at least 30 bits. But the numerator in the division operation needed up to 38 bits, so it would overflow a u64. Fortunately, we could shift the frequency right by 4 bits at the start to make everything fit, as long as the input frequencies are a multiple of 250kHz (they all are in the tests, at least, and I think they are likely to be in people's designs as well). So now we shift the frequency right by 4 bits and use 30 bits for the fractional part. Tests are all accurate enough to pass. |
maximeborges
requested review from
a team,
eldruin,
maximeborges and
mvertescher
and removed request for
a team
eldruin
reviewed
Mar 22, 2024
eldruin
approved these changes
Mar 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This avoids the need for almost 1K of code to implement division for u64 values (which the Cortex-M7 can't do in hardware). It can multiply u64 values in hardware, so instead of dividing by (integer) x, we find 1/x in fixed-point, multiply, and shift the fixed-point offset back out. It can also divide u32 values, and finding 1/x can be done in a u32 since our shift is small enough.
Fixes #211