From 0d5cf38c014cb098b87b56fa0c59ffa5208d5c78 Mon Sep 17 00:00:00 2001 From: Florian Zeitz Date: Sat, 8 Jun 2024 21:35:44 +0200 Subject: [PATCH] Add eh1 impls for LPSPI --- src/chip/dma.rs | 6 + src/common/lpspi.rs | 366 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 326 insertions(+), 46 deletions(-) diff --git a/src/chip/dma.rs b/src/chip/dma.rs index 4fa17b68..063b8a00 100644 --- a/src/chip/dma.rs +++ b/src/chip/dma.rs @@ -141,6 +141,12 @@ impl lpuart::Lpuart { // LPSPI use crate::lpspi; +impl lpspi::Lpspi { + pub(crate) fn wait_for_transmit_fifo_space(&self) -> Result<(), lpspi::LpspiError> { + crate::spin_on(self.spin_for_fifo_space()) + } +} + unsafe impl peripheral::Source for lpspi::Lpspi { fn source_signal(&self) -> u32 { LPSPI_DMA_RX_MAPPING[N as usize - 1] diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 93493943..645b0c11 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -16,9 +16,9 @@ //! Blocking full-duplex transfers and writes will observe an asserted chip select while data //! frames are exchanged / written. //! -//! 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). +//! If you instead want to manage chip select in software, you can use [`NoChipSelect`] as +//! the CS pin. You can also construct the driver [`without_pins`](Lpspi::without_pins) and +//! multiplex your own pins. //! //! # Device support //! @@ -75,6 +75,8 @@ //! //! # Limitations //! +//! ## embedded-hal 0.2 Transaction API +//! //! Due to [a hardware defect][1], this driver does not yet support the EH02 SPI transaction API. //! An early iteration of this driver reproduced the issue discussed in that forum. This driver may //! be able to work around the defect in software, but it hasn't been explored. @@ -84,6 +86,17 @@ //! [`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. +//! +//! ## embedded-hal 1.0 `SpiDevice` +//! +//! The current implementation of the EH1 `SpiDevice` trait does not support the `DelayNs` +//! operation. Implementing this was impossible while keeping backwards compatibility. +//! This may change in a future release. +//! +//! If you need support for the `DelayNs` operation you can use one of the devices from +//! [`embedded_hal_bus::spi`][2]. +//! +//! [2]: https://docs.rs/embedded-hal-bus/latest/embedded_hal_bus/spi/index.html use core::marker::PhantomData; use core::task::Poll; @@ -93,6 +106,13 @@ use crate::ral; pub use eh02::spi::{Mode, Phase, Polarity, MODE_0, MODE_1, MODE_2, MODE_3}; +mod private { + pub trait Sealed {} + + impl Sealed for T where T: super::lpspi::Pin {} + impl Sealed for super::NoChipSelect {} +} + /// Data direction. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Direction { @@ -372,6 +392,51 @@ pub struct ClockConfigs { pub sckdiv: u8, } +/// A pin usable as chip select. +/// +/// This is the same as [`imxrt_iomuxc::lpspi::Pin`](imxrt_iomuxc::lpspi::Pin), +/// except it also allows passing [`NoChipSelect`]. +pub trait ChipSelect: private::Sealed { + #[doc(hidden)] + type Module: consts::Unsigned; + + #[doc(hidden)] + fn prepare(&mut self); +} + +impl ChipSelect for T +where + T: lpspi::Pin, +{ + type Module = T::Module; + + fn prepare(&mut self) { + lpspi::prepare(self); + } +} + +/// A virtual pin indicating that peripheral-controlled chip select should not be used. +#[derive(Debug, Default)] +pub struct NoChipSelect { + _module: PhantomData, +} + +impl ChipSelect for NoChipSelect { + type Module = Module; + + fn prepare(&mut self) {} +} + +/// Types that are assumed to have a (real) chip select pin. +pub trait HasChipSelect {} + +impl HasChipSelect for Pins where + PCS0: lpspi::Pin +{ +} + +impl HasChipSelect for () {} + /// An LPSPI driver. /// /// The driver exposes low-level methods for coordinating @@ -431,7 +496,7 @@ 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>, + PCS0: ChipSelect>, { /// Create a new LPSPI driver from the RAL LPSPI instance and a set of pins. /// @@ -442,7 +507,7 @@ where lpspi::prepare(&mut pins.sdo); lpspi::prepare(&mut pins.sdi); lpspi::prepare(&mut pins.sck); - lpspi::prepare(&mut pins.pcs0); + pins.pcs0.prepare(); Self::init(lpspi, pins) } } @@ -670,10 +735,6 @@ impl Lpspi { .await } - pub(crate) fn wait_for_transmit_fifo_space(&self) -> Result<(), LpspiError> { - crate::spin_on(self.spin_for_fifo_space()) - } - /// Wait for receive data in a (concurrent) spin loop. /// /// This future does not care about the RX FIFO watermark. Instead, it @@ -788,55 +849,137 @@ impl Lpspi { } } - fn exchange(&mut self, data: &mut [W]) -> Result<(), LpspiError> { + async fn spin_transfer_in_place( + &mut self, + continuous: bool, + continuing: bool, + data: &mut [W], + ) -> Result<(), LpspiError> { if data.is_empty() { return Ok(()); } - let mut transaction = Transaction::new_words(data)?; - transaction.bit_order = self.bit_order(); - - self.wait_for_transmit_fifo_space()?; + let transaction = Transaction { + continuing, + continuous, + bit_order: self.bit_order(), + ..Transaction::new_words(data)? + }; + self.spin_for_fifo_space().await?; self.enqueue_transaction(&transaction); let word_count = word_count(data); let (tx, rx) = transfer_in_place(data); + futures::future::try_join( + self.spin_transmit(tx, word_count), + self.spin_receive(rx, word_count), + ) + .await?; - crate::spin_on(futures::future::try_join( + Ok(()) + } + + async fn spin_transfer( + &mut self, + continuous: bool, + continuing: bool, + read: &mut [W], + write: &[W], + ) -> Result<(), LpspiError> { + if read.is_empty() { + return self.spin_write_no_read(continuous, continuing, write).await; + } + + if write.is_empty() { + return self.spin_read_no_write(continuous, continuing, read).await; + } + + let read_len = read.len(); + let write_len = write.len(); + + let min_len = read_len.min(write_len); + + let (read_front, read_back) = read.split_at_mut(min_len); + let (write_front, write_back) = write.split_at(min_len); + + let transaction = Transaction { + continuing, + continuous: continuous | (read_len != write_len), + bit_order: self.bit_order(), + ..Transaction::new_words(read_front)? + }; + self.spin_for_fifo_space().await?; + self.enqueue_transaction(&transaction); + + let word_count = word_count(read_front); + let tx = TransmitBuffer::new(write_front); + let rx = ReceiveBuffer::new(read_front); + futures::future::try_join( self.spin_transmit(tx, word_count), self.spin_receive(rx, word_count), - )) - .map_err(|err| { - self.recover_from_error(); - err - })?; + ) + .await?; - self.flush()?; + if !read_back.is_empty() { + self.spin_read_no_write(continuous, true, read_back).await?; + } else if !write_back.is_empty() { + self.spin_write_no_read(continuous, true, write_back) + .await?; + } Ok(()) } - fn write_no_read(&mut self, data: &[W]) -> Result<(), LpspiError> { + async fn spin_read_no_write( + &mut self, + continuous: bool, + continuing: bool, + data: &mut [W], + ) -> Result<(), LpspiError> { if data.is_empty() { return Ok(()); } - let mut transaction = Transaction::new_words(data)?; - transaction.receive_data_mask = true; - transaction.bit_order = self.bit_order(); - - self.wait_for_transmit_fifo_space()?; + let transaction = Transaction { + continuous, + continuing, + transmit_data_mask: true, + bit_order: self.bit_order(), + ..Transaction::new_words(data)? + }; + self.spin_for_fifo_space().await?; self.enqueue_transaction(&transaction); let word_count = word_count(data); - let tx = TransmitBuffer::new(data); + let rx = ReceiveBuffer::new(data); + self.spin_receive(rx, word_count).await?; - crate::spin_on(self.spin_transmit(tx, word_count)).map_err(|err| { - self.recover_from_error(); - err - })?; + Ok(()) + } - self.flush()?; + async fn spin_write_no_read( + &mut self, + continuous: bool, + continuing: bool, + data: &[W], + ) -> Result<(), LpspiError> { + if data.is_empty() { + return Ok(()); + } + + let transaction = Transaction { + continuous, + continuing, + receive_data_mask: true, + bit_order: self.bit_order(), + ..Transaction::new_words(data)? + }; + self.spin_for_fifo_space().await?; + self.enqueue_transaction(&transaction); + + let word_count = word_count(data); + let tx = TransmitBuffer::new(data); + self.spin_transmit(tx, word_count).await?; Ok(()) } @@ -1231,54 +1374,186 @@ impl Drop for Disabled<'_, N> { } } -impl eh02::blocking::spi::Transfer for Lpspi { +impl eh02::blocking::spi::Transfer for Lpspi +where + P: HasChipSelect, +{ type Error = LpspiError; fn transfer<'a>(&mut self, words: &'a mut [u8]) -> Result<&'a [u8], Self::Error> { - self.exchange(words)?; + eh1::spi::SpiDevice::transfer_in_place(self, words)?; Ok(words) } } -impl eh02::blocking::spi::Transfer for Lpspi { +impl eh02::blocking::spi::Transfer for Lpspi +where + P: HasChipSelect, +{ type Error = LpspiError; fn transfer<'a>(&mut self, words: &'a mut [u16]) -> Result<&'a [u16], Self::Error> { - self.exchange(words)?; + eh1::spi::SpiDevice::transfer_in_place(self, words)?; Ok(words) } } -impl eh02::blocking::spi::Transfer for Lpspi { +impl eh02::blocking::spi::Transfer for Lpspi +where + P: HasChipSelect, +{ type Error = LpspiError; fn transfer<'a>(&mut self, words: &'a mut [u32]) -> Result<&'a [u32], Self::Error> { - self.exchange(words)?; + eh1::spi::SpiDevice::transfer_in_place(self, words)?; Ok(words) } } -impl eh02::blocking::spi::Write for Lpspi { +impl eh02::blocking::spi::Write for Lpspi +where + P: HasChipSelect, +{ type Error = LpspiError; fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { - self.write_no_read(words) + eh1::spi::SpiDevice::write(self, words) } } -impl eh02::blocking::spi::Write for Lpspi { +impl eh02::blocking::spi::Write for Lpspi +where + P: HasChipSelect, +{ type Error = LpspiError; fn write(&mut self, words: &[u16]) -> Result<(), Self::Error> { - self.write_no_read(words) + eh1::spi::SpiDevice::write(self, words) } } -impl eh02::blocking::spi::Write for Lpspi { +impl eh02::blocking::spi::Write for Lpspi +where + P: HasChipSelect, +{ type Error = LpspiError; fn write(&mut self, words: &[u32]) -> Result<(), Self::Error> { - self.write_no_read(words) + eh1::spi::SpiDevice::write(self, words) + } +} + + +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>>, N> +{ + fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + crate::spin_on(self.spin_read_no_write(false, false, words)).map_err(|err| { + self.recover_from_error(); + err + }) + } + + fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { + crate::spin_on(self.spin_write_no_read(false, false, words)).map_err(|err| { + self.recover_from_error(); + err + }) + } + + fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> { + crate::spin_on(self.spin_transfer(false, false, read, write)).map_err(|err| { + self.recover_from_error(); + err + }) + } + + fn transfer_in_place(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + crate::spin_on(self.spin_transfer_in_place(false, false, words)).map_err(|err| { + self.recover_from_error(); + err + }) + } + + fn flush(&mut self) -> Result<(), Self::Error> { + self.flush() + } +} + +impl eh1::spi::SpiDevice for Lpspi +where + W: Word + 'static, + P: HasChipSelect, +{ + fn transaction( + &mut self, + operations: &mut [eh1::spi::Operation<'_, W>], + ) -> Result<(), Self::Error> { + use eh1::spi::Operation; + + let operations_len = operations.len(); + + if operations_len == 0 { + return Ok(()); + } + + crate::spin_on(async { + let mut continuing = false; + + for (i, op) in operations.iter_mut().enumerate() { + // For the last transaction `continuous` needs to be set to false. + // Otherwise the hardware fails to correctly de-assert CS. + let continuous = i + 1 != operations_len; + match op { + Operation::Read(data) | Operation::Transfer(data, []) => { + self.spin_read_no_write(continuous, continuing, data) + .await?; + } + Operation::Write(data) | Operation::Transfer([], data) => { + self.spin_write_no_read(continuous, continuing, data) + .await?; + } + Operation::Transfer(read, write) => { + self.spin_transfer(continuous, continuing, read, write) + .await?; + } + Operation::TransferInPlace(data) => { + self.spin_transfer_in_place(continuous, continuing, data) + .await?; + } + Operation::DelayNs(_) => { + panic!( + "DelayNs not implemented, please use embedded-hal-bus if you need it." + ); + } + } + + continuing = true; + } + + Ok(()) + }) + .map_err(|err| { + self.recover_from_error(); + err + })?; + + self.flush() } } @@ -1463,7 +1738,6 @@ impl<'a, W> ReceiveBuffer<'a, W> where W: Word, { - #[cfg(test)] // TODO(mciantyre) remove once needed in non-test code. fn new(buffer: &'a mut [W]) -> Self { // Safety: pointer offset math meets expectations. unsafe { Self::from_raw(buffer.as_mut_ptr(), buffer.len()) }