Introduce traits for the DMA buffer objects (#1976)

* Introduce traits for the DMA buffer objects

* Move preparation to DMA module for reuse

---------

Co-authored-by: Dominic Fischer <git@dominicfischer.me>

esp-hal/CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- Introduce traits for the DMA buffer objects (#1976)
 - Implement `embedded-hal` output pin traits for `DummyPin` (#2019)
 - Added `esp_hal::init` to simplify HAL initialisation (#1970, #1999)
 - Added GpioPin::degrade to create ErasePins easily. Same for AnyPin by accident. (#2075)

esp-hal/src/dma/mod.rs

@@ -1187,10 +1187,10 @@ pub trait RxPrivate: crate::private::Sealed {
         chain: &DescriptorChain,
     ) -> Result<(), DmaError>;
 
-    unsafe fn prepare_transfer(
+    unsafe fn prepare_transfer<BUF: DmaRxBuffer>(
         &mut self,
         peri: DmaPeripheral,
-        first_desc: *mut DmaDescriptor,
+        buffer: &mut BUF,
     ) -> Result<(), DmaError>;
 
     fn start_transfer(&mut self) -> Result<(), DmaError>;
@@ -1371,18 +1371,20 @@ where
             .prepare_transfer_without_start(chain.first() as _, peri)
     }
 
-    unsafe fn prepare_transfer(
+    unsafe fn prepare_transfer<BUF: DmaRxBuffer>(
         &mut self,
         peri: DmaPeripheral,
-        first_desc: *mut DmaDescriptor,
+        buffer: &mut BUF,
     ) -> Result<(), DmaError> {
-        // TODO: Figure out burst mode for DmaBuf.
+        let preparation = buffer.prepare();
+
+        // TODO: Get burst mode from DmaBuf.
         if self.burst_mode {
             return Err(DmaError::InvalidAlignment);
         }
 
         self.rx_impl
-            .prepare_transfer_without_start(first_desc, peri)
+            .prepare_transfer_without_start(preparation.start, peri)
     }
 
     fn start_transfer(&mut self) -> Result<(), DmaError> {
@@ -1509,10 +1511,10 @@ pub trait TxPrivate: crate::private::Sealed {
         chain: &DescriptorChain,
     ) -> Result<(), DmaError>;
 
-    unsafe fn prepare_transfer(
+    unsafe fn prepare_transfer<BUF: DmaTxBuffer>(
         &mut self,
         peri: DmaPeripheral,
-        desc: *mut DmaDescriptor,
+        buffer: &mut BUF,
     ) -> Result<(), DmaError>;
 
     fn start_transfer(&mut self) -> Result<(), DmaError>;
@@ -1696,12 +1698,15 @@ where
             .prepare_transfer_without_start(chain.first() as _, peri)
     }
 
-    unsafe fn prepare_transfer(
+    unsafe fn prepare_transfer<BUF: DmaTxBuffer>(
         &mut self,
         peri: DmaPeripheral,
-        desc: *mut DmaDescriptor,
+        buffer: &mut BUF,
     ) -> Result<(), DmaError> {
-        self.tx_impl.prepare_transfer_without_start(desc, peri)
+        let preparation = buffer.prepare();
+
+        self.tx_impl
+            .prepare_transfer_without_start(preparation.start, peri)
     }
 
     fn start_transfer(&mut self) -> Result<(), DmaError> {
@@ -1934,6 +1939,49 @@ where
     }
 }
 
+/// Holds all the information needed to configure a DMA channel for a transfer.
+pub struct Preparation {
+    start: *mut DmaDescriptor,
+    // burst_mode, alignment, check_owner, etc.
+}
+
+/// [DmaTxBuffer] is a DMA descriptor + memory combo that can be used for
+/// transmitting data from a DMA channel to a peripheral's FIFO.
+pub trait DmaTxBuffer {
+    /// Prepares the buffer for an imminent transfer and returns
+    /// information required to use this buffer.
+    ///
+    /// Note: This operation is idempotent.
+    fn prepare(&mut self) -> Preparation;
+
+    /// Returns the maximum number of bytes that would be transmitted by this
+    /// buffer.
+    ///
+    /// This is a convenience hint for SPI. Most peripherals don't care how long
+    /// the transfer is.
+    fn length(&self) -> usize;
+}
+
+/// [DmaRxBuffer] is a DMA descriptor + memory combo that can be used for
+/// receiving data from a peripheral's FIFO to a DMA channel.
+///
+/// Note: Implementations of this trait may only support having a single EOF bit
+/// which resides in the last descriptor. There will be a separate trait in
+/// future to support multiple EOFs.
+pub trait DmaRxBuffer {
+    /// Prepares the buffer for an imminent transfer and returns
+    /// information required to use this buffer.
+    ///
+    /// Note: This operation is idempotent.
+    fn prepare(&mut self) -> Preparation;
+
+    /// Returns the maximum number of bytes that can be received by this buffer.
+    ///
+    /// This is a convenience hint for SPI. Most peripherals don't care how long
+    /// the transfer is.
+    fn length(&self) -> usize;
+}
+
 /// Error returned from Dma[Rx|Tx|RxTx]Buf operations.
 #[derive(Debug)]
 pub enum DmaBufError {
@@ -2072,9 +2120,26 @@ impl DmaTxBuf {
     pub fn as_slice(&self) -> &[u8] {
         self.buffer
     }
+}
 
-    pub(crate) fn first(&self) -> *mut DmaDescriptor {
+impl DmaTxBuffer for DmaTxBuf {
-        self.descriptors.as_ptr() as _
+    fn prepare(&mut self) -> Preparation {
+        for desc in self.descriptors.iter_mut() {
+            // Give ownership to the DMA
+            desc.set_owner(Owner::Dma);
+
+            if desc.next.is_null() {
+                break;
+            }
+        }
+
+        Preparation {
+            start: self.descriptors.as_mut_ptr(),
+        }
+    }
+
+    fn length(&self) -> usize {
+        self.len()
     }
 }
 
@@ -2283,9 +2348,34 @@ impl DmaRxBuf {
             Some(chunk)
         })
     }
+}
 
-    pub(crate) fn first(&self) -> *mut DmaDescriptor {
+impl DmaRxBuffer for DmaRxBuf {
-        self.descriptors.as_ptr() as _
+    fn prepare(&mut self) -> Preparation {
+        for desc in self.descriptors.iter_mut() {
+            // Give ownership to the DMA
+            desc.set_owner(Owner::Dma);
+
+            // Clear this to allow hardware to set it when the peripheral returns an EOF
+            // bit.
+            desc.set_suc_eof(false);
+
+            // Clear this to allow hardware to set it when it's
+            // done receiving data for this descriptor.
+            desc.set_length(0);
+
+            if desc.next.is_null() {
+                break;
+            }
+        }
+
+        Preparation {
+            start: self.descriptors.as_mut_ptr(),
+        }
+    }
+
+    fn length(&self) -> usize {
+        self.len()
     }
 }
 
@@ -2370,13 +2460,26 @@ impl DmaRxTxBuf {
         self.buffer.len()
     }
 
+    /// Return the number of bytes that would be transmitted by this buf.
+    #[allow(clippy::len_without_is_empty)]
+    pub fn len(&self) -> usize {
+        let mut result = 0;
+        for desc in self.tx_descriptors.iter() {
+            result += desc.len();
+            if desc.next.is_null() {
+                break;
+            }
+        }
+        result
+    }
+
     /// Returns the entire buf as a slice than can be read.
     pub fn as_slice(&self) -> &[u8] {
         self.buffer
     }
 
     /// Returns the entire buf as a slice than can be written.
-    pub fn as_slice_mut(&mut self) -> &mut [u8] {
+    pub fn as_mut_slice(&mut self) -> &mut [u8] {
         &mut self.buffer[..]
     }
 
@@ -2447,6 +2550,56 @@ impl DmaRxTxBuf {
     }
 }
 
+impl DmaTxBuffer for DmaRxTxBuf {
+    fn prepare(&mut self) -> Preparation {
+        for desc in self.tx_descriptors.iter_mut() {
+            // Give ownership to the DMA
+            desc.set_owner(Owner::Dma);
+
+            if desc.next.is_null() {
+                break;
+            }
+        }
+
+        Preparation {
+            start: self.tx_descriptors.as_mut_ptr(),
+        }
+    }
+
+    fn length(&self) -> usize {
+        self.len()
+    }
+}
+
+impl DmaRxBuffer for DmaRxTxBuf {
+    fn prepare(&mut self) -> Preparation {
+        for desc in self.rx_descriptors.iter_mut() {
+            // Give ownership to the DMA
+            desc.set_owner(Owner::Dma);
+
+            // Clear this to allow hardware to set it when the peripheral returns an EOF
+            // bit.
+            desc.set_suc_eof(false);
+
+            // Clear this to allow hardware to set it when it's
+            // done receiving data for this descriptor.
+            desc.set_length(0);
+
+            if desc.next.is_null() {
+                break;
+            }
+        }
+
+        Preparation {
+            start: self.rx_descriptors.as_mut_ptr(),
+        }
+    }
+
+    fn length(&self) -> usize {
+        self.len()
+    }
+}
+
 pub(crate) mod dma_private {
     use super::*;
 

esp-hal/src/spi/master.rs

@@ -81,7 +81,7 @@ use super::{
 };
 use crate::{
     clock::Clocks,
-    dma::{DmaDescriptor, DmaPeripheral, Rx, Tx},
+    dma::{DmaPeripheral, DmaRxBuffer, DmaTxBuffer, Rx, Tx},
     gpio::{InputPin, InputSignal, OutputPin, OutputSignal},
     interrupt::InterruptHandler,
     peripheral::{Peripheral, PeripheralRef},
@@ -947,7 +947,9 @@ mod dma {
             Channel,
             DmaChannel,
             DmaRxBuf,
+            DmaRxBuffer,
             DmaTxBuf,
+            DmaTxBuffer,
             RxPrivate,
             Spi2Peripheral,
             SpiPeripheral,
@@ -1266,23 +1268,18 @@ mod dma {
         /// bytes.
         #[allow(clippy::type_complexity)]
         #[cfg_attr(feature = "place-spi-driver-in-ram", ram)]
-        pub fn dma_write(
+        pub fn dma_write<TX: DmaTxBuffer>(
             mut self,
-            buffer: DmaTxBuf,
+            mut buffer: TX,
-        ) -> Result<SpiDmaTransfer<'d, T, C, FullDuplexMode, M, DmaTxBuf>, (Error, Self, DmaTxBuf)>
+        ) -> Result<SpiDmaTransfer<'d, T, C, FullDuplexMode, M, TX>, (Error, Self, TX)> {
-        {
+            let bytes_to_write = buffer.length();
-            let bytes_to_write = buffer.len();
             if bytes_to_write > MAX_DMA_SIZE {
                 return Err((Error::MaxDmaTransferSizeExceeded, self, buffer));
             }
 
             let result = unsafe {
-                self.spi.start_write_bytes_dma(
+                self.spi
-                    buffer.first(),
+                    .start_write_bytes_dma(&mut buffer, &mut self.channel.tx, true)
-                    bytes_to_write,
-                    &mut self.channel.tx,
-                    true,
-                )
             };
             if let Err(e) = result {
                 return Err((e, self, buffer));
@@ -1298,23 +1295,18 @@ mod dma {
         /// received is 32736 bytes.
         #[allow(clippy::type_complexity)]
         #[cfg_attr(feature = "place-spi-driver-in-ram", ram)]
-        pub fn dma_read(
+        pub fn dma_read<RX: DmaRxBuffer>(
             mut self,
-            buffer: DmaRxBuf,
+            mut buffer: RX,
-        ) -> Result<SpiDmaTransfer<'d, T, C, FullDuplexMode, M, DmaRxBuf>, (Error, Self, DmaRxBuf)>
+        ) -> Result<SpiDmaTransfer<'d, T, C, FullDuplexMode, M, RX>, (Error, Self, RX)> {
-        {
+            let bytes_to_read = buffer.length();
-            let bytes_to_read = buffer.len();
             if bytes_to_read > MAX_DMA_SIZE {
                 return Err((Error::MaxDmaTransferSizeExceeded, self, buffer));
             }
 
             let result = unsafe {
-                self.spi.start_read_bytes_dma(
+                self.spi
-                    buffer.first(),
+                    .start_read_bytes_dma(&mut buffer, &mut self.channel.rx, true)
-                    bytes_to_read,
-                    &mut self.channel.rx,
-                    true,
-                )
             };
             if let Err(e) = result {
                 return Err((e, self, buffer));
@@ -1329,16 +1321,14 @@ mod dma {
         /// the SPI instance. The maximum amount of data to be
         /// sent/received is 32736 bytes.
         #[allow(clippy::type_complexity)]
-        pub fn dma_transfer(
+        pub fn dma_transfer<RX: DmaRxBuffer, TX: DmaTxBuffer>(
             mut self,
-            rx_buffer: DmaRxBuf,
+            mut rx_buffer: RX,
-            tx_buffer: DmaTxBuf,
+            mut tx_buffer: TX,
-        ) -> Result<
+        ) -> Result<SpiDmaTransfer<'d, T, C, FullDuplexMode, M, (RX, TX)>, (Error, Self, RX, TX)>
-            SpiDmaTransfer<'d, T, C, FullDuplexMode, M, (DmaRxBuf, DmaTxBuf)>,
+        {
-            (Error, Self, DmaRxBuf, DmaTxBuf),
+            let bytes_to_read = rx_buffer.length();
-        > {
+            let bytes_to_write = tx_buffer.length();
-            let bytes_to_read = rx_buffer.len();
-            let bytes_to_write = tx_buffer.len();
 
             if bytes_to_write > MAX_DMA_SIZE || bytes_to_read > MAX_DMA_SIZE {
                 return Err((
@@ -1351,10 +1341,8 @@ mod dma {
 
             let result = unsafe {
                 self.spi.start_transfer_dma(
-                    rx_buffer.first(),
+                    &mut rx_buffer,
-                    tx_buffer.first(),
+                    &mut tx_buffer,
-                    bytes_to_read,
-                    bytes_to_write,
                     &mut self.channel.rx,
                     &mut self.channel.tx,
                 )
@@ -1382,16 +1370,15 @@ mod dma {
         /// Perform a half-duplex read operation using DMA.
         #[allow(clippy::type_complexity)]
         #[cfg_attr(feature = "place-spi-driver-in-ram", ram)]
-        pub fn read(
+        pub fn read<RX: DmaRxBuffer>(
             mut self,
             data_mode: SpiDataMode,
             cmd: Command,
             address: Address,
             dummy: u8,
-            buffer: DmaRxBuf,
+            mut buffer: RX,
-        ) -> Result<SpiDmaTransfer<'d, T, C, HalfDuplexMode, M, DmaRxBuf>, (Error, Self, DmaRxBuf)>
+        ) -> Result<SpiDmaTransfer<'d, T, C, HalfDuplexMode, M, RX>, (Error, Self, RX)> {
-        {
+            let bytes_to_read = buffer.length();
-            let bytes_to_read = buffer.len();
             if bytes_to_read > MAX_DMA_SIZE {
                 return Err((Error::MaxDmaTransferSizeExceeded, self, buffer));
             }
@@ -1447,12 +1434,8 @@ mod dma {
             }
 
             let result = unsafe {
-                self.spi.start_read_bytes_dma(
+                self.spi
-                    buffer.first(),
+                    .start_read_bytes_dma(&mut buffer, &mut self.channel.rx, false)
-                    bytes_to_read,
-                    &mut self.channel.rx,
-                    false,
-                )
             };
             if let Err(e) = result {
                 return Err((e, self, buffer));
@@ -1464,16 +1447,15 @@ mod dma {
         /// Perform a half-duplex write operation using DMA.
         #[allow(clippy::type_complexity)]
         #[cfg_attr(feature = "place-spi-driver-in-ram", ram)]
-        pub fn write(
+        pub fn write<TX: DmaTxBuffer>(
             mut self,
             data_mode: SpiDataMode,
             cmd: Command,
             address: Address,
             dummy: u8,
-            buffer: DmaTxBuf,
+            mut buffer: TX,
-        ) -> Result<SpiDmaTransfer<'d, T, C, HalfDuplexMode, M, DmaTxBuf>, (Error, Self, DmaTxBuf)>
+        ) -> Result<SpiDmaTransfer<'d, T, C, HalfDuplexMode, M, TX>, (Error, Self, TX)> {
-        {
+            let bytes_to_write = buffer.length();
-            let bytes_to_write = buffer.len();
             if bytes_to_write > MAX_DMA_SIZE {
                 return Err((Error::MaxDmaTransferSizeExceeded, self, buffer));
             }
@@ -1529,12 +1511,8 @@ mod dma {
             }
 
             let result = unsafe {
-                self.spi.start_write_bytes_dma(
+                self.spi
-                    buffer.first(),
+                    .start_write_bytes_dma(&mut buffer, &mut self.channel.tx, false)
-                    bytes_to_write,
-                    &mut self.channel.tx,
-                    false,
-                )
             };
             if let Err(e) = result {
                 return Err((e, self, buffer));
@@ -2296,10 +2274,8 @@ pub trait InstanceDma: Instance {
     #[allow(clippy::too_many_arguments)]
     unsafe fn start_transfer_dma<RX: Rx, TX: Tx>(
         &mut self,
-        rx_desc: *mut DmaDescriptor,
+        rx_buffer: &mut impl DmaRxBuffer,
-        tx_desc: *mut DmaDescriptor,
+        tx_buffer: &mut impl DmaTxBuffer,
-        read_buffer_len: usize,
-        write_buffer_len: usize,
         rx: &mut RX,
         tx: &mut TX,
     ) -> Result<(), Error> {
@@ -2312,7 +2288,7 @@ pub trait InstanceDma: Instance {
             reg_block.dma_in_link().write(|w| w.bits(0));
         }
 
-        self.configure_datalen(usize::max(read_buffer_len, write_buffer_len) as u32 * 8);
+        self.configure_datalen(usize::max(rx_buffer.length(), tx_buffer.length()) as u32 * 8);
 
         rx.is_done();
         tx.is_done();
@@ -2327,9 +2303,9 @@ pub trait InstanceDma: Instance {
 
         self.clear_dma_interrupts();
         reset_dma_before_load_dma_dscr(reg_block);
-        rx.prepare_transfer(self.dma_peripheral(), rx_desc)
+        rx.prepare_transfer(self.dma_peripheral(), rx_buffer)
             .and_then(|_| rx.start_transfer())?;
-        tx.prepare_transfer(self.dma_peripheral(), tx_desc)
+        tx.prepare_transfer(self.dma_peripheral(), tx_buffer)
             .and_then(|_| tx.start_transfer())?;
 
         reset_dma_before_usr_cmd(reg_block);
@@ -2342,13 +2318,12 @@ pub trait InstanceDma: Instance {
     #[cfg_attr(feature = "place-spi-driver-in-ram", ram)]
     unsafe fn start_write_bytes_dma<TX: Tx>(
         &mut self,
-        first_desc: *mut DmaDescriptor,
+        buffer: &mut impl DmaTxBuffer,
-        len: usize,
         tx: &mut TX,
         full_duplex: bool,
     ) -> Result<(), Error> {
         let reg_block = self.register_block();
-        self.configure_datalen(len as u32 * 8);
+        self.configure_datalen(buffer.length() as u32 * 8);
 
         tx.is_done();
 
@@ -2379,7 +2354,7 @@ pub trait InstanceDma: Instance {
         reset_dma_before_load_dma_dscr(reg_block);
         self.clear_dma_interrupts();
 
-        tx.prepare_transfer(self.dma_peripheral(), first_desc)?;
+        tx.prepare_transfer(self.dma_peripheral(), buffer)?;
         tx.start_transfer()?;
         reset_dma_before_usr_cmd(reg_block);
 
@@ -2396,10 +2371,9 @@ pub trait InstanceDma: Instance {
     }
 
     #[cfg_attr(feature = "place-spi-driver-in-ram", ram)]
-    unsafe fn start_read_bytes_dma<RX: Rx>(
+    unsafe fn start_read_bytes_dma<RX: Rx, BUF: DmaRxBuffer>(
         &mut self,
-        desc: *mut DmaDescriptor,
+        buffer: &mut BUF,
-        data_length: usize,
         rx: &mut RX,
         full_duplex: bool,
     ) -> Result<(), Error> {
@@ -2412,7 +2386,7 @@ pub trait InstanceDma: Instance {
             reg_block.dma_in_link().write(|w| w.bits(0));
         }
 
-        self.configure_datalen(data_length as u32 * 8);
+        self.configure_datalen(buffer.length() as u32 * 8);
 
         rx.is_done();
 
@@ -2431,7 +2405,7 @@ pub trait InstanceDma: Instance {
         self.clear_dma_interrupts();
         reset_dma_before_usr_cmd(reg_block);
 
-        rx.prepare_transfer(self.dma_peripheral(), desc)?;
+        rx.prepare_transfer(self.dma_peripheral(), buffer)?;
         rx.start_transfer()?;
 
         reg_block.cmd().modify(|_, w| w.usr().set_bit());