From 85ec9dd16fb5649900722228e46e9c68370901a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timo=20Kr=C3=B6ger?= <timokroeger93@gmail.com>
Date: Sun, 3 Jan 2021 12:09:51 +0100
Subject: [PATCH] uarte: Be on safe side with potentially racy code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The PS does not specify how many cycles it takes for a STARTXX task to
generate a XXSTARTED event. I think it is instantaneous but let’s be on
the safe side for the following sequence:
1. poll() starttx
2. drop() txstarted not yet set, but future gets dropped
3. txstarted set by hardware, peripheral enabled after it was dropped
---
 embassy-nrf/src/uarte.rs | 55 +++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs
index 80c31b67..156dbbef 100644
--- a/embassy-nrf/src/uarte.rs
+++ b/embassy-nrf/src/uarte.rs
@@ -289,27 +289,31 @@ where
     fn poll(self: core::pin::Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
         let Self { uarte, buf } = unsafe { self.get_unchecked_mut() };
 
-        match T::state().tx_done.poll_wait(cx) {
-            Poll::Pending if !uarte.tx_started() => {
-                let uarte = &uarte.instance;
-                let ptr = buf.as_ptr();
-                let len = buf.len();
-                assert!(len <= EASY_DMA_SIZE);
-                // TODO: panic if buffer is not in SRAM
+        if T::state().tx_done.poll_wait(cx).is_pending() {
+            let ptr = buf.as_ptr();
+            let len = buf.len();
+            assert!(len <= EASY_DMA_SIZE);
+            // TODO: panic if buffer is not in SRAM
 
-                compiler_fence(Ordering::SeqCst);
-                uarte.txd.ptr.write(|w| unsafe { w.ptr().bits(ptr as u32) });
-                uarte
-                    .txd
-                    .maxcnt
-                    .write(|w| unsafe { w.maxcnt().bits(len as _) });
+            compiler_fence(Ordering::SeqCst);
+            uarte
+                .instance
+                .txd
+                .ptr
+                .write(|w| unsafe { w.ptr().bits(ptr as u32) });
+            uarte
+                .instance
+                .txd
+                .maxcnt
+                .write(|w| unsafe { w.maxcnt().bits(len as _) });
 
-                trace!("starttx");
-                uarte.tasks_starttx.write(|w| unsafe { w.bits(1) });
-                Poll::Pending
-            }
-            Poll::Pending => Poll::Pending,
-            Poll::Ready(_) => Poll::Ready(Ok(())),
+            trace!("starttx");
+            uarte.instance.tasks_starttx.write(|w| unsafe { w.bits(1) });
+            while !uarte.tx_started() {} // Make sure transmission has started
+
+            Poll::Pending
+        } else {
+            Poll::Ready(Ok(()))
         }
     }
 }
@@ -352,21 +356,26 @@ where
 
         match T::state().rx_done.poll_wait(cx) {
             Poll::Pending if !uarte.rx_started() => {
-                let uarte = &uarte.instance;
-
                 let ptr = buf.as_ptr();
                 let len = buf.len();
                 assert!(len <= EASY_DMA_SIZE);
 
                 compiler_fence(Ordering::SeqCst);
-                uarte.rxd.ptr.write(|w| unsafe { w.ptr().bits(ptr as u32) });
                 uarte
+                    .instance
+                    .rxd
+                    .ptr
+                    .write(|w| unsafe { w.ptr().bits(ptr as u32) });
+                uarte
+                    .instance
                     .rxd
                     .maxcnt
                     .write(|w| unsafe { w.maxcnt().bits(len as _) });
 
                 trace!("startrx");
-                uarte.tasks_startrx.write(|w| unsafe { w.bits(1) });
+                uarte.instance.tasks_startrx.write(|w| unsafe { w.bits(1) });
+                while !uarte.rx_started() {} // Make sure reception has started
+
                 Poll::Pending
             }
             Poll::Pending => Poll::Pending,