Merge #664

664: stm32: more spi fixes r=Dirbaio a=Dirbaio



Co-authored-by: Dario Nieuwenhuis <[email protected]>

3 files changed, 84 insertions, 35 deletions

diff --git a/embassy-stm32/src/spi/mod.rs b/embassy-stm32/src/spi/mod.rs
index 5271d941c..d8ffabb11 100644
--- a/embassy-stm32/src/spi/mod.rs
+++ b/embassy-stm32/src/spi/mod.rs
@@ -7,7 +7,7 @@ use embassy_hal_common::unborrow;
 use futures::future::join;
 
 use self::sealed::WordSize;
-use crate::dma::{NoDma, Transfer};
+use crate::dma::{slice_ptr_parts, NoDma, Transfer};
 use crate::gpio::sealed::{AFType, Pin as _};
 use crate::gpio::AnyPin;
 use crate::pac::spi::Spi as Regs;
@@ -440,9 +440,6 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
 
         tx_f.await;
 
-        // flush here otherwise `finish_dma` hangs waiting for the rx fifo to empty
-        flush_rx_fifo(T::REGS);
-
         finish_dma(T::REGS);
 
         Ok(())
@@ -465,6 +462,10 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
             set_rxdmaen(T::REGS, true);
         }
 
+        // SPIv3 clears rxfifo on SPE=0
+        #[cfg(not(spi_v3))]
+        flush_rx_fifo(T::REGS);
+
         let clock_byte_count = data.len();
 
         let rx_request = self.rxdma.request();
@@ -501,14 +502,19 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
         Ok(())
     }
 
-    pub async fn transfer<W: Word>(&mut self, read: &mut [W], write: &[W]) -> Result<(), Error>
+    async fn transfer_inner<W: Word>(
+        &mut self,
+        read: *mut [W],
+        write: *const [W],
+    ) -> Result<(), Error>
     where
         Tx: TxDma<T>,
         Rx: RxDma<T>,
     {
-        assert_eq!(read.len(), write.len());
-
-        if read.len() == 0 {
+        let (_, rx_len) = slice_ptr_parts(read);
+        let (_, tx_len) = slice_ptr_parts(write);
+        assert_eq!(rx_len, tx_len);
+        if rx_len == 0 {
             return Ok(());
         }
 
@@ -520,8 +526,8 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
             set_rxdmaen(T::REGS, true);
         }
 
-        // TODO: This is unnecessary in some versions because
-        // clearing SPE automatically clears the fifos
+        // SPIv3 clears rxfifo on SPE=0
+        #[cfg(not(spi_v3))]
         flush_rx_fifo(T::REGS);
 
         let rx_request = self.rxdma.request();
@@ -552,6 +558,22 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> {
         Ok(())
     }
 
+    pub async fn transfer<W: Word>(&mut self, read: &mut [W], write: &[W]) -> Result<(), Error>
+    where
+        Tx: TxDma<T>,
+        Rx: RxDma<T>,
+    {
+        self.transfer_inner(read, write).await
+    }
+
+    pub async fn transfer_in_place<W: Word>(&mut self, data: &mut [W]) -> Result<(), Error>
+    where
+        Tx: TxDma<T>,
+        Rx: RxDma<T>,
+    {
+        self.transfer_inner(data, data).await
+    }
+
     pub fn blocking_write<W: Word>(&mut self, words: &[W]) -> Result<(), Error> {
         self.set_word_size(W::WORDSIZE);
         for word in words.iter() {
@@ -705,26 +727,7 @@ fn spin_until_rx_ready(regs: Regs) -> Result<(), Error> {
     }
 }
 
-fn spin_until_idle(regs: Regs) {
-    #[cfg(any(spi_v1, spi_f1))]
-    unsafe {
-        while regs.sr().read().bsy() {}
-    }
-
-    #[cfg(spi_v2)]
-    unsafe {
-        while regs.sr().read().ftlvl() > 0 {}
-        while regs.sr().read().frlvl() > 0 {}
-        while regs.sr().read().bsy() {}
-    }
-
-    #[cfg(spi_v3)]
-    unsafe {
-        while !regs.sr().read().txc() {}
-        while regs.sr().read().rxplvl().0 > 0 {}
-    }
-}
-
+#[cfg(not(spi_v3))]
 fn flush_rx_fifo(regs: Regs) {
     unsafe {
         #[cfg(not(spi_v3))]
@@ -765,9 +768,15 @@ fn set_rxdmaen(regs: Regs, val: bool) {
 }
 
 fn finish_dma(regs: Regs) {
-    spin_until_idle(regs);
-
     unsafe {
+        #[cfg(spi_v2)]
+        while regs.sr().read().ftlvl() > 0 {}
+
+        #[cfg(spi_v3)]
+        while !regs.sr().read().txc() {}
+        #[cfg(not(spi_v3))]
+        while regs.sr().read().bsy() {}
+
         regs.cr1().modify(|w| {
             w.set_spe(false);
         });
@@ -935,9 +944,7 @@ cfg_if::cfg_if! {
                 &'a mut self,
                 words: &'a mut [W],
             ) -> Self::TransferInPlaceFuture<'a> {
-                // TODO: Implement async version
-                let result = self.blocking_transfer_in_place(words);
-                async move { result }
+                self.transfer_in_place(words)
             }
         }
     }
diff --git a/tests/stm32/src/bin/spi.rs b/tests/stm32/src/bin/spi.rs
index 47d0017ac..b079472d5 100644
--- a/tests/stm32/src/bin/spi.rs
+++ b/tests/stm32/src/bin/spi.rs
@@ -37,9 +37,30 @@ async fn main(_spawner: Spawner, p: Peripherals) {
     // Arduino pins D11 and D12 (MOSI-MISO) are connected together with a 1K resistor.
     // so we should get the data we sent back.
     let mut buf = data;
+    spi.blocking_transfer(&mut buf, &data).unwrap();
+    assert_eq!(buf, data);
+
     spi.blocking_transfer_in_place(&mut buf).unwrap();
     assert_eq!(buf, data);
 
+    // Check read/write don't hang. We can't check they transfer the right data
+    // without fancier test mechanisms.
+    spi.blocking_write(&buf).unwrap();
+    spi.blocking_read(&mut buf).unwrap();
+    spi.blocking_write(&buf).unwrap();
+    spi.blocking_read(&mut buf).unwrap();
+    spi.blocking_write(&buf).unwrap();
+
+    // Check transfer doesn't break after having done a write, due to garbage in the FIFO
+    spi.blocking_transfer(&mut buf, &data).unwrap();
+    assert_eq!(buf, data);
+
+    // Check zero-length operations, these should be noops.
+    spi.blocking_transfer::<u8>(&mut [], &[]).unwrap();
+    spi.blocking_transfer_in_place::<u8>(&mut []).unwrap();
+    spi.blocking_read::<u8>(&mut []).unwrap();
+    spi.blocking_write::<u8>(&[]).unwrap();
+
     info!("Test OK");
     cortex_m::asm::bkpt();
 }
diff --git a/tests/stm32/src/bin/spi_dma.rs b/tests/stm32/src/bin/spi_dma.rs
index 59a5bcd0c..3e9521ae7 100644
--- a/tests/stm32/src/bin/spi_dma.rs
+++ b/tests/stm32/src/bin/spi_dma.rs
@@ -47,6 +47,27 @@ async fn main(_spawner: Spawner, p: Peripherals) {
     spi.transfer(&mut buf, &data).await.unwrap();
     assert_eq!(buf, data);
 
+    spi.transfer_in_place(&mut buf).await.unwrap();
+    assert_eq!(buf, data);
+
+    // Check read/write don't hang. We can't check they transfer the right data
+    // without fancier test mechanisms.
+    spi.write(&buf).await.unwrap();
+    spi.read(&mut buf).await.unwrap();
+    spi.write(&buf).await.unwrap();
+    spi.read(&mut buf).await.unwrap();
+    spi.write(&buf).await.unwrap();
+
+    // Check transfer doesn't break after having done a write, due to garbage in the FIFO
+    spi.transfer(&mut buf, &data).await.unwrap();
+    assert_eq!(buf, data);
+
+    // Check zero-length operations, these should be noops.
+    spi.transfer::<u8>(&mut [], &[]).await.unwrap();
+    spi.transfer_in_place::<u8>(&mut []).await.unwrap();
+    spi.read::<u8>(&mut []).await.unwrap();
+    spi.write::<u8>(&[]).await.unwrap();
+
     info!("Test OK");
     cortex_m::asm::bkpt();
 }