Stop async DMA transfers when they are dropped

hal/src/dmac/channel/mod.rs

@@ -277,12 +277,17 @@ impl<Id: ChId, S: Status> Channel<Id, S> {
     #[inline]
     pub(crate) fn stop(&mut self) {
         self.regs.chctrla.modify(|_, w| w.enable().clear_bit());
+
+        // Wait for the burst to finish
+        while !self.xfer_complete() {
+            core::hint::spin_loop();
+        }
     }
 
     /// Returns whether or not the transfer is complete.
     #[inline]
     pub(crate) fn xfer_complete(&mut self) -> bool {
-        !self.regs.chctrla.read().enable().bit_is_set()
+        self.regs.chctrla.read().enable().bit_is_clear()
     }
 
     /// Returns the transfer's success status.
@@ -586,7 +591,7 @@ impl<Id: ChId> From<Channel<Id, Ready>> for Channel<Id, Uninitialized> {
 impl<Id: ChId> Channel<Id, ReadyFuture> {
     /// Begin DMA transfer using `async` operation.
     ///
-    /// If [TriggerSource::DISABLE] is used, a software
+    /// If [`TriggerSource::Disable`] is used, a software
     /// trigger will be issued to the DMA channel to launch the transfer. It
     /// is therefore not necessary, in most cases, to manually issue a
     /// software trigger to the channel.
@@ -596,7 +601,7 @@ impl<Id: ChId> Channel<Id, ReadyFuture> {
     /// In `async` mode, a transfer does NOT require `'static` source and
     /// destination buffers. This, in theory, makes
     /// [`transfer_future`](Channel::transfer_future) an `unsafe` function,
-    /// although it is marked as safe (for ergonomics).
+    /// although it is marked as safe for better ergonomics.
     ///
     /// This means that, as an user, you **must** ensure that the [`Future`]
     /// returned by this function may never be forgotten through [`forget`].
@@ -627,10 +632,6 @@ impl<Id: ChId> Channel<Id, ReadyFuture> {
         S: super::Buffer,
         D: super::Buffer<Beat = S::Beat>,
     {
-        use crate::dmac::waker::WAKERS;
-        use core::sync::atomic;
-        use core::task::Poll;
-
         super::Transfer::<Self, super::transfer::BufferPair<S, D>>::check_buffer_pair(
             &source, &dest,
         )?;
@@ -644,35 +645,8 @@ impl<Id: ChId> Channel<Id, ReadyFuture> {
         );
 
         self.configure_trigger(trig_src, trig_act);
-        let mut triggered = false;
-
-        core::future::poll_fn(|cx| {
-            atomic::fence(atomic::Ordering::Release);
-            self._enable_private();
-
-            if !triggered && trig_src == TriggerSource::Disable {
-                triggered = true;
-                self._trigger_private();
-            }
-
-            let flags_to_check = InterruptFlags::new().with_tcmpl(true).with_terr(true);
-
-            if self.check_and_clear_interrupts(flags_to_check).tcmpl() {
-                return Poll::Ready(());
-            }
-
-            WAKERS[Id::USIZE].register(cx.waker());
-            self.enable_interrupts(flags_to_check);
-
-            if self.check_and_clear_interrupts(flags_to_check).tcmpl() {
-                self.disable_interrupts(flags_to_check);
 
-                return Poll::Ready(());
-            }
-
-            Poll::Pending
-        })
-        .await;
+        TransferFuture::start(self, trig_src).await;
 
         // Defensively disable channel
         self.stop();
@@ -682,9 +656,63 @@ impl<Id: ChId> Channel<Id, ReadyFuture> {
     }
 }
 
-impl Default for InterruptFlags {
-    fn default() -> Self {
-        Self::new()
+struct TransferFuture<'a, Id: ChId> {
+    triggered: bool,
+    trig_src: TriggerSource,
+    chan: &'a mut Channel<Id, ReadyFuture>,
+}
+
+impl<'a, Id: ChId> TransferFuture<'a, Id> {
+    fn start(chan: &'a mut Channel<Id, ReadyFuture>, trig_src: TriggerSource) -> Self {
+        Self {
+            triggered: false,
+            trig_src,
+            chan,
+        }
+    }
+}
+
+impl<'a, Id: ChId> Drop for TransferFuture<'a, Id> {
+    fn drop(&mut self) {
+        self.chan.stop();
+    }
+}
+
+impl<'a, Id: ChId> core::future::Future for TransferFuture<'a, Id> {
+    type Output = ();
+
+    fn poll(
+        mut self: core::pin::Pin<&mut Self>,
+        cx: &mut core::task::Context<'_>,
+    ) -> core::task::Poll<Self::Output> {
+        use crate::dmac::waker::WAKERS;
+        use core::sync::atomic;
+        use core::task::Poll;
+
+        atomic::fence(atomic::Ordering::Release);
+        self.chan._enable_private();
+
+        if !self.triggered && self.trig_src == TriggerSource::Disable {
+            self.triggered = true;
+            self.chan._trigger_private();
+        }
+
+        let flags_to_check = InterruptFlags::new().with_tcmpl(true).with_terr(true);
+
+        if self.chan.check_and_clear_interrupts(flags_to_check).tcmpl() {
+            return Poll::Ready(());
+        }
+
+        WAKERS[Id::USIZE].register(cx.waker());
+        self.chan.enable_interrupts(flags_to_check);
+
+        if self.chan.check_and_clear_interrupts(flags_to_check).tcmpl() {
+            self.chan.disable_interrupts(flags_to_check);
+
+            return Poll::Ready(());
+        }
+
+        Poll::Pending
     }
 }
 
@@ -703,6 +731,12 @@ pub struct InterruptFlags {
     _reserved: B5,
 }
 
+impl Default for InterruptFlags {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 /// Generate a [`DmacDescriptor`], and write it to the provided descriptor
 /// reference.
 ///

hal/src/dmac/channel/reg.rs

@@ -320,5 +320,9 @@ impl<Id: ChId> Drop for RegisterBlock<Id> {
     fn drop(&mut self) {
         // Stop any potentially ongoing transfers
         self.chctrla.modify(|_, w| w.enable().clear_bit());
+
+        while self.chctrla.read().enable().bit_is_set() {
+            core::hint::spin_loop();
+        }
     }
 }

hal/src/sercom/i2c/async_api.rs

@@ -336,6 +336,7 @@ mod dma {
         S: Sercom,
         D: AnyChannel<Status = ReadyFuture>,
     {
+        #[inline]
         fn sercom_ptr(&self) -> SercomPtr<i2c::Word> {
             SercomPtr(self.i2c.data_ptr())
         }

hal/src/sercom/spi/async_api.rs

@@ -479,6 +479,7 @@ mod dma {
         T: AnyChannel<Status = ReadyFuture>,
         S: Sercom + 'static,
     {
+        #[inline]
         fn sercom_ptr(&self) -> SercomPtr<C::Word> {
             SercomPtr(self.spi.data_ptr())
         }

hal/src/sercom/uart/async_api.rs

@@ -383,26 +383,11 @@ mod dma {
     use crate::{
         dmac::{AnyChannel, Beat, ReadyFuture},
         sercom::{
-            dma::{
-                async_dma::{read_dma, write_dma},
-                SercomPtr,
-            },
+            dma::async_dma::{read_dma, write_dma},
             uart,
         },
     };
 
-    impl<C, A, S, R, T> UartFuture<C, A, R, T>
-    where
-        C: ValidConfig<Sercom = S>,
-        C::Word: Beat,
-        A: Capability,
-        S: Sercom + 'static,
-    {
-        fn sercom_ptr(&self) -> SercomPtr<C::Word> {
-            SercomPtr(self.uart.data_ptr())
-        }
-    }
-
     impl<C, D, S, R, T> UartFuture<C, D, R, T>
     where
         C: ValidConfig<Sercom = S>,
@@ -418,7 +403,7 @@ mod dma {
         pub async fn read(&mut self, words: &mut [C::Word]) -> Result<(), Error> {
             // SAFETY: Using SercomPtr is safe because we hold on
             // to &mut self as long as the transfer hasn't completed.
-            let uart_ptr = self.sercom_ptr();
+            let uart_ptr = self.uart.sercom_ptr();
 
             read_dma::<_, S>(&mut self.rx_channel, uart_ptr, words)
                 .await
@@ -441,7 +426,7 @@ mod dma {
         pub async fn write(&mut self, words: &[C::Word]) {
             // SAFETY: Using SercomPtr is safe because we hold on
             // to &mut self as long as the transfer hasn't completed.
-            let uart_ptr = self.sercom_ptr();
+            let uart_ptr = self.uart.sercom_ptr();
 
             write_dma::<_, S>(&mut self.tx_channel, uart_ptr, words)
                 .await

hal/src/sercom/uart/impl_ehal.rs

@@ -202,7 +202,7 @@ mod dma {
         D: Capability,
         W: Beat,
     {
-        fn sercom_ptr(&self) -> SercomPtr<W> {
+        pub(in super::super) fn sercom_ptr(&self) -> SercomPtr<W> {
             SercomPtr(self.data_ptr())
         }
     }