From 8243da625aa51c7e43fae641bdbd9260ee2cd228 Mon Sep 17 00:00:00 2001 From: Justin Beaurivage Date: Fri, 6 Dec 2024 13:19:27 -0500 Subject: [PATCH] changed(spi): Remove explicit flushes from SpiBus impl --- hal/src/sercom/spi.rs | 18 ++++++++++++++++++ hal/src/sercom/spi/async_api/dma.rs | 5 ++--- hal/src/sercom/spi/async_api/mod.rs | 7 ------- hal/src/sercom/spi/impl_ehal/dma.rs | 7 +++---- hal/src/sercom/spi/impl_ehal/mod.rs | 11 ----------- 5 files changed, 23 insertions(+), 25 deletions(-) diff --git a/hal/src/sercom/spi.rs b/hal/src/sercom/spi.rs index 1069d0d02c8..d79414f3971 100644 --- a/hal/src/sercom/spi.rs +++ b/hal/src/sercom/spi.rs @@ -258,6 +258,24 @@ //! let rcvd: u16 = block!(spi.read()); //! ``` //! +//! ## Flushing the bus +//! +//! The [`SpiBus`](crate::ehal::spi::SpiBus) methods do not flush the bus when a +//! transaction is complete. This is in part to increase performance and allow +//! for pipelining SPI transactions. This is true for both sync and async +//! operation. As such, you should ensure you manually call +//! [`flush`](crate::ehal::spi::SpiBus::flush) when: +//! * You must synchronize SPI activity and GPIO activity, for example before +//! deasserting a CS pin. +//! * Before deinitializing the SPI peripheral. +//! +//! Take note that the [`SpiDevice`](crate::ehal::spi::SpiDevice) +//! implementations automatically take care of flushing, so no further flushing +//! is needed. +//! +//! [See the embedded-hal spec](https://docs.rs/embedded-hal/latest/embedded_hal/spi/index.html#flushing) +//! for more information. +//! //! # [`PanicOnRead`] and [`PanicOnWrite`] //! //! Some driver libraries take a type implementing [`embedded_hal::spi::SpiBus`] diff --git a/hal/src/sercom/spi/async_api/dma.rs b/hal/src/sercom/spi/async_api/dma.rs index 070db74d237..bfb1c9465b6 100644 --- a/hal/src/sercom/spi/async_api/dma.rs +++ b/hal/src/sercom/spi/async_api/dma.rs @@ -194,7 +194,6 @@ where self.spi.config.as_mut().regs.rx_enable(); } - self.flush_tx().await; tx_result?; Ok(words.len()) } @@ -229,7 +228,7 @@ where ); // Check for overflows or DMA errors - self.flush_tx_rx().await?; + self.spi.read_status().check_bus_error()?; rx_result.and(tx_result)?; Ok(()) } @@ -365,7 +364,7 @@ where }; // Check for overflows or DMA errors - self.flush_tx_rx().await?; + self.spi.read_status().check_bus_error()?; rx_result.and(tx_result)?; Ok(()) } diff --git a/hal/src/sercom/spi/async_api/mod.rs b/hal/src/sercom/spi/async_api/mod.rs index 701ff4bcb43..f07fdeb0274 100644 --- a/hal/src/sercom/spi/async_api/mod.rs +++ b/hal/src/sercom/spi/async_api/mod.rs @@ -283,13 +283,6 @@ where async fn flush_rx(&mut self) -> Result<(), Error> { self.wait_flags(Flags::RXC).await } - - /// Wait on TXC and RXC flags - #[inline] - #[cfg(feature = "dma")] - async fn flush_tx_rx(&mut self) -> Result<(), Error> { - self.wait_flags(Flags::TXC | Flags::RXC).await - } } impl AsRef> for SpiFuture diff --git a/hal/src/sercom/spi/impl_ehal/dma.rs b/hal/src/sercom/spi/impl_ehal/dma.rs index 8c2e0a3ae80..ed7fb52dea1 100644 --- a/hal/src/sercom/spi/impl_ehal/dma.rs +++ b/hal/src/sercom/spi/impl_ehal/dma.rs @@ -74,7 +74,6 @@ where } self._tx_channel.as_mut().xfer_success()?; - self.flush_tx(); Ok(buf.len()) } } @@ -119,7 +118,7 @@ where rx.stop(); // Check for overflows or DMA errors - self.flush_tx_rx()?; + self.read_status().check_bus_error()?; self._rx_channel .as_mut() .xfer_success() @@ -259,7 +258,7 @@ where rx.stop(); // Check for overflows or DMA errors - self.flush_tx_rx()?; + self.read_status().check_bus_error()?; self._rx_channel .as_mut() .xfer_success() @@ -368,7 +367,7 @@ where rx.stop(); // Check for overflows or DMA errors - self.flush_rx()?; + self.read_status().check_bus_error()?; self._rx_channel.as_mut().xfer_success()?; Ok(buf.len()) } diff --git a/hal/src/sercom/spi/impl_ehal/mod.rs b/hal/src/sercom/spi/impl_ehal/mod.rs index b1ebaee830c..fc8a1171bd1 100644 --- a/hal/src/sercom/spi/impl_ehal/mod.rs +++ b/hal/src/sercom/spi/impl_ehal/mod.rs @@ -113,7 +113,6 @@ where *r = self.transfer_word_in_place(*w)?; } - self.flush_tx(); Ok(()) } @@ -129,13 +128,6 @@ where fn flush_rx(&mut self) -> Result<(), Error> { self.block_on_flags(Flags::RXC) } - - /// Wait on TXC and RXC flags - #[inline] - #[cfg(feature = "dma")] - fn flush_tx_rx(&mut self) -> Result<(), Error> { - self.block_on_flags(Flags::TXC | Flags::RXC) - } } // Implementations specific to Master mode SPIs. @@ -156,7 +148,6 @@ where for word in words.iter_mut() { *word = self.transfer_word_in_place(self.config.nop_word.as_())?; } - self.flush_tx(); Ok(()) } @@ -171,7 +162,6 @@ where self.write_data(word.as_()); } } - self.flush_tx(); // Reenable receiver only if necessary if D::RX_ENABLE { @@ -213,7 +203,6 @@ where *word = read; } - self.flush_tx(); Ok(()) }