From 2f7c45e4bd631dc0445eec4a5879819afe2a9bd8 Mon Sep 17 00:00:00 2001 From: Thales <46510852+thalesfragoso@users.noreply.github.com> Date: Thu, 15 Oct 2020 15:40:15 -0300 Subject: [PATCH] Change DMA API to use embedded-dma traits (#274) * Change DMA API to use embedded-dma traits * Fix dma::Transfer::peek * Update docs on DMA code --- CHANGELOG.md | 1 + Cargo.toml | 2 +- src/adc.rs | 75 ++++++++++++++++----------------- src/dma.rs | 112 +++++++++++++++++++++++++++++++------------------- src/serial.rs | 94 ++++++++++++++++++++++-------------------- src/spi.rs | 35 ++++++++-------- 6 files changed, 176 insertions(+), 143 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e05182ae..1ce92604 100644 --- 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 index e543e37d..9ea320f0 100644 --- 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 index ffc26d93..a0f3384a 100644 --- 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 crate::dma::CircReadDma for AdcDma where Self: TransferPayload, - B: as_slice::AsMutSlice, + &'static mut [B; 2]: StaticWriteBuffer, + B: 'static, { - fn circ_read(mut self, buffer: &'static mut [B; 2]) -> CircBuffer { - { - 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 { + // 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 crate::dma::ReadDma for AdcDma where Self: TransferPayload, - B: as_slice::AsMutSlice, + B: StaticWriteBuffer, { - fn read(mut self, buffer: &'static mut B) -> Transfer { - { - 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 { + // 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 index 9628587d..b999fc28 100644 --- 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 CircBuffer { +impl CircBuffer +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 CircBuffer { } } -pub trait Static { - fn borrow(&self) -> &B; -} - -impl Static for &'static B { - fn borrow(&self) -> &B { - *self - } -} - -impl Static 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 { +pub struct Transfer +where + PAYLOAD: TransferPayload, +{ _mode: PhantomData, buffer: BUFFER, payload: PAYLOAD, } -impl Transfer { +impl Transfer +where + PAYLOAD: TransferPayload, +{ pub(crate) fn r(buffer: BUFFER, payload: PAYLOAD) -> Self { Transfer { _mode: PhantomData, @@ -86,7 +82,10 @@ impl Transfer { } } -impl Transfer { +impl Transfer +where + PAYLOAD: TransferPayload, +{ pub(crate) fn w(buffer: BUFFER, payload: PAYLOAD) -> Self { Transfer { _mode: PhantomData, @@ -96,11 +95,13 @@ impl Transfer { } } -impl ops::Deref for Transfer { - type Target = BUFFER; - - fn deref(&self) -> &BUFFER { - &self.buffer +impl Drop for Transfer +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 Transfer> + impl Transfer> where RxDma: TransferPayload, { @@ -480,27 +505,30 @@ pub trait Transmit { type ReceivedWord; } +/// Trait for circular DMA readings from peripheral to memory. pub trait CircReadDma: Receive where - B: as_slice::AsMutSlice, + &'static mut [B; 2]: StaticWriteBuffer, + B: 'static, Self: core::marker::Sized, { fn circ_read(self, buffer: &'static mut [B; 2]) -> CircBuffer; } +/// Trait for DMA readings from peripheral to memory. pub trait ReadDma: Receive where - B: as_slice::AsMutSlice, - Self: core::marker::Sized, + B: StaticWriteBuffer, + Self: core::marker::Sized + TransferPayload, { - fn read(self, buffer: &'static mut B) -> Transfer; + fn read(self, buffer: B) -> Transfer; } -pub trait WriteDma: Transmit +/// Trait for DMA writing from memory to peripheral. +pub trait WriteDma: Transmit where - A: as_slice::AsSlice, - B: Static, - Self: core::marker::Sized, + B: StaticReadBuffer, + Self: core::marker::Sized + TransferPayload, { fn write(self, buffer: B) -> Transfer; } diff --git a/src/serial.rs b/src/serial.rs index f8b5d2de..31c16365 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -44,10 +44,11 @@ use core::sync::atomic::{self, Ordering}; use crate::pac::{USART1, USART2, USART3}; use core::convert::Infallible; +use embedded_dma::{StaticReadBuffer, StaticWriteBuffer}; use embedded_hal::serial::Write; use crate::afio::MAPR; -use crate::dma::{dma1, CircBuffer, RxDma, Static, Transfer, TxDma, R, W}; +use crate::dma::{dma1, CircBuffer, RxDma, Transfer, TxDma, R, W}; use crate::gpio::gpioa::{PA10, PA2, PA3, PA9}; use crate::gpio::gpiob::{PB10, PB11, PB6, PB7}; use crate::gpio::gpioc::{PC10, PC11}; @@ -66,6 +67,7 @@ pub enum Event { /// Serial error #[derive(Debug)] +#[non_exhaustive] pub enum Error { /// Framing error Framing, @@ -75,8 +77,6 @@ pub enum Error { Overrun, /// Parity check error Parity, - #[doc(hidden)] - _Extensible, } // USART REMAPPING, see: https://www.st.com/content/ccc/resource/technical/document/reference_manual/59/b9/ba/7f/11/af/43/d5/CD00171190.pdf/files/CD00171190.pdf/jcr:content/translations/en.CD00171190.pdf @@ -304,6 +304,7 @@ macro_rules! hal { #[allow(unused_unsafe)] mapr.modify_mapr(|_, w| unsafe{ + #[allow(clippy::redundant_closure_call)] w.$usartX_remap().$bit(($closure)(PINS::REMAP)) }); @@ -584,27 +585,29 @@ macro_rules! serialdma { } } - impl crate::dma::CircReadDma for $rxdma where B: as_slice::AsMutSlice { - fn circ_read(mut self, buffer: &'static mut [B; 2], - ) -> CircBuffer - { - { - let buffer = buffer[0].as_mut_slice(); - self.channel.set_peripheral_address(unsafe{ &(*$USARTX::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() .bits8() - .psize() .bits8() - .circ() .set_bit() - .dir() .clear_bit() - }); - } + impl crate::dma::CircReadDma for $rxdma + where + &'static mut [B; 2]: StaticWriteBuffer, + B: 'static, + { + fn circ_read(mut self, mut buffer: &'static mut [B; 2]) -> CircBuffer { + // 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{ &(*$USARTX::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() .bits8() + .psize() .bits8() + .circ() .set_bit() + .dir() .clear_bit() + }); self.start(); @@ -612,16 +615,18 @@ macro_rules! serialdma { } } - impl crate::dma::ReadDma for $rxdma where B: as_slice::AsMutSlice { - fn read(mut self, buffer: &'static mut B, - ) -> Transfer - { - { - let buffer = buffer.as_mut_slice(); - self.channel.set_peripheral_address(unsafe{ &(*$USARTX::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()); - } + impl crate::dma::ReadDma for $rxdma + where + B: StaticWriteBuffer, + { + fn read(mut self, mut buffer: B) -> Transfer { + // 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{ &(*$USARTX::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() @@ -637,18 +642,19 @@ macro_rules! serialdma { } } - impl crate::dma::WriteDma for $txdma where A: as_slice::AsSlice, B: Static { - fn write(mut self, buffer: B - ) -> Transfer - { - { - let buffer = buffer.borrow().as_slice(); + impl crate::dma::WriteDma for $txdma + where + B: StaticReadBuffer, + { + fn write(mut self, buffer: B) -> Transfer { + // 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_read_buffer() }; - self.channel.set_peripheral_address(unsafe{ &(*$USARTX::ptr()).dr as *const _ as u32 }, false); + self.channel.set_peripheral_address(unsafe{ &(*$USARTX::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()); - } + self.channel.set_memory_address(ptr as u32, true); + self.channel.set_transfer_length(len); atomic::compiler_fence(Ordering::Release); diff --git a/src/spi.rs b/src/spi.rs index 445b742f..234316a0 100644 --- a/src/spi.rs +++ b/src/spi.rs @@ -43,7 +43,7 @@ use crate::afio::MAPR; use crate::dma::dma1::{C3, C5}; #[cfg(feature = "connectivity")] use crate::dma::dma2::C2; -use crate::dma::{Static, Transfer, TransferPayload, Transmit, TxDma, R}; +use crate::dma::{Transfer, TransferPayload, Transmit, TxDma, R}; use crate::gpio::gpioa::{PA5, PA6, PA7}; use crate::gpio::gpiob::{PB13, PB14, PB15, PB3, PB4, PB5}; #[cfg(feature = "connectivity")] @@ -53,11 +53,11 @@ use crate::rcc::{Clocks, Enable, GetBusFreq, Reset, APB1, APB2}; use crate::time::Hertz; use core::sync::atomic::{self, Ordering}; - -use as_slice::AsSlice; +use embedded_dma::StaticReadBuffer; /// SPI error #[derive(Debug)] +#[non_exhaustive] pub enum Error { /// Overrun occurred Overrun, @@ -65,8 +65,6 @@ pub enum Error { ModeFault, /// CRC error Crc, - #[doc(hidden)] - _Extensible, } use core::marker::PhantomData; @@ -246,7 +244,7 @@ where fn read_data_reg(&mut self) -> FrameSize { // NOTE(read_volatile) read only 1 byte (the svd2rust API only allows // reading a half-word) - return unsafe { ptr::read_volatile(&self.spi.dr as *const _ as *const FrameSize) }; + unsafe { ptr::read_volatile(&self.spi.dr as *const _ as *const FrameSize) } } fn write_data_reg(&mut self, data: FrameSize) { @@ -541,22 +539,21 @@ macro_rules! spi_dma { } } - impl crate::dma::WriteDma for SpiTxDma<$SPIi, REMAP, PIN, $TCi> + impl crate::dma::WriteDma for SpiTxDma<$SPIi, REMAP, PIN, $TCi> where - A: AsSlice, - B: Static, + B: StaticReadBuffer, { fn write(mut self, buffer: B) -> Transfer { - { - let buffer = buffer.borrow().as_slice(); - self.channel.set_peripheral_address( - unsafe { &(*$SPIi::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()); - } + // 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_read_buffer() }; + self.channel.set_peripheral_address( + unsafe { &(*$SPIi::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