From 79e452922a6b467f2e8547a6b28698ed5f409705 Mon Sep 17 00:00:00 2001 From: 1-rafael-1 Date: Mon, 12 May 2025 21:33:47 +0200 Subject: [PATCH] Add ClockError enum and update system_freq to return Result for error handling --- embassy-rp/src/clocks.rs | 66 ++++++++++++++++++++-------- examples/rp/src/bin/overclock.rs | 2 +- examples/rp235x/src/bin/overclock.rs | 2 +- tests/rp/src/bin/overclock.rs | 2 +- 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/embassy-rp/src/clocks.rs b/embassy-rp/src/clocks.rs index bcd08c204..5872ef789 100644 --- a/embassy-rp/src/clocks.rs +++ b/embassy-rp/src/clocks.rs @@ -82,6 +82,18 @@ use crate::{pac, reset, Peri}; // be very useful until we have runtime clock reconfiguration. once this // happens we can resurrect the commented-out gpin bits. +/// Clock error types. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum ClockError { + /// PLL failed to lock within the timeout period. + PllLockTimedOut, + /// Could not find valid PLL parameters for system clock. + InvalidPllParameters, + /// Reading the core voltage failed due to an unexpected value in the register. + UnexpectedCoreVoltageRead, +} + struct Clocks { xosc: AtomicU32, sys: AtomicU32, @@ -144,8 +156,9 @@ pub enum PeriClkSrc { /// **Note**: For RP235x the maximum voltage is 1.30V, unless unlocked by setting unless the voltage limit /// is disabled using the disable_voltage_limit field in the vreg_ctrl register. For lack of practical use at this /// point in time, this is not implemented here. So the maximum voltage in this enum is 1.30V for now. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] #[repr(u8)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum CoreVoltage { // RP2040 voltage levels #[cfg(feature = "rp2040")] @@ -468,7 +481,7 @@ impl ClockConfig { /// # Returns /// /// A ClockConfig configured to achieve the requested system frequency using the - /// the usual 12Mhz crystal, or panic if no valid parameters can be found. + /// the usual 12Mhz crystal, or an error if no valid parameters can be found. /// /// # Note on core voltage: /// @@ -482,7 +495,11 @@ impl ClockConfig { /// **For RP235x**: /// At this point in time there is no official manufacturer endorsement for running the chip on other core voltages and/or other clock speeds than the defaults. /// Using this function is experimental and may not work as expected or even damage the chip. - pub fn system_freq(hz: u32) -> Self { + /// + /// # Returns + /// + /// A Result containing either the configured ClockConfig or a ClockError. + pub fn system_freq(hz: u32) -> Result { // Start with the standard configuration from crystal() const DEFAULT_CRYSTAL_HZ: u32 = 12_000_000; let mut config = Self::crystal(DEFAULT_CRYSTAL_HZ); @@ -491,16 +508,15 @@ impl ClockConfig { // (which is what crystal() configures by default) #[cfg(feature = "rp2040")] if hz == 125_000_000 { - return config; + return Ok(config); } #[cfg(feature = "_rp235x")] if hz == 150_000_000 { - return config; + return Ok(config); } // Find optimal PLL parameters for the requested frequency - let sys_pll_params = find_pll_params(DEFAULT_CRYSTAL_HZ, hz) - .unwrap_or_else(|| panic!("Could not find valid PLL parameters for system clock")); + let sys_pll_params = find_pll_params(DEFAULT_CRYSTAL_HZ, hz).ok_or(ClockError::InvalidPllParameters)?; // Replace the sys_pll configuration with our custom parameters if let Some(xosc) = &mut config.xosc { @@ -525,7 +541,7 @@ impl ClockConfig { }; } - config + Ok(config) } /// Configure with manual PLL settings for full control over system clock @@ -620,6 +636,7 @@ impl ClockConfig { #[repr(u16)] #[non_exhaustive] #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum RoscRange { /// Low range. Low = pac::rosc::vals::FreqRange::LOW.0, @@ -726,6 +743,7 @@ pub struct RefClkConfig { /// Reference clock source. #[non_exhaustive] #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum RefClkSrc { /// XOSC. Xosc, @@ -741,6 +759,7 @@ pub enum RefClkSrc { /// SYS clock source. #[non_exhaustive] #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum SysClkSrc { /// REF. Ref, @@ -779,6 +798,7 @@ pub struct SysClkConfig { #[repr(u8)] #[non_exhaustive] #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum UsbClkSrc { /// PLL USB. PllUsb = ClkUsbCtrlAuxsrc::CLKSRC_PLL_USB as _, @@ -807,6 +827,7 @@ pub struct UsbClkConfig { #[repr(u8)] #[non_exhaustive] #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum AdcClkSrc { /// PLL USB. PllUsb = ClkAdcCtrlAuxsrc::CLKSRC_PLL_USB as _, @@ -835,6 +856,7 @@ pub struct AdcClkConfig { #[repr(u8)] #[non_exhaustive] #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg(feature = "rp2040")] pub enum RtcClkSrc { /// PLL USB. @@ -1084,14 +1106,20 @@ pub(crate) unsafe fn init(config: ClockConfig) { let pll_sys_freq = match config.sys_pll { Some(sys_pll_config) => match configure_pll(pac::PLL_SYS, config.hz, sys_pll_config) { Ok(freq) => freq, + #[cfg(feature = "defmt")] Err(e) => panic!("Failed to configure PLL_SYS: {}", e), + #[cfg(not(feature = "defmt"))] + Err(_e) => panic!("Failed to configure PLL_SYS"), }, None => 0, }; let pll_usb_freq = match config.usb_pll { Some(usb_pll_config) => match configure_pll(pac::PLL_USB, config.hz, usb_pll_config) { Ok(freq) => freq, + #[cfg(feature = "defmt")] Err(e) => panic!("Failed to configure PLL_USB: {}", e), + #[cfg(not(feature = "defmt"))] + Err(_e) => panic!("Failed to configure PLL_USB"), }, None => 0, }; @@ -1401,7 +1429,7 @@ pub fn clk_rtc_freq() -> u16 { /// /// Returns the current core voltage or an error if the voltage register /// contains an unknown value. -pub fn core_voltage() -> Result { +pub fn core_voltage() -> Result { #[cfg(feature = "rp2040")] { let vreg = pac::VREG_AND_CHIP_RESET; @@ -1418,7 +1446,7 @@ pub fn core_voltage() -> Result { 0b1101 => Ok(CoreVoltage::V1_20), 0b1110 => Ok(CoreVoltage::V1_25), 0b1111 => Ok(CoreVoltage::V1_30), - _ => Err("Unexpected value in register"), + _ => Err(ClockError::UnexpectedCoreVoltageRead), } } @@ -1443,7 +1471,7 @@ pub fn core_voltage() -> Result { 0b01101 => Ok(CoreVoltage::V1_20), 0b01110 => Ok(CoreVoltage::V1_25), 0b01111 => Ok(CoreVoltage::V1_30), - _ => Err("Unexpected value in register"), + _ => Err(ClockError::UnexpectedCoreVoltageRead), // see CoreVoltage: we do not support setting Voltages higher than 1.30V at this point } } @@ -1461,7 +1489,7 @@ fn start_xosc(crystal_hz: u32, delay_multiplier: u32) { /// PLL (Phase-Locked Loop) configuration #[inline(always)] -fn configure_pll(p: pac::pll::Pll, input_freq: u32, config: PllConfig) -> Result { +fn configure_pll(p: pac::pll::Pll, input_freq: u32, config: PllConfig) -> Result { // Calculate reference frequency let ref_freq = input_freq / config.refdiv as u32; @@ -1532,7 +1560,7 @@ fn configure_pll(p: pac::pll::Pll, input_freq: u32, config: PllConfig) -> Result timeout -= 1; if timeout == 0 { // PLL failed to lock, return 0 to indicate failure - return Err("PLL failed to lock"); + return Err(ClockError::PllLockTimedOut); } } @@ -2103,21 +2131,21 @@ mod tests { { // Test automatic voltage scaling based on frequency // Under 133 MHz should use default voltage (V1_10) - let config = ClockConfig::system_freq(125_000_000); + let config = ClockConfig::system_freq(125_000_000).unwrap(); assert_eq!(config.core_voltage, CoreVoltage::V1_10); // 133-200 MHz should use V1_15 - let config = ClockConfig::system_freq(150_000_000); + let config = ClockConfig::system_freq(150_000_000).unwrap(); assert_eq!(config.core_voltage, CoreVoltage::V1_15); - let config = ClockConfig::system_freq(200_000_000); + let config = ClockConfig::system_freq(200_000_000).unwrap(); assert_eq!(config.core_voltage, CoreVoltage::V1_15); - // Above 200 MHz should use V1_25 - let config = ClockConfig::system_freq(250_000_000); + // Above 200 MHz should use V1_15 + let config = ClockConfig::system_freq(250_000_000).unwrap(); assert_eq!(config.core_voltage, CoreVoltage::V1_15); // Below 125 MHz should use V1_10 - let config = ClockConfig::system_freq(100_000_000); + let config = ClockConfig::system_freq(100_000_000).unwrap(); assert_eq!(config.core_voltage, CoreVoltage::V1_10); } } diff --git a/examples/rp/src/bin/overclock.rs b/examples/rp/src/bin/overclock.rs index 89147ba42..2706399af 100644 --- a/examples/rp/src/bin/overclock.rs +++ b/examples/rp/src/bin/overclock.rs @@ -18,7 +18,7 @@ const COUNT_TO: i64 = 10_000_000; #[embassy_executor::main] async fn main(_spawner: Spawner) -> ! { // Set up for clock frequency of 200 MHz, setting all necessary defaults. - let config = Config::new(ClockConfig::system_freq(200_000_000)); + let config = Config::new(ClockConfig::system_freq(200_000_000).unwrap()); // Initialize the peripherals let p = embassy_rp::init(config); diff --git a/examples/rp235x/src/bin/overclock.rs b/examples/rp235x/src/bin/overclock.rs index 8713df688..178fd62ca 100644 --- a/examples/rp235x/src/bin/overclock.rs +++ b/examples/rp235x/src/bin/overclock.rs @@ -23,7 +23,7 @@ const COUNT_TO: i64 = 10_000_000; #[embassy_executor::main] async fn main(_spawner: Spawner) -> ! { // Set up for clock frequency of 200 MHz, setting all necessary defaults. - let mut config = Config::new(ClockConfig::system_freq(200_000_000)); + let mut config = Config::new(ClockConfig::system_freq(200_000_000).unwrap()); // since for the rp235x there is no official support for higher clock frequencies, `system_freq()` will not set a voltage for us. // We need to guess the core voltage, that is needed for the higher clock frequency. Going with a small increase from the default 1.1V here, based on diff --git a/tests/rp/src/bin/overclock.rs b/tests/rp/src/bin/overclock.rs index a568d7fed..167a26eb2 100644 --- a/tests/rp/src/bin/overclock.rs +++ b/tests/rp/src/bin/overclock.rs @@ -20,7 +20,7 @@ async fn main(_spawner: Spawner) { let mut config = Config::default(); // Initialize with 200MHz clock configuration - config.clocks = ClockConfig::system_freq(200_000_000); + config.clocks = ClockConfig::system_freq(200_000_000).unwrap(); // if we are rp235x, we need to manually set the core voltage. rp2040 should do this automatically #[cfg(feature = "rp235xb")]