serial: Enable DMA only when we actually use it

Instead of always enabling DMA on the USART peripherals, we want to
enable it only when we actually do a transfer. To this end, this commit
renames the `Target` trait mapping DMA targets to their channels to
`OnChannel`. The new `Target` trait now provides methods to start and
stop DMA on the implementing target, which are called when a `Transfer`
starts or stops DMA.

Author: teskje

src/dma.rs

@@ -27,13 +27,21 @@ pub trait DmaExt {
     fn split(self, ahb: &mut AHB) -> Self::Channels;
 }
 
+/// Trait implemented by DMA targets.
+pub trait Target {
+    /// Enable DMA on the target
+    fn enable_dma(&mut self) {}
+    /// Disable DMA on the target
+    fn disable_dma(&mut self) {}
+}
+
 /// An in-progress one-shot DMA transfer
-pub struct Transfer<B, C: Channel, T> {
+pub struct Transfer<B, C: Channel, T: Target> {
     // This is always a `Some` outside of `drop`.
     inner: Option<TransferInner<B, C, T>>,
 }
 
-impl<B, C: Channel, T> Transfer<B, C, T> {
+impl<B, C: Channel, T: Target> Transfer<B, C, T> {
     /// Start a DMA write transfer.
     ///
     /// # Panics
@@ -42,7 +50,7 @@ impl<B, C: Channel, T> Transfer<B, C, T> {
     pub fn start_write(mut buffer: B, mut channel: C, target: T) -> Self
     where
         B: WriteBuffer + 'static,
-        T: Target<C>,
+        T: OnChannel<C>,
     {
         // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its
         // concrete type is unknown here
@@ -65,7 +73,7 @@ impl<B, C: Channel, T> Transfer<B, C, T> {
     pub fn start_read(buffer: B, mut channel: C, target: T) -> Self
     where
         B: ReadBuffer + 'static,
-        T: Target<C>,
+        T: OnChannel<C>,
     {
         // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its
         // concrete type is unknown here
@@ -86,14 +94,15 @@ impl<B, C: Channel, T> Transfer<B, C, T> {
     ///
     /// - the given buffer will be valid for the duration of the transfer
     /// - the DMA channel is configured correctly for the given target and buffer
-    unsafe fn start(buffer: B, mut channel: C, target: T) -> Self
+    unsafe fn start(buffer: B, mut channel: C, mut target: T) -> Self
     where
-        T: Target<C>,
+        T: OnChannel<C>,
     {
         assert!(!channel.is_enabled());
 
         atomic::compiler_fence(Ordering::Release);
 
+        target.enable_dma();
         channel.enable();
 
         Self {
@@ -127,7 +136,7 @@ impl<B, C: Channel, T> Transfer<B, C, T> {
     }
 }
 
-impl<B, C: Channel, T> Drop for Transfer<B, C, T> {
+impl<B, C: Channel, T: Target> Drop for Transfer<B, C, T> {
     fn drop(&mut self) {
         if let Some(inner) = self.inner.as_mut() {
             inner.stop();
@@ -142,10 +151,12 @@ struct TransferInner<B, C, T> {
     target: T,
 }
 
-impl<B, C: Channel, T> TransferInner<B, C, T> {
+impl<B, C: Channel, T: Target> TransferInner<B, C, T> {
     /// Stop this transfer
     fn stop(&mut self) {
         self.channel.disable();
+        self.target.disable_dma();
+
         atomic::compiler_fence(Ordering::SeqCst);
     }
 }
@@ -674,7 +685,6 @@ macro_rules! dma {
     };
 }
 
-#[cfg(feature = "stm32f303")]
 dma!(
     DMA1, dma1, dma1en,
     channels: {
@@ -705,20 +715,25 @@ dma!(
     },
 );
 
-/// Marker trait for DMA targets and their channels
-pub unsafe trait Target<C: Channel> {}
+/// Marker trait mapping DMA targets to their channels
+///
+/// # Safety
+///
+/// `C` must be the correct DMA channel for the peripheral implementing
+/// this trait.
+pub unsafe trait OnChannel<C: Channel>: Target {}
 
-macro_rules! targets {
+macro_rules! on_channel {
     (
         $dma:ident,
         $( $target:ty => $C:ident, )+
     ) => {
-        $( unsafe impl Target<$dma::$C> for $target {} )+
+        $( unsafe impl OnChannel<$dma::$C> for $target {} )+
     };
 }
 
 #[cfg(feature = "stm32f303")]
-targets!(dma1,
+on_channel!(dma1,
     serial::Rx<pac::USART1> => C5,
     serial::Tx<pac::USART1> => C4,
     serial::Rx<pac::USART2> => C6,

src/serial.rs

@@ -8,6 +8,7 @@ use crate::{
     time::Bps,
 };
 use core::{convert::Infallible, marker::PhantomData, ptr};
+use cortex_m::interrupt;
 use nb;
 
 #[cfg(any(
@@ -39,7 +40,7 @@ use crate::gpio::gpiod;
 use crate::gpio::gpioe;
 
 #[cfg(feature = "stm32f303")]
-pub use crate::dma;
+use crate::dma;
 
 /// Interrupt event
 pub enum Event {
@@ -242,11 +243,6 @@ macro_rules! hal {
                     // NOTE(write): uses all bits of this register.
                     usart.brr.write(|w| unsafe { w.bits(brr) });
 
-                    usart.cr3.modify(|_, w| {
-                        w.dmar().enabled();  // enable DMA for reception
-                        w.dmat().enabled()   // enable DMA for transmission
-                    });
-
                     usart.cr1.modify(|_, w| {
                         w.ue().enabled();  // enable USART
                         w.re().enabled();  // enable receiver
@@ -377,7 +373,7 @@ macro_rules! hal {
                     mut channel: C
                 ) -> dma::Transfer<B, C, Self>
                 where
-                    Self: dma::Target<C>,
+                    Self: dma::OnChannel<C>,
                     B: dma::WriteBuffer<Word = u8> + 'static,
                     C: dma::Channel,
                 {
@@ -398,7 +394,7 @@ macro_rules! hal {
                     mut channel: C
                 ) -> dma::Transfer<B, C, Self>
                 where
-                    Self: dma::Target<C>,
+                    Self: dma::OnChannel<C>,
                     B: dma::ReadBuffer<Word = u8> + 'static,
                     C: dma::Channel,
                 {
@@ -409,6 +405,44 @@ macro_rules! hal {
                     dma::Transfer::start_read(buffer, channel, self)
                 }
             }
+
+            #[cfg(feature = "stm32f303")]
+            impl dma::Target for Rx<$USARTX> {
+                fn enable_dma(&mut self) {
+                    // NOTE(unsafe) critical section prevents races
+                    interrupt::free(|_| unsafe {
+                        let cr3 = &(*$USARTX::ptr()).cr3;
+                        cr3.modify(|_, w| w.dmar().enabled());
+                    });
+                }
+
+                fn disable_dma(&mut self) {
+                    // NOTE(unsafe) critical section prevents races
+                    interrupt::free(|_| unsafe {
+                        let cr3 = &(*$USARTX::ptr()).cr3;
+                        cr3.modify(|_, w| w.dmar().disabled());
+                    });
+                }
+            }
+
+            #[cfg(feature = "stm32f303")]
+            impl dma::Target for Tx<$USARTX> {
+                fn enable_dma(&mut self) {
+                    // NOTE(unsafe) critical section prevents races
+                    interrupt::free(|_| unsafe {
+                        let cr3 = &(*$USARTX::ptr()).cr3;
+                        cr3.modify(|_, w| w.dmat().enabled());
+                    });
+                }
+
+                fn disable_dma(&mut self) {
+                    // NOTE(unsafe) critical section prevents races
+                    interrupt::free(|_| unsafe {
+                        let cr3 = &(*$USARTX::ptr()).cr3;
+                        cr3.modify(|_, w| w.dmat().disabled());
+                    });
+                }
+            }
         )+
     }
 }