only apply errata E5 and E15 for rp2040. rp2350 already fixes these

4 files changed, 105 insertions, 120 deletions

diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c
index 952245cb1..bdd0d1728 100644
--- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c
+++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c
@@ -92,6 +92,12 @@ static void hw_endpoint_init(hw_endpoint_t *ep, uint8_t ep_addr, uint16_t wMaxPa
     // double buffered Bulk endpoint
     if (transfer_type == TUSB_XFER_BULK) {
       size *= 2u;
+
+  #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+      if (tu_edpt_dir(ep_addr) == TUSB_DIR_IN) {
+        ep->e15_bulk_in = true;
+      }
+  #endif
     }
 
     // assign buffer
@@ -221,17 +227,14 @@ static void __tusb_irq_path_func(dcd_rp2040_irq)(void) {
       struct hw_endpoint *ep = hw_endpoint_get(i, TUSB_DIR_IN);
 
       // Active Bulk IN endpoint requires SOF
-      if ((ep->transfer_type == TUSB_XFER_BULK) && ep->active) {
+      if (ep->e15_bulk_in && ep->active) {
         keep_sof_alive = true;
 
         hw_endpoint_lock_update(ep, 1);
-
-        // Deferred enable?
         if (ep->pending) {
           ep->pending = 0;
           hw_endpoint_start_next_buffer(ep);
         }
-
         hw_endpoint_lock_update(ep, -1);
       }
     }
@@ -292,7 +295,7 @@ static void __tusb_irq_path_func(dcd_rp2040_irq)(void) {
     dcd_event_bus_reset(0, TUSB_SPEED_FULL, true);
     usb_hw_clear->sie_status = USB_SIE_STATUS_BUS_RESET_BITS;
 
-#if TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX
+  #if TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX
     // Only run enumeration workaround if pull up is enabled
     if (usb_hw->sie_ctrl & USB_SIE_CTRL_PULLUP_EN_BITS) {
       rp2040_usb_device_enumeration_fix();
@@ -491,7 +494,6 @@ bool dcd_edpt_iso_activate(uint8_t rhport, const tusb_desc_endpoint_t *ep_desc)
 
 void dcd_edpt_close_all(uint8_t rhport) {
   (void) rhport;
-
   // may need to use EP Abort
   reset_non_control_endpoints();
 }
diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c
index db3a81c87..1193009ef 100644
--- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c
+++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c
@@ -82,15 +82,13 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t dev_speed(void) {
   return (usb_hw->sie_status & USB_SIE_STATUS_SPEED_BITS) >> USB_SIE_STATUS_SPEED_LSB;
 }
 
-TU_ATTR_ALWAYS_INLINE static inline bool need_pre(uint8_t dev_addr)
-{
+TU_ATTR_ALWAYS_INLINE static inline bool need_pre(uint8_t dev_addr) {
   // If this device is different to the speed of the root device
   // (i.e. is a low speed device on a full speed hub) then need pre
   return hcd_port_speed_get(0) != tuh_speed_get(dev_addr);
 }
 
-static void __tusb_irq_path_func(hw_xfer_complete)(struct hw_endpoint *ep, xfer_result_t xfer_result)
-{
+static void __tusb_irq_path_func(hw_xfer_complete)(struct hw_endpoint *ep, xfer_result_t xfer_result) {
   // Mark transfer as done before we tell the tinyusb stack
   uint8_t dev_addr = ep->dev_addr;
   uint8_t ep_addr = ep->ep_addr;
@@ -99,35 +97,28 @@ static void __tusb_irq_path_func(hw_xfer_complete)(struct hw_endpoint *ep, xfer_
   hcd_event_xfer_complete(dev_addr, ep_addr, xferred_len, xfer_result, true);
 }
 
-static void __tusb_irq_path_func(_handle_buff_status_bit)(uint bit, struct hw_endpoint *ep) {
+static void __tusb_irq_path_func(handle_hhwbuf_status_bit)(uint bit, struct hw_endpoint *ep) {
   usb_hw_clear->buf_status = bit;
-  // EP may have been stalled?
-  assert(ep->active);
-  bool done = hw_endpoint_xfer_continue(ep);
-  if ( done )
-  {
+  const bool done          = hw_endpoint_xfer_continue(ep);
+  if (done) {
     hw_xfer_complete(ep, XFER_RESULT_SUCCESS);
   }
 }
 
 static void __tusb_irq_path_func(handle_hwbuf_status)(void) {
-  uint32_t remaining_buffers = usb_hw->buf_status;
-  pico_trace("buf_status 0x%08lx\n", remaining_buffers);
+  uint32_t buf_status = usb_hw->buf_status;
+  pico_trace("buf_status 0x%08lx\n", buf_status);
 
   // Check EPX first
   uint bit = 0b1;
-  if (remaining_buffers & bit) {
-    remaining_buffers &= ~bit;
+  if (buf_status & bit) {
+    buf_status &= ~bit;
     struct hw_endpoint * ep = &epx;
-
-    // uint32_t ep_ctrl = *hwep_ctrl_reg_host(ep);
-    // TU_LOG_HEX(3, ep_ctrl);
-
-    _handle_buff_status_bit(bit, ep);
+    handle_hhwbuf_status_bit(bit, ep);
   }
 
   // Check "interrupt" (asynchronous) endpoints for both IN and OUT
-  for (uint i = 1; i <= USB_HOST_INTERRUPT_ENDPOINTS && remaining_buffers; i++) {
+  for (uint i = 1; i <= USB_HOST_INTERRUPT_ENDPOINTS && buf_status; i++) {
     // EPX is bit 0 & 1
     // IEP1 IN  is bit 2
     // IEP1 OUT is bit 3
@@ -138,15 +129,15 @@ static void __tusb_irq_path_func(handle_hwbuf_status)(void) {
     // etc
     for (uint j = 0; j < 2; j++) {
       bit = 1 << (i * 2 + j);
-      if (remaining_buffers & bit) {
-        remaining_buffers &= ~bit;
-        _handle_buff_status_bit(bit, &ep_pool[i]);
+      if (buf_status & bit) {
+        buf_status &= ~bit;
+        handle_hhwbuf_status_bit(bit, &ep_pool[i]);
       }
     }
   }
 
-  if (remaining_buffers) {
-    panic("Unhandled buffer %d\n", remaining_buffers);
+  if (buf_status) {
+    panic("Unhandled buffer %d\n", buf_status);
   }
 }
 
@@ -251,7 +242,7 @@ static struct hw_endpoint *_next_free_interrupt_ep(void)
     ep = &ep_pool[i];
     if ( !ep->configured )
     {
-      // Will be configured by _hw_endpoint_init / _hw_endpoint_allocate
+      // Will be configured by hw_endpoint_init / hw_endpoint_allocate
       ep->interrupt_num = (uint8_t) (i - 1);
       return ep;
     }
@@ -259,12 +250,13 @@ static struct hw_endpoint *_next_free_interrupt_ep(void)
   return ep;
 }
 
-static struct hw_endpoint *_hw_endpoint_allocate(uint8_t transfer_type)
-{
-  struct hw_endpoint * ep = NULL;
+static hw_endpoint_t *hw_endpoint_allocate(uint8_t transfer_type) {
+  hw_endpoint_t *ep = NULL;
 
-  if ( transfer_type != TUSB_XFER_CONTROL )
-  {
+  if (transfer_type == TUSB_XFER_CONTROL) {
+    ep              = &epx;
+    ep->hw_data_buf = &usbh_dpram->epx_data[0];
+  } else {
     // Note: even though datasheet name these "Interrupt" endpoints. These are actually
     // "Asynchronous" endpoints and can be used for other type such as: Bulk  (ISO need confirmation)
     ep = _next_free_interrupt_ep();
@@ -276,17 +268,12 @@ static struct hw_endpoint *_hw_endpoint_allocate(uint8_t transfer_type)
     // etc
     ep->hw_data_buf = &usbh_dpram->epx_data[64 * (ep->interrupt_num + 2)];
   }
-  else
-  {
-    ep = &epx;
-    ep->hw_data_buf = &usbh_dpram->epx_data[0];
-  }
 
   return ep;
 }
 
-static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t transfer_type, uint8_t bmInterval)
-{
+static void hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t ep_addr, uint16_t wMaxPacketSize,
+                             uint8_t transfer_type, uint8_t bmInterval) {
   // Already has data buffer, endpoint control, and buffer control allocated at this point
   assert(ep->hw_data_buf);
 
@@ -299,7 +286,6 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t
   // Response to a setup packet on EP0 starts with pid of 1
   ep->next_pid = (num == 0 ? 1u : 0u);
   ep->wMaxPacketSize = wMaxPacketSize;
-  ep->transfer_type = transfer_type;
 
   pico_trace("hw_endpoint_init dev %d ep %02X xfer %d\n", ep->dev_addr, ep->ep_addr, ep->transfer_type);
   pico_trace("dev %d ep %02X setup buffer @ 0x%p\n", ep->dev_addr, ep->ep_addr, ep->hw_data_buf);
@@ -463,46 +449,33 @@ void hcd_device_close(uint8_t rhport, uint8_t dev_addr) {
   }
 }
 
-uint32_t hcd_frame_number(uint8_t rhport)
-{
-  (void) rhport;
+uint32_t hcd_frame_number(uint8_t rhport) {
+  (void)rhport;
   return usb_hw->sof_rd;
 }
 
-void hcd_int_enable(uint8_t rhport)
-{
-  (void) rhport;
-  assert(rhport == 0);
+void hcd_int_enable(uint8_t rhport) {
+  (void)rhport;
   irq_set_enabled(USBCTRL_IRQ, true);
 }
 
-void hcd_int_disable(uint8_t rhport)
-{
-  (void) rhport;
+void hcd_int_disable(uint8_t rhport) {
+  (void)rhport;
   // todo we should check this is disabling from the correct core; note currently this is never called
-  assert(rhport == 0);
   irq_set_enabled(USBCTRL_IRQ, false);
 }
 
 //--------------------------------------------------------------------+
 // Endpoint API
 //--------------------------------------------------------------------+
-bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_endpoint_t const * ep_desc)
-{
-  (void) rhport;
-
+bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, const tusb_desc_endpoint_t *ep_desc) {
+  (void)rhport;
   pico_trace("hcd_edpt_open dev_addr %d, ep_addr %d\n", dev_addr, ep_desc->bEndpointAddress);
-
-  // Allocated differently based on if it's an interrupt endpoint or not
-  struct hw_endpoint *ep = _hw_endpoint_allocate(ep_desc->bmAttributes.xfer);
+  hw_endpoint_t *ep = hw_endpoint_allocate(ep_desc->bmAttributes.xfer);
   TU_ASSERT(ep);
 
-  _hw_endpoint_init(ep,
-                    dev_addr,
-                    ep_desc->bEndpointAddress,
-                    tu_edpt_packet_size(ep_desc),
-                    ep_desc->bmAttributes.xfer,
-                    ep_desc->bInterval);
+  hw_endpoint_init(ep, dev_addr, ep_desc->bEndpointAddress, tu_edpt_packet_size(ep_desc), ep_desc->bmAttributes.xfer,
+                   ep_desc->bInterval);
 
   return true;
 }
@@ -512,13 +485,12 @@ bool hcd_edpt_close(uint8_t rhport, uint8_t daddr, uint8_t ep_addr) {
   return false; // TODO not implemented yet
 }
 
-bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * buffer, uint16_t buflen)
-{
+bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t *buffer, uint16_t buflen) {
   (void) rhport;
 
   pico_trace("hcd_edpt_xfer dev_addr %d, ep_addr 0x%x, len %d\n", dev_addr, ep_addr, buflen);
 
-  uint8_t const ep_num = tu_edpt_number(ep_addr);
+  const uint8_t    ep_num = tu_edpt_number(ep_addr);
   tusb_dir_t const ep_dir = tu_edpt_dir(ep_addr);
 
   // Get appropriate ep. Either EPX or interrupt endpoint
@@ -530,19 +502,17 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t *
   assert(!ep->active);
 
   // Control endpoint can change direction 0x00 <-> 0x80
-  if ( ep_addr != ep->ep_addr )
-  {
+  if (ep_addr != ep->ep_addr) {
     assert(ep_num == 0);
 
     // Direction has flipped on endpoint control so re init it but with same properties
-    _hw_endpoint_init(ep, dev_addr, ep_addr, ep->wMaxPacketSize, ep->transfer_type, 0);
+    hw_endpoint_init(ep, dev_addr, ep_addr, ep->wMaxPacketSize, TUSB_XFER_CONTROL, 0);
   }
 
   // If a normal transfer (non-interrupt) then initiate using
   // sie ctrl registers. Otherwise, interrupt ep registers should
   // already be configured
-  if ( ep == &epx )
-  {
+  if (ep == &epx) {
     hw_endpoint_xfer_start(ep, buffer, buflen);
 
     // That has set up buffer control, endpoint control etc
@@ -558,8 +528,7 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t *
     usb_hw->sie_ctrl = flags & ~USB_SIE_CTRL_START_TRANS_BITS;
     busy_wait_at_least_cycles(12);
     usb_hw->sie_ctrl = flags;
-  }else
-  {
+  } else {
     hw_endpoint_xfer_start(ep, buffer, buflen);
   }
 
@@ -584,14 +553,14 @@ bool hcd_setup_send(uint8_t rhport, uint8_t dev_addr, uint8_t const setup_packet
   }
 
   // Configure EP0 struct with setup info for the trans complete
-  struct hw_endpoint * ep = _hw_endpoint_allocate( (uint8_t) TUSB_XFER_CONTROL);
+  hw_endpoint_t *ep = hw_endpoint_allocate((uint8_t)TUSB_XFER_CONTROL);
   TU_ASSERT(ep);
 
   // EPX should be inactive
   assert(!ep->active);
 
   // EP0 out
-  _hw_endpoint_init(ep, dev_addr, 0x00, ep->wMaxPacketSize, 0, 0);
+  hw_endpoint_init(ep, dev_addr, 0x00, ep->wMaxPacketSize, 0, 0);
   assert(ep->configured);
 
   ep->remaining_len = 8;
diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c
index 9c9255518..8f91ecf22 100644
--- a/src/portable/raspberrypi/rp2040/rp2040_usb.c
+++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c
@@ -38,10 +38,8 @@
 static void hwep_xfer_sync(hw_endpoint_t *ep);
 
   #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
-static bool e15_is_bulkin_ep(struct hw_endpoint *ep);
 static bool e15_is_critical_frame_period(struct hw_endpoint *ep);
   #else
-    #define e15_is_bulkin_ep(x)             (false)
     #define e15_is_critical_frame_period(x) (false)
   #endif
 
@@ -228,13 +226,16 @@ void hw_endpoint_xfer_start(struct hw_endpoint* ep, uint8_t* buffer, uint16_t to
   ep->active = true;
   ep->user_buf = buffer;
 
-  if (e15_is_bulkin_ep(ep)) {
+  #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+  if (ep->e15_bulk_in) {
     usb_hw_set->inte = USB_INTS_DEV_SOF_BITS;
   }
 
   if (e15_is_critical_frame_period(ep)) {
-    ep->pending = 1;
-  } else {
+    ep->pending = 1; // skip transfer if we are in critical frame period
+  } else
+  #endif
+  {
     hw_endpoint_start_next_buffer(ep);
   }
 
@@ -360,9 +361,12 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint* ep) {
     hw_endpoint_lock_update(ep, -1);
     return true;
   } else {
+  #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
     if (e15_is_critical_frame_period(ep)) {
       ep->pending = 1;
-    } else {
+    } else
+  #endif
+    {
       hw_endpoint_start_next_buffer(ep);
     }
   }
@@ -397,15 +401,12 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint* ep) {
 
 volatile uint32_t e15_last_sof = 0;
 
-// check if Errata 15 is needed for this endpoint i.e device bulk-in
-static bool __tusb_irq_path_func(e15_is_bulkin_ep)(struct hw_endpoint *ep) {
-  return (!rp2usb_is_host_mode() && tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN && ep->transfer_type == TUSB_XFER_BULK);
-}
-
 // check if we need to apply Errata 15 workaround : i.e
 // Endpoint is BULK IN and is currently in critical frame period i.e 20% of last usb frame
 static bool __tusb_irq_path_func(e15_is_critical_frame_period)(struct hw_endpoint* ep) {
-  TU_VERIFY(e15_is_bulkin_ep(ep));
+  if (!ep->e15_bulk_in) {
+    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
diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.h b/src/portable/raspberrypi/rp2040/rp2040_usb.h
index 368bb6509..944d604bc 100644
--- a/src/portable/raspberrypi/rp2040/rp2040_usb.h
+++ b/src/portable/raspberrypi/rp2040/rp2040_usb.h
@@ -13,17 +13,30 @@
   #error TinyUSB device and host mode not supported at the same time
 #endif
 
-#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
+// E5 and E15 only apply to RP2040
+#if defined(PICO_RP2040) && PICO_RP2040 == 1
+  // RP2040 E5: USB device fails to exit RESET state on busy USB bus.
+  #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
+
+  // RP2040 E15: USB Device controller will hang if certain bus errors occur during an IN transfer.
+  #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
+#endif
+
+#ifndef TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX
+  #define TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX 0
 #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
+#ifndef TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+  #define TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX 0
 #endif
 
 #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
-#undef PICO_RP2040_USB_FAST_IRQ
-#define PICO_RP2040_USB_FAST_IRQ 1
+  #undef PICO_RP2040_USB_FAST_IRQ
+  #define PICO_RP2040_USB_FAST_IRQ 1
 #endif
 
 #ifndef PICO_RP2040_USB_FAST_IRQ
@@ -46,36 +59,36 @@
 #define pico_trace(...) TU_LOG(3, __VA_ARGS__)
 
 // Hardware information per endpoint
-typedef struct hw_endpoint
-{
-    uint8_t ep_addr;
-    uint8_t next_pid;
-    // Interrupt, bulk, etc
-    uint8_t transfer_type;
+typedef struct hw_endpoint {
+  uint8_t ep_addr;
+  uint8_t next_pid;
+  uint8_t transfer_type;
 
-    bool    active;  // Endpoint is in use
-    uint8_t pending; // Transfer scheduled but not active
+  bool active;         // transferring data
 
-    uint16_t wMaxPacketSize;
+#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX
+  bool    e15_bulk_in; // Errata15 device bulk in
+  uint8_t pending;     // Transfer scheduled but not active
+#endif
 
-    uint8_t *hw_data_buf; // Buffer pointer in usb dpram
+  uint16_t wMaxPacketSize;
+  uint8_t *hw_data_buf; // Buffer pointer in usb dpram
 
-    // Current transfer information
-    uint8_t *user_buf; // User buffer in main memory
-    uint16_t remaining_len;
-    uint16_t xferred_len;
+  // Current transfer information
+  uint8_t *user_buf; // User buffer in main memory
+  uint16_t remaining_len;
+  uint16_t xferred_len;
 
 #if CFG_TUH_ENABLED
-    // Is this a valid struct
-    bool configured;
+  // Is this a valid struct
+  bool configured;
 
-    // Only needed for host
-    uint8_t dev_addr;
+  // Only needed for host
+  uint8_t dev_addr;
 
-    // If interrupt endpoint
-    uint8_t interrupt_num;
+  // If interrupt endpoint
+  uint8_t interrupt_num;
 #endif
-
 } hw_endpoint_t;
 
 #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX