Fix double writes (#2159)

* Fix double writes

* Only flush if we need to read

esp-hal/CHANGELOG.md

@@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix i2c embedded-hal transaction (#2028)
 - Fix SPI DMA alternating `write` and `read` for ESP32 and ESP32-S2 (#2131)
 - Fix I2C ending up in a state when only re-creating the peripheral makes it useable again (#2141)
+- Fix `SpiBus::transfer` transferring data twice in some cases (#2159)
 
 ### Removed
 

esp-hal/src/spi/master.rs

@@ -2143,9 +2143,9 @@ mod ehal1 {
         fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> {
             // Optimizations
             if read.is_empty() {
-                SpiBus::write(self, write)?;
+                return SpiBus::write(self, write);
             } else if write.is_empty() {
-                SpiBus::read(self, read)?;
+                return SpiBus::read(self, read);
             }
 
             let mut write_from = 0;
@@ -2163,6 +2163,8 @@ mod ehal1 {
                     break;
                 }
 
+                // No need to flush here, `SpiBus::write` will do it for us
+
                 if write_to < read_to {
                     // Read more than we write, must pad writing part with zeros
                     let mut empty = [EMPTY_WRITE_PAD; FIFO_SIZE];
@@ -2172,9 +2174,8 @@ mod ehal1 {
                     SpiBus::write(self, &write[write_from..write_to])?;
                 }
 
-                SpiBus::flush(self)?;
-
                 if read_inc > 0 {
+                    SpiBus::flush(self)?;
                     self.spi
                         .read_bytes_from_fifo(&mut read[read_from..read_to])?;
                 }

hil-test/tests/spi_full_duplex.rs

@@ -6,15 +6,25 @@
 #![no_main]
 
 use embedded_hal::spi::SpiBus;
+#[cfg(pcnt)]
+use esp_hal::{
+    gpio::interconnect::InputSignal,
+    pcnt::{channel::EdgeMode, unit::Unit, Pcnt},
+};
 use esp_hal::{
     gpio::Io,
+    peripherals::SPI2,
     prelude::*,
     spi::{master::Spi, FullDuplexMode, SpiMode},
 };
 use hil_test as _;
 
 struct Context {
-    spi: Spi<'static, esp_hal::peripherals::SPI2, FullDuplexMode>,
+    spi: Spi<'static, SPI2, FullDuplexMode>,
+    #[cfg(pcnt)]
+    pcnt_source: InputSignal,
+    #[cfg(pcnt)]
+    pcnt_unit: Unit<'static, 0>,
 }
 
 #[cfg(test)]
@@ -34,12 +44,23 @@ mod tests {
         let (_, mosi) = hil_test::common_test_pins!(io);
 
         let mosi_loopback = mosi.peripheral_input();
+        #[cfg(pcnt)]
+        let mosi_loopback_pcnt = mosi.peripheral_input();
         let spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0)
             .with_sck(sclk)
             .with_mosi(mosi)
             .with_miso(mosi_loopback);
 
-        Context { spi }
+        #[cfg(pcnt)]
+        let pcnt = Pcnt::new(peripherals.PCNT);
+
+        Context {
+            spi,
+            #[cfg(pcnt)]
+            pcnt_source: mosi_loopback_pcnt,
+            #[cfg(pcnt)]
+            pcnt_unit: pcnt.unit0,
+        }
     }
 
     #[test]
@@ -65,6 +86,44 @@ mod tests {
         assert_eq!(read[2], 0x00u8);
     }
 
+    #[test]
+    #[timeout(3)]
+    #[cfg(pcnt)]
+    fn test_asymmetric_write(mut ctx: Context) {
+        let write = [0xde, 0xad, 0xbe, 0xef];
+
+        let unit = ctx.pcnt_unit;
+
+        unit.channel0.set_edge_signal(ctx.pcnt_source);
+        unit.channel0
+            .set_input_mode(EdgeMode::Hold, EdgeMode::Increment);
+
+        SpiBus::write(&mut ctx.spi, &write[..]).expect("Asymmetric write failed");
+        // Flush because we're not reading, so the write may happen in the background
+        ctx.spi.flush().expect("Flush failed");
+
+        assert_eq!(unit.get_value(), 9);
+    }
+
+    #[test]
+    #[timeout(3)]
+    #[cfg(pcnt)]
+    fn test_asymmetric_write_transfer(mut ctx: Context) {
+        let write = [0xde, 0xad, 0xbe, 0xef];
+
+        let unit = ctx.pcnt_unit;
+
+        unit.channel0.set_edge_signal(ctx.pcnt_source);
+        unit.channel0
+            .set_input_mode(EdgeMode::Hold, EdgeMode::Increment);
+
+        SpiBus::transfer(&mut ctx.spi, &mut [], &write[..]).expect("Asymmetric transfer failed");
+        // Flush because we're not reading, so the write may happen in the background
+        ctx.spi.flush().expect("Flush failed");
+
+        assert_eq!(unit.get_value(), 9);
+    }
+
     #[test]
     #[timeout(3)]
     fn test_symmetric_transfer_huge_buffer(mut ctx: Context) {

hil-test/tests/spi_full_duplex_dma_pcnt.rs

@@ -1,4 +1,4 @@
-//! SPI Full Duplex DMA ASYNC Test with PCNT readback.
+//! SPI Full Duplex DMA Test with PCNT readback.
 
 //% CHIPS: esp32 esp32c6 esp32h2 esp32s2 esp32s3