Refactor I2S driver to take DmaDescriptors later in construction (#3324)

* Refactor I2S driver to take `DmaDescriptor`s later in construction

* unused import

* stray test

esp-hal/CHANGELOG.md

@@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `esp_hal::i2c::master::AnyI2c` has been moved to `esp_hal::i2c::AnyI2c` (#3226)
 - `SpiDmaBus` no longer adjusts the DMA buffer length for each transfer (#3263)
 - `SpiDma<Async>` now uses the SPI interrupt (instead of DMA) to wait for completion (#3303)
-
+- I2S driver now takes `DmaDescriptor`s later in construction (#3324)
 - `gpio::interconnect` types now have a lifetime associated with them (#3302)
 - The `critical-section` implementation is now gated behind the `critical-section-impl` feature (#3293)
 

esp-hal/MIGRATING-1.0.0-beta.0.md

@@ -21,3 +21,33 @@ traits are now implemented for GPIO pins (stably) and driver structs (unstably)
 
 This change means it's no longer possible to pass a reference to a GPIO driver to a peripheral
 driver. For example, it's no longer possible to pass an `&mut Input` to `Spi::with_miso`.
+
+## I2S driver now takes `DmaDescriptor`s later in construction
+
+```diff
+  let i2s = I2s::new(
+      peripherals.I2S0,
+      Standard::Philips,
+      DataFormat::Data16Channel16,
+      Rate::from_hz(44100),
+      dma_channel,
+-     rx_descriptors,
+-     tx_descriptors,
+  );
+
+  let i2s_tx = i2s
+      .i2s_tx
+      .with_bclk(peripherals.GPIO2)
+      .with_ws(peripherals.GPIO4)
+      .with_dout(peripherals.GPIO5)
+-     .build();
++     .build(tx_descriptors);
+
+  let i2s_rx = i2s
+      .i2s_rx
+      .with_bclk(peripherals.GPIO2)
+      .with_ws(peripherals.GPIO4)
+      .with_din(peripherals.GPIO5)
+-     .build();
++     .build(rx_descriptors);
+```

esp-hal/src/i2s/master.rs

@@ -36,8 +36,7 @@
     not(any(esp32, esp32s2)),
     doc = "let dma_channel = peripherals.DMA_CH0;"
 )]
-//! let (mut rx_buffer, rx_descriptors, _, tx_descriptors) =
+//! let (mut rx_buffer, rx_descriptors, _, _) = dma_buffers!(4 * 4092, 0);
-//! dma_buffers!(0, 4 * 4092);
 //!
 //! let i2s = I2s::new(
 //!     peripherals.I2S0,
@@ -45,15 +44,13 @@
 //!     DataFormat::Data16Channel16,
 //!     Rate::from_hz(44100),
 //!     dma_channel,
-//!     rx_descriptors,
-//!     tx_descriptors,
 //! );
 #![cfg_attr(not(esp32), doc = "let i2s = i2s.with_mclk(peripherals.GPIO0);")]
 //! let mut i2s_rx = i2s.i2s_rx
 //!     .with_bclk(peripherals.GPIO1)
 //!     .with_ws(peripherals.GPIO2)
 //!     .with_din(peripherals.GPIO5)
-//!     .build();
+//!     .build(rx_descriptors);
 //!
 //! let mut transfer = i2s_rx.read_dma_circular(&mut rx_buffer)?;
 //!
@@ -83,7 +80,6 @@ use crate::{
         ChannelTx,
         DescriptorChain,
         DmaChannelFor,
-        DmaDescriptor,
         DmaEligible,
         DmaError,
         DmaTransferRx,
@@ -336,8 +332,6 @@ impl<'d> I2s<'d, Blocking> {
         data_format: DataFormat,
         sample_rate: Rate,
         channel: impl Peripheral<P = CH> + 'd,
-        rx_descriptors: &'static mut [DmaDescriptor],
-        tx_descriptors: &'static mut [DmaDescriptor],
     ) -> Self
     where
         CH: DmaChannelFor<AnyI2s>,
@@ -367,13 +361,11 @@ impl<'d> I2s<'d, Blocking> {
             i2s_rx: RxCreator {
                 i2s: unsafe { i2s.clone_unchecked() },
                 rx_channel: channel.rx,
-                descriptors: rx_descriptors,
                 guard: rx_guard,
             },
             i2s_tx: TxCreator {
                 i2s,
                 tx_channel: channel.tx,
-                descriptors: tx_descriptors,
                 guard: tx_guard,
             },
         }
@@ -385,13 +377,11 @@ impl<'d> I2s<'d, Blocking> {
             i2s_rx: RxCreator {
                 i2s: self.i2s_rx.i2s,
                 rx_channel: self.i2s_rx.rx_channel.into_async(),
-                descriptors: self.i2s_rx.descriptors,
                 guard: self.i2s_rx.guard,
             },
             i2s_tx: TxCreator {
                 i2s: self.i2s_tx.i2s,
                 tx_channel: self.i2s_tx.tx_channel.into_async(),
-                descriptors: self.i2s_tx.descriptors,
                 guard: self.i2s_tx.guard,
             },
         }
@@ -711,7 +701,6 @@ mod private {
     {
         pub i2s: PeripheralRef<'d, AnyI2s>,
         pub tx_channel: ChannelTx<'d, Dm, PeripheralTxChannel<AnyI2s>>,
-        pub descriptors: &'static mut [DmaDescriptor],
         pub(crate) guard: PeripheralGuard,
     }
 
@@ -719,12 +708,12 @@ mod private {
     where
         Dm: DriverMode,
     {
-        pub fn build(self) -> I2sTx<'d, Dm> {
+        pub fn build(self, descriptors: &'static mut [DmaDescriptor]) -> I2sTx<'d, Dm> {
             let peripheral = self.i2s.peripheral();
             I2sTx {
                 i2s: self.i2s,
                 tx_channel: self.tx_channel,
-                tx_chain: DescriptorChain::new(self.descriptors),
+                tx_chain: DescriptorChain::new(descriptors),
                 _guard: PeripheralGuard::new(peripheral),
             }
         }
@@ -760,7 +749,6 @@ mod private {
     {
         pub i2s: PeripheralRef<'d, AnyI2s>,
         pub rx_channel: ChannelRx<'d, Dm, PeripheralRxChannel<AnyI2s>>,
-        pub descriptors: &'static mut [DmaDescriptor],
         pub(crate) guard: PeripheralGuard,
     }
 
@@ -768,12 +756,12 @@ mod private {
     where
         Dm: DriverMode,
     {
-        pub fn build(self) -> I2sRx<'d, Dm> {
+        pub fn build(self, descriptors: &'static mut [DmaDescriptor]) -> I2sRx<'d, Dm> {
             let peripheral = self.i2s.peripheral();
             I2sRx {
                 i2s: self.i2s,
                 rx_channel: self.rx_channel,
-                rx_chain: DescriptorChain::new(self.descriptors),
+                rx_chain: DescriptorChain::new(descriptors),
                 _guard: PeripheralGuard::new(peripheral),
             }
         }

hil-test/tests/embassy_interrupt_spi_dma.rs

@@ -67,7 +67,9 @@ async fn interrupt_driven_task(spi: esp_hal::spi::master::SpiDma<'static, Blocki
 #[cfg(not(any(esp32, esp32s2, esp32s3)))]
 #[embassy_executor::task]
 async fn interrupt_driven_task(i2s_tx: esp_hal::i2s::master::I2s<'static, Blocking>) {
-    let mut i2s_tx = i2s_tx.into_async().i2s_tx.build();
+    let (_, _, _, tx_descriptors) = dma_buffers!(128);
+
+    let mut i2s_tx = i2s_tx.into_async().i2s_tx.build(tx_descriptors);
 
     loop {
         let mut buffer: [u8; 8] = [0; 8];
@@ -151,19 +153,13 @@ mod test {
         .with_dma(dma_channel2);
 
         #[cfg(not(any(esp32, esp32s2, esp32s3)))]
-        let other_peripheral = {
+        let other_peripheral = esp_hal::i2s::master::I2s::new(
-            let (_, rx_descriptors, _, tx_descriptors) = dma_buffers!(128);
-
-            esp_hal::i2s::master::I2s::new(
             peripherals.I2S0,
             esp_hal::i2s::master::Standard::Philips,
             esp_hal::i2s::master::DataFormat::Data8Channel8,
             Rate::from_khz(8),
             dma_channel2,
-                rx_descriptors,
+        );
-                tx_descriptors,
-            )
-        };
 
         let sw_ints = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT);
 

hil-test/tests/i2s.rs

@@ -141,8 +141,6 @@ mod tests {
             DataFormat::Data16Channel16,
             Rate::from_hz(16000),
             ctx.dma_channel,
-            rx_descriptors,
-            tx_descriptors,
         )
         .into_async();
 
@@ -153,14 +151,14 @@ mod tests {
             .with_bclk(NoPin)
             .with_ws(NoPin)
             .with_dout(dout)
-            .build();
+            .build(tx_descriptors);
 
         let i2s_rx = i2s
             .i2s_rx
             .with_bclk(NoPin)
             .with_ws(NoPin)
             .with_din(din)
-            .build();
+            .build(rx_descriptors);
 
         enable_loopback();
 
@@ -194,8 +192,6 @@ mod tests {
             DataFormat::Data16Channel16,
             Rate::from_hz(16000),
             ctx.dma_channel,
-            rx_descriptors,
-            tx_descriptors,
         );
 
         let (din, dout) = ctx.dout.split();
@@ -205,14 +201,14 @@ mod tests {
             .with_bclk(NoPin)
             .with_ws(NoPin)
             .with_dout(dout)
-            .build();
+            .build(tx_descriptors);
 
         let mut i2s_rx = i2s
             .i2s_rx
             .with_bclk(NoPin)
             .with_ws(NoPin)
             .with_din(din)
-            .build();
+            .build(rx_descriptors);
 
         enable_loopback();
 
@@ -295,7 +291,7 @@ mod tests {
 
     #[test]
     fn test_i2s_push_too_late(ctx: Context) {
-        let (_, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(0, 16000);
+        let (_, _, tx_buffer, tx_descriptors) = dma_buffers!(0, 16000);
 
         let i2s = I2s::new(
             ctx.i2s,
@@ -303,8 +299,6 @@ mod tests {
             DataFormat::Data16Channel16,
             Rate::from_hz(16000),
             ctx.dma_channel,
-            rx_descriptors,
-            tx_descriptors,
         );
 
         let mut i2s_tx = i2s
@@ -312,7 +306,7 @@ mod tests {
             .with_bclk(NoPin)
             .with_ws(NoPin)
             .with_dout(ctx.dout)
-            .build();
+            .build(tx_descriptors);
 
         let mut tx_transfer = i2s_tx.write_dma_circular(tx_buffer).unwrap();
 
@@ -325,7 +319,7 @@ mod tests {
     #[test]
     #[timeout(1)]
     fn test_i2s_read_too_late(ctx: Context) {
-        let (rx_buffer, rx_descriptors, _, tx_descriptors) = dma_buffers!(16000, 0);
+        let (rx_buffer, rx_descriptors, _, _) = dma_buffers!(16000, 0);
 
         let i2s = I2s::new(
             ctx.i2s,
@@ -333,8 +327,6 @@ mod tests {
             DataFormat::Data16Channel16,
             Rate::from_hz(16000),
             ctx.dma_channel,
-            rx_descriptors,
-            tx_descriptors,
         );
 
         let mut i2s_rx = i2s
@@ -342,7 +334,7 @@ mod tests {
             .with_bclk(NoPin)
             .with_ws(NoPin)
             .with_din(ctx.dout) // not a typo
-            .build();
+            .build(rx_descriptors);
 
         let mut buffer = [0u8; 1024];
         let mut rx_transfer = i2s_rx.read_dma_circular(rx_buffer).unwrap();

qa-test/src/bin/embassy_i2s_read.rs

@@ -42,7 +42,7 @@ async fn main(_spawner: Spawner) {
         }
     }
 
-    let (rx_buffer, rx_descriptors, _, tx_descriptors) = dma_buffers!(4092 * 4, 0);
+    let (rx_buffer, rx_descriptors, _, _) = dma_buffers!(4092 * 4, 0);
 
     let i2s = I2s::new(
         peripherals.I2S0,
@@ -50,8 +50,6 @@ async fn main(_spawner: Spawner) {
         DataFormat::Data16Channel16,
         Rate::from_hz(44100),
         dma_channel,
-        rx_descriptors,
-        tx_descriptors,
     )
     .into_async();
 
@@ -63,7 +61,7 @@ async fn main(_spawner: Spawner) {
         .with_bclk(peripherals.GPIO2)
         .with_ws(peripherals.GPIO4)
         .with_din(peripherals.GPIO5)
-        .build();
+        .build(rx_descriptors);
 
     let buffer = rx_buffer;
     println!("Start");

qa-test/src/bin/embassy_i2s_sound.rs

@@ -64,7 +64,7 @@ async fn main(_spawner: Spawner) {
         }
     }
 
-    let (_, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(0, 32000);
+    let (_, _, tx_buffer, tx_descriptors) = dma_buffers!(0, 32000);
 
     let i2s = I2s::new(
         peripherals.I2S0,
@@ -72,8 +72,6 @@ async fn main(_spawner: Spawner) {
         DataFormat::Data16Channel16,
         Rate::from_hz(44100),
         dma_channel,
-        rx_descriptors,
-        tx_descriptors,
     )
     .into_async();
 
@@ -82,7 +80,7 @@ async fn main(_spawner: Spawner) {
         .with_bclk(peripherals.GPIO2)
         .with_ws(peripherals.GPIO4)
         .with_dout(peripherals.GPIO5)
-        .build();
+        .build(tx_descriptors);
 
     let data =
         unsafe { core::slice::from_raw_parts(&SINE as *const _ as *const u8, SINE.len() * 2) };