Fix SPI DMA write/read for ESP32, ESP32-S2 (#2131)

* Fix SPI DMA write/read for ESP32, ESP32-S2

* CHANGELOG.md

* Review comments

esp-hal/CHANGELOG.md

@@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed an issue with DMA transfers potentially not waking up the correct async task (#2065)
 - Fixed an issue with LCD_CAM i8080 where it would send double the clocks in 16bit mode (#2085)
 - Fix i2c embedded-hal transaction (#2028)
+- Fix SPI DMA alternating `write` and `read` for ESP32 and ESP32-S2 (#2131)
 
 ### Removed
 

esp-hal/src/spi/master.rs

@@ -2304,6 +2304,14 @@ pub trait InstanceDma: Instance {
         tx: &mut TX,
     ) -> Result<(), Error> {
         let reg_block = self.register_block();
+
+        #[cfg(esp32s2)]
+        {
+            // without this a transfer after a write will fail
+            reg_block.dma_out_link().write(|w| w.bits(0));
+            reg_block.dma_in_link().write(|w| w.bits(0));
+        }
+
         self.configure_datalen(usize::max(read_buffer_len, write_buffer_len) as u32 * 8);
 
         rx.is_done();
@@ -2351,6 +2359,20 @@ pub trait InstanceDma: Instance {
                 .modify(|_, w| w.usr_miso().bit(false).usr_mosi().bit(true));
         }
 
+        #[cfg(esp32)]
+        {
+            // see https://github.com/espressif/esp-idf/commit/366e4397e9dae9d93fe69ea9d389b5743295886f
+            // see https://github.com/espressif/esp-idf/commit/0c3653b1fd7151001143451d4aa95dbf15ee8506
+            if full_duplex {
+                reg_block
+                    .dma_in_link()
+                    .modify(|_, w| unsafe { w.inlink_addr().bits(0) });
+                reg_block
+                    .dma_in_link()
+                    .modify(|_, w| w.inlink_start().set_bit());
+            }
+        }
+
         self.enable_dma();
         self.update();
 
@@ -2382,6 +2404,14 @@ pub trait InstanceDma: Instance {
         full_duplex: bool,
     ) -> Result<(), Error> {
         let reg_block = self.register_block();
+
+        #[cfg(esp32s2)]
+        {
+            // without this a read after a write will fail
+            reg_block.dma_out_link().write(|w| w.bits(0));
+            reg_block.dma_in_link().write(|w| w.bits(0));
+        }
+
         self.configure_datalen(data_length as u32 * 8);
 
         rx.is_done();

hil-test/Cargo.toml

@@ -99,6 +99,10 @@ harness = false
 name    = "spi_full_duplex_dma_pcnt"
 harness = false
 
+[[test]]
+name    = "spi_full_duplex_dma_write_read"
+harness = false
+
 [[test]]
 name    = "spi_half_duplex_read"
 harness = false

hil-test/tests/spi_full_duplex_dma_write_read.rs

@@ -0,0 +1,103 @@
+//! SPI Full Duplex DMA write + read Test
+//! See issue #2059
+//!
+//! Uses the [hil_test::common_test_pins] pair connected to each other.
+
+//% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3
+
+#![no_std]
+#![no_main]
+
+use esp_hal::{
+    dma::{Dma, DmaPriority, DmaRxBuf, DmaTxBuf},
+    dma_buffers,
+    gpio::Io,
+    peripherals::SPI2,
+    prelude::*,
+    spi::{
+        master::{Spi, SpiDma},
+        FullDuplexMode,
+        SpiMode,
+    },
+    Blocking,
+};
+use hil_test as _;
+
+cfg_if::cfg_if! {
+    if #[cfg(any(
+        feature = "esp32",
+        feature = "esp32s2",
+    ))] {
+        use esp_hal::dma::Spi2DmaChannel as DmaChannel0;
+    } else {
+        use esp_hal::dma::DmaChannel0;
+    }
+}
+
+struct Context {
+    spi: SpiDma<'static, SPI2, DmaChannel0, FullDuplexMode, Blocking>,
+}
+
+#[cfg(test)]
+#[embedded_test::tests]
+mod tests {
+    use defmt::assert_eq;
+    use esp_hal::gpio::{Level, Output};
+
+    use super::*;
+
+    #[init]
+    fn init() -> Context {
+        let peripherals = esp_hal::init(esp_hal::Config::default());
+
+        let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);
+        let sclk = io.pins.gpio0;
+        let (miso, gpio) = hil_test::common_test_pins!(io);
+        let _gpio = Output::new(gpio, Level::High);
+
+        let dma = Dma::new(peripherals.DMA);
+
+        cfg_if::cfg_if! {
+            if #[cfg(any(feature = "esp32", feature = "esp32s2"))] {
+                let dma_channel = dma.spi2channel;
+            } else {
+                let dma_channel = dma.channel0;
+            }
+        }
+
+        let spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0)
+            .with_sck(sclk)
+            .with_mosi(esp_hal::gpio::DummyPin::new())
+            .with_miso(miso)
+            .with_dma(dma_channel.configure(false, DmaPriority::Priority0));
+
+        Context { spi }
+    }
+
+    #[test]
+    #[timeout(3)]
+    fn test_write_read(ctx: Context) {
+        let spi = ctx.spi;
+        let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(4);
+        let mut dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap();
+        let mut dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap();
+
+        dma_tx_buf.fill(&[0xde, 0xad, 0xbe, 0xef]);
+
+        let transfer = spi.dma_write(dma_tx_buf).map_err(|e| e.0).unwrap();
+        let (spi, dma_tx_buf) = transfer.wait();
+
+        dma_rx_buf.as_mut_slice().fill(0);
+        let transfer = spi.dma_read(dma_rx_buf).map_err(|e| e.0).unwrap();
+        let (spi, mut dma_rx_buf) = transfer.wait();
+
+        let transfer = spi.dma_write(dma_tx_buf).map_err(|e| e.0).unwrap();
+        let (spi, _dma_tx_buf) = transfer.wait();
+
+        dma_rx_buf.as_mut_slice().fill(0);
+        let transfer = spi.dma_read(dma_rx_buf).map_err(|e| e.0).unwrap();
+        let (_, dma_rx_buf) = transfer.wait();
+
+        assert_eq!(&[0xff, 0xff, 0xff, 0xff], dma_rx_buf.as_slice());
+    }
+}