From 49455792cb5406f3e2df0f3850c0d650965e6b2b Mon Sep 17 00:00:00 2001 From: Rasmus Melchior Jacobsen Date: Wed, 26 Apr 2023 10:51:23 +0200 Subject: [PATCH 01/12] Ring-buffered uart rx with one-period overrun detection --- embassy-stm32/src/dma/dma.rs | 186 +++++++- embassy-stm32/src/dma/mod.rs | 1 + embassy-stm32/src/dma/ringbuffer.rs | 433 +++++++++++++++++++ embassy-stm32/src/usart/mod.rs | 43 +- embassy-stm32/src/usart/rx_ringbuffered.rs | 286 ++++++++++++ tests/stm32/Cargo.toml | 2 + tests/stm32/src/bin/usart_rx_ringbuffered.rs | 188 ++++++++ tests/utils/Cargo.toml | 10 + tests/utils/src/bin/saturate_serial.rs | 52 +++ 9 files changed, 1177 insertions(+), 24 deletions(-) create mode 100644 embassy-stm32/src/dma/ringbuffer.rs create mode 100644 embassy-stm32/src/usart/rx_ringbuffered.rs create mode 100644 tests/stm32/src/bin/usart_rx_ringbuffered.rs create mode 100644 tests/utils/Cargo.toml create mode 100644 tests/utils/src/bin/saturate_serial.rs diff --git a/embassy-stm32/src/dma/dma.rs b/embassy-stm32/src/dma/dma.rs index ef1d27573..17d82fe2d 100644 --- a/embassy-stm32/src/dma/dma.rs +++ b/embassy-stm32/src/dma/dma.rs @@ -9,6 +9,7 @@ use embassy_hal_common::{into_ref, Peripheral, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; use pac::dma::regs; +use super::ringbuffer::{DmaCtrl, DmaRingBuffer, OverrunError}; use super::word::{Word, WordSize}; use super::Dir; use crate::_generated::DMA_CHANNEL_COUNT; @@ -445,7 +446,6 @@ impl<'a, C: Channel> Future for Transfer<'a, C> { // ================================== -#[must_use = "futures do nothing unless you `.await` or poll them"] pub struct DoubleBuffered<'a, C: Channel, W: Word> { channel: PeripheralRef<'a, C>, _phantom: PhantomData, @@ -578,15 +578,6 @@ impl<'a, C: Channel, W: Word> DoubleBuffered<'a, C, W> { let ch = self.channel.regs().st(self.channel.num()); unsafe { ch.ndtr().read() }.ndt() } - - pub fn blocking_wait(mut self) { - while self.is_running() {} - - // "Subsequent reads and writes cannot be moved ahead of preceding reads." - fence(Ordering::SeqCst); - - core::mem::forget(self); - } } impl<'a, C: Channel, W: Word> Drop for DoubleBuffered<'a, C, W> { @@ -598,3 +589,178 @@ impl<'a, C: Channel, W: Word> Drop for DoubleBuffered<'a, C, W> { fence(Ordering::SeqCst); } } + +// ============================== + +impl DmaCtrl for C { + fn tcif(&self) -> bool { + let channel_number = self.num(); + let dma = self.regs(); + let isrn = channel_number / 4; + let isrbit = channel_number % 4; + + unsafe { dma.isr(isrn).read() }.tcif(isrbit) + } + + fn clear_tcif(&mut self) { + let channel_number = self.num(); + let dma = self.regs(); + let isrn = channel_number / 4; + let isrbit = channel_number % 4; + + unsafe { + dma.ifcr(isrn).write(|w| { + w.set_tcif(isrbit, true); + }) + } + } + + fn ndtr(&self) -> usize { + let ch = self.regs().st(self.num()); + unsafe { ch.ndtr().read() }.ndt() as usize + } +} + +pub struct RingBuffer<'a, C: Channel, W: Word> { + cr: regs::Cr, + channel: PeripheralRef<'a, C>, + ringbuf: DmaRingBuffer<'a, W>, +} + +impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { + pub unsafe fn new_read( + channel: impl Peripheral

+ 'a, + _request: Request, + peri_addr: *mut W, + buffer: &'a mut [W], + options: TransferOptions, + ) -> Self { + into_ref!(channel); + + let len = buffer.len(); + assert!(len > 0 && len <= 0xFFFF); + + let dir = Dir::PeripheralToMemory; + let data_size = W::size(); + + let channel_number = channel.num(); + let dma = channel.regs(); + + // "Preceding reads and writes cannot be moved past subsequent writes." + fence(Ordering::SeqCst); + + let mut w = regs::Cr(0); + w.set_dir(dir.into()); + w.set_msize(data_size.into()); + w.set_psize(data_size.into()); + w.set_pl(vals::Pl::VERYHIGH); + w.set_minc(vals::Inc::INCREMENTED); + w.set_pinc(vals::Inc::FIXED); + w.set_teie(true); + w.set_tcie(false); + w.set_circ(vals::Circ::ENABLED); + #[cfg(dma_v1)] + w.set_trbuff(true); + #[cfg(dma_v2)] + w.set_chsel(_request); + w.set_pburst(options.pburst.into()); + w.set_mburst(options.mburst.into()); + w.set_pfctrl(options.flow_ctrl.into()); + w.set_en(true); + + let buffer_ptr = buffer.as_mut_ptr(); + let mut this = Self { + channel, + cr: w, + ringbuf: DmaRingBuffer::new(buffer), + }; + this.clear_irqs(); + + #[cfg(dmamux)] + super::dmamux::configure_dmamux(&mut *this.channel, _request); + + let ch = dma.st(channel_number); + ch.par().write_value(peri_addr as u32); + ch.m0ar().write_value(buffer_ptr as u32); + ch.ndtr().write_value(regs::Ndtr(len as _)); + ch.fcr().write(|w| { + if let Some(fth) = options.fifo_threshold { + // FIFO mode + w.set_dmdis(vals::Dmdis::DISABLED); + w.set_fth(fth.into()); + } else { + // Direct mode + w.set_dmdis(vals::Dmdis::ENABLED); + } + }); + + this + } + + pub fn start(&mut self) { + let ch = self.channel.regs().st(self.channel.num()); + unsafe { ch.cr().write_value(self.cr) } + } + + pub fn clear(&mut self) { + self.ringbuf.clear(); + } + + /// Read bytes from the ring buffer + /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + pub fn read(&mut self, buf: &mut [W]) -> Result { + self.ringbuf.read(&mut *self.channel, buf) + } + + fn clear_irqs(&mut self) { + let channel_number = self.channel.num(); + let dma = self.channel.regs(); + let isrn = channel_number / 4; + let isrbit = channel_number % 4; + + unsafe { + dma.ifcr(isrn).write(|w| { + w.set_tcif(isrbit, true); + w.set_teif(isrbit, true); + }) + } + } + + pub fn request_stop(&mut self) { + let ch = self.channel.regs().st(self.channel.num()); + + // Disable the channel. Keep the IEs enabled so the irqs still fire. + unsafe { + ch.cr().write(|w| { + w.set_teie(true); + w.set_tcie(true); + }) + } + } + + pub fn is_running(&mut self) -> bool { + let ch = self.channel.regs().st(self.channel.num()); + unsafe { ch.cr().read() }.en() + } + + /// Gets the total remaining transfers for the channel + /// Note: this will be zero for transfers that completed without cancellation. + pub fn get_remaining_transfers(&self) -> usize { + let ch = self.channel.regs().st(self.channel.num()); + unsafe { ch.ndtr().read() }.ndt() as usize + } + + pub fn set_ndtr(&mut self, ndtr: usize) { + self.ringbuf.ndtr = ndtr; + } +} + +impl<'a, C: Channel, W: Word> Drop for RingBuffer<'a, C, W> { + fn drop(&mut self) { + self.request_stop(); + while self.is_running() {} + + // "Subsequent reads and writes cannot be moved ahead of preceding reads." + fence(Ordering::SeqCst); + } +} diff --git a/embassy-stm32/src/dma/mod.rs b/embassy-stm32/src/dma/mod.rs index 3312ca752..3ac0d1b3d 100644 --- a/embassy-stm32/src/dma/mod.rs +++ b/embassy-stm32/src/dma/mod.rs @@ -21,6 +21,7 @@ pub use gpdma::*; #[cfg(dmamux)] mod dmamux; +pub(crate) mod ringbuffer; pub mod word; use core::mem; diff --git a/embassy-stm32/src/dma/ringbuffer.rs b/embassy-stm32/src/dma/ringbuffer.rs new file mode 100644 index 000000000..f9ace6018 --- /dev/null +++ b/embassy-stm32/src/dma/ringbuffer.rs @@ -0,0 +1,433 @@ +use core::ops::Range; +use core::sync::atomic::{compiler_fence, Ordering}; + +use super::word::Word; + +/// A "read-only" ring-buffer to be used together with the DMA controller which +/// writes in a circular way, "uncontrolled" to the buffer. +/// +/// A snapshot of the ring buffer state can be attained by setting the `ndtr` field +/// to the current register value. `ndtr` describes the current position of the DMA +/// write. +/// +/// # Safety +/// +/// The ring buffer controls the TCIF (transfer completed interrupt flag) to +/// detect buffer overruns, hence this interrupt must be disabled. +/// The buffer can detect overruns up to one period, that is, for a X byte buffer, +/// overruns can be detected if they happen from byte X+1 up to 2X. After this +/// point, overrunds may or may not be detected. +/// +/// # Buffer layout +/// +/// ```text +/// Without wraparound: With wraparound: +/// +/// + buf +--- NDTR ---+ + buf +---------- NDTR ----------+ +/// | | | | | | +/// v v v v v v +/// +-----------------------------------------+ +-----------------------------------------+ +/// |oooooooooooXXXXXXXXXXXXXXXXoooooooooooooo| |XXXXXXXXXXXXXooooooooooooXXXXXXXXXXXXXXXX| +/// +-----------------------------------------+ +-----------------------------------------+ +/// ^ ^ ^ ^ ^ ^ +/// | | | | | | +/// +- first --+ | +- end ------+ | +/// | | | | +/// +- end --------------------+ +- first ----------------+ +/// ``` +pub struct DmaRingBuffer<'a, W: Word> { + pub(crate) dma_buf: &'a mut [W], + first: usize, + pub ndtr: usize, + expect_next_read_to_wrap: bool, +} + +#[derive(Debug, PartialEq)] +pub struct OverrunError; + +pub trait DmaCtrl { + /// Get the NDTR register value, i.e. the space left in the underlying + /// buffer until the dma writer wraps. + fn ndtr(&self) -> usize; + + /// Read the transfer completed interrupt flag + /// This flag is set by the dma controller when NDTR is reloaded, + /// i.e. when the writing wraps. + fn tcif(&self) -> bool; + + /// Clear the transfer completed interrupt flag + fn clear_tcif(&mut self); +} + +impl<'a, W: Word> DmaRingBuffer<'a, W> { + pub fn new(dma_buf: &'a mut [W]) -> Self { + let ndtr = dma_buf.len(); + Self { + dma_buf, + first: 0, + ndtr, + expect_next_read_to_wrap: false, + } + } + + /// Reset the ring buffer to its initial state + pub fn clear(&mut self) { + self.first = 0; + self.ndtr = self.dma_buf.len(); + self.expect_next_read_to_wrap = false; + } + + /// The buffer end position + fn end(&self) -> usize { + self.dma_buf.len() - self.ndtr + } + + /// Returns whether the buffer is empty + #[allow(dead_code)] + pub fn is_empty(&self) -> bool { + self.first == self.end() + } + + /// The current number of bytes in the buffer + /// This may change at any time if dma is currently active + #[allow(dead_code)] + pub fn len(&self) -> usize { + // Read out a stable end (the dma periheral can change it at anytime) + let end = self.end(); + if self.first <= end { + // No wrap + end - self.first + } else { + self.dma_buf.len() - self.first + end + } + } + + /// Read bytes from the ring buffer + /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result { + let end = self.end(); + + compiler_fence(Ordering::SeqCst); + + if self.first == end { + // The buffer is currently empty + + if dma.tcif() { + // The dma controller has written such that the ring buffer now wraps + // This is the special case where exactly n*dma_buf.len(), n = 1,2,..., bytes was written, + // but where additional bytes are now written causing the ring buffer to wrap. + // This is only an error if the writing has passed the current unread region. + self.ndtr = dma.ndtr(); + if self.end() > self.first { + dma.clear_tcif(); + return Err(OverrunError); + } + } + + self.expect_next_read_to_wrap = false; + Ok(0) + } else if self.first < end { + // The available, unread portion in the ring buffer DOES NOT wrap + + if self.expect_next_read_to_wrap { + // The read was expected to wrap but it did not + + dma.clear_tcif(); + return Err(OverrunError); + } + + // Copy out the bytes from the dma buffer + let len = self.copy_to(buf, self.first..end); + + compiler_fence(Ordering::SeqCst); + + if dma.tcif() { + // The dma controller has written such that the ring buffer now wraps + + self.ndtr = dma.ndtr(); + if self.end() > self.first { + // The bytes that we have copied out have overflowed + // as the writer has now both wrapped and is currently writing + // within the region that we have just copied out + + // Clear transfer completed interrupt flag + dma.clear_tcif(); + return Err(OverrunError); + } + } + + self.first = (self.first + len) % self.dma_buf.len(); + self.expect_next_read_to_wrap = false; + Ok(len) + } else { + // The available, unread portion in the ring buffer DOES wrap + // The dma controller has wrapped since we last read and is currently + // writing (or the next byte added will be) in the beginning of the ring buffer. + + // If the unread portion wraps then the writer must also have wrapped, + // or it has wrapped and we already cleared the TCIF flag + assert!(dma.tcif() || self.expect_next_read_to_wrap); + + // Clear transfer completed interrupt flag + dma.clear_tcif(); + + if self.first + buf.len() < self.dma_buf.len() { + // The provided read buffer is not large enough to include all bytes from the tail of the dma buffer. + + // Copy out from the dma buffer + let len = self.copy_to(buf, self.first..self.dma_buf.len()); + + compiler_fence(Ordering::SeqCst); + + // We have now copied out the data from dma_buf + // Make sure that the just read part was not overwritten during the copy + self.ndtr = dma.ndtr(); + if self.end() > self.first || dma.tcif() { + // The writer has entered the data that we have just read since we read out `end` in the beginning and until now. + return Err(OverrunError); + } + + self.first = (self.first + len) % self.dma_buf.len(); + self.expect_next_read_to_wrap = true; + Ok(len) + } else { + // The provided read buffer is large enough to include all bytes from the tail of the dma buffer, + // so the next read will not have any unread tail bytes in the ring buffer. + + // Copy out from the dma buffer + let tail = self.copy_to(buf, self.first..self.dma_buf.len()); + let head = self.copy_to(&mut buf[tail..], 0..end); + + compiler_fence(Ordering::SeqCst); + + // We have now copied out the data from dma_buf + // Make sure that the just read part was not overwritten during the copy + self.ndtr = dma.ndtr(); + if self.end() > self.first || dma.tcif() { + return Err(OverrunError); + } + + self.first = head; + self.expect_next_read_to_wrap = false; + Ok(tail + head) + } + } + } + + /// Copy from the dma buffer at `data_range` into `buf` + fn copy_to(&mut self, buf: &mut [W], data_range: Range) -> usize { + // Limit the number of bytes that can be copied + let length = usize::min(data_range.len(), buf.len()); + + // Copy from dma buffer into read buffer + // We need to do it like this instead of a simple copy_from_slice() because + // reading from a part of memory that may be simultaneously written to is unsafe + unsafe { + let dma_buf = self.dma_buf.as_ptr(); + + for i in 0..length { + buf[i] = core::ptr::read_volatile(dma_buf.offset((data_range.start + i) as isize)); + } + } + + length + } +} + +#[cfg(test)] +mod tests { + use core::array; + use core::cell::RefCell; + + use super::*; + + struct TestCtrl { + next_ndtr: RefCell>, + tcif: bool, + } + + impl TestCtrl { + pub const fn new() -> Self { + Self { + next_ndtr: RefCell::new(None), + tcif: false, + } + } + + pub fn set_next_ndtr(&mut self, ndtr: usize) { + self.next_ndtr.borrow_mut().replace(ndtr); + } + } + + impl DmaCtrl for TestCtrl { + fn ndtr(&self) -> usize { + self.next_ndtr.borrow_mut().unwrap() + } + + fn tcif(&self) -> bool { + self.tcif + } + + fn clear_tcif(&mut self) { + self.tcif = false; + } + } + + #[test] + fn empty() { + let mut dma_buf = [0u8; 16]; + let ringbuf = DmaRingBuffer::new(&mut dma_buf); + + assert!(ringbuf.is_empty()); + assert_eq!(0, ringbuf.len()); + } + + #[test] + fn can_read() { + let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 + let mut ctrl = TestCtrl::new(); + let mut ringbuf = DmaRingBuffer::new(&mut dma_buf); + ringbuf.ndtr = 6; + + assert!(!ringbuf.is_empty()); + assert_eq!(10, ringbuf.len()); + + let mut buf = [0; 2]; + assert_eq!(2, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + assert_eq!([0, 1], buf); + assert_eq!(8, ringbuf.len()); + + let mut buf = [0; 2]; + assert_eq!(2, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + assert_eq!([2, 3], buf); + assert_eq!(6, ringbuf.len()); + + let mut buf = [0; 8]; + assert_eq!(6, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + assert_eq!([4, 5, 6, 7, 8, 9], buf[..6]); + assert_eq!(0, ringbuf.len()); + + let mut buf = [0; 2]; + assert_eq!(0, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + } + + #[test] + fn can_read_with_wrap() { + let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 + let mut ctrl = TestCtrl::new(); + let mut ringbuf = DmaRingBuffer::new(&mut dma_buf); + ringbuf.first = 12; + ringbuf.ndtr = 10; + + // The dma controller has written 4 + 6 bytes and has reloaded NDTR + ctrl.tcif = true; + ctrl.set_next_ndtr(10); + + assert!(!ringbuf.is_empty()); + assert_eq!(6 + 4, ringbuf.len()); + + let mut buf = [0; 2]; + assert_eq!(2, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + assert_eq!([12, 13], buf); + assert_eq!(6 + 2, ringbuf.len()); + + let mut buf = [0; 4]; + assert_eq!(4, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + assert_eq!([14, 15, 0, 1], buf); + assert_eq!(4, ringbuf.len()); + } + + #[test] + fn can_read_when_dma_writer_is_wrapped_and_read_does_not_wrap() { + let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 + let mut ctrl = TestCtrl::new(); + let mut ringbuf = DmaRingBuffer::new(&mut dma_buf); + ringbuf.first = 2; + ringbuf.ndtr = 6; + + // The dma controller has written 6 + 2 bytes and has reloaded NDTR + ctrl.tcif = true; + ctrl.set_next_ndtr(14); + + let mut buf = [0; 2]; + assert_eq!(2, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + assert_eq!([2, 3], buf); + + assert_eq!(true, ctrl.tcif); // The interrupt flag IS NOT cleared + } + + #[test] + fn can_read_when_dma_writer_is_wrapped_and_read_wraps() { + let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 + let mut ctrl = TestCtrl::new(); + let mut ringbuf = DmaRingBuffer::new(&mut dma_buf); + ringbuf.first = 12; + ringbuf.ndtr = 10; + + // The dma controller has written 6 + 2 bytes and has reloaded NDTR + ctrl.tcif = true; + ctrl.set_next_ndtr(14); + + let mut buf = [0; 10]; + assert_eq!(10, ringbuf.read(&mut ctrl, &mut buf).unwrap()); + assert_eq!([12, 13, 14, 15, 0, 1, 2, 3, 4, 5], buf); + + assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + } + + #[test] + fn cannot_read_when_dma_writer_wraps_with_same_ndtr() { + let mut dma_buf = [0u8; 16]; + let mut ctrl = TestCtrl::new(); + let mut ringbuf = DmaRingBuffer::new(&mut dma_buf); + ringbuf.first = 6; + ringbuf.ndtr = 10; + ctrl.set_next_ndtr(9); + + assert!(ringbuf.is_empty()); // The ring buffer thinks that it is empty + + // The dma controller has written exactly 16 bytes + ctrl.tcif = true; + + let mut buf = [0; 2]; + assert_eq!(Err(OverrunError), ringbuf.read(&mut ctrl, &mut buf)); + + assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + } + + #[test] + fn cannot_read_when_dma_writer_overwrites_during_not_wrapping_read() { + let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 + let mut ctrl = TestCtrl::new(); + let mut ringbuf = DmaRingBuffer::new(&mut dma_buf); + ringbuf.first = 2; + ringbuf.ndtr = 6; + + // The dma controller has written 6 + 3 bytes and has reloaded NDTR + ctrl.tcif = true; + ctrl.set_next_ndtr(13); + + let mut buf = [0; 2]; + assert_eq!(Err(OverrunError), ringbuf.read(&mut ctrl, &mut buf)); + + assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + } + + #[test] + fn cannot_read_when_dma_writer_overwrites_during_wrapping_read() { + let mut dma_buf: [u8; 16] = array::from_fn(|idx| idx as u8); // 0, 1, ..., 15 + let mut ctrl = TestCtrl::new(); + let mut ringbuf = DmaRingBuffer::new(&mut dma_buf); + ringbuf.first = 12; + ringbuf.ndtr = 10; + + // The dma controller has written 6 + 13 bytes and has reloaded NDTR + ctrl.tcif = true; + ctrl.set_next_ndtr(3); + + let mut buf = [0; 2]; + assert_eq!(Err(OverrunError), ringbuf.read(&mut ctrl, &mut buf)); + + assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + } +} diff --git a/embassy-stm32/src/usart/mod.rs b/embassy-stm32/src/usart/mod.rs index 266561659..fea0c5f11 100644 --- a/embassy-stm32/src/usart/mod.rs +++ b/embassy-stm32/src/usart/mod.rs @@ -283,8 +283,8 @@ impl<'d, T: BasicInstance, RxDma> UartRx<'d, T, RxDma> { let (sr, cr1, cr3) = unsafe { (sr(r).read(), r.cr1().read(), r.cr3().read()) }; + let mut wake = false; let has_errors = (sr.pe() && cr1.peie()) || ((sr.fe() || sr.ne() || sr.ore()) && cr3.eie()); - if has_errors { // clear all interrupts and DMA Rx Request unsafe { @@ -304,22 +304,35 @@ impl<'d, T: BasicInstance, RxDma> UartRx<'d, T, RxDma> { }); } - compiler_fence(Ordering::SeqCst); + wake = true; + } else { + if cr1.idleie() && sr.idle() { + // IDLE detected: no more data will come + unsafe { + r.cr1().modify(|w| { + // disable idle line detection + w.set_idleie(false); + }); - s.rx_waker.wake(); - } else if cr1.idleie() && sr.idle() { - // IDLE detected: no more data will come - unsafe { - r.cr1().modify(|w| { - // disable idle line detection - w.set_idleie(false); - }); + r.cr3().modify(|w| { + // disable DMA Rx Request + w.set_dmar(false); + }); + } - r.cr3().modify(|w| { - // disable DMA Rx Request - w.set_dmar(false); - }); + wake = true; } + + if cr1.rxneie() { + // We cannot check the RXNE flag as it is auto-cleared by the DMA controller + + // It is up to the listener to determine if this in fact was a RX event and disable the RXNE detection + + wake = true; + } + } + + if wake { compiler_fence(Ordering::SeqCst); s.rx_waker.wake(); @@ -972,6 +985,8 @@ mod eio { pub use buffered::*; #[cfg(feature = "nightly")] mod buffered; +mod rx_ringbuffered; +pub use rx_ringbuffered::RingBufferedUartRx; #[cfg(usart_v1)] fn tdr(r: crate::pac::usart::Usart) -> *mut u8 { diff --git a/embassy-stm32/src/usart/rx_ringbuffered.rs b/embassy-stm32/src/usart/rx_ringbuffered.rs new file mode 100644 index 000000000..0dc90ece7 --- /dev/null +++ b/embassy-stm32/src/usart/rx_ringbuffered.rs @@ -0,0 +1,286 @@ +use core::future::poll_fn; +use core::sync::atomic::{compiler_fence, Ordering}; +use core::task::Poll; + +use embassy_hal_common::drop::OnDrop; +use embassy_hal_common::PeripheralRef; + +use super::{rdr, sr, BasicInstance, Error, UartRx}; +use crate::dma::ringbuffer::OverrunError; +use crate::dma::RingBuffer; + +pub struct RingBufferedUartRx<'d, T: BasicInstance, RxDma: super::RxDma> { + _peri: PeripheralRef<'d, T>, + ring_buf: RingBuffer<'d, RxDma, u8>, +} + +impl<'d, T: BasicInstance, RxDma: super::RxDma> UartRx<'d, T, RxDma> { + /// Turn the `UartRx` into a buffered uart which can continously receive in the background + /// without the possibility of loosing bytes. The `dma_buf` is a buffer registered to the + /// DMA controller, and must be sufficiently large, such that it will not overflow. + pub fn into_ring_buffered(self, dma_buf: &'d mut [u8]) -> RingBufferedUartRx<'d, T, RxDma> { + assert!(dma_buf.len() > 0 && dma_buf.len() <= 0xFFFF); + + let request = self.rx_dma.request(); + let opts = Default::default(); + let ring_buf = unsafe { RingBuffer::new_read(self.rx_dma, request, rdr(T::regs()), dma_buf, opts) }; + RingBufferedUartRx { + _peri: self._peri, + ring_buf, + } + } +} + +impl<'d, T: BasicInstance, RxDma: super::RxDma> RingBufferedUartRx<'d, T, RxDma> { + pub fn start(&mut self) -> Result<(), Error> { + // Clear the ring buffer so that it is ready to receive data + self.ring_buf.clear(); + + self.setup_uart(); + + Ok(()) + } + + /// Start uart background receive + fn setup_uart(&mut self) { + // fence before starting DMA. + compiler_fence(Ordering::SeqCst); + + self.ring_buf.start(); + + let r = T::regs(); + // clear all interrupts and DMA Rx Request + // SAFETY: only clears Rx related flags + unsafe { + r.cr1().modify(|w| { + // disable RXNE interrupt + w.set_rxneie(false); + // enable parity interrupt if not ParityNone + w.set_peie(w.pce()); + // disable idle line interrupt + w.set_idleie(false); + }); + r.cr3().modify(|w| { + // enable Error Interrupt: (Frame error, Noise error, Overrun error) + w.set_eie(true); + // enable DMA Rx Request + w.set_dmar(true); + }); + } + } + + /// Stop uart background receive + fn teardown_uart(&mut self) { + let r = T::regs(); + // clear all interrupts and DMA Rx Request + // SAFETY: only clears Rx related flags + unsafe { + r.cr1().modify(|w| { + // disable RXNE interrupt + w.set_rxneie(false); + // disable parity interrupt + w.set_peie(false); + // disable idle line interrupt + w.set_idleie(false); + }); + r.cr3().modify(|w| { + // disable Error Interrupt: (Frame error, Noise error, Overrun error) + w.set_eie(false); + // disable DMA Rx Request + w.set_dmar(false); + }); + } + + compiler_fence(Ordering::SeqCst); + + self.ring_buf.request_stop(); + while self.ring_buf.is_running() {} + } + + /// Read bytes that are readily available in the ring buffer. + /// If no bytes are currently available in the buffer the call waits until data are received. + /// + /// Background receive is started if `start()` has not been previously called. + /// + /// Receive in the background is terminated if an error is returned. + /// It must then manually be started again by calling `start()` or by re-calling `read()`. + pub async fn read(&mut self, buf: &mut [u8]) -> Result { + let r = T::regs(); + + // SAFETY: read only + let is_started = unsafe { r.cr3().read().dmar() }; + + // Start background receive if it was not already started + if !is_started { + self.start()?; + } + + // SAFETY: read only and we only use Rx related flags + let s = unsafe { sr(r).read() }; + let has_errors = s.pe() || s.fe() || s.ne() || s.ore(); + if has_errors { + self.teardown_uart(); + + if s.pe() { + return Err(Error::Parity); + } else if s.fe() { + return Err(Error::Framing); + } else if s.ne() { + return Err(Error::Noise); + } else { + return Err(Error::Overrun); + } + } + + let ndtr = self.ring_buf.get_remaining_transfers(); + self.ring_buf.set_ndtr(ndtr); + match self.ring_buf.read(buf) { + Ok(len) if len == 0 => {} + Ok(len) => { + assert!(len > 0); + return Ok(len); + } + Err(OverrunError) => { + // Stop any transfer from now on + // The user must re-start to receive any more data + self.teardown_uart(); + return Err(Error::Overrun); + } + } + + // Wait for any data since `ndtr` + self.wait_for_data(ndtr).await?; + + // ndtr is now different than the value provided to `wait_for_data()` + // Re-sample ndtr now when it has changed. + self.ring_buf.set_ndtr(self.ring_buf.get_remaining_transfers()); + let len = self.ring_buf.read(buf).map_err(|_err| Error::Overrun)?; + assert!(len > 0); + Ok(len) + } + + /// Wait for uart data + async fn wait_for_data(&mut self, old_ndtr: usize) -> Result<(), Error> { + let r = T::regs(); + + // make sure USART state is restored to neutral state when this future is dropped + let _drop = OnDrop::new(move || { + // SAFETY: only clears Rx related flags + unsafe { + r.cr1().modify(|w| { + // disable RXNE interrupt + w.set_rxneie(false); + }); + } + }); + + // SAFETY: only sets Rx related flags + unsafe { + r.cr1().modify(|w| { + // enable RXNE interrupt + w.set_rxneie(true); + }); + } + + // future which completes when RX "not empty" is detected, + // i.e. when there is data in uart rx register + let rxne = poll_fn(|cx| { + let s = T::state(); + + // Register waker to be awaken when RXNE interrupt is received + s.rx_waker.register(cx.waker()); + + compiler_fence(Ordering::SeqCst); + + // SAFETY: read only and we only use Rx related flags + let s = unsafe { sr(r).read() }; + let has_errors = s.pe() || s.fe() || s.ne() || s.ore(); + if has_errors { + if s.pe() { + return Poll::Ready(Err(Error::Parity)); + } else if s.fe() { + return Poll::Ready(Err(Error::Framing)); + } else if s.ne() { + return Poll::Ready(Err(Error::Noise)); + } else { + return Poll::Ready(Err(Error::Overrun)); + } + } + + // Re-sample ndtr and determine if it has changed since we started + // waiting for data. + let new_ndtr = self.ring_buf.get_remaining_transfers(); + if new_ndtr != old_ndtr { + // Some data was received as NDTR has changed + Poll::Ready(Ok(())) + } else { + // It may be that the DMA controller is currently busy consuming the + // RX data register. We therefore wait register to become empty. + while unsafe { sr(r).read().rxne() } {} + + compiler_fence(Ordering::SeqCst); + + // Re-get again: This time we know that the DMA controller has consumed + // the current read register if it was busy doing so + let new_ndtr = self.ring_buf.get_remaining_transfers(); + if new_ndtr != old_ndtr { + // Some data was received as NDTR has changed + Poll::Ready(Ok(())) + } else { + Poll::Pending + } + } + }); + + compiler_fence(Ordering::SeqCst); + + let new_ndtr = self.ring_buf.get_remaining_transfers(); + if new_ndtr != old_ndtr { + // Fast path - NDTR has already changed, no reason to poll + Ok(()) + } else { + // NDTR has not changed since we first read from the ring buffer + // Wait for RXNE interrupt... + match rxne.await { + Ok(()) => Ok(()), + Err(e) => { + self.teardown_uart(); + Err(e) + } + } + } + } +} + +impl> Drop for RingBufferedUartRx<'_, T, RxDma> { + fn drop(&mut self) { + self.teardown_uart(); + } +} + +#[cfg(all(feature = "unstable-traits", feature = "nightly"))] +mod eio { + use embedded_io::asynch::Read; + use embedded_io::Io; + + use super::RingBufferedUartRx; + use crate::usart::{BasicInstance, Error, RxDma}; + + impl Io for RingBufferedUartRx<'_, T, Rx> + where + T: BasicInstance, + Rx: RxDma, + { + type Error = Error; + } + + impl Read for RingBufferedUartRx<'_, T, Rx> + where + T: BasicInstance, + Rx: RxDma, + { + async fn read(&mut self, buf: &mut [u8]) -> Result { + self.read(buf).await + } + } +} diff --git a/tests/stm32/Cargo.toml b/tests/stm32/Cargo.toml index d10d01e29..240fad522 100644 --- a/tests/stm32/Cargo.toml +++ b/tests/stm32/Cargo.toml @@ -33,6 +33,8 @@ embedded-hal = "0.2.6" embedded-hal-1 = { package = "embedded-hal", version = "=1.0.0-alpha.10" } embedded-hal-async = { version = "=0.2.0-alpha.1" } panic-probe = { version = "0.3.0", features = ["print-defmt"] } +rand_core = { version = "0.6", default-features = false } +rand_chacha = { version = "0.3", default-features = false } chrono = { version = "^0.4", default-features = false, optional = true} diff --git a/tests/stm32/src/bin/usart_rx_ringbuffered.rs b/tests/stm32/src/bin/usart_rx_ringbuffered.rs new file mode 100644 index 000000000..3ea8bfb7b --- /dev/null +++ b/tests/stm32/src/bin/usart_rx_ringbuffered.rs @@ -0,0 +1,188 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +#[path = "../example_common.rs"] +mod example_common; +use embassy_executor::Spawner; +use embassy_stm32::interrupt; +use embassy_stm32::usart::{Config, DataBits, Parity, RingBufferedUartRx, StopBits, Uart, UartTx}; +use embassy_time::{Duration, Timer}; +use example_common::*; +use rand_chacha::ChaCha8Rng; +use rand_core::{RngCore, SeedableRng}; + +#[cfg(feature = "stm32f103c8")] +mod board { + pub type Uart = embassy_stm32::peripherals::USART1; + pub type TxDma = embassy_stm32::peripherals::DMA1_CH4; + pub type RxDma = embassy_stm32::peripherals::DMA1_CH5; +} +#[cfg(feature = "stm32g491re")] +mod board { + pub type Uart = embassy_stm32::peripherals::USART1; + pub type TxDma = embassy_stm32::peripherals::DMA1_CH1; + pub type RxDma = embassy_stm32::peripherals::DMA1_CH2; +} +#[cfg(feature = "stm32g071rb")] +mod board { + pub type Uart = embassy_stm32::peripherals::USART1; + pub type TxDma = embassy_stm32::peripherals::DMA1_CH1; + pub type RxDma = embassy_stm32::peripherals::DMA1_CH2; +} +#[cfg(feature = "stm32f429zi")] +mod board { + pub type Uart = embassy_stm32::peripherals::USART2; + pub type TxDma = embassy_stm32::peripherals::DMA1_CH6; + pub type RxDma = embassy_stm32::peripherals::DMA1_CH5; +} +#[cfg(feature = "stm32wb55rg")] +mod board { + pub type Uart = embassy_stm32::peripherals::LPUART1; + pub type TxDma = embassy_stm32::peripherals::DMA1_CH1; + pub type RxDma = embassy_stm32::peripherals::DMA1_CH2; +} +#[cfg(feature = "stm32h755zi")] +mod board { + pub type Uart = embassy_stm32::peripherals::USART1; + pub type TxDma = embassy_stm32::peripherals::DMA1_CH0; + pub type RxDma = embassy_stm32::peripherals::DMA1_CH1; +} +#[cfg(feature = "stm32u585ai")] +mod board { + pub type Uart = embassy_stm32::peripherals::USART3; + pub type TxDma = embassy_stm32::peripherals::GPDMA1_CH0; + pub type RxDma = embassy_stm32::peripherals::GPDMA1_CH1; +} + +const ONE_BYTE_DURATION_US: u32 = 9_000_000 / 115200; + +#[embassy_executor::main] +async fn main(spawner: Spawner) { + let p = embassy_stm32::init(config()); + info!("Hello World!"); + + // Arduino pins D0 and D1 + // They're connected together with a 1K resistor. + #[cfg(feature = "stm32f103c8")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = ( + p.PA9, + p.PA10, + p.USART1, + interrupt::take!(USART1), + p.DMA1_CH4, + p.DMA1_CH5, + ); + #[cfg(feature = "stm32g491re")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = + (p.PC4, p.PC5, p.USART1, interrupt::take!(USART1), p.DMA1_CH1, p.DMA1_CH2); + #[cfg(feature = "stm32g071rb")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = + (p.PC4, p.PC5, p.USART1, interrupt::take!(USART1), p.DMA1_CH1, p.DMA1_CH2); + #[cfg(feature = "stm32f429zi")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = + (p.PA2, p.PA3, p.USART2, interrupt::take!(USART2), p.DMA1_CH6, p.DMA1_CH5); + #[cfg(feature = "stm32wb55rg")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = ( + p.PA2, + p.PA3, + p.LPUART1, + interrupt::take!(LPUART1), + p.DMA1_CH1, + p.DMA1_CH2, + ); + #[cfg(feature = "stm32h755zi")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = + (p.PB6, p.PB7, p.USART1, interrupt::take!(USART1), p.DMA1_CH0, p.DMA1_CH1); + #[cfg(feature = "stm32u585ai")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = ( + p.PD8, + p.PD9, + p.USART3, + interrupt::take!(USART3), + p.GPDMA1_CH0, + p.GPDMA1_CH1, + ); + + // To run this test, use the saturating_serial test utility to saturate the serial port + + let mut config = Config::default(); + config.baudrate = 115200; + config.data_bits = DataBits::DataBits8; + config.stop_bits = StopBits::STOP1; + config.parity = Parity::ParityNone; + + let usart = Uart::new(usart, rx, tx, irq, tx_dma, rx_dma, config); + let (tx, rx) = usart.split(); + static mut DMA_BUF: [u8; 64] = [0; 64]; + let dma_buf = unsafe { DMA_BUF.as_mut() }; + let rx = rx.into_ring_buffered(dma_buf); + + info!("Spawning tasks"); + spawner.spawn(transmit_task(tx)).unwrap(); + spawner.spawn(receive_task(rx)).unwrap(); +} + +#[embassy_executor::task] +async fn transmit_task(mut tx: UartTx<'static, board::Uart, board::TxDma>) { + let mut rng = ChaCha8Rng::seed_from_u64(1337); + + info!("Starting random transmissions into void..."); + + let mut i: u8 = 0; + loop { + let mut buf = [0; 32]; + let len = 1 + (rng.next_u32() as usize % (buf.len() - 1)); + for b in &mut buf[..len] { + *b = i; + i = i.wrapping_add(1); + } + + tx.write(&buf[..len]).await.unwrap(); + Timer::after(Duration::from_micros((rng.next_u32() % 10000) as _)).await; + + //i += 1; + //if i % 1000 == 0 { + // trace!("Wrote {} times", i); + //} + } +} + +#[embassy_executor::task] +async fn receive_task(mut rx: RingBufferedUartRx<'static, board::Uart, board::RxDma>) { + info!("Ready to receive..."); + + let mut rng = ChaCha8Rng::seed_from_u64(1337); + + let mut i = 0; + let mut expected: Option = None; + loop { + let mut buf = [0; 100]; + let max_len = 1 + (rng.next_u32() as usize % (buf.len() - 1)); + let received = rx.read(&mut buf[..max_len]).await.unwrap(); + + if expected.is_none() { + info!("Test started"); + expected = Some(buf[0]); + } + + for byte in &buf[..received] { + if byte != &expected.unwrap() { + error!("Test fail! received {}, expected {}", *byte, expected.unwrap()); + cortex_m::asm::bkpt(); + return; + } + expected = Some(expected.unwrap().wrapping_add(1)); + } + + if received < max_len { + let byte_count = rng.next_u32() % 64; + Timer::after(Duration::from_micros((byte_count * ONE_BYTE_DURATION_US) as _)).await; + } + + i += 1; + if i % 1000 == 0 { + trace!("Read {} times", i); + } + } +} diff --git a/tests/utils/Cargo.toml b/tests/utils/Cargo.toml new file mode 100644 index 000000000..7d66fd586 --- /dev/null +++ b/tests/utils/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "test-utils" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +rand = "0.8" +serial = "0.4" diff --git a/tests/utils/src/bin/saturate_serial.rs b/tests/utils/src/bin/saturate_serial.rs new file mode 100644 index 000000000..28480516d --- /dev/null +++ b/tests/utils/src/bin/saturate_serial.rs @@ -0,0 +1,52 @@ +use std::path::Path; +use std::time::Duration; +use std::{env, io, thread}; + +use rand::random; +use serial::SerialPort; + +pub fn main() { + if let Some(port_name) = env::args().nth(1) { + let sleep = env::args().position(|x| x == "--sleep").is_some(); + + println!("Saturating port {:?} with 115200 8N1", port_name); + println!("Sleep: {}", sleep); + let mut port = serial::open(&port_name).unwrap(); + if saturate(&mut port, sleep).is_err() { + eprintln!("Unable to saturate port"); + } + } else { + let path = env::args().next().unwrap(); + let basepath = Path::new(&path).with_extension(""); + let basename = basepath.file_name().unwrap(); + eprintln!("USAGE: {} ", basename.to_string_lossy()); + } +} + +fn saturate(port: &mut T, sleep: bool) -> io::Result<()> { + port.reconfigure(&|settings| { + settings.set_baud_rate(serial::Baud115200)?; + settings.set_char_size(serial::Bits8); + settings.set_parity(serial::ParityNone); + settings.set_stop_bits(serial::Stop1); + Ok(()) + })?; + + let mut written = 0; + loop { + let len = random::() % 0x1000; + let buf: Vec = (written..written + len).map(|x| x as u8).collect(); + + port.write_all(&buf)?; + + if sleep { + let micros = (random::() % 1000) as u64; + println!("Sleeping {}us", micros); + port.flush().unwrap(); + thread::sleep(Duration::from_micros(micros)); + } + + written += len; + println!("Written: {}", written); + } +} \ No newline at end of file From 4ea6662e55f32ff90b564c1c611f5ac4e0d8ab1c Mon Sep 17 00:00:00 2001 From: Rasmus Melchior Jacobsen Date: Thu, 27 Apr 2023 10:43:52 +0200 Subject: [PATCH 02/12] Do not disable dma request when idle line is detected --- embassy-stm32/src/usart/mod.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/embassy-stm32/src/usart/mod.rs b/embassy-stm32/src/usart/mod.rs index fea0c5f11..f3dea3391 100644 --- a/embassy-stm32/src/usart/mod.rs +++ b/embassy-stm32/src/usart/mod.rs @@ -313,11 +313,6 @@ impl<'d, T: BasicInstance, RxDma> UartRx<'d, T, RxDma> { // disable idle line detection w.set_idleie(false); }); - - r.cr3().modify(|w| { - // disable DMA Rx Request - w.set_dmar(false); - }); } wake = true; From fc268df6f56661d5f43450c7a03850044ae8e136 Mon Sep 17 00:00:00 2001 From: Rasmus Melchior Jacobsen Date: Thu, 27 Apr 2023 10:48:38 +0200 Subject: [PATCH 03/12] Support overflow detection for more than one ring-period --- embassy-stm32/src/dma/dma.rs | 115 ++++++++++++----- embassy-stm32/src/dma/ringbuffer.rs | 127 ++++++++----------- embassy-stm32/src/usart/rx_ringbuffered.rs | 121 +++++++++--------- tests/stm32/src/bin/usart_rx_ringbuffered.rs | 19 ++- tests/utils/src/bin/saturate_serial.rs | 15 ++- 5 files changed, 216 insertions(+), 181 deletions(-) diff --git a/embassy-stm32/src/dma/dma.rs b/embassy-stm32/src/dma/dma.rs index 17d82fe2d..10ae20f5c 100644 --- a/embassy-stm32/src/dma/dma.rs +++ b/embassy-stm32/src/dma/dma.rs @@ -4,6 +4,7 @@ use core::pin::Pin; use core::sync::atomic::{fence, Ordering}; use core::task::{Context, Poll, Waker}; +use atomic_polyfill::AtomicUsize; use embassy_cortex_m::interrupt::Priority; use embassy_hal_common::{into_ref, Peripheral, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; @@ -129,13 +130,16 @@ impl From for vals::Fth { struct State { ch_wakers: [AtomicWaker; DMA_CHANNEL_COUNT], + complete_count: [AtomicUsize; DMA_CHANNEL_COUNT], } impl State { const fn new() -> Self { + const ZERO: AtomicUsize = AtomicUsize::new(0); const AW: AtomicWaker = AtomicWaker::new(); Self { ch_wakers: [AW; DMA_CHANNEL_COUNT], + complete_count: [ZERO; DMA_CHANNEL_COUNT], } } } @@ -184,13 +188,43 @@ pub(crate) unsafe fn on_irq_inner(dma: pac::dma::Dma, channel_num: usize, index: panic!("DMA: error on DMA@{:08x} channel {}", dma.0 as u32, channel_num); } - if isr.tcif(channel_num % 4) && cr.read().tcie() { - /* acknowledge transfer complete interrupt */ - dma.ifcr(channel_num / 4).write(|w| w.set_tcif(channel_num % 4, true)); + let mut wake = false; + + if isr.htif(channel_num % 4) && cr.read().htie() { + // Acknowledge half transfer complete interrupt + dma.ifcr(channel_num / 4).write(|w| w.set_htif(channel_num % 4, true)); + wake = true; + } + + wake |= process_tcif(dma, channel_num, index); + + if wake { STATE.ch_wakers[index].wake(); } } +unsafe fn process_tcif(dma: pac::dma::Dma, channel_num: usize, index: usize) -> bool { + let isr_reg = dma.isr(channel_num / 4); + let cr_reg = dma.st(channel_num).cr(); + + // First, figure out if tcif is set without a cs. + if isr_reg.read().tcif(channel_num % 4) && cr_reg.read().tcie() { + // Make tcif test again within a cs to avoid race when incrementing complete_count. + critical_section::with(|_| { + if isr_reg.read().tcif(channel_num % 4) && cr_reg.read().tcie() { + // Acknowledge transfer complete interrupt + dma.ifcr(channel_num / 4).write(|w| w.set_tcif(channel_num % 4, true)); + STATE.complete_count[index].fetch_add(1, Ordering::Release); + true + } else { + false + } + }) + } else { + false + } +} + #[cfg(any(dma_v2, dmamux))] pub type Request = u8; #[cfg(not(any(dma_v2, dmamux)))] @@ -530,6 +564,7 @@ impl<'a, C: Channel, W: Word> DoubleBuffered<'a, C, W> { unsafe { dma.ifcr(isrn).write(|w| { + w.set_htif(isrbit, true); w.set_tcif(isrbit, true); w.set_teif(isrbit, true); }) @@ -593,32 +628,28 @@ impl<'a, C: Channel, W: Word> Drop for DoubleBuffered<'a, C, W> { // ============================== impl DmaCtrl for C { - fn tcif(&self) -> bool { - let channel_number = self.num(); - let dma = self.regs(); - let isrn = channel_number / 4; - let isrbit = channel_number % 4; - - unsafe { dma.isr(isrn).read() }.tcif(isrbit) - } - - fn clear_tcif(&mut self) { - let channel_number = self.num(); - let dma = self.regs(); - let isrn = channel_number / 4; - let isrbit = channel_number % 4; - - unsafe { - dma.ifcr(isrn).write(|w| { - w.set_tcif(isrbit, true); - }) - } - } - fn ndtr(&self) -> usize { let ch = self.regs().st(self.num()); unsafe { ch.ndtr().read() }.ndt() as usize } + + fn get_complete_count(&self) -> usize { + let dma = self.regs(); + let channel_num = self.num(); + let index = self.index(); + // Manually process tcif in case transfer was completed and we are in a higher priority task. + unsafe { process_tcif(dma, channel_num, index) }; + STATE.complete_count[index].load(Ordering::Acquire) + } + + fn reset_complete_count(&mut self) -> usize { + let dma = self.regs(); + let channel_num = self.num(); + let index = self.index(); + // Manually process tcif in case transfer was completed and we are in a higher priority task. + unsafe { process_tcif(dma, channel_num, index) }; + STATE.complete_count[index].swap(0, Ordering::AcqRel) + } } pub struct RingBuffer<'a, C: Channel, W: Word> { @@ -657,7 +688,8 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { w.set_minc(vals::Inc::INCREMENTED); w.set_pinc(vals::Inc::FIXED); w.set_teie(true); - w.set_tcie(false); + w.set_htie(true); + w.set_tcie(true); w.set_circ(vals::Circ::ENABLED); #[cfg(dma_v1)] w.set_trbuff(true); @@ -703,7 +735,7 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { } pub fn clear(&mut self) { - self.ringbuf.clear(); + self.ringbuf.clear(&mut *self.channel); } /// Read bytes from the ring buffer @@ -712,6 +744,22 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { self.ringbuf.read(&mut *self.channel, buf) } + pub fn is_empty(&self) -> bool { + self.ringbuf.is_empty() + } + + pub fn len(&self) -> usize { + self.ringbuf.len() + } + + pub fn capacity(&self) -> usize { + self.ringbuf.dma_buf.len() + } + + pub fn set_waker(&mut self, waker: &Waker) { + STATE.ch_wakers[self.channel.index()].register(waker); + } + fn clear_irqs(&mut self) { let channel_number = self.channel.num(); let dma = self.channel.regs(); @@ -720,6 +768,7 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { unsafe { dma.ifcr(isrn).write(|w| { + w.set_htif(isrbit, true); w.set_tcif(isrbit, true); w.set_teif(isrbit, true); }) @@ -733,6 +782,7 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { unsafe { ch.cr().write(|w| { w.set_teie(true); + w.set_htie(true); w.set_tcie(true); }) } @@ -743,15 +793,10 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { unsafe { ch.cr().read() }.en() } - /// Gets the total remaining transfers for the channel - /// Note: this will be zero for transfers that completed without cancellation. - pub fn get_remaining_transfers(&self) -> usize { + /// Synchronize the position of the ring buffer to the actual DMA controller position + pub fn reload_position(&mut self) { let ch = self.channel.regs().st(self.channel.num()); - unsafe { ch.ndtr().read() }.ndt() as usize - } - - pub fn set_ndtr(&mut self, ndtr: usize) { - self.ringbuf.ndtr = ndtr; + self.ringbuf.ndtr = unsafe { ch.ndtr().read() }.ndt() as usize; } } diff --git a/embassy-stm32/src/dma/ringbuffer.rs b/embassy-stm32/src/dma/ringbuffer.rs index f9ace6018..544ec9461 100644 --- a/embassy-stm32/src/dma/ringbuffer.rs +++ b/embassy-stm32/src/dma/ringbuffer.rs @@ -10,14 +10,6 @@ use super::word::Word; /// to the current register value. `ndtr` describes the current position of the DMA /// write. /// -/// # Safety -/// -/// The ring buffer controls the TCIF (transfer completed interrupt flag) to -/// detect buffer overruns, hence this interrupt must be disabled. -/// The buffer can detect overruns up to one period, that is, for a X byte buffer, -/// overruns can be detected if they happen from byte X+1 up to 2X. After this -/// point, overrunds may or may not be detected. -/// /// # Buffer layout /// /// ```text @@ -39,7 +31,6 @@ pub struct DmaRingBuffer<'a, W: Word> { pub(crate) dma_buf: &'a mut [W], first: usize, pub ndtr: usize, - expect_next_read_to_wrap: bool, } #[derive(Debug, PartialEq)] @@ -50,13 +41,13 @@ pub trait DmaCtrl { /// buffer until the dma writer wraps. fn ndtr(&self) -> usize; - /// Read the transfer completed interrupt flag - /// This flag is set by the dma controller when NDTR is reloaded, + /// Get the transfer completed counter. + /// This counter is incremented by the dma controller when NDTR is reloaded, /// i.e. when the writing wraps. - fn tcif(&self) -> bool; + fn get_complete_count(&self) -> usize; - /// Clear the transfer completed interrupt flag - fn clear_tcif(&mut self); + /// Reset the transfer completed counter to 0 and return the value just prior to the reset. + fn reset_complete_count(&mut self) -> usize; } impl<'a, W: Word> DmaRingBuffer<'a, W> { @@ -66,15 +57,14 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { dma_buf, first: 0, ndtr, - expect_next_read_to_wrap: false, } } /// Reset the ring buffer to its initial state - pub fn clear(&mut self) { + pub fn clear(&mut self, dma: &mut impl DmaCtrl) { self.first = 0; self.ndtr = self.dma_buf.len(); - self.expect_next_read_to_wrap = false; + dma.reset_complete_count(); } /// The buffer end position @@ -83,14 +73,12 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { } /// Returns whether the buffer is empty - #[allow(dead_code)] pub fn is_empty(&self) -> bool { self.first == self.end() } /// The current number of bytes in the buffer /// This may change at any time if dma is currently active - #[allow(dead_code)] pub fn len(&self) -> usize { // Read out a stable end (the dma periheral can change it at anytime) let end = self.end(); @@ -112,27 +100,19 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { if self.first == end { // The buffer is currently empty - if dma.tcif() { - // The dma controller has written such that the ring buffer now wraps - // This is the special case where exactly n*dma_buf.len(), n = 1,2,..., bytes was written, - // but where additional bytes are now written causing the ring buffer to wrap. - // This is only an error if the writing has passed the current unread region. + if dma.get_complete_count() > 0 { + // The DMA has written such that the ring buffer wraps at least once self.ndtr = dma.ndtr(); - if self.end() > self.first { - dma.clear_tcif(); + if self.end() > self.first || dma.get_complete_count() > 1 { return Err(OverrunError); } } - self.expect_next_read_to_wrap = false; Ok(0) } else if self.first < end { // The available, unread portion in the ring buffer DOES NOT wrap - if self.expect_next_read_to_wrap { - // The read was expected to wrap but it did not - - dma.clear_tcif(); + if dma.get_complete_count() > 1 { return Err(OverrunError); } @@ -141,35 +121,39 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { compiler_fence(Ordering::SeqCst); - if dma.tcif() { - // The dma controller has written such that the ring buffer now wraps - - self.ndtr = dma.ndtr(); - if self.end() > self.first { - // The bytes that we have copied out have overflowed - // as the writer has now both wrapped and is currently writing - // within the region that we have just copied out - - // Clear transfer completed interrupt flag - dma.clear_tcif(); + match dma.get_complete_count() { + 0 => { + // The DMA writer has not wrapped before nor after the copy + } + 1 => { + // The DMA writer has written such that the ring buffer now wraps + self.ndtr = dma.ndtr(); + if self.end() > self.first || dma.get_complete_count() > 1 { + // The bytes that we have copied out have overflowed + // as the writer has now both wrapped and is currently writing + // within the region that we have just copied out + return Err(OverrunError); + } + } + _ => { return Err(OverrunError); } } self.first = (self.first + len) % self.dma_buf.len(); - self.expect_next_read_to_wrap = false; Ok(len) } else { // The available, unread portion in the ring buffer DOES wrap - // The dma controller has wrapped since we last read and is currently + // The DMA writer has wrapped since we last read and is currently // writing (or the next byte added will be) in the beginning of the ring buffer. - // If the unread portion wraps then the writer must also have wrapped, - // or it has wrapped and we already cleared the TCIF flag - assert!(dma.tcif() || self.expect_next_read_to_wrap); + let complete_count = dma.get_complete_count(); + if complete_count > 1 { + return Err(OverrunError); + } - // Clear transfer completed interrupt flag - dma.clear_tcif(); + // If the unread portion wraps then the writer must also have wrapped + assert!(complete_count == 1); if self.first + buf.len() < self.dma_buf.len() { // The provided read buffer is not large enough to include all bytes from the tail of the dma buffer. @@ -182,13 +166,12 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { // We have now copied out the data from dma_buf // Make sure that the just read part was not overwritten during the copy self.ndtr = dma.ndtr(); - if self.end() > self.first || dma.tcif() { + if self.end() > self.first || dma.get_complete_count() > 1 { // The writer has entered the data that we have just read since we read out `end` in the beginning and until now. return Err(OverrunError); } self.first = (self.first + len) % self.dma_buf.len(); - self.expect_next_read_to_wrap = true; Ok(len) } else { // The provided read buffer is large enough to include all bytes from the tail of the dma buffer, @@ -201,14 +184,14 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { compiler_fence(Ordering::SeqCst); // We have now copied out the data from dma_buf - // Make sure that the just read part was not overwritten during the copy + // Reset complete counter and make sure that the just read part was not overwritten during the copy self.ndtr = dma.ndtr(); - if self.end() > self.first || dma.tcif() { + let complete_count = dma.reset_complete_count(); + if self.end() > self.first || complete_count > 1 { return Err(OverrunError); } self.first = head; - self.expect_next_read_to_wrap = false; Ok(tail + head) } } @@ -243,14 +226,14 @@ mod tests { struct TestCtrl { next_ndtr: RefCell>, - tcif: bool, + complete_count: usize, } impl TestCtrl { pub const fn new() -> Self { Self { next_ndtr: RefCell::new(None), - tcif: false, + complete_count: 0, } } @@ -264,12 +247,14 @@ mod tests { self.next_ndtr.borrow_mut().unwrap() } - fn tcif(&self) -> bool { - self.tcif + fn get_complete_count(&self) -> usize { + self.complete_count } - fn clear_tcif(&mut self) { - self.tcif = false; + fn reset_complete_count(&mut self) -> usize { + let old = self.complete_count; + self.complete_count = 0; + old } } @@ -320,7 +305,7 @@ mod tests { ringbuf.ndtr = 10; // The dma controller has written 4 + 6 bytes and has reloaded NDTR - ctrl.tcif = true; + ctrl.complete_count = 1; ctrl.set_next_ndtr(10); assert!(!ringbuf.is_empty()); @@ -346,14 +331,14 @@ mod tests { ringbuf.ndtr = 6; // The dma controller has written 6 + 2 bytes and has reloaded NDTR - ctrl.tcif = true; + ctrl.complete_count = 1; ctrl.set_next_ndtr(14); let mut buf = [0; 2]; assert_eq!(2, ringbuf.read(&mut ctrl, &mut buf).unwrap()); assert_eq!([2, 3], buf); - assert_eq!(true, ctrl.tcif); // The interrupt flag IS NOT cleared + assert_eq!(1, ctrl.complete_count); // The interrupt flag IS NOT cleared } #[test] @@ -365,14 +350,14 @@ mod tests { ringbuf.ndtr = 10; // The dma controller has written 6 + 2 bytes and has reloaded NDTR - ctrl.tcif = true; + ctrl.complete_count = 1; ctrl.set_next_ndtr(14); let mut buf = [0; 10]; assert_eq!(10, ringbuf.read(&mut ctrl, &mut buf).unwrap()); assert_eq!([12, 13, 14, 15, 0, 1, 2, 3, 4, 5], buf); - assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + assert_eq!(0, ctrl.complete_count); // The interrupt flag IS cleared } #[test] @@ -387,12 +372,12 @@ mod tests { assert!(ringbuf.is_empty()); // The ring buffer thinks that it is empty // The dma controller has written exactly 16 bytes - ctrl.tcif = true; + ctrl.complete_count = 1; let mut buf = [0; 2]; assert_eq!(Err(OverrunError), ringbuf.read(&mut ctrl, &mut buf)); - assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + assert_eq!(1, ctrl.complete_count); // The complete counter is not reset } #[test] @@ -404,13 +389,13 @@ mod tests { ringbuf.ndtr = 6; // The dma controller has written 6 + 3 bytes and has reloaded NDTR - ctrl.tcif = true; + ctrl.complete_count = 1; ctrl.set_next_ndtr(13); let mut buf = [0; 2]; assert_eq!(Err(OverrunError), ringbuf.read(&mut ctrl, &mut buf)); - assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + assert_eq!(1, ctrl.complete_count); // The complete counter is not reset } #[test] @@ -422,12 +407,12 @@ mod tests { ringbuf.ndtr = 10; // The dma controller has written 6 + 13 bytes and has reloaded NDTR - ctrl.tcif = true; + ctrl.complete_count = 1; ctrl.set_next_ndtr(3); let mut buf = [0; 2]; assert_eq!(Err(OverrunError), ringbuf.read(&mut ctrl, &mut buf)); - assert_eq!(false, ctrl.tcif); // The interrupt flag IS cleared + assert_eq!(1, ctrl.complete_count); // The complete counter is not reset } } diff --git a/embassy-stm32/src/usart/rx_ringbuffered.rs b/embassy-stm32/src/usart/rx_ringbuffered.rs index 0dc90ece7..dc21f557b 100644 --- a/embassy-stm32/src/usart/rx_ringbuffered.rs +++ b/embassy-stm32/src/usart/rx_ringbuffered.rs @@ -4,8 +4,9 @@ use core::task::Poll; use embassy_hal_common::drop::OnDrop; use embassy_hal_common::PeripheralRef; +use futures::future::{select, Either}; -use super::{rdr, sr, BasicInstance, Error, UartRx}; +use super::{clear_interrupt_flags, rdr, sr, BasicInstance, Error, UartRx}; use crate::dma::ringbuffer::OverrunError; use crate::dma::RingBuffer; @@ -98,7 +99,8 @@ impl<'d, T: BasicInstance, RxDma: super::RxDma> RingBufferedUartRx<'d, T, RxD } /// Read bytes that are readily available in the ring buffer. - /// If no bytes are currently available in the buffer the call waits until data are received. + /// If no bytes are currently available in the buffer the call waits until the some + /// bytes are available (at least one byte and at most half the buffer size) /// /// Background receive is started if `start()` has not been previously called. /// @@ -107,10 +109,9 @@ impl<'d, T: BasicInstance, RxDma: super::RxDma> RingBufferedUartRx<'d, T, RxD pub async fn read(&mut self, buf: &mut [u8]) -> Result { let r = T::regs(); + // Start background receive if it was not already started // SAFETY: read only let is_started = unsafe { r.cr3().read().dmar() }; - - // Start background receive if it was not already started if !is_started { self.start()?; } @@ -132,8 +133,7 @@ impl<'d, T: BasicInstance, RxDma: super::RxDma> RingBufferedUartRx<'d, T, RxD } } - let ndtr = self.ring_buf.get_remaining_transfers(); - self.ring_buf.set_ndtr(ndtr); + self.ring_buf.reload_position(); match self.ring_buf.read(buf) { Ok(len) if len == 0 => {} Ok(len) => { @@ -148,28 +148,32 @@ impl<'d, T: BasicInstance, RxDma: super::RxDma> RingBufferedUartRx<'d, T, RxD } } - // Wait for any data since `ndtr` - self.wait_for_data(ndtr).await?; + loop { + self.wait_for_data_or_idle().await?; + + self.ring_buf.reload_position(); + if !self.ring_buf.is_empty() { + break; + } + } - // ndtr is now different than the value provided to `wait_for_data()` - // Re-sample ndtr now when it has changed. - self.ring_buf.set_ndtr(self.ring_buf.get_remaining_transfers()); let len = self.ring_buf.read(buf).map_err(|_err| Error::Overrun)?; assert!(len > 0); + Ok(len) } - /// Wait for uart data - async fn wait_for_data(&mut self, old_ndtr: usize) -> Result<(), Error> { + /// Wait for uart idle or dma half-full or full + async fn wait_for_data_or_idle(&mut self) -> Result<(), Error> { let r = T::regs(); - // make sure USART state is restored to neutral state when this future is dropped - let _drop = OnDrop::new(move || { + // make sure USART state is restored to neutral state + let _on_drop = OnDrop::new(move || { // SAFETY: only clears Rx related flags unsafe { r.cr1().modify(|w| { - // disable RXNE interrupt - w.set_rxneie(false); + // disable idle line interrupt + w.set_idleie(false); }); } }); @@ -177,76 +181,65 @@ impl<'d, T: BasicInstance, RxDma: super::RxDma> RingBufferedUartRx<'d, T, RxD // SAFETY: only sets Rx related flags unsafe { r.cr1().modify(|w| { - // enable RXNE interrupt - w.set_rxneie(true); + // enable idle line interrupt + w.set_idleie(true); }); } - // future which completes when RX "not empty" is detected, - // i.e. when there is data in uart rx register - let rxne = poll_fn(|cx| { - let s = T::state(); + compiler_fence(Ordering::SeqCst); - // Register waker to be awaken when RXNE interrupt is received + // Future which completes when there is dma is half full or full + let dma = poll_fn(|cx| { + self.ring_buf.set_waker(cx.waker()); + + compiler_fence(Ordering::SeqCst); + + self.ring_buf.reload_position(); + if !self.ring_buf.is_empty() { + // Some data is now available + Poll::Ready(()) + } else { + Poll::Pending + } + }); + + // Future which completes when idle line is detected + let uart = poll_fn(|cx| { + let s = T::state(); s.rx_waker.register(cx.waker()); compiler_fence(Ordering::SeqCst); // SAFETY: read only and we only use Rx related flags - let s = unsafe { sr(r).read() }; - let has_errors = s.pe() || s.fe() || s.ne() || s.ore(); + let sr = unsafe { sr(r).read() }; + + let has_errors = sr.pe() || sr.fe() || sr.ne() || sr.ore(); if has_errors { - if s.pe() { + if sr.pe() { return Poll::Ready(Err(Error::Parity)); - } else if s.fe() { + } else if sr.fe() { return Poll::Ready(Err(Error::Framing)); - } else if s.ne() { + } else if sr.ne() { return Poll::Ready(Err(Error::Noise)); } else { return Poll::Ready(Err(Error::Overrun)); } } - // Re-sample ndtr and determine if it has changed since we started - // waiting for data. - let new_ndtr = self.ring_buf.get_remaining_transfers(); - if new_ndtr != old_ndtr { - // Some data was received as NDTR has changed + if sr.idle() { + // Idle line is detected Poll::Ready(Ok(())) } else { - // It may be that the DMA controller is currently busy consuming the - // RX data register. We therefore wait register to become empty. - while unsafe { sr(r).read().rxne() } {} - - compiler_fence(Ordering::SeqCst); - - // Re-get again: This time we know that the DMA controller has consumed - // the current read register if it was busy doing so - let new_ndtr = self.ring_buf.get_remaining_transfers(); - if new_ndtr != old_ndtr { - // Some data was received as NDTR has changed - Poll::Ready(Ok(())) - } else { - Poll::Pending - } + Poll::Pending } }); - compiler_fence(Ordering::SeqCst); - - let new_ndtr = self.ring_buf.get_remaining_transfers(); - if new_ndtr != old_ndtr { - // Fast path - NDTR has already changed, no reason to poll - Ok(()) - } else { - // NDTR has not changed since we first read from the ring buffer - // Wait for RXNE interrupt... - match rxne.await { - Ok(()) => Ok(()), - Err(e) => { - self.teardown_uart(); - Err(e) - } + match select(dma, uart).await { + Either::Left(((), _)) => Ok(()), + Either::Right((Ok(()), _)) => Ok(()), + Either::Right((Err(e), _)) => { + self.teardown_uart(); + Err(e) } } } diff --git a/tests/stm32/src/bin/usart_rx_ringbuffered.rs b/tests/stm32/src/bin/usart_rx_ringbuffered.rs index 3ea8bfb7b..48dc25b0e 100644 --- a/tests/stm32/src/bin/usart_rx_ringbuffered.rs +++ b/tests/stm32/src/bin/usart_rx_ringbuffered.rs @@ -56,6 +56,7 @@ mod board { } const ONE_BYTE_DURATION_US: u32 = 9_000_000 / 115200; +const DMA_BUF_SIZE: usize = 64; #[embassy_executor::main] async fn main(spawner: Spawner) { @@ -114,7 +115,7 @@ async fn main(spawner: Spawner) { let usart = Uart::new(usart, rx, tx, irq, tx_dma, rx_dma, config); let (tx, rx) = usart.split(); - static mut DMA_BUF: [u8; 64] = [0; 64]; + static mut DMA_BUF: [u8; DMA_BUF_SIZE] = [0; DMA_BUF_SIZE]; let dma_buf = unsafe { DMA_BUF.as_mut() }; let rx = rx.into_ring_buffered(dma_buf); @@ -159,7 +160,14 @@ async fn receive_task(mut rx: RingBufferedUartRx<'static, board::Uart, board::Rx loop { let mut buf = [0; 100]; let max_len = 1 + (rng.next_u32() as usize % (buf.len() - 1)); - let received = rx.read(&mut buf[..max_len]).await.unwrap(); + let received = match rx.read(&mut buf[..max_len]).await { + Ok(r) => r, + Err(e) => { + error!("Test fail! read error: {:?}", e); + cortex_m::asm::bkpt(); + return; + } + }; if expected.is_none() { info!("Test started"); @@ -176,8 +184,11 @@ async fn receive_task(mut rx: RingBufferedUartRx<'static, board::Uart, board::Rx } if received < max_len { - let byte_count = rng.next_u32() % 64; - Timer::after(Duration::from_micros((byte_count * ONE_BYTE_DURATION_US) as _)).await; + let byte_count = rng.next_u32() % (DMA_BUF_SIZE as u32); + let random_delay_us = (byte_count * ONE_BYTE_DURATION_US) as u64; + if random_delay_us > 200 { + Timer::after(Duration::from_micros(random_delay_us - 200)).await; + } } i += 1; diff --git a/tests/utils/src/bin/saturate_serial.rs b/tests/utils/src/bin/saturate_serial.rs index 28480516d..18ca12fb7 100644 --- a/tests/utils/src/bin/saturate_serial.rs +++ b/tests/utils/src/bin/saturate_serial.rs @@ -1,18 +1,19 @@ use std::path::Path; use std::time::Duration; -use std::{env, io, thread}; +use std::{env, io, process, thread}; use rand::random; use serial::SerialPort; pub fn main() { if let Some(port_name) = env::args().nth(1) { - let sleep = env::args().position(|x| x == "--sleep").is_some(); + let idles = env::args().position(|x| x == "--idles").is_some(); println!("Saturating port {:?} with 115200 8N1", port_name); - println!("Sleep: {}", sleep); + println!("Idles: {}", idles); + println!("Process ID: {}", process::id()); let mut port = serial::open(&port_name).unwrap(); - if saturate(&mut port, sleep).is_err() { + if saturate(&mut port, idles).is_err() { eprintln!("Unable to saturate port"); } } else { @@ -23,7 +24,7 @@ pub fn main() { } } -fn saturate(port: &mut T, sleep: bool) -> io::Result<()> { +fn saturate(port: &mut T, idles: bool) -> io::Result<()> { port.reconfigure(&|settings| { settings.set_baud_rate(serial::Baud115200)?; settings.set_char_size(serial::Bits8); @@ -39,7 +40,7 @@ fn saturate(port: &mut T, sleep: bool) -> io::Result<()> { port.write_all(&buf)?; - if sleep { + if idles { let micros = (random::() % 1000) as u64; println!("Sleeping {}us", micros); port.flush().unwrap(); @@ -49,4 +50,4 @@ fn saturate(port: &mut T, sleep: bool) -> io::Result<()> { written += len; println!("Written: {}", written); } -} \ No newline at end of file +} From 775740590839904a6b16a8afaac9e7861226bf92 Mon Sep 17 00:00:00 2001 From: Rasmus Melchior Jacobsen Date: Thu, 27 Apr 2023 10:58:41 +0200 Subject: [PATCH 04/12] Remove unused import --- embassy-stm32/src/usart/rx_ringbuffered.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-stm32/src/usart/rx_ringbuffered.rs b/embassy-stm32/src/usart/rx_ringbuffered.rs index dc21f557b..8d3848269 100644 --- a/embassy-stm32/src/usart/rx_ringbuffered.rs +++ b/embassy-stm32/src/usart/rx_ringbuffered.rs @@ -6,7 +6,7 @@ use embassy_hal_common::drop::OnDrop; use embassy_hal_common::PeripheralRef; use futures::future::{select, Either}; -use super::{clear_interrupt_flags, rdr, sr, BasicInstance, Error, UartRx}; +use super::{rdr, sr, BasicInstance, Error, UartRx}; use crate::dma::ringbuffer::OverrunError; use crate::dma::RingBuffer; From 45843034ec7c53f3f619bf1dcc6c145c428b0601 Mon Sep 17 00:00:00 2001 From: Rasmus Melchior Jacobsen Date: Thu, 27 Apr 2023 11:59:51 +0200 Subject: [PATCH 05/12] Actually clear idle flag --- embassy-stm32/src/usart/rx_ringbuffered.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/embassy-stm32/src/usart/rx_ringbuffered.rs b/embassy-stm32/src/usart/rx_ringbuffered.rs index 8d3848269..33b750497 100644 --- a/embassy-stm32/src/usart/rx_ringbuffered.rs +++ b/embassy-stm32/src/usart/rx_ringbuffered.rs @@ -6,7 +6,7 @@ use embassy_hal_common::drop::OnDrop; use embassy_hal_common::PeripheralRef; use futures::future::{select, Either}; -use super::{rdr, sr, BasicInstance, Error, UartRx}; +use super::{clear_interrupt_flags, rdr, sr, BasicInstance, Error, UartRx}; use crate::dma::ringbuffer::OverrunError; use crate::dma::RingBuffer; @@ -213,6 +213,13 @@ impl<'d, T: BasicInstance, RxDma: super::RxDma> RingBufferedUartRx<'d, T, RxD // SAFETY: read only and we only use Rx related flags let sr = unsafe { sr(r).read() }; + // SAFETY: only clears Rx related flags + unsafe { + // This read also clears the error and idle interrupt flags on v1. + rdr(r).read_volatile(); + clear_interrupt_flags(r, sr); + } + let has_errors = sr.pe() || sr.fe() || sr.ne() || sr.ore(); if has_errors { if sr.pe() { From 14e0090cb1e60ce477ae4f080d0deb724dbc3b9e Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 1 May 2023 18:14:34 +0200 Subject: [PATCH 06/12] stm32/dma: remove separate process_tcif. --- embassy-stm32/src/dma/dma.rs | 46 +++++++----------------------------- 1 file changed, 9 insertions(+), 37 deletions(-) diff --git a/embassy-stm32/src/dma/dma.rs b/embassy-stm32/src/dma/dma.rs index 10ae20f5c..69a5ec4e4 100644 --- a/embassy-stm32/src/dma/dma.rs +++ b/embassy-stm32/src/dma/dma.rs @@ -8,14 +8,13 @@ use atomic_polyfill::AtomicUsize; use embassy_cortex_m::interrupt::Priority; use embassy_hal_common::{into_ref, Peripheral, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; -use pac::dma::regs; use super::ringbuffer::{DmaCtrl, DmaRingBuffer, OverrunError}; use super::word::{Word, WordSize}; use super::Dir; use crate::_generated::DMA_CHANNEL_COUNT; use crate::interrupt::{Interrupt, InterruptExt}; -use crate::pac::dma::vals; +use crate::pac::dma::{regs, vals}; use crate::{interrupt, pac}; #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -196,35 +195,18 @@ pub(crate) unsafe fn on_irq_inner(dma: pac::dma::Dma, channel_num: usize, index: wake = true; } - wake |= process_tcif(dma, channel_num, index); + if isr.tcif(channel_num % 4) && cr.read().tcie() { + // Acknowledge transfer complete interrupt + dma.ifcr(channel_num / 4).write(|w| w.set_tcif(channel_num % 4, true)); + STATE.complete_count[index].fetch_add(1, Ordering::Release); + wake = true; + } if wake { STATE.ch_wakers[index].wake(); } } -unsafe fn process_tcif(dma: pac::dma::Dma, channel_num: usize, index: usize) -> bool { - let isr_reg = dma.isr(channel_num / 4); - let cr_reg = dma.st(channel_num).cr(); - - // First, figure out if tcif is set without a cs. - if isr_reg.read().tcif(channel_num % 4) && cr_reg.read().tcie() { - // Make tcif test again within a cs to avoid race when incrementing complete_count. - critical_section::with(|_| { - if isr_reg.read().tcif(channel_num % 4) && cr_reg.read().tcie() { - // Acknowledge transfer complete interrupt - dma.ifcr(channel_num / 4).write(|w| w.set_tcif(channel_num % 4, true)); - STATE.complete_count[index].fetch_add(1, Ordering::Release); - true - } else { - false - } - }) - } else { - false - } -} - #[cfg(any(dma_v2, dmamux))] pub type Request = u8; #[cfg(not(any(dma_v2, dmamux)))] @@ -634,21 +616,11 @@ impl DmaCtrl for C { } fn get_complete_count(&self) -> usize { - let dma = self.regs(); - let channel_num = self.num(); - let index = self.index(); - // Manually process tcif in case transfer was completed and we are in a higher priority task. - unsafe { process_tcif(dma, channel_num, index) }; - STATE.complete_count[index].load(Ordering::Acquire) + STATE.complete_count[self.index()].load(Ordering::Acquire) } fn reset_complete_count(&mut self) -> usize { - let dma = self.regs(); - let channel_num = self.num(); - let index = self.index(); - // Manually process tcif in case transfer was completed and we are in a higher priority task. - unsafe { process_tcif(dma, channel_num, index) }; - STATE.complete_count[index].swap(0, Ordering::AcqRel) + STATE.complete_count[self.index()].swap(0, Ordering::AcqRel) } } From 25864ae4dc3e5a765f6a3e2bb52bceb4df2e0199 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 1 May 2023 18:14:53 +0200 Subject: [PATCH 07/12] stm32/bdma: add ringbuffer support. --- embassy-stm32/src/dma/bdma.rs | 187 +++++++++++++++++++++++++++++++++- 1 file changed, 183 insertions(+), 4 deletions(-) diff --git a/embassy-stm32/src/dma/bdma.rs b/embassy-stm32/src/dma/bdma.rs index a23bb8cd7..88df76ba7 100644 --- a/embassy-stm32/src/dma/bdma.rs +++ b/embassy-stm32/src/dma/bdma.rs @@ -3,18 +3,20 @@ use core::future::Future; use core::pin::Pin; use core::sync::atomic::{fence, Ordering}; -use core::task::{Context, Poll}; +use core::task::{Context, Poll, Waker}; +use atomic_polyfill::AtomicUsize; use embassy_cortex_m::interrupt::Priority; use embassy_hal_common::{into_ref, Peripheral, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; +use super::ringbuffer::{DmaCtrl, DmaRingBuffer, OverrunError}; use super::word::{Word, WordSize}; use super::Dir; use crate::_generated::BDMA_CHANNEL_COUNT; use crate::interrupt::{Interrupt, InterruptExt}; use crate::pac; -use crate::pac::bdma::vals; +use crate::pac::bdma::{regs, vals}; #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] @@ -48,13 +50,16 @@ impl From

for vals::Dir { struct State { ch_wakers: [AtomicWaker; BDMA_CHANNEL_COUNT], + complete_count: [AtomicUsize; BDMA_CHANNEL_COUNT], } impl State { const fn new() -> Self { + const ZERO: AtomicUsize = AtomicUsize::new(0); const AW: AtomicWaker = AtomicWaker::new(); Self { ch_wakers: [AW; BDMA_CHANNEL_COUNT], + complete_count: [ZERO; BDMA_CHANNEL_COUNT], } } } @@ -105,8 +110,23 @@ pub(crate) unsafe fn on_irq_inner(dma: pac::bdma::Dma, channel_num: usize, index if isr.teif(channel_num) { panic!("DMA: error on BDMA@{:08x} channel {}", dma.0 as u32, channel_num); } + + let mut wake = false; + + if isr.htif(channel_num) && cr.read().htie() { + // Acknowledge half transfer complete interrupt + dma.ifcr().write(|w| w.set_htif(channel_num, true)); + wake = true; + } + if isr.tcif(channel_num) && cr.read().tcie() { - cr.write(|_| ()); // Disable channel interrupts with the default value. + // Acknowledge transfer complete interrupt + dma.ifcr().write(|w| w.set_tcif(channel_num, true)); + STATE.complete_count[index].fetch_add(1, Ordering::Release); + wake = true; + } + + if wake { STATE.ch_wakers[index].wake(); } } @@ -252,6 +272,7 @@ impl<'a, C: Channel> Transfer<'a, C> { let mut this = Self { channel }; this.clear_irqs(); + STATE.complete_count[this.channel.index()].store(0, Ordering::Release); #[cfg(dmamux)] super::dmamux::configure_dmamux(&mut *this.channel, _request); @@ -299,7 +320,9 @@ impl<'a, C: Channel> Transfer<'a, C> { pub fn is_running(&mut self) -> bool { let ch = self.channel.regs().ch(self.channel.num()); - unsafe { ch.cr().read() }.en() + let en = unsafe { ch.cr().read() }.en(); + let tcif = STATE.complete_count[self.channel.index()].load(Ordering::Acquire) != 0; + en && !tcif } /// Gets the total remaining transfers for the channel @@ -342,3 +365,159 @@ impl<'a, C: Channel> Future for Transfer<'a, C> { } } } + +// ============================== + +impl DmaCtrl for C { + fn ndtr(&self) -> usize { + let ch = self.regs().ch(self.num()); + unsafe { ch.ndtr().read() }.ndt() as usize + } + + fn get_complete_count(&self) -> usize { + STATE.complete_count[self.index()].load(Ordering::Acquire) + } + + fn reset_complete_count(&mut self) -> usize { + STATE.complete_count[self.index()].swap(0, Ordering::AcqRel) + } +} + +pub struct RingBuffer<'a, C: Channel, W: Word> { + cr: regs::Cr, + channel: PeripheralRef<'a, C>, + ringbuf: DmaRingBuffer<'a, W>, +} + +impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { + pub unsafe fn new_read( + channel: impl Peripheral

+ 'a, + _request: Request, + peri_addr: *mut W, + buffer: &'a mut [W], + _options: TransferOptions, + ) -> Self { + into_ref!(channel); + + let len = buffer.len(); + assert!(len > 0 && len <= 0xFFFF); + + let dir = Dir::PeripheralToMemory; + let data_size = W::size(); + + let channel_number = channel.num(); + let dma = channel.regs(); + + // "Preceding reads and writes cannot be moved past subsequent writes." + fence(Ordering::SeqCst); + + #[cfg(bdma_v2)] + critical_section::with(|_| channel.regs().cselr().modify(|w| w.set_cs(channel.num(), _request))); + + let mut w = regs::Cr(0); + w.set_psize(data_size.into()); + w.set_msize(data_size.into()); + w.set_minc(vals::Inc::ENABLED); + w.set_dir(dir.into()); + w.set_teie(true); + w.set_htie(true); + w.set_tcie(true); + w.set_circ(vals::Circ::ENABLED); + w.set_pl(vals::Pl::VERYHIGH); + w.set_en(true); + + let buffer_ptr = buffer.as_mut_ptr(); + let mut this = Self { + channel, + cr: w, + ringbuf: DmaRingBuffer::new(buffer), + }; + this.clear_irqs(); + + #[cfg(dmamux)] + super::dmamux::configure_dmamux(&mut *this.channel, _request); + + let ch = dma.ch(channel_number); + ch.par().write_value(peri_addr as u32); + ch.mar().write_value(buffer_ptr as u32); + ch.ndtr().write(|w| w.set_ndt(len as u16)); + + this + } + + pub fn start(&mut self) { + let ch = self.channel.regs().ch(self.channel.num()); + unsafe { ch.cr().write_value(self.cr) } + } + + pub fn clear(&mut self) { + self.ringbuf.clear(&mut *self.channel); + } + + /// Read bytes from the ring buffer + /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. + pub fn read(&mut self, buf: &mut [W]) -> Result { + self.ringbuf.read(&mut *self.channel, buf) + } + + pub fn is_empty(&self) -> bool { + self.ringbuf.is_empty() + } + + pub fn len(&self) -> usize { + self.ringbuf.len() + } + + pub fn capacity(&self) -> usize { + self.ringbuf.dma_buf.len() + } + + pub fn set_waker(&mut self, waker: &Waker) { + STATE.ch_wakers[self.channel.index()].register(waker); + } + + fn clear_irqs(&mut self) { + let dma = self.channel.regs(); + unsafe { + dma.ifcr().write(|w| { + w.set_htif(self.channel.num(), true); + w.set_tcif(self.channel.num(), true); + w.set_teif(self.channel.num(), true); + }) + } + } + + pub fn request_stop(&mut self) { + let ch = self.channel.regs().ch(self.channel.num()); + + // Disable the channel. Keep the IEs enabled so the irqs still fire. + unsafe { + ch.cr().write(|w| { + w.set_teie(true); + w.set_htie(true); + w.set_tcie(true); + }) + } + } + + pub fn is_running(&mut self) -> bool { + let ch = self.channel.regs().ch(self.channel.num()); + unsafe { ch.cr().read() }.en() + } + + /// Synchronize the position of the ring buffer to the actual DMA controller position + pub fn reload_position(&mut self) { + let ch = self.channel.regs().ch(self.channel.num()); + self.ringbuf.ndtr = unsafe { ch.ndtr().read() }.ndt() as usize; + } +} + +impl<'a, C: Channel, W: Word> Drop for RingBuffer<'a, C, W> { + fn drop(&mut self) { + self.request_stop(); + while self.is_running() {} + + // "Subsequent reads and writes cannot be moved ahead of preceding reads." + fence(Ordering::SeqCst); + } +} From 96e8a7ddb9b768a2827dffa5c72723b1075381ad Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 1 May 2023 18:15:46 +0200 Subject: [PATCH 08/12] stm32/uart: feature-gate ringbuffer out when using gpdma, not supported yet. --- embassy-stm32/src/dma/ringbuffer.rs | 2 ++ embassy-stm32/src/usart/mod.rs | 3 +++ tests/stm32/Cargo.toml | 20 +++++++++++++------- tests/stm32/src/bin/usart_rx_ringbuffered.rs | 2 ++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/embassy-stm32/src/dma/ringbuffer.rs b/embassy-stm32/src/dma/ringbuffer.rs index 544ec9461..02964eb62 100644 --- a/embassy-stm32/src/dma/ringbuffer.rs +++ b/embassy-stm32/src/dma/ringbuffer.rs @@ -1,3 +1,5 @@ +#![cfg_attr(gpdma, allow(unused))] + use core::ops::Range; use core::sync::atomic::{compiler_fence, Ordering}; diff --git a/embassy-stm32/src/usart/mod.rs b/embassy-stm32/src/usart/mod.rs index f3dea3391..ad450f2b3 100644 --- a/embassy-stm32/src/usart/mod.rs +++ b/embassy-stm32/src/usart/mod.rs @@ -980,7 +980,10 @@ mod eio { pub use buffered::*; #[cfg(feature = "nightly")] mod buffered; + +#[cfg(not(gpdma))] mod rx_ringbuffered; +#[cfg(not(gpdma))] pub use rx_ringbuffered::RingBufferedUartRx; #[cfg(usart_v1)] diff --git a/tests/stm32/Cargo.toml b/tests/stm32/Cargo.toml index 240fad522..eca470358 100644 --- a/tests/stm32/Cargo.toml +++ b/tests/stm32/Cargo.toml @@ -5,18 +5,19 @@ version = "0.1.0" license = "MIT OR Apache-2.0" [features] -stm32f103c8 = ["embassy-stm32/stm32f103c8"] # Blue Pill -stm32f429zi = ["embassy-stm32/stm32f429zi", "sdmmc", "chrono"] # Nucleo -stm32g071rb = ["embassy-stm32/stm32g071rb"] # Nucleo -stm32c031c6 = ["embassy-stm32/stm32c031c6"] # Nucleo -stm32g491re = ["embassy-stm32/stm32g491re"] # Nucleo -stm32h755zi = ["embassy-stm32/stm32h755zi-cm7"] # Nucleo -stm32wb55rg = ["embassy-stm32/stm32wb55rg"] # Nucleo +stm32f103c8 = ["embassy-stm32/stm32f103c8", "not-gpdma"] # Blue Pill +stm32f429zi = ["embassy-stm32/stm32f429zi", "sdmmc", "chrono", "not-gpdma"] # Nucleo +stm32g071rb = ["embassy-stm32/stm32g071rb", "not-gpdma"] # Nucleo +stm32c031c6 = ["embassy-stm32/stm32c031c6", "not-gpdma"] # Nucleo +stm32g491re = ["embassy-stm32/stm32g491re", "not-gpdma"] # Nucleo +stm32h755zi = ["embassy-stm32/stm32h755zi-cm7", "not-gpdma"] # Nucleo +stm32wb55rg = ["embassy-stm32/stm32wb55rg", "not-gpdma"] # Nucleo stm32h563zi = ["embassy-stm32/stm32h563zi"] # Nucleo stm32u585ai = ["embassy-stm32/stm32u585ai"] # IoT board sdmmc = [] chrono = ["embassy-stm32/chrono", "dep:chrono"] +not-gpdma = [] [dependencies] embassy-sync = { version = "0.2.0", path = "../../embassy-sync", features = ["defmt"] } @@ -80,6 +81,11 @@ name = "usart_dma" path = "src/bin/usart_dma.rs" required-features = [] +[[bin]] +name = "usart_rx_ringbuffered" +path = "src/bin/usart_rx_ringbuffered.rs" +required-features = [ "not-gpdma",] + # END TESTS [profile.dev] diff --git a/tests/stm32/src/bin/usart_rx_ringbuffered.rs b/tests/stm32/src/bin/usart_rx_ringbuffered.rs index 48dc25b0e..86bcfab8d 100644 --- a/tests/stm32/src/bin/usart_rx_ringbuffered.rs +++ b/tests/stm32/src/bin/usart_rx_ringbuffered.rs @@ -1,3 +1,5 @@ +// required-features: not-gpdma + #![no_std] #![no_main] #![feature(type_alias_impl_trait)] From 00cde67abe23aa299f94fb6e1aa6cb2c3de69788 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 1 May 2023 19:10:00 +0200 Subject: [PATCH 09/12] stm32/dma: solve overlapping impl on DmaCtrl on stm32h7 --- embassy-stm32/src/dma/bdma.rs | 14 ++++++++------ embassy-stm32/src/dma/dma.rs | 14 ++++++++------ embassy-stm32/src/dma/ringbuffer.rs | 6 +++--- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/embassy-stm32/src/dma/bdma.rs b/embassy-stm32/src/dma/bdma.rs index 88df76ba7..0202ec379 100644 --- a/embassy-stm32/src/dma/bdma.rs +++ b/embassy-stm32/src/dma/bdma.rs @@ -368,18 +368,20 @@ impl<'a, C: Channel> Future for Transfer<'a, C> { // ============================== -impl DmaCtrl for C { +struct DmaCtrlImpl<'a, C: Channel>(PeripheralRef<'a, C>); + +impl<'a, C: Channel> DmaCtrl for DmaCtrlImpl<'a, C> { fn ndtr(&self) -> usize { - let ch = self.regs().ch(self.num()); + let ch = self.0.regs().ch(self.0.num()); unsafe { ch.ndtr().read() }.ndt() as usize } fn get_complete_count(&self) -> usize { - STATE.complete_count[self.index()].load(Ordering::Acquire) + STATE.complete_count[self.0.index()].load(Ordering::Acquire) } fn reset_complete_count(&mut self) -> usize { - STATE.complete_count[self.index()].swap(0, Ordering::AcqRel) + STATE.complete_count[self.0.index()].swap(0, Ordering::AcqRel) } } @@ -451,13 +453,13 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { } pub fn clear(&mut self) { - self.ringbuf.clear(&mut *self.channel); + self.ringbuf.clear(DmaCtrlImpl(self.channel.reborrow())); } /// Read bytes from the ring buffer /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. pub fn read(&mut self, buf: &mut [W]) -> Result { - self.ringbuf.read(&mut *self.channel, buf) + self.ringbuf.read(DmaCtrlImpl(self.channel.reborrow()), buf) } pub fn is_empty(&self) -> bool { diff --git a/embassy-stm32/src/dma/dma.rs b/embassy-stm32/src/dma/dma.rs index 69a5ec4e4..7b17d9e49 100644 --- a/embassy-stm32/src/dma/dma.rs +++ b/embassy-stm32/src/dma/dma.rs @@ -609,18 +609,20 @@ impl<'a, C: Channel, W: Word> Drop for DoubleBuffered<'a, C, W> { // ============================== -impl DmaCtrl for C { +struct DmaCtrlImpl<'a, C: Channel>(PeripheralRef<'a, C>); + +impl<'a, C: Channel> DmaCtrl for DmaCtrlImpl<'a, C> { fn ndtr(&self) -> usize { - let ch = self.regs().st(self.num()); + let ch = self.0.regs().st(self.0.num()); unsafe { ch.ndtr().read() }.ndt() as usize } fn get_complete_count(&self) -> usize { - STATE.complete_count[self.index()].load(Ordering::Acquire) + STATE.complete_count[self.0.index()].load(Ordering::Acquire) } fn reset_complete_count(&mut self) -> usize { - STATE.complete_count[self.index()].swap(0, Ordering::AcqRel) + STATE.complete_count[self.0.index()].swap(0, Ordering::AcqRel) } } @@ -707,13 +709,13 @@ impl<'a, C: Channel, W: Word> RingBuffer<'a, C, W> { } pub fn clear(&mut self) { - self.ringbuf.clear(&mut *self.channel); + self.ringbuf.clear(DmaCtrlImpl(self.channel.reborrow())); } /// Read bytes from the ring buffer /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. pub fn read(&mut self, buf: &mut [W]) -> Result { - self.ringbuf.read(&mut *self.channel, buf) + self.ringbuf.read(DmaCtrlImpl(self.channel.reborrow()), buf) } pub fn is_empty(&self) -> bool { diff --git a/embassy-stm32/src/dma/ringbuffer.rs b/embassy-stm32/src/dma/ringbuffer.rs index 02964eb62..38cc87ae9 100644 --- a/embassy-stm32/src/dma/ringbuffer.rs +++ b/embassy-stm32/src/dma/ringbuffer.rs @@ -63,7 +63,7 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { } /// Reset the ring buffer to its initial state - pub fn clear(&mut self, dma: &mut impl DmaCtrl) { + pub fn clear(&mut self, mut dma: impl DmaCtrl) { self.first = 0; self.ndtr = self.dma_buf.len(); dma.reset_complete_count(); @@ -94,7 +94,7 @@ impl<'a, W: Word> DmaRingBuffer<'a, W> { /// Read bytes from the ring buffer /// OverrunError is returned if the portion to be read was overwritten by the DMA controller. - pub fn read(&mut self, dma: &mut impl DmaCtrl, buf: &mut [W]) -> Result { + pub fn read(&mut self, mut dma: impl DmaCtrl, buf: &mut [W]) -> Result { let end = self.end(); compiler_fence(Ordering::SeqCst); @@ -244,7 +244,7 @@ mod tests { } } - impl DmaCtrl for TestCtrl { + impl DmaCtrl for &mut TestCtrl { fn ndtr(&self) -> usize { self.next_ndtr.borrow_mut().unwrap() } From 1806422763be4e9e47201f741a03e5968fc8dedb Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 1 May 2023 18:16:29 +0200 Subject: [PATCH 10/12] stm32/test: add real defmt timestamp --- tests/stm32/Cargo.toml | 3 ++- tests/stm32/src/example_common.rs | 11 ----------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/stm32/Cargo.toml b/tests/stm32/Cargo.toml index eca470358..5cd949661 100644 --- a/tests/stm32/Cargo.toml +++ b/tests/stm32/Cargo.toml @@ -22,8 +22,9 @@ not-gpdma = [] [dependencies] embassy-sync = { version = "0.2.0", path = "../../embassy-sync", features = ["defmt"] } embassy-executor = { version = "0.2.0", path = "../../embassy-executor", features = ["arch-cortex-m", "executor-thread", "defmt", "integrated-timers"] } -embassy-time = { version = "0.1.0", path = "../../embassy-time", features = ["defmt", "tick-hz-32_768"] } +embassy-time = { version = "0.1.0", path = "../../embassy-time", features = ["defmt", "tick-hz-32_768", "defmt-timestamp-uptime"] } embassy-stm32 = { version = "0.1.0", path = "../../embassy-stm32", features = ["nightly", "defmt", "unstable-pac", "memory-x", "time-driver-any"] } +embassy-futures = { version = "0.1.0", path = "../../embassy-futures" } defmt = "0.3.0" defmt-rtt = "0.4" diff --git a/tests/stm32/src/example_common.rs b/tests/stm32/src/example_common.rs index c47ed75c4..a4f8668c7 100644 --- a/tests/stm32/src/example_common.rs +++ b/tests/stm32/src/example_common.rs @@ -1,22 +1,11 @@ #![macro_use] -use core::sync::atomic::{AtomicUsize, Ordering}; - pub use defmt::*; #[allow(unused)] use embassy_stm32::time::Hertz; use embassy_stm32::Config; use {defmt_rtt as _, panic_probe as _}; -defmt::timestamp! {"{=u64}", { - static COUNT: AtomicUsize = AtomicUsize::new(0); - // NOTE(no-CAS) `timestamps` runs with interrupts disabled - let n = COUNT.load(Ordering::Relaxed); - COUNT.store(n + 1, Ordering::Relaxed); - n as u64 - } -} - pub fn config() -> Config { #[allow(unused_mut)] let mut config = Config::default(); From 76017796933f0335bcdadbd6b765721833fdacec Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 1 May 2023 18:17:02 +0200 Subject: [PATCH 11/12] stm32/test: cleanup ringbuffer test, exit on success (transferring 100kb) --- tests/stm32/src/bin/usart_rx_ringbuffered.rs | 77 ++++++++++---------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/tests/stm32/src/bin/usart_rx_ringbuffered.rs b/tests/stm32/src/bin/usart_rx_ringbuffered.rs index 86bcfab8d..2c4a8fdf4 100644 --- a/tests/stm32/src/bin/usart_rx_ringbuffered.rs +++ b/tests/stm32/src/bin/usart_rx_ringbuffered.rs @@ -6,6 +6,7 @@ #[path = "../example_common.rs"] mod example_common; +use defmt::{assert_eq, panic}; use embassy_executor::Spawner; use embassy_stm32::interrupt; use embassy_stm32::usart::{Config, DataBits, Parity, RingBufferedUartRx, StopBits, Uart, UartTx}; @@ -34,9 +35,9 @@ mod board { } #[cfg(feature = "stm32f429zi")] mod board { - pub type Uart = embassy_stm32::peripherals::USART2; - pub type TxDma = embassy_stm32::peripherals::DMA1_CH6; - pub type RxDma = embassy_stm32::peripherals::DMA1_CH5; + pub type Uart = embassy_stm32::peripherals::USART6; + pub type TxDma = embassy_stm32::peripherals::DMA2_CH6; + pub type RxDma = embassy_stm32::peripherals::DMA2_CH1; } #[cfg(feature = "stm32wb55rg")] mod board { @@ -56,9 +57,14 @@ mod board { pub type TxDma = embassy_stm32::peripherals::GPDMA1_CH0; pub type RxDma = embassy_stm32::peripherals::GPDMA1_CH1; } +#[cfg(feature = "stm32c031c6")] +mod board { + pub type Uart = embassy_stm32::peripherals::USART1; + pub type TxDma = embassy_stm32::peripherals::DMA1_CH1; + pub type RxDma = embassy_stm32::peripherals::DMA1_CH2; +} -const ONE_BYTE_DURATION_US: u32 = 9_000_000 / 115200; -const DMA_BUF_SIZE: usize = 64; +const DMA_BUF_SIZE: usize = 256; #[embassy_executor::main] async fn main(spawner: Spawner) { @@ -83,8 +89,14 @@ async fn main(spawner: Spawner) { let (tx, rx, usart, irq, tx_dma, rx_dma) = (p.PC4, p.PC5, p.USART1, interrupt::take!(USART1), p.DMA1_CH1, p.DMA1_CH2); #[cfg(feature = "stm32f429zi")] - let (tx, rx, usart, irq, tx_dma, rx_dma) = - (p.PA2, p.PA3, p.USART2, interrupt::take!(USART2), p.DMA1_CH6, p.DMA1_CH5); + let (tx, rx, usart, irq, tx_dma, rx_dma) = ( + p.PG14, + p.PG9, + p.USART6, + interrupt::take!(USART6), + p.DMA2_CH6, + p.DMA2_CH1, + ); #[cfg(feature = "stm32wb55rg")] let (tx, rx, usart, irq, tx_dma, rx_dma) = ( p.PA2, @@ -106,11 +118,16 @@ async fn main(spawner: Spawner) { p.GPDMA1_CH0, p.GPDMA1_CH1, ); + #[cfg(feature = "stm32c031c6")] + let (tx, rx, usart, irq, tx_dma, rx_dma) = + (p.PB6, p.PB7, p.USART1, interrupt::take!(USART1), p.DMA1_CH1, p.DMA1_CH2); // To run this test, use the saturating_serial test utility to saturate the serial port let mut config = Config::default(); - config.baudrate = 115200; + // this is the fastest we can go without tuning RCC + // some chips have default pclk=8mhz, and uart can run at max pclk/16 + config.baudrate = 500_000; config.data_bits = DataBits::DataBits8; config.stop_bits = StopBits::STOP1; config.parity = Parity::ParityNone; @@ -135,19 +152,14 @@ async fn transmit_task(mut tx: UartTx<'static, board::Uart, board::TxDma>) { let mut i: u8 = 0; loop { let mut buf = [0; 32]; - let len = 1 + (rng.next_u32() as usize % (buf.len() - 1)); + let len = 1 + (rng.next_u32() as usize % buf.len()); for b in &mut buf[..len] { *b = i; i = i.wrapping_add(1); } tx.write(&buf[..len]).await.unwrap(); - Timer::after(Duration::from_micros((rng.next_u32() % 10000) as _)).await; - - //i += 1; - //if i % 1000 == 0 { - // trace!("Wrote {} times", i); - //} + Timer::after(Duration::from_micros((rng.next_u32() % 1000) as _)).await; } } @@ -158,44 +170,31 @@ async fn receive_task(mut rx: RingBufferedUartRx<'static, board::Uart, board::Rx let mut rng = ChaCha8Rng::seed_from_u64(1337); let mut i = 0; - let mut expected: Option = None; + let mut expected = 0; loop { let mut buf = [0; 100]; - let max_len = 1 + (rng.next_u32() as usize % (buf.len() - 1)); + let max_len = 1 + (rng.next_u32() as usize % buf.len()); let received = match rx.read(&mut buf[..max_len]).await { Ok(r) => r, Err(e) => { - error!("Test fail! read error: {:?}", e); - cortex_m::asm::bkpt(); - return; + panic!("Test fail! read error: {:?}", e); } }; - if expected.is_none() { - info!("Test started"); - expected = Some(buf[0]); - } - for byte in &buf[..received] { - if byte != &expected.unwrap() { - error!("Test fail! received {}, expected {}", *byte, expected.unwrap()); - cortex_m::asm::bkpt(); - return; - } - expected = Some(expected.unwrap().wrapping_add(1)); + assert_eq!(*byte, expected); + expected = expected.wrapping_add(1); } if received < max_len { - let byte_count = rng.next_u32() % (DMA_BUF_SIZE as u32); - let random_delay_us = (byte_count * ONE_BYTE_DURATION_US) as u64; - if random_delay_us > 200 { - Timer::after(Duration::from_micros(random_delay_us - 200)).await; - } + Timer::after(Duration::from_micros((rng.next_u32() % 1000) as _)).await; } - i += 1; - if i % 1000 == 0 { - trace!("Read {} times", i); + i += received; + + if i > 100000 { + info!("Test OK!"); + cortex_m::asm::bkpt(); } } } From a1d45303c336434929eb8eb7e55629c504a95b0e Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 1 May 2023 18:17:29 +0200 Subject: [PATCH 12/12] stm32/test: fix race condition in uart_dma. --- tests/stm32/src/bin/usart_dma.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/stm32/src/bin/usart_dma.rs b/tests/stm32/src/bin/usart_dma.rs index d673df0f3..de6cd41d1 100644 --- a/tests/stm32/src/bin/usart_dma.rs +++ b/tests/stm32/src/bin/usart_dma.rs @@ -6,6 +6,7 @@ mod example_common; use defmt::assert_eq; use embassy_executor::Spawner; +use embassy_futures::join::join; use embassy_stm32::interrupt; use embassy_stm32::usart::{Config, Uart}; use example_common::*; @@ -76,18 +77,26 @@ async fn main(_spawner: Spawner) { (p.PB6, p.PB7, p.USART1, interrupt::take!(USART1), p.DMA1_CH1, p.DMA1_CH2); let config = Config::default(); - let mut usart = Uart::new(usart, rx, tx, irq, tx_dma, rx_dma, config); + let usart = Uart::new(usart, rx, tx, irq, tx_dma, rx_dma, 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. - // For whatever reason, blocking works with 2 bytes but DMA only with 1?? + const LEN: usize = 128; + let mut tx_buf = [0; LEN]; + let mut rx_buf = [0; LEN]; + for i in 0..LEN { + tx_buf[i] = i as u8; + } - let data = [0x42]; - usart.write(&data).await.unwrap(); + let (mut tx, mut rx) = usart.split(); - let mut buf = [0; 1]; - usart.read(&mut buf).await.unwrap(); - assert_eq!(buf, data); + let tx_fut = async { + tx.write(&tx_buf).await.unwrap(); + }; + let rx_fut = async { + rx.read(&mut rx_buf).await.unwrap(); + }; + join(rx_fut, tx_fut).await; + + assert_eq!(tx_buf, rx_buf); info!("Test OK"); cortex_m::asm::bkpt();