From eaddb819341dc18547eaefbd8a7c0c488b3a13fc Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Sun, 5 Apr 2020 17:57:56 +0200 Subject: [PATCH 01/16] Add initial DMA support This only includes support for one-shot DMA transfers on STM32f303 MCUs for now. --- src/dma.rs | 473 +++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 + src/prelude.rs | 1 + 3 files changed, 476 insertions(+) create mode 100644 src/dma.rs diff --git a/src/dma.rs b/src/dma.rs new file mode 100644 index 000000000..156242165 --- /dev/null +++ b/src/dma.rs @@ -0,0 +1,473 @@ +//! Direct memory access (DMA) controller + +// Currently DMA is only supported for STM32F303 MCUs. +// Remove these `allow`s once all models have support. +#![cfg_attr(not(feature = "stm32f303"), allow(unused_imports, unused_macros))] + +use crate::{ + rcc::AHB, + serial, + stm32::{self, dma1::ch::cr}, +}; +use cast::u16; +use core::{ + pin::Pin, + sync::atomic::{self, Ordering}, +}; + +/// Extension trait to split a DMA peripheral into independent channels +pub trait DmaExt { + /// The type to split the DMA into + type Channels; + + /// Split the DMA into independent channels + fn split(self, ahb: &mut AHB) -> Self::Channels; +} + +/// An in-progress one-shot DMA transfer +pub struct Transfer { + // This is always a `Some` outside of `drop`. + inner: Option>, +} + +impl Transfer { + /// Start a DMA transfer. + /// + /// # Safety + /// + /// Callers must ensure that the DMA channel is configured correctly for + /// the given target and buffer. + pub unsafe fn start(buffer: Pin, mut channel: C, target: T) -> Self + where + B: 'static, + T: Target, + { + assert!(!channel.is_enabled()); + + atomic::compiler_fence(Ordering::Release); + + channel.enable(); + + Self { + inner: Some(TransferInner { + buffer, + channel, + target, + }), + } + } + + /// Is this transfer complete? + pub fn is_complete(&self) -> bool { + let inner = self.inner.as_ref().unwrap(); + inner.channel.event_occured(Event::TransferComplete) + } + + /// Stop this transfer and return ownership over its parts + pub fn stop(mut self) -> (Pin, C, T) { + let mut inner = self.inner.take().unwrap(); + inner.stop(); + + (inner.buffer, inner.channel, inner.target) + } + + /// Block until this transfer is done and return ownership over its parts + pub fn wait(self) -> (Pin, C, T) { + while !self.is_complete() {} + + self.stop() + } +} + +impl Drop for Transfer { + fn drop(&mut self) { + if let Some(inner) = self.inner.as_mut() { + inner.stop(); + } + } +} + +/// This only exists so we can implement `Drop` for `Transfer`. +struct TransferInner { + buffer: Pin, + channel: C, + target: T, +} + +impl TransferInner { + /// Stop this transfer + fn stop(&mut self) { + self.channel.disable(); + atomic::compiler_fence(Ordering::Acquire); + } +} + +/// DMA address increment mode +pub enum Increment { + /// Enable increment + Enable, + /// Disable increment + Disable, +} + +impl From for cr::PINC_A { + fn from(inc: Increment) -> Self { + match inc { + Increment::Enable => cr::PINC_A::ENABLED, + Increment::Disable => cr::PINC_A::DISABLED, + } + } +} + +/// DMA word size +pub enum WordSize { + /// 8 bits + Bits8, + /// 16 bits + Bits16, + /// 32 bits + Bits32, +} + +impl From for cr::PSIZE_A { + fn from(size: WordSize) -> Self { + match size { + WordSize::Bits8 => cr::PSIZE_A::BITS8, + WordSize::Bits16 => cr::PSIZE_A::BITS16, + WordSize::Bits32 => cr::PSIZE_A::BITS32, + } + } +} + +/// Channel priority level +pub enum Priority { + /// Low + Low, + /// Medium + Medium, + /// High + High, + /// Very high + VeryHigh, +} + +impl From for cr::PL_A { + fn from(prio: Priority) -> Self { + match prio { + Priority::Low => cr::PL_A::LOW, + Priority::Medium => cr::PL_A::MEDIUM, + Priority::High => cr::PL_A::HIGH, + Priority::VeryHigh => cr::PL_A::VERYHIGH, + } + } +} + +/// DMA transfer direction +pub enum Direction { + /// From memory to peripheral + FromMemory, + /// From peripheral to memory + FromPeripheral, +} + +impl From for cr::DIR_A { + fn from(dir: Direction) -> Self { + match dir { + Direction::FromMemory => cr::DIR_A::FROMMEMORY, + Direction::FromPeripheral => cr::DIR_A::FROMPERIPHERAL, + } + } +} + +/// DMA events +pub enum Event { + /// First half of a transfer is done + HalfTransfer, + /// Transfer is complete + TransferComplete, + /// A transfer error occurred + TransferError, + /// Any of the above events occurred + Any, +} + +/// Trait implemented by all DMA channels +pub trait Channel: private::Channel { + /// Is the interrupt flag for the given event set? + fn event_occured(&self, event: Event) -> bool; + /// Clear the interrupt flag for the given event + fn clear_event(&mut self, event: Event); + + /// Reset the control registers of this channel. + /// This stops any ongoing transfers. + fn reset(&mut self) { + self.ch().cr.reset(); + self.ch().ndtr.reset(); + self.ch().par.reset(); + self.ch().mar.reset(); + self.clear_event(Event::Any); + } + + /// Set the base address of the peripheral data register from/to which the + /// data will be read/written. + /// + /// Only call this method on disabled channels. + /// + /// # Panics + /// + /// Panics if this channel is enabled. + fn set_peripheral_address(&mut self, address: u32, inc: Increment) { + assert!(!self.is_enabled()); + + self.ch().par.write(|w| w.pa().bits(address)); + self.ch().cr.modify(|_, w| w.pinc().variant(inc.into())); + } + + /// Set the base address of the memory area from/to which + /// the data will be read/written. + /// + /// Only call this method on disabled channels. + /// + /// # Panics + /// + /// Panics if this channel is enabled. + fn set_memory_address(&mut self, address: u32, inc: Increment) { + assert!(!self.is_enabled()); + + self.ch().mar.write(|w| w.ma().bits(address)); + self.ch().cr.modify(|_, w| w.minc().variant(inc.into())); + } + + /// Set the number of words to transfer. + /// + /// Only call this method on disabled channels. + /// + /// # Panics + /// + /// Panics if this channel is enabled. + fn set_transfer_length(&mut self, len: usize) { + assert!(!self.is_enabled()); + + let len = u16(len).expect("DMA transfer length too large"); + self.ch().ndtr.write(|w| w.ndt().bits(len)); + } + + /// Set the word size + fn set_word_size(&mut self, size: WordSize) { + let psize = size.into(); + self.ch().cr.modify(|_, w| { + w.psize().variant(psize); + w.msize().variant(psize) + }); + } + + /// Set the priority level of this channel + fn set_priority_level(&mut self, priority: Priority) { + let pl = priority.into(); + self.ch().cr.modify(|_, w| w.pl().variant(pl)); + } + + /// Set the transfer direction + fn set_direction(&mut self, direction: Direction) { + let dir = direction.into(); + self.ch().cr.modify(|_, w| w.dir().variant(dir)); + } + + /// Enable the interrupt for the given event + fn listen(&mut self, event: Event) { + use Event::*; + match event { + HalfTransfer => self.ch().cr.modify(|_, w| w.htie().enabled()), + TransferComplete => self.ch().cr.modify(|_, w| w.tcie().enabled()), + TransferError => self.ch().cr.modify(|_, w| w.teie().enabled()), + Any => self.ch().cr.modify(|_, w| { + w.htie().enabled(); + w.tcie().enabled(); + w.teie().enabled() + }), + } + } + + /// Disable the interrupt for the given event + fn unlisten(&mut self, event: Event) { + use Event::*; + match event { + HalfTransfer => self.ch().cr.modify(|_, w| w.htie().disabled()), + TransferComplete => self.ch().cr.modify(|_, w| w.tcie().disabled()), + TransferError => self.ch().cr.modify(|_, w| w.teie().disabled()), + Any => self.ch().cr.modify(|_, w| { + w.htie().disabled(); + w.tcie().disabled(); + w.teie().disabled() + }), + } + } + + /// Start a transfer + fn enable(&mut self) { + self.clear_event(Event::Any); + self.ch().cr.modify(|_, w| w.en().enabled()); + } + + /// Stop the current transfer + fn disable(&mut self) { + self.ch().cr.modify(|_, w| w.en().disabled()); + } + + /// Is there a transfer in progress on this channel? + fn is_enabled(&self) -> bool { + self.ch().cr.read().en().is_enabled() + } +} + +mod private { + use crate::stm32; + + /// Channel methods private to this module + pub trait Channel { + /// Return the register block for this channel + fn ch(&self) -> &stm32::dma1::CH; + } +} + +macro_rules! dma { + ( + $DMAx:ident, $dmax:ident, $dmaxen:ident, + channels: { + $( $Ci:ident: ( + $chi:ident, + $htifi:ident, $tcifi:ident, $teifi:ident, $gifi:ident, + $chtifi:ident, $ctcifi:ident, $cteifi:ident, $cgifi:ident + ), )+ + }, + ) => { + pub mod $dmax { + use super::*; + use crate::stm32::$DMAx; + + impl DmaExt for $DMAx { + type Channels = Channels; + + fn split(self, ahb: &mut AHB) -> Channels { + ahb.enr().modify(|_, w| w.$dmaxen().set_bit()); + + let mut channels = Channels { + $( $chi: $Ci { _0: () }, )+ + }; + + channels.reset(); + channels + } + } + + /// DMA channels + pub struct Channels { + $( pub $chi: $Ci, )+ + } + + impl Channels { + /// Reset the control registers of all channels. + /// This stops any ongoing transfers. + fn reset(&mut self) { + $( self.$chi.reset(); )+ + } + } + + $( + /// Singleton that represents a DMA channel + pub struct $Ci { + _0: (), + } + + impl private::Channel for $Ci { + fn ch(&self) -> &stm32::dma1::CH { + // NOTE(unsafe) $Ci grants exclusive access to this register + unsafe { &(*$DMAx::ptr()).$chi } + } + } + + impl Channel for $Ci { + fn event_occured(&self, event: Event) -> bool { + use Event::*; + + // NOTE(unsafe) atomic read + let flags = unsafe { (*$DMAx::ptr()).isr.read() }; + match event { + HalfTransfer => flags.$htifi().bit_is_set(), + TransferComplete => flags.$tcifi().bit_is_set(), + TransferError => flags.$teifi().bit_is_set(), + Any => flags.$gifi().bit_is_set(), + } + } + + fn clear_event(&mut self, event: Event) { + use Event::*; + + // NOTE(unsafe) atomic write to a stateless register + unsafe { + &(*$DMAx::ptr()).ifcr.write(|w| match event { + HalfTransfer => w.$chtifi().set_bit(), + TransferComplete => w.$ctcifi().set_bit(), + TransferError => w.$cteifi().set_bit(), + Any => w.$cgifi().set_bit(), + }); + } + } + } + )+ + } + }; +} + +#[cfg(feature = "stm32f303")] +dma!( + DMA1, dma1, dma1en, + channels: { + C1: (ch1, htif1, tcif1, teif1, gif1, chtif1, ctcif1, cteif1, cgif1), + C2: (ch2, htif2, tcif2, teif2, gif2, chtif2, ctcif2, cteif2, cgif2), + C3: (ch3, htif3, tcif3, teif3, gif3, chtif3, ctcif3, cteif3, cgif3), + C4: (ch4, htif4, tcif4, teif4, gif4, chtif4, ctcif4, cteif4, cgif4), + C5: (ch5, htif5, tcif5, teif5, gif5, chtif5, ctcif5, cteif5, cgif5), + C6: (ch6, htif6, tcif6, teif6, gif6, chtif6, ctcif6, cteif6, cgif6), + C7: (ch7, htif7, tcif7, teif7, gif7, chtif7, ctcif7, cteif7, cgif7), + }, +); + +#[cfg(any( + feature = "stm32f303xb", + feature = "stm32f303xc", + feature = "stm32f303xd", + feature = "stm32f303xe" +))] +dma!( + DMA2, dma2, dma2en, + channels: { + C1: (ch1, htif1, tcif1, teif1, gif1, chtif1, ctcif1, cteif1, cgif1), + C2: (ch2, htif2, tcif2, teif2, gif2, chtif2, ctcif2, cteif2, cgif2), + C3: (ch3, htif3, tcif3, teif3, gif3, chtif3, ctcif3, cteif3, cgif3), + C4: (ch4, htif4, tcif4, teif4, gif4, chtif4, ctcif4, cteif4, cgif4), + C5: (ch5, htif5, tcif5, teif5, gif5, chtif5, ctcif5, cteif5, cgif5), + }, +); + +/// Marker trait for DMA targets and their channels +pub unsafe trait Target {} + +macro_rules! targets { + ( + $dma:ident, + $( $target:ty => $C:ident, )+ + ) => { + $( unsafe impl Target<$dma::$C> for $target {} )+ + }; +} + +#[cfg(feature = "stm32f303")] +targets!(dma1, + serial::Rx => C5, + serial::Tx => C4, + serial::Rx => C6, + serial::Tx => C7, + serial::Rx => C3, + serial::Tx => C2, +); diff --git a/src/lib.rs b/src/lib.rs index eedbd4b17..0f5fb02ea 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -123,6 +123,8 @@ pub use crate::pac::interrupt; #[cfg(feature = "device-selected")] pub mod delay; #[cfg(feature = "device-selected")] +pub mod dma; +#[cfg(feature = "device-selected")] pub mod flash; #[cfg(feature = "device-selected")] pub mod gpio; diff --git a/src/prelude.rs b/src/prelude.rs index 62339efc6..1b4122750 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -1,5 +1,6 @@ //! Prelude +pub use crate::dma::DmaExt as _stm32f3xx_hal_dma_DmaExt; pub use crate::flash::FlashExt as _stm32f3xx_hal_flash_FlashExt; pub use crate::gpio::GpioExt as _stm32f3xx_hal_gpio_GpioExt; pub use crate::hal::prelude::*; From e44b8538f2b4d484b522f64d68b4697ebc219d49 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Sun, 5 Apr 2020 18:05:57 +0200 Subject: [PATCH 02/16] Add DMA support for serial receive and transmit --- Cargo.toml | 1 + src/serial.rs | 187 +++++++++++++++++++++++++++++++------------------- 2 files changed, 116 insertions(+), 72 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9127e8fea..5acc43c06 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ targets = ["thumbv7em-none-eabihf"] travis-ci = { repository = "stm32-rs/stm32f3xx-hal" } [dependencies] +as-slice = "0.1" cortex-m = "0.6" cortex-m-rt = "0.6" embedded-hal = "0.2" diff --git a/src/serial.rs b/src/serial.rs index 8e93e25c2..d0a2c9cea 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -1,28 +1,15 @@ //! Serial -use core::convert::Infallible; -use core::marker::PhantomData; -use core::ptr; - -use crate::hal::blocking::serial::write::Default; -use crate::hal::serial; -use crate::pac::{USART1, USART2, USART3}; +use crate::{ + gpio::{gpioa, gpiob, gpioc, AF7}, + hal::{blocking, serial}, + pac::{USART1, USART2, USART3}, + rcc::{Clocks, APB1, APB2}, + time::Bps, +}; +use core::{convert::Infallible, marker::PhantomData, ptr}; use nb; -use crate::gpio::gpioa::{PA10, PA2, PA3, PA9}; -#[cfg(any( - feature = "stm32f301", - feature = "stm32f318", - feature = "stm32f302", - feature = "stm32f303", - feature = "stm32f334", - feature = "stm32f328", - feature = "stm32f358", - feature = "stm32f398" -))] -use crate::gpio::gpiob::PB11; -use crate::gpio::gpiob::{PB10, PB6, PB7}; -use crate::gpio::gpioc::{PC10, PC11, PC4, PC5}; #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -36,24 +23,20 @@ use crate::gpio::gpioc::{PC10, PC11, PC4, PC5}; feature = "stm32f358", feature = "stm32f398" ))] -use crate::gpio::gpiod::{PD5, PD6, PD8, PD9}; -#[cfg(any( - feature = "stm32f302", - feature = "stm32f303xb", - feature = "stm32f303xc", - feature = "stm32f303xd", - feature = "stm32f303xe", - feature = "stm32f373", - feature = "stm32f378", - feature = "stm32f328", - feature = "stm32f358", - feature = "stm32f398" -))] -use crate::gpio::gpioe::{PE0, PE1, PE15}; +use crate::gpio::{gpiod, gpioe}; + +#[cfg(feature = "stm32f303")] +mod dma_imports { + pub use crate::dma; + pub use as_slice::{AsMutSlice, AsSlice}; + pub use core::{ + ops::{Deref, DerefMut}, + pin::Pin, + }; +} -use crate::gpio::AF7; -use crate::rcc::{Clocks, APB1, APB2}; -use crate::time::Bps; +#[cfg(feature = "stm32f303")] +use dma_imports::*; /// Interrupt event pub enum Event { @@ -85,9 +68,9 @@ pub unsafe trait TxPin {} /// RX pin - DO NOT IMPLEMENT THIS TRAIT pub unsafe trait RxPin {} -unsafe impl TxPin for PA9 {} -unsafe impl TxPin for PB6 {} -unsafe impl TxPin for PC4 {} +unsafe impl TxPin for gpioa::PA9 {} +unsafe impl TxPin for gpiob::PB6 {} +unsafe impl TxPin for gpioc::PC4 {} #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -100,11 +83,11 @@ unsafe impl TxPin for PC4 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl TxPin for PE0 {} +unsafe impl TxPin for gpioe::PE0 {} -unsafe impl RxPin for PA10 {} -unsafe impl RxPin for PB7 {} -unsafe impl RxPin for PC5 {} +unsafe impl RxPin for gpioa::PA10 {} +unsafe impl RxPin for gpiob::PB7 {} +unsafe impl RxPin for gpioc::PC5 {} #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -117,11 +100,11 @@ unsafe impl RxPin for PC5 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl RxPin for PE1 {} +unsafe impl RxPin for gpioe::PE1 {} -unsafe impl TxPin for PA2 {} -// unsafe impl TxPin for PA14 {} -// unsafe impl TxPin for PB3 {} +unsafe impl TxPin for gpioa::PA2 {} +// unsafe impl TxPin for gpioa::PA14 {} +// unsafe impl TxPin for gpiob::PB3 {} #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -135,11 +118,11 @@ unsafe impl TxPin for PA2 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl TxPin for PD5 {} +unsafe impl TxPin for gpiod::PD5 {} -unsafe impl RxPin for PA3 {} -// unsafe impl RxPin for PA15 {} -// unsafe impl RxPin for PB4 {} +unsafe impl RxPin for gpioa::PA3 {} +// unsafe impl RxPin for gpioa::PA15 {} +// unsafe impl RxPin for gpiob::PB4 {} #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -153,10 +136,10 @@ unsafe impl RxPin for PA3 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl RxPin for PD6 {} +unsafe impl RxPin for gpiod::PD6 {} -unsafe impl TxPin for PB10 {} -unsafe impl TxPin for PC10 {} +unsafe impl TxPin for gpiob::PB10 {} +unsafe impl TxPin for gpioc::PC10 {} #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -170,7 +153,7 @@ unsafe impl TxPin for PC10 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl TxPin for PD8 {} +unsafe impl TxPin for gpiod::PD8 {} #[cfg(any( feature = "stm32f301", @@ -182,8 +165,8 @@ unsafe impl TxPin for PD8 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl RxPin for PB11 {} -unsafe impl RxPin for PC11 {} +unsafe impl RxPin for gpiob::PB11 {} +unsafe impl RxPin for gpioc::PC11 {} #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -197,7 +180,7 @@ unsafe impl RxPin for PC11 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl RxPin for PD9 {} +unsafe impl RxPin for gpiod::PD9 {} #[cfg(any( feature = "stm32f302", feature = "stm32f303xb", @@ -210,7 +193,7 @@ unsafe impl RxPin for PD9 {} feature = "stm32f358", feature = "stm32f398" ))] -unsafe impl RxPin for PE15 {} +unsafe impl RxPin for gpioe::PE15 {} /// Serial abstraction pub struct Serial { @@ -251,21 +234,21 @@ macro_rules! hal { apb.rstr().modify(|_, w| w.$usartXrst().set_bit()); apb.rstr().modify(|_, w| w.$usartXrst().clear_bit()); - // disable hardware flow control - // TODO enable DMA - // usart.cr3.write(|w| w.rtse().clear_bit().ctse().clear_bit()); - let brr = clocks.$pclkX().0 / baud_rate.0; assert!(brr >= 16, "impossible baud rate"); // NOTE(write): uses all bits of this register. usart.brr.write(|w| unsafe { w.bits(brr) }); - // UE: enable USART - // RE: enable receiver - // TE: enable transceiver - usart - .cr1 - .modify(|_, w| w.ue().set_bit().re().set_bit().te().set_bit()); + 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 + w.te().enabled() // enable transmitter + }); Serial { usart, pins } } @@ -380,7 +363,67 @@ macro_rules! hal { } } - impl Default for Tx<$USARTX> {} + impl blocking::serial::write::Default for Tx<$USARTX> {} + + #[cfg(feature = "stm32f303")] + impl Rx<$USARTX> { + /// Fill the buffer with received data using DMA. + pub fn read_all( + self, + mut buffer: Pin, + mut channel: C + ) -> dma::Transfer + where + Self: dma::Target, + B: DerefMut + 'static, + B::Target: AsMutSlice + Unpin, + C: dma::Channel, + { + // NOTE(unsafe) taking the address of a register + let pa = unsafe { &(*$USARTX::ptr()).rdr } as *const _ as u32; + channel.set_peripheral_address(pa, dma::Increment::Disable); + + let slice = buffer.as_mut_slice(); + let (ma, len) = (slice.as_mut_ptr() as u32, slice.len()); + channel.set_memory_address(ma, dma::Increment::Enable); + channel.set_transfer_length(len); + channel.set_word_size(dma::WordSize::Bits8); + + channel.set_direction(dma::Direction::FromPeripheral); + + unsafe { dma::Transfer::start(buffer, channel, self) } + } + } + + #[cfg(feature = "stm32f303")] + impl Tx<$USARTX> { + /// Transmit all data in the buffer using DMA. + pub fn write_all( + self, + buffer: Pin, + mut channel: C + ) -> dma::Transfer + where + Self: dma::Target, + B: Deref + 'static, + B::Target: AsSlice, + C: dma::Channel, + { + // NOTE(unsafe) taking the address of a register + let pa = unsafe { &(*$USARTX::ptr()).tdr } as *const _ as u32; + channel.set_peripheral_address(pa, dma::Increment::Disable); + + let slice = buffer.as_slice(); + let (ma, len) = (slice.as_ptr() as u32, slice.len()); + channel.set_memory_address(ma, dma::Increment::Enable); + channel.set_transfer_length(len); + channel.set_word_size(dma::WordSize::Bits8); + + channel.set_direction(dma::Direction::FromMemory); + + unsafe { dma::Transfer::start(buffer, channel, self) } + } + } )+ } } From 8cc42a1504bdfc6a34de8856ebbe0b1c6ba5075d Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Sun, 5 Apr 2020 18:06:40 +0200 Subject: [PATCH 03/16] Add an example for serial communication using DMA --- Cargo.toml | 4 +++ examples/serial_dma.rs | 69 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 examples/serial_dma.rs diff --git a/Cargo.toml b/Cargo.toml index 5acc43c06..c3c715c7f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -103,3 +103,7 @@ required-features = ["rt", "stm32f303xc", "stm32-usbd"] [[example]] name = "spi" required-features = ["stm32f303"] + +[[example]] +name = "serial_dma" +required-features = ["stm32f303"] diff --git a/examples/serial_dma.rs b/examples/serial_dma.rs new file mode 100644 index 000000000..fdfb7117d --- /dev/null +++ b/examples/serial_dma.rs @@ -0,0 +1,69 @@ +//! Example of transmitting data over serial interface using DMA. +//! For this to work, the PA9 and PA10 pins must be connected. +//! Target board: STM32F3DISCOVERY + +#![no_std] +#![no_main] + +use panic_semihosting as _; + +use core::pin::Pin; +use cortex_m::singleton; +use cortex_m_rt::entry; +use stm32f3xx_hal::{prelude::*, serial::Serial, stm32}; + +#[entry] +fn main() -> ! { + let dp = stm32::Peripherals::take().unwrap(); + + let mut flash = dp.FLASH.constrain(); + let mut rcc = dp.RCC.constrain(); + let clocks = rcc.cfgr.freeze(&mut flash.acr); + + let mut gpioa = dp.GPIOA.split(&mut rcc.ahb); + + let pins = ( + gpioa.pa9.into_af7(&mut gpioa.moder, &mut gpioa.afrh), + gpioa.pa10.into_af7(&mut gpioa.moder, &mut gpioa.afrh), + ); + let serial = Serial::usart1(dp.USART1, pins, 9600.bps(), clocks, &mut rcc.apb2); + let (tx, rx) = serial.split(); + + let dma1 = dp.DMA1.split(&mut rcc.ahb); + + // the data we are going to send over serial + let tx_buf = singleton!(: [u8; 9] = *b"hello DMA").unwrap(); + // the buffer we are going to receive the transmitted data in + let rx_buf = singleton!(: [u8; 9] = [0; 9]).unwrap(); + + // DMA channel selection depends on the peripheral: + // - USART1: TX = 4, RX = 5 + // - USART2: TX = 6, RX = 7 + // - USART3: TX = 3, RX = 2 + let (tx_channel, rx_channel) = (dma1.ch4, dma1.ch5); + + // start separate DMAs for sending and receiving the data + let sending = tx.write_all(Pin::new(tx_buf), tx_channel); + let receiving = rx.read_all(Pin::new(rx_buf), rx_channel); + + // block until all data was transmitted and received + let (mut tx_buf, tx_channel, tx) = sending.wait(); + let (rx_buf, rx_channel, rx) = receiving.wait(); + + assert_eq!(tx_buf, rx_buf); + + // After a transfer is finished its parts can be re-used for another one. + tx_buf.copy_from_slice(b"hi again!"); + + let sending = tx.write_all(tx_buf, tx_channel); + let receiving = rx.read_all(rx_buf, rx_channel); + + let (tx_buf, ..) = sending.wait(); + let (rx_buf, ..) = receiving.wait(); + + assert_eq!(tx_buf, rx_buf); + + loop { + continue; + } +} From 27516edd68888ae1684922908c00500241a82691 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Sun, 5 Apr 2020 18:37:35 +0200 Subject: [PATCH 04/16] Fix imports in serial module --- src/serial.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/serial.rs b/src/serial.rs index d0a2c9cea..f34ce2582 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -23,7 +23,20 @@ use nb; feature = "stm32f358", feature = "stm32f398" ))] -use crate::gpio::{gpiod, gpioe}; +use crate::gpio::gpiod; +#[cfg(any( + feature = "stm32f302", + feature = "stm32f303xb", + feature = "stm32f303xc", + feature = "stm32f303xd", + feature = "stm32f303xe", + feature = "stm32f373", + feature = "stm32f378", + feature = "stm32f328", + feature = "stm32f358", + feature = "stm32f398" +))] +use crate::gpio::gpioe; #[cfg(feature = "stm32f303")] mod dma_imports { From ffb4eff0923c95a897ab87ae948f98c63d6f5173 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Sun, 5 Apr 2020 18:42:10 +0200 Subject: [PATCH 05/16] Add new features to CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4162be2fb..dc690db9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Added + +- Support for safe one-shot DMA transfers ([#86](https://github.com/stm32-rs/stm32f3xx-hal/pull/86)) +- DMA support for serial reception and transmission ([#86](https://github.com/stm32-rs/stm32f3xx-hal/pull/86)) + ### Fixed - `PLL` was calculated wrong for devices, which do not divide `HSI` ([#67](https://github.com/stm32-rs/stm32f3xx-hal/pull/67)) From e9d7a38c4f446368d8a0d778b0673c287be49038 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Tue, 7 Apr 2020 09:31:14 +0200 Subject: [PATCH 06/16] Rename serial read_all to read_exact This better reflects what that function is doing, namely reading as many bytes as there is space in the given buffer. --- examples/serial_dma.rs | 4 ++-- src/serial.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/serial_dma.rs b/examples/serial_dma.rs index fdfb7117d..4b7d0e8d9 100644 --- a/examples/serial_dma.rs +++ b/examples/serial_dma.rs @@ -44,7 +44,7 @@ fn main() -> ! { // start separate DMAs for sending and receiving the data let sending = tx.write_all(Pin::new(tx_buf), tx_channel); - let receiving = rx.read_all(Pin::new(rx_buf), rx_channel); + let receiving = rx.read_exact(Pin::new(rx_buf), rx_channel); // block until all data was transmitted and received let (mut tx_buf, tx_channel, tx) = sending.wait(); @@ -56,7 +56,7 @@ fn main() -> ! { tx_buf.copy_from_slice(b"hi again!"); let sending = tx.write_all(tx_buf, tx_channel); - let receiving = rx.read_all(rx_buf, rx_channel); + let receiving = rx.read_exact(rx_buf, rx_channel); let (tx_buf, ..) = sending.wait(); let (rx_buf, ..) = receiving.wait(); diff --git a/src/serial.rs b/src/serial.rs index f34ce2582..8f4ba58bc 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -381,7 +381,7 @@ macro_rules! hal { #[cfg(feature = "stm32f303")] impl Rx<$USARTX> { /// Fill the buffer with received data using DMA. - pub fn read_all( + pub fn read_exact( self, mut buffer: Pin, mut channel: C From ae2c57c03d1b8c05ad9132b1ef1c266f9cf19293 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Tue, 7 Apr 2020 09:54:49 +0200 Subject: [PATCH 07/16] Replace use of Pin with StableDeref bounds Using Pin for DMA buffers does not provide the necessary safety. See https://github.com/rust-embedded/embedonomicon/issues/64 --- Cargo.toml | 10 +++++++--- examples/serial_dma.rs | 7 +++---- src/dma.rs | 16 +++++++--------- src/serial.rs | 16 +++++++--------- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c3c715c7f..c3b27e7a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,17 +31,21 @@ version = "0.2" features = ["const-fn"] [dependencies.cast] -default-features = false version = "0.2" - -[dependencies.void] default-features = false + +[dependencies.stable_deref_trait] version = "1" +default-features = false [dependencies.stm32-usbd] version = "0.5" optional = true +[dependencies.void] +version = "1" +default-features = false + [dev-dependencies] panic-semihosting = "0.5" usb-device = "0.2" diff --git a/examples/serial_dma.rs b/examples/serial_dma.rs index 4b7d0e8d9..7a10058e3 100644 --- a/examples/serial_dma.rs +++ b/examples/serial_dma.rs @@ -7,7 +7,6 @@ use panic_semihosting as _; -use core::pin::Pin; use cortex_m::singleton; use cortex_m_rt::entry; use stm32f3xx_hal::{prelude::*, serial::Serial, stm32}; @@ -43,11 +42,11 @@ fn main() -> ! { let (tx_channel, rx_channel) = (dma1.ch4, dma1.ch5); // start separate DMAs for sending and receiving the data - let sending = tx.write_all(Pin::new(tx_buf), tx_channel); - let receiving = rx.read_exact(Pin::new(rx_buf), rx_channel); + let sending = tx.write_all(tx_buf, tx_channel); + let receiving = rx.read_exact(rx_buf, rx_channel); // block until all data was transmitted and received - let (mut tx_buf, tx_channel, tx) = sending.wait(); + let (tx_buf, tx_channel, tx) = sending.wait(); let (rx_buf, rx_channel, rx) = receiving.wait(); assert_eq!(tx_buf, rx_buf); diff --git a/src/dma.rs b/src/dma.rs index 156242165..0fb3ad3ab 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -10,10 +10,8 @@ use crate::{ stm32::{self, dma1::ch::cr}, }; use cast::u16; -use core::{ - pin::Pin, - sync::atomic::{self, Ordering}, -}; +use core::sync::atomic::{self, Ordering}; +use stable_deref_trait::StableDeref; /// Extension trait to split a DMA peripheral into independent channels pub trait DmaExt { @@ -37,9 +35,9 @@ impl Transfer { /// /// Callers must ensure that the DMA channel is configured correctly for /// the given target and buffer. - pub unsafe fn start(buffer: Pin, mut channel: C, target: T) -> Self + pub unsafe fn start(buffer: B, mut channel: C, target: T) -> Self where - B: 'static, + B: StableDeref + 'static, T: Target, { assert!(!channel.is_enabled()); @@ -64,7 +62,7 @@ impl Transfer { } /// Stop this transfer and return ownership over its parts - pub fn stop(mut self) -> (Pin, C, T) { + pub fn stop(mut self) -> (B, C, T) { let mut inner = self.inner.take().unwrap(); inner.stop(); @@ -72,7 +70,7 @@ impl Transfer { } /// Block until this transfer is done and return ownership over its parts - pub fn wait(self) -> (Pin, C, T) { + pub fn wait(self) -> (B, C, T) { while !self.is_complete() {} self.stop() @@ -89,7 +87,7 @@ impl Drop for Transfer { /// This only exists so we can implement `Drop` for `Transfer`. struct TransferInner { - buffer: Pin, + buffer: B, channel: C, target: T, } diff --git a/src/serial.rs b/src/serial.rs index 8f4ba58bc..e882af803 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -42,10 +42,8 @@ use crate::gpio::gpioe; mod dma_imports { pub use crate::dma; pub use as_slice::{AsMutSlice, AsSlice}; - pub use core::{ - ops::{Deref, DerefMut}, - pin::Pin, - }; + pub use core::ops::{Deref, DerefMut}; + pub use stable_deref_trait::StableDeref; } #[cfg(feature = "stm32f303")] @@ -383,13 +381,13 @@ macro_rules! hal { /// Fill the buffer with received data using DMA. pub fn read_exact( self, - mut buffer: Pin, + mut buffer: B, mut channel: C ) -> dma::Transfer where Self: dma::Target, - B: DerefMut + 'static, - B::Target: AsMutSlice + Unpin, + B: DerefMut + StableDeref + 'static, + B::Target: AsMutSlice, C: dma::Channel, { // NOTE(unsafe) taking the address of a register @@ -413,12 +411,12 @@ macro_rules! hal { /// Transmit all data in the buffer using DMA. pub fn write_all( self, - buffer: Pin, + buffer: B, mut channel: C ) -> dma::Transfer where Self: dma::Target, - B: Deref + 'static, + B: Deref + StableDeref + 'static, B::Target: AsSlice, C: dma::Channel, { From d48a7b0b3864d95603ffa962bd11cbc7a8ae79a1 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Sat, 9 May 2020 15:05:09 +0200 Subject: [PATCH 08/16] Use safe DMA buffer traits This commit migrates the current DMA implementation to the safe DMA buffer traits proposed by the "DMA API Documentation" focus project. See: https://github.com/ra-kete/dma-poc/blob/master/README.md --- Cargo.toml | 1 - src/dma.rs | 286 +++++++++++++++++++++++++++++++++++++++++++++----- src/serial.rs | 38 ++----- 3 files changed, 263 insertions(+), 62 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c3b27e7a3..acf993ae2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,6 @@ targets = ["thumbv7em-none-eabihf"] travis-ci = { repository = "stm32-rs/stm32f3xx-hal" } [dependencies] -as-slice = "0.1" cortex-m = "0.6" cortex-m-rt = "0.6" embedded-hal = "0.2" diff --git a/src/dma.rs b/src/dma.rs index 0fb3ad3ab..0461357e5 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -10,7 +10,11 @@ use crate::{ stm32::{self, dma1::ch::cr}, }; use cast::u16; -use core::sync::atomic::{self, Ordering}; +use core::{ + mem::{self, MaybeUninit}, + ops::{Deref, DerefMut}, + sync::atomic::{self, Ordering}, +}; use stable_deref_trait::StableDeref; /// Extension trait to split a DMA peripheral into independent channels @@ -29,15 +33,46 @@ pub struct Transfer { } impl Transfer { - /// Start a DMA transfer. - /// + /// Start a DMA write transfer. + pub fn start_write(mut buffer: B, mut channel: C, target: T) -> Self + where + B: WriteBuffer + 'static, + T: Target, + { + let (ptr, len) = buffer.write_buffer(); + + channel.set_memory_address(ptr as u32, Increment::Enable); + channel.set_transfer_length(len); + channel.set_word_size::(); + channel.set_direction(Direction::FromPeripheral); + + unsafe { Self::start(buffer, channel, target) } + } + + /// Start a DMA read transfer. + pub fn start_read(buffer: B, mut channel: C, target: T) -> Self + where + B: ReadBuffer + 'static, + T: Target, + { + let (ptr, len) = buffer.read_buffer(); + + channel.set_memory_address(ptr as u32, Increment::Enable); + channel.set_transfer_length(len); + channel.set_word_size::(); + channel.set_direction(Direction::FromMemory); + + unsafe { Self::start(buffer, channel, target) } + } + /// # Safety /// - /// Callers must ensure that the DMA channel is configured correctly for - /// the given target and buffer. - pub unsafe fn start(buffer: B, mut channel: C, target: T) -> Self + /// Callers must ensure that: + /// + /// - 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 where - B: StableDeref + 'static, T: Target, { assert!(!channel.is_enabled()); @@ -100,6 +135,211 @@ impl TransferInner { } } +/// Trait for buffers that can be given to DMA for reading. +/// +/// # Safety +/// +/// The implementing type must be safe to use for DMA reads. This means: +/// +/// - It must be a pointer that references the actual buffer. +/// - The requirements documented on `read_buffer` must be fulfilled. +pub unsafe trait ReadBuffer { + type Word; + + /// Provide a buffer usable for DMA reads. + /// + /// The return value is: + /// + /// - pointer to the start of the buffer + /// - buffer size in words + /// + /// # Safety + /// + /// - This function must always return the same values, if called multiple + /// times. + /// - The memory specified by the returned pointer and size must not be + /// freed as long as `self` is not dropped. + fn read_buffer(&self) -> (*const Self::Word, usize); +} + +/// Trait for buffers that can be given to DMA for writing. +/// +/// # Safety +/// +/// The implementing type must be safe to use for DMA writes. This means: +/// +/// - It must be a pointer that references the actual buffer. +/// - `Target` must be a type that is valid for any possible byte pattern. +/// - The requirements documented on `write_buffer` must be fulfilled. +pub unsafe trait WriteBuffer { + type Word; + + /// Provide a buffer usable for DMA writes. + /// + /// The return value is: + /// + /// - pointer to the start of the buffer + /// - buffer size in words + /// + /// # Safety + /// + /// - This function must always return the same values, if called multiple + /// times. + /// - The memory specified by the returned pointer and size must not be + /// freed as long as `self` is not dropped. + fn write_buffer(&mut self) -> (*mut Self::Word, usize); +} + +// Blanked implementations for common DMA buffer types. + +unsafe impl ReadBuffer for B +where + B: Deref + StableDeref, + T: ReadTarget + ?Sized, +{ + type Word = T::Word; + + fn read_buffer(&self) -> (*const Self::Word, usize) { + self.as_read_buffer() + } +} + +unsafe impl WriteBuffer for B +where + B: DerefMut + StableDeref, + T: WriteTarget + ?Sized, +{ + type Word = T::Word; + + fn write_buffer(&mut self) -> (*mut Self::Word, usize) { + self.as_write_buffer() + } +} + +/// Trait for DMA word types used by the blanket DMA buffer impls. +/// +/// # Safety +/// +/// Types that implement this trait must be valid for every possible byte +/// pattern. This is to ensure that, whatever DMA writes into the buffer, +/// we won't get UB due to invalid values. +pub unsafe trait Word {} + +unsafe impl Word for u8 {} +unsafe impl Word for u16 {} +unsafe impl Word for u32 {} + +/// Trait for `Deref` targets used by the blanket `DmaReadBuffer` impl. +/// +/// This trait exists solely to work around +/// https://github.com/rust-lang/rust/issues/20400. +/// +/// # Safety +/// +/// - `as_read_buffer` must adhere to the safety requirements +/// documented for `DmaReadBuffer::dma_read_buffer`. +pub unsafe trait ReadTarget { + type Word: Word; + + fn as_read_buffer(&self) -> (*const Self::Word, usize) { + let ptr = self as *const _ as *const Self::Word; + let len = mem::size_of_val(self) / mem::size_of::(); + (ptr, len) + } +} + +/// Trait for `DerefMut` targets used by the blanket `DmaWriteBuffer` impl. +/// +/// This trait exists solely to work around +/// https://github.com/rust-lang/rust/issues/20400. +/// +/// # Safety +/// +/// - `as_write_buffer` must adhere to the safety requirements +/// documented for `DmaWriteBuffer::dma_write_buffer`. +pub unsafe trait WriteTarget { + type Word: Word; + + fn as_write_buffer(&mut self) -> (*mut Self::Word, usize) { + let ptr = self as *mut _ as *mut Self::Word; + let len = mem::size_of_val(self) / mem::size_of::(); + (ptr, len) + } +} + +unsafe impl ReadTarget for W { + type Word = W; +} + +unsafe impl WriteTarget for W { + type Word = W; +} + +unsafe impl ReadTarget for [T] { + type Word = T::Word; +} + +unsafe impl WriteTarget for [T] { + type Word = T::Word; +} + +macro_rules! dma_target_array_impls { + ( $( $i:expr, )+ ) => { + $( + unsafe impl ReadTarget for [T; $i] { + type Word = T::Word; + } + + unsafe impl WriteTarget for [T; $i] { + type Word = T::Word; + } + )+ + }; +} + +#[rustfmt::skip] +dma_target_array_impls!( + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, + 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, + 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, + 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, + 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, + 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, + 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, + 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, + 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, + 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, + 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, + 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, + 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, + 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, + 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, + 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, + 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, + 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, + 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, + 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, + 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, + 250, 251, 252, 253, 254, 255, 256, + + 1 << 9, + 1 << 10, + 1 << 11, + 1 << 12, + 1 << 13, + 1 << 14, + 1 << 15, + 1 << 16, +); + +unsafe impl WriteTarget for MaybeUninit { + type Word = T::Word; +} + /// DMA address increment mode pub enum Increment { /// Enable increment @@ -117,26 +357,6 @@ impl From for cr::PINC_A { } } -/// DMA word size -pub enum WordSize { - /// 8 bits - Bits8, - /// 16 bits - Bits16, - /// 32 bits - Bits32, -} - -impl From for cr::PSIZE_A { - fn from(size: WordSize) -> Self { - match size { - WordSize::Bits8 => cr::PSIZE_A::BITS8, - WordSize::Bits16 => cr::PSIZE_A::BITS16, - WordSize::Bits32 => cr::PSIZE_A::BITS32, - } - } -} - /// Channel priority level pub enum Priority { /// Low @@ -251,8 +471,16 @@ pub trait Channel: private::Channel { } /// Set the word size - fn set_word_size(&mut self, size: WordSize) { - let psize = size.into(); + fn set_word_size(&mut self) { + use cr::PSIZE_A::*; + + let psize = match mem::size_of::() { + 1 => BITS8, + 2 => BITS16, + 4 => BITS32, + s => panic!("unsupported word size: {}", s), + }; + self.ch().cr.modify(|_, w| { w.psize().variant(psize); w.msize().variant(psize) diff --git a/src/serial.rs b/src/serial.rs index e882af803..9a925a726 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -39,15 +39,7 @@ use crate::gpio::gpiod; use crate::gpio::gpioe; #[cfg(feature = "stm32f303")] -mod dma_imports { - pub use crate::dma; - pub use as_slice::{AsMutSlice, AsSlice}; - pub use core::ops::{Deref, DerefMut}; - pub use stable_deref_trait::StableDeref; -} - -#[cfg(feature = "stm32f303")] -use dma_imports::*; +pub use crate::dma; /// Interrupt event pub enum Event { @@ -381,28 +373,19 @@ macro_rules! hal { /// Fill the buffer with received data using DMA. pub fn read_exact( self, - mut buffer: B, + buffer: B, mut channel: C ) -> dma::Transfer where Self: dma::Target, - B: DerefMut + StableDeref + 'static, - B::Target: AsMutSlice, + B: dma::WriteBuffer + 'static, C: dma::Channel, { // NOTE(unsafe) taking the address of a register let pa = unsafe { &(*$USARTX::ptr()).rdr } as *const _ as u32; channel.set_peripheral_address(pa, dma::Increment::Disable); - let slice = buffer.as_mut_slice(); - let (ma, len) = (slice.as_mut_ptr() as u32, slice.len()); - channel.set_memory_address(ma, dma::Increment::Enable); - channel.set_transfer_length(len); - channel.set_word_size(dma::WordSize::Bits8); - - channel.set_direction(dma::Direction::FromPeripheral); - - unsafe { dma::Transfer::start(buffer, channel, self) } + dma::Transfer::start_write(buffer, channel, self) } } @@ -416,23 +399,14 @@ macro_rules! hal { ) -> dma::Transfer where Self: dma::Target, - B: Deref + StableDeref + 'static, - B::Target: AsSlice, + B: dma::ReadBuffer + 'static, C: dma::Channel, { // NOTE(unsafe) taking the address of a register let pa = unsafe { &(*$USARTX::ptr()).tdr } as *const _ as u32; channel.set_peripheral_address(pa, dma::Increment::Disable); - let slice = buffer.as_slice(); - let (ma, len) = (slice.as_ptr() as u32, slice.len()); - channel.set_memory_address(ma, dma::Increment::Enable); - channel.set_transfer_length(len); - channel.set_word_size(dma::WordSize::Bits8); - - channel.set_direction(dma::Direction::FromMemory); - - unsafe { dma::Transfer::start(buffer, channel, self) } + dma::Transfer::start_read(buffer, channel, self) } } )+ From 970f5dbffc62e9a12ea70f52e289f03be9093181 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Fri, 17 Jul 2020 13:52:44 +0200 Subject: [PATCH 09/16] dma: Refine safety requirements of buffer traits --- src/dma.rs | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/dma.rs b/src/dma.rs index 0461357e5..74111d234 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -39,7 +39,9 @@ impl Transfer { B: WriteBuffer + 'static, T: Target, { - let (ptr, len) = buffer.write_buffer(); + // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its + // concrete type is unknown here + let (ptr, len) = unsafe { buffer.write_buffer() }; channel.set_memory_address(ptr as u32, Increment::Enable); channel.set_transfer_length(len); @@ -55,7 +57,9 @@ impl Transfer { B: ReadBuffer + 'static, T: Target, { - let (ptr, len) = buffer.read_buffer(); + // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its + // concrete type is unknown here + let (ptr, len) = unsafe { buffer.read_buffer() }; channel.set_memory_address(ptr as u32, Increment::Enable); channel.set_transfer_length(len); @@ -142,7 +146,11 @@ impl TransferInner { /// The implementing type must be safe to use for DMA reads. This means: /// /// - It must be a pointer that references the actual buffer. -/// - The requirements documented on `read_buffer` must be fulfilled. +/// - The following requirements must be fulfilled by `read_buffer`: +/// - The function must always return the same values, if called multiple +/// times. +/// - The memory specified by the returned pointer and size must not be +/// freed as long as `self` is not dropped. pub unsafe trait ReadBuffer { type Word; @@ -155,11 +163,9 @@ pub unsafe trait ReadBuffer { /// /// # Safety /// - /// - This function must always return the same values, if called multiple - /// times. - /// - The memory specified by the returned pointer and size must not be - /// freed as long as `self` is not dropped. - fn read_buffer(&self) -> (*const Self::Word, usize); + /// Once this method has been called, it is unsafe to call any `&mut self` + /// methods on this object as long as the returned value is used (for DMA). + unsafe fn read_buffer(&self) -> (*const Self::Word, usize); } /// Trait for buffers that can be given to DMA for writing. @@ -170,7 +176,11 @@ pub unsafe trait ReadBuffer { /// /// - It must be a pointer that references the actual buffer. /// - `Target` must be a type that is valid for any possible byte pattern. -/// - The requirements documented on `write_buffer` must be fulfilled. +/// - The following requirements must be fulfilled by `write_buffer`: +/// - The function must always return the same values, if called multiple +/// times. +/// - The memory specified by the returned pointer and size must not be +/// freed as long as `self` is not dropped. pub unsafe trait WriteBuffer { type Word; @@ -183,11 +193,9 @@ pub unsafe trait WriteBuffer { /// /// # Safety /// - /// - This function must always return the same values, if called multiple - /// times. - /// - The memory specified by the returned pointer and size must not be - /// freed as long as `self` is not dropped. - fn write_buffer(&mut self) -> (*mut Self::Word, usize); + /// Once this method has been called, it is unsafe to call any `&mut self` + /// methods on this object as long as the returned value is used (for DMA).. + unsafe fn write_buffer(&mut self) -> (*mut Self::Word, usize); } // Blanked implementations for common DMA buffer types. @@ -199,7 +207,7 @@ where { type Word = T::Word; - fn read_buffer(&self) -> (*const Self::Word, usize) { + unsafe fn read_buffer(&self) -> (*const Self::Word, usize) { self.as_read_buffer() } } @@ -211,7 +219,7 @@ where { type Word = T::Word; - fn write_buffer(&mut self) -> (*mut Self::Word, usize) { + unsafe fn write_buffer(&mut self) -> (*mut Self::Word, usize) { self.as_write_buffer() } } From f0b258084888a86f230d20bc2c4eccbc5a6ba453 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Fri, 17 Jul 2020 14:05:14 +0200 Subject: [PATCH 10/16] dma: Improve documentation of panics Also fixes a typo. --- src/dma.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/dma.rs b/src/dma.rs index 74111d234..8f710840b 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -34,6 +34,10 @@ pub struct Transfer { impl Transfer { /// Start a DMA write transfer. + /// + /// # Panics + /// + /// Panics if the buffer is longer than 65535 words. pub fn start_write(mut buffer: B, mut channel: C, target: T) -> Self where B: WriteBuffer + 'static, @@ -42,6 +46,7 @@ impl Transfer { // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its // concrete type is unknown here let (ptr, len) = unsafe { buffer.write_buffer() }; + let len = u16(len).expect("buffer is too large"); channel.set_memory_address(ptr as u32, Increment::Enable); channel.set_transfer_length(len); @@ -52,6 +57,10 @@ impl Transfer { } /// Start a DMA read transfer. + /// + /// # Panics + /// + /// Panics if the buffer is longer than 65535 words. pub fn start_read(buffer: B, mut channel: C, target: T) -> Self where B: ReadBuffer + 'static, @@ -60,6 +69,7 @@ impl Transfer { // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its // concrete type is unknown here let (ptr, len) = unsafe { buffer.read_buffer() }; + let len = u16(len).expect("buffer is too large"); channel.set_memory_address(ptr as u32, Increment::Enable); channel.set_transfer_length(len); @@ -97,7 +107,7 @@ impl Transfer { /// Is this transfer complete? pub fn is_complete(&self) -> bool { let inner = self.inner.as_ref().unwrap(); - inner.channel.event_occured(Event::TransferComplete) + inner.channel.event_occurred(Event::TransferComplete) } /// Stop this transfer and return ownership over its parts @@ -420,7 +430,7 @@ pub enum Event { /// Trait implemented by all DMA channels pub trait Channel: private::Channel { /// Is the interrupt flag for the given event set? - fn event_occured(&self, event: Event) -> bool; + fn event_occurred(&self, event: Event) -> bool; /// Clear the interrupt flag for the given event fn clear_event(&mut self, event: Event); @@ -471,14 +481,17 @@ pub trait Channel: private::Channel { /// # Panics /// /// Panics if this channel is enabled. - fn set_transfer_length(&mut self, len: usize) { + fn set_transfer_length(&mut self, len: u16) { assert!(!self.is_enabled()); - let len = u16(len).expect("DMA transfer length too large"); self.ch().ndtr.write(|w| w.ndt().bits(len)); } - /// Set the word size + /// Set the word size. + /// + /// # Panics + /// + /// Panics if the word size is not one of 8, 16, or 32 bits. fn set_word_size(&mut self) { use cr::PSIZE_A::*; @@ -621,7 +634,7 @@ macro_rules! dma { } impl Channel for $Ci { - fn event_occured(&self, event: Event) -> bool { + fn event_occurred(&self, event: Event) -> bool { use Event::*; // NOTE(unsafe) atomic read From 7cc4c0560eaa697ae29d7778a43181798657cd0a Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Fri, 17 Jul 2020 14:13:04 +0200 Subject: [PATCH 11/16] Make the dma module depend on the stm32f303 feature --- src/dma.rs | 6 ++---- src/lib.rs | 2 +- src/prelude.rs | 1 + 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/dma.rs b/src/dma.rs index 8f710840b..a7140d5d8 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -1,8 +1,6 @@ //! Direct memory access (DMA) controller - -// Currently DMA is only supported for STM32F303 MCUs. -// Remove these `allow`s once all models have support. -#![cfg_attr(not(feature = "stm32f303"), allow(unused_imports, unused_macros))] +//! +//! Currently DMA is only supported for STM32F303 MCUs. use crate::{ rcc::AHB, diff --git a/src/lib.rs b/src/lib.rs index 0f5fb02ea..edd67042a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,7 +122,7 @@ pub use crate::pac::interrupt; #[cfg(feature = "device-selected")] pub mod delay; -#[cfg(feature = "device-selected")] +#[cfg(feature = "stm32f303")] pub mod dma; #[cfg(feature = "device-selected")] pub mod flash; diff --git a/src/prelude.rs b/src/prelude.rs index 1b4122750..fa17f9449 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -1,5 +1,6 @@ //! Prelude +#[cfg(feature = "stm32f303")] pub use crate::dma::DmaExt as _stm32f3xx_hal_dma_DmaExt; pub use crate::flash::FlashExt as _stm32f3xx_hal_flash_FlashExt; pub use crate::gpio::GpioExt as _stm32f3xx_hal_gpio_GpioExt; From 00cb75836185f202c9bac160108c56845981bae0 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Fri, 17 Jul 2020 15:03:50 +0200 Subject: [PATCH 12/16] dma: Use the `pac` name instead of `stm32` --- examples/serial_dma.rs | 4 ++-- src/dma.rs | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/examples/serial_dma.rs b/examples/serial_dma.rs index 7a10058e3..796f84638 100644 --- a/examples/serial_dma.rs +++ b/examples/serial_dma.rs @@ -9,11 +9,11 @@ use panic_semihosting as _; use cortex_m::singleton; use cortex_m_rt::entry; -use stm32f3xx_hal::{prelude::*, serial::Serial, stm32}; +use stm32f3xx_hal::{pac, prelude::*, serial::Serial}; #[entry] fn main() -> ! { - let dp = stm32::Peripherals::take().unwrap(); + let dp = pac::Peripherals::take().unwrap(); let mut flash = dp.FLASH.constrain(); let mut rcc = dp.RCC.constrain(); diff --git a/src/dma.rs b/src/dma.rs index a7140d5d8..c47824777 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -3,9 +3,9 @@ //! Currently DMA is only supported for STM32F303 MCUs. use crate::{ + pac::{self, dma1::ch::cr}, rcc::AHB, serial, - stm32::{self, dma1::ch::cr}, }; use cast::u16; use core::{ @@ -566,12 +566,12 @@ pub trait Channel: private::Channel { } mod private { - use crate::stm32; + use crate::pac; /// Channel methods private to this module pub trait Channel { /// Return the register block for this channel - fn ch(&self) -> &stm32::dma1::CH; + fn ch(&self) -> &pac::dma1::CH; } } @@ -588,7 +588,7 @@ macro_rules! dma { ) => { pub mod $dmax { use super::*; - use crate::stm32::$DMAx; + use crate::pac::$DMAx; impl DmaExt for $DMAx { type Channels = Channels; @@ -625,7 +625,7 @@ macro_rules! dma { } impl private::Channel for $Ci { - fn ch(&self) -> &stm32::dma1::CH { + fn ch(&self) -> &pac::dma1::CH { // NOTE(unsafe) $Ci grants exclusive access to this register unsafe { &(*$DMAx::ptr()).$chi } } @@ -709,10 +709,10 @@ macro_rules! targets { #[cfg(feature = "stm32f303")] targets!(dma1, - serial::Rx => C5, - serial::Tx => C4, - serial::Rx => C6, - serial::Tx => C7, - serial::Rx => C3, - serial::Tx => C2, + serial::Rx => C5, + serial::Tx => C4, + serial::Rx => C6, + serial::Tx => C7, + serial::Rx => C3, + serial::Tx => C2, ); From 11aa9cc592326c7de51fc7398c8b4952cd26af98 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Fri, 17 Jul 2020 20:10:24 +0200 Subject: [PATCH 13/16] dma: Improve documentation --- src/dma.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/dma.rs b/src/dma.rs index c47824777..317485df4 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -2,6 +2,9 @@ //! //! Currently DMA is only supported for STM32F303 MCUs. +// To learn about most of the ideas implemented here, check out the DMA section +// of the Embedonomicon: https://docs.rust-embedded.org/embedonomicon/dma.html + use crate::{ pac::{self, dma1::ch::cr}, rcc::AHB, @@ -429,7 +432,14 @@ pub enum Event { pub trait Channel: private::Channel { /// Is the interrupt flag for the given event set? fn event_occurred(&self, event: Event) -> bool; - /// Clear the interrupt flag for the given event + + /// Clear the interrupt flag for the given event. + /// + /// Passing `Event::Any` clears all interrupt flags. + /// + /// Note that the the global interrupt flag is not automatically cleared + /// even when all other flags are cleared. The only way to clear it is to + /// call this method with `Event::Any`. fn clear_event(&mut self, event: Event); /// Reset the control registers of this channel. From 20d4b190b9bc1060d34e21fa5b4638c08f9393dd Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Fri, 17 Jul 2020 20:10:44 +0200 Subject: [PATCH 14/16] dma: Fix compiler fence on Transfer stop The ordering previously used here (Acquire) was not strong enough, since it only prevents memory reads after the fence to be moved before the last read before the fence. We want to prevent moving before the last *write* before the fence (the one that disables DMA). The SeqCst ordering does that for us. --- src/dma.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dma.rs b/src/dma.rs index 317485df4..d0922960c 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -146,7 +146,7 @@ impl TransferInner { /// Stop this transfer fn stop(&mut self) { self.channel.disable(); - atomic::compiler_fence(Ordering::Acquire); + atomic::compiler_fence(Ordering::SeqCst); } } From 0f1d6a3f3890c7fe5f64cbea16bab58b63fc5938 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Fri, 17 Jul 2020 21:50:30 +0200 Subject: [PATCH 15/16] 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. --- src/dma.rs | 43 +++++++++++++++++++++++++++++-------------- src/serial.rs | 50 ++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/src/dma.rs b/src/dma.rs index d0922960c..fc3992a66 100644 --- a/src/dma.rs +++ b/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 { +pub struct Transfer { // This is always a `Some` outside of `drop`. inner: Option>, } -impl Transfer { +impl Transfer { /// Start a DMA write transfer. /// /// # Panics @@ -42,7 +50,7 @@ impl Transfer { pub fn start_write(mut buffer: B, mut channel: C, target: T) -> Self where B: WriteBuffer + 'static, - T: Target, + T: OnChannel, { // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its // concrete type is unknown here @@ -65,7 +73,7 @@ impl Transfer { pub fn start_read(buffer: B, mut channel: C, target: T) -> Self where B: ReadBuffer + 'static, - T: Target, + T: OnChannel, { // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its // concrete type is unknown here @@ -86,14 +94,15 @@ impl Transfer { /// /// - 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, + T: OnChannel, { assert!(!channel.is_enabled()); atomic::compiler_fence(Ordering::Release); + target.enable_dma(); channel.enable(); Self { @@ -127,7 +136,7 @@ impl Transfer { } } -impl Drop for Transfer { +impl Drop for Transfer { fn drop(&mut self) { if let Some(inner) = self.inner.as_mut() { inner.stop(); @@ -142,10 +151,12 @@ struct TransferInner { target: T, } -impl TransferInner { +impl TransferInner { /// 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 {} +/// 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: 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 => C5, serial::Tx => C4, serial::Rx => C6, diff --git a/src/serial.rs b/src/serial.rs index 9a925a726..e419ff821 100644 --- a/src/serial.rs +++ b/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 where - Self: dma::Target, + Self: dma::OnChannel, B: dma::WriteBuffer + 'static, C: dma::Channel, { @@ -398,7 +394,7 @@ macro_rules! hal { mut channel: C ) -> dma::Transfer where - Self: dma::Target, + Self: dma::OnChannel, B: dma::ReadBuffer + '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()); + }); + } + } )+ } } From 25dda1bd0a6ebe3201aba41a98e84d4c383921f2 Mon Sep 17 00:00:00 2001 From: Jan Teske Date: Sat, 18 Jul 2020 12:31:23 +0200 Subject: [PATCH 16/16] dma: Improve wording of Safety notes --- src/dma.rs | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/dma.rs b/src/dma.rs index fc3992a66..cd6467b74 100644 --- a/src/dma.rs +++ b/src/dma.rs @@ -52,8 +52,10 @@ impl Transfer { B: WriteBuffer + 'static, T: OnChannel, { - // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its - // concrete type is unknown here + // NOTE(unsafe) We don't know the concrete type of `buffer` here, all + // we can use are its `WriteBuffer` methods. Hence the only `&mut self` + // method we can call is `write_buffer`, which is allowed by + // `WriteBuffer`'s safety requirements. let (ptr, len) = unsafe { buffer.write_buffer() }; let len = u16(len).expect("buffer is too large"); @@ -75,8 +77,10 @@ impl Transfer { B: ReadBuffer + 'static, T: OnChannel, { - // NOTE(unsafe) cannot call `&mut self` methods on `buffer` because its - // concrete type is unknown here + // NOTE(unsafe) We don't know the concrete type of `buffer` here, all + // we can use are its `ReadBuffer` methods. Hence there are no + // `&mut self` methods we can call, so we are safe according to + // `ReadBuffer`'s safety requirements. let (ptr, len) = unsafe { buffer.read_buffer() }; let len = u16(len).expect("buffer is too large"); @@ -168,11 +172,11 @@ impl TransferInner { /// The implementing type must be safe to use for DMA reads. This means: /// /// - It must be a pointer that references the actual buffer. -/// - The following requirements must be fulfilled by `read_buffer`: -/// - The function must always return the same values, if called multiple +/// - As long as no `&mut self` method is called on the implementing object: +/// - `read_buffer` must always return the same value, if called multiple /// times. -/// - The memory specified by the returned pointer and size must not be -/// freed as long as `self` is not dropped. +/// - The memory specified by the pointer and size returned by `read_buffer` +/// must not be freed as long as `self` is not dropped. pub unsafe trait ReadBuffer { type Word; @@ -186,7 +190,7 @@ pub unsafe trait ReadBuffer { /// # Safety /// /// Once this method has been called, it is unsafe to call any `&mut self` - /// methods on this object as long as the returned value is used (for DMA). + /// methods on this object as long as the returned value is in use (by DMA). unsafe fn read_buffer(&self) -> (*const Self::Word, usize); } @@ -198,11 +202,12 @@ pub unsafe trait ReadBuffer { /// /// - It must be a pointer that references the actual buffer. /// - `Target` must be a type that is valid for any possible byte pattern. -/// - The following requirements must be fulfilled by `write_buffer`: -/// - The function must always return the same values, if called multiple +/// - As long as no `&mut self` method, except for `write_buffer`, is called on +/// the implementing object: +/// - `write_buffer` must always return the same value, if called multiple /// times. -/// - The memory specified by the returned pointer and size must not be -/// freed as long as `self` is not dropped. +/// - The memory specified by the pointer and size returned by `write_buffer` +/// must not be freed as long as `self` is not dropped. pub unsafe trait WriteBuffer { type Word; @@ -216,7 +221,8 @@ pub unsafe trait WriteBuffer { /// # Safety /// /// Once this method has been called, it is unsafe to call any `&mut self` - /// methods on this object as long as the returned value is used (for DMA).. + /// methods, except for `write_buffer`, on this object as long as the + /// returned value is in use (by DMA). unsafe fn write_buffer(&mut self) -> (*mut Self::Word, usize); }