Add free method, don't consume self in abort/wait_for_transfer_complete

Author: astapleton

examples/dma.rs

@@ -7,52 +7,12 @@ mod utilities;
 use cortex_m::singleton;
 use cortex_m_rt::entry;
 use cortex_m_semihosting::debug;
-use embedded_dma::{ReadBuffer, WriteBuffer};
 use stm32h5xx_hal::{
-    gpdma::{config::transform::*, DmaConfig, DmaTransfer, Word},
+    gpdma::{config::transform::*, DmaConfig, DmaTransfer},
     pac,
     prelude::*,
 };
 
-// Buffer is used to manage a reference to a static buffer returned by the cortex_m::singleton!
-// macro and which can be with the DmaTransfer API (which requires passing ReadBuffer and
-// WriteBuffer implementations by value) and then used to access the buffer after the transfer has
-// completed.
-struct Buffer<T: Word + 'static, const N: usize> {
-    data: &'static mut [T; N],
-}
-
-impl<T, const N: usize> Buffer<T, N>
-where
-    T: Word + 'static,
-{
-    fn new(data: &'static mut [T; N]) -> Self {
-        Self { data }
-    }
-}
-
-unsafe impl<T, const N: usize> ReadBuffer for &Buffer<T, N>
-where
-    T: Word + 'static,
-{
-    type Word = T;
-
-    unsafe fn read_buffer(&self) -> (*const Self::Word, usize) {
-        (self.data.as_ptr(), N)
-    }
-}
-
-unsafe impl<T, const N: usize> WriteBuffer for &mut Buffer<T, N>
-where
-    T: Word + 'static,
-{
-    type Word = T;
-
-    unsafe fn write_buffer(&mut self) -> (*mut Self::Word, usize) {
-        (self.data.as_mut_ptr(), N)
-    }
-}
-
 #[entry]
 fn main() -> ! {
     utilities::logger::init();
@@ -72,108 +32,90 @@ fn main() -> ! {
     let src =
         singleton!(: [u8; 40] = core::array::from_fn(|i| i as u8)).unwrap();
 
-    let src_buffer = Buffer::new(src);
     let dest = singleton!(: [u8; 40] = [0u8; 40]).unwrap();
-    let mut dest_buffer = Buffer::new(dest);
 
     let mut channel = channels.0;
     let config = DmaConfig::new();
-    let mut transfer = DmaTransfer::memory_to_memory(
-        config,
-        &mut channel,
-        &src_buffer,
-        &mut dest_buffer,
-    );
+    let mut transfer =
+        DmaTransfer::memory_to_memory(config, &mut channel, src, dest);
     transfer.start().unwrap();
     transfer.wait_for_transfer_complete().unwrap();
-    assert_eq!(src_buffer.data, dest_buffer.data);
+    let (src, dest) = transfer.free();
+    assert_eq!(src, dest);
 
     log::info!("u32 to u32 with data transform");
     let src = singleton!(: [u32; 10] = [0x12345678u32; 10]).unwrap();
     let dest = singleton!(: [u32; 10] = [0u32; 10]).unwrap();
-    let mut dest_buffer = Buffer::new(dest);
 
     let config = DmaConfig::new().with_data_transform(
         DataTransform::builder()
             .swap_destination_half_words()
             .swap_destination_half_word_byte_order(),
     );
 
-    let mut transfer = DmaTransfer::memory_to_memory(
-        config,
-        &mut channel,
-        src,
-        &mut dest_buffer,
-    );
+    let mut transfer =
+        DmaTransfer::memory_to_memory(config, &mut channel, src, dest);
 
     transfer.start().unwrap();
     transfer.wait_for_transfer_complete().unwrap();
+    let (_, dest) = transfer.free();
+
     let expected = [0x78563412; 10];
-    assert_eq!(expected, *dest_buffer.data);
+    assert_eq!(expected, *dest);
 
     log::info!("u32 to u16 with truncate");
     let src = singleton!(: [u32; 10] = [0x12345678u32; 10]).unwrap();
     let dest = singleton!(: [u16; 20] = [0u16; 20]).unwrap();
-    let mut dest_buffer = Buffer::new(dest);
 
     let config = DmaConfig::new().with_data_transform(
         DataTransform::builder().left_align_right_truncate(),
     );
-    let mut transfer = DmaTransfer::memory_to_memory(
-        config,
-        &mut channel,
-        src,
-        &mut dest_buffer,
-    );
+    let mut transfer =
+        DmaTransfer::memory_to_memory(config, &mut channel, src, dest);
 
     transfer.start().unwrap();
     transfer.wait_for_transfer_complete().unwrap();
+    let (_, dest) = transfer.free();
+
     let expected = [0x1234; 10];
-    assert_eq!(expected, (*dest_buffer.data)[0..10]);
+    assert_eq!(expected, (*dest)[0..10]);
 
     log::info!("u32 to u8 with unpack");
     let src = singleton!(: [u32; 10] = [0x12345678u32; 10]).unwrap();
     let dest = singleton!(: [u8; 40] = [0u8; 40]).unwrap();
-    let mut dest_buffer = Buffer::new(dest);
 
     let config =
         DmaConfig::new().with_data_transform(DataTransform::builder().unpack());
-    let mut transfer = DmaTransfer::memory_to_memory(
-        config,
-        &mut channel,
-        src,
-        &mut dest_buffer,
-    );
+    let mut transfer =
+        DmaTransfer::memory_to_memory(config, &mut channel, src, dest);
 
     transfer.start().unwrap();
     transfer.wait_for_transfer_complete().unwrap();
+    let (_, dest) = transfer.free();
     let expected = [0x78, 0x56, 0x34, 0x12];
-    assert_eq!(expected, (*dest_buffer.data)[0..4]);
-    assert_eq!(expected, (*dest_buffer.data)[36..40]);
+    assert_eq!(expected, (*dest)[0..4]);
+    assert_eq!(expected, (*dest)[36..40]);
 
     log::info!("u8 to u32 with pack");
     let src = singleton!(: [u8; 40] = [0u8; 40]).unwrap();
     let dest = singleton!(: [u32; 10] = [0u32; 10]).unwrap();
-    let mut dest_buffer = Buffer::new(dest);
 
     for chunk in src.chunks_mut(4) {
         chunk.copy_from_slice(&[0x78, 0x56, 0x34, 0x12]);
     }
 
     let config =
         DmaConfig::new().with_data_transform(DataTransform::builder().pack());
-    let mut transfer = DmaTransfer::memory_to_memory(
-        config,
-        &mut channel,
-        src,
-        &mut dest_buffer,
-    );
+    let mut transfer =
+        DmaTransfer::memory_to_memory(config, &mut channel, src, dest);
 
     transfer.start().unwrap();
     transfer.wait_for_transfer_complete().unwrap();
+    let (_, dest) = transfer.free();
+
     let expected = [0x12345678; 10];
-    assert_eq!(expected, (*dest_buffer.data));
-    assert_eq!(expected, (*dest_buffer.data));
+    assert_eq!(expected, *dest);
+    assert_eq!(expected, *dest);
 
     log::info!("All tests passed!");
     loop {

src/gpdma.rs

@@ -82,6 +82,7 @@ use crate::{
 };
 use core::{
     marker::PhantomData,
+    mem,
     ops::Deref,
     sync::atomic::{fence, Ordering},
 };
@@ -252,12 +253,8 @@ where
     D: WriteBuffer<Word: Word>,
 {
     channel: &'a mut CH,
-
-    // Hold onto source and destination for the lifetime of the transfer to ensure that any
-    // user implementations of ReadBuffer and WriteBuffer that are themselves references are held
-    // for the duration of the transfer and so prevent access to the underlying data.
-    _source: S,
-    _destination: D,
+    source: S,
+    destination: D,
 }
 
 impl<'a, CH, S, D> DmaTransfer<'a, CH, S, D>
@@ -289,8 +286,8 @@ where
 
         Self {
             channel,
-            _source: source,
-            _destination: destination,
+            source,
+            destination,
         }
     }
 
@@ -501,13 +498,12 @@ where
 
     /// Blocks waiting for a transfer to complete. Returns an error if one occurred during the
     /// transfer.
-    pub fn wait_for_transfer_complete(self) -> Result<(), Error> {
+    pub fn wait_for_transfer_complete(&mut self) -> Result<(), Error> {
         let result = self.channel.wait_for_transfer_complete();
         // Preserve the instruction and bus sequence of the preceding operation and
         // the subsequent buffer access.
         fence(Ordering::SeqCst);
 
-        core::mem::forget(self); // Prevents self from being dropped and attempting to abort
         result
     }
 
@@ -529,8 +525,28 @@ where
     }
 
     /// Abort a transaction and wait for it to suspend the transfer before resetting the channel
-    pub fn abort(self) {
-        // Allow Drop implementation to handle transfer abortion
+    pub fn abort(&mut self) {
+        if self.is_running() {
+            self.channel.abort();
+        }
+
+        self.disable_interrupts();
+
+        // Preserve the instruction and bus sequence of the preceding operation and
+        // the subsequent buffer access.
+        fence(Ordering::SeqCst);
+    }
+
+    pub fn free(mut self) -> (S, D) {
+        self.abort();
+        let (src, dest) = unsafe {
+            (
+                core::ptr::read(&self.source),
+                core::ptr::read(&self.destination),
+            )
+        };
+        mem::forget(self);
+        (src, dest)
     }
 }
 
@@ -541,14 +557,6 @@ where
     D: WriteBuffer<Word: Word>,
 {
     fn drop(&mut self) {
-        if self.is_running() {
-            self.channel.abort();
-        }
-
-        self.disable_interrupts();
-
-        // Preserve the instruction and bus sequence of the preceding operation and
-        // the subsequent buffer access.
-        fence(Ordering::SeqCst);
+        self.abort();
     }
 }