From 42462681bd604750dfe8fa709453edf43c25b09d Mon Sep 17 00:00:00 2001 From: chemicstry Date: Thu, 23 Feb 2023 16:57:21 +0200 Subject: [PATCH 1/3] stm32/sdmmc: Implement proper clock configuration --- embassy-stm32/src/sdmmc/mod.rs | 87 +++++++++++++++++++++++++------ examples/stm32f4/src/bin/sdmmc.rs | 3 +- examples/stm32f7/src/bin/sdmmc.rs | 1 + 3 files changed, 75 insertions(+), 16 deletions(-) diff --git a/embassy-stm32/src/sdmmc/mod.rs b/embassy-stm32/src/sdmmc/mod.rs index 3c99a0b6..0bcd42bc 100644 --- a/embassy-stm32/src/sdmmc/mod.rs +++ b/embassy-stm32/src/sdmmc/mod.rs @@ -140,10 +140,21 @@ cfg_if::cfg_if! { /// Calculate clock divisor. Returns a SDMMC_CK less than or equal to /// `sdmmc_ck` in Hertz. /// - /// Returns `(clk_div, clk_f)`, where `clk_div` is the divisor register - /// value and `clk_f` is the resulting new clock frequency. - fn clk_div(ker_ck: Hertz, sdmmc_ck: u32) -> Result<(u8, Hertz), Error> { - let clk_div = match ker_ck.0 / sdmmc_ck { + /// Returns `(bypass, clk_div, clk_f)`, where `bypass` enables clock divisor bypass (only sdmmc_v1), + /// `clk_div` is the divisor register value and `clk_f` is the resulting new clock frequency. + fn clk_div(ker_ck: Hertz, sdmmc_ck: u32) -> Result<(bool, u8, Hertz), Error> { + // sdmmc_v1 maximum clock is 50 MHz + if sdmmc_ck > 50_000_000 { + return Err(Error::BadClock); + } + + // bypass divisor + if ker_ck.0 <= sdmmc_ck { + return Ok((true, 0, ker_ck)); + } + + // `ker_ck / sdmmc_ck` rounded up + let clk_div = match (ker_ck.0 + sdmmc_ck - 1) / sdmmc_ck { 0 | 1 => Ok(0), x @ 2..=258 => { Ok((x - 2) as u8) @@ -153,22 +164,24 @@ cfg_if::cfg_if! { // SDIO_CK frequency = SDIOCLK / [CLKDIV + 2] let clk_f = Hertz(ker_ck.0 / (clk_div as u32 + 2)); - Ok((clk_div, clk_f)) + Ok((false, clk_div, clk_f)) } } else if #[cfg(sdmmc_v2)] { /// Calculate clock divisor. Returns a SDMMC_CK less than or equal to /// `sdmmc_ck` in Hertz. /// - /// Returns `(clk_div, clk_f)`, where `clk_div` is the divisor register - /// value and `clk_f` is the resulting new clock frequency. - fn clk_div(ker_ck: Hertz, sdmmc_ck: u32) -> Result<(u16, Hertz), Error> { + /// Returns `(bypass, clk_div, clk_f)`, where `bypass` enables clock divisor bypass (only sdmmc_v1), + /// `clk_div` is the divisor register value and `clk_f` is the resulting new clock frequency. + fn clk_div(ker_ck: Hertz, sdmmc_ck: u32) -> Result<(bool, u16, Hertz), Error> { + // `ker_ck / sdmmc_ck` rounded up match (ker_ck.0 + sdmmc_ck - 1) / sdmmc_ck { - 0 | 1 => Ok((0, ker_ck)), + 0 | 1 => Ok((false, 0, ker_ck)), x @ 2..=2046 => { + // SDMMC_CK frequency = SDMMCCLK / [CLKDIV + 2] let clk_div = ((x + 1) / 2) as u16; let clk = Hertz(ker_ck.0 / (clk_div as u32 * 2)); - Ok((clk_div, clk)) + Ok((false, clk_div, clk)) } _ => Err(Error::BadClock), } @@ -478,7 +491,7 @@ impl<'d, T: Instance, Dma: SdmmcDma> Sdmmc<'d, T, Dma> { bus_width, &mut self.card, &mut self.signalling, - T::frequency(), + Self::kernel_clock(), &mut self.clock, T::state(), self.config.data_transfer_timeout, @@ -550,6 +563,44 @@ impl<'d, T: Instance, Dma: SdmmcDma> Sdmmc<'d, T, Dma> { regs.data_interrupts(false); state.wake(); } + + /// Returns kernel clock (SDIOCLK) for the SD-card facing domain + fn kernel_clock() -> Hertz { + cfg_if::cfg_if! { + // TODO, these could not be implemented, because required clocks are not exposed in RCC: + // - H7 uses pll1_q_ck or pll2_r_ck depending on SDMMCSEL + // - L1 uses pll48 + // - L4 uses clk48(pll48) + // - L4+, L5, U5 uses clk48(pll48) or PLLSAI3CLK(PLLP) depending on SDMMCSEL + if #[cfg(stm32f1)] { + // F1 uses AHB1(HCLK), which is correct in PAC + T::frequency() + } else if #[cfg(any(stm32f2, stm32f4))] { + // F2, F4 always use pll48 + critical_section::with(|_| unsafe { + crate::rcc::get_freqs().pll48 + }).expect("PLL48 is required for SDIO") + } else if #[cfg(stm32f7)] { + critical_section::with(|_| unsafe { + use core::any::TypeId; + let sdmmcsel = if TypeId::of::() == TypeId::of::() { + crate::pac::RCC.dckcfgr2().read().sdmmc1sel() + } else { + crate::pac::RCC.dckcfgr2().read().sdmmc2sel() + }; + + if sdmmcsel == crate::pac::rcc::vals::Sdmmcsel::SYSCLK { + crate::rcc::get_freqs().sys + } else { + crate::rcc::get_freqs().pll48.expect("PLL48 is required for SDMMC") + } + }) + } else { + // Use default peripheral clock and hope it works + T::frequency() + } + } + } } impl<'d, T: Instance, Dma> Drop for Sdmmc<'d, T, Dma> { @@ -625,7 +676,7 @@ impl SdmmcInner { unsafe { // While the SD/SDIO card or eMMC is in identification mode, // the SDMMC_CK frequency must be no more than 400 kHz. - let (clkdiv, init_clock) = unwrap!(clk_div(ker_ck, SD_INIT_FREQ.0)); + let (_bypass, clkdiv, init_clock) = unwrap!(clk_div(ker_ck, SD_INIT_FREQ.0)); *clock = init_clock; // CPSMACT and DPSMACT must be 0 to set WIDBUS @@ -634,6 +685,8 @@ impl SdmmcInner { regs.clkcr().modify(|w| { w.set_widbus(0); w.set_clkdiv(clkdiv); + #[cfg(sdmmc_v1)] + w.set_bypass(_bypass); }); regs.power().modify(|w| w.set_pwrctrl(PowerCtrl::On as u8)); @@ -1052,7 +1105,8 @@ impl SdmmcInner { _ => panic!("Invalid Bus Width"), }; - let (clkdiv, new_clock) = clk_div(ker_ck, freq)?; + let (_bypass, clkdiv, new_clock) = clk_div(ker_ck, freq)?; + // Enforce AHB and SDMMC_CK clock relation. See RM0433 Rev 7 // Section 55.5.8 let sdmmc_bus_bandwidth = new_clock.0 * width_u32; @@ -1063,7 +1117,11 @@ impl SdmmcInner { unsafe { // CPSMACT and DPSMACT must be 0 to set CLKDIV self.wait_idle(); - regs.clkcr().modify(|w| w.set_clkdiv(clkdiv)); + regs.clkcr().modify(|w| { + w.set_clkdiv(clkdiv); + #[cfg(sdmmc_v1)] + w.set_bypass(_bypass); + }); } Ok(()) @@ -1152,7 +1210,6 @@ impl SdmmcInner { } /// Query the card status (CMD13, returns R1) - /// fn read_status(&self, card: &Card) -> Result { let regs = self.0; let rca = card.rca; diff --git a/examples/stm32f4/src/bin/sdmmc.rs b/examples/stm32f4/src/bin/sdmmc.rs index b57e955f..1d0e60cb 100644 --- a/examples/stm32f4/src/bin/sdmmc.rs +++ b/examples/stm32f4/src/bin/sdmmc.rs @@ -17,6 +17,7 @@ const ALLOW_WRITES: bool = false; async fn main(_spawner: Spawner) -> ! { let mut config = Config::default(); config.rcc.sys_ck = Some(mhz(48)); + config.rcc.pll48 = true; let p = embassy_stm32::init(config); info!("Hello World!"); @@ -38,7 +39,7 @@ async fn main(_spawner: Spawner) -> ! { // Should print 400kHz for initialization info!("Configured clock: {}", sdmmc.clock().0); - unwrap!(sdmmc.init_card(mhz(24)).await); + unwrap!(sdmmc.init_card(mhz(48)).await); let card = unwrap!(sdmmc.card()); diff --git a/examples/stm32f7/src/bin/sdmmc.rs b/examples/stm32f7/src/bin/sdmmc.rs index 3bf427ec..cf8128e2 100644 --- a/examples/stm32f7/src/bin/sdmmc.rs +++ b/examples/stm32f7/src/bin/sdmmc.rs @@ -13,6 +13,7 @@ use {defmt_rtt as _, panic_probe as _}; async fn main(_spawner: Spawner) -> ! { let mut config = Config::default(); config.rcc.sys_ck = Some(mhz(200)); + config.rcc.pll48 = true; let p = embassy_stm32::init(config); info!("Hello World!"); From 896764bb8562b483ceecb39db2827061fc90d598 Mon Sep 17 00:00:00 2001 From: chemicstry Date: Thu, 23 Feb 2023 17:38:52 +0200 Subject: [PATCH 2/3] stm32/sdmmc: Refactor TypeId into a macro --- embassy-stm32/src/sdmmc/mod.rs | 100 ++++++++++++++++++++------------- 1 file changed, 61 insertions(+), 39 deletions(-) diff --git a/embassy-stm32/src/sdmmc/mod.rs b/embassy-stm32/src/sdmmc/mod.rs index 0bcd42bc..2d91286f 100644 --- a/embassy-stm32/src/sdmmc/mod.rs +++ b/embassy-stm32/src/sdmmc/mod.rs @@ -491,7 +491,7 @@ impl<'d, T: Instance, Dma: SdmmcDma> Sdmmc<'d, T, Dma> { bus_width, &mut self.card, &mut self.signalling, - Self::kernel_clock(), + T::kernel_clk(), &mut self.clock, T::state(), self.config.data_transfer_timeout, @@ -563,44 +563,6 @@ impl<'d, T: Instance, Dma: SdmmcDma> Sdmmc<'d, T, Dma> { regs.data_interrupts(false); state.wake(); } - - /// Returns kernel clock (SDIOCLK) for the SD-card facing domain - fn kernel_clock() -> Hertz { - cfg_if::cfg_if! { - // TODO, these could not be implemented, because required clocks are not exposed in RCC: - // - H7 uses pll1_q_ck or pll2_r_ck depending on SDMMCSEL - // - L1 uses pll48 - // - L4 uses clk48(pll48) - // - L4+, L5, U5 uses clk48(pll48) or PLLSAI3CLK(PLLP) depending on SDMMCSEL - if #[cfg(stm32f1)] { - // F1 uses AHB1(HCLK), which is correct in PAC - T::frequency() - } else if #[cfg(any(stm32f2, stm32f4))] { - // F2, F4 always use pll48 - critical_section::with(|_| unsafe { - crate::rcc::get_freqs().pll48 - }).expect("PLL48 is required for SDIO") - } else if #[cfg(stm32f7)] { - critical_section::with(|_| unsafe { - use core::any::TypeId; - let sdmmcsel = if TypeId::of::() == TypeId::of::() { - crate::pac::RCC.dckcfgr2().read().sdmmc1sel() - } else { - crate::pac::RCC.dckcfgr2().read().sdmmc2sel() - }; - - if sdmmcsel == crate::pac::rcc::vals::Sdmmcsel::SYSCLK { - crate::rcc::get_freqs().sys - } else { - crate::rcc::get_freqs().pll48.expect("PLL48 is required for SDMMC") - } - }) - } else { - // Use default peripheral clock and hope it works - T::frequency() - } - } - } } impl<'d, T: Instance, Dma> Drop for Sdmmc<'d, T, Dma> { @@ -1580,6 +1542,7 @@ pub(crate) mod sealed { fn inner() -> SdmmcInner; fn state() -> &'static AtomicWaker; + fn kernel_clk() -> Hertz; } pub trait Pins {} @@ -1607,6 +1570,61 @@ cfg_if::cfg_if! { } } +cfg_if::cfg_if! { + // TODO, these could not be implemented, because required clocks are not exposed in RCC: + // - H7 uses pll1_q_ck or pll2_r_ck depending on SDMMCSEL + // - L1 uses pll48 + // - L4 uses clk48(pll48) + // - L4+, L5, U5 uses clk48(pll48) or PLLSAI3CLK(PLLP) depending on SDMMCSEL + if #[cfg(stm32f1)] { + // F1 uses AHB1(HCLK), which is correct in PAC + macro_rules! kernel_clk { + ($inst:ident) => { + peripherals::$inst::frequency() + } + } + } else if #[cfg(any(stm32f2, stm32f4))] { + // F2, F4 always use pll48 + macro_rules! kernel_clk { + ($inst:ident) => { + critical_section::with(|_| unsafe { + crate::rcc::get_freqs().pll48 + }).expect("PLL48 is required for SDIO") + } + } + } else if #[cfg(stm32f7)] { + macro_rules! kernel_clk { + (SDMMC1) => { + critical_section::with(|_| unsafe { + let sdmmcsel = crate::pac::RCC.dckcfgr2().read().sdmmc1sel(); + if sdmmcsel == crate::pac::rcc::vals::Sdmmcsel::SYSCLK { + crate::rcc::get_freqs().sys + } else { + crate::rcc::get_freqs().pll48.expect("PLL48 is required for SDMMC") + } + }) + }; + (SDMMC2) => { + critical_section::with(|_| unsafe { + let sdmmcsel = crate::pac::RCC.dckcfgr2().read().sdmmc2sel(); + if sdmmcsel == crate::pac::rcc::vals::Sdmmcsel::SYSCLK { + crate::rcc::get_freqs().sys + } else { + crate::rcc::get_freqs().pll48.expect("PLL48 is required for SDMMC") + } + }) + }; + } + } else { + // Use default peripheral clock and hope it works + macro_rules! kernel_clk { + ($inst:ident) => { + peripherals::$inst::frequency() + } + } + } +} + foreach_peripheral!( (sdmmc, $inst:ident) => { impl sealed::Instance for peripherals::$inst { @@ -1621,6 +1639,10 @@ foreach_peripheral!( static WAKER: ::embassy_sync::waitqueue::AtomicWaker = ::embassy_sync::waitqueue::AtomicWaker::new(); &WAKER } + + fn kernel_clk() -> Hertz { + kernel_clk!($inst) + } } impl Instance for peripherals::$inst {} From 73ef85b7650eea65c2f52e570f26062dd8ec38d0 Mon Sep 17 00:00:00 2001 From: chemicstry Date: Thu, 23 Feb 2023 18:00:55 +0200 Subject: [PATCH 3/3] stm32/sdmmc: Fix compile errors --- embassy-stm32/src/sdmmc/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/embassy-stm32/src/sdmmc/mod.rs b/embassy-stm32/src/sdmmc/mod.rs index 2d91286f..03d24dcb 100644 --- a/embassy-stm32/src/sdmmc/mod.rs +++ b/embassy-stm32/src/sdmmc/mod.rs @@ -1580,7 +1580,7 @@ cfg_if::cfg_if! { // F1 uses AHB1(HCLK), which is correct in PAC macro_rules! kernel_clk { ($inst:ident) => { - peripherals::$inst::frequency() + ::frequency() } } } else if #[cfg(any(stm32f2, stm32f4))] { @@ -1619,7 +1619,7 @@ cfg_if::cfg_if! { // Use default peripheral clock and hope it works macro_rules! kernel_clk { ($inst:ident) => { - peripherals::$inst::frequency() + ::frequency() } } }