LailaTheElf/embassy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #3989 from embedded-rust-iml/fix/ringbuffered-error-handling

Browse Source 
Rework status handling (idle and errors) in ringbuffered uart
This commit is contained in:
Dario Nieuwenhuis 2025-03-31 15:09:01 +00:00 committed by GitHub
commit a44abaf7e4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 44 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
embassy-stm32/src/usart/mod.rs
View File

@ -18,11 +18,6 @@ use crate::gpio::{self, AfType, AnyPin, OutputType, Pull, SealedPin as _, Speed}
use crate::interrupt::typelevel::Interrupt as _;
use crate::interrupt::{self, Interrupt, InterruptExt};
use crate::mode::{Async, Blocking, Mode};
#[allow(unused_imports)]
#[cfg(not(any(usart_v1, usart_v2)))]
use crate::pac::usart::regs::Isr as Sr;
#[cfg(any(usart_v1, usart_v2))]
use crate::pac::usart::regs::Sr;
#[cfg(not(any(usart_v1, usart_v2)))]
use crate::pac::usart::Lpuart as Regs;
#[cfg(any(usart_v1, usart_v2))]
@ -403,7 +398,7 @@ pub struct UartRx<'d, M: Mode> {
rx_dma: Option<ChannelAndRequest<'d>>,
detect_previous_overrun: bool,
#[cfg(any(usart_v1, usart_v2))]
buffered_sr: stm32_metapac::usart::regs::Sr,
buffered_sr: regs::Sr,
_phantom: PhantomData<M>,
}
@ -950,7 +945,7 @@ impl<'d, M: Mode> UartRx<'d, M> {
rx_dma,
detect_previous_overrun: config.detect_previous_overrun,
#[cfg(any(usart_v1, usart_v2))]
buffered_sr: stm32_metapac::usart::regs::Sr(0),
buffered_sr: regs::Sr(0),
};
this.enable_and_configure(&config)?;
Ok(this)
@ -1449,7 +1444,7 @@ impl<'d, M: Mode> Uart<'d, M> {
rx_dma,
detect_previous_overrun: config.detect_previous_overrun,
#[cfg(any(usart_v1, usart_v2))]
buffered_sr: stm32_metapac::usart::regs::Sr(0),
buffered_sr: regs::Sr(0),
},
};
this.enable_and_configure(&config)?;

88
embassy-stm32/src/usart/ringbuffered.rs
View File

@ -7,16 +7,12 @@ use embassy_embedded_hal::SetConfig;
use embedded_io_async::ReadReady;
use futures_util::future::{select, Either};
use super::{
clear_interrupt_flags, rdr, reconfigure, set_baudrate, sr, Config, ConfigError, Error, Info, State, UartRx,
};
use super::{rdr, reconfigure, set_baudrate, sr, Config, ConfigError, Error, Info, State, UartRx};
use crate::dma::ReadableRingBuffer;
use crate::gpio::{AnyPin, SealedPin as _};
use crate::mode::Async;
#[cfg(any(usart_v3, usart_v4))]
use crate::pac::usart::regs;
use crate::time::Hertz;
use crate::usart::{Regs, Sr};
use crate::usart::Regs;
use crate::Peri;
/// Rx-only Ring-buffered UART Driver
@ -99,8 +95,6 @@ impl<'d> RingBufferedUartRx<'d> {
// enable idle line interrupt
w.set_idleie(true);
});
// Clear all potential error interrupt flags
clear_interrupt_flags(r, sr(r).read());
r.cr3().modify(|w| {
// enable Error Interrupt: (Frame error, Noise error, Overrun error)
w.set_eie(true);
@ -134,17 +128,15 @@ impl<'d> RingBufferedUartRx<'d> {
}
/// (Re-)start DMA and Uart if it is not running (has not been started yet or has failed), and
/// check for errors in status register. Error flags are cleared in `start_uart()` so they need
/// to be read first without returning yet.
/// check for errors in status register. Error flags are checked/cleared first.
fn start_dma_or_check_errors(&mut self) -> Result<(), Error> {
let r = self.info.regs;
let sr = clear_idle_flag(r);
let res = check_for_errors(sr);
check_idle_and_errors(r)?;
if !r.cr3().read().dmar() {
self.start_uart();
}
res
Ok(())
}
/// Read bytes that are readily available in the ring buffer.
@ -191,13 +183,7 @@ impl<'d> RingBufferedUartRx<'d> {
compiler_fence(Ordering::SeqCst);
// Critical section is needed so that IDLE isn't set after
// our read but before we clear it.
let sr = critical_section::with(|_| clear_idle_flag(self.info.regs));
check_for_errors(sr)?;
if sr.idle() {
if check_idle_and_errors(self.info.regs)? {
// Idle line is detected
Poll::Ready(Ok(()))
} else {
@ -240,41 +226,49 @@ impl Drop for RingBufferedUartRx<'_> {
}
}
/// Return an error result if the Sr register has errors
fn check_for_errors(s: Sr) -> Result<(), Error> {
if s.pe() {
/// Check and clear idle and error interrupts, return true if idle, Err(e) on error
///
/// All flags are read and cleared in a single step, respectively. When more than one flag is set
/// at the same time, all flags will be cleared but only one flag will be reported. So the other
/// flag(s) will gone missing unnoticed. The error flags are checked first, the idle flag last.
///
/// For usart_v1 and usart_v2, all status flags must be handled together anyway because all flags
/// are cleared by a single read to the RDR register.
fn check_idle_and_errors(r: Regs) -> Result<bool, Error> {
// Critical section is required so that the flags aren't set after read and before clear
let sr = critical_section::with(|_| {
// SAFETY: read only and we only use Rx related flags
let sr = sr(r).read();
#[cfg(any(usart_v3, usart_v4))]
r.icr().write(|w| {
w.set_idle(true);
w.set_pe(true);
w.set_fe(true);
w.set_ne(true);
w.set_ore(true);
});
#[cfg(not(any(usart_v3, usart_v4)))]
unsafe {
// This read also clears the error and idle interrupt flags on v1 (TODO and v2?)
rdr(r).read_volatile()
};
sr
});
if sr.pe() {
Err(Error::Parity)
} else if s.fe() {
} else if sr.fe() {
Err(Error::Framing)
} else if s.ne() {
} else if sr.ne() {
Err(Error::Noise)
} else if s.ore() {
} else if sr.ore() {
Err(Error::Overrun)
} else {
Ok(())
r.cr1().modify(|w| w.set_idleie(true));
Ok(sr.idle())
}
}
/// Clear IDLE and return the Sr register
fn clear_idle_flag(r: Regs) -> Sr {
// SAFETY: read only and we only use Rx related flags
let sr = sr(r).read();
// This read also clears the error and idle interrupt flags on v1.
unsafe { rdr(r).read_volatile() };
#[cfg(any(usart_v3, usart_v4))]
{
let mut clear_idle = regs::Icr(0);
clear_idle.set_idle(true);
r.icr().write_value(clear_idle);
}
r.cr1().modify(|w| w.set_idleie(true));
sr
}
impl embedded_io_async::ErrorType for RingBufferedUartRx<'_> {
type Error = Error;
}