From d35a1c9790e32477aac2fd6abe38a8c76263fcc8 Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 18 Aug 2022 19:39:13 +0200 Subject: [PATCH 01/16] Preliminary DMA support for RP2040 --- embassy-rp/src/dma.rs | 143 ++++++++++++++++++++++-------- embassy-rp/src/uart.rs | 197 ++++++++++++++++++++++++++++++++--------- 2 files changed, 260 insertions(+), 80 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 42c4fd13..dfa047a2 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -1,39 +1,71 @@ +use core::pin::Pin; use core::sync::atomic::{compiler_fence, Ordering}; +use core::task::{Context, Poll}; -use embassy_hal_common::impl_peripheral; +use embassy_hal_common::{impl_peripheral, into_ref, Peripheral, PeripheralRef}; +use futures::Future; use crate::pac::dma::vals; use crate::{pac, peripherals}; -pub struct Dma { - _inner: T, +pub fn copy<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: &[W], to: &mut [W]) -> Transfer<'a, C> { + assert!(from.len() == to.len()); + + into_ref!(ch); + + unsafe { + let p = ch.regs(); + + p.read_addr().write_value(from.as_ptr() as u32); + p.write_addr().write_value(to.as_mut_ptr() as u32); + p.trans_count().write_value(from.len() as u32); + + compiler_fence(Ordering::SeqCst); + + p.ctrl_trig().write(|w| { + w.set_data_size(W::size()); + w.set_incr_read(true); + w.set_incr_write(true); + w.set_chain_to(ch.number()); + w.set_en(true); + }); + + // FIXME: + while p.ctrl_trig().read().busy() {} + + compiler_fence(Ordering::SeqCst); + } + Transfer::new(ch) } -impl Dma { - pub fn copy(inner: T, from: &[u32], to: &mut [u32]) { - assert!(from.len() == to.len()); +pub(crate) struct Transfer<'a, C: Channel> { + channel: PeripheralRef<'a, C>, +} - unsafe { - let p = inner.regs(); +impl<'a, C: Channel> Transfer<'a, C> { + pub(crate) fn new(channel: impl Peripheral

+ 'a) -> Self { + into_ref!(channel); + Self { channel } + } +} - p.read_addr().write_value(from.as_ptr() as u32); - p.write_addr().write_value(to.as_mut_ptr() as u32); - p.trans_count().write_value(from.len() as u32); +impl<'a, C: Channel> Drop for Transfer<'a, C> { + fn drop(&mut self) { + // self.channel.request_stop(); + // while self.channel.is_running() {} + } +} - compiler_fence(Ordering::SeqCst); - - p.ctrl_trig().write(|w| { - w.set_data_size(vals::DataSize::SIZE_WORD); - w.set_incr_read(true); - w.set_incr_write(true); - w.set_chain_to(inner.number()); - w.set_en(true); - }); - - while p.ctrl_trig().read().busy() {} - - compiler_fence(Ordering::SeqCst); - } +impl<'a, C: Channel> Unpin for Transfer<'a, C> {} +impl<'a, C: Channel> Future for Transfer<'a, C> { + type Output = (); + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // self.channel.set_waker(cx.waker()); + // if self.channel.is_running() { + // Poll::Pending + // } else { + Poll::Ready(()) + // } } } @@ -42,38 +74,77 @@ pub struct NoDma; impl_peripheral!(NoDma); mod sealed { - use super::*; + pub trait Channel {} - pub trait Channel { - fn number(&self) -> u8; + pub trait Word {} +} - fn regs(&self) -> pac::dma::Channel { - pac::DMA.ch(self.number() as _) +pub trait Channel: Peripheral

+ sealed::Channel + Into + Sized + 'static { + fn number(&self) -> u8; + + fn regs(&self) -> pac::dma::Channel { + pac::DMA.ch(self.number() as _) + } + + fn degrade(self) -> AnyChannel { + AnyChannel { + number: self.number(), } } } -pub trait Channel: sealed::Channel {} +pub trait Word: sealed::Word { + fn size() -> vals::DataSize; +} + +impl sealed::Word for u8 {} +impl Word for u8 { + fn size() -> vals::DataSize { + vals::DataSize::SIZE_BYTE + } +} + +impl sealed::Word for u16 {} +impl Word for u16 { + fn size() -> vals::DataSize { + vals::DataSize::SIZE_HALFWORD + } +} + +impl sealed::Word for u32 {} +impl Word for u32 { + fn size() -> vals::DataSize { + vals::DataSize::SIZE_WORD + } +} pub struct AnyChannel { number: u8, } -impl Channel for AnyChannel {} -impl sealed::Channel for AnyChannel { +impl_peripheral!(AnyChannel); + +impl sealed::Channel for AnyChannel {} +impl Channel for AnyChannel { fn number(&self) -> u8 { self.number } } macro_rules! channel { - ($type:ident, $num:expr) => { - impl Channel for peripherals::$type {} - impl sealed::Channel for peripherals::$type { + ($name:ident, $num:expr) => { + impl sealed::Channel for peripherals::$name {} + impl Channel for peripherals::$name { fn number(&self) -> u8 { $num } } + + impl From for crate::dma::AnyChannel { + fn from(val: peripherals::$name) -> Self { + crate::dma::Channel::degrade(val) + } + } }; } diff --git a/embassy-rp/src/uart.rs b/embassy-rp/src/uart.rs index 6c5ab351..c1596960 100644 --- a/embassy-rp/src/uart.rs +++ b/embassy-rp/src/uart.rs @@ -2,6 +2,7 @@ use core::marker::PhantomData; use embassy_hal_common::{into_ref, PeripheralRef}; +use crate::dma::{AnyChannel, Channel}; use crate::gpio::sealed::Pin; use crate::gpio::AnyPin; use crate::{pac, peripherals, Peripheral}; @@ -76,26 +77,27 @@ pub enum Error { Framing, } -pub struct Uart<'d, T: Instance> { - tx: UartTx<'d, T>, - rx: UartRx<'d, T>, +pub struct Uart<'d, T: Instance, M: Mode> { + tx: UartTx<'d, T, M>, + rx: UartRx<'d, T, M>, } -pub struct UartTx<'d, T: Instance> { - phantom: PhantomData<&'d mut T>, +pub struct UartTx<'d, T: Instance, M: Mode> { + tx_dma: Option>, + phantom: PhantomData<(&'d mut T, M)>, } -pub struct UartRx<'d, T: Instance> { - phantom: PhantomData<&'d mut T>, +pub struct UartRx<'d, T: Instance, M: Mode> { + rx_dma: Option>, + phantom: PhantomData<(&'d mut T, M)>, } -impl<'d, T: Instance> UartTx<'d, T> { - fn new() -> Self { - Self { phantom: PhantomData } - } - - pub async fn write(&mut self, _buffer: &[u8]) -> Result<(), Error> { - todo!() +impl<'d, T: Instance, M: Mode> UartTx<'d, T, M> { + fn new(tx_dma: Option>) -> Self { + Self { + tx_dma, + phantom: PhantomData, + } } pub fn blocking_write(&mut self, buffer: &[u8]) -> Result<(), Error> { @@ -116,13 +118,29 @@ impl<'d, T: Instance> UartTx<'d, T> { } } -impl<'d, T: Instance> UartRx<'d, T> { - fn new() -> Self { - Self { phantom: PhantomData } +impl<'d, T: Instance> UartTx<'d, T, Async> { + pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { + if let Some(ch) = &mut self.tx_dma { + unsafe { + T::regs().uartdmacr().modify(|reg| { + reg.set_txdmae(true); + }); + } + // If we don't assign future to a variable, the data register pointer + // is held across an await and makes the future non-Send. + let transfer = crate::dma::copy(ch, buffer, unsafe { T::regs().uartdr().ptr() }); + transfer.await; + } + Ok(()) } +} - pub async fn read(&mut self, _buffer: &mut [u8]) -> Result<(), Error> { - todo!(); +impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> { + fn new(rx_dma: Option>) -> Self { + Self { + rx_dma, + phantom: PhantomData, + } } pub fn blocking_read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { @@ -150,25 +168,42 @@ impl<'d, T: Instance> UartRx<'d, T> { } } -impl<'d, T: Instance> Uart<'d, T> { +impl<'d, T: Instance> UartRx<'d, T, Async> { + pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { + if let Some(ch) = &mut self.rx_dma { + unsafe { + T::regs().uartdmacr().modify(|reg| { + reg.set_rxdmae(true); + }); + } + // If we don't assign future to a variable, the data register pointer + // is held across an await and makes the future non-Send. + let transfer = crate::dma::copy(ch, unsafe { T::regs().uartdr().ptr() }, buffer); + transfer.await; + } + Ok(()) + } +} + +impl<'d, T: Instance> Uart<'d, T, Blocking> { /// Create a new UART without hardware flow control - pub fn new( + pub fn new_blocking( uart: impl Peripheral

+ 'd, tx: impl Peripheral

> + 'd, rx: impl Peripheral

> + 'd, config: Config, ) -> Self { into_ref!(tx, rx); - Self::new_inner(uart, rx.map_into(), tx.map_into(), None, None, config) + Self::new_inner(uart, rx.map_into(), tx.map_into(), None, None, None, None, config) } /// Create a new UART with hardware flow control (RTS/CTS) - pub fn new_with_rtscts( + pub fn new_with_rtscts_blocking( uart: impl Peripheral

+ 'd, tx: impl Peripheral

> + 'd, rx: impl Peripheral

> + 'd, - cts: impl Peripheral

> + 'd, rts: impl Peripheral

> + 'd, + cts: impl Peripheral

> + 'd, config: Config, ) -> Self { into_ref!(tx, rx, cts, rts); @@ -176,18 +211,72 @@ impl<'d, T: Instance> Uart<'d, T> { uart, rx.map_into(), tx.map_into(), - Some(cts.map_into()), Some(rts.map_into()), + Some(cts.map_into()), + None, + None, + config, + ) + } +} + +impl<'d, T: Instance> Uart<'d, T, Async> { + /// Create a new DMA enabled UART without hardware flow control + pub fn new( + uart: impl Peripheral

+ 'd, + tx: impl Peripheral

> + 'd, + rx: impl Peripheral

> + 'd, + tx_dma: impl Peripheral

+ 'd, + rx_dma: impl Peripheral

+ 'd, + config: Config, + ) -> Self { + into_ref!(tx, rx, tx_dma, rx_dma); + Self::new_inner( + uart, + rx.map_into(), + tx.map_into(), + None, + None, + Some(tx_dma.map_into()), + Some(rx_dma.map_into()), config, ) } + /// Create a new DMA enabled UART with hardware flow control (RTS/CTS) + pub fn new_with_rtscts( + uart: impl Peripheral

+ 'd, + tx: impl Peripheral

> + 'd, + rx: impl Peripheral

> + 'd, + rts: impl Peripheral

> + 'd, + cts: impl Peripheral

> + 'd, + tx_dma: impl Peripheral

+ 'd, + rx_dma: impl Peripheral

+ 'd, + config: Config, + ) -> Self { + into_ref!(tx, rx, cts, rts, tx_dma, rx_dma); + Self::new_inner( + uart, + rx.map_into(), + tx.map_into(), + Some(rts.map_into()), + Some(cts.map_into()), + Some(tx_dma.map_into()), + Some(rx_dma.map_into()), + config, + ) + } +} + +impl<'d, T: Instance, M: Mode> Uart<'d, T, M> { fn new_inner( _uart: impl Peripheral

+ 'd, tx: PeripheralRef<'d, AnyPin>, rx: PeripheralRef<'d, AnyPin>, - cts: Option>, rts: Option>, + cts: Option>, + tx_dma: Option>, + rx_dma: Option>, config: Config, ) -> Self { into_ref!(_uart); @@ -246,15 +335,13 @@ impl<'d, T: Instance> Uart<'d, T> { } Self { - tx: UartTx::new(), - rx: UartRx::new(), + tx: UartTx::new(tx_dma), + rx: UartRx::new(rx_dma), } } +} - pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { - self.tx.write(buffer).await - } - +impl<'d, T: Instance, M: Mode> Uart<'d, T, M> { pub fn blocking_write(&mut self, buffer: &[u8]) -> Result<(), Error> { self.tx.blocking_write(buffer) } @@ -263,26 +350,31 @@ impl<'d, T: Instance> Uart<'d, T> { self.tx.blocking_flush() } - pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { - self.rx.read(buffer).await - } - pub fn blocking_read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { self.rx.blocking_read(buffer) } - /// Split the Uart into a transmitter and receiver, which is - /// particuarly useful when having two tasks correlating to - /// transmitting and receiving. - pub fn split(self) -> (UartTx<'d, T>, UartRx<'d, T>) { + /// Split the Uart into a transmitter and receiver, which is particuarly + /// useful when having two tasks correlating to transmitting and receiving. + pub fn split(self) -> (UartTx<'d, T, M>, UartRx<'d, T, M>) { (self.tx, self.rx) } } +impl<'d, T: Instance> Uart<'d, T, Async> { + pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { + self.tx.write(buffer).await + } + + pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { + self.rx.read(buffer).await + } +} + mod eh02 { use super::*; - impl<'d, T: Instance> embedded_hal_02::serial::Read for UartRx<'d, T> { + impl<'d, T: Instance, M: Mode> embedded_hal_02::serial::Read for UartRx<'d, T, M> { type Error = Error; fn read(&mut self) -> Result> { let r = T::regs(); @@ -306,7 +398,7 @@ mod eh02 { } } - impl<'d, T: Instance> embedded_hal_02::blocking::serial::Write for UartTx<'d, T> { + impl<'d, T: Instance, M: Mode> embedded_hal_02::blocking::serial::Write for UartTx<'d, T, M> { type Error = Error; fn bwrite_all(&mut self, buffer: &[u8]) -> Result<(), Self::Error> { self.blocking_write(buffer) @@ -316,14 +408,14 @@ mod eh02 { } } - impl<'d, T: Instance> embedded_hal_02::serial::Read for Uart<'d, T> { + impl<'d, T: Instance, M: Mode> embedded_hal_02::serial::Read for Uart<'d, T, M> { type Error = Error; fn read(&mut self) -> Result> { embedded_hal_02::serial::Read::read(&mut self.rx) } } - impl<'d, T: Instance> embedded_hal_02::blocking::serial::Write for Uart<'d, T> { + impl<'d, T: Instance, M: Mode> embedded_hal_02::blocking::serial::Write for Uart<'d, T, M> { type Error = Error; fn bwrite_all(&mut self, buffer: &[u8]) -> Result<(), Self::Error> { self.blocking_write(buffer) @@ -419,6 +511,8 @@ cfg_if::cfg_if! { mod sealed { use super::*; + pub trait Mode {} + pub trait Instance { fn regs() -> pac::uart::Uart; } @@ -428,6 +522,21 @@ mod sealed { pub trait RtsPin {} } +pub trait Mode: sealed::Mode {} + +macro_rules! impl_mode { + ($name:ident) => { + impl sealed::Mode for $name {} + impl Mode for $name {} + }; +} + +pub struct Blocking; +pub struct Async; + +impl_mode!(Blocking); +impl_mode!(Async); + pub trait Instance: sealed::Instance {} macro_rules! impl_instance { From 3bbfc11f45149c5624e2ff6370f8e71b956402af Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 18 Aug 2022 20:30:24 +0200 Subject: [PATCH 02/16] Stop active DMA transfer on drop --- embassy-rp/src/dma.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index dfa047a2..bf15e1f4 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -30,9 +30,6 @@ pub fn copy<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: &[W] w.set_en(true); }); - // FIXME: - while p.ctrl_trig().read().busy() {} - compiler_fence(Ordering::SeqCst); } Transfer::new(ch) @@ -51,8 +48,11 @@ impl<'a, C: Channel> Transfer<'a, C> { impl<'a, C: Channel> Drop for Transfer<'a, C> { fn drop(&mut self) { - // self.channel.request_stop(); - // while self.channel.is_running() {} + let p = self.channel.regs(); + unsafe { + p.ctrl_trig().write(|w| w.set_en(false)); + while p.ctrl_trig().read().busy() {} + } } } @@ -64,7 +64,7 @@ impl<'a, C: Channel> Future for Transfer<'a, C> { // if self.channel.is_running() { // Poll::Pending // } else { - Poll::Ready(()) + Poll::Ready(()) // } } } @@ -87,9 +87,7 @@ pub trait Channel: Peripheral

+ sealed::Channel + Into + S } fn degrade(self) -> AnyChannel { - AnyChannel { - number: self.number(), - } + AnyChannel { number: self.number() } } } From 55a63a5417ccfd2ca2792215920de14519a3d27b Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 18 Aug 2022 20:30:50 +0200 Subject: [PATCH 03/16] Attempt to implement future for DMA transfer --- embassy-rp/src/dma.rs | 49 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index bf15e1f4..ec09a769 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -1,8 +1,9 @@ use core::pin::Pin; use core::sync::atomic::{compiler_fence, Ordering}; -use core::task::{Context, Poll}; +use core::task::{Context, Poll, Waker}; use embassy_hal_common::{impl_peripheral, into_ref, Peripheral, PeripheralRef}; +use embassy_util::waitqueue::AtomicWaker; use futures::Future; use crate::pac::dma::vals; @@ -60,15 +61,41 @@ impl<'a, C: Channel> Unpin for Transfer<'a, C> {} impl<'a, C: Channel> Future for Transfer<'a, C> { type Output = (); fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - // self.channel.set_waker(cx.waker()); - // if self.channel.is_running() { - // Poll::Pending - // } else { - Poll::Ready(()) - // } + self.channel.set_waker(cx.waker()); + + if self.channel.is_running() { + Poll::Pending + } else { + Poll::Ready(()) + } } } +struct ChannelState { + waker: AtomicWaker, +} + +impl ChannelState { + const fn new() -> Self { + Self { + waker: AtomicWaker::new(), + } + } +} + +struct State { + channels: [ChannelState; 12], +} + +impl State { + const fn new() -> Self { + const CH: ChannelState = ChannelState::new(); + Self { channels: [CH; 12] } + } +} + +static STATE: State = State::new(); + pub struct NoDma; impl_peripheral!(NoDma); @@ -86,6 +113,14 @@ pub trait Channel: Peripheral

+ sealed::Channel + Into + S pac::DMA.ch(self.number() as _) } + fn is_running(&self) -> bool { + self.regs().ctrl_trig().read().en() + } + + fn set_waker(&self, waker: &Waker) { + STATE.channels[self.number() as usize].waker.register(waker); + } + fn degrade(self) -> AnyChannel { AnyChannel { number: self.number() } } From 9c9b7b1a66dc90a9183886867811d2db57df714c Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 18 Aug 2022 20:34:55 +0200 Subject: [PATCH 04/16] Remove unneeded NoDma struct --- embassy-rp/src/dma.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index ec09a769..531ed802 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -96,10 +96,6 @@ impl State { static STATE: State = State::new(); -pub struct NoDma; - -impl_peripheral!(NoDma); - mod sealed { pub trait Channel {} From 1d49b3444f2bd3e049f19a72da63804192ee0402 Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 18 Aug 2022 21:09:50 +0200 Subject: [PATCH 05/16] Add DMA read + write functions --- embassy-rp/src/dma.rs | 48 +++++++++++++++++++++++++++++++++++------- embassy-rp/src/uart.rs | 4 ++-- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 531ed802..cfaa6dd3 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -5,26 +5,45 @@ use core::task::{Context, Poll, Waker}; use embassy_hal_common::{impl_peripheral, into_ref, Peripheral, PeripheralRef}; use embassy_util::waitqueue::AtomicWaker; use futures::Future; +use pac::dma::vals::DataSize; use crate::pac::dma::vals; use crate::{pac, peripherals}; -pub fn copy<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: &[W], to: &mut [W]) -> Transfer<'a, C> { - assert!(from.len() == to.len()); +pub(crate) fn read<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: *const W, to: *mut [W]) -> Transfer<'a, C> { + let (ptr, len) = crate::dma::slice_ptr_parts_mut(to); + copy(ch, from as *const u32, ptr as *mut u32, len, W::size()) +} +pub(crate) fn write<'a, C: Channel, W: Word>( + ch: impl Peripheral

+ 'a, + from: *const [W], + to: *mut W, +) -> Transfer<'a, C> { + let (from_ptr, len) = crate::dma::slice_ptr_parts(from); + copy(ch, from_ptr as *const u32, to as *mut u32, len, W::size()) +} + +fn copy<'a, C: Channel>( + ch: impl Peripheral

+ 'a, + from: *const u32, + to: *mut u32, + len: usize, + data_size: DataSize, +) -> Transfer<'a, C> { into_ref!(ch); unsafe { let p = ch.regs(); - p.read_addr().write_value(from.as_ptr() as u32); - p.write_addr().write_value(to.as_mut_ptr() as u32); - p.trans_count().write_value(from.len() as u32); + p.read_addr().write_value(from as u32); + p.write_addr().write_value(to as u32); + p.trans_count().write_value(len as u32); compiler_fence(Ordering::SeqCst); p.ctrl_trig().write(|w| { - w.set_data_size(W::size()); + w.set_data_size(data_size); w.set_incr_read(true); w.set_incr_write(true); w.set_chain_to(ch.number()); @@ -60,7 +79,7 @@ impl<'a, C: Channel> Drop for Transfer<'a, C> { impl<'a, C: Channel> Unpin for Transfer<'a, C> {} impl<'a, C: Channel> Future for Transfer<'a, C> { type Output = (); - fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { self.channel.set_waker(cx.waker()); if self.channel.is_running() { @@ -110,13 +129,15 @@ pub trait Channel: Peripheral

+ sealed::Channel + Into + S } fn is_running(&self) -> bool { - self.regs().ctrl_trig().read().en() + unsafe { self.regs().ctrl_trig().read().en() } } fn set_waker(&self, waker: &Waker) { STATE.channels[self.number() as usize].waker.register(waker); } + fn on_irq() {} + fn degrade(self) -> AnyChannel { AnyChannel { number: self.number() } } @@ -177,6 +198,17 @@ macro_rules! channel { }; } +// TODO: replace transmutes with core::ptr::metadata once it's stable +#[allow(unused)] +pub(crate) fn slice_ptr_parts(slice: *const [T]) -> (usize, usize) { + unsafe { core::mem::transmute(slice) } +} + +#[allow(unused)] +pub(crate) fn slice_ptr_parts_mut(slice: *mut [T]) -> (usize, usize) { + unsafe { core::mem::transmute(slice) } +} + channel!(DMA_CH0, 0); channel!(DMA_CH1, 1); channel!(DMA_CH2, 2); diff --git a/embassy-rp/src/uart.rs b/embassy-rp/src/uart.rs index c1596960..09cadf03 100644 --- a/embassy-rp/src/uart.rs +++ b/embassy-rp/src/uart.rs @@ -128,7 +128,7 @@ impl<'d, T: Instance> UartTx<'d, T, Async> { } // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - let transfer = crate::dma::copy(ch, buffer, unsafe { T::regs().uartdr().ptr() }); + let transfer = crate::dma::write(ch, buffer, T::regs().uartdr().ptr() as *mut _); transfer.await; } Ok(()) @@ -178,7 +178,7 @@ impl<'d, T: Instance> UartRx<'d, T, Async> { } // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - let transfer = crate::dma::copy(ch, unsafe { T::regs().uartdr().ptr() }, buffer); + let transfer = crate::dma::read(ch, T::regs().uartdr().ptr() as *const _, buffer); transfer.await; } Ok(()) From debff0980d6a4c5527ebdaea6f91f32b63477bc5 Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 18 Aug 2022 21:14:57 +0200 Subject: [PATCH 06/16] Don't increment read address in DMA copy from peripherals --- embassy-rp/src/dma.rs | 12 +++++++++--- embassy-rp/src/uart.rs | 14 +++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index cfaa6dd3..8cf8c394 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -10,7 +10,11 @@ use pac::dma::vals::DataSize; use crate::pac::dma::vals; use crate::{pac, peripherals}; -pub(crate) fn read<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: *const W, to: *mut [W]) -> Transfer<'a, C> { +pub(crate) fn read<'a, C: Channel, W: Word>( + ch: impl Peripheral

+ 'a, + from: *const W, + to: *mut [W], +) -> Transfer<'a, C> { let (ptr, len) = crate::dma::slice_ptr_parts_mut(to); copy(ch, from as *const u32, ptr as *mut u32, len, W::size()) } @@ -44,7 +48,7 @@ fn copy<'a, C: Channel>( p.ctrl_trig().write(|w| { w.set_data_size(data_size); - w.set_incr_read(true); + w.set_incr_read(false); w.set_incr_write(true); w.set_chain_to(ch.number()); w.set_en(true); @@ -136,7 +140,9 @@ pub trait Channel: Peripheral

+ sealed::Channel + Into + S STATE.channels[self.number() as usize].waker.register(waker); } - fn on_irq() {} + fn on_irq() { + // FIXME: + } fn degrade(self) -> AnyChannel { AnyChannel { number: self.number() } diff --git a/embassy-rp/src/uart.rs b/embassy-rp/src/uart.rs index 09cadf03..03623a9f 100644 --- a/embassy-rp/src/uart.rs +++ b/embassy-rp/src/uart.rs @@ -441,15 +441,15 @@ mod eh1 { } } - impl<'d, T: Instance> embedded_hal_1::serial::ErrorType for Uart<'d, T> { + impl<'d, T: Instance, M: Mode> embedded_hal_1::serial::ErrorType for Uart<'d, T, M> { type Error = Error; } - impl<'d, T: Instance> embedded_hal_1::serial::ErrorType for UartTx<'d, T> { + impl<'d, T: Instance, M: Mode> embedded_hal_1::serial::ErrorType for UartTx<'d, T, M> { type Error = Error; } - impl<'d, T: Instance> embedded_hal_1::serial::ErrorType for UartRx<'d, T> { + impl<'d, T: Instance, M: Mode> embedded_hal_1::serial::ErrorType for UartRx<'d, T, M> { type Error = Error; } } @@ -458,7 +458,7 @@ cfg_if::cfg_if! { if #[cfg(all(feature = "unstable-traits", feature = "nightly", feature = "_todo_embedded_hal_serial"))] { use core::future::Future; - impl<'d, T: Instance> embedded_hal_async::serial::Write for UartTx<'d, T> + impl<'d, T: Instance, M: Mode> embedded_hal_async::serial::Write for UartTx<'d, T, M> { type WriteFuture<'a> = impl Future> + 'a where Self: 'a; @@ -473,7 +473,7 @@ cfg_if::cfg_if! { } } - impl<'d, T: Instance> embedded_hal_async::serial::Read for UartRx<'d, T> + impl<'d, T: Instance, M: Mode> embedded_hal_async::serial::Read for UartRx<'d, T, M> { type ReadFuture<'a> = impl Future> + 'a where Self: 'a; @@ -482,7 +482,7 @@ cfg_if::cfg_if! { } } - impl<'d, T: Instance> embedded_hal_async::serial::Write for Uart<'d, T> + impl<'d, T: Instance, M: Mode> embedded_hal_async::serial::Write for Uart<'d, T, M> { type WriteFuture<'a> = impl Future> + 'a where Self: 'a; @@ -497,7 +497,7 @@ cfg_if::cfg_if! { } } - impl<'d, T: Instance> embedded_hal_async::serial::Read for Uart<'d, T> + impl<'d, T: Instance, M: Mode> embedded_hal_async::serial::Read for Uart<'d, T, M> { type ReadFuture<'a> = impl Future> + 'a where Self: 'a; From aa586fe1dee309808fca34500812bf33f2a5c558 Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 18 Aug 2022 21:27:37 +0200 Subject: [PATCH 07/16] Simplify waker storage for DMA state --- embassy-rp/src/dma.rs | 45 ++++++------------------------------------- 1 file changed, 6 insertions(+), 39 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 8cf8c394..a56d77c1 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -1,6 +1,6 @@ use core::pin::Pin; use core::sync::atomic::{compiler_fence, Ordering}; -use core::task::{Context, Poll, Waker}; +use core::task::{Context, Poll}; use embassy_hal_common::{impl_peripheral, into_ref, Peripheral, PeripheralRef}; use embassy_util::waitqueue::AtomicWaker; @@ -84,9 +84,9 @@ impl<'a, C: Channel> Unpin for Transfer<'a, C> {} impl<'a, C: Channel> Future for Transfer<'a, C> { type Output = (); fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - self.channel.set_waker(cx.waker()); + CHANNEL_WAKERS[self.channel.number() as usize].register(cx.waker()); - if self.channel.is_running() { + if unsafe { self.channel.regs().ctrl_trig().read().en() } { Poll::Pending } else { Poll::Ready(()) @@ -94,30 +94,9 @@ impl<'a, C: Channel> Future for Transfer<'a, C> { } } -struct ChannelState { - waker: AtomicWaker, -} - -impl ChannelState { - const fn new() -> Self { - Self { - waker: AtomicWaker::new(), - } - } -} - -struct State { - channels: [ChannelState; 12], -} - -impl State { - const fn new() -> Self { - const CH: ChannelState = ChannelState::new(); - Self { channels: [CH; 12] } - } -} - -static STATE: State = State::new(); +const CHANNEL_COUNT: usize = 12; +const NEW_AW: AtomicWaker = AtomicWaker::new(); +static CHANNEL_WAKERS: [AtomicWaker; CHANNEL_COUNT] = [NEW_AW; CHANNEL_COUNT]; mod sealed { pub trait Channel {} @@ -132,18 +111,6 @@ pub trait Channel: Peripheral

+ sealed::Channel + Into + S pac::DMA.ch(self.number() as _) } - fn is_running(&self) -> bool { - unsafe { self.regs().ctrl_trig().read().en() } - } - - fn set_waker(&self, waker: &Waker) { - STATE.channels[self.number() as usize].waker.register(waker); - } - - fn on_irq() { - // FIXME: - } - fn degrade(self) -> AnyChannel { AnyChannel { number: self.number() } } From a29972413bcfb30e6e28766491229c6ac38eed22 Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 19 Aug 2022 08:48:52 +0200 Subject: [PATCH 08/16] Fix uart rp2040 blocking example --- examples/rp/src/bin/uart.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/rp/src/bin/uart.rs b/examples/rp/src/bin/uart.rs index c63b31ca..05177a6b 100644 --- a/examples/rp/src/bin/uart.rs +++ b/examples/rp/src/bin/uart.rs @@ -10,7 +10,7 @@ use {defmt_rtt as _, panic_probe as _}; async fn main(_spawner: Spawner) { let p = embassy_rp::init(Default::default()); let config = uart::Config::default(); - let mut uart = uart::Uart::new_with_rtscts(p.UART0, p.PIN_0, p.PIN_1, p.PIN_2, p.PIN_3, config); + let mut uart = uart::Uart::new_with_rtscts_blocking(p.UART0, p.PIN_0, p.PIN_1, p.PIN_3, p.PIN_2, config); uart.blocking_write("Hello World!\r\n".as_bytes()).unwrap(); loop { From 140ef4febf2a74a14be0b4421f7114c68ecba571 Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 19 Aug 2022 09:48:58 +0200 Subject: [PATCH 09/16] Add DMA_IRQ0 handling to Transfer --- embassy-rp/src/dma.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index a56d77c1..23cd8532 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -2,13 +2,28 @@ use core::pin::Pin; use core::sync::atomic::{compiler_fence, Ordering}; use core::task::{Context, Poll}; +use embassy_cortex_m::interrupt::{Interrupt, InterruptExt}; use embassy_hal_common::{impl_peripheral, into_ref, Peripheral, PeripheralRef}; use embassy_util::waitqueue::AtomicWaker; use futures::Future; use pac::dma::vals::DataSize; use crate::pac::dma::vals; -use crate::{pac, peripherals}; +use crate::{interrupt, pac, peripherals}; + +#[interrupt] +unsafe fn DMA_IRQ_0() { + let ints0 = pac::DMA.ints0().read().ints0(); + + critical_section::with(|_| { + for channel in 0..CHANNEL_COUNT { + if ints0 & (1 << channel) == 1 { + CHANNEL_WAKERS[channel].wake(); + } + } + pac::DMA.ints0().write(|w| w.set_ints0(ints0)); + }); +} pub(crate) fn read<'a, C: Channel, W: Word>( ch: impl Peripheral

+ 'a, @@ -66,6 +81,17 @@ pub(crate) struct Transfer<'a, C: Channel> { impl<'a, C: Channel> Transfer<'a, C> { pub(crate) fn new(channel: impl Peripheral

+ 'a) -> Self { into_ref!(channel); + + unsafe { + let irq = interrupt::DMA_IRQ_0::steal(); + irq.disable(); + irq.set_priority(interrupt::Priority::P6); + + pac::DMA.inte0().write(|w| w.set_inte0(1 << channel.number())); + + irq.enable(); + } + Self { channel } } } @@ -84,6 +110,8 @@ impl<'a, C: Channel> Unpin for Transfer<'a, C> {} impl<'a, C: Channel> Future for Transfer<'a, C> { type Output = (); fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // We need to register/re-register the waker for each poll because any + // calls to wake will deregister the waker. CHANNEL_WAKERS[self.channel.number() as usize].register(cx.waker()); if unsafe { self.channel.regs().ctrl_trig().read().en() } { From 331a64a4eac4accd91f6f9de8f77837181d8cab0 Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 19 Aug 2022 10:11:03 +0200 Subject: [PATCH 10/16] Add back public dma::copy, and correct dma incr settings for read/write --- embassy-rp/src/dma.rs | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 23cd8532..820ce005 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -25,30 +25,39 @@ unsafe fn DMA_IRQ_0() { }); } -pub(crate) fn read<'a, C: Channel, W: Word>( - ch: impl Peripheral

+ 'a, - from: *const W, - to: *mut [W], -) -> Transfer<'a, C> { +pub fn read<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: *const W, to: &mut [W]) -> Transfer<'a, C> { let (ptr, len) = crate::dma::slice_ptr_parts_mut(to); - copy(ch, from as *const u32, ptr as *mut u32, len, W::size()) + copy_inner(ch, from as *const u32, ptr as *mut u32, len, W::size(), false, true) } -pub(crate) fn write<'a, C: Channel, W: Word>( - ch: impl Peripheral

+ 'a, - from: *const [W], - to: *mut W, -) -> Transfer<'a, C> { +pub fn write<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: &[W], to: *mut W) -> Transfer<'a, C> { let (from_ptr, len) = crate::dma::slice_ptr_parts(from); - copy(ch, from_ptr as *const u32, to as *mut u32, len, W::size()) + copy_inner(ch, from_ptr as *const u32, to as *mut u32, len, W::size(), true, false) } -fn copy<'a, C: Channel>( +pub fn copy<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: &[W], to: &mut [W]) -> Transfer<'a, C> { + let (from_ptr, from_len) = crate::dma::slice_ptr_parts(from); + let (to_ptr, to_len) = crate::dma::slice_ptr_parts_mut(to); + assert_eq!(from_len, to_len); + copy_inner( + ch, + from_ptr as *const u32, + to_ptr as *mut u32, + from_len, + W::size(), + true, + true, + ) +} + +fn copy_inner<'a, C: Channel>( ch: impl Peripheral

+ 'a, from: *const u32, to: *mut u32, len: usize, data_size: DataSize, + incr_read: bool, + incr_write: bool, ) -> Transfer<'a, C> { into_ref!(ch); @@ -63,8 +72,8 @@ fn copy<'a, C: Channel>( p.ctrl_trig().write(|w| { w.set_data_size(data_size); - w.set_incr_read(false); - w.set_incr_write(true); + w.set_incr_read(incr_read); + w.set_incr_write(incr_write); w.set_chain_to(ch.number()); w.set_en(true); }); @@ -74,7 +83,7 @@ fn copy<'a, C: Channel>( Transfer::new(ch) } -pub(crate) struct Transfer<'a, C: Channel> { +pub struct Transfer<'a, C: Channel> { channel: PeripheralRef<'a, C>, } @@ -114,7 +123,7 @@ impl<'a, C: Channel> Future for Transfer<'a, C> { // calls to wake will deregister the waker. CHANNEL_WAKERS[self.channel.number() as usize].register(cx.waker()); - if unsafe { self.channel.regs().ctrl_trig().read().en() } { + if unsafe { self.channel.regs().ctrl_trig().read().busy() } { Poll::Pending } else { Poll::Ready(()) From 295af2a057beec74bb765f6e3dad28b44e30cb19 Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 19 Aug 2022 14:16:19 +0200 Subject: [PATCH 11/16] Fix bit checking in DMA irq --- embassy-rp/src/dma.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 820ce005..75ad640c 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -17,7 +17,7 @@ unsafe fn DMA_IRQ_0() { critical_section::with(|_| { for channel in 0..CHANNEL_COUNT { - if ints0 & (1 << channel) == 1 { + if ints0 & (1 << channel) == (1 << channel) { CHANNEL_WAKERS[channel].wake(); } } From f6c2e26372961747a0bc148d1675b1bc5dfb7e75 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 23 Aug 2022 12:28:17 +0200 Subject: [PATCH 12/16] Address code review comments --- embassy-rp/src/dma.rs | 50 +++++++++++++++++++++++++----------------- embassy-rp/src/lib.rs | 1 + embassy-rp/src/uart.rs | 34 ++++++++++++++-------------- 3 files changed, 47 insertions(+), 38 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 75ad640c..25e3ec8f 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -15,27 +15,47 @@ use crate::{interrupt, pac, peripherals}; unsafe fn DMA_IRQ_0() { let ints0 = pac::DMA.ints0().read().ints0(); - critical_section::with(|_| { - for channel in 0..CHANNEL_COUNT { - if ints0 & (1 << channel) == (1 << channel) { - CHANNEL_WAKERS[channel].wake(); - } + for channel in 0..CHANNEL_COUNT { + if ints0 & (1 << channel) == (1 << channel) { + CHANNEL_WAKERS[channel].wake(); } - pac::DMA.ints0().write(|w| w.set_ints0(ints0)); - }); + } + pac::DMA.ints0().write(|w| w.set_ints0(ints0)); } -pub fn read<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: *const W, to: &mut [W]) -> Transfer<'a, C> { +pub(crate) unsafe fn init() { + let irq = interrupt::DMA_IRQ_0::steal(); + irq.disable(); + irq.set_priority(interrupt::Priority::P6); + + pac::DMA.inte0().write(|w| w.set_inte0(0xFFFF)); + + irq.enable(); +} + +pub unsafe fn read<'a, C: Channel, W: Word>( + ch: impl Peripheral

+ 'a, + from: *const W, + to: &mut [W], +) -> Transfer<'a, C> { let (ptr, len) = crate::dma::slice_ptr_parts_mut(to); copy_inner(ch, from as *const u32, ptr as *mut u32, len, W::size(), false, true) } -pub fn write<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: &[W], to: *mut W) -> Transfer<'a, C> { +pub unsafe fn write<'a, C: Channel, W: Word>( + ch: impl Peripheral

+ 'a, + from: &[W], + to: *mut W, +) -> Transfer<'a, C> { let (from_ptr, len) = crate::dma::slice_ptr_parts(from); copy_inner(ch, from_ptr as *const u32, to as *mut u32, len, W::size(), true, false) } -pub fn copy<'a, C: Channel, W: Word>(ch: impl Peripheral

+ 'a, from: &[W], to: &mut [W]) -> Transfer<'a, C> { +pub unsafe fn copy<'a, C: Channel, W: Word>( + ch: impl Peripheral

+ 'a, + from: &[W], + to: &mut [W], +) -> Transfer<'a, C> { let (from_ptr, from_len) = crate::dma::slice_ptr_parts(from); let (to_ptr, to_len) = crate::dma::slice_ptr_parts_mut(to); assert_eq!(from_len, to_len); @@ -91,16 +111,6 @@ impl<'a, C: Channel> Transfer<'a, C> { pub(crate) fn new(channel: impl Peripheral

+ 'a) -> Self { into_ref!(channel); - unsafe { - let irq = interrupt::DMA_IRQ_0::steal(); - irq.disable(); - irq.set_priority(interrupt::Priority::P6); - - pac::DMA.inte0().write(|w| w.set_inte0(1 << channel.number())); - - irq.enable(); - } - Self { channel } } } diff --git a/embassy-rp/src/lib.rs b/embassy-rp/src/lib.rs index 8c053a4f..6db77b8c 100644 --- a/embassy-rp/src/lib.rs +++ b/embassy-rp/src/lib.rs @@ -105,6 +105,7 @@ pub fn init(_config: config::Config) -> Peripherals { unsafe { clocks::init(); timer::init(); + dma::init(); } peripherals diff --git a/embassy-rp/src/uart.rs b/embassy-rp/src/uart.rs index 03623a9f..9d94dcf2 100644 --- a/embassy-rp/src/uart.rs +++ b/embassy-rp/src/uart.rs @@ -120,17 +120,16 @@ impl<'d, T: Instance, M: Mode> UartTx<'d, T, M> { impl<'d, T: Instance> UartTx<'d, T, Async> { pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { - if let Some(ch) = &mut self.tx_dma { - unsafe { - T::regs().uartdmacr().modify(|reg| { - reg.set_txdmae(true); - }); - } + let ch = self.tx_dma.as_mut().unwrap(); + let transfer = unsafe { + T::regs().uartdmacr().modify(|reg| { + reg.set_txdmae(true); + }); // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - let transfer = crate::dma::write(ch, buffer, T::regs().uartdr().ptr() as *mut _); - transfer.await; - } + crate::dma::write(ch, buffer, T::regs().uartdr().ptr() as *mut _) + }; + transfer.await; Ok(()) } } @@ -170,17 +169,16 @@ impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> { impl<'d, T: Instance> UartRx<'d, T, Async> { pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { - if let Some(ch) = &mut self.rx_dma { - unsafe { - T::regs().uartdmacr().modify(|reg| { - reg.set_rxdmae(true); - }); - } + let ch = self.rx_dma.as_mut().unwrap(); + let transfer = unsafe { + T::regs().uartdmacr().modify(|reg| { + reg.set_rxdmae(true); + }); // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - let transfer = crate::dma::read(ch, T::regs().uartdr().ptr() as *const _, buffer); - transfer.await; - } + crate::dma::read(ch, T::regs().uartdr().ptr() as *const _, buffer) + }; + transfer.await; Ok(()) } } From 7e3ce2c90befe19ea30c6a784710f6cebf5a48f2 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 23 Aug 2022 13:20:36 +0200 Subject: [PATCH 13/16] Abort DMA operation when dropping a Transfer, and panic on DMA errors --- embassy-rp/src/dma.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 25e3ec8f..6423d193 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -14,8 +14,18 @@ use crate::{interrupt, pac, peripherals}; #[interrupt] unsafe fn DMA_IRQ_0() { let ints0 = pac::DMA.ints0().read().ints0(); - for channel in 0..CHANNEL_COUNT { + let ctrl_trig = pac::DMA.ch(channel).ctrl_trig().read(); + if ctrl_trig.ahb_error() { + panic!("DMA: ahb error on DMA_0 channel {}", channel); + } + if ctrl_trig.read_error() { + panic!("DMA: read error on DMA_0 channel {}", channel); + } + if ctrl_trig.write_error() { + panic!("DMA: write error on DMA_0 channel {}", channel); + } + if ints0 & (1 << channel) == (1 << channel) { CHANNEL_WAKERS[channel].wake(); } @@ -119,7 +129,9 @@ impl<'a, C: Channel> Drop for Transfer<'a, C> { fn drop(&mut self) { let p = self.channel.regs(); unsafe { - p.ctrl_trig().write(|w| w.set_en(false)); + pac::DMA + .chan_abort() + .modify(|m| m.set_chan_abort(1 << self.channel.number())); while p.ctrl_trig().read().busy() {} } } From 594a64b3bfd5757f98ea4118fcaf62f59fc2157b Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 23 Aug 2022 13:28:14 +0200 Subject: [PATCH 14/16] Change to using embassy-sync --- embassy-rp/src/dma.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 6423d193..137e9a0c 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -4,7 +4,7 @@ use core::task::{Context, Poll}; use embassy_cortex_m::interrupt::{Interrupt, InterruptExt}; use embassy_hal_common::{impl_peripheral, into_ref, Peripheral, PeripheralRef}; -use embassy_util::waitqueue::AtomicWaker; +use embassy_sync::waitqueue::AtomicWaker; use futures::Future; use pac::dma::vals::DataSize; @@ -36,7 +36,7 @@ unsafe fn DMA_IRQ_0() { pub(crate) unsafe fn init() { let irq = interrupt::DMA_IRQ_0::steal(); irq.disable(); - irq.set_priority(interrupt::Priority::P6); + irq.set_priority(interrupt::Priority::P3); pac::DMA.inte0().write(|w| w.set_inte0(0xFFFF)); From b88ef032141e87411dabb5d90ef488f5d6af2285 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 23 Aug 2022 13:46:48 +0200 Subject: [PATCH 15/16] Only check for ahb error in DMA --- embassy-rp/src/dma.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 137e9a0c..69482374 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -17,13 +17,7 @@ unsafe fn DMA_IRQ_0() { for channel in 0..CHANNEL_COUNT { let ctrl_trig = pac::DMA.ch(channel).ctrl_trig().read(); if ctrl_trig.ahb_error() { - panic!("DMA: ahb error on DMA_0 channel {}", channel); - } - if ctrl_trig.read_error() { - panic!("DMA: read error on DMA_0 channel {}", channel); - } - if ctrl_trig.write_error() { - panic!("DMA: write error on DMA_0 channel {}", channel); + panic!("DMA: error on DMA_0 channel {}", channel); } if ints0 & (1 << channel) == (1 << channel) { From bd27b9080fd9019e69e84fd30894a71db9fd61b5 Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 26 Aug 2022 12:50:12 +0200 Subject: [PATCH 16/16] Add HIL tests of DMA & UART, and correctly set DREQ for uart DMA --- embassy-rp/src/dma.rs | 31 +++++++++++-- embassy-rp/src/uart.rs | 71 +++++++++++++++++++++--------- tests/rp/src/bin/dma_copy_async.rs | 41 +++++++++++++++++ tests/rp/src/bin/uart.rs | 32 ++++++++++++++ tests/rp/src/bin/uart_dma.rs | 32 ++++++++++++++ 5 files changed, 184 insertions(+), 23 deletions(-) create mode 100644 tests/rp/src/bin/dma_copy_async.rs create mode 100644 tests/rp/src/bin/uart.rs create mode 100644 tests/rp/src/bin/uart_dma.rs diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 69482374..75d7492e 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -41,18 +41,38 @@ pub unsafe fn read<'a, C: Channel, W: Word>( ch: impl Peripheral

+ 'a, from: *const W, to: &mut [W], + dreq: u8, ) -> Transfer<'a, C> { - let (ptr, len) = crate::dma::slice_ptr_parts_mut(to); - copy_inner(ch, from as *const u32, ptr as *mut u32, len, W::size(), false, true) + let (to_ptr, len) = crate::dma::slice_ptr_parts_mut(to); + copy_inner( + ch, + from as *const u32, + to_ptr as *mut u32, + len, + W::size(), + false, + true, + dreq, + ) } pub unsafe fn write<'a, C: Channel, W: Word>( ch: impl Peripheral

+ 'a, from: &[W], to: *mut W, + dreq: u8, ) -> Transfer<'a, C> { let (from_ptr, len) = crate::dma::slice_ptr_parts(from); - copy_inner(ch, from_ptr as *const u32, to as *mut u32, len, W::size(), true, false) + copy_inner( + ch, + from_ptr as *const u32, + to as *mut u32, + len, + W::size(), + true, + false, + dreq, + ) } pub unsafe fn copy<'a, C: Channel, W: Word>( @@ -71,6 +91,7 @@ pub unsafe fn copy<'a, C: Channel, W: Word>( W::size(), true, true, + vals::TreqSel::PERMANENT.0, ) } @@ -82,6 +103,7 @@ fn copy_inner<'a, C: Channel>( data_size: DataSize, incr_read: bool, incr_write: bool, + dreq: u8, ) -> Transfer<'a, C> { into_ref!(ch); @@ -95,6 +117,9 @@ fn copy_inner<'a, C: Channel>( compiler_fence(Ordering::SeqCst); p.ctrl_trig().write(|w| { + // TODO: Add all DREQ options to pac vals::TreqSel, and use + // `set_treq:sel` + w.0 = ((dreq as u32) & 0x3f) << 15usize; w.set_data_size(data_size); w.set_incr_read(incr_read); w.set_incr_write(incr_write); diff --git a/embassy-rp/src/uart.rs b/embassy-rp/src/uart.rs index 9d94dcf2..4a3c7a0c 100644 --- a/embassy-rp/src/uart.rs +++ b/embassy-rp/src/uart.rs @@ -113,7 +113,7 @@ impl<'d, T: Instance, M: Mode> UartTx<'d, T, M> { pub fn blocking_flush(&mut self) -> Result<(), Error> { let r = T::regs(); - unsafe { while r.uartfr().read().txff() {} } + unsafe { while !r.uartfr().read().txfe() {} } Ok(()) } } @@ -127,7 +127,7 @@ impl<'d, T: Instance> UartTx<'d, T, Async> { }); // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - crate::dma::write(ch, buffer, T::regs().uartdr().ptr() as *mut _) + crate::dma::write(ch, buffer, T::regs().uartdr().ptr() as *mut _, T::TX_DREQ) }; transfer.await; Ok(()) @@ -147,6 +147,10 @@ impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> { unsafe { for b in buffer { *b = loop { + if r.uartfr().read().rxfe() { + continue; + } + let dr = r.uartdr().read(); if dr.oe() { @@ -157,7 +161,7 @@ impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> { return Err(Error::Parity); } else if dr.fe() { return Err(Error::Framing); - } else if dr.fe() { + } else { break dr.data(); } }; @@ -176,7 +180,7 @@ impl<'d, T: Instance> UartRx<'d, T, Async> { }); // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - crate::dma::read(ch, T::regs().uartdr().ptr() as *const _, buffer) + crate::dma::read(ch, T::regs().uartdr().ptr() as *const _, buffer, T::RX_DREQ) }; transfer.await; Ok(()) @@ -282,6 +286,30 @@ impl<'d, T: Instance, M: Mode> Uart<'d, T, M> { unsafe { let r = T::regs(); + tx.io().ctrl().write(|w| w.set_funcsel(2)); + rx.io().ctrl().write(|w| w.set_funcsel(2)); + + tx.pad_ctrl().write(|w| { + w.set_ie(true); + }); + + rx.pad_ctrl().write(|w| { + w.set_ie(true); + }); + + if let Some(pin) = &cts { + pin.io().ctrl().write(|w| w.set_funcsel(2)); + pin.pad_ctrl().write(|w| { + w.set_ie(true); + }); + } + if let Some(pin) = &rts { + pin.io().ctrl().write(|w| w.set_funcsel(2)); + pin.pad_ctrl().write(|w| { + w.set_ie(true); + }); + } + let clk_base = crate::clocks::clk_peri_freq(); let baud_rate_div = (8 * clk_base) / config.baudrate; @@ -302,10 +330,14 @@ impl<'d, T: Instance, M: Mode> Uart<'d, T, M> { let (pen, eps) = match config.parity { Parity::ParityNone => (false, false), - Parity::ParityEven => (true, true), Parity::ParityOdd => (true, false), + Parity::ParityEven => (true, true), }; + // PL011 needs a (dummy) line control register write to latch in the + // divisors. We don't want to actually change LCR contents here. + r.uartlcr_h().modify(|_| {}); + r.uartlcr_h().write(|w| { w.set_wlen(config.data_bits.bits()); w.set_stp2(config.stop_bits == StopBits::STOP2); @@ -321,15 +353,6 @@ impl<'d, T: Instance, M: Mode> Uart<'d, T, M> { w.set_ctsen(cts.is_some()); w.set_rtsen(rts.is_some()); }); - - tx.io().ctrl().write(|w| w.set_funcsel(2)); - rx.io().ctrl().write(|w| w.set_funcsel(2)); - if let Some(pin) = &cts { - pin.io().ctrl().write(|w| w.set_funcsel(2)); - } - if let Some(pin) = &rts { - pin.io().ctrl().write(|w| w.set_funcsel(2)); - } } Self { @@ -377,6 +400,10 @@ mod eh02 { fn read(&mut self) -> Result> { let r = T::regs(); unsafe { + if r.uartfr().read().rxfe() { + return Err(nb::Error::WouldBlock); + } + let dr = r.uartdr().read(); if dr.oe() { @@ -387,10 +414,8 @@ mod eh02 { Err(nb::Error::Other(Error::Parity)) } else if dr.fe() { Err(nb::Error::Other(Error::Framing)) - } else if dr.fe() { - Ok(dr.data()) } else { - Err(nb::Error::WouldBlock) + Ok(dr.data()) } } } @@ -512,6 +537,9 @@ mod sealed { pub trait Mode {} pub trait Instance { + const TX_DREQ: u8; + const RX_DREQ: u8; + fn regs() -> pac::uart::Uart; } pub trait TxPin {} @@ -538,8 +566,11 @@ impl_mode!(Async); pub trait Instance: sealed::Instance {} macro_rules! impl_instance { - ($inst:ident, $irq:ident) => { + ($inst:ident, $irq:ident, $tx_dreq:expr, $rx_dreq:expr) => { impl sealed::Instance for peripherals::$inst { + const TX_DREQ: u8 = $tx_dreq; + const RX_DREQ: u8 = $rx_dreq; + fn regs() -> pac::uart::Uart { pac::$inst } @@ -548,8 +579,8 @@ macro_rules! impl_instance { }; } -impl_instance!(UART0, UART0); -impl_instance!(UART1, UART1); +impl_instance!(UART0, UART0, 20, 21); +impl_instance!(UART1, UART1, 22, 23); pub trait TxPin: sealed::TxPin + crate::gpio::Pin {} pub trait RxPin: sealed::RxPin + crate::gpio::Pin {} diff --git a/tests/rp/src/bin/dma_copy_async.rs b/tests/rp/src/bin/dma_copy_async.rs new file mode 100644 index 00000000..c53f644b --- /dev/null +++ b/tests/rp/src/bin/dma_copy_async.rs @@ -0,0 +1,41 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +use defmt::{assert_eq, *}; +use embassy_executor::Spawner; +use embassy_rp::dma::copy; +use {defmt_rtt as _, panic_probe as _}; + +#[embassy_executor::main] +async fn main(_spawner: Spawner) { + let p = embassy_rp::init(Default::default()); + info!("Hello World!"); + + // Check `u8` copy + { + let data: [u8; 2] = [0xC0, 0xDE]; + let mut buf = [0; 2]; + unsafe { copy(p.DMA_CH0, &data, &mut buf).await }; + assert_eq!(buf, data); + } + + // Check `u16` copy + { + let data: [u16; 2] = [0xC0BE, 0xDEAD]; + let mut buf = [0; 2]; + unsafe { copy(p.DMA_CH1, &data, &mut buf).await }; + assert_eq!(buf, data); + } + + // Check `u32` copy + { + let data: [u32; 2] = [0xC0BEDEAD, 0xDEADAAFF]; + let mut buf = [0; 2]; + unsafe { copy(p.DMA_CH2, &data, &mut buf).await }; + assert_eq!(buf, data); + } + + info!("Test OK"); + cortex_m::asm::bkpt(); +} diff --git a/tests/rp/src/bin/uart.rs b/tests/rp/src/bin/uart.rs new file mode 100644 index 00000000..92f61464 --- /dev/null +++ b/tests/rp/src/bin/uart.rs @@ -0,0 +1,32 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +use defmt::{assert_eq, *}; +use embassy_executor::Spawner; +use embassy_rp::uart::{Config, Uart}; +use {defmt_rtt as _, panic_probe as _}; + +#[embassy_executor::main] +async fn main(_spawner: Spawner) { + let p = embassy_rp::init(Default::default()); + info!("Hello World!"); + + let (tx, rx, uart) = (p.PIN_0, p.PIN_1, p.UART0); + + let config = Config::default(); + let mut uart = Uart::new_blocking(uart, tx, rx, config); + + // We can't send too many bytes, they have to fit in the FIFO. + // This is because we aren't sending+receiving at the same time. + + let data = [0xC0, 0xDE]; + uart.blocking_write(&data).unwrap(); + + let mut buf = [0; 2]; + uart.blocking_read(&mut buf).unwrap(); + assert_eq!(buf, data); + + info!("Test OK"); + cortex_m::asm::bkpt(); +} diff --git a/tests/rp/src/bin/uart_dma.rs b/tests/rp/src/bin/uart_dma.rs new file mode 100644 index 00000000..963c2270 --- /dev/null +++ b/tests/rp/src/bin/uart_dma.rs @@ -0,0 +1,32 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +use defmt::{assert_eq, *}; +use embassy_executor::Spawner; +use embassy_rp::uart::{Config, Uart}; +use {defmt_rtt as _, panic_probe as _}; + +#[embassy_executor::main] +async fn main(_spawner: Spawner) { + let p = embassy_rp::init(Default::default()); + info!("Hello World!"); + + let (tx, rx, uart) = (p.PIN_0, p.PIN_1, p.UART0); + + let config = Config::default(); + let mut uart = Uart::new(uart, tx, rx, p.DMA_CH0, p.DMA_CH1, config); + + // We can't send too many bytes, they have to fit in the FIFO. + // This is because we aren't sending+receiving at the same time. + + let data = [0xC0, 0xDE]; + uart.write(&data).await.unwrap(); + + let mut buf = [0; 2]; + uart.read(&mut buf).await.unwrap(); + assert_eq!(buf, data); + + info!("Test OK"); + cortex_m::asm::bkpt(); +}