Merge #1288

1288: fix(rp): spi transfer r=elpiel a=elpiel

Fixes #1181 

Co-authored-by: Lachezar Lechev <elpiel93@gmail.com>

embassy-rp/src/dma.rs

@@ -1,3 +1,4 @@
+//! Direct Memory Access (DMA)
 use core::future::Future;
 use core::pin::Pin;
 use core::sync::atomic::{compiler_fence, Ordering};

embassy-rp/src/spi.rs

@@ -1,3 +1,4 @@
+//! Serial Peripheral Interface
 use core::marker::PhantomData;
 
 use embassy_embedded_hal::SetConfig;
@@ -383,21 +384,33 @@ impl<'d, T: Instance> Spi<'d, T, Async> {
     }
 
     async fn transfer_inner(&mut self, rx_ptr: *mut [u8], tx_ptr: *const [u8]) -> Result<(), Error> {
-        let (_, from_len) = crate::dma::slice_ptr_parts(tx_ptr);
+        let (_, tx_len) = crate::dma::slice_ptr_parts(tx_ptr);
-        let (_, to_len) = crate::dma::slice_ptr_parts_mut(rx_ptr);
+        let (_, rx_len) = crate::dma::slice_ptr_parts_mut(rx_ptr);
-        assert_eq!(from_len, to_len);
+
         unsafe {
             self.inner.regs().dmacr().write(|reg| {
                 reg.set_rxdmae(true);
                 reg.set_txdmae(true);
             })
         };
-        let tx_ch = self.tx_dma.as_mut().unwrap();
+
-        let tx_transfer = unsafe {
+        let mut tx_ch = self.tx_dma.as_mut().unwrap();
-            // If we don't assign future to a variable, the data register pointer
+        // If we don't assign future to a variable, the data register pointer
-            // is held across an await and makes the future non-Send.
+        // is held across an await and makes the future non-Send.
-            crate::dma::write(tx_ch, tx_ptr, self.inner.regs().dr().ptr() as *mut _, T::TX_DREQ)
+        let tx_transfer = async {
+            let p = self.inner.regs();
+            unsafe {
+                crate::dma::write(&mut tx_ch, tx_ptr, p.dr().ptr() as *mut _, T::TX_DREQ).await;
+
+                if rx_len > tx_len {
+                    let write_bytes_len = rx_len - tx_len;
+                    // write dummy data
+                    // this will disable incrementation of the buffers
+                    crate::dma::write_repeated(tx_ch, p.dr().ptr() as *mut u8, write_bytes_len, T::TX_DREQ).await
+                }
+            }
         };
+
         let rx_ch = self.rx_dma.as_mut().unwrap();
         let rx_transfer = unsafe {
             // If we don't assign future to a variable, the data register pointer
@@ -405,6 +418,22 @@ impl<'d, T: Instance> Spi<'d, T, Async> {
             crate::dma::read(rx_ch, self.inner.regs().dr().ptr() as *const _, rx_ptr, T::RX_DREQ)
         };
         join(tx_transfer, rx_transfer).await;
+
+        // if tx > rx we should clear any overflow of the FIFO SPI buffer
+        if tx_len > rx_len {
+            let p = self.inner.regs();
+            unsafe {
+                while p.sr().read().bsy() {}
+
+                // clear RX FIFO contents to prevent stale reads
+                while p.sr().read().rne() {
+                    let _: u16 = p.dr().read().data();
+                }
+                // clear RX overrun interrupt
+                p.icr().write(|w| w.set_roric(true));
+            }
+        }
+
         Ok(())
     }
 }

tests/rp/src/bin/spi_async.rs

@@ -1,3 +1,6 @@
+//! Make sure to connect GPIO pins 3 (`PIN_3`) and 4 (`PIN_4`) together
+//! to run this test.
+//!
 #![no_std]
 #![no_main]
 #![feature(type_alias_impl_trait)]
@@ -18,10 +21,63 @@ async fn main(_spawner: Spawner) {
 
     let mut spi = Spi::new(p.SPI0, clk, mosi, miso, p.DMA_CH0, p.DMA_CH1, Config::default());
 
-    let tx_buf = [1_u8, 2, 3, 4, 5, 6];
+    // equal rx & tx buffers
-    let mut rx_buf = [0_u8; 6];
+    {
-    spi.transfer(&mut rx_buf, &tx_buf).await.unwrap();
+        let tx_buf = [1_u8, 2, 3, 4, 5, 6];
-    assert_eq!(rx_buf, tx_buf);
+        let mut rx_buf = [0_u8; 6];
+        spi.transfer(&mut rx_buf, &tx_buf).await.unwrap();
+        assert_eq!(rx_buf, tx_buf);
+    }
+
+    // tx > rx buffer
+    {
+        let tx_buf = [7_u8, 8, 9, 10, 11, 12];
+
+        let mut rx_buf = [0_u8; 3];
+        spi.transfer(&mut rx_buf, &tx_buf).await.unwrap();
+        assert_eq!(rx_buf, tx_buf[..3]);
+
+        defmt::info!("tx > rx buffer - OK");
+    }
+
+    // we make sure to that clearing FIFO works after the uneven buffers
+
+    // equal rx & tx buffers
+    {
+        let tx_buf = [13_u8, 14, 15, 16, 17, 18];
+        let mut rx_buf = [0_u8; 6];
+        spi.transfer(&mut rx_buf, &tx_buf).await.unwrap();
+        assert_eq!(rx_buf, tx_buf);
+
+        defmt::info!("buffer rx length == tx length - OK");
+    }
+
+    // rx > tx buffer
+    {
+        let tx_buf = [19_u8, 20, 21];
+        let mut rx_buf = [0_u8; 6];
+
+        // we should have written dummy data to tx buffer to sync clock.
+        spi.transfer(&mut rx_buf, &tx_buf).await.unwrap();
+
+        assert_eq!(
+            rx_buf[..3],
+            tx_buf,
+            "only the first 3 TX bytes should have been received in the RX buffer"
+        );
+        assert_eq!(rx_buf[3..], [0, 0, 0], "the rest of the RX bytes should be empty");
+        defmt::info!("buffer rx length > tx length - OK");
+    }
+
+    // equal rx & tx buffers
+    {
+        let tx_buf = [22_u8, 23, 24, 25, 26, 27];
+        let mut rx_buf = [0_u8; 6];
+        spi.transfer(&mut rx_buf, &tx_buf).await.unwrap();
+
+        assert_eq!(rx_buf, tx_buf);
+        defmt::info!("buffer rx length = tx length - OK");
+    }
 
     info!("Test OK");
     cortex_m::asm::bkpt();