Change DMA API to use embedded-dma traits (#274)

thalesfragoso · web-flow · commit 2f7c45e4bd63 · 2020-10-15T20:40:15.000+02:00
* Change DMA API to use embedded-dma traits

* Fix dma::Transfer::peek

* Update docs on DMA code
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - SPI objects now have a `FrameSize` type field
 - Bit banding functions (`bb::*`) are now correctly marked as unsafe
 - Add missing remap to `spi3` constructor. Requires a new `mapr` argument.
+- Change DMA API to use embedded-dma traits.
 
 ### Added
 
diff --git a/Cargo.toml b/Cargo.toml
@@ -20,7 +20,7 @@ cortex-m = "0.6.0"
 nb = "0.1.2"
 cortex-m-rt = "0.6.8"
 stm32f1 = "0.11.0"
-as-slice = "0.1"
+embedded-dma = "0.1.2"
 
 [dependencies.void]
 default-features = false
diff --git a/src/adc.rs b/src/adc.rs
@@ -9,6 +9,7 @@ use crate::gpio::{gpioa, gpiob, gpioc};
 use crate::rcc::{Clocks, Enable, Reset, APB2};
 use core::sync::atomic::{self, Ordering};
 use cortex_m::asm::delay;
+use embedded_dma::StaticWriteBuffer;
 
 use crate::pac::ADC1;
 #[cfg(feature = "stm32f103")]
@@ -701,34 +702,34 @@ where
 impl<B, PINS, MODE> crate::dma::CircReadDma<B, u16> for AdcDma<PINS, MODE>
 where
     Self: TransferPayload,
-    B: as_slice::AsMutSlice<Element = u16>,
+    &'static mut [B; 2]: StaticWriteBuffer<Word = u16>,
+    B: 'static,
 {
-    fn circ_read(mut self, buffer: &'static mut [B; 2]) -> CircBuffer<B, Self> {
-        {
-            let buffer = buffer[0].as_mut_slice();
-            self.channel
-                .set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
-            self.channel
-                .set_memory_address(buffer.as_ptr() as u32, true);
-            self.channel.set_transfer_length(buffer.len() * 2);
-
-            atomic::compiler_fence(Ordering::Release);
-
-            self.channel.ch().cr.modify(|_, w| {
-                w.mem2mem()
-                    .clear_bit()
-                    .pl()
-                    .medium()
-                    .msize()
-                    .bits16()
-                    .psize()
-                    .bits16()
-                    .circ()
-                    .set_bit()
-                    .dir()
-                    .clear_bit()
-            });
-        }
+    fn circ_read(mut self, mut buffer: &'static mut [B; 2]) -> CircBuffer<B, Self> {
+        // NOTE(unsafe) We own the buffer now and we won't call other `&mut` on it
+        // until the end of the transfer.
+        let (ptr, len) = unsafe { buffer.static_write_buffer() };
+        self.channel
+            .set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
+        self.channel.set_memory_address(ptr as u32, true);
+        self.channel.set_transfer_length(len);
+
+        atomic::compiler_fence(Ordering::Release);
+
+        self.channel.ch().cr.modify(|_, w| {
+            w.mem2mem()
+                .clear_bit()
+                .pl()
+                .medium()
+                .msize()
+                .bits16()
+                .psize()
+                .bits16()
+                .circ()
+                .set_bit()
+                .dir()
+                .clear_bit()
+        });
 
         self.start();
 
@@ -739,17 +740,17 @@ where
 impl<B, PINS, MODE> crate::dma::ReadDma<B, u16> for AdcDma<PINS, MODE>
 where
     Self: TransferPayload,
-    B: as_slice::AsMutSlice<Element = u16>,
+    B: StaticWriteBuffer<Word = u16>,
 {
-    fn read(mut self, buffer: &'static mut B) -> Transfer<W, &'static mut B, Self> {
-        {
-            let buffer = buffer.as_mut_slice();
-            self.channel
-                .set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
-            self.channel
-                .set_memory_address(buffer.as_ptr() as u32, true);
-            self.channel.set_transfer_length(buffer.len());
-        }
+    fn read(mut self, mut buffer: B) -> Transfer<W, B, Self> {
+        // NOTE(unsafe) We own the buffer now and we won't call other `&mut` on it
+        // until the end of the transfer.
+        let (ptr, len) = unsafe { buffer.static_write_buffer() };
+        self.channel
+            .set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
+        self.channel.set_memory_address(ptr as u32, true);
+        self.channel.set_transfer_length(len);
+
         atomic::compiler_fence(Ordering::Release);
         self.channel.ch().cr.modify(|_, w| {
             w.mem2mem()
diff --git a/src/dma.rs b/src/dma.rs
@@ -1,16 +1,18 @@
 //! # Direct Memory Access
 #![allow(dead_code)]
 
-use core::marker::PhantomData;
-use core::ops;
+use core::{
+    marker::PhantomData,
+    sync::atomic::{compiler_fence, Ordering},
+};
+use embedded_dma::{StaticReadBuffer, StaticWriteBuffer};
 
 use crate::rcc::AHB;
 
 #[derive(Debug)]
+#[non_exhaustive]
 pub enum Error {
     Overrun,
-    #[doc(hidden)]
-    _Extensible,
 }
 
 pub enum Event {
@@ -33,7 +35,11 @@ where
     readable_half: Half,
 }
 
-impl<BUFFER, PAYLOAD> CircBuffer<BUFFER, PAYLOAD> {
+impl<BUFFER, PAYLOAD> CircBuffer<BUFFER, PAYLOAD>
+where
+    &'static mut [BUFFER; 2]: StaticWriteBuffer,
+    BUFFER: 'static,
+{
     pub(crate) fn new(buf: &'static mut [BUFFER; 2], payload: PAYLOAD) -> Self {
         CircBuffer {
             buffer: buf,
@@ -43,22 +49,6 @@ impl<BUFFER, PAYLOAD> CircBuffer<BUFFER, PAYLOAD> {
     }
 }
 
-pub trait Static<B> {
-    fn borrow(&self) -> &B;
-}
-
-impl<B> Static<B> for &'static B {
-    fn borrow(&self) -> &B {
-        *self
-    }
-}
-
-impl<B> Static<B> for &'static mut B {
-    fn borrow(&self) -> &B {
-        *self
-    }
-}
-
 pub trait DmaExt {
     type Channels;
 
@@ -70,13 +60,19 @@ pub trait TransferPayload {
     fn stop(&mut self);
 }
 
-pub struct Transfer<MODE, BUFFER, PAYLOAD> {
+pub struct Transfer<MODE, BUFFER, PAYLOAD>
+where
+    PAYLOAD: TransferPayload,
+{
     _mode: PhantomData<MODE>,
     buffer: BUFFER,
     payload: PAYLOAD,
 }
 
-impl<BUFFER, PAYLOAD> Transfer<R, BUFFER, PAYLOAD> {
+impl<BUFFER, PAYLOAD> Transfer<R, BUFFER, PAYLOAD>
+where
+    PAYLOAD: TransferPayload,
+{
     pub(crate) fn r(buffer: BUFFER, payload: PAYLOAD) -> Self {
         Transfer {
             _mode: PhantomData,
@@ -86,7 +82,10 @@ impl<BUFFER, PAYLOAD> Transfer<R, BUFFER, PAYLOAD> {
     }
 }
 
-impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, PAYLOAD> {
+impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, PAYLOAD>
+where
+    PAYLOAD: TransferPayload,
+{
     pub(crate) fn w(buffer: BUFFER, payload: PAYLOAD) -> Self {
         Transfer {
             _mode: PhantomData,
@@ -96,11 +95,13 @@ impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, PAYLOAD> {
     }
 }
 
-impl<BUFFER, PAYLOAD> ops::Deref for Transfer<R, BUFFER, PAYLOAD> {
-    type Target = BUFFER;
-
-    fn deref(&self) -> &BUFFER {
-        &self.buffer
+impl<MODE, BUFFER, PAYLOAD> Drop for Transfer<MODE, BUFFER, PAYLOAD>
+where
+    PAYLOAD: TransferPayload,
+{
+    fn drop(&mut self) {
+        self.payload.stop();
+        compiler_fence(Ordering::SeqCst);
     }
 }
 
@@ -123,14 +124,14 @@ macro_rules! dma {
     }),)+) => {
         $(
             pub mod $dmaX {
-                use core::sync::atomic::{self, Ordering};
-                use core::ptr;
+                use core::{sync::atomic::{self, Ordering}, ptr, mem};
 
                 use crate::pac::{$DMAX, dma1};
 
                 use crate::dma::{CircBuffer, DmaExt, Error, Event, Half, Transfer, W, RxDma, TxDma, TransferPayload};
                 use crate::rcc::{AHB, Enable};
 
+                #[allow(clippy::manual_non_exhaustive)]
                 pub struct Channels((), $(pub $CX),+);
 
                 $(
@@ -316,7 +317,19 @@ macro_rules! dma {
                             // we need a fence here for the same reason we need one in `Transfer.wait`
                             atomic::compiler_fence(Ordering::Acquire);
 
-                            (self.buffer, self.payload)
+                            // `Transfer` needs to have a `Drop` implementation, because we accept
+                            // managed buffers that can free their memory on drop. Because of that
+                            // we can't move out of the `Transfer`'s fields, so we use `ptr::read`
+                            // and `mem::forget`.
+                            //
+                            // NOTE(unsafe) There is no panic branch between getting the resources
+                            // and forgetting `self`.
+                            unsafe {
+                                let buffer = ptr::read(&self.buffer);
+                                let payload = ptr::read(&self.payload);
+                                mem::forget(self);
+                                (buffer, payload)
+                            }
                         }
                     }
 
@@ -342,11 +355,23 @@ macro_rules! dma {
                             // we need a fence here for the same reason we need one in `Transfer.wait`
                             atomic::compiler_fence(Ordering::Acquire);
 
-                            (self.buffer, self.payload)
+                            // `Transfer` needs to have a `Drop` implementation, because we accept
+                            // managed buffers that can free their memory on drop. Because of that
+                            // we can't move out of the `Transfer`'s fields, so we use `ptr::read`
+                            // and `mem::forget`.
+                            //
+                            // NOTE(unsafe) There is no panic branch between getting the resources
+                            // and forgetting `self`.
+                            unsafe {
+                                let buffer = ptr::read(&self.buffer);
+                                let payload = ptr::read(&self.payload);
+                                mem::forget(self);
+                                (buffer, payload)
+                            }
                         }
                     }
 
-                    impl<BUFFER, PAYLOAD> Transfer<W, &'static mut BUFFER, RxDma<PAYLOAD, $CX>>
+                    impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, RxDma<PAYLOAD, $CX>>
                     where
                         RxDma<PAYLOAD, $CX>: TransferPayload,
                     {
@@ -480,27 +505,30 @@ pub trait Transmit {
     type ReceivedWord;
 }
 
+/// Trait for circular DMA readings from peripheral to memory.
 pub trait CircReadDma<B, RS>: Receive
 where
-    B: as_slice::AsMutSlice<Element = RS>,
+    &'static mut [B; 2]: StaticWriteBuffer<Word = RS>,
+    B: 'static,
     Self: core::marker::Sized,
 {
     fn circ_read(self, buffer: &'static mut [B; 2]) -> CircBuffer<B, Self>;
 }
 
+/// Trait for DMA readings from peripheral to memory.
 pub trait ReadDma<B, RS>: Receive
 where
-    B: as_slice::AsMutSlice<Element = RS>,
-    Self: core::marker::Sized,
+    B: StaticWriteBuffer<Word = RS>,
+    Self: core::marker::Sized + TransferPayload,
 {
-    fn read(self, buffer: &'static mut B) -> Transfer<W, &'static mut B, Self>;
+    fn read(self, buffer: B) -> Transfer<W, B, Self>;
 }
 
-pub trait WriteDma<A, B, TS>: Transmit
+/// Trait for DMA writing from memory to peripheral.
+pub trait WriteDma<B, TS>: Transmit
 where
-    A: as_slice::AsSlice<Element = TS>,
-    B: Static<A>,
-    Self: core::marker::Sized,
+    B: StaticReadBuffer<Word = TS>,
+    Self: core::marker::Sized + TransferPayload,
 {
     fn write(self, buffer: B) -> Transfer<R, B, Self>;
 }
diff --git a/src/serial.rs b/src/serial.rs
diff --git a/src/spi.rs b/src/spi.rs
