Fix many quirks in USB implementation

Author: elfmimi

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## [Unreleased]
+- Fix issues with USB that were affecting reliable operation.
 
 ## [v0.4.6] 2025-03-31
 

hal/Cargo.toml

@@ -33,7 +33,7 @@ embedded-hal = { version = "0.2.7" }
 nb = "1"
 fugit = "0.3.6"
 paste = "1"
-usb-device = "0.3.1"
+usb-device = "0.3.2"
 bit-iter = "1"
 rtic-monotonic = "1"
 void = { version = "1", default-features = false }

hal/src/usb.rs

@@ -141,7 +141,6 @@ impl Endpoints {
 
 struct Inner {
     endpoints: Endpoints,
-    set_address: bool,
 }
 
 /// [`usb_device`] implementation.
@@ -157,7 +156,6 @@ impl Usb {
 
         let inner = Inner {
             endpoints: Endpoints::new(),
-            set_address: false,
         };
 
         Self {
@@ -277,6 +275,12 @@ impl Inner {
                 .deveptidr_ctrl_mode(ep)
                 .write(|w| w.stallrqc().set_bit());
 
+            if ep == 0 {
+                self.reg()
+                    .devepticr_ctrl_mode(ep)
+                    .write(|w| w.rxoutic().set_bit());
+            }
+
             if ep != 0 {
                 // Configure endpoint direction.
                 self.reg()
@@ -292,9 +296,15 @@ impl Inner {
                 .cfgok()
                 .bit_is_set()
             {
-                self.reg()
-                    .deveptier_ctrl_mode(ep)
-                    .write(|w| w.rxstpes().set_bit());
+                if ep == 0 {
+                    self.reg()
+                        .deveptier_ctrl_mode(ep)
+                        .write(|w| w.rxstpes().set_bit());
+                } else if conf.ep_dir == UsbDirection::Out {
+                    self.reg()
+                        .deveptier_blk_mode(ep)
+                        .write(|w| w.rxoutes().set_bit());
+                }
                 self.enable_endpoint_interrupt(ep);
             } else {
                 todo!("endpoint configuration failed");
@@ -326,11 +336,6 @@ impl Inner {
         // enable interrupts
         self.reg().devier().write(|w| {
             w.eorstes().set_bit();
-            w.suspes().set_bit();
-            w.wakeupes().set_bit();
-
-            // should we use this?
-            w.sofes().set_bit();
 
             w
         });
@@ -411,16 +416,14 @@ impl Inner {
     }
 
     fn set_device_address(&mut self, addr: u8) {
-        // Set the address in hardware, but do not enable it. This is
-        // done in poll() on an TXINI.
+        // Set the address in hardware and enable it.
         self.reg().devctrl().modify(|_, w| {
             unsafe {
                 w.uadd().bits(addr);
             }
-            w.adden().clear_bit();
+            w.adden().set_bit();
             w
         });
-        self.set_address = true;
     }
 
     fn poll(&mut self) -> PollResult {
@@ -441,27 +444,37 @@ impl Inner {
         const DEVISR_PEPS_MASK: u32 = 0x3ff000;
         const DEVISR_PEPS_OFFSET: u8 = 12;
         for ep in BitIter::from((dev_isr.bits() & DEVISR_PEPS_MASK) >> DEVISR_PEPS_OFFSET) {
-            let sr = self.reg().deveptisr_ctrl_mode(ep).read();
-
-            // SETUP packet?
-            if sr.rxstpi().bit_is_set() {
-                ep_setup |= 1 << ep;
-            };
-
-            // OUT packet?
-            if sr.rxouti().bit_is_set() {
-                ep_out |= 1 << ep;
-            };
+            let isr = self.reg().deveptisr_ctrl_mode(ep).read();
 
-            // IN packet?
-            if sr.txini().bit_is_set() {
-                if self.set_address {
-                    // commit the new address
-                    self.reg().devctrl().modify(|_, w| w.adden().set_bit());
-                    self.set_address = false;
+            if ep == 0 {
+                // OUT packet?
+                let imr = self.reg().deveptimr_ctrl_mode(ep).read();
+                if isr.rxouti().bit_is_set() && imr.rxoute().bit_is_set() {
+                    ep_out |= 1;
+                }
+                // SETUP packet?
+                if ep_out == 0 {
+                    if isr.rxstpi().bit_is_set() {
+                        ep_setup |= 1;
+                    };
+                }
+                // IN packet?
+                if isr.txini().bit_is_set() && imr.txine().bit_is_set() {
+                    ep_in_complete |= 1;
+                    // disable TXINI interrupt
+                    self.reg()
+                        .deveptidr_ctrl_mode(ep)
+                        .write(|w| w.txinec().set_bit());
+                }
+            } else {
+                // OUT packet?
+                if isr.rxouti().bit_is_set() {
+                    ep_out |= 1 << ep;
+                };
+                // IN packet?
+                if isr.txini().bit_is_set() {
+                    ep_in_complete |= 1 << ep;
                 }
-
-                ep_in_complete |= 1 << ep;
             };
         }
 
@@ -514,25 +527,27 @@ impl Inner {
 
     fn read(&self, ep_addr: EndpointAddress, buf: &mut [u8]) -> UsbResult<usize> {
         let ep = ep_addr.index();
-        let len = core::cmp::min(
-            self.reg().deveptisr_ctrl_mode(ep).read().byct().bits() as usize,
-            buf.len(),
-        );
+        let isr = self.reg().deveptisr_ctrl_mode(ep).read();
+        let len = core::cmp::min(isr.byct().bits() as _, buf.len());
 
         self.read_fifo(ep, &mut buf[0..len]);
 
         if ep == 0 {
             // control endpoints
-
-            // Clear RXSTPI interrupt, and make FIFO available
-            self.reg()
-                .devepticr_ctrl_mode(0)
-                .write(|w| w.rxstpic().set_bit());
-
-            // Clear RXOUTI
-            self.reg()
-                .devepticr_ctrl_mode(0)
-                .write(|w| w.rxoutic().set_bit());
+            if isr.rxouti().bit_is_set() {
+                // Clear RXOUTI
+                self.reg()
+                    .devepticr_ctrl_mode(0)
+                    .write(|w| w.rxoutic().set_bit());
+            } else if isr.rxstpi().bit_is_set() {
+                // Clear RXSTPI interrupt, and make FIFO available
+                self.reg()
+                    .devepticr_ctrl_mode(0)
+                    .write(|w| w.rxstpic().set_bit());
+            } else {
+                // Should never come here.
+                return Err(UsbError::WouldBlock);
+            }
         } else {
             // Other Endpoints
 
@@ -550,21 +565,32 @@ impl Inner {
         Ok(len)
     }
 
-    fn is_stalled(&self, _ep: EndpointAddress) -> bool {
-        // stub: seemingly not required
-        false
+    fn is_stalled(&self, ep_addr: EndpointAddress) -> bool {
+        let ep = ep_addr.index();
+        if ep == 0 {
+            self.reg().deveptimr_ctrl_mode(ep).read().stallrq().bit()
+        } else {
+            false
+        }
     }
 
-    fn set_stalled(&self, _ep: EndpointAddress, _stalled: bool) {
-        // stub: seemingly not required
+    fn set_stalled(&self, ep_addr: EndpointAddress, stalled: bool) {
+        let ep = ep_addr.index();
+        if ep == 0 {
+            if stalled {
+                self.reg()
+                    .deveptier_ctrl_mode(ep)
+                    .write(|w| w.stallrqs().set_bit());
+            } else {
+                self.reg()
+                    .deveptidr_ctrl_mode(ep)
+                    .write(|w| w.stallrqc().set_bit());
+            }
+        }
     }
 }
 
 impl usb_device::bus::UsbBus for Usb {
-    /// Ensure the address is set before write of zero sized packet to
-    /// confirm a SET_ADDRESS transaction.
-    const QUIRK_SET_ADDRESS_BEFORE_STATUS: bool = true;
-
     fn enable(&mut self) {
         interrupt::free(|cs| unsafe { &mut *self.inner.borrow(cs).get() }.enable());
     }