From 907d55ea82ac09b507afdc7ccb4d5997f827a748 Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Thu, 19 Sep 2024 18:14:09 +0200 Subject: [PATCH 1/4] stm32: Added request_pause to DMA, and use it for RingBufferedUartRx --- embassy-stm32/src/dma/dma_bdma.rs | 54 ++++++++++++++++++++++++- embassy-stm32/src/usart/ringbuffered.rs | 2 +- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/embassy-stm32/src/dma/dma_bdma.rs b/embassy-stm32/src/dma/dma_bdma.rs index df041c4e9..2887536c3 100644 --- a/embassy-stm32/src/dma/dma_bdma.rs +++ b/embassy-stm32/src/dma/dma_bdma.rs @@ -493,6 +493,26 @@ impl AnyChannel { } } + fn request_pause(&self) { + let info = self.info(); + match self.info().dma { + #[cfg(dma)] + DmaInfo::Dma(r) => { + r.st(info.num).cr().modify(|w| { + // Disable the channel without overwriting the existing configuration + w.set_en(false); + }); + } + #[cfg(bdma)] + DmaInfo::Bdma(r) => { + r.ch(info.num).cr().modify(|w| { + // Disable the channel without overwriting the existing configuration + w.set_en(false); + }); + } + } + } + fn is_running(&self) -> bool { let info = self.info(); match self.info().dma { @@ -667,12 +687,22 @@ impl<'a> Transfer<'a> { } /// Request the transfer to stop. + /// The configuration for this channel will **not be preserved**. If you need to restart the transfer + /// at a later point with the same configuration, see [`request_pause`](Self::request_pause) instead. /// /// This doesn't immediately stop the transfer, you have to wait until [`is_running`](Self::is_running) returns false. pub fn request_stop(&mut self) { self.channel.request_stop() } + /// Request the transfer to pause, keeping the existing configuration for this channel. + /// To restart the transfer, call [`start`](Self::start) again. + /// + /// This doesn't immediately stop the transfer, you have to wait until [`is_running`](Self::is_running) returns false. + pub fn request_pause(&mut self) { + self.channel.request_pause() + } + /// Return whether this transfer is still running. /// /// If this returns `false`, it can be because either the transfer finished, or @@ -846,13 +876,23 @@ impl<'a, W: Word> ReadableRingBuffer<'a, W> { DmaCtrlImpl(self.channel.reborrow()).set_waker(waker); } - /// Request DMA to stop. + /// Request the DMA to stop. + /// The configuration for this channel will **not be preserved**. If you need to restart the transfer + /// at a later point with the same configuration, see [`request_pause`](Self::request_pause) instead. /// /// This doesn't immediately stop the transfer, you have to wait until [`is_running`](Self::is_running) returns false. pub fn request_stop(&mut self) { self.channel.request_stop() } + /// Request the transfer to pause, keeping the existing configuration for this channel. + /// To restart the transfer, call [`start`](Self::start) again. + /// + /// This doesn't immediately stop the transfer, you have to wait until [`is_running`](Self::is_running) returns false. + pub fn request_pause(&mut self) { + self.channel.request_pause() + } + /// Return whether DMA is still running. /// /// If this returns `false`, it can be because either the transfer finished, or @@ -977,13 +1017,23 @@ impl<'a, W: Word> WritableRingBuffer<'a, W> { DmaCtrlImpl(self.channel.reborrow()).set_waker(waker); } - /// Request DMA to stop. + /// Request the DMA to stop. + /// The configuration for this channel will **not be preserved**. If you need to restart the transfer + /// at a later point with the same configuration, see [`request_pause`](Self::request_pause) instead. /// /// This doesn't immediately stop the transfer, you have to wait until [`is_running`](Self::is_running) returns false. pub fn request_stop(&mut self) { self.channel.request_stop() } + /// Request the transfer to pause, keeping the existing configuration for this channel. + /// To restart the transfer, call [`start`](Self::start) again. + /// + /// This doesn't immediately stop the transfer, you have to wait until [`is_running`](Self::is_running) returns false. + pub fn request_pause(&mut self) { + self.channel.request_pause() + } + /// Return whether DMA is still running. /// /// If this returns `false`, it can be because either the transfer finished, or diff --git a/embassy-stm32/src/usart/ringbuffered.rs b/embassy-stm32/src/usart/ringbuffered.rs index b0652046c..bb95af966 100644 --- a/embassy-stm32/src/usart/ringbuffered.rs +++ b/embassy-stm32/src/usart/ringbuffered.rs @@ -120,7 +120,7 @@ impl<'d> RingBufferedUartRx<'d> { /// Stop uart background receive fn teardown_uart(&mut self) { - self.ring_buf.request_stop(); + self.ring_buf.request_pause(); let r = self.info.regs; // clear all interrupts and DMA Rx Request From 2a9cdaabaa315795722886f94fa553e8f3e3c14d Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Thu, 19 Sep 2024 18:25:08 +0200 Subject: [PATCH 2/4] stm32: Moved comment to match request_stop --- embassy-stm32/src/dma/dma_bdma.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/embassy-stm32/src/dma/dma_bdma.rs b/embassy-stm32/src/dma/dma_bdma.rs index 2887536c3..d10b5554f 100644 --- a/embassy-stm32/src/dma/dma_bdma.rs +++ b/embassy-stm32/src/dma/dma_bdma.rs @@ -498,15 +498,15 @@ impl AnyChannel { match self.info().dma { #[cfg(dma)] DmaInfo::Dma(r) => { + // Disable the channel without overwriting the existing configuration r.st(info.num).cr().modify(|w| { - // Disable the channel without overwriting the existing configuration w.set_en(false); }); } #[cfg(bdma)] DmaInfo::Bdma(r) => { + // Disable the channel without overwriting the existing configuration r.ch(info.num).cr().modify(|w| { - // Disable the channel without overwriting the existing configuration w.set_en(false); }); } From 4fcc8e39d6b3e486d2b94aa45d255d9c645d028a Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Thu, 19 Sep 2024 19:21:34 +0200 Subject: [PATCH 3/4] stm32: Only check errors on running RingBufferedUartRx, reduce number of small one-time functions --- embassy-stm32/src/usart/ringbuffered.rs | 49 ++++++++++--------------- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/embassy-stm32/src/usart/ringbuffered.rs b/embassy-stm32/src/usart/ringbuffered.rs index bb95af966..2d9c63820 100644 --- a/embassy-stm32/src/usart/ringbuffered.rs +++ b/embassy-stm32/src/usart/ringbuffered.rs @@ -71,33 +71,18 @@ impl<'d> UartRx<'d, Async> { } impl<'d> RingBufferedUartRx<'d> { - /// Clear the ring buffer and start receiving in the background - 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(()) - } - - fn stop(&mut self, err: Error) -> Result { - self.teardown_uart(); - - Err(err) - } - /// Reconfigure the driver pub fn set_config(&mut self, config: &Config) -> Result<(), ConfigError> { reconfigure(self.info, self.kernel_clock, config) } - /// Start uart background receive - fn setup_uart(&mut self) { - // fence before starting DMA. + /// Configure and start the DMA backed UART receiver + /// + /// Note: This is also done automatically by [`read()`] if required. + pub fn start_uart(&mut self) { + // Clear the buffer so that it is ready to receive data + self.ring_buf.clear(); compiler_fence(Ordering::SeqCst); - - // start the dma controller self.ring_buf.start(); let r = self.info.regs; @@ -118,8 +103,8 @@ impl<'d> RingBufferedUartRx<'d> { }); } - /// Stop uart background receive - fn teardown_uart(&mut self) { + /// Stop DMA backed UART receiver + fn stop_uart(&mut self) { self.ring_buf.request_pause(); let r = self.info.regs; @@ -153,13 +138,15 @@ impl<'d> RingBufferedUartRx<'d> { pub async fn read(&mut self, buf: &mut [u8]) -> Result { let r = self.info.regs; - // Start background receive if it was not already started + // Start DMA and Uart if it was not already started, + // otherwise check for errors in status register. + let sr = clear_idle_flag(r); if !r.cr3().read().dmar() { - self.start()?; + self.start_uart(); + } else { + check_for_errors(sr)?; } - check_for_errors(clear_idle_flag(r))?; - loop { match self.ring_buf.read(buf) { Ok((0, _)) => {} @@ -167,14 +154,16 @@ impl<'d> RingBufferedUartRx<'d> { return Ok(len); } Err(_) => { - return self.stop(Error::Overrun); + self.stop_uart(); + return Err(Error::Overrun); } } match self.wait_for_data_or_idle().await { Ok(_) => {} Err(err) => { - return self.stop(err); + self.stop_uart(); + return Err(err); } } } @@ -228,7 +217,7 @@ impl<'d> RingBufferedUartRx<'d> { impl Drop for RingBufferedUartRx<'_> { fn drop(&mut self) { - self.teardown_uart(); + self.stop_uart(); self.rx.as_ref().map(|x| x.set_as_disconnected()); self.rts.as_ref().map(|x| x.set_as_disconnected()); super::drop_tx_rx(self.info, self.state); From 3aeeeb0d784d843b521b4b8b222114ef7ba71363 Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Thu, 19 Sep 2024 20:07:08 +0200 Subject: [PATCH 4/4] stm32: Start DMA before clearing, avoid panic in updater ringbuffer impl --- embassy-stm32/src/usart/ringbuffered.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-stm32/src/usart/ringbuffered.rs b/embassy-stm32/src/usart/ringbuffered.rs index 2d9c63820..75834bf37 100644 --- a/embassy-stm32/src/usart/ringbuffered.rs +++ b/embassy-stm32/src/usart/ringbuffered.rs @@ -81,9 +81,9 @@ impl<'d> RingBufferedUartRx<'d> { /// Note: This is also done automatically by [`read()`] if required. pub fn start_uart(&mut self) { // Clear the buffer so that it is ready to receive data - self.ring_buf.clear(); compiler_fence(Ordering::SeqCst); self.ring_buf.start(); + self.ring_buf.clear(); let r = self.info.regs; // clear all interrupts and DMA Rx Request