Cache bus clock register and support clock source

esp-hal-procmacros/src/lib.rs

@@ -429,7 +429,7 @@ pub fn main(args: TokenStream, item: TokenStream) -> TokenStream {
 /// ```
 ///
 /// [Builder Lite]: https://matklad.github.io/2022/05/29/builder-lite.html
-#[proc_macro_derive(BuilderLite)]
+#[proc_macro_derive(BuilderLite, attributes(builder_lite_into))]
 pub fn builder_lite_derive(item: TokenStream) -> TokenStream {
     let input = syn::parse_macro_input!(item as syn::DeriveInput);
 
@@ -455,12 +455,22 @@ pub fn builder_lite_derive(item: TokenStream) -> TokenStream {
                     _ => None,
                 });
 
-            let (field_type, field_assigns) = if let Some(inner_type) = maybe_path_type {
+            let (mut field_type, mut field_assigns) = if let Some(inner_type) = maybe_path_type {
-                (inner_type, quote! { Some(#field_ident) })
+                (quote! { #inner_type }, quote! { Some(#field_ident) })
             } else {
-                (field_type, quote! { #field_ident })
+                (quote! { #field_type }, quote! { #field_ident })
             };
 
+            // Wrap type and assignment with `Into` if needed.
+            if field
+                .attrs
+                .iter()
+                .any(|attr| attr.path().is_ident("builder_lite_into"))
+            {
+                field_type = quote! { impl Into<#field_type> };
+                field_assigns = quote! { #field_ident .into() };
+            }
+
             fns.push(quote! {
                 #[doc = concat!(" Assign the given value to the `", stringify!(#field_ident) ,"` field.")]
                 #[must_use]

esp-hal/src/dma/mod.rs

@@ -28,7 +28,7 @@
 //!
 //! let mut spi = Spi::new(
 //!     peripherals.SPI2,
-//!     Config::default().with_frequency(100.kHz()).with_mode(Mode::_0)
+//!     Config::default().with_clock(100.kHz()).with_mode(Mode::_0)
 //! )
 //! .unwrap()
 //! .with_sck(sclk)

esp-hal/src/spi/master.rs

@@ -44,7 +44,7 @@
 //!
 //! let mut spi = Spi::new(
 //!     peripherals.SPI2,
-//!     Config::default().with_frequency(100.kHz()).with_mode(Mode::_0)
+//!     Config::default().with_clock(100.kHz()).with_mode(Mode::_0)
 //! )
 //! .unwrap()
 //! .with_sck(sclk)
@@ -56,14 +56,14 @@
 //! [`embedded-hal-bus`]: https://docs.rs/embedded-hal-bus/latest/embedded_hal_bus/spi/index.html
 //! [`embassy-embedded-hal`]: https://docs.embassy.dev/embassy-embedded-hal/git/default/shared_bus/index.html
 
-use core::marker::PhantomData;
+use core::{cell::Cell, marker::PhantomData};
 
 #[instability::unstable]
 pub use dma::*;
 #[cfg(any(doc, feature = "unstable"))]
 use embassy_embedded_hal::SetConfig;
 use enumset::{EnumSet, EnumSetType};
-use fugit::HertzU32;
+use fugit::{HertzU32, RateExtU32};
 #[cfg(place_spi_driver_in_ram)]
 use procmacros::ram;
 
@@ -437,13 +437,190 @@ impl Address {
     }
 }
 
+/// SPI clock source.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum ClockSource {
+    /// Use the APB clock.
+    Apb,
+    // #[cfg(any(esp32c2, esp32c3, esp32s3))]
+    // Xtal,
+}
+
+/// Bus clock configuration.
+///
+/// This struct holds information necessary to configure the SPI bus clock.
+#[derive(Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub struct BusClockConfig {
+    /// The saved register value, calculated when first needed.
+    reg: Cell<Option<Result<u32, ConfigError>>>,
+
+    /// The target frequency
+    frequency: HertzU32,
+
+    /// The clock source
+    clock_source: ClockSource,
+}
+
+impl Clone for BusClockConfig {
+    fn clone(&self) -> Self {
+        _ = self.recalculate();
+
+        Self {
+            reg: self.reg.clone(),
+            frequency: self.frequency,
+            clock_source: self.clock_source,
+        }
+    }
+}
+
+impl Default for BusClockConfig {
+    fn default() -> Self {
+        BusClockConfig {
+            reg: Cell::new(None),
+            frequency: 1_u32.MHz(),
+            clock_source: ClockSource::Apb,
+        }
+    }
+}
+
+impl BusClockConfig {
+    /// Set the frequency of the SPI bus clock.
+    pub fn with_frequency(mut self, frequency: HertzU32) -> Self {
+        self.frequency = frequency;
+        self.reg.set(None);
+
+        self
+    }
+
+    /// Set the clock source of the SPI bus.
+    pub fn with_clock_source(mut self, clock_source: ClockSource) -> Self {
+        self.clock_source = clock_source;
+        self.reg.set(None);
+
+        self
+    }
+
+    fn recalculate(&self) -> Result<u32, ConfigError> {
+        if let Some(result) = self.reg.get() {
+            return result;
+        }
+
+        // taken from https://github.com/apache/incubator-nuttx/blob/8267a7618629838231256edfa666e44b5313348e/arch/risc-v/src/esp32c3/esp32c3_spi.c#L496
+
+        let clocks = Clocks::get();
+        cfg_if::cfg_if! {
+            if #[cfg(esp32h2)] {
+                // ESP32-H2 is using PLL_48M_CLK source instead of APB_CLK
+                let apb_clk_freq = HertzU32::Hz(clocks.pll_48m_clock.to_Hz());
+            } else {
+                let apb_clk_freq = HertzU32::Hz(clocks.apb_clock.to_Hz());
+            }
+        }
+
+        let reg_val: u32;
+        let duty_cycle = 128;
+
+        // In HW, n, h and l fields range from 1 to 64, pre ranges from 1 to 8K.
+        // The value written to register is one lower than the used value.
+
+        if self.frequency > ((apb_clk_freq / 4) * 3) {
+            // Using APB frequency directly will give us the best result here.
+            reg_val = 1 << 31;
+        } else {
+            // For best duty cycle resolution, we want n to be as close to 32 as
+            // possible, but we also need a pre/n combo that gets us as close as
+            // possible to the intended frequency. To do this, we bruteforce n and
+            // calculate the best pre to go along with that. If there's a choice
+            // between pre/n combos that give the same result, use the one with the
+            // higher n.
+
+            let mut pre: i32;
+            let mut bestn: i32 = -1;
+            let mut bestpre: i32 = -1;
+            let mut besterr: i32 = 0;
+            let mut errval: i32;
+
+            let raw_freq = self.frequency.raw() as i32;
+
+            // Start at n = 2. We need to be able to set h/l so we have at least
+            // one high and one low pulse.
+
+            for n in 2..64 {
+                // Effectively, this does:
+                // pre = round((APB_CLK_FREQ / n) / frequency)
+
+                pre = ((apb_clk_freq.raw() as i32 / n) + (raw_freq / 2)) / raw_freq;
+
+                if pre <= 0 {
+                    pre = 1;
+                }
+
+                if pre > 16 {
+                    pre = 16;
+                }
+
+                errval = (apb_clk_freq.raw() as i32 / (pre * n) - raw_freq).abs();
+                if bestn == -1 || errval <= besterr {
+                    besterr = errval;
+                    bestn = n;
+                    bestpre = pre;
+                }
+            }
+
+            let n: i32 = bestn;
+            pre = bestpre;
+            let l: i32 = n;
+
+            // Effectively, this does:
+            // h = round((duty_cycle * n) / 256)
+
+            let mut h: i32 = (duty_cycle * n + 127) / 256;
+            if h <= 0 {
+                h = 1;
+            }
+
+            reg_val = (l as u32 - 1)
+                | ((h as u32 - 1) << 6)
+                | ((n as u32 - 1) << 12)
+                | ((pre as u32 - 1) << 18);
+        }
+
+        self.reg.set(Some(Ok(reg_val)));
+        Ok(reg_val)
+    }
+
+    fn raw_clock_reg_value(&self) -> Result<u32, ConfigError> {
+        self.recalculate()
+    }
+}
+
+impl From<HertzU32> for BusClockConfig {
+    fn from(frequency: HertzU32) -> Self {
+        BusClockConfig {
+            frequency,
+            ..Default::default()
+        }
+    }
+}
+
+impl core::hash::Hash for BusClockConfig {
+    fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
+        self.reg.get().hash(state);
+        self.frequency.to_Hz().hash(state); // HertzU32 doesn't implement Hash
+        self.clock_source.hash(state);
+    }
+}
+
 /// SPI peripheral configuration
-#[derive(Clone, Copy, Debug, PartialEq, Eq, procmacros::BuilderLite)]
+#[derive(Clone, Debug, PartialEq, Eq, procmacros::BuilderLite)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 #[non_exhaustive]
 pub struct Config {
     /// SPI bus clock frequency.
-    pub frequency: HertzU32,
+    #[builder_lite_into]
+    pub clock: BusClockConfig,
 
     /// SPI sample/shift mode.
     pub mode: Mode,
@@ -455,20 +632,10 @@ pub struct Config {
     pub write_bit_order: BitOrder,
 }
 
-impl core::hash::Hash for Config {
-    fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
-        self.frequency.to_Hz().hash(state); // HertzU32 doesn't implement Hash
-        self.mode.hash(state);
-        self.read_bit_order.hash(state);
-        self.write_bit_order.hash(state);
-    }
-}
-
 impl Default for Config {
     fn default() -> Self {
-        use fugit::RateExtU32;
         Config {
-            frequency: 1_u32.MHz(),
+            clock: BusClockConfig::default(),
             mode: Mode::_0,
             read_bit_order: BitOrder::MsbFirst,
             write_bit_order: BitOrder::MsbFirst,
@@ -2621,90 +2788,6 @@ impl Driver {
         Ok(())
     }
 
-    // taken from https://github.com/apache/incubator-nuttx/blob/8267a7618629838231256edfa666e44b5313348e/arch/risc-v/src/esp32c3/esp32c3_spi.c#L496
-    fn setup(&self, frequency: HertzU32) {
-        let clocks = Clocks::get();
-        cfg_if::cfg_if! {
-            if #[cfg(esp32h2)] {
-                // ESP32-H2 is using PLL_48M_CLK source instead of APB_CLK
-                let apb_clk_freq = HertzU32::Hz(clocks.pll_48m_clock.to_Hz());
-            } else {
-                let apb_clk_freq = HertzU32::Hz(clocks.apb_clock.to_Hz());
-            }
-        }
-
-        let reg_val: u32;
-        let duty_cycle = 128;
-
-        // In HW, n, h and l fields range from 1 to 64, pre ranges from 1 to 8K.
-        // The value written to register is one lower than the used value.
-
-        if frequency > ((apb_clk_freq / 4) * 3) {
-            // Using APB frequency directly will give us the best result here.
-            reg_val = 1 << 31;
-        } else {
-            // For best duty cycle resolution, we want n to be as close to 32 as
-            // possible, but we also need a pre/n combo that gets us as close as
-            // possible to the intended frequency. To do this, we bruteforce n and
-            // calculate the best pre to go along with that. If there's a choice
-            // between pre/n combos that give the same result, use the one with the
-            // higher n.
-
-            let mut pre: i32;
-            let mut bestn: i32 = -1;
-            let mut bestpre: i32 = -1;
-            let mut besterr: i32 = 0;
-            let mut errval: i32;
-
-            // Start at n = 2. We need to be able to set h/l so we have at least
-            // one high and one low pulse.
-
-            for n in 2..64 {
-                // Effectively, this does:
-                // pre = round((APB_CLK_FREQ / n) / frequency)
-
-                pre = ((apb_clk_freq.raw() as i32 / n) + (frequency.raw() as i32 / 2))
-                    / frequency.raw() as i32;
-
-                if pre <= 0 {
-                    pre = 1;
-                }
-
-                if pre > 16 {
-                    pre = 16;
-                }
-
-                errval = (apb_clk_freq.raw() as i32 / (pre * n) - frequency.raw() as i32).abs();
-                if bestn == -1 || errval <= besterr {
-                    besterr = errval;
-                    bestn = n;
-                    bestpre = pre;
-                }
-            }
-
-            let n: i32 = bestn;
-            pre = bestpre;
-            let l: i32 = n;
-
-            // Effectively, this does:
-            // h = round((duty_cycle * n) / 256)
-
-            let mut h: i32 = (duty_cycle * n + 127) / 256;
-            if h <= 0 {
-                h = 1;
-            }
-
-            reg_val = (l as u32 - 1)
-                | ((h as u32 - 1) << 6)
-                | ((n as u32 - 1) << 12)
-                | ((pre as u32 - 1) << 18);
-        }
-
-        self.register_block()
-            .clock()
-            .write(|w| unsafe { w.bits(reg_val) });
-    }
-
     /// Enable or disable listening for the given interrupts.
     #[cfg_attr(not(feature = "unstable"), allow(dead_code))]
     fn enable_listen(&self, interrupts: EnumSet<SpiInterrupt>, enable: bool) {
@@ -2829,7 +2912,7 @@ impl Driver {
     }
 
     fn apply_config(&self, config: &Config) -> Result<(), ConfigError> {
-        self.ch_bus_freq(config.frequency);
+        self.ch_bus_freq(&config.clock)?;
         self.set_bit_order(config.read_bit_order, config.write_bit_order);
         self.set_data_mode(config.mode);
         Ok(())
@@ -2856,22 +2939,28 @@ impl Driver {
         });
     }
 
-    fn ch_bus_freq(&self, frequency: HertzU32) {
+    fn ch_bus_freq(&self, bus_clock_config: &BusClockConfig) -> Result<(), ConfigError> {
         fn enable_clocks(_reg_block: &RegisterBlock, _enable: bool) {
             #[cfg(gdma)]
             _reg_block.clk_gate().modify(|_, w| {
                 w.clk_en().bit(_enable);
                 w.mst_clk_active().bit(_enable);
-                w.mst_clk_sel().bit(_enable)
+                w.mst_clk_sel().bit(true) // TODO: support XTAL clock source
             });
         }
 
+        // Change clock frequency
+        let raw = bus_clock_config.raw_clock_reg_value()?;
+
         enable_clocks(self.register_block(), false);
 
-        // Change clock frequency
+        self.register_block()
-        self.setup(frequency);
+            .clock()
+            .write(|w| unsafe { w.bits(raw) });
 
         enable_clocks(self.register_block(), true);
+
+        Ok(())
     }
 
     #[cfg(not(any(esp32, esp32c3, esp32s2)))]

examples/src/bin/embassy_spi.rs

@@ -59,9 +59,7 @@ async fn main(_spawner: Spawner) {
 
     let mut spi = Spi::new(
         peripherals.SPI2,
-        Config::default()
+        Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-            .with_frequency(100.kHz())
-            .with_mode(Mode::_0),
     )
     .unwrap()
     .with_sck(sclk)

examples/src/bin/spi_loopback.rs

@@ -41,9 +41,7 @@ fn main() -> ! {
 
     let mut spi = Spi::new(
         peripherals.SPI2,
-        Config::default()
+        Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-            .with_frequency(100.kHz())
-            .with_mode(Mode::_0),
     )
     .unwrap()
     .with_sck(sclk)

examples/src/bin/spi_loopback_dma_psram.rs

@@ -91,9 +91,7 @@ fn main() -> ! {
     // output connection (because we are using the same pin to loop back)
     let mut spi = Spi::new(
         peripherals.SPI2,
-        Config::default()
+        Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-            .with_frequency(100.kHz())
-            .with_mode(Mode::_0),
     )
     .unwrap()
     .with_sck(sclk)

hil-test/tests/embassy_interrupt_spi_dma.rs

@@ -120,7 +120,7 @@ mod test {
         let mut spi = Spi::new(
             peripherals.SPI2,
             Config::default()
-                .with_frequency(10000.kHz())
+                .with_clock(10000.kHz())
                 .with_mode(Mode::_0),
         )
         .unwrap()
@@ -134,7 +134,7 @@ mod test {
         let other_peripheral = Spi::new(
             peripherals.SPI3,
             Config::default()
-                .with_frequency(10000.kHz())
+                .with_clock(10000.kHz())
                 .with_mode(Mode::_0),
         )
         .unwrap()
@@ -227,9 +227,7 @@ mod test {
 
             let mut spi = Spi::new(
                 peripherals.spi,
-                Config::default()
+                Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-                    .with_frequency(100.kHz())
-                    .with_mode(Mode::_0),
             )
             .unwrap()
             .with_dma(peripherals.dma_channel)

hil-test/tests/qspi.rs

@@ -211,9 +211,7 @@ mod tests {
 
         let spi = Spi::new(
             peripherals.SPI2,
-            Config::default()
+            Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-                .with_frequency(100.kHz())
-                .with_mode(Mode::_0),
         )
         .unwrap();
 

hil-test/tests/spi_full_duplex.rs

@@ -73,7 +73,7 @@ mod tests {
 
         // Need to set miso first so that mosi can overwrite the
         // output connection (because we are using the same pin to loop back)
-        let spi = Spi::new(peripherals.SPI2, Config::default().with_frequency(10.MHz()))
+        let spi = Spi::new(peripherals.SPI2, Config::default().with_clock(10.MHz()))
             .unwrap()
             .with_sck(sclk)
             .with_miso(miso)
@@ -194,7 +194,7 @@ mod tests {
         let mut spi = ctx.spi.into_async();
 
         // Slow down SCLK so that transferring the buffer takes a while.
-        spi.apply_config(&Config::default().with_frequency(80.kHz()))
+        spi.apply_config(&Config::default().with_clock(80.kHz()))
             .expect("Apply config failed");
 
         SpiBus::write(&mut spi, &write[..]).expect("Sync write failed");
@@ -661,7 +661,7 @@ mod tests {
         // This means that without working cancellation, the test case should
         // fail.
         ctx.spi
-            .apply_config(&Config::default().with_frequency(80.kHz()))
+            .apply_config(&Config::default().with_clock(80.kHz()))
             .unwrap();
 
         // Set up a large buffer that would trigger a timeout
@@ -683,7 +683,7 @@ mod tests {
     fn can_transmit_after_cancel(mut ctx: Context) {
         // Slow down. At 80kHz, the transfer is supposed to take a bit over 3 seconds.
         ctx.spi
-            .apply_config(&Config::default().with_frequency(80.kHz()))
+            .apply_config(&Config::default().with_clock(80.kHz()))
             .unwrap();
 
         // Set up a large buffer that would trigger a timeout
@@ -700,7 +700,7 @@ mod tests {
         transfer.cancel();
         (spi, (dma_rx_buf, dma_tx_buf)) = transfer.wait();
 
-        spi.apply_config(&Config::default().with_frequency(10.MHz()))
+        spi.apply_config(&Config::default().with_clock(10.MHz()))
             .unwrap();
 
         let transfer = spi

hil-test/tests/spi_half_duplex_read.rs

@@ -49,9 +49,7 @@ mod tests {
 
         let spi = Spi::new(
             peripherals.SPI2,
-            Config::default()
+            Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-                .with_frequency(100.kHz())
-                .with_mode(Mode::_0),
         )
         .unwrap()
         .with_sck(sclk)

hil-test/tests/spi_half_duplex_write.rs

@@ -53,9 +53,7 @@ mod tests {
 
         let spi = Spi::new(
             peripherals.SPI2,
-            Config::default()
+            Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-                .with_frequency(100.kHz())
-                .with_mode(Mode::_0),
         )
         .unwrap()
         .with_sck(sclk)

hil-test/tests/spi_half_duplex_write_psram.rs

@@ -65,9 +65,7 @@ mod tests {
 
         let spi = Spi::new(
             peripherals.SPI2,
-            Config::default()
+            Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-                .with_frequency(100.kHz())
-                .with_mode(Mode::_0),
         )
         .unwrap()
         .with_sck(sclk)

qa-test/src/bin/qspi_flash.rs

@@ -79,9 +79,7 @@ fn main() -> ! {
 
     let mut spi = Spi::new(
         peripherals.SPI2,
-        Config::default()
+        Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-            .with_frequency(100.kHz())
-            .with_mode(Mode::_0),
     )
     .unwrap()
     .with_sck(sclk)

qa-test/src/bin/spi_halfduplex_read_manufacturer_id.rs

@@ -65,9 +65,7 @@ fn main() -> ! {
 
     let mut spi = Spi::new(
         peripherals.SPI2,
-        Config::default()
+        Config::default().with_clock(100.kHz()).with_mode(Mode::_0),
-            .with_frequency(100.kHz())
-            .with_mode(Mode::_0),
     )
     .unwrap()
     .with_sck(sclk)