Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

stm32: Fix RingBufferedUartRx hard-resetting DMA after initial error #3356

Merged
merged 4 commits into from
Sep 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 52 additions & 2 deletions embassy-stm32/src/dma/dma_bdma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,26 @@ impl AnyChannel {
}
}

fn request_pause(&self) {
let info = self.info();
match self.info().dma {
#[cfg(dma)]
DmaInfo::Dma(r) => {
// Disable the channel without overwriting the existing configuration
r.st(info.num).cr().modify(|w| {
w.set_en(false);
});
}
#[cfg(bdma)]
DmaInfo::Bdma(r) => {
// Disable the channel without overwriting the existing configuration
r.ch(info.num).cr().modify(|w| {
w.set_en(false);
});
}
}
}

fn is_running(&self) -> bool {
let info = self.info();
match self.info().dma {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
51 changes: 20 additions & 31 deletions embassy-stm32/src/usart/ringbuffered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,34 +71,19 @@ 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<usize, Error> {
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
compiler_fence(Ordering::SeqCst);

// start the dma controller
self.ring_buf.start();
self.ring_buf.clear();

let r = self.info.regs;
// clear all interrupts and DMA Rx Request
Expand All @@ -118,9 +103,9 @@ impl<'d> RingBufferedUartRx<'d> {
});
}

/// Stop uart background receive
fn teardown_uart(&mut self) {
self.ring_buf.request_stop();
/// Stop DMA backed UART receiver
fn stop_uart(&mut self) {
self.ring_buf.request_pause();

let r = self.info.regs;
// clear all interrupts and DMA Rx Request
Expand Down Expand Up @@ -153,28 +138,32 @@ impl<'d> RingBufferedUartRx<'d> {
pub async fn read(&mut self, buf: &mut [u8]) -> Result<usize, Error> {
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, _)) => {}
Ok((len, _)) => {
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);
}
}
}
Expand Down Expand Up @@ -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);
Expand Down