From c438ee08274093eed18812fadb6a4c91bb9ff53c Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Sun, 1 Dec 2024 11:54:58 -0500 Subject: [PATCH 1/9] Run tests with miri We're testing unsafe code in the HAL. Make sure miri is happy with that unsafe code. We build with minimal optimizations by default, no matter the target. Turn those off during the miri invocation. --- .github/workflows/rust.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index dea14b8f..11588a65 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -118,6 +118,24 @@ jobs: - name: Run unit, integration tests run: cargo test --features=${{ matrix.chips }} --tests --package=imxrt-hal --package=imxrt-log + # Make sure our unit tests can pass (some recent version of) miri. + miri: + needs: tests + strategy: + matrix: + chips: + - imxrt-ral/imxrt1011,imxrt1010 + - imxrt-ral/imxrt1021,imxrt1020 + - imxrt-ral/imxrt1062,imxrt1060 + - imxrt-ral/imxrt1176_cm7,imxrt1170 + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install a nightly toolchain with miri + run: rustup toolchain install nightly --no-self-update --profile minimal --component miri + - name: Run unit, integration tests with miri + run: cargo +nightly miri test --features=${{ matrix.chips }} --tests --package=imxrt-hal --package=imxrt-log --config "profile.dev.opt-level = 0" + # Ensures that documentation builds, and that links are valid docs: needs: format From 8f23840a096fdf0f6b5b79d15591d6ab437339dc Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Fri, 17 Mar 2023 19:32:24 -0400 Subject: [PATCH 2/9] Remove PCS0 from Lpspi::new pin requirements When we support the embedded-hal 1.0 traits, we should distinguish between the bus -- having the data and clock pins -- and the device(s), which have the chip select. Removing the chip select type state is the first step in supporting this interface. Temporarily enable PCS0 in the board setup routines. This keeps the examples working as expected. --- CHANGELOG.md | 5 ++++- board/src/imxrt1010evk.rs | 9 +++++++-- board/src/imxrt1060evk.rs | 8 ++++++-- board/src/imxrt1170evk-cm7.rs | 8 ++++++-- board/src/teensy4.rs | 9 +++++++-- src/common/lpspi.rs | 14 +++----------- 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8770f33d..527816b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,10 @@ - `timer::*PitChan` - `lpspi::Disabled::{set_mode, set_watermark}` -**BREAKING** `LpspiError::{Busy, NoData}` are removed as possible LPSPI errors. +**BREAKING** Change the LPSPI driver: + +- `LpspiError::{Busy, NoData}` are removed as possible LPSPI errors. +- There is no more `PCS0` type state associated with the LPSPI bus. ## [0.5.9] 2024-11-24 diff --git a/board/src/imxrt1010evk.rs b/board/src/imxrt1010evk.rs index 6c556aa4..09704017 100644 --- a/board/src/imxrt1010evk.rs +++ b/board/src/imxrt1010evk.rs @@ -64,9 +64,11 @@ pub type SpiPins = hal::lpspi::Pins< iomuxc::gpio_ad::GPIO_AD_04, // SDO, J57_8 iomuxc::gpio_ad::GPIO_AD_03, // SDI, J57_10 iomuxc::gpio_ad::GPIO_AD_06, // SCK, J57_12 - iomuxc::gpio_ad::GPIO_AD_05, // PCS0, J57_6 >; +/// SPI PCS0 (J57_6). +pub type SpiPcs0 = iomuxc::gpio_ad::GPIO_AD_05; + #[cfg(feature = "spi")] pub type Spi = hal::lpspi::Lpspi; @@ -192,8 +194,11 @@ impl Specifics { sdo: iomuxc.gpio_ad.p04, sdi: iomuxc.gpio_ad.p03, sck: iomuxc.gpio_ad.p06, - pcs0: iomuxc.gpio_ad.p05, }; + crate::iomuxc::lpspi::prepare({ + let pcs0: &mut SpiPcs0 = &mut iomuxc.gpio_ad.p05; + pcs0 + }); let mut spi = Spi::new(lpspi1, pins); spi.disabled(|spi| { spi.set_clock_hz(super::LPSPI_CLK_FREQUENCY, super::SPI_BAUD_RATE_FREQUENCY); diff --git a/board/src/imxrt1060evk.rs b/board/src/imxrt1060evk.rs index 4afbb5f2..6689ce2c 100644 --- a/board/src/imxrt1060evk.rs +++ b/board/src/imxrt1060evk.rs @@ -62,8 +62,9 @@ pub type SpiPins = hal::lpspi::Pins< iomuxc::gpio_sd_b0::GPIO_SD_B0_02, // SDO, J24_4 iomuxc::gpio_sd_b0::GPIO_SD_B0_03, // SDI, J24_5 iomuxc::gpio_sd_b0::GPIO_SD_B0_00, // SCK, J24_6 - iomuxc::gpio_sd_b0::GPIO_SD_B0_01, // PCS0, J24_3 >; +/// SPI PCS0 (J24_3). +pub type SpiPcs0 = iomuxc::gpio_sd_b0::GPIO_SD_B0_01; #[cfg(not(feature = "spi"))] /// Activate the `"spi"` feature to configure the SPI peripheral. @@ -180,8 +181,11 @@ impl Specifics { sdo: iomuxc.gpio_sd_b0.p02, sdi: iomuxc.gpio_sd_b0.p03, sck: iomuxc.gpio_sd_b0.p00, - pcs0: iomuxc.gpio_sd_b0.p01, }; + crate::iomuxc::lpspi::prepare({ + let pcs0: &mut SpiPcs0 = &mut iomuxc.gpio_sd_b0.p01; + pcs0 + }); let mut spi = Spi::new(lpspi1, pins); spi.disabled(|spi| { spi.set_clock_hz(super::LPSPI_CLK_FREQUENCY, super::SPI_BAUD_RATE_FREQUENCY); diff --git a/board/src/imxrt1170evk-cm7.rs b/board/src/imxrt1170evk-cm7.rs index 3e5a734b..5136fd23 100644 --- a/board/src/imxrt1170evk-cm7.rs +++ b/board/src/imxrt1170evk-cm7.rs @@ -92,8 +92,9 @@ pub type SpiPins = hal::lpspi::Pins< iomuxc::gpio_ad::GPIO_AD_30, // SDO, J10_8 iomuxc::gpio_ad::GPIO_AD_31, // SDI, J10_10 iomuxc::gpio_ad::GPIO_AD_28, // SCK, J10_12 - iomuxc::gpio_ad::GPIO_AD_29, // PCS0, J10_6 >; +/// SPI PCS0 (J10_6). +pub type SpiPcs0 = iomuxc::gpio_ad::GPIO_AD_29; const SPI_INSTANCE: u8 = 1; #[cfg(feature = "spi")] @@ -207,8 +208,11 @@ impl Specifics { sdo: iomuxc.gpio_ad.p30, sdi: iomuxc.gpio_ad.p31, sck: iomuxc.gpio_ad.p28, - pcs0: iomuxc.gpio_ad.p29, }; + crate::iomuxc::lpspi::prepare({ + let pcs0: &mut SpiPcs0 = &mut iomuxc.gpio_ad.p29; + pcs0 + }); let mut spi = Spi::new(lpspi1, pins); spi.disabled(|spi| { spi.set_clock_hz(LPSPI_CLK_FREQUENCY, super::SPI_BAUD_RATE_FREQUENCY); diff --git a/board/src/teensy4.rs b/board/src/teensy4.rs index 7b71f3ab..1d8bf612 100644 --- a/board/src/teensy4.rs +++ b/board/src/teensy4.rs @@ -49,9 +49,11 @@ pub type SpiPins = hal::lpspi::Pins< iomuxc::gpio_b0::GPIO_B0_02, // SDO, P11 iomuxc::gpio_b0::GPIO_B0_01, // SDI, P12 iomuxc::gpio_b0::GPIO_B0_03, // SCK, P13 - iomuxc::gpio_b0::GPIO_B0_00, // PCS0, P10 >; +/// SPI PCS0 (P10). +pub type SpiPcs0 = iomuxc::gpio_b0::GPIO_B0_00; + #[cfg(not(feature = "spi"))] /// Activate the `"spi"` feature to configure the SPI peripheral. pub type Spi = (); @@ -152,8 +154,11 @@ impl Specifics { sdo: iomuxc.gpio_b0.p02, sdi: iomuxc.gpio_b0.p01, sck: iomuxc.gpio_b0.p03, - pcs0: iomuxc.gpio_b0.p00, }; + crate::iomuxc::lpspi::prepare({ + let pcs0: &mut SpiPcs0 = &mut iomuxc.gpio_b0.p00; + pcs0 + }); let mut spi = Spi::new(lpspi4, pins); spi.disabled(|spi| { spi.set_clock_hz(super::LPSPI_CLK_FREQUENCY, super::SPI_BAUD_RATE_FREQUENCY); diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 5078e6d3..77e08769 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -48,7 +48,6 @@ //! sdo: pads.gpio_b0.p02, //! sdi: pads.gpio_b0.p01, //! sck: pads.gpio_b0.p03, -//! pcs0: pads.gpio_b0.p00, //! }; //! //! let mut spi4 = unsafe { LPSPI4::instance() }; @@ -399,13 +398,12 @@ pub struct Lpspi { /// GPIO_B0_02, /// GPIO_B0_01, /// GPIO_B0_03, -/// GPIO_B0_00, /// >; /// /// // Helper type for your SPI peripheral /// type Lpspi = hal::lpspi::Lpspi; /// ``` -pub struct Pins { +pub struct Pins { /// Serial data out /// /// Data travels from the SPI host controller to the SPI device. @@ -416,29 +414,23 @@ pub struct Pins { pub sdi: SDI, /// Serial clock pub sck: SCK, - /// Chip select 0 - /// - /// (PCSx) convention matches the hardware. - pub pcs0: PCS0, } -impl Lpspi, N> +impl Lpspi, N> where SDO: lpspi::Pin, Signal = lpspi::Sdo>, SDI: lpspi::Pin, Signal = lpspi::Sdi>, SCK: lpspi::Pin, Signal = lpspi::Sck>, - PCS0: lpspi::Pin, Signal = lpspi::Pcs0>, { /// Create a new LPSPI driver from the RAL LPSPI instance and a set of pins. /// /// When this call returns, the LPSPI pins are configured for their function. /// The peripheral is enabled after reset. The LPSPI clock speed is unspecified. /// The mode is [`MODE_0`]. The sample point is [`SamplePoint::DelayedEdge`]. - pub fn new(lpspi: ral::lpspi::Instance, mut pins: Pins) -> Self { + pub fn new(lpspi: ral::lpspi::Instance, mut pins: Pins) -> Self { lpspi::prepare(&mut pins.sdo); lpspi::prepare(&mut pins.sdi); lpspi::prepare(&mut pins.sck); - lpspi::prepare(&mut pins.pcs0); Self::init(lpspi, pins) } } From c33f037efcdeca598edacd755912e4873ccba0ae Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Fri, 17 Mar 2023 19:43:27 -0400 Subject: [PATCH 3/9] Add runtime selection for LPSPI chip select We'll need something like this when we implement SPI devices. --- CHANGELOG.md | 2 ++ src/common/lpspi.rs | 29 +++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 527816b8..31cdd8e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ - `LpspiError::{Busy, NoData}` are removed as possible LPSPI errors. - There is no more `PCS0` type state associated with the LPSPI bus. +Introduce a hardware chip select into each LPSPI transaction. + ## [0.5.9] 2024-11-24 Correct LPSPI receive operations. Previously, `u8` and `u16` elements received diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 77e08769..324e17b4 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -133,6 +133,25 @@ pub enum LpspiError { Fifo(Direction), } +/// Peripheral chip select. +/// +/// Use this in [`Transaction`] to select the hardware-managed +/// chip select. Note that the hardware only uses the hardware-managed +/// chip select if the pin is muxed for its PCS alternate. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[repr(u32)] +pub enum Pcs { + /// Use PCS0 (default) for the next transaction. + #[default] + Pcs0, + /// Use PCS1 for the next transaction. + Pcs1, + /// Use PCS2 for the next transaction. + Pcs2, + /// Use PCS3 for the next transaction. + Pcs3, +} + /// An LPSPI transaction definition. /// /// The transaction defines how many bits the driver sends or recieves. @@ -142,6 +161,7 @@ pub enum LpspiError { /// - bit order /// - transmit and receive masking /// - continuous and continuing transfers (default: both disabled) +/// - the hardware-managed peripheral chip select, [`Pcs`] /// /// The LPSPI enqueues the transaction data into the transmit /// FIFO. When it pops the values from the FIFO, the values take @@ -241,7 +261,10 @@ pub struct Transaction { /// `Transaction`, one that had [`continuous`](Self::continuous) set. /// The default value is `false`. pub continuing: bool, - + /// Selects the hardware-managed peripheral chip select for the transaction. + /// + /// See [`Pcs`] for more information. + pub pcs: Pcs, frame_size: u16, } @@ -300,6 +323,7 @@ impl Transaction { frame_size: frame_size - 1, continuing: false, continuous: false, + pcs: Default::default(), }) } else { Err(LpspiError::FrameSize) @@ -741,7 +765,8 @@ impl Lpspi { TXMSK: transaction.transmit_data_mask as u32, FRAMESZ: transaction.frame_size as u32, CONT: transaction.continuous as u32, - CONTC: transaction.continuing as u32 + CONTC: transaction.continuing as u32, + PCS: transaction.pcs as u32 ); } From 2f71694732924a032decf0a269f7b865fb2df970 Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Sun, 10 Nov 2024 12:05:51 -0500 Subject: [PATCH 4/9] Consolidate LPSPI bus-derived transaction data This'll make it easier to associate bus state with each transaction. --- src/chip/dma.rs | 13 +++++-------- src/common/lpspi.rs | 13 +++++++++---- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/chip/dma.rs b/src/chip/dma.rs index 385ec6ea..26071dab 100644 --- a/src/chip/dma.rs +++ b/src/chip/dma.rs @@ -196,10 +196,9 @@ impl lpspi::Lpspi { channel: &'a mut Channel, buffer: &'a [u32], ) -> Result, lpspi::LpspiError> { - let mut transaction = lpspi::Transaction::new_u32s(buffer)?; - transaction.bit_order = self.bit_order(); - + let mut transaction = self.bus_transaction(buffer)?; transaction.receive_data_mask = true; + self.wait_for_transmit_fifo_space()?; self.enqueue_transaction(&transaction); Ok(peripheral::write(channel, buffer, self)) @@ -216,10 +215,9 @@ impl lpspi::Lpspi { channel: &'a mut Channel, buffer: &'a mut [u32], ) -> Result, lpspi::LpspiError> { - let mut transaction = lpspi::Transaction::new_u32s(buffer)?; - transaction.bit_order = self.bit_order(); - + let mut transaction = self.bus_transaction(buffer)?; transaction.transmit_data_mask = true; + self.wait_for_transmit_fifo_space()?; self.enqueue_transaction(&transaction); Ok(peripheral::read(channel, self, buffer)) @@ -238,8 +236,7 @@ impl lpspi::Lpspi { tx: &'a mut Channel, buffer: &'a mut [u32], ) -> Result, lpspi::LpspiError> { - let mut transaction = lpspi::Transaction::new_u32s(buffer)?; - transaction.bit_order = self.bit_order(); + let transaction = self.bus_transaction(buffer)?; self.wait_for_transmit_fifo_space()?; self.enqueue_transaction(&transaction); diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 324e17b4..2d980367 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -806,8 +806,7 @@ impl Lpspi { return Ok(()); } - let mut transaction = Transaction::new_words(data)?; - transaction.bit_order = self.bit_order(); + let transaction = self.bus_transaction(data)?; self.wait_for_transmit_fifo_space()?; self.enqueue_transaction(&transaction); @@ -831,9 +830,8 @@ impl Lpspi { return Ok(()); } - let mut transaction = Transaction::new_words(data)?; + let mut transaction = self.bus_transaction(data)?; transaction.receive_data_mask = true; - transaction.bit_order = self.bit_order(); self.wait_for_transmit_fifo_space()?; self.enqueue_transaction(&transaction); @@ -988,6 +986,13 @@ impl Lpspi { sckdiv: sckdiv as u8, } } + + /// Produce a transaction that considers bus-managed software state. + pub(crate) fn bus_transaction(&self, words: &[W]) -> Result { + let mut transaction = Transaction::new_words(words)?; + transaction.bit_order = self.bit_order(); + Ok(transaction) + } } bitflags::bitflags! { From 177925cd3cbb65d244395a7382ce1791a4cf0d85 Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Sun, 10 Nov 2024 12:12:42 -0500 Subject: [PATCH 5/9] Attach SPI mode to transactions Default behavior is unchanged, in that the mode is still set by the user's `Lpspi::set_mode` value. But now users could define it as part of a one-off transaction. --- CHANGELOG.md | 2 +- src/common/lpspi.rs | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31cdd8e8..c91ca5ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ - `LpspiError::{Busy, NoData}` are removed as possible LPSPI errors. - There is no more `PCS0` type state associated with the LPSPI bus. -Introduce a hardware chip select into each LPSPI transaction. +Introduce a hardware chip select and SPI mode into each LPSPI transaction. ## [0.5.9] 2024-11-24 diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 2d980367..cd1cb2ee 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -261,6 +261,10 @@ pub struct Transaction { /// `Transaction`, one that had [`continuous`](Self::continuous) set. /// The default value is `false`. pub continuing: bool, + /// The SPI mode for the transaction. + /// + /// By default, this is [`MODE_0`]. + pub mode: Mode, /// Selects the hardware-managed peripheral chip select for the transaction. /// /// See [`Pcs`] for more information. @@ -323,6 +327,7 @@ impl Transaction { frame_size: frame_size - 1, continuing: false, continuous: false, + mode: MODE_0, pcs: Default::default(), }) } else { @@ -752,10 +757,10 @@ impl Lpspi { /// /// You're responsible for making sure there's space in the transmit /// FIFO for this transaction command. - pub fn enqueue_transaction(&mut self, transaction: &Transaction) { + pub fn enqueue_transaction(&self, transaction: &Transaction) { ral::write_reg!(ral::lpspi, self.lpspi, TCR, - CPOL: if self.mode.polarity == Polarity::IdleHigh { CPOL_1 } else { CPOL_0 }, - CPHA: if self.mode.phase == Phase::CaptureOnSecondTransition { CPHA_1 } else { CPHA_0 }, + CPOL: if transaction.mode.polarity == Polarity::IdleHigh { CPOL_1 } else { CPOL_0 }, + CPHA: if transaction.mode.phase == Phase::CaptureOnSecondTransition { CPHA_1 } else { CPHA_0 }, PRESCALE: PRESCALE_0, PCS: PCS_0, WIDTH: WIDTH_0, @@ -991,6 +996,7 @@ impl Lpspi { pub(crate) fn bus_transaction(&self, words: &[W]) -> Result { let mut transaction = Transaction::new_words(words)?; transaction.bit_order = self.bit_order(); + transaction.mode = self.mode; Ok(transaction) } } From dc81c0a1e1ac6adc8a77676986ef2f446574bad4 Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Wed, 27 Nov 2024 20:46:50 -0500 Subject: [PATCH 6/9] Add hardware chip select polarity setting --- CHANGELOG.md | 1 + src/common/lpspi.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c91ca5ed..d98509f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - There is no more `PCS0` type state associated with the LPSPI bus. Introduce a hardware chip select and SPI mode into each LPSPI transaction. +Add an LPSPI configuration for hardware chip selects. ## [0.5.9] 2024-11-24 diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index cd1cb2ee..2c263b17 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -152,6 +152,26 @@ pub enum Pcs { Pcs3, } +/// The hardware chip select polarity. +/// +/// Use [`Disabled::set_chip_select_polarity`] to configure +/// each chip select's polarity. Consult your peripheral's +/// documentation to understand which polarity is expected. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[repr(u32)] +pub enum PcsPolarity { + /// The chip select is active low. + /// + /// When idle, the chip select is high. This is + /// the default state. + #[default] + ActiveLow, + /// The chip select is active high. + /// + /// When idle, the chip select is low. + ActiveHigh, +} + /// An LPSPI transaction definition. /// /// The transaction defines how many bits the driver sends or recieves. @@ -1214,6 +1234,21 @@ impl<'a, const N: u8> Disabled<'a, N> { pub fn set_peripheral_enable(&mut self, enable: bool) { ral::modify_reg!(ral::lpspi, self.lpspi, CFGR1, MASTER: !enable as u32); } + + /// Set the polarity for the `pcs` hardware chip select. + /// + /// By default, all polarities are active low. + #[inline] + pub fn set_chip_select_polarity(&mut self, pcs: Pcs, polarity: PcsPolarity) { + let pcspol = ral::read_reg!(ral::lpspi, self.lpspi, CFGR1, PCSPOL); + let mask = 1 << pcs as u32; + let pcspol = if polarity == PcsPolarity::ActiveHigh { + pcspol | mask + } else { + pcspol & !mask + }; + ral::modify_reg!(ral::lpspi, self.lpspi, CFGR1, PCSPOL: pcspol); + } } impl Drop for Disabled<'_, N> { From 402580a7320b046597af4a3886f571948eb7574e Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Sun, 1 Dec 2024 11:18:37 -0500 Subject: [PATCH 7/9] Cache LPSPI CCR[DBT] and CCR[SCKDIV] in memory On the RT1180, these two fields are WO/RAZ. Since we can't trust the hardware to retrieve these values for us, we cache them in memory. Writes will behave as before. The driver ensures that all accesses on CCR pass through the cache. --- src/common/lpspi.rs | 84 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 64 insertions(+), 20 deletions(-) diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 2c263b17..3b050468 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -359,7 +359,7 @@ impl Transaction { /// Sets the clock speed parameters. /// /// This should only happen when the LPSPI peripheral is disabled. -fn set_spi_clock(source_clock_hz: u32, spi_clock_hz: u32, reg: &ral::lpspi::RegisterBlock) { +fn compute_spi_clock(source_clock_hz: u32, spi_clock_hz: u32) -> ClockConfigs { // Round up, so we always get a resulting SPI clock that is // equal or less than the requested frequency. let half_div = @@ -372,16 +372,16 @@ fn set_spi_clock(source_clock_hz: u32, spi_clock_hz: u32, reg: &ral::lpspi::Regi // Because half_div is in range [3,128], sckdiv is in range [4, 254]. let sckdiv = 2 * (half_div - 1); - ral::write_reg!(ral::lpspi, reg, CCR, + ClockConfigs { // Delay between two clock transitions of two consecutive transfers // is exactly sckdiv/2, which causes the transfer to be seamless. - DBT: half_div - 1, + dbt: (half_div - 1) as u8, // Add one sckdiv/2 setup and hold time before and after the transfer, // to make sure the signal is stable at sample time - PCSSCK: half_div - 1, - SCKPCS: half_div - 1, - SCKDIV: sckdiv - ); + pcssck: (half_div - 1) as u8, + sckpcs: (half_div - 1) as u8, + sckdiv: sckdiv as u8, + } } /// LPSPI clock configurations. @@ -416,6 +416,47 @@ pub struct ClockConfigs { pub sckdiv: u8, } +impl ClockConfigs { + const fn as_raw(self) -> u32 { + use ral::lpspi::CCR; + (self.sckpcs as u32) << CCR::SCKPCS::offset + | (self.pcssck as u32) << CCR::PCSSCK::offset + | (self.dbt as u32) << CCR::DBT::offset + | (self.sckdiv as u32) << CCR::SCKDIV::offset + } +} + +/// In-memory clock configuration register values. +/// +/// Newer LPSPI IP blocks, like those found on the 1180, have `CCR[DBT]` +/// and `CCR[SCKDIV]` fields that are WO/RAZ. RAZ is incompatible +/// with older IP blocks, like those found on all the other MCUs. +/// +/// If we cache these values, all RMW on CCR will behave as if we +/// read those values through the register. +struct CcrCache { + dbt: u8, + sckdiv: u8, +} + +impl CcrCache { + /// Reac the clock configuration values, considering the cached values. + fn read_ccr(&self, lpspi: &ral::lpspi::RegisterBlock) -> ClockConfigs { + let (sckpcs, pcssck) = ral::read_reg!(ral::lpspi, lpspi, CCR, SCKPCS, PCSSCK); + ClockConfigs { + sckpcs: sckpcs as u8, + pcssck: pcssck as u8, + dbt: self.dbt, + sckdiv: self.sckdiv, + } + } + /// Update the cached values. + fn update(&mut self, clock_configs: ClockConfigs) { + self.dbt = clock_configs.dbt; + self.sckdiv = clock_configs.sckdiv; + } +} + /// An LPSPI driver. /// /// The driver exposes low-level methods for coordinating @@ -432,6 +473,7 @@ pub struct Lpspi { pins: P, bit_order: BitOrder, mode: Mode, + ccr_cache: CcrCache, } /// Pins for a LPSPI device. @@ -505,6 +547,8 @@ impl Lpspi { pins, bit_order: BitOrder::default(), mode: MODE_0, + // Once we issue a reset, below, these are zero. + ccr_cache: CcrCache { dbt: 0, sckdiv: 0 }, }; // Reset and disable @@ -593,7 +637,7 @@ impl Lpspi { /// The handle to a [`Disabled`](crate::lpspi::Disabled) driver lets you modify /// LPSPI settings that require a fully disabled peripheral. pub fn disabled(&mut self, func: impl FnOnce(&mut Disabled) -> R) -> R { - let mut disabled = Disabled::new(&mut self.lpspi); + let mut disabled = Disabled::new(&mut self.lpspi, &mut self.ccr_cache); func(&mut disabled) } @@ -945,7 +989,7 @@ impl Lpspi { let cfgr1 = ral::read_reg!(ral::lpspi, self.lpspi, CFGR1); let dmr0 = ral::read_reg!(ral::lpspi, self.lpspi, DMR0); let dmr1 = ral::read_reg!(ral::lpspi, self.lpspi, DMR1); - let ccr = ral::read_reg!(ral::lpspi, self.lpspi, CCR); + let ccr = self.clock_configs().as_raw(); let fcr = ral::read_reg!(ral::lpspi, self.lpspi, FCR); // Backup enabled state @@ -1002,14 +1046,7 @@ impl Lpspi { /// These values are decided by calls to [`set_clock_hz`](Disabled::set_clock_hz) /// and [`set_clock_configs`](Disabled::set_clock_configs). pub fn clock_configs(&self) -> ClockConfigs { - let (sckpcs, pcssck, dbt, sckdiv) = - ral::read_reg!(ral::lpspi, self.lpspi, CCR, SCKPCS, PCSSCK, DBT, SCKDIV); - ClockConfigs { - sckpcs: sckpcs as u8, - pcssck: pcssck as u8, - dbt: dbt as u8, - sckdiv: sckdiv as u8, - } + self.ccr_cache.read_ccr(&self.lpspi) } /// Produce a transaction that considers bus-managed software state. @@ -1175,10 +1212,11 @@ fn set_watermark(lpspi: &ral::lpspi::RegisterBlock, direction: Direction, waterm pub struct Disabled<'a, const N: u8> { lpspi: &'a ral::lpspi::Instance, men: bool, + ccr_cache: &'a mut CcrCache, } impl<'a, const N: u8> Disabled<'a, N> { - fn new(lpspi: &'a mut ral::lpspi::Instance) -> Self { + fn new(lpspi: &'a mut ral::lpspi::Instance, ccr_cache: &'a mut CcrCache) -> Self { let men = ral::read_reg!(ral::lpspi, lpspi, CR, MEN == MEN_1); // Request disable @@ -1186,7 +1224,11 @@ impl<'a, const N: u8> Disabled<'a, N> { // Wait for the driver to finish its current transfer // and enter disabled state while ral::read_reg!(ral::lpspi, lpspi, CR, MEN == MEN_1) {} - Self { lpspi, men } + Self { + lpspi, + men, + ccr_cache, + } } /// Set the LPSPI clock speed (Hz). @@ -1194,7 +1236,8 @@ impl<'a, const N: u8> Disabled<'a, N> { /// `source_clock_hz` is the LPSPI peripheral clock speed. To specify the /// peripheral clock, see the [`ccm::lpspi_clk`](crate::ccm::lpspi_clk) documentation. pub fn set_clock_hz(&mut self, source_clock_hz: u32, clock_hz: u32) { - set_spi_clock(source_clock_hz, clock_hz, self.lpspi); + let clock_configs = compute_spi_clock(source_clock_hz, clock_hz); + self.set_clock_configs(clock_configs); } /// Set LPSPI timing configurations. @@ -1202,6 +1245,7 @@ impl<'a, const N: u8> Disabled<'a, N> { /// If you're not sure how to select these timing values, prefer /// [`set_clock_hz`](Self::set_clock_hz). pub fn set_clock_configs(&mut self, timing: ClockConfigs) { + self.ccr_cache.update(timing); ral::write_reg!(ral::lpspi, self.lpspi, CCR, SCKPCS: timing.sckpcs as u32, PCSSCK: timing.pcssck as u32, From 130210eaa904c983e6426a8ba8124579a4444f86 Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Sun, 10 Nov 2024 12:27:42 -0500 Subject: [PATCH 8/9] Revert "Remove unused LPSPI I/O helpers" This reverts commit 0cb3b8d320e3d2a0e43304f4beca06efba261af1. --- src/common/lpspi.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 3b050468..509065fc 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -1518,6 +1518,15 @@ where } } +/// Transmits dummy values. +struct TransmitDummies; + +impl TransmitData for TransmitDummies { + fn next_word(&mut self, _: BitOrder) -> u32 { + u32::MAX + } +} + /// Receive data into a buffer. struct ReceiveBuffer<'a, W> { /// The write position. @@ -1582,6 +1591,13 @@ where } } +/// Receive dummy data. +struct ReceiveDummies; + +impl ReceiveData for ReceiveDummies { + fn next_word(&mut self, _: u32) {} +} + /// Computes how may Ws fit inside a LPSPI word. const fn per_word() -> usize { core::mem::size_of::() / core::mem::size_of::() From 1b1b64f14050ea36f220750874495fa78f7fbda5 Mon Sep 17 00:00:00 2001 From: Ian McIntyre Date: Sun, 10 Nov 2024 13:48:57 -0500 Subject: [PATCH 9/9] Implement SpiBus, SpiDevice for LPSPI We implement all I/O in terms of `transact`. We'll manually flush at the end of eh02 SPI writes. Since the `SpiBus` interface exposes a flush, we have no need to perform an internal flush when transacting I/O. Keep the spinning async state machines to manage the TX and FX FIFOs. We introduce a schedule to eagerly execute transmit operations ahead of receive operations. Without this ability, we'll stall the LPSPI bus. (The NOSTALL field can change this behavior, but then we move complexity into TX FIFO underrun handling.) There's some miri tests to simulate our hardware utilization. Our `SpiDevice` impls mimic the embedded-bus design. There's an "exclusive" device that owns the bus and uses a hardware chip select. There's a `RefCellDevice` that lets users share the bus and use hardware chip selects. There's no critical section / mutex device, but it should be straightforward to add that later. --- board/Cargo.toml | 4 + board/src/imxrt1170evk-cm7.rs | 15 +- board/src/lib.rs | 14 + examples/rtic_spi_blocking.rs | 53 +- src/common/lpspi.rs | 1057 +++++++++++++++++++++++++++++---- 5 files changed, 996 insertions(+), 147 deletions(-) diff --git a/board/Cargo.toml b/board/Cargo.toml index 53d2e3b8..79f4dd41 100644 --- a/board/Cargo.toml +++ b/board/Cargo.toml @@ -8,6 +8,10 @@ publish = false [dependencies.defmt] version = "0.3" +[dependencies.eh1] +package = "embedded-hal" +version = "1.0" + [dependencies.imxrt-hal] workspace = true diff --git a/board/src/imxrt1170evk-cm7.rs b/board/src/imxrt1170evk-cm7.rs index 5136fd23..03692165 100644 --- a/board/src/imxrt1170evk-cm7.rs +++ b/board/src/imxrt1170evk-cm7.rs @@ -98,7 +98,8 @@ pub type SpiPcs0 = iomuxc::gpio_ad::GPIO_AD_29; const SPI_INSTANCE: u8 = 1; #[cfg(feature = "spi")] -pub type Spi = hal::lpspi::Lpspi; +pub type Spi = + hal::lpspi::ExclusiveDevice; #[cfg(not(feature = "spi"))] pub type Spi = (); @@ -209,15 +210,15 @@ impl Specifics { sdi: iomuxc.gpio_ad.p31, sck: iomuxc.gpio_ad.p28, }; - crate::iomuxc::lpspi::prepare({ - let pcs0: &mut SpiPcs0 = &mut iomuxc.gpio_ad.p29; - pcs0 - }); - let mut spi = Spi::new(lpspi1, pins); + let mut spi = hal::lpspi::Lpspi::new(lpspi1, pins); spi.disabled(|spi| { spi.set_clock_hz(LPSPI_CLK_FREQUENCY, super::SPI_BAUD_RATE_FREQUENCY); }); - spi + hal::lpspi::ExclusiveDevice::with_pcs0( + spi, + iomuxc.gpio_ad.p29, + crate::PanickingDelay::new(), + ) }; #[cfg(not(feature = "spi"))] #[allow(clippy::let_unit_value)] diff --git a/board/src/lib.rs b/board/src/lib.rs index c5819562..ddd50fc5 100644 --- a/board/src/lib.rs +++ b/board/src/lib.rs @@ -282,6 +282,20 @@ pub mod blocking { } } +pub struct PanickingDelay(()); + +impl PanickingDelay { + pub fn new() -> Self { + Self(()) + } +} + +impl eh1::delay::DelayNs for PanickingDelay { + fn delay_ns(&mut self, _: u32) { + unimplemented!() + } +} + /// Configurations for the logger. /// /// If your board is ready to support the logging infrastructure, diff --git a/examples/rtic_spi_blocking.rs b/examples/rtic_spi_blocking.rs index 81b81c2f..852a6a75 100644 --- a/examples/rtic_spi_blocking.rs +++ b/examples/rtic_spi_blocking.rs @@ -60,26 +60,26 @@ mod app { // size and bit order, use this sequence to evaluate how // the driver packs your transfer elements. { - use eh02::blocking::spi::Write; + use eh1::spi::SpiDevice; use hal::lpspi::BitOrder::{self, *}; const BIT_ORDERS: [BitOrder; 2] = [Msb, Lsb]; const U32_WORDS: [u32; 2] = [0xDEADBEEFu32, 0xAD1CAC1D]; for bit_order in BIT_ORDERS { - spi.set_bit_order(bit_order); + spi.bus_mut().set_bit_order(bit_order); spi.write(&U32_WORDS).unwrap(); } const U8_WORDS: [u8; 7] = [0xDEu8, 0xAD, 0xBE, 0xEF, 0x12, 0x34, 0x56]; for bit_order in BIT_ORDERS { - spi.set_bit_order(bit_order); + spi.bus_mut().set_bit_order(bit_order); spi.write(&U8_WORDS).unwrap(); } const U16_WORDS: [u16; 3] = [0xDEADu16, 0xBEEF, 0x1234]; for bit_order in BIT_ORDERS { - spi.set_bit_order(bit_order); + spi.bus_mut().set_bit_order(bit_order); spi.write(&U16_WORDS).unwrap(); } @@ -88,12 +88,12 @@ mod app { // Change me to explore bit order behavors in the // remaining write / loopback transfer tests. - spi.set_bit_order(hal::lpspi::BitOrder::Msb); + spi.bus_mut().set_bit_order(hal::lpspi::BitOrder::Msb); // Make sure concatenated elements look correct on the wire. // Make sure we can read those elements. { - use eh02::blocking::spi::Transfer; + use eh1::spi::SpiDevice; use hal::lpspi::BitOrder; macro_rules! transfer_test { @@ -103,9 +103,9 @@ mod app { BitOrder::Lsb => "LSB", }; - spi.set_bit_order($bit_order); + spi.bus_mut().set_bit_order($bit_order); let mut buffer = $arr; - spi.transfer(&mut buffer).unwrap(); + spi.transfer_in_place(&mut buffer).unwrap(); defmt::assert_eq!(buffer, $arr, "Bit Order {}", bit_order_name); }; } @@ -137,12 +137,12 @@ mod app { transfer_test!([0x01020304u32, 0x05060708, 0x090A0B0C], BitOrder::Msb); transfer_test!([0x01020304u32, 0x05060708, 0x090A0B0C], BitOrder::Lsb); - spi.set_bit_order(BitOrder::Msb); + spi.bus_mut().set_bit_order(BitOrder::Msb); delay(); } { - use eh02::blocking::spi::{Transfer, Write}; + use eh1::spi::SpiDevice; // Change me to test different Elem sizes, buffer sizes, // bit patterns. @@ -153,10 +153,8 @@ mod app { // Simple loopback transfer. Easy to find with your // scope. let mut buffer = BUFFER; - spi.transfer(&mut buffer).unwrap(); - if buffer != BUFFER { - defmt::error!("Simple transfer buffer mismatch!"); - } + spi.transfer_in_place(&mut buffer).unwrap(); + defmt::assert_eq!(buffer, BUFFER); delay(); @@ -167,7 +165,7 @@ mod app { for idx in 0u32..16 { buffer.fill(SENTINEL.rotate_right(idx)); let expected = buffer; - spi.transfer(&mut buffer).unwrap(); + spi.transfer_in_place(&mut buffer).unwrap(); error |= buffer != expected; } if error { @@ -194,6 +192,31 @@ mod app { delay(); } + + { + use eh1::spi::{ + Operation::{Read, TransferInPlace, Write}, + SpiDevice, + }; + + let mut read = [0u8; 7]; + let mut xfer = [0u8; 10]; + for idx in 0..xfer.len() { + xfer[idx] = idx as u8; + } + + spi.transaction(&mut [ + TransferInPlace(&mut xfer), + Read(&mut read), + Write(&[0xA5; 13][..]), + ]) + .unwrap(); + + assert_eq!(read, [0xff; 7]); + assert_eq!(xfer, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]); + + delay(); + } } } } diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 509065fc..2a7a600c 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -11,14 +11,25 @@ //! //! # Chip selects (CS) for SPI peripherals //! -//! The iMXRT SPI peripherals have one or more peripheral-controlled chip selects (CS). Using -//! the peripheral-controlled CS means that you do not need a GPIO to coordinate SPI operations. -//! Blocking full-duplex transfers and writes will observe an asserted chip select while data -//! frames are exchanged / written. +//! The iMXRT SPI peripherals have one or more hardware-controlled chip selects (CS). Using +//! the hardware-controlled CS means that you do not need a GPIO to coordinate SPI operations. +//! The driver nevertheless works with software-managed CS. //! -//! This driver generally assumes that you're using the peripheral-controlled chip select. If -//! you instead want to manage chip select in software, you should be able to multiplex your own -//! pins, then construct the driver [`without_pins`](Lpspi::without_pins). +//! Generally, an [`Lpspi`] is capable of using hardware-controlled chip select. However, it only +//! does that if you mux your hardware chip select to the LPSPI instance. The device abstractions +//! provided by this module can do that for you. +//! +//! Here's how [`Lpspi`] implements the various embedded-hal traits, when considering hardware- +//! and software-managed CS. +//! +//! - `Lpspi` implements the blocking traits defined in embedded-hal 0.2. If you're using +//! software-managed CS, then you're responsible for toggling your pin as required. If you're +//! using hardware-managed CS, then the transactions performed on the bus use [`Pcs::Pcs0`] +//! by default, or the value you supply to [`Lpspi::set_chip_select`]. +//! - `Lpspi` implements the blocking *bus* traits defined in embedded-hal 1.0. If you're using +//! software-managed CS, you can use `embedded-hal-bus` to pair `Lpspi` with a GPIO. If you're +//! using hardware-managed CS, use [`ExclusiveDevice`] or [`RefCellDevice`] to pair `Lpspi` +//! with a chip select. //! //! # Device support //! @@ -83,7 +94,11 @@ //! [`Transaction`] exposes the continuous / continuing flags, so you're free to model advanced //! transactions. However, keep in mind that disabling the receiver during a continuous transaction //! may not work as expected. +//! +//! Since we cannot reliably use continuous transactions, we're forced to use a single transaction to +//! realize `SpiDevice` transactions. This limits the total number of bytes transacted to 512. +use core::cell::RefCell; use core::marker::PhantomData; use core::task::Poll; @@ -91,6 +106,7 @@ use crate::iomuxc::{consts, lpspi}; use crate::ral; pub use eh02::spi::{Mode, Phase, Polarity, MODE_0, MODE_1, MODE_2, MODE_3}; +use eh1::spi::Operation; /// Data direction. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -308,11 +324,8 @@ impl Transaction { } fn new_words(data: &[W]) -> Result { - if let Ok(frame_size) = u16::try_from(8 * core::mem::size_of_val(data)) { - Transaction::new(frame_size) - } else { - Err(LpspiError::FrameSize) - } + let bits = frame_size(data)?; + Transaction::new(bits) } fn frame_size_valid(frame_size: u16) -> bool { @@ -356,6 +369,14 @@ impl Transaction { } } +fn frame_size(buffer: &[W]) -> Result { + let bits = size_of_val(buffer) + .checked_mul(8) + .ok_or(LpspiError::FrameSize)?; + + bits.try_into().map_err(|_| LpspiError::FrameSize) +} + /// Sets the clock speed parameters. /// /// This should only happen when the LPSPI peripheral is disabled. @@ -474,6 +495,7 @@ pub struct Lpspi { bit_order: BitOrder, mode: Mode, ccr_cache: CcrCache, + pcs: Pcs, } /// Pins for a LPSPI device. @@ -549,6 +571,7 @@ impl Lpspi { mode: MODE_0, // Once we issue a reset, below, these are zero. ccr_cache: CcrCache { dbt: 0, sckdiv: 0 }, + pcs: Pcs::default(), }; // Reset and disable @@ -632,6 +655,23 @@ impl Lpspi { self.bit_order = bit_order; } + /// Returns the hardware chip select used by the LPSPI bus. + /// + /// If you have not multiplexed a PCS pin to this LPSPI instance, + /// then the value is meaningless. + pub fn chip_select(&self) -> Pcs { + self.pcs + } + + /// Set the hardware chip select used by this LPSPI bus. + /// + /// This affects all subsequent transactions. In-flight transactions, + /// if any, are not affected. If you have not multiplexed a PCS pin + /// to this LPSPI instance, then this configuration is meaningless. + pub fn set_chip_select(&mut self, pcs: Pcs) { + self.pcs = pcs; + } + /// Temporarily disable the LPSPI peripheral. /// /// The handle to a [`Disabled`](crate::lpspi::Disabled) driver lets you modify @@ -751,6 +791,43 @@ impl Lpspi { .await } + /// Spin until the transmit FIFO is empty. + /// + /// Check for both receive and transmit errors, allowing us to realize + /// a flush operation. + async fn spin_for_fifo_exhaustion(&self) -> Result<(), LpspiError> { + core::future::poll_fn(|_| { + let status = self.status(); + + if status.intersects(Status::RECEIVE_ERROR) { + return Poll::Ready(Err(LpspiError::Fifo(Direction::Rx))); + } + if status.intersects(Status::TRANSMIT_ERROR) { + return Poll::Ready(Err(LpspiError::Fifo(Direction::Tx))); + } + + // Contributor testing reveals that the busy flag may not set once + // the TX FIFO is filled. This means a sequence like + // + // lpspi.write(&[...])?; + // lpspi.flush()?; + // + // could pass through flush without observing busy. Therefore, we + // also check the FIFO contents. Even if the peripheral isn't + // busy, the FIFO should be non-empty. + // + // We can't just rely on the FIFO contents, since the FIFO could be + // empty while the transaction is completing. (There's data in the + // shift register, and PCS is still asserted.) + if !status.intersects(Status::BUSY) && self.fifo_status().is_empty(Direction::Tx) { + return Poll::Ready(Ok(())); + } + + Poll::Pending + }) + .await + } + pub(crate) fn wait_for_transmit_fifo_space(&self) -> Result<(), LpspiError> { crate::spin_on(self.spin_for_fifo_space()) } @@ -841,80 +918,7 @@ impl Lpspi { /// Wait for all ongoing transactions to be finished. pub fn flush(&mut self) -> Result<(), LpspiError> { - loop { - let status = self.status(); - - if status.intersects(Status::RECEIVE_ERROR) { - return Err(LpspiError::Fifo(Direction::Rx)); - } - if status.intersects(Status::TRANSMIT_ERROR) { - return Err(LpspiError::Fifo(Direction::Tx)); - } - - // Contributor testing reveals that the busy flag may not set once - // the TX FIFO is filled. This means a sequence like - // - // lpspi.write(&[...])?; - // lpspi.flush()?; - // - // could pass through flush without observing busy. Therefore, we - // also check the FIFO contents. Even if the peripheral isn't - // busy, the FIFO should be non-empty. - // - // We can't just rely on the FIFO contents, since the FIFO could be - // empty while the transaction is completing. (There's data in the - // shift register, and PCS is still asserted.) - if !status.intersects(Status::BUSY) && self.fifo_status().is_empty(Direction::Tx) { - return Ok(()); - } - } - } - - fn exchange(&mut self, data: &mut [W]) -> Result<(), LpspiError> { - if data.is_empty() { - return Ok(()); - } - - let transaction = self.bus_transaction(data)?; - - self.wait_for_transmit_fifo_space()?; - self.enqueue_transaction(&transaction); - - let word_count = word_count(data); - let (tx, rx) = transfer_in_place(data); - - crate::spin_on(futures::future::try_join( - self.spin_transmit(tx, word_count), - self.spin_receive(rx, word_count), - )) - .inspect_err(|_| self.recover_from_error())?; - - self.flush()?; - - Ok(()) - } - - fn write_no_read(&mut self, data: &[W]) -> Result<(), LpspiError> { - if data.is_empty() { - return Ok(()); - } - - let mut transaction = self.bus_transaction(data)?; - transaction.receive_data_mask = true; - - self.wait_for_transmit_fifo_space()?; - self.enqueue_transaction(&transaction); - - let word_count = word_count(data); - let tx = TransmitBuffer::new(data); - - crate::spin_on(self.spin_transmit(tx, word_count)).inspect_err(|_| { - self.recover_from_error(); - })?; - - self.flush()?; - - Ok(()) + crate::spin_on(self.spin_for_fifo_exhaustion()) } /// Let the peripheral act as a DMA source. @@ -1051,11 +1055,129 @@ impl Lpspi { /// Produce a transaction that considers bus-managed software state. pub(crate) fn bus_transaction(&self, words: &[W]) -> Result { - let mut transaction = Transaction::new_words(words)?; + self.transaction_for_frame_size(frame_size(words)?) + } + + fn transaction_for_frame_size(&self, bits: u16) -> Result { + let mut transaction = Transaction::new(bits)?; transaction.bit_order = self.bit_order(); transaction.mode = self.mode; + transaction.pcs = self.chip_select(); Ok(transaction) } + + fn operation_transaction( + &self, + op: &Operation, + ) -> Result, LpspiError> { + let buffer: &[W] = match op { + Operation::DelayNs(_) => return Ok(None), + Operation::Read(buffer) => buffer, + Operation::Write(buffer) => buffer, + Operation::TransferInPlace(buffer) => buffer, + Operation::Transfer(recv, send) => { + if recv.len() > send.len() { + recv + } else { + send + } + } + }; + Transaction::new_words(buffer).map(Some) + } + + /// Perform one or more operations within an LPSPI transaction. + /// + /// If you know there's no delay operations, you may pass in a no-op / panicking + /// `delay_ns`. Any operation with an empty buffer is skipped. + /// + /// The implementation executes all operations within a single, non-continuous + /// transaction. We can't use a continuous transaction without risking the + /// undocumented hardware errata described in this module's documentation. + /// This means users are limited to transacting 512 bytes across all operations. + /// + /// This will not flush when complete. Callers may need to do that themselves. + fn transact( + &mut self, + operations: &mut [Operation], + mut delay_ns: impl FnMut(u32), + ) -> Result<(), LpspiError> { + if operations.is_empty() { + return Ok(()); + } + + for op in operations.iter() { + self.operation_transaction(op)?; + } + + let len = operations.len(); + let (tx_schedule, rx_schedule) = schedule(operations); + + let tx_state_machine = async { + let mut continuing = false; + for mut desc in tx_schedule { + // This delay is blocking, so it stalls the RX state machine. + // We need to prevent an RX FIFO overrun while we're stalled. + // To prevent that, we'll flush the transmit FIFO before delaying. + // This increases the transaction latency when writes operations + // preceded delay operations, but this is acceptable; our delay + // is "at least" best effort. + if let Some(ns) = desc.delay_ns() { + self.spin_for_fifo_exhaustion().await?; + delay_ns(ns); + continue; + } + + let mut transaction = self.transaction_for_frame_size(desc.frame_size)?; + transaction.continuous = len > 1; + transaction.continuing = continuing; + + self.spin_for_fifo_space().await?; + self.enqueue_transaction(&transaction); + + let total_words = desc.total_words(); + if let Some(tx_buffer) = desc.tx_buffer.take() { + self.spin_transmit(tx_buffer, desc.tx_words).await?; + } + + self.spin_transmit(TransmitDummies, total_words.saturating_sub(desc.tx_words)) + .await?; + + continuing = true; + } + + let fin = self.transaction_for_frame_size(8).unwrap(); + self.spin_for_fifo_space().await?; + self.enqueue_transaction(&fin); + + Ok(()) + }; + + let rx_state_machine = async { + for mut desc in rx_schedule { + if desc.is_delay_ns() { + continue; + } + let total_words = desc.total_words(); + if let Some(rx_buffer) = desc.rx_buffer.take() { + self.spin_receive(rx_buffer, desc.rx_words).await?; + } + + self.spin_receive(ReceiveDummies, total_words.saturating_sub(desc.rx_words)) + .await?; + } + Ok(()) + }; + + // Run those transmit and receive state machines together. + crate::spin_on(futures::future::try_join( + tx_state_machine, + rx_state_machine, + )) + .inspect_err(|_| self.recover_from_error())?; + + Ok(()) + } } bitflags::bitflags! { @@ -1295,6 +1417,10 @@ impl<'a, const N: u8> Disabled<'a, N> { } } +fn no_delay_needed(_: u32) { + unreachable!() +} + impl Drop for Disabled<'_, N> { fn drop(&mut self) { ral::modify_reg!(ral::lpspi, self.lpspi, CR, MEN: self.men as u32); @@ -1305,7 +1431,7 @@ impl eh02::blocking::spi::Transfer for Lpspi { type Error = LpspiError; fn transfer<'a>(&mut self, words: &'a mut [u8]) -> Result<&'a [u8], Self::Error> { - self.exchange(words)?; + self.transact(&mut [Operation::TransferInPlace(words)], no_delay_needed)?; Ok(words) } } @@ -1314,7 +1440,7 @@ impl eh02::blocking::spi::Transfer for Lpspi { type Error = LpspiError; fn transfer<'a>(&mut self, words: &'a mut [u16]) -> Result<&'a [u16], Self::Error> { - self.exchange(words)?; + self.transact(&mut [Operation::TransferInPlace(words)], no_delay_needed)?; Ok(words) } } @@ -1323,7 +1449,7 @@ impl eh02::blocking::spi::Transfer for Lpspi { type Error = LpspiError; fn transfer<'a>(&mut self, words: &'a mut [u32]) -> Result<&'a [u32], Self::Error> { - self.exchange(words)?; + self.transact(&mut [Operation::TransferInPlace(words)], no_delay_needed)?; Ok(words) } } @@ -1332,7 +1458,9 @@ impl eh02::blocking::spi::Write for Lpspi { type Error = LpspiError; fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { - self.write_no_read(words) + self.transact(&mut [Operation::Write(words)], no_delay_needed)?; + self.flush()?; + Ok(()) } } @@ -1340,7 +1468,9 @@ impl eh02::blocking::spi::Write for Lpspi { type Error = LpspiError; fn write(&mut self, words: &[u16]) -> Result<(), Self::Error> { - self.write_no_read(words) + self.transact(&mut [Operation::Write(words)], no_delay_needed)?; + self.flush()?; + Ok(()) } } @@ -1348,7 +1478,90 @@ impl eh02::blocking::spi::Write for Lpspi { type Error = LpspiError; fn write(&mut self, words: &[u32]) -> Result<(), Self::Error> { - self.write_no_read(words) + self.transact(&mut [Operation::Write(words)], no_delay_needed)?; + self.flush()?; + Ok(()) + } +} + +impl eh1::spi::Error for LpspiError { + fn kind(&self) -> eh1::spi::ErrorKind { + use eh1::spi::ErrorKind; + + match self { + LpspiError::Fifo(Direction::Rx) => ErrorKind::Overrun, + _ => ErrorKind::Other, + } + } +} + +impl eh1::spi::ErrorType for Lpspi { + type Error = LpspiError; +} + +impl eh1::spi::SpiBus for Lpspi { + fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Read(words)], no_delay_needed) + } + + fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Write(words)], no_delay_needed) + } + + fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Transfer(read, write)], no_delay_needed) + } + + fn transfer_in_place(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::TransferInPlace(words)], no_delay_needed) + } + + fn flush(&mut self) -> Result<(), Self::Error> { + Lpspi::flush(self) + } +} + +impl eh1::spi::SpiBus for Lpspi { + fn read(&mut self, words: &mut [u16]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Read(words)], no_delay_needed) + } + + fn write(&mut self, words: &[u16]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Write(words)], no_delay_needed) + } + + fn transfer(&mut self, read: &mut [u16], write: &[u16]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Transfer(read, write)], no_delay_needed) + } + + fn transfer_in_place(&mut self, words: &mut [u16]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::TransferInPlace(words)], no_delay_needed) + } + + fn flush(&mut self) -> Result<(), Self::Error> { + Lpspi::flush(self) + } +} + +impl eh1::spi::SpiBus for Lpspi { + fn read(&mut self, words: &mut [u32]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Read(words)], no_delay_needed) + } + + fn write(&mut self, words: &[u32]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Write(words)], no_delay_needed) + } + + fn transfer(&mut self, read: &mut [u32], write: &[u32]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::Transfer(read, write)], no_delay_needed) + } + + fn transfer_in_place(&mut self, words: &mut [u32]) -> Result<(), Self::Error> { + self.transact(&mut [Operation::TransferInPlace(words)], no_delay_needed) + } + + fn flush(&mut self) -> Result<(), Self::Error> { + Lpspi::flush(self) } } @@ -1450,6 +1663,222 @@ impl Word for u32 { } } +/// Low-level description of an LPSPI operation. +/// +/// Interpretation depends on the TX or RX state machine. +/// The TX state machine interprets a delay using `delay_ns`. +struct Descriptor<'w, W> { + frame_size: u16, + tx_buffer: Option>, + tx_words: usize, + rx_buffer: Option>, + rx_words: usize, +} + +impl Descriptor<'_, W> { + /// The number of 32-bit LPSPI words transacted by this descriptor. + fn total_words(&self) -> usize { + self.tx_words.max(self.rx_words) + } + + fn is_delay_ns(&self) -> bool { + self.tx_buffer.is_none() && self.rx_buffer.is_none() + } + + /// Returns the delay, in nanoseconds, if this descriptor describes + /// a delay operation. + fn delay_ns(&self) -> Option { + if self.is_delay_ns() { + Some(self.tx_words as u32) + } else { + None + } + } +} + +/// The schedule splits the user's SPI operations for transmit +/// and receive state machines. +/// +/// Use [`schedule`] to produce two of these, one per state machine. +/// Then, execute the [`Descriptor`]s produced by the schedule. +struct Schedule<'o, 'w, W: 'static> { + /// Our position in the user's (slice of) operations. + ptr: *mut Operation<'w, W>, + /// The address at the end of the slice. + end: *mut Operation<'w, W>, + /// Capture the lifetime of the (slice of) operations. + _lt: PhantomData<&'o mut [Operation<'w, W>]>, +} + +impl<'w, W: Word + 'static> Iterator for Schedule<'_, 'w, W> { + type Item = Descriptor<'w, W>; + fn next(&mut self) -> Option { + // Safety: We do not form multiple mutable references to either + // the Operation variants, or the mutable buffers wrapped by Operation + // variants. The pointer-len bounds are in bounds, and their lifetimes + // do not exceed the lifetimes of the derived slices. + unsafe { + if self.ptr == self.end { + return None; + } + + /// Access a pointer to the inner slice. + fn decay_ref_ref_slice(buffer: &&[W]) -> *const W { + buffer.as_ptr() + } + + /// Access the pointer to the inner slice. + fn decay_ref_mut_slice(buffer: &&mut [W]) -> *mut W { + // Safety: see inline notes. + unsafe { + // Dispell the shared reference which conveys that the + // inner exclusive reference cannot be mutated. + let buffer: *const &mut [W] = buffer; + + // We know that &mut [T] -> *mut [T] is OK. + // Perform the same cast through the outer pointer. + let buffer: *const *mut [W] = buffer.cast(); + + // Get the fat pointer. We're reading initialized + // and aligned memory, since the outer shared + // reference must be initialized and aligned. + let buffer: *mut [W] = buffer.read(); + + // This is + // + // impl *mut [T] { pub fn as_ptr(self) } + // + // which isn't yet stable. + buffer as *mut W + } + } + + // Shared reference to an operation from + // a mutable pointer. This shared reference + // can exist with other shared references. + // (We'll also never form multiple shared + // references, since iteration of all schedules + // occurs in the same execution context). + let op: &Operation<'w, W> = &*self.ptr; + let descriptor = match op { + // Sentinel handled by the transmit state machine. + Operation::DelayNs(ns) => Descriptor { + frame_size: 0, + tx_buffer: None, + tx_words: *ns as _, + rx_buffer: None, + rx_words: 0, + }, + Operation::Read(buffer) => { + let frame_size = frame_size(buffer).unwrap(); + let len = buffer.len(); + let word_count = word_count(buffer); + let buffer: *mut W = decay_ref_mut_slice(buffer); + let rx = ReceiveBuffer::from_raw(buffer, len); + Descriptor { + frame_size, + tx_buffer: None, + tx_words: 0, + rx_buffer: Some(rx), + rx_words: word_count, + } + } + Operation::Write(buffer) => { + let frame_size = frame_size(buffer).unwrap(); + let len = buffer.len(); + let word_count = word_count(buffer); + let buffer = decay_ref_ref_slice(buffer); + let tx = TransmitBuffer::from_raw(buffer, len); + Descriptor { + frame_size, + tx_buffer: Some(tx), + tx_words: word_count, + rx_buffer: None, + rx_words: 0, + } + } + Operation::TransferInPlace(buffer) => { + let frame_size = frame_size(buffer).unwrap(); + let len = buffer.len(); + let word_count = word_count(buffer); + let buffer: *mut W = decay_ref_mut_slice(buffer); + let tx = TransmitBuffer::from_raw(buffer, len); + let rx = ReceiveBuffer::from_raw(buffer, len); + Descriptor { + frame_size, + tx_buffer: Some(tx), + tx_words: word_count, + rx_buffer: Some(rx), + rx_words: word_count, + } + } + Operation::Transfer(recv, send) => { + let frame_size = if recv.len() > send.len() { + frame_size(recv) + } else { + frame_size(send) + } + .unwrap(); + + let recv_words = word_count(recv); + let send_words = word_count(send); + + let rx_len = recv.len(); + let tx_len = send.len(); + + let recv: *mut W = decay_ref_mut_slice(recv); + let send: *const W = decay_ref_ref_slice(send); + + let rx = ReceiveBuffer::from_raw(recv, rx_len); + let tx = TransmitBuffer::from_raw(send, tx_len); + + Descriptor { + frame_size, + tx_buffer: Some(tx), + tx_words: send_words, + rx_buffer: Some(rx), + rx_words: recv_words, + } + } + }; + + // Remains in bounds, or points to the element at the + // end of the slice. Bounds check is at the top. + self.ptr = self.ptr.add(1); + + Some(descriptor) + } + } +} + +/// Produce a schedule, one for each TX and RX state machine. +fn schedule<'ops, 'w, W: Word + 'static>( + ops: &mut [Operation<'w, W>], +) -> (Schedule<'ops, 'w, W>, Schedule<'ops, 'w, W>) { + // Safety: We track the lifetime of all memory through + // the schedule. Inspection of the Schedule interation + // shows that we never form an exclusive reference to + // any memory. + unsafe { + let len = ops.len(); + let ptr = ops.as_mut_ptr(); + let end = ptr.add(len); + + ( + Schedule { + ptr, + end, + _lt: PhantomData, + }, + Schedule { + ptr, + end, + _lt: PhantomData, + }, + ) + } +} + /// Generalizes how we prepare LPSPI words for transmit. trait TransmitData { /// Get the next word for the transmit FIFO. @@ -1473,15 +1902,10 @@ struct TransmitBuffer<'a, W> { _buffer: PhantomData<&'a [W]>, } -impl<'a, W> TransmitBuffer<'a, W> +impl TransmitBuffer<'_, W> where W: Word, { - fn new(buffer: &'a [W]) -> Self { - // Safety: pointer offset math meets expectations. - unsafe { Self::from_raw(buffer.as_ptr(), buffer.len()) } - } - /// # Safety /// /// `ptr + len` must be in bounds, or at the end of the @@ -1540,12 +1964,6 @@ impl ReceiveBuffer<'_, W> where W: Word, { - #[cfg(test)] // TODO(mciantyre) remove once needed in non-test code. - fn new(buffer: &mut [W]) -> Self { - // Safety: pointer offset math meets expectations. - unsafe { Self::from_raw(buffer.as_mut_ptr(), buffer.len()) } - } - /// # Safety /// /// `ptr + len` must be in bounds, or at the end of the @@ -1595,7 +2013,7 @@ where struct ReceiveDummies; impl ReceiveData for ReceiveDummies { - fn next_word(&mut self, _: u32) {} + fn next_word(&mut self, _: BitOrder, _: u32) {} } /// Computes how may Ws fit inside a LPSPI word. @@ -1608,21 +2026,215 @@ const fn word_count(words: &[W]) -> usize { words.len().div_ceil(per_word::()) } -/// Creates the transmit and receive buffer objects for an -/// in-place transfer. -fn transfer_in_place(buffer: &mut [W]) -> (TransmitBuffer<'_, W>, ReceiveBuffer<'_, W>) { - // Safety: pointer math meets expectation. This produces - // a mutable and immutable pointer to the same mutable buffer. - // Module inspection shows that these pointers never become - // references. We maintain the lifetime across both objects, - // so the buffer isn't dropped. - unsafe { - let len = buffer.len(); - let ptr = buffer.as_mut_ptr(); - ( - TransmitBuffer::from_raw(ptr, len), - ReceiveBuffer::from_raw(ptr, len), - ) +/// A SPI device with hardware-managed chip select. +/// +/// `B` is the [`Lpspi`] bus type. The helper types +/// [`ExclusiveDevice`] and [`RefCellDevice`] impose +/// a specific kind of `B`. Prefer those types. +/// +/// `CS` is your chip select pad. If you don't care about +/// pin type states, you can elide this type using the +/// various `without_pin` constructors. +/// +/// `D` is a delay type. If you don't need a delay, you can +/// use a dummy type that panics or does nothing. +pub struct Device { + bus: B, + cs: CS, + delay: D, + pcs: Pcs, +} + +impl Device { + /// Borrow the bus. + pub fn bus(&self) -> &B { + &self.bus + } + /// Exclusively borrow the bus. + /// + /// Be careful when changing bus settings, especially the + /// chip select. + pub fn bus_mut(&mut self) -> &mut B { + &mut self.bus + } + + /// Release the device components. + /// + /// The components' states are unspecified. + pub fn release(self) -> (B, CS, D) { + (self.bus, self.cs, self.delay) + } + + fn with_pin(bus: B, mut cs: CS, delay: D, pcs: Pcs) -> Self + where + CS: lpspi::Pin, + { + lpspi::prepare(&mut cs); + Self::new(bus, cs, delay, pcs) + } + + fn new(bus: B, cs: CS, delay: D, pcs: Pcs) -> Self { + Self { + bus, + cs, + delay, + pcs, + } + } +} + +impl eh1::spi::ErrorType for Device { + type Error = LpspiError; +} + +/// A device that owns the LPSPI bus. +/// +/// Use this is you have no other SPI peripherals connected +/// to your bus. +pub type ExclusiveDevice = Device, CS, D>; + +impl ExclusiveDevice { + /// Construct the device with its PCS0 hardware chip select. + pub fn with_pcs0(mut bus: Lpspi, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs0>, + { + bus.set_chip_select(Pcs::Pcs0); + Self::with_pin(bus, cs, delay, Pcs::Pcs0) + } + /// Construct the device with its PCS1 hardware chip select. + pub fn with_pcs1(mut bus: Lpspi, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs1>, + { + bus.set_chip_select(Pcs::Pcs1); + Self::with_pin(bus, cs, delay, Pcs::Pcs1) + } + /// Construct the device with its PCS2 hardware chip select. + pub fn with_pcs2(mut bus: Lpspi, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs2>, + { + bus.set_chip_select(Pcs::Pcs2); + Self::with_pin(bus, cs, delay, Pcs::Pcs2) + } + /// Construct the device with its PCS3 hardware chip select. + pub fn with_pcs3(mut bus: Lpspi, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs3>, + { + bus.set_chip_select(Pcs::Pcs3); + Self::with_pin(bus, cs, delay, Pcs::Pcs3) + } +} + +impl ExclusiveDevice { + /// Construct the device without a chip select type state. + /// + /// You're responsible for muxing the chip select pin to the hardware. + pub fn without_pin(mut bus: Lpspi, pcs: Pcs, delay: D) -> Self { + bus.set_chip_select(pcs); + Self::new(bus, (), delay, pcs) + } +} + +impl eh1::spi::SpiDevice + for ExclusiveDevice +{ + fn transaction(&mut self, operations: &mut [Operation<'_, u8>]) -> Result<(), Self::Error> { + self.bus.transact(operations, |ns| self.delay.delay_ns(ns)) + } +} + +impl eh1::spi::SpiDevice + for ExclusiveDevice +{ + fn transaction(&mut self, operations: &mut [Operation<'_, u16>]) -> Result<(), Self::Error> { + self.bus.transact(operations, |ns| self.delay.delay_ns(ns)) + } +} + +impl eh1::spi::SpiDevice + for ExclusiveDevice +{ + fn transaction(&mut self, operations: &mut [Operation<'_, u32>]) -> Result<(), Self::Error> { + self.bus.transact(operations, |ns| self.delay.delay_ns(ns)) + } +} + +/// A device that can share the LPSPI bus in a single context. +/// +/// Use this if you can share all of your devices within a single +/// execution context. +pub type RefCellDevice<'a, P, CS, D, const N: u8> = Device<&'a RefCell>, CS, D>; + +impl<'a, P, CS, D: eh1::delay::DelayNs, const N: u8> RefCellDevice<'a, P, CS, D, N> { + /// Construct the device with its PCS0 hardware chip select. + pub fn with_pcs0(bus: &'a RefCell>, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs0>, + { + Self::with_pin(bus, cs, delay, Pcs::Pcs0) + } + /// Construct the device with its PCS1 hardware chip select. + pub fn with_pcs1(bus: &'a RefCell>, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs1>, + { + Self::with_pin(bus, cs, delay, Pcs::Pcs1) + } + /// Construct the device with its PCS2 hardware chip select. + pub fn with_pcs2(bus: &'a RefCell>, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs2>, + { + Self::with_pin(bus, cs, delay, Pcs::Pcs2) + } + /// Construct the device with its PCS3 hardware chip select. + pub fn with_pcs3(bus: &'a RefCell>, cs: CS, delay: D) -> Self + where + CS: lpspi::Pin, Signal = lpspi::Pcs3>, + { + Self::with_pin(bus, cs, delay, Pcs::Pcs3) + } +} + +impl<'a, P, D: eh1::delay::DelayNs, const N: u8> RefCellDevice<'a, P, (), D, N> { + /// Construct the device without a chip select type state. + /// + /// You're responsible for muxing the chip select pin to the hardware. + pub fn without_pin(bus: &'a RefCell>, pcs: Pcs, delay: D) -> Self { + Self::new(bus, (), delay, pcs) + } +} + +impl eh1::spi::SpiDevice + for RefCellDevice<'_, P, CS, D, N> +{ + fn transaction(&mut self, operations: &mut [Operation<'_, u8>]) -> Result<(), Self::Error> { + let mut bus = self.bus.borrow_mut(); + bus.set_chip_select(self.pcs); + bus.transact(operations, |ns| self.delay.delay_ns(ns)) + } +} + +impl eh1::spi::SpiDevice + for RefCellDevice<'_, P, CS, D, N> +{ + fn transaction(&mut self, operations: &mut [Operation<'_, u16>]) -> Result<(), Self::Error> { + let mut bus = self.bus.borrow_mut(); + bus.set_chip_select(self.pcs); + bus.transact(operations, |ns| self.delay.delay_ns(ns)) + } +} + +impl eh1::spi::SpiDevice + for RefCellDevice<'_, P, CS, D, N> +{ + fn transaction(&mut self, operations: &mut [Operation<'_, u32>]) -> Result<(), Self::Error> { + let mut bus = self.bus.borrow_mut(); + bus.set_chip_select(self.pcs); + bus.transact(operations, |ns| self.delay.delay_ns(ns)) } } @@ -1630,11 +2242,206 @@ fn transfer_in_place(buffer: &mut [W]) -> (TransmitBuffer<'_, W>, Recei /// in firmware. Consider running these with miri to evaluate unsafe usages. #[cfg(test)] mod tests { + use super::{Descriptor, Operation, ReceiveBuffer, TransmitBuffer, Word}; + + impl ReceiveBuffer<'_, W> + where + W: Word, + { + fn new(buffer: &mut [W]) -> Self { + // Safety: pointer offset math meets expectations. + unsafe { Self::from_raw(buffer.as_mut_ptr(), buffer.len()) } + } + } + + impl TransmitBuffer<'_, W> + where + W: Word, + { + fn new(buffer: &[W]) -> Self { + // Safety: pointer offset math meets expectations. + unsafe { Self::from_raw(buffer.as_ptr(), buffer.len()) } + } + } + + /// Creates the transmit and receive buffer objects for an + /// in-place transfer. + fn transfer_in_place( + buffer: &mut [W], + ) -> (TransmitBuffer<'_, W>, ReceiveBuffer<'_, W>) { + // Safety: pointer math meets expectation. This produces + // a mutable and immutable pointer to the same mutable buffer. + // Module inspection shows that these pointers never become + // references. We maintain the lifetime across both objects, + // so the buffer isn't dropped. + unsafe { + let len = buffer.len(); + let ptr = buffer.as_mut_ptr(); + ( + TransmitBuffer::from_raw(ptr, len), + ReceiveBuffer::from_raw(ptr, len), + ) + } + } + + #[test] + fn schedule_tranfers_in_place_interleaved_u32() { + const FST: [u32; 7] = [3_u32, 4, 5, 6, 7, 8, 9]; + let mut fst = FST; + + const SND: [u32; 2] = [55_u32, 77]; + let mut snd = SND; + + const TRD: [u32; 9] = [87_u32, 88, 89, 90, 91, 92, 93, 94, 95]; + let mut trd = TRD; + + let mut operations = [ + Operation::TransferInPlace(&mut fst), + Operation::TransferInPlace(&mut snd), + Operation::TransferInPlace(&mut trd), + ]; + + let initials: [&[u32]; 3] = [&FST, &SND, &TRD]; + + let (tx_sched, rx_sched) = super::schedule(&mut operations); + let sched = tx_sched.zip(rx_sched); + + let update = |elem| elem + 1; + for ((tx_desc, rx_desc), initial) in sched.zip(initials) { + let mut tx = tx_desc.tx_buffer.unwrap(); + let mut rx = rx_desc.rx_buffer.unwrap(); + + for elem in initial { + assert_eq!(*elem, tx.next_read().unwrap()); + rx.next_write(update(*elem)); + } + } + + assert_eq!(fst, FST.map(update)); + assert_eq!(snd, SND.map(update)); + assert_eq!(trd, TRD.map(update)); + } + + #[test] + fn schedule_tranfers_scattered_u32() { + const DATA: [u32; 7] = [3_u32, 4, 5, 6, 7, 8, 9]; + let mut incoming = [0; 13]; + let outgoing = DATA; + + let mut operations = [Operation::Transfer(&mut incoming, &outgoing)]; + + let (tx_sched, rx_sched) = super::schedule(&mut operations); + let sched = tx_sched.zip(rx_sched); + + let update = |elem| elem + 13; + for (tx_desc, rx_desc) in sched { + let mut tx = tx_desc.tx_buffer.unwrap(); + let mut rx = rx_desc.rx_buffer.unwrap(); + + for elem in DATA { + assert_eq!(elem, tx.next_read().unwrap()); + rx.next_write(update(elem)); + } + + for rest in 249..255 { + rx.next_write(rest) + } + } + + assert_eq!( + incoming, + [16, 17, 18, 19, 20, 21, 22, 249, 250, 251, 252, 253, 254] + ); + } + + #[test] + fn schedule_writes_u32() { + const DATA: [u32; 7] = [3_u32, 4, 5, 6, 7, 8, 9]; + let outgoing = DATA; + + let mut operations = [Operation::Write(&outgoing)]; + + let (mut tx_sched, mut rx_sched) = super::schedule(&mut operations); + + fn assert_descriptor(desc: &mut Descriptor) { + let mut tx_buffer = desc.tx_buffer.take().unwrap(); + for elem in DATA { + assert_eq!(elem, tx_buffer.next_read().unwrap()) + } + assert_eq!(desc.tx_words, 7); + assert_eq!(desc.rx_words, 0); + assert!(desc.rx_buffer.is_none()); + } + + let mut tx_desc = tx_sched.next().unwrap(); + assert_descriptor(&mut tx_desc); + + let mut rx_desc = rx_sched.next().unwrap(); + assert_descriptor(&mut rx_desc); + + assert!(tx_sched.next().is_none()); + assert!(rx_sched.next().is_none()); + } + + #[test] + fn schedule_reads_u32() { + let mut incoming = [0; 13]; + let mut operations = [Operation::Read(&mut incoming)]; + + let (mut tx_sched, mut rx_sched) = super::schedule(&mut operations); + + fn assert_descriptor(desc: &mut Descriptor, fill: &[u32]) { + let mut rx_buffer = desc.rx_buffer.take().unwrap(); + for &elem in fill { + rx_buffer.next_write(elem); + } + assert_eq!(desc.rx_words, 13); + assert_eq!(desc.tx_words, 0); + assert!(desc.tx_buffer.is_none()); + } + + let mut tx_desc = tx_sched.next().unwrap(); + assert_descriptor(&mut tx_desc, &[22, 33, 44, 55, 66]); + + let mut rx_desc = rx_sched.next().unwrap(); + assert_descriptor(&mut rx_desc, &[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + + assert!(tx_sched.next().is_none()); + assert!(rx_sched.next().is_none()); + + assert_eq!(incoming, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + } + + #[test] + fn schedule_delay_ns() { + let mut operations: [Operation; 1] = [Operation::DelayNs(314)]; + let (mut tx_sched, mut rx_sched) = super::schedule(&mut operations); + + let tx = tx_sched.next().unwrap(); + assert!(tx.rx_buffer.is_none()); + assert!(tx.tx_buffer.is_none()); + assert_eq!(tx.rx_words, 0); + assert_eq!(tx.tx_words, 314); + assert!(tx.is_delay_ns()); + assert_eq!(tx.delay_ns(), Some(314)); + + let rx = rx_sched.next().unwrap(); + assert!(rx.rx_buffer.is_none()); + assert!(rx.tx_buffer.is_none()); + assert_eq!(rx.rx_words, 0); + assert_eq!(rx.tx_words, 314); + assert!(rx.is_delay_ns()); + assert_eq!(rx.delay_ns(), Some(314)); + + assert!(tx_sched.next().is_none()); + assert!(rx_sched.next().is_none()); + } + #[test] fn transfer_in_place_interleaved_read_write_u32() { const BUFFER: [u32; 9] = [42u32, 43, 44, 45, 46, 47, 48, 49, 50]; let mut buffer = BUFFER; - let (mut tx, mut rx) = super::transfer_in_place(&mut buffer); + let (mut tx, mut rx) = transfer_in_place(&mut buffer); for elem in BUFFER { assert_eq!(elem, tx.next_read().unwrap()); @@ -1648,7 +2455,7 @@ mod tests { fn transfer_in_place_interleaved_write_read_u32() { const BUFFER: [u32; 9] = [42u32, 43, 44, 45, 46, 47, 48, 49, 50]; let mut buffer = BUFFER; - let (mut tx, mut rx) = super::transfer_in_place(&mut buffer); + let (mut tx, mut rx) = transfer_in_place(&mut buffer); for elem in BUFFER { rx.next_write(elem + 1); @@ -1662,7 +2469,7 @@ mod tests { fn transfer_in_place_bulk_read_write_u32() { const BUFFER: [u32; 9] = [42u32, 43, 44, 45, 46, 47, 48, 49, 50]; let mut buffer = BUFFER; - let (mut tx, mut rx) = super::transfer_in_place(&mut buffer); + let (mut tx, mut rx) = transfer_in_place(&mut buffer); for elem in BUFFER { assert_eq!(elem, tx.next_read().unwrap()); @@ -1678,7 +2485,7 @@ mod tests { fn transfer_in_place_bulk_write_read_u32() { const BUFFER: [u32; 9] = [42u32, 43, 44, 45, 46, 47, 48, 49, 50]; let mut buffer = BUFFER; - let (mut tx, mut rx) = super::transfer_in_place(&mut buffer); + let (mut tx, mut rx) = transfer_in_place(&mut buffer); for elem in BUFFER { rx.next_write(elem + 1);