STM32: USART: Add `eager_reads` config option

3 files changed, 95 insertions, 47 deletions

diff --git a/embassy-stm32/src/usart/buffered.rs b/embassy-stm32/src/usart/buffered.rs
index c734eed49..83aa4439b 100644
--- a/embassy-stm32/src/usart/buffered.rs
+++ b/embassy-stm32/src/usart/buffered.rs
@@ -68,8 +68,14 @@ unsafe fn on_interrupt(r: Regs, state: &'static State) {
             // FIXME: Should we disable any further RX interrupts when the buffer becomes full.
         }
 
-        if !state.rx_buf.is_empty() {
-            state.rx_waker.wake();
+        if state.eager_reads.load(Ordering::Relaxed) {
+            if !state.rx_buf.is_empty() {
+                state.rx_waker.wake();
+            }
+        } else {
+            if state.rx_buf.is_half_full() {
+                state.rx_waker.wake();
+            }
         }
     }
 
@@ -132,6 +138,7 @@ pub(super) struct State {
     tx_done: AtomicBool,
     tx_rx_refcount: AtomicU8,
     half_duplex_readback: AtomicBool,
+    eager_reads: AtomicBool,
 }
 
 impl State {
@@ -144,6 +151,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),
         }
     }
 }
@@ -419,6 +427,7 @@ impl<'d> BufferedUart<'d> {
         let state = T::buffered_state();
         let kernel_clock = T::frequency();
 
+        state.eager_reads.store(config.eager_reads, Ordering::Relaxed);
         state.half_duplex_readback.store(
             config.duplex == Duplex::Half(HalfDuplexReadback::Readback),
             Ordering::Relaxed,
@@ -456,6 +465,7 @@ 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);
 
         info.rcc.enable_and_reset();
 
@@ -527,6 +537,8 @@ 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.info.regs.cr1().modify(|w| {
             w.set_rxneie(true);
             w.set_idleie(true);
@@ -633,6 +645,8 @@ 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.info.regs.cr1().modify(|w| {
             w.set_rxneie(true);
             w.set_idleie(true);
diff --git a/embassy-stm32/src/usart/mod.rs b/embassy-stm32/src/usart/mod.rs
index ff211e0c9..e439f2cee 100644
--- a/embassy-stm32/src/usart/mod.rs
+++ b/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, AtomicU8, Ordering};
+use core::sync::atomic::{compiler_fence, AtomicBool, AtomicU8, Ordering};
 use core::task::Poll;
 
 use embassy_embedded_hal::SetConfig;
@@ -206,6 +206,18 @@ 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 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.
+    ///
+    /// 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,
+
     /// Set this to true if the line is considered noise free.
     /// This will increase the receiver’s tolerance to clock deviations,
     /// but will effectively disable noise detection.
@@ -270,6 +282,7 @@ impl Default for Config {
             parity: Parity::ParityNone,
             // historical behavior
             detect_previous_overrun: false,
+            eager_reads: false,
             #[cfg(not(usart_v1))]
             assume_noise_free: false,
             #[cfg(any(usart_v3, usart_v4))]
@@ -966,6 +979,7 @@ 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);
 
         info.rcc.enable_and_reset();
 
@@ -982,6 +996,7 @@ 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);
         reconfigure(self.info, self.kernel_clock, config)
     }
 
@@ -1462,6 +1477,7 @@ 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);
 
         info.rcc.enable_and_reset();
 
@@ -2022,6 +2038,7 @@ struct State {
     rx_waker: AtomicWaker,
     tx_waker: AtomicWaker,
     tx_rx_refcount: AtomicU8,
+    eager_reads: AtomicBool,
 }
 
 impl State {
@@ -2030,6 +2047,7 @@ impl State {
             rx_waker: AtomicWaker::new(),
             tx_waker: AtomicWaker::new(),
             tx_rx_refcount: AtomicU8::new(0),
+            eager_reads: AtomicBool::new(false),
         }
     }
 }
diff --git a/embassy-stm32/src/usart/ringbuffered.rs b/embassy-stm32/src/usart/ringbuffered.rs
index 5f4e87834..d818e0bcc 100644
--- a/embassy-stm32/src/usart/ringbuffered.rs
+++ b/embassy-stm32/src/usart/ringbuffered.rs
@@ -26,9 +26,9 @@ use crate::Peri;
 /// contain enough bytes to fill the buffer passed by the caller of
 /// the function, or is empty.
 ///
-/// Waiting for bytes operates in one of two modes, depending on
-/// the behavior of the sender and the size of the buffer passed
-/// to the function:
+/// Waiting for bytes operates in one of three modes, depending on
+/// the behavior of the sender, the size of the buffer passed
+/// to the function, and the configuration:
 ///
 /// - If the sender sends intermittently, the 'idle line'
 /// condition will be detected when the sender stops, and any
@@ -47,7 +47,11 @@ use crate::Peri;
 /// interrupt when those specific buffer addresses have been
 /// written.
 ///
-/// In both cases this will result in variable latency due to the
+/// - If `eager_reads` is enabled in `config`, the UART interrupt
+/// is enabled on all data reception and the call will only wait
+/// for at least one byte to be available before returning.
+///
+/// In the first two cases this will result in variable latency due to the
 /// buffering effect. For example, if the baudrate is 2400 bps, and
 /// the configuration is 8 data bits, no parity bit, and one stop bit,
 /// then a byte will be received every ~4.16ms. If the ring buffer is
@@ -68,15 +72,10 @@ use crate::Peri;
 /// sending, but would be falsely triggered in the worst-case
 /// buffer delay scenario.
 ///
-/// Note: This latency is caused by the limited capabilities of the
-/// STM32 DMA controller; since it cannot generate an interrupt when
-/// it stores a byte into an empty ring buffer, or in any other
-/// configurable conditions, it is not possible to take notice of the
-/// contents of the ring buffer more quickly without introducing
-/// polling. As a result the latency can be reduced by calling the
-/// read functions repeatedly with smaller buffers to receive the
-/// available bytes, as each call to a read function will explicitly
-/// check the ring buffer for available bytes.
+/// Note: Enabling `eager_reads` with `RingBufferedUartRx` will enable
+/// an UART RXNE interrupt, which will cause an interrupt to occur on
+/// every received data byte. The data is still copied using DMA, but
+/// there is nevertheless additional processing overhead for each byte.
 pub struct RingBufferedUartRx<'d> {
     info: &'static Info,
     state: &'static State,
@@ -133,6 +132,7 @@ 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);
         reconfigure(self.info, self.kernel_clock, config)
     }
 
@@ -148,8 +148,8 @@ impl<'d> RingBufferedUartRx<'d> {
         let r = self.info.regs;
         // clear all interrupts and DMA Rx Request
         r.cr1().modify(|w| {
-            // disable RXNE interrupt
-            w.set_rxneie(false);
+            // use RXNE only when returning reads early
+            w.set_rxneie(self.state.eager_reads.load(Ordering::Relaxed));
             // enable parity interrupt if not ParityNone
             w.set_peie(w.pce());
             // enable idle line interrupt
@@ -248,39 +248,55 @@ impl<'d> RingBufferedUartRx<'d> {
     async fn wait_for_data_or_idle(&mut self) -> Result<(), Error> {
         compiler_fence(Ordering::SeqCst);
 
-        // Future which completes when idle line is detected
-        let s = self.state;
-        let uart = poll_fn(|cx| {
-            s.rx_waker.register(cx.waker());
-
-            compiler_fence(Ordering::SeqCst);
-
-            if check_idle_and_errors(self.info.regs)? {
-                // Idle line is detected
-                Poll::Ready(Ok(()))
-            } else {
-                Poll::Pending
-            }
-        });
+        loop {
+            // Future which completes when idle line is detected
+            let s = self.state;
+            let mut uart_init = false;
+            let uart = poll_fn(|cx| {
+                s.rx_waker.register(cx.waker());
+
+                compiler_fence(Ordering::SeqCst);
+
+                // We may have been woken by IDLE or, if eager_reads is set, by RXNE.
+                // 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);
+
+                if check_idle_and_errors(self.info.regs)? || (eager && uart_init) {
+                    // Idle line is detected, or eager reads is set and some data is available.
+                    Poll::Ready(Ok(()))
+                } else {
+                    uart_init = true;
+                    Poll::Pending
+                }
+            });
 
-        let mut dma_init = false;
-        // Future which completes when the DMA controller indicates it
-        // has written to the ring buffer's middle byte, or last byte
-        let dma = poll_fn(|cx| {
-            self.ring_buf.set_waker(cx.waker());
+            let mut dma_init = false;
+            // Future which completes when the DMA controller indicates it
+            // has written to the ring buffer's middle byte, or last byte
+            let dma = poll_fn(|cx| {
+                self.ring_buf.set_waker(cx.waker());
 
-            let status = match dma_init {
-                false => Poll::Pending,
-                true => Poll::Ready(()),
-            };
+                let status = match dma_init {
+                    false => Poll::Pending,
+                    true => Poll::Ready(()),
+                };
 
-            dma_init = true;
-            status
-        });
+                dma_init = true;
+                status
+            });
 
-        match select(uart, dma).await {
-            Either::Left((result, _)) => result,
-            Either::Right(((), _)) => Ok(()),
+            match select(uart, dma).await {
+                Either::Left((result, _)) => {
+                    if self.ring_buf.len().unwrap_or(0) > 0 || result.is_err() {
+                        return result;
+                    } else {
+                        continue;
+                    }
+                }
+                Either::Right(((), _)) => return Ok(()),
+            }
         }
     }