Merge #768

768: nrf/usb: fix control out transfers getting corrupted due to ep0rcvout sticking from earlier. r=Dirbaio a=Dirbaio

bors r+

Co-authored-by: Dario Nieuwenhuis <[email protected]>

3 files changed, 19 insertions, 27 deletions

diff --git a/embassy-nrf/src/usb.rs b/embassy-nrf/src/usb.rs
index 1162946a9..8d589aeda 100644
--- a/embassy-nrf/src/usb.rs
+++ b/embassy-nrf/src/usb.rs
@@ -9,7 +9,6 @@ use embassy::interrupt::InterruptExt;
 use embassy::util::Unborrow;
 use embassy::waitqueue::AtomicWaker;
 use embassy_hal_common::unborrow;
-use embassy_usb::control::Request;
 use embassy_usb::driver::{self, EndpointError, Event, Unsupported};
 use embassy_usb::types::{EndpointAddress, EndpointInfo, EndpointType, UsbDirection};
 use futures::future::poll_fn;
@@ -526,10 +525,6 @@ unsafe fn read_dma<T: Instance>(i: usize, buf: &mut [u8]) -> Result<usize, Endpo
         return Err(EndpointError::BufferOverflow);
     }
 
-    if i == 0 {
-        regs.events_ep0datadone.reset();
-    }
-
     let epout = [
         &regs.epout0,
         &regs.epout1,
@@ -640,7 +635,7 @@ pub struct ControlPipe<'d, T: Instance> {
 }
 
 impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> {
-    type SetupFuture<'a> = impl Future<Output = Request> + 'a where Self: 'a;
+    type SetupFuture<'a> = impl Future<Output = [u8;8]> + 'a where Self: 'a;
     type DataOutFuture<'a> = impl Future<Output = Result<usize, EndpointError>> + 'a where Self: 'a;
     type DataInFuture<'a> = impl Future<Output = Result<(), EndpointError>> + 'a where Self: 'a;
 
@@ -652,11 +647,11 @@ impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> {
         async move {
             let regs = T::regs();
 
+            // Reset shorts
+            regs.shorts.write(|w| w);
+
             // Wait for SETUP packet
-            regs.intenset.write(|w| {
-                w.ep0setup().set();
-                w.ep0datadone().set()
-            });
+            regs.intenset.write(|w| w.ep0setup().set());
             poll_fn(|cx| {
                 EP0_WAKER.register(cx.waker());
                 let regs = T::regs();
@@ -668,8 +663,6 @@ impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> {
             })
             .await;
 
-            // Reset shorts
-            regs.shorts.write(|w| w);
             regs.events_ep0setup.reset();
 
             let mut buf = [0; 8];
@@ -682,14 +675,7 @@ impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> {
             buf[6] = regs.wlengthl.read().wlengthl().bits();
             buf[7] = regs.wlengthh.read().wlengthh().bits();
 
-            let req = Request::parse(&buf);
-
-            if req.direction == UsbDirection::Out {
-                regs.tasks_ep0rcvout
-                    .write(|w| w.tasks_ep0rcvout().set_bit());
-            }
-
-            req
+            buf
         }
     }
 
@@ -697,6 +683,12 @@ impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> {
         async move {
             let regs = T::regs();
 
+            regs.events_ep0datadone.reset();
+
+            // This starts a RX on EP0. events_ep0datadone notifies when done.
+            regs.tasks_ep0rcvout
+                .write(|w| w.tasks_ep0rcvout().set_bit());
+
             // Wait until ready
             regs.intenset.write(|w| {
                 w.usbreset().set();
@@ -728,13 +720,13 @@ impl<'d, T: Instance> driver::ControlPipe for ControlPipe<'d, T> {
         async move {
             let regs = T::regs();
             regs.events_ep0datadone.reset();
-            unsafe {
-                write_dma::<T>(0, buf);
-            }
 
             regs.shorts
                 .write(|w| w.ep0datadone_ep0status().bit(last_packet));
 
+            // This starts a TX on EP0. events_ep0datadone notifies when done.
+            unsafe { write_dma::<T>(0, buf) }
+
             regs.intenset.write(|w| {
                 w.usbreset().set();
                 w.ep0setup().set();
diff --git a/embassy-usb/src/driver.rs b/embassy-usb/src/driver.rs
index 57f2b0656..8454b041f 100644
--- a/embassy-usb/src/driver.rs
+++ b/embassy-usb/src/driver.rs
@@ -1,7 +1,5 @@
 use core::future::Future;
 
-use crate::control::Request;
-
 use super::types::*;
 
 /// Driver for a specific USB peripheral. Implement this to add support for a new hardware
@@ -146,7 +144,7 @@ pub trait EndpointOut: Endpoint {
 }
 
 pub trait ControlPipe {
-    type SetupFuture<'a>: Future<Output = Request> + 'a
+    type SetupFuture<'a>: Future<Output = [u8; 8]> + 'a
     where
         Self: 'a;
     type DataOutFuture<'a>: Future<Output = Result<usize, EndpointError>> + 'a
diff --git a/embassy-usb/src/lib.rs b/embassy-usb/src/lib.rs
index b135f5eb3..7b85a2884 100644
--- a/embassy-usb/src/lib.rs
+++ b/embassy-usb/src/lib.rs
@@ -246,7 +246,9 @@ impl<'d, D: Driver<'d>> UsbDevice<'d, D> {
         }
     }
 
-    async fn handle_control(&mut self, req: Request) {
+    async fn handle_control(&mut self, req: [u8; 8]) {
+        let req = Request::parse(&req);
+
         trace!("control request: {:02x}", req);
 
         match req.direction {