From 402b7f1eb8c8477837b0b7d1803e0d0d497a9ce0 Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Thu, 10 Feb 2022 13:39:36 +0000 Subject: [PATCH 1/4] Use u32 instead of u64 division in clock calculations This saves about 1kB of flash by removing compiler_builtins::int::specialized_div_rem::u64_div_rem if no other code uses u64 divisions. --- rp2040-hal/CHANGELOG.md | 3 ++- rp2040-hal/src/clocks/macros.rs | 25 ++++++----------------- rp2040-hal/src/clocks/mod.rs | 36 ++++++++++++++++++++++++++++----- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/rp2040-hal/CHANGELOG.md b/rp2040-hal/CHANGELOG.md index 73a76c0..c9b051b 100644 --- a/rp2040-hal/CHANGELOG.md +++ b/rp2040-hal/CHANGELOG.md @@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Updated embedded-hal alpha support to version 1.0.0-alpha.7 +- Update embedded-hal alpha support to version 1.0.0-alpha.7 +- Avoid 64-bit division in clock calculations ## [0.3.0] - 2021-12-19 diff --git a/rp2040-hal/src/clocks/macros.rs b/rp2040-hal/src/clocks/macros.rs index fc75261..e814640 100644 --- a/rp2040-hal/src/clocks/macros.rs +++ b/rp2040-hal/src/clocks/macros.rs @@ -180,19 +180,13 @@ macro_rules! clock { #[doc = "Configure `"$name"`"] fn configure_clock>(&mut self, src: &S, freq: Hertz) -> Result<(), ClockError>{ - let src_freq: Hertz = src.get_freq().into(); + let src_freq: Hertz = src.get_freq().into(); if freq.gt(&src_freq){ return Err(ClockError::CantIncreaseFreq); } - // Div register is 24.8) int.frac divider so multiply by 2^8 (left shift by 8) - let shifted_src_freq = src_freq * (1 << 8); - let div = if freq.eq(&src_freq) { - 1 << 8 - } else { - (shifted_src_freq / freq.integer() as u64).integer() as u32 - }; + let div = fractional_div(src_freq.integer(), freq.integer()).ok_or(ClockError::FrequencyTooLow)?; // If increasing divisor, set divisor before source. Otherwise set source // before divisor. This avoids a momentary overspeed when e.g. switching @@ -223,8 +217,7 @@ macro_rules! clock { self.set_div(div); // Store the configured frequency - // div contains both the integer part and the fractional part so we need to shift the src_freq equally - self.frequency = (shifted_src_freq / div as u64).try_into().map_err(|_| ClockError::FrequencyToHigh)?; + self.frequency = fractional_div(src_freq.integer(), div).ok_or(ClockError::FrequencyTooHigh)?.Hz(); Ok(()) } @@ -337,19 +330,13 @@ macro_rules! stoppable_clock { #[doc = "Configure `"$name"`"] fn configure_clock>(&mut self, src: &S, freq: Hertz) -> Result<(), ClockError>{ - let src_freq: Hertz = src.get_freq().into(); + let src_freq: Hertz = src.get_freq().into(); if freq.gt(&src_freq){ return Err(ClockError::CantIncreaseFreq); } - // Div register is 24.8) int.frac divider so multiply by 2^8 (left shift by 8) - let shifted_src_freq = src_freq * (1 << 8); - let div = if freq.eq(&src_freq) { - 1 << 8 - } else { - (shifted_src_freq / freq.integer() as u64).integer() as u32 - }; + let div = fractional_div(src_freq.integer(), freq.integer()).ok_or(ClockError::FrequencyTooLow)?; // If increasing divisor, set divisor before source. Otherwise set source // before divisor. This avoids a momentary overspeed when e.g. switching @@ -386,7 +373,7 @@ macro_rules! stoppable_clock { self.set_div(div); // Store the configured frequency - self.frequency = (shifted_src_freq / div as u64).try_into().map_err(|_| ClockError::FrequencyToHigh)?; + self.frequency = fractional_div(src_freq.integer(), div).ok_or(ClockError::FrequencyTooHigh)?.Hz(); Ok(()) } diff --git a/rp2040-hal/src/clocks/mod.rs b/rp2040-hal/src/clocks/mod.rs index b1a546c..e8cb8f8 100644 --- a/rp2040-hal/src/clocks/mod.rs +++ b/rp2040-hal/src/clocks/mod.rs @@ -70,10 +70,7 @@ use crate::{ watchdog::Watchdog, xosc::{setup_xosc_blocking, CrystalOscillator, Error as XoscError, Stable}, }; -use core::{ - convert::{Infallible, TryInto}, - marker::PhantomData, -}; +use core::{convert::Infallible, marker::PhantomData}; use embedded_time::rate::*; use pac::{CLOCKS, PLL_SYS, PLL_USB, RESETS, XOSC}; @@ -106,7 +103,9 @@ pub enum ClockError { /// The frequency desired is higher than the source frequency CantIncreaseFreq, /// The desired frequency is to high (would overflow an u32) - FrequencyToHigh, + FrequencyTooHigh, + /// The desired frequency is too low (divider can't reach the desired value) + FrequencyTooLow, } /// For clocks @@ -354,3 +353,30 @@ pub fn init_clocks_and_plls( .map_err(InitError::ClockError)?; Ok(clocks) } + +// Calculates (numerator<<8)/denominator, avoiding 64bit division +// Returns None if the result would not fit in 32 bit. +fn fractional_div(numerator: u32, denominator: u32) -> Option { + if denominator.eq(&numerator) { + return Some(1 << 8); + } + + let div_int = numerator / denominator; + if div_int >= 1 << 24 { + return None; + } + + let div_rem = numerator - (div_int * denominator); + + let div_frac = if div_rem < 1 << 24 { + // div_rem is small enough to shift it by 8 bits without overflow + (div_rem << 8) / denominator + } else { + // div_rem is too large. Shift denominator right, instead. + // As 1<<24 < div_rem < denominator, relative error caused by the + // lost lower 8 bits of denominator is smaller than 2^-16 + (div_rem) / (denominator >> 8) + }; + + Some((div_int << 8) + div_frac) +} From fecde70cf96550fe7bcba751cb16676b8a253969 Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Thu, 10 Feb 2022 18:49:18 +0000 Subject: [PATCH 2/4] Derive several traits for ClockError --- rp2040-hal/src/clocks/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rp2040-hal/src/clocks/mod.rs b/rp2040-hal/src/clocks/mod.rs index e8cb8f8..db1a7d4 100644 --- a/rp2040-hal/src/clocks/mod.rs +++ b/rp2040-hal/src/clocks/mod.rs @@ -99,6 +99,8 @@ impl ShareableClocks { } /// Something when wrong setting up the clock +#[non_exhaustive] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum ClockError { /// The frequency desired is higher than the source frequency CantIncreaseFreq, From b46ddd735177186647d4d3843e4d3553b5611691 Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Tue, 22 Feb 2022 23:09:33 +0000 Subject: [PATCH 3/4] Add test cases for fractional_div() --- rp2040-hal/src/clocks/mod.rs | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/rp2040-hal/src/clocks/mod.rs b/rp2040-hal/src/clocks/mod.rs index db1a7d4..19f04aa 100644 --- a/rp2040-hal/src/clocks/mod.rs +++ b/rp2040-hal/src/clocks/mod.rs @@ -382,3 +382,43 @@ fn fractional_div(numerator: u32, denominator: u32) -> Option { Some((div_int << 8) + div_frac) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_fractional_div() { + // easy values + assert_eq!(fractional_div(1, 1), Some(1 << 8)); + + // typical values + assert_eq!(fractional_div(125_000_000, 48_000_000), Some(666)); + assert_eq!(fractional_div(48_000_000, 46875), Some(1024 << 8)); + + // resulting frequencies + assert_eq!( + fractional_div( + 125_000_000, + fractional_div(125_000_000, 48_000_000).unwrap() + ), + Some(48_048_048) + ); + assert_eq!( + fractional_div(48_000_000, fractional_div(48_000_000, 46875).unwrap()), + Some(46875) + ); + + // not allowed in src/clocks/mod.rs, but should still deliver correct results + assert_eq!(fractional_div(1, 2), Some(128)); + assert_eq!(fractional_div(1, 256), Some(1)); + assert_eq!(fractional_div(1, 257), Some(0)); + + // borderline cases + assert_eq!(fractional_div((1 << 24) - 1, 1), Some(((1 << 24) - 1) << 8)); + assert_eq!(fractional_div(1 << 24, 1), None); + assert_eq!(fractional_div(1 << 24, 2), Some(1 << (23 + 8))); + assert_eq!(fractional_div(1 << 24, (1 << 24) + 1), Some(1 << 8)); + assert_eq!(fractional_div(u32::MAX, u32::MAX), Some(1 << 8)); + } +} From 00b49d52b503fe48871d9f8a3ed3ca85c3d58a14 Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Tue, 22 Feb 2022 23:12:59 +0000 Subject: [PATCH 4/4] Actually run fractional_div test case from CI --- .github/workflows/build_and_test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 3d48219..43a0fcd 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -48,3 +48,7 @@ jobs: with: command: test args: --doc --target x86_64-unknown-linux-gnu --features chrono + - uses: actions-rs/cargo@v1 + with: + command: test + args: --tests --target x86_64-unknown-linux-gnu