SPI DMA: use State for both blocking and async operations (#1985)

* use `State` for both blocking and async operations, remove async version of SpiDmaBus in favour of being generic over the mode

* reuse wait_for_idle more

* changelog

* rename generic params for consistency

* Add duplex mode to SpiDmaBus

* implement HalfDuplexReadWrite for SpiDmaBus

* Docs on new async APIs

* Limit half duplex transfers to the capacity of the DmaBuf

* docs

* rebase tests

* address review comments

* remove duplex traits from spi

* fix tests

* spi docs rejig

* s/InUse/TemporarilyRemoved/g

esp-hal/CHANGELOG.md

@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Introduce DMA buffer objects (#1856)
+- Introduce DMA buffer objects (#1856, #1985)
 - Added new `Io::new_no_bind_interrupt` constructor (#1861)
 - Added touch pad support for esp32 (#1873, #1956)
 - Allow configuration of period updating method for MCPWM timers (#1898)
@@ -23,7 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Peripheral driver constructors don't take `InterruptHandler`s anymore. Use `set_interrupt_handler` to explicitly set the interrupt handler now. (#1819)
-- Migrate SPI driver to use DMA buffer objects (#1856)
+- Migrate SPI driver to use DMA buffer objects (#1856, #1985)
 - Use the peripheral ref pattern for `OneShotTimer` and `PeriodicTimer` (#1855)
 - Improve SYSTIMER API (#1871)
 - DMA buffers now don't require a static lifetime. Make sure to never `mem::forget` an in-progress DMA transfer (consider using `#[deny(clippy::mem_forget)]`) (#1837)
@@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The `AesFlavour` trait no longer has the `ENCRYPT_MODE`/`DECRYPT_MODE` associated constants (#1849)
 - Removed `FlashSafeDma` (#1856)
 - Remove redundant WithDmaSpi traits (#1975)
+- `IsFullDuplex` and `IsHalfDuplex` traits (#1985)
 
 ## [0.19.0] - 2024-07-15
 

esp-hal/src/dma/mod.rs

@@ -19,7 +19,7 @@
 #![doc = crate::before_snippet!()]
 //! # use esp_hal::dma_buffers;
 //! # use esp_hal::gpio::Io;
-//! # use esp_hal::spi::{master::{Spi, prelude::*}, SpiMode};
+//! # use esp_hal::spi::{master::Spi, SpiMode};
 //! # use esp_hal::dma::{Dma, DmaPriority};
 //! # use crate::esp_hal::prelude::_fugit_RateExtU32;
 //! let dma = Dma::new(peripherals.DMA);

esp-hal/src/spi/mod.rs

@@ -79,14 +79,10 @@ pub enum SpiBitOrder {
 }
 
 /// Trait marker for defining SPI duplex modes.
-pub trait DuplexMode {}
+pub trait DuplexMode: crate::private::Sealed {}
-/// Trait marker for SPI full-duplex mode.
-pub trait IsFullDuplex: DuplexMode {}
-/// Trait marker for SPI half-duplex mode.
-pub trait IsHalfDuplex: DuplexMode {}
 
 /// SPI data mode
-#[derive(Debug, Clone, Copy, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum SpiDataMode {
     /// `Single` Data Mode - 1 bit, 2 wires.
@@ -100,9 +96,9 @@ pub enum SpiDataMode {
 /// Full-duplex operation
 pub struct FullDuplexMode {}
 impl DuplexMode for FullDuplexMode {}
-impl IsFullDuplex for FullDuplexMode {}
+impl crate::private::Sealed for FullDuplexMode {}
 
 /// Half-duplex operation
 pub struct HalfDuplexMode {}
 impl DuplexMode for HalfDuplexMode {}
-impl IsHalfDuplex for HalfDuplexMode {}
+impl crate::private::Sealed for HalfDuplexMode {}

hil-test/tests/spi_full_duplex_dma.rs

@@ -21,7 +21,7 @@ use esp_hal::{
     peripherals::{Peripherals, SPI2},
     prelude::*,
     spi::{
-        master::{dma::SpiDma, Spi},
+        master::{Spi, SpiDma},
         FullDuplexMode,
         SpiMode,
     },
@@ -49,6 +49,7 @@ struct Context {
 #[embedded_test::tests]
 mod tests {
     use defmt::assert_eq;
+    use esp_hal::dma::{DmaRxBuf, DmaTxBuf};
 
     use super::*;
 

hil-test/tests/spi_full_duplex_dma_async.rs

@@ -34,10 +34,12 @@ use esp_hal::{
     peripherals::{Peripherals, SPI2},
     prelude::*,
     spi::{
-        master::{dma::asynch::SpiDmaAsyncBus, Spi},
+        master::{Spi, SpiDmaBus},
+        FullDuplexMode,
         SpiMode,
     },
     system::SystemControl,
+    Async,
 };
 use hil_test as _;
 
@@ -55,7 +57,7 @@ cfg_if::cfg_if! {
 const DMA_BUFFER_SIZE: usize = 5;
 
 struct Context {
-    spi: SpiDmaAsyncBus<'static, SPI2, DmaChannel0>,
+    spi: SpiDmaBus<'static, SPI2, DmaChannel0, FullDuplexMode, Async>,
     pcnt_unit: Unit<'static, 0>,
     out_pin: Output<'static, GpioPin<5>>,
     mosi_mirror: GpioPin<2>,

hil-test/tests/spi_full_duplex_dma_pcnt.rs

@@ -30,7 +30,7 @@ use esp_hal::{
     peripherals::{Peripherals, SPI2},
     prelude::*,
     spi::{
-        master::{dma::SpiDma, Spi},
+        master::{Spi, SpiDma},
         FullDuplexMode,
         SpiMode,
     },

hil-test/tests/spi_half_duplex_read.rs

@@ -15,13 +15,13 @@
 
 use esp_hal::{
     clock::ClockControl,
-    dma::{Dma, DmaPriority, DmaRxBuf},
+    dma::{Dma, DmaPriority, DmaRxBuf, DmaTxBuf},
     dma_buffers,
     gpio::{GpioPin, Io, Level, Output},
     peripherals::{Peripherals, SPI2},
     prelude::*,
     spi::{
-        master::{dma::SpiDma, Address, Command, Spi},
+        master::{Address, Command, HalfDuplexReadWrite, Spi, SpiDma},
         HalfDuplexMode,
         SpiDataMode,
         SpiMode,
@@ -129,4 +129,45 @@ mod tests {
 
         assert_eq!(dma_rx_buf.as_slice(), &[0xFF; DMA_BUFFER_SIZE]);
     }
+
+    #[test]
+    #[timeout(3)]
+    fn test_spidmabus_reads_correctly_from_gpio_pin(mut ctx: Context) {
+        const DMA_BUFFER_SIZE: usize = 4;
+
+        let (buffer, descriptors, tx, txd) = dma_buffers!(DMA_BUFFER_SIZE, 1);
+        let dma_rx_buf = DmaRxBuf::new(descriptors, buffer).unwrap();
+        let dma_tx_buf = DmaTxBuf::new(txd, tx).unwrap();
+
+        let mut spi = ctx.spi.with_buffers(dma_tx_buf, dma_rx_buf);
+
+        // SPI should read '0's from the MISO pin
+        ctx.miso_mirror.set_low();
+
+        let mut buffer = [0xAA; DMA_BUFFER_SIZE];
+        spi.read(
+            SpiDataMode::Single,
+            Command::None,
+            Address::None,
+            0,
+            &mut buffer,
+        )
+        .unwrap();
+
+        assert_eq!(buffer.as_slice(), &[0x00; DMA_BUFFER_SIZE]);
+
+        // SPI should read '1's from the MISO pin
+        ctx.miso_mirror.set_high();
+
+        spi.read(
+            SpiDataMode::Single,
+            Command::None,
+            Address::None,
+            0,
+            &mut buffer,
+        )
+        .unwrap();
+
+        assert_eq!(buffer.as_slice(), &[0xFF; DMA_BUFFER_SIZE]);
+    }
 }

hil-test/tests/spi_half_duplex_write.rs

@@ -15,7 +15,7 @@
 
 use esp_hal::{
     clock::ClockControl,
-    dma::{Dma, DmaPriority, DmaTxBuf},
+    dma::{Dma, DmaPriority, DmaRxBuf, DmaTxBuf},
     dma_buffers,
     gpio::{GpioPin, Io, Pull},
     pcnt::{
@@ -26,7 +26,7 @@ use esp_hal::{
     peripherals::{Peripherals, SPI2},
     prelude::*,
     spi::{
-        master::{dma::SpiDma, Address, Command, Spi},
+        master::{Address, Command, HalfDuplexReadWrite, Spi, SpiDma},
         HalfDuplexMode,
         SpiDataMode,
         SpiMode,
@@ -143,4 +143,48 @@ mod tests {
 
         assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _);
     }
+
+    #[test]
+    #[timeout(3)]
+    fn test_spidmabus_writes_are_correctly_by_pcnt(ctx: Context) {
+        const DMA_BUFFER_SIZE: usize = 4;
+
+        let (buffer, descriptors, rx, rxd) = dma_buffers!(DMA_BUFFER_SIZE, 1);
+        let dma_tx_buf = DmaTxBuf::new(descriptors, buffer).unwrap();
+        let dma_rx_buf = DmaRxBuf::new(rxd, rx).unwrap();
+
+        let unit = ctx.pcnt_unit;
+        let mut spi = ctx.spi.with_buffers(dma_tx_buf, dma_rx_buf);
+
+        unit.channel0.set_edge_signal(PcntSource::from_pin(
+            ctx.mosi_mirror,
+            PcntInputConfig { pull: Pull::Down },
+        ));
+        unit.channel0
+            .set_input_mode(EdgeMode::Hold, EdgeMode::Increment);
+
+        let buffer = [0b0110_1010; DMA_BUFFER_SIZE];
+        // Write the buffer where each byte has 3 pos edges.
+        spi.write(
+            SpiDataMode::Single,
+            Command::None,
+            Address::None,
+            0,
+            &buffer,
+        )
+        .unwrap();
+
+        assert_eq!(unit.get_value(), (3 * DMA_BUFFER_SIZE) as _);
+
+        spi.write(
+            SpiDataMode::Single,
+            Command::None,
+            Address::None,
+            0,
+            &buffer,
+        )
+        .unwrap();
+
+        assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _);
+    }
 }