Skip to content

Change DMA API to use embedded-dma traits #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 15, 2020
Merged

Change DMA API to use embedded-dma traits #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
888bd3e
Change DMA API to use embedded-dma traits
thalesfragoso Oct 3, 2020
029f686
Fix dma::Transfer::peek
thalesfragoso Oct 3, 2020
cb4c5d5
Update docs on DMA code
thalesfragoso Oct 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
75 changes: 38 additions & 37 deletions src/adc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -701,34 +702,34 @@ where
impl<B, PINS, MODE> crate::dma::CircReadDma<B, u16> for AdcDma<PINS, MODE>
where
Self: TransferPayload,
B: as_slice::AsMutSlice<Element = u16>,
&'static mut [B; 2]: StaticWriteBuffer<Word = u16>,
B: 'static,
{
fn circ_read(mut self, buffer: &'static mut [B; 2]) -> CircBuffer<B, Self> {
{
let buffer = buffer[0].as_mut_slice();
self.channel
.set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
self.channel
.set_memory_address(buffer.as_ptr() as u32, true);
self.channel.set_transfer_length(buffer.len() * 2);

atomic::compiler_fence(Ordering::Release);

self.channel.ch().cr.modify(|_, w| {
w.mem2mem()
.clear_bit()
.pl()
.medium()
.msize()
.bits16()
.psize()
.bits16()
.circ()
.set_bit()
.dir()
.clear_bit()
});
}
fn circ_read(mut self, mut buffer: &'static mut [B; 2]) -> CircBuffer<B, Self> {
// NOTE(unsafe) We own the buffer now and we won't call other `&mut` on it
// until the end of the transfer.
let (ptr, len) = unsafe { buffer.static_write_buffer() };
self.channel
.set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
self.channel.set_memory_address(ptr as u32, true);
self.channel.set_transfer_length(len);

atomic::compiler_fence(Ordering::Release);

self.channel.ch().cr.modify(|_, w| {
w.mem2mem()
.clear_bit()
.pl()
.medium()
.msize()
.bits16()
.psize()
.bits16()
.circ()
.set_bit()
.dir()
.clear_bit()
});

self.start();

Expand All @@ -739,17 +740,17 @@ where
impl<B, PINS, MODE> crate::dma::ReadDma<B, u16> for AdcDma<PINS, MODE>
where
Self: TransferPayload,
B: as_slice::AsMutSlice<Element = u16>,
B: StaticWriteBuffer<Word = u16>,
{
fn read(mut self, buffer: &'static mut B) -> Transfer<W, &'static mut B, Self> {
{
let buffer = buffer.as_mut_slice();
self.channel
.set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
self.channel
.set_memory_address(buffer.as_ptr() as u32, true);
self.channel.set_transfer_length(buffer.len());
}
fn read(mut self, mut buffer: B) -> Transfer<W, B, Self> {
// NOTE(unsafe) We own the buffer now and we won't call other `&mut` on it
// until the end of the transfer.
let (ptr, len) = unsafe { buffer.static_write_buffer() };
self.channel
.set_peripheral_address(unsafe { &(*ADC1::ptr()).dr as *const _ as u32 }, false);
self.channel.set_memory_address(ptr as u32, true);
self.channel.set_transfer_length(len);

atomic::compiler_fence(Ordering::Release);
self.channel.ch().cr.modify(|_, w| {
w.mem2mem()
Expand Down
112 changes: 70 additions & 42 deletions src/dma.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -33,7 +35,11 @@ where
readable_half: Half,
}

impl<BUFFER, PAYLOAD> CircBuffer<BUFFER, PAYLOAD> {
impl<BUFFER, PAYLOAD> CircBuffer<BUFFER, PAYLOAD>
where
&'static mut [BUFFER; 2]: StaticWriteBuffer,
BUFFER: 'static,
{
pub(crate) fn new(buf: &'static mut [BUFFER; 2], payload: PAYLOAD) -> Self {
CircBuffer {
buffer: buf,
Expand All @@ -43,22 +49,6 @@ impl<BUFFER, PAYLOAD> CircBuffer<BUFFER, PAYLOAD> {
}
}

pub trait Static<B> {
fn borrow(&self) -> &B;
}

impl<B> Static<B> for &'static B {
fn borrow(&self) -> &B {
*self
}
}

impl<B> Static<B> for &'static mut B {
fn borrow(&self) -> &B {
*self
}
}

pub trait DmaExt {
type Channels;

Expand All @@ -70,13 +60,19 @@ pub trait TransferPayload {
fn stop(&mut self);
}

pub struct Transfer<MODE, BUFFER, PAYLOAD> {
pub struct Transfer<MODE, BUFFER, PAYLOAD>
where
PAYLOAD: TransferPayload,
{
_mode: PhantomData<MODE>,
buffer: BUFFER,
payload: PAYLOAD,
}

impl<BUFFER, PAYLOAD> Transfer<R, BUFFER, PAYLOAD> {
impl<BUFFER, PAYLOAD> Transfer<R, BUFFER, PAYLOAD>
where
PAYLOAD: TransferPayload,
{
pub(crate) fn r(buffer: BUFFER, payload: PAYLOAD) -> Self {
Transfer {
_mode: PhantomData,
Expand All @@ -86,7 +82,10 @@ impl<BUFFER, PAYLOAD> Transfer<R, BUFFER, PAYLOAD> {
}
}

impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, PAYLOAD> {
impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, PAYLOAD>
where
PAYLOAD: TransferPayload,
{
pub(crate) fn w(buffer: BUFFER, payload: PAYLOAD) -> Self {
Transfer {
_mode: PhantomData,
Expand All @@ -96,11 +95,13 @@ impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, PAYLOAD> {
}
}

impl<BUFFER, PAYLOAD> ops::Deref for Transfer<R, BUFFER, PAYLOAD> {
type Target = BUFFER;

fn deref(&self) -> &BUFFER {
&self.buffer
impl<MODE, BUFFER, PAYLOAD> Drop for Transfer<MODE, BUFFER, PAYLOAD>
where
PAYLOAD: TransferPayload,
{
fn drop(&mut self) {
self.payload.stop();
compiler_fence(Ordering::SeqCst);
}
}

Expand All @@ -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),+);

$(
Expand Down Expand Up @@ -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)
}
Comment on lines +327 to +332
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably missing something here, but why is this required? It seems like this would be the smae as just moving self.buffer and self.payload into the tuple directly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transfer now implements Drop, so we can't move out of its fields.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right. Could you add a comment clarifying that in the code? Seems like something that will come up in the future

}
}

Expand All @@ -342,11 +355,23 @@ macro_rules! dma {
// we need a fence here for the same reason we need one in `Transfer.wait`
atomic::compiler_fence(Ordering::Acquire);

(self.buffer, self.payload)
// `Transfer` needs to have a `Drop` implementation, because we accept
// managed buffers that can free their memory on drop. Because of that
// we can't move out of the `Transfer`'s fields, so we use `ptr::read`
// and `mem::forget`.
//
// NOTE(unsafe) There is no panic branch between getting the resources
// and forgetting `self`.
unsafe {
let buffer = ptr::read(&self.buffer);
let payload = ptr::read(&self.payload);
mem::forget(self);
(buffer, payload)
}
}
}

impl<BUFFER, PAYLOAD> Transfer<W, &'static mut BUFFER, RxDma<PAYLOAD, $CX>>
impl<BUFFER, PAYLOAD> Transfer<W, BUFFER, RxDma<PAYLOAD, $CX>>
where
RxDma<PAYLOAD, $CX>: TransferPayload,
{
Expand Down Expand Up @@ -480,27 +505,30 @@ pub trait Transmit {
type ReceivedWord;
}

/// Trait for circular DMA readings from peripheral to memory.
pub trait CircReadDma<B, RS>: Receive
where
B: as_slice::AsMutSlice<Element = RS>,
&'static mut [B; 2]: StaticWriteBuffer<Word = RS>,
B: 'static,
Self: core::marker::Sized,
{
fn circ_read(self, buffer: &'static mut [B; 2]) -> CircBuffer<B, Self>;
}

/// Trait for DMA readings from peripheral to memory.
pub trait ReadDma<B, RS>: Receive
where
B: as_slice::AsMutSlice<Element = RS>,
Self: core::marker::Sized,
B: StaticWriteBuffer<Word = RS>,
Self: core::marker::Sized + TransferPayload,
{
fn read(self, buffer: &'static mut B) -> Transfer<W, &'static mut B, Self>;
fn read(self, buffer: B) -> Transfer<W, B, Self>;
}

pub trait WriteDma<A, B, TS>: Transmit
/// Trait for DMA writing from memory to peripheral.
pub trait WriteDma<B, TS>: Transmit
where
A: as_slice::AsSlice<Element = TS>,
B: Static<A>,
Self: core::marker::Sized,
B: StaticReadBuffer<Word = TS>,
Self: core::marker::Sized + TransferPayload,
{
fn write(self, buffer: B) -> Transfer<R, B, Self>;
}
Loading