- only abort ep0 if it is active

- rename reset_ep0_pid() to reset_ep0()
- minor update log message

2 files changed, 35 insertions, 68 deletions

diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c
index 0e9dd84e5..5c564cb1c 100644
--- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c
+++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c
@@ -60,7 +60,7 @@ TU_ATTR_ALWAYS_INLINE static inline struct hw_endpoint* hw_endpoint_get_by_num(u
   return &hw_endpoints[num][dir];
 }
 
-static struct hw_endpoint* hw_endpoint_get_by_addr(uint8_t ep_addr) {
+TU_ATTR_ALWAYS_INLINE static inline struct hw_endpoint* hw_endpoint_get_by_addr(uint8_t ep_addr) {
   uint8_t num = tu_edpt_number(ep_addr);
   tusb_dir_t dir = tu_edpt_dir(ep_addr);
   return hw_endpoint_get_by_num(num, dir);
@@ -192,47 +192,31 @@ static void __tusb_irq_path_func(hw_handle_buff_status)(void) {
   }
 }
 
-TU_ATTR_ALWAYS_INLINE static inline void reset_ep0_pid(void)
-{
-    // Abort any transactions from a prior control transfer, because
-    // receiving SETUP doesn't reset buffer control state. This works around
-    // a possible USB hardware erratum.
+TU_ATTR_ALWAYS_INLINE static inline void reset_ep0(void) {
+  // If we have finished this transfer on EP0 set pid back to 1 for next
+  // setup transfer. Also clear a stall in case
+  for (uint8_t dir = 0; dir < 2; dir++) {
+    struct hw_endpoint* ep = hw_endpoint_get_by_num(0, dir);
+    if (ep->active) {
+      // Abort any pending transfer from a prior control transfer per USB specs
+      // Due to Errata RP2040-E2: ABORT flag is only applicable for B2 and later (unusable for B0, B1).
+      // Which means we are not guaranteed to safely abort pending transfer on B0 and B1.
+      uint32_t const abort_mask = (dir ? USB_EP_ABORT_EP0_IN_BITS : USB_EP_ABORT_EP0_OUT_BITS);
+      if (rp2040_chip_version() >= 2) {
+        usb_hw_set->abort = abort_mask;
+        while ((usb_hw->abort_done & abort_mask) != abort_mask) {}
+      }
 
-    // With this workaround a race window still exists, but smaller.
-    // ABORT flag is unusable prior to hardware B2 (RP2040-E2), so a larger
-    // race window exists for B1 and earlier.
-    if (rp2040_chip_version() >= 2) {
-        usb_hw_set->abort = 0x3;
-        while ((usb_hw->abort_done & 0x3) != 0x3)
-            ;
-    }
-    // If we have finished this transfer on EP0 set pid back to 1 for next
-    // setup transfer. Also clear a stall in case
-    uint8_t addrs[] = {0x0, 0x80};
-    for (uint i = 0 ; i < TU_ARRAY_SIZE(addrs); i++)
-    {
-        struct hw_endpoint *ep = hw_endpoint_get_by_addr(addrs[i]);
-        ep->next_pid = 1u;
-        // Reset the buffer control now to minimize race conditions
-        _hw_endpoint_buffer_control_set_value32(ep, USB_BUF_CTRL_DATA1_PID | USB_BUF_CTRL_SEL);
-        // Explicit delay, because the one in
-        // _hw_endpoint_buffer_control_set_value32 is only to set AVAILABLE
-        __asm volatile (
-            "b 1f\n"
-            "1: b 1f\n"
-            "1: b 1f\n"
-            "1: b 1f\n"
-            "1: b 1f\n"
-            "1: b 1f\n"
-            "1:\n"
-            : : : "memory");
-        // Make sure local ep state matches peripheral
-        hw_endpoint_reset_transfer(ep);
-    }
-    if (rp2040_chip_version() >= 2) {
-        usb_hw_clear->abort = 0x3;
-        usb_hw_clear->abort_done = 0x3;
+      _hw_endpoint_buffer_control_set_value32(ep, USB_BUF_CTRL_DATA1_PID | USB_BUF_CTRL_SEL);
+      hw_endpoint_reset_transfer(ep);
+
+      if (rp2040_chip_version() >= 2) {
+        usb_hw_clear->abort_done = abort_mask;
+        usb_hw_clear->abort = abort_mask;
+      }
     }
+    ep->next_pid = 1u;
+  }
 }
 
 static void __tusb_irq_path_func(reset_non_control_endpoints)(void) {
@@ -300,7 +284,7 @@ static void __tusb_irq_path_func(dcd_rp2040_irq)(void) {
     uint8_t const* setup = remove_volatile_cast(uint8_t const*, &usb_dpram->setup_packet);
 
     // reset pid to both 1 (data and ack)
-    reset_ep0_pid();
+    reset_ep0();
 
     // Pass setup packet to tiny usb
     dcd_event_setup_received(0, setup, true);
@@ -388,6 +372,8 @@ static void __tusb_irq_path_func(dcd_rp2040_irq)(void) {
 void dcd_init(uint8_t rhport) {
   assert(rhport == 0);
 
+  TU_LOG(2, "Chip Version B%u\r\n", rp2040_chip_version());
+
   // Reset hardware to default state
   rp2040_usb_init();
 
diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c
index b5bace972..1ca711c77 100644
--- a/src/portable/raspberrypi/rp2040/rp2040_usb.c
+++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c
@@ -35,13 +35,6 @@
 //--------------------------------------------------------------------+
 // MACRO CONSTANT TYPEDEF PROTOTYPE
 //--------------------------------------------------------------------+
-
-// Direction strings for debug
-const char* ep_dir_string[] = {
-    "out",
-    "in",
-};
-
 static void _hw_endpoint_xfer_sync(struct hw_endpoint* ep);
 
 #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
@@ -105,22 +98,14 @@ void __tusb_irq_path_func(_hw_endpoint_buffer_control_update32)(struct hw_endpoi
     value |= or_mask;
     if (or_mask & USB_BUF_CTRL_AVAIL) {
       if (*ep->buffer_control & USB_BUF_CTRL_AVAIL) {
-        panic("ep %d %s was already available", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]);
+        panic("ep %02X was already available", ep->ep_addr);
       }
       *ep->buffer_control = value & ~USB_BUF_CTRL_AVAIL;
-      // 12 cycle delay.. (should be good for 48*12Mhz = 576Mhz)
+      // 4.1.2.5.1 Con-current access: 12 cycles (should be good for 48*12Mhz = 576Mhz) after write to buffer control
       // Don't need delay in host mode as host is in charge
-#if !CFG_TUH_ENABLED
-      __asm volatile (
-          "b 1f\n"
-          "1: b 1f\n"
-          "1: b 1f\n"
-          "1: b 1f\n"
-          "1: b 1f\n"
-          "1: b 1f\n"
-          "1:\n"
-          : : : "memory");
-#endif
+      if ( !is_host_mode()) {
+        busy_wait_at_least_cycles(12);
+      }
     }
   }
 
@@ -204,9 +189,7 @@ void hw_endpoint_xfer_start(struct hw_endpoint* ep, uint8_t* buffer, uint16_t to
 
   if (ep->active) {
     // TODO: Is this acceptable for interrupt packets?
-    TU_LOG(1, "WARN: starting new transfer on already active ep %d %s\r\n", tu_edpt_number(ep->ep_addr),
-           ep_dir_string[tu_edpt_dir(ep->ep_addr)]);
-
+    TU_LOG(1, "WARN: starting new transfer on already active ep %02X\r\n", ep->ep_addr);
     hw_endpoint_reset_transfer(ep);
   }
 
@@ -314,8 +297,7 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint* ep) {
 
   // Part way through a transfer
   if (!ep->active) {
-    panic("Can't continue xfer on inactive ep %d %s", tu_edpt_number(ep->ep_addr),
-          ep_dir_string[tu_edpt_dir(ep->ep_addr)]);
+    panic("Can't continue xfer on inactive ep %02X", ep->ep_addr);
   }
 
   // Update EP struct from hardware state
@@ -324,8 +306,7 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint* ep) {
   // Now we have synced our state with the hardware. Is there more data to transfer?
   // If we are done then notify tinyusb
   if (ep->remaining_len == 0) {
-    pico_trace("Completed transfer of %d bytes on ep %d %s\r\n",
-               ep->xferred_len, tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]);
+    pico_trace("Completed transfer of %d bytes on ep %02X\r\n", ep->xferred_len, ep->ep_addr);
     // Notify caller we are done so it can notify the tinyusb stack
     hw_endpoint_lock_update(ep, -1);
     return true;