Skip to content

Commit

Permalink
Lift static requirement on DMA buffers (esp-rs#1837)
Browse files Browse the repository at this point in the history
* Lift `static` requirement on DMA buffers

* Seal `Word`, use `size_of_val`

* Fix

* CHANGELOG.md

* Use Clippy's safety comment style

* Top-level docs regarding mem::forget
  • Loading branch information
bjoernQ authored Jul 25, 2024
1 parent 45db1b5 commit c6207c0
Show file tree
Hide file tree
Showing 14 changed files with 255 additions and 82 deletions.
1 change: 1 addition & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Use the peripheral ref pattern for `OneShotTimer` and `PeriodicTimer` (#1855)

- Allow DMA to/from psram for esp32s3 (#1827)
- DMA buffers now don't require a static lifetime. Make sure to never `mem::forget` an in-progress DMA transfer (consider using `#[deny(clippy::mem_forget)]`) (#1837)

### Fixed

Expand Down
1 change: 0 additions & 1 deletion esp-hal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ embassy-sync = { version = "0.6.0", optional = true }
embassy-usb-driver = { version = "0.1.0", optional = true }
embassy-usb-synopsys-otg = { version = "0.1.0", optional = true }
embedded-can = { version = "0.4.1", optional = true }
embedded-dma = "0.2.0"
embedded-hal-02 = { version = "0.2.7", optional = true, features = ["unproven"], package = "embedded-hal" }
embedded-hal = { version = "1.0.0", optional = true }
embedded-hal-async = { version = "1.0.0", optional = true }
Expand Down
8 changes: 4 additions & 4 deletions esp-hal/src/aes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,6 @@ pub enum Endianness {
/// CTR, CFB8, and CFB128.
#[cfg(any(esp32c3, esp32c6, esp32h2, esp32s3))]
pub mod dma {
use embedded_dma::{ReadBuffer, WriteBuffer};

use crate::{
aes::{Key, Mode},
dma::{
Expand All @@ -235,8 +233,10 @@ pub mod dma {
DmaDescriptor,
DmaPeripheral,
DmaTransferTxRx,
ReadBuffer,
RxPrivate,
TxPrivate,
WriteBuffer,
},
};

Expand Down Expand Up @@ -409,8 +409,8 @@ pub mod dma {
) -> Result<DmaTransferTxRx<Self>, crate::dma::DmaError>
where
K: Into<Key>,
TXBUF: ReadBuffer<Word = u8>,
RXBUF: WriteBuffer<Word = u8>,
TXBUF: ReadBuffer,
RXBUF: WriteBuffer,
{
let (write_ptr, write_len) = unsafe { words.read_buffer() };
let (read_ptr, read_len) = unsafe { read_buffer.write_buffer() };
Expand Down
8 changes: 4 additions & 4 deletions esp-hal/src/dma/gdma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,6 @@ impl<'d> Dma<'d> {

pub use m2m::*;
mod m2m {
use embedded_dma::{ReadBuffer, WriteBuffer};

#[cfg(esp32s3)]
use crate::dma::DmaExtMemBKSize;
use crate::dma::{
Expand All @@ -653,8 +651,10 @@ mod m2m {
DmaError,
DmaPeripheral,
DmaTransferRx,
ReadBuffer,
RxPrivate,
TxPrivate,
WriteBuffer,
};

/// DMA Memory to Memory pseudo-Peripheral
Expand Down Expand Up @@ -751,8 +751,8 @@ mod m2m {
rx_buffer: &'t mut RXBUF,
) -> Result<DmaTransferRx<Self>, DmaError>
where
TXBUF: ReadBuffer<Word = u8>,
RXBUF: WriteBuffer<Word = u8>,
TXBUF: ReadBuffer,
RXBUF: WriteBuffer,
{
let (tx_ptr, tx_len) = unsafe { tx_buffer.read_buffer() };
let (rx_ptr, rx_len) = unsafe { rx_buffer.write_buffer() };
Expand Down
186 changes: 173 additions & 13 deletions esp-hal/src/dma/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,135 @@

use core::{fmt::Debug, marker::PhantomData, ptr::addr_of_mut, sync::atomic::compiler_fence};

trait Word: crate::private::Sealed {}

macro_rules! impl_word {
($w:ty) => {
impl $crate::private::Sealed for $w {}
impl Word for $w {}
};
}

impl_word!(u8);
impl_word!(u16);
impl_word!(u32);
impl_word!(i8);
impl_word!(i16);
impl_word!(i32);

impl<W, const S: usize> crate::private::Sealed for [W; S] where W: Word {}

impl<W, const S: usize> crate::private::Sealed for &[W; S] where W: Word {}

impl<W> crate::private::Sealed for &[W] where W: Word {}

impl<W> crate::private::Sealed for &mut [W] where W: Word {}

/// Trait for buffers that can be given to DMA for reading.
pub trait ReadBuffer: crate::private::Sealed {
/// Provide a buffer usable for DMA reads.
///
/// The return value is:
///
/// - pointer to the start of the buffer
/// - buffer size in bytes
///
/// # 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 in use (by DMA).
unsafe fn read_buffer(&self) -> (*const u8, usize);
}

impl<W, const S: usize> ReadBuffer for [W; S]
where
W: Word,
{
unsafe fn read_buffer(&self) -> (*const u8, usize) {
(self.as_ptr() as *const u8, core::mem::size_of_val(self))
}
}

impl<W, const S: usize> ReadBuffer for &[W; S]
where
W: Word,
{
unsafe fn read_buffer(&self) -> (*const u8, usize) {
(self.as_ptr() as *const u8, core::mem::size_of_val(*self))
}
}

impl<W, const S: usize> ReadBuffer for &mut [W; S]
where
W: Word,
{
unsafe fn read_buffer(&self) -> (*const u8, usize) {
(self.as_ptr() as *const u8, core::mem::size_of_val(*self))
}
}

impl<W> ReadBuffer for &[W]
where
W: Word,
{
unsafe fn read_buffer(&self) -> (*const u8, usize) {
(self.as_ptr() as *const u8, core::mem::size_of_val(*self))
}
}

impl<W> ReadBuffer for &mut [W]
where
W: Word,
{
unsafe fn read_buffer(&self) -> (*const u8, usize) {
(self.as_ptr() as *const u8, core::mem::size_of_val(*self))
}
}

/// Trait for buffers that can be given to DMA for writing.
pub trait WriteBuffer: crate::private::Sealed {
/// Provide a buffer usable for DMA writes.
///
/// The return value is:
///
/// - pointer to the start of the buffer
/// - buffer size in bytes
///
/// # Safety
///
/// Once this method has been called, it is unsafe to call any `&mut self`
/// 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 u8, usize);
}

impl<W, const S: usize> WriteBuffer for [W; S]
where
W: Word,
{
unsafe fn write_buffer(&mut self) -> (*mut u8, usize) {
(self.as_mut_ptr() as *mut u8, core::mem::size_of_val(self))
}
}

impl<W, const S: usize> WriteBuffer for &mut [W; S]
where
W: Word,
{
unsafe fn write_buffer(&mut self) -> (*mut u8, usize) {
(self.as_mut_ptr() as *mut u8, core::mem::size_of_val(*self))
}
}

impl<W> WriteBuffer for &mut [W]
where
W: Word,
{
unsafe fn write_buffer(&mut self) -> (*mut u8, usize) {
(self.as_mut_ptr() as *mut u8, core::mem::size_of_val(*self))
}
}

bitfield::bitfield! {
#[doc(hidden)]
#[derive(Clone, Copy)]
Expand Down Expand Up @@ -130,7 +259,6 @@ impl DmaDescriptor {
}
}

use embedded_dma::{ReadBuffer, WriteBuffer};
use enumset::{EnumSet, EnumSetType};

#[cfg(gdma)]
Expand Down Expand Up @@ -1741,6 +1869,10 @@ pub(crate) mod dma_private {
}

/// DMA transaction for TX only transfers
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferTx<'a, I>
Expand Down Expand Up @@ -1785,6 +1917,10 @@ where
}

/// DMA transaction for RX only transfers
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferRx<'a, I>
Expand Down Expand Up @@ -1829,6 +1965,10 @@ where
}

/// DMA transaction for TX+RX transfers
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferTxRx<'a, I>
Expand Down Expand Up @@ -1874,12 +2014,16 @@ where

/// DMA transaction for TX transfers with moved-in/moved-out peripheral and
/// buffer
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferTxOwned<I, T>
where
I: dma_private::DmaSupportTx,
T: ReadBuffer<Word = u8>,
T: ReadBuffer,
{
instance: I,
tx_buffer: T,
Expand All @@ -1888,7 +2032,7 @@ where
impl<I, T> DmaTransferTxOwned<I, T>
where
I: dma_private::DmaSupportTx,
T: ReadBuffer<Word = u8>,
T: ReadBuffer,
{
pub(crate) fn new(instance: I, tx_buffer: T) -> Self {
Self {
Expand Down Expand Up @@ -1936,7 +2080,7 @@ where
impl<I, T> Drop for DmaTransferTxOwned<I, T>
where
I: dma_private::DmaSupportTx,
T: ReadBuffer<Word = u8>,
T: ReadBuffer,
{
fn drop(&mut self) {
self.instance.peripheral_wait_dma(true, false);
Expand All @@ -1945,12 +2089,16 @@ where

/// DMA transaction for RX transfers with moved-in/moved-out peripheral and
/// buffer
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferRxOwned<I, R>
where
I: dma_private::DmaSupportRx,
R: WriteBuffer<Word = u8>,
R: WriteBuffer,
{
instance: I,
rx_buffer: R,
Expand All @@ -1959,7 +2107,7 @@ where
impl<I, R> DmaTransferRxOwned<I, R>
where
I: dma_private::DmaSupportRx,
R: WriteBuffer<Word = u8>,
R: WriteBuffer,
{
pub(crate) fn new(instance: I, rx_buffer: R) -> Self {
Self {
Expand Down Expand Up @@ -2007,7 +2155,7 @@ where
impl<I, R> Drop for DmaTransferRxOwned<I, R>
where
I: dma_private::DmaSupportRx,
R: WriteBuffer<Word = u8>,
R: WriteBuffer,
{
fn drop(&mut self) {
self.instance.peripheral_wait_dma(false, true);
Expand All @@ -2016,13 +2164,17 @@ where

/// DMA transaction for TX+RX transfers with moved-in/moved-out peripheral and
/// buffers
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferTxRxOwned<I, T, R>
where
I: dma_private::DmaSupportTx + dma_private::DmaSupportRx,
T: ReadBuffer<Word = u8>,
R: WriteBuffer<Word = u8>,
T: ReadBuffer,
R: WriteBuffer,
{
instance: I,
tx_buffer: T,
Expand All @@ -2032,8 +2184,8 @@ where
impl<I, T, R> DmaTransferTxRxOwned<I, T, R>
where
I: dma_private::DmaSupportTx + dma_private::DmaSupportRx,
T: ReadBuffer<Word = u8>,
R: WriteBuffer<Word = u8>,
T: ReadBuffer,
R: WriteBuffer,
{
pub(crate) fn new(instance: I, tx_buffer: T, rx_buffer: R) -> Self {
Self {
Expand Down Expand Up @@ -2084,15 +2236,19 @@ where
impl<I, T, R> Drop for DmaTransferTxRxOwned<I, T, R>
where
I: dma_private::DmaSupportTx + dma_private::DmaSupportRx,
T: ReadBuffer<Word = u8>,
R: WriteBuffer<Word = u8>,
T: ReadBuffer,
R: WriteBuffer,
{
fn drop(&mut self) {
self.instance.peripheral_wait_dma(true, true);
}
}

/// DMA transaction for TX only circular transfers
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferTxCircular<'a, I>
Expand Down Expand Up @@ -2157,6 +2313,10 @@ where
}

/// DMA transaction for RX only circular transfers
///
/// # Safety
///
/// Never use [core::mem::forget] on an in-progress transfer
#[non_exhaustive]
#[must_use]
pub struct DmaTransferRxCircular<'a, I>
Expand Down
Loading

0 comments on commit c6207c0

Please sign in to comment.