rp2040: avoid device-mode state machine hang

Don't mark IN buffers as available during the last 200us of a full-speed
frame. This avoids a situation seen with the USB2.0 hub on a Raspberry
Pi 4 where a late IN token before the next full-speed SOF can cause port
babble and a corrupt ACK packet. The nature of the data corruption has a
chance to cause device lockup.

Use the next SOF to mark delayed buffers as available. This reduces
available Bulk IN bandwidth by approximately 20%, and requires that the
SOF interrupt is enabled while these transfers are ongoing.

Inherit the top-level enable from the corresponding Pico-SDK flag.
Applications that will not use the device in a situation where it could
be plugged into a Pi 4 or Pi 400 (for example, when directly connected
to a commodity hub or other host) can turn off the flag in the SDK.

v2: use a field in hw_endpoint to mark pending.

v3: Partial rewrite following review comments

- Stub functions out if the workaround is not required
- Only force-enable SOF while any vulnerable endpoints are active
- Respect dcd_sof_enable() functionality
- Get rid of all but necessary ifdef hackery
- Fix a bug where the "endpoint lock" was used with an uninitialised pointer.

hw/bsp/rp2040/family.cmake

@@ -116,6 +116,7 @@ if (NOT TARGET _rp2040_family_inclusion_marker)
 
 	target_compile_definitions(tinyusb_additions INTERFACE
 		PICO_RP2040_USB_DEVICE_ENUMERATION_FIX=1
+		PICO_RP2040_USB_DEVICE_UFRAME_FIX=1
 	)
 
 	if(DEFINED LOG)

src/portable/raspberrypi/rp2040/dcd_rp2040.c

@@ -246,13 +246,32 @@ static void __tusb_irq_path_func(dcd_rp2040_irq)(void)
 {
     uint32_t const status = usb_hw->ints;
     uint32_t handled = 0;
+    bool keep_sof_alive = false;
 
     if (status & USB_INTF_DEV_SOF_BITS)
     {
       handled |= USB_INTF_DEV_SOF_BITS;
 
+#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+      last_sof = time_us_32();
+
+      for (uint8_t i = 0; i < USB_MAX_ENDPOINTS; i++) {
+        struct hw_endpoint *ep = hw_endpoint_get_by_num(i, TUSB_DIR_IN);
+        hw_endpoint_lock_update(ep, 1);
+        // Bulk IN endpoint in a transfer?
+        if (rp2040_ep_needs_sof(ep) && ep->active)
+          keep_sof_alive = true;
+        // Deferred enable?
+        if (ep->pending) {
+          hw_endpoint_start_next_buffer(ep);
+          ep->pending = 0;
+        }
+        hw_endpoint_lock_update(ep, -1);
+      }
+#endif
       // disable SOF interrupt if it is used for RESUME in remote wakeup
-      if (!_sof_enable) usb_hw_clear->inte = USB_INTS_DEV_SOF_BITS;
+      if (!keep_sof_alive && !_sof_enable)
+        usb_hw_clear->inte = USB_INTS_DEV_SOF_BITS;
 
       dcd_event_sof(0, usb_hw->sof_rd & USB_SOF_RD_BITS, true);
     }
@@ -449,7 +468,11 @@ void dcd_sof_enable(uint8_t rhport, bool en)
     usb_hw_set->inte = USB_INTS_DEV_SOF_BITS;
   }else
   {
+    // Don't clear immediately if the SOF workaround is in use.
+    // The SOF handler will conditionally disable the interrupt.
+#if !TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
     usb_hw_clear->inte = USB_INTS_DEV_SOF_BITS;
+#endif
   }
 }
 

src/portable/raspberrypi/rp2040/rp2040_usb.c

@@ -38,10 +38,6 @@ const char *ep_dir_string[] = {
         "in",
 };
 
-#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
-volatile uint32_t last_sof = 0;
-#endif
-
 static void _hw_endpoint_xfer_sync(struct hw_endpoint *ep);
 
 // if usb hardware is in host mode
@@ -54,6 +50,41 @@ TU_ATTR_ALWAYS_INLINE static inline bool is_host_mode(void)
 //
 //--------------------------------------------------------------------+
 
+#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+volatile uint32_t last_sof = 0;
+
+bool rp2040_critical_frame_period(struct hw_endpoint *ep)
+{
+  uint32_t delta;
+
+  if (usb_hw->main_ctrl & USB_MAIN_CTRL_HOST_NDEVICE_BITS)
+    return false;
+
+  if (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_OUT ||
+      ep->transfer_type == TUSB_XFER_INTERRUPT ||
+      ep->transfer_type == TUSB_XFER_ISOCHRONOUS)
+    return false;
+
+  /* Avoid the last 200us (uframe 6.5-7) of a frame, up to the EOF2 point.
+   * The device state machine cannot recover from receiving an incorrect PID
+   * when it is expecting an ACK.
+   */
+  delta = time_us_32() - last_sof;
+  if (delta < 800 || delta > 998) {
+    return false;
+  }
+  TU_LOG(3, "Avoiding sof %u now %lu last %lu\n", (usb_hw->sof_rd + 1) & USB_SOF_RD_BITS, now, last_sof);
+  return true;
+}
+
+bool rp2040_ep_needs_sof(struct hw_endpoint *ep) {
+  if (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN &&
+      ep->transfer_type == TUSB_XFER_BULK)
+    return true;
+  return false;
+}
+#endif
+
 void rp2040_usb_init(void)
 {
   // Reset usb controller
@@ -215,7 +246,14 @@ void hw_endpoint_xfer_start(struct hw_endpoint *ep, uint8_t *buffer, uint16_t to
   ep->active        = true;
   ep->user_buf      = buffer;
 
-  hw_endpoint_start_next_buffer(ep);
+  if (rp2040_ep_needs_sof(ep))
+    usb_hw_set->inte = USB_INTS_DEV_SOF_BITS;
+
+  if(!rp2040_critical_frame_period(ep)) {
+    hw_endpoint_start_next_buffer(ep);
+  } else {
+    ep->pending = 1;
+  }
   hw_endpoint_lock_update(ep, -1);
 }
 
@@ -331,7 +369,11 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint *ep)
   }
   else
   {
-    hw_endpoint_start_next_buffer(ep);
+    if(!rp2040_critical_frame_period(ep)) {
+      hw_endpoint_start_next_buffer(ep);
+    } else {
+      ep->pending = 1;
+    }
   }
 
   hw_endpoint_lock_update(ep, -1);

src/portable/raspberrypi/rp2040/rp2040_usb.h

@@ -11,11 +11,21 @@
 #include "hardware/structs/usb.h"
 #include "hardware/irq.h"
 #include "hardware/resets.h"
+#include "hardware/timer.h"
 
 #if defined(PICO_RP2040_USB_DEVICE_ENUMERATION_FIX) && !defined(TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX)
 #define TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX PICO_RP2040_USB_DEVICE_ENUMERATION_FIX
 #endif
 
+#if defined(PICO_RP2040_USB_DEVICE_UFRAME_FIX) && !defined(TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX)
+#define TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX PICO_RP2040_USB_DEVICE_UFRAME_FIX
+#endif
+
+#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+#undef PICO_RP2040_USB_FAST_IRQ
+#define PICO_RP2040_USB_FAST_IRQ 1
+#endif
+
 #ifndef PICO_RP2040_USB_FAST_IRQ
 #define PICO_RP2040_USB_FAST_IRQ 0
 #endif
@@ -67,7 +77,9 @@ typedef struct hw_endpoint
 
     // Interrupt, bulk, etc
     uint8_t transfer_type;
-    
+
+    // Transfer scheduled but not active
+    uint8_t pending;
 #if CFG_TUH_ENABLED
     // Only needed for host
     uint8_t dev_addr;
@@ -77,6 +89,16 @@ typedef struct hw_endpoint
 #endif
 } hw_endpoint_t;
 
+#if !TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+#define rp2040_critical_frame_period(x) false
+#define rp2040_ep_needs_sof(x) false
+#else
+extern volatile uint32_t last_sof;
+
+bool rp2040_critical_frame_period(struct hw_endpoint *ep);
+bool rp2040_ep_needs_sof(struct hw_endpoint *ep);
+#endif
+
 void rp2040_usb_init(void);
 
 void hw_endpoint_xfer_start(struct hw_endpoint *ep, uint8_t *buffer, uint16_t total_len);