Check and clear idle and error flags together

embassy-stm32/src/usart/ringbuffered.rs

@@ -99,8 +99,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 +132,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
+    /// check for errors in status register. Error flags are checked/cleared first.
-    /// to be read first without returning yet.
     fn start_dma_or_check_errors(&mut self) -> Result<(), Error> {
         let r = self.info.regs;
 
-        let sr = clear_idle_flag(r);
+        check_idle_and_errors(r)?;
-        let res = check_for_errors(sr);
         if !r.cr3().read().dmar() {
             self.start_uart();
         }
-        res
+        Ok(())
     }
 
     /// Read bytes that are readily available in the ring buffer.
@@ -191,13 +187,7 @@ impl<'d> RingBufferedUartRx<'d> {
 
             compiler_fence(Ordering::SeqCst);
 
-            // Critical section is needed so that IDLE isn't set after
+            if check_idle_and_errors(self.info.regs)? {
-            // 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() {
                 // Idle line is detected
                 Poll::Ready(Ok(()))
             } else {
@@ -240,38 +230,47 @@ impl Drop for RingBufferedUartRx<'_> {
     }
 }
 
-/// Return an error result if the Sr register has errors
+/// Check and clear idle and error interrupts, return true if idle, Err(e) on error
-fn check_for_errors(s: Sr) -> Result<(), Error> {
+///
-    if s.pe() {
+/// All flags are read and cleared in a single step, respectively. When more than one flag is set
-        Err(Error::Parity)
+/// at the same time, all flags will be cleared but only one flag will be reported. So the other
-    } else if s.fe() {
+/// flag(s) will gone missing unnoticed. The error flags are checked first, the idle flag last.
-        Err(Error::Framing)
+///
-    } else if s.ne() {
+/// For usart_v1 and usart_v2, all status flags must be handled together anyway because all flags
-        Err(Error::Noise)
+/// are cleared by a single read to the RDR register.
-    } else if s.ore() {
+fn check_idle_and_errors(r: Regs) -> Result<bool, Error> {
-        Err(Error::Overrun)
+    // Critical section is required so that the flags aren't set after read and before clear
-    } else {
+    let sr = critical_section::with(|_| {
-        Ok(())
-    }
-}
-
-/// 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();
 
         #[cfg(any(usart_v3, usart_v4))]
-    r.icr().write(|w| w.set_idle(true));
+        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.
+            // This read also clears the error and idle interrupt flags on v1 (TODO and v2?)
             rdr(r).read_volatile()
         };
-
-    r.cr1().modify(|w| w.set_idleie(true));
-
         sr
+    });
+    if sr.pe() {
+        Err(Error::Parity)
+    } else if sr.fe() {
+        Err(Error::Framing)
+    } else if sr.ne() {
+        Err(Error::Noise)
+    } else if sr.ore() {
+        Err(Error::Overrun)
+    } else {
+        r.cr1().modify(|w| w.set_idleie(true));
+        Ok(sr.idle())
+    }
 }
 
 impl embedded_io_async::ErrorType for RingBufferedUartRx<'_> {