From 871e630e87c7eeb0479c0b6ad7a9ed8aec15fd36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?D=C3=A1niel=20Buga?= <bugadani@gmail.com>
Date: Sat, 14 Sep 2024 12:22:42 +0200
Subject: [PATCH] Fix double writes

---
 esp-hal/CHANGELOG.md                       |  1 +
 esp-hal/src/spi/master.rs                  |  4 +-
 hil-test/tests/spi_full_duplex.rs          | 63 +++++++++++++++++++++-
 hil-test/tests/spi_full_duplex_dma_pcnt.rs |  2 +-
 4 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md
index 47734e434b9..0d235ae6809 100644
--- a/esp-hal/CHANGELOG.md
+++ b/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
 
diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs
index 6d039b81ee7..9fa421907d7 100644
--- a/esp-hal/src/spi/master.rs
+++ b/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;
diff --git a/hil-test/tests/spi_full_duplex.rs b/hil-test/tests/spi_full_duplex.rs
index ce77f8b8a8f..89a09e950e7 100644
--- a/hil-test/tests/spi_full_duplex.rs
+++ b/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) {
diff --git a/hil-test/tests/spi_full_duplex_dma_pcnt.rs b/hil-test/tests/spi_full_duplex_dma_pcnt.rs
index 3818073ff3e..3ac4357c63e 100644
--- a/hil-test/tests/spi_full_duplex_dma_pcnt.rs
+++ b/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