STM32: USART: Change eager_reads from bool to Option<usize>

Author: adamgreig

embassy-hal-internal/src/atomic_ring_buffer.rs

@@ -133,6 +133,18 @@ impl RingBuffer {
         self.len.load(Ordering::Relaxed)
     }
 
+    /// Return number of items available to read.
+    pub fn available(&self) -> usize {
+        let end = self.end.load(Ordering::Relaxed);
+        let len = self.len.load(Ordering::Relaxed);
+        let start = self.start.load(Ordering::Relaxed);
+        if end >= start {
+            end - start
+        } else {
+            2 * len - start + end
+        }
+    }
+
     /// Check if buffer is full.
     pub fn is_full(&self) -> bool {
         let len = self.len.load(Ordering::Relaxed);
@@ -144,15 +156,7 @@ impl RingBuffer {
 
     /// Check if buffer is at least half full.
     pub fn is_half_full(&self) -> bool {
-        let len = self.len.load(Ordering::Relaxed);
-        let start = self.start.load(Ordering::Relaxed);
-        let end = self.end.load(Ordering::Relaxed);
-        let n = if end >= start {
-            end - start
-        } else {
-            2 * len - start + end
-        };
-        n >= len / 2
+        self.available() >= self.len.load(Ordering::Relaxed) / 2
     }
 
     /// Check if buffer is empty.

embassy-stm32/src/usart/buffered.rs

@@ -1,7 +1,7 @@
 use core::future::poll_fn;
 use core::marker::PhantomData;
 use core::slice;
-use core::sync::atomic::{AtomicBool, AtomicU8, Ordering};
+use core::sync::atomic::{AtomicBool, AtomicU8, AtomicUsize, Ordering};
 use core::task::Poll;
 
 use embassy_embedded_hal::SetConfig;
@@ -68,8 +68,9 @@ unsafe fn on_interrupt(r: Regs, state: &'static State) {
             // FIXME: Should we disable any further RX interrupts when the buffer becomes full.
         }
 
-        if state.eager_reads.load(Ordering::Relaxed) {
-            if !state.rx_buf.is_empty() {
+        let eager = state.eager_reads.load(Ordering::Relaxed);
+        if eager > 0 {
+            if state.rx_buf.available() >= eager {
                 state.rx_waker.wake();
             }
         } else {
@@ -138,7 +139,7 @@ pub(super) struct State {
     tx_done: AtomicBool,
     tx_rx_refcount: AtomicU8,
     half_duplex_readback: AtomicBool,
-    eager_reads: AtomicBool,
+    eager_reads: AtomicUsize,
 }
 
 impl State {
@@ -151,7 +152,7 @@ impl State {
             tx_done: AtomicBool::new(true),
             tx_rx_refcount: AtomicU8::new(0),
             half_duplex_readback: AtomicBool::new(false),
-            eager_reads: AtomicBool::new(false),
+            eager_reads: AtomicUsize::new(0),
         }
     }
 }
@@ -427,7 +428,9 @@ impl<'d> BufferedUart<'d> {
         let state = T::buffered_state();
         let kernel_clock = T::frequency();
 
-        state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
         state.half_duplex_readback.store(
             config.duplex == Duplex::Half(HalfDuplexReadback::Readback),
             Ordering::Relaxed,
@@ -465,7 +468,9 @@ impl<'d> BufferedUart<'d> {
         let info = self.rx.info;
         let state = self.rx.state;
         state.tx_rx_refcount.store(2, Ordering::Relaxed);
-        state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
 
         info.rcc.enable_and_reset();
 
@@ -537,7 +542,10 @@ impl<'d> BufferedUart<'d> {
     pub fn set_config(&mut self, config: &Config) -> Result<(), ConfigError> {
         reconfigure(self.rx.info, self.rx.kernel_clock, config)?;
 
-        self.rx.state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        self.rx
+            .state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
 
         self.rx.info.regs.cr1().modify(|w| {
             w.set_rxneie(true);
@@ -654,7 +662,9 @@ impl<'d> BufferedUartRx<'d> {
     pub fn set_config(&mut self, config: &Config) -> Result<(), ConfigError> {
         reconfigure(self.info, self.kernel_clock, config)?;
 
-        self.state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        self.state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
 
         self.info.regs.cr1().modify(|w| {
             w.set_rxneie(true);

embassy-stm32/src/usart/mod.rs

@@ -4,7 +4,7 @@
 
 use core::future::poll_fn;
 use core::marker::PhantomData;
-use core::sync::atomic::{compiler_fence, AtomicBool, AtomicU8, Ordering};
+use core::sync::atomic::{compiler_fence, AtomicU8, AtomicUsize, Ordering};
 use core::task::Poll;
 
 use embassy_embedded_hal::SetConfig;
@@ -212,17 +212,20 @@ pub struct Config {
     /// If false: the error is ignored and cleared
     pub detect_previous_overrun: bool,
 
-    /// If true then read-like calls on `BufferedUartRx` and `RingBufferedUartRx`
-    /// are woken/return as soon as any data is available in the buffer.
+    /// If `None` (the default) then read-like calls on `BufferedUartRx` and `RingBufferedUartRx`
+    /// typically only wake/return after line idle or after the buffer is at least half full
+    /// (for `BufferedUartRx`) or the DMA buffer is written at the half or full positions
+    /// (for `RingBufferedUartRx`), though it may also wake/return earlier in some circumstances.
     ///
-    /// If false (the default) then reads started typically only wake/return after
-    /// line idle or after the buffer is at least half full (`BufferedUartRx`) or
-    /// the DMA buffer is written at the half or full positions (`RingBufferedUartRx`),
-    /// though it may also wake/return earlier in some circumstances.
+    /// If `Some(n)` then such reads are also woken/return as soon as at least `n` words are
+    /// available in the buffer, in addition to waking/returning when the conditions described
+    /// above are met. `Some(0)` is treated as `None`. Setting this for `RingBufferedUartRx`
+    /// will trigger an interrupt for every received word to check the buffer level, which may
+    /// impact performance at high data rates.
     ///
     /// Has no effect on plain `Uart` or `UartRx` reads, which are specified to either
     /// return a single word, a full buffer, or after line idle.
-    pub eager_reads: bool,
+    pub eager_reads: Option<usize>,
 
     /// Set this to true if the line is considered noise free.
     /// This will increase the receiver’s tolerance to clock deviations,
@@ -296,7 +299,7 @@ impl Default for Config {
             parity: Parity::ParityNone,
             // historical behavior
             detect_previous_overrun: false,
-            eager_reads: false,
+            eager_reads: None,
             #[cfg(not(usart_v1))]
             assume_noise_free: false,
             #[cfg(any(usart_v3, usart_v4))]
@@ -997,7 +1000,9 @@ impl<'d, M: Mode> UartRx<'d, M> {
         let info = self.info;
         let state = self.state;
         state.tx_rx_refcount.store(1, Ordering::Relaxed);
-        state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
 
         info.rcc.enable_and_reset();
 
@@ -1014,7 +1019,9 @@ impl<'d, M: Mode> UartRx<'d, M> {
 
     /// Reconfigure the driver
     pub fn set_config(&mut self, config: &Config) -> Result<(), ConfigError> {
-        self.state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        self.state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
         reconfigure(self.info, self.kernel_clock, config)
     }
 
@@ -1495,7 +1502,9 @@ impl<'d, M: Mode> Uart<'d, M> {
         let info = self.rx.info;
         let state = self.rx.state;
         state.tx_rx_refcount.store(2, Ordering::Relaxed);
-        state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
 
         info.rcc.enable_and_reset();
 
@@ -2080,7 +2089,7 @@ struct State {
     rx_waker: AtomicWaker,
     tx_waker: AtomicWaker,
     tx_rx_refcount: AtomicU8,
-    eager_reads: AtomicBool,
+    eager_reads: AtomicUsize,
 }
 
 impl State {
@@ -2089,7 +2098,7 @@ impl State {
             rx_waker: AtomicWaker::new(),
             tx_waker: AtomicWaker::new(),
             tx_rx_refcount: AtomicU8::new(0),
-            eager_reads: AtomicBool::new(false),
+            eager_reads: AtomicUsize::new(0),
         }
     }
 }

embassy-stm32/src/usart/ringbuffered.rs

@@ -132,7 +132,9 @@ impl<'d> UartRx<'d, Async> {
 impl<'d> RingBufferedUartRx<'d> {
     /// Reconfigure the driver
     pub fn set_config(&mut self, config: &Config) -> Result<(), ConfigError> {
-        self.state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
+        self.state
+            .eager_reads
+            .store(config.eager_reads.unwrap_or(0), Ordering::Relaxed);
         reconfigure(self.info, self.kernel_clock, config)
     }
 
@@ -149,7 +151,7 @@ impl<'d> RingBufferedUartRx<'d> {
         // clear all interrupts and DMA Rx Request
         r.cr1().modify(|w| {
             // use RXNE only when returning reads early
-            w.set_rxneie(self.state.eager_reads.load(Ordering::Relaxed));
+            w.set_rxneie(self.state.eager_reads.load(Ordering::Relaxed) > 0);
             // enable parity interrupt if not ParityNone
             w.set_peie(w.pce());
             // enable idle line interrupt
@@ -261,11 +263,12 @@ impl<'d> RingBufferedUartRx<'d> {
                 // However, DMA will clear RXNE, so we can't check directly, and because
                 // the other future borrows `ring_buf`, we can't check `len()` here either.
                 // Instead, return from this future and we'll check the length afterwards.
-                let eager = s.eager_reads.load(Ordering::Relaxed);
+                let eager = s.eager_reads.load(Ordering::Relaxed) > 0;
 
-                if check_idle_and_errors(self.info.regs)? || (eager && uart_init) {
+                let idle = check_idle_and_errors(self.info.regs)?;
+                if idle || (eager && uart_init) {
                     // Idle line is detected, or eager reads is set and some data is available.
-                    Poll::Ready(Ok(()))
+                    Poll::Ready(Ok(idle))
                 } else {
                     uart_init = true;
                     Poll::Pending
@@ -288,13 +291,24 @@ impl<'d> RingBufferedUartRx<'d> {
             });
 
             match select(uart, dma).await {
-                Either::Left((result, _)) => {
-                    if self.ring_buf.len().unwrap_or(0) > 0 || result.is_err() {
-                        return result;
+                // UART woke with line idle
+                Either::Left((Ok(true), _)) => {
+                    return Ok(());
+                }
+                // UART woke without idle or error: word received
+                Either::Left((Ok(false), _)) => {
+                    let eager = self.state.eager_reads.load(Ordering::Relaxed);
+                    if eager > 0 && self.ring_buf.len().unwrap_or(0) >= eager {
+                        return Ok(());
                     } else {
                         continue;
                     }
                 }
+                // UART woke with error
+                Either::Left((Err(e), _)) => {
+                    return Err(e);
+                }
+                // DMA woke
                 Either::Right(((), _)) => return Ok(()),
             }
         }