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

Merge #1453

Browse Source 
1453: stm32 uart: Fix error flag handling for blocking operations r=Dirbaio a=timokroeger

Clear and report the error flags one by one and pop the data byte only after all error flags were handled.

For v1/v2 we emulate the v3/v4 behaviour by buffering the status register because a read to the data register clears all flags at once which means we might loose all but the first error.

Only tested on stm32f3 discovery board with loopback. Let‘s see what CI says for the other families.
Fixes #1452 

Co-authored-by: Timo Kröger <timokroeger93@gmail.com>
This commit is contained in:
bors[bot] 2023-05-14 20:20:45 +00:00 committed by GitHub
commit cdd326284a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 82 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -109,6 +109,8 @@ pub struct UartRx<'d, T: BasicInstance, RxDma = NoDma> {
_peri: PeripheralRef<'d, T>,
rx_dma: PeripheralRef<'d, RxDma>,
detect_previous_overrun: bool,
#[cfg(any(usart_v1, usart_v2))]
buffered_sr: stm32_metapac::usart::regs::Sr,
}
impl<'d, T: BasicInstance, TxDma> UartTx<'d, T, TxDma> {
@ -275,6 +277,8 @@ impl<'d, T: BasicInstance, RxDma> UartRx<'d, T, RxDma> {
_peri: peri,
rx_dma,
detect_previous_overrun: config.detect_previous_overrun,
#[cfg(any(usart_v1, usart_v2))]
buffered_sr: stm32_metapac::usart::regs::Sr(0),
}
}
@ -335,6 +339,59 @@ impl<'d, T: BasicInstance, RxDma> UartRx<'d, T, RxDma> {
}
}
#[cfg(any(usart_v1, usart_v2))]
unsafe fn check_rx_flags(&mut self) -> Result<bool, Error> {
let r = T::regs();
loop {
// Handle all buffered error flags.
if self.buffered_sr.pe() {
self.buffered_sr.set_pe(false);
return Err(Error::Parity);
} else if self.buffered_sr.fe() {
self.buffered_sr.set_fe(false);
return Err(Error::Framing);
} else if self.buffered_sr.ne() {
self.buffered_sr.set_ne(false);
return Err(Error::Noise);
} else if self.buffered_sr.ore() {
self.buffered_sr.set_ore(false);
return Err(Error::Overrun);
} else if self.buffered_sr.rxne() {
self.buffered_sr.set_rxne(false);
return Ok(true);
} else {
// No error flags from previous iterations were set: Check the actual status register
let sr = r.sr().read();
if !sr.rxne() {
return Ok(false);
}
// Buffer the status register and let the loop handle the error flags.
self.buffered_sr = sr;
}
}
}
#[cfg(any(usart_v3, usart_v4))]
unsafe fn check_rx_flags(&mut self) -> Result<bool, Error> {
let r = T::regs();
let sr = r.isr().read();
if sr.pe() {
r.icr().write(|w| w.set_pe(true));
return Err(Error::Parity);
} else if sr.fe() {
r.icr().write(|w| w.set_fe(true));
return Err(Error::Framing);
} else if sr.ne() {
r.icr().write(|w| w.set_ne(true));
return Err(Error::Noise);
} else if sr.ore() {
r.icr().write(|w| w.set_ore(true));
return Err(Error::Overrun);
}
Ok(sr.rxne())
}
pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error>
where
RxDma: crate::usart::RxDma<T>,
@ -347,20 +404,7 @@ impl<'d, T: BasicInstance, RxDma> UartRx<'d, T, RxDma> {
pub fn nb_read(&mut self) -> Result<u8, nb::Error<Error>> {
let r = T::regs();
unsafe {
let sr = sr(r).read();
if sr.pe() {
rdr(r).read_volatile();
Err(nb::Error::Other(Error::Parity))
} else if sr.fe() {
rdr(r).read_volatile();
Err(nb::Error::Other(Error::Framing))
} else if sr.ne() {
rdr(r).read_volatile();
Err(nb::Error::Other(Error::Noise))
} else if sr.ore() {
rdr(r).read_volatile();
Err(nb::Error::Other(Error::Overrun))
} else if sr.rxne() {
if self.check_rx_flags()? {
Ok(rdr(r).read_volatile())
} else {
Err(nb::Error::WouldBlock)
@ -372,24 +416,7 @@ impl<'d, T: BasicInstance, RxDma> UartRx<'d, T, RxDma> {
unsafe {
let r = T::regs();
for b in buffer {
loop {
let sr = sr(r).read();
if sr.pe() {
rdr(r).read_volatile();
return Err(Error::Parity);
} else if sr.fe() {
rdr(r).read_volatile();
return Err(Error::Framing);
} else if sr.ne() {
rdr(r).read_volatile();
return Err(Error::Noise);
} else if sr.ore() {
rdr(r).read_volatile();
return Err(Error::Overrun);
} else if sr.rxne() {
break;
}
}
while !self.check_rx_flags()? {}
*b = rdr(r).read_volatile();
}
}
@ -715,6 +742,8 @@ impl<'d, T: BasicInstance, TxDma, RxDma> Uart<'d, T, TxDma, RxDma> {
_peri: peri,
rx_dma,
detect_previous_overrun: config.detect_previous_overrun,
#[cfg(any(usart_v1, usart_v2))]
buffered_sr: stm32_metapac::usart::regs::Sr(0),
},
}
}

22
tests/stm32/src/bin/usart.rs
View File

@ -8,7 +8,7 @@ use defmt::assert_eq;
use embassy_executor::Spawner;
use embassy_stm32::dma::NoDma;
use embassy_stm32::interrupt;
use embassy_stm32::usart::{Config, Uart};
use embassy_stm32::usart::{Config, Error, Uart};
use embassy_time::{Duration, Instant};
use example_common::*;
@ -53,6 +53,26 @@ async fn main(_spawner: Spawner) {
assert_eq!(buf, data);
}
// Test error handling with with an overflow error
{
let config = Config::default();
let mut usart = Uart::new(&mut usart, &mut rx, &mut tx, &mut irq, NoDma, NoDma, config);
// Send enough bytes to fill the RX FIFOs off all USART versions.
let data = [0xC0, 0xDE, 0x12, 0x23, 0x34];
usart.blocking_write(&data).unwrap();
usart.blocking_flush().unwrap();
// The error should be reported first.
let mut buf = [0; 1];
let err = usart.blocking_read(&mut buf);
assert_eq!(err, Err(Error::Overrun));
// At least the first data byte should still be available on all USART versions.
usart.blocking_read(&mut buf).unwrap();
assert_eq!(buf[0], data[0]);
}
// Test that baudrate divider is calculated correctly.
// Do it by comparing the time it takes to send a known number of bytes.
for baudrate in [