update hcd to handle interrupt per buf

3 files changed, 63 insertions, 62 deletions

diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c
index ec73cafc1..0665484a0 100644
--- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c
+++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c
@@ -69,7 +69,7 @@ TU_ATTR_ALWAYS_INLINE static inline hw_endpoint_t *hw_endpoint_get_by_addr(uint8
   return hw_endpoint_get(num, dir);
 }
 
-TU_ATTR_ALWAYS_INLINE static inline io_rw_32 *get_ep_ctrl(const uint8_t epnum, tusb_dir_t dir) {
+TU_ATTR_ALWAYS_INLINE static inline io_rw_32 *get_ep_ctrl(uint8_t epnum, tusb_dir_t dir) {
   if (epnum == 0) {
     // EP0 has no endpoint control register because the buffer offsets are fixed and always enabled
     return NULL;
@@ -78,7 +78,7 @@ TU_ATTR_ALWAYS_INLINE static inline io_rw_32 *get_ep_ctrl(const uint8_t epnum, t
   return (dir == TUSB_DIR_IN) ? &ep_ctrl->in : &ep_ctrl->out;
 }
 
-TU_ATTR_ALWAYS_INLINE static inline io_rw_32 *get_buf_ctrl(const uint8_t epnum, tusb_dir_t dir) {
+TU_ATTR_ALWAYS_INLINE static inline io_rw_32 *get_buf_ctrl(uint8_t epnum, tusb_dir_t dir) {
   struct usb_device_dpram_ep_buf_ctrl *buf_ctrl = &usb_dpram->ep_buf_ctrl[epnum];
   return (dir == TUSB_DIR_IN) ? &buf_ctrl->in : &buf_ctrl->out;
 }
@@ -182,6 +182,7 @@ static void __tusb_irq_path_func(handle_hw_buff_status)(void) {
     // Read which buffer to handle BEFORE clearing buf_status
     uint8_t buf_id = (usb_hw->buf_cpu_should_handle & bit) ? 1 : 0;
     usb_hw_clear->buf_status = bit;
+    buf_status &= ~bit;
 
     // IN transfer for even i, OUT transfer for odd i
     const uint8_t    epnum = i >> 1u;
@@ -205,8 +206,6 @@ static void __tusb_irq_path_func(handle_hw_buff_status)(void) {
       hw_endpoint_reset_transfer(ep);
       dcd_event_xfer_complete(0, ep->ep_addr, xferred_len, XFER_RESULT_SUCCESS, true);
     }
-
-    buf_status &= ~bit;
   }
 }
 
diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c
index 9ee15d343..4ebdf8284 100644
--- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c
+++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c
@@ -94,7 +94,13 @@ static hw_endpoint_t *edpt_find(uint8_t daddr, uint8_t ep_addr) {
   return NULL;
 }
 
-// static hw_endpoint_t* epdt_find_interrupt(uint8_t )
+TU_ATTR_ALWAYS_INLINE static inline io_rw_32 *dpram_int_ep_ctrl(uint8_t int_num) {
+  return &usbh_dpram->int_ep_ctrl[int_num-1].ctrl;
+}
+
+TU_ATTR_ALWAYS_INLINE static inline io_rw_32 * dpram_int_ep_buffer_ctrl(uint8_t int_num) {
+  return &usbh_dpram->int_ep_buffer_ctrl[int_num-1].ctrl;
+}
 
 //--------------------------------------------------------------------+
 //
@@ -129,57 +135,53 @@ static void __tusb_irq_path_func(hw_xfer_complete)(hw_endpoint_t *ep, xfer_resul
   }
 }
 
-static void __tusb_irq_path_func(handle_hwbuf_status_bit)(hw_endpoint_t *ep, io_rw_32 *ep_reg, io_rw_32 *buf_reg) {
-  const bool done = hw_endpoint_xfer_continue(ep, ep_reg, buf_reg);
-  if (done) {
-    hw_xfer_complete(ep, XFER_RESULT_SUCCESS);
-  }
-}
-
 static void __tusb_irq_path_func(handle_hwbuf_status)(void) {
-  uint32_t buf_status = usb_hw->buf_status;
   pico_trace("buf_status 0x%08lx\n", buf_status);
+  enum {
+    BUF_STATUS_EPX = 1u
+  };
 
-  // Check EPX first
-  uint32_t bit = 1u;
-  if (buf_status & bit) {
-    buf_status &= ~bit;
-    usb_hw_clear->buf_status = bit;
+  // Check EPX first (bit 0). EPX is currently single-buffered, always use buf_id=0.
+  // Double-buffered: if both buffers completed at once, buf_status re-sets
+  // immediately after clearing (datasheet Table 406). Process the second buffer too.
+  while (usb_hw->buf_status & BUF_STATUS_EPX) {
+    const uint8_t buf_id = (usb_hw->buf_cpu_should_handle & BUF_STATUS_EPX) ? 1 : 0;
+    usb_hw_clear->buf_status = 1u; // clear
 
     io_rw_32 *ep_reg  = &usbh_dpram->epx_ctrl;
     io_rw_32 *buf_reg = &usbh_dpram->epx_buf_ctrl;
-    handle_hwbuf_status_bit(epx, ep_reg, buf_reg);
+    if (hw_endpoint_xfer_continue(epx, ep_reg, buf_reg, buf_id)) {
+      hw_xfer_complete(epx, XFER_RESULT_SUCCESS);
+    }
   }
 
   // Check "interrupt" (asynchronous) endpoints for both IN and OUT
-  // TODO use clz for better efficiency
-  for (uint i = 1; i <= USB_HOST_INTERRUPT_ENDPOINTS && buf_status; i++) {
-    // EPX  IN/OUT is bit 0, 1
-    // IEP1 IN/OUT is bit 2, 3
-    // IEP2 IN/OUT is bit 4, 5
-    // etc
-    for (uint j = 0; j < 2; j++) {
-      bit = 1 << (i * 2 + j);
-      if (buf_status & bit) {
-        buf_status &= ~bit;
-        usb_hw_clear->buf_status = bit;
+  uint32_t buf_status = usb_hw->buf_status & ~1u;
+  while (buf_status) {
+    // ctz/clz is faster than loop which has only a few bit set in general
+    const uint8_t idx = (uint8_t) __builtin_ctz(buf_status);
+    const uint bit = TU_BIT(idx);
+    usb_hw_clear->buf_status = bit;
+    buf_status &= ~bit;
 
-        for (uint8_t e = 0; e < USB_MAX_ENDPOINTS; e++) {
-          hw_endpoint_t *ep = &ep_pool[e];
-          if (ep->interrupt_num == i) {
-            io_rw_32 *ep_reg  = &usbh_dpram->int_ep_ctrl[ep->interrupt_num - 1].ctrl;
-            io_rw_32 *buf_reg = &usbh_dpram->int_ep_buffer_ctrl[ep->interrupt_num - 1].ctrl;
-            handle_hwbuf_status_bit(ep, ep_reg, buf_reg);
-            break;
-          }
+    // IN transfer for even i, OUT transfer for odd i
+    // EPX  is bit 0. Bit 1 is not used
+    // IEP1 IN/OUT is bit 2, 3
+    // IEP2 IN/OUT is bit 4, 5 etc
+    const uint8_t    epnum = idx >> 1u;
+    for (size_t e = 0; e < TU_ARRAY_SIZE(ep_pool); e++) {
+      hw_endpoint_t *ep = &ep_pool[e];
+      if (ep->interrupt_num == epnum) {
+        io_rw_32 *ep_reg  = dpram_int_ep_ctrl(ep->interrupt_num);
+        io_rw_32 *buf_reg = dpram_int_ep_buffer_ctrl(ep->interrupt_num);
+        const bool done = hw_endpoint_xfer_continue(ep, ep_reg, buf_reg, 0);
+        if (done) {
+          hw_xfer_complete(ep, XFER_RESULT_SUCCESS);
         }
+        break;
       }
     }
   }
-
-  if (buf_status) {
-    panic("Unhandled buffer %d\n", buf_status);
-  }
 }
 
 // All non-interrupt endpoints use shared EPX.
@@ -491,8 +493,10 @@ void hcd_device_close(uint8_t rhport, uint8_t dev_addr) {
         usb_hw_clear->int_ep_ctrl                       = 1u << ep->interrupt_num;
         usb_hw->int_ep_addr_ctrl[ep->interrupt_num - 1] = 0;
 
-        usbh_dpram->int_ep_buffer_ctrl[ep->interrupt_num - 1].ctrl = 0;
-        usbh_dpram->int_ep_ctrl[ep->interrupt_num - 1].ctrl        = 0;
+        io_rw_32 *ep_reg  = dpram_int_ep_ctrl(ep->interrupt_num);
+        io_rw_32 *buf_reg = dpram_int_ep_buffer_ctrl(ep->interrupt_num);
+        *buf_reg = 0;
+        *ep_reg = 0;
       }
 
       ep->max_packet_size = 0; // mark as unused
@@ -547,13 +551,12 @@ TU_ATTR_ALWAYS_INLINE static inline void sie_start_xfer(uint32_t value) {
   usb_hw->sie_ctrl = value | USB_SIE_CTRL_START_TRANS_BITS;
 }
 
-// xfer using epx
 static void edpt_xfer(hw_endpoint_t *ep, uint8_t *buffer, tu_fifo_t *ff, uint16_t total_len) {
   if (ep->transfer_type == TUSB_XFER_INTERRUPT) {
     // For interrupt endpoint control and buffer is already configured
     // Note: Interrupt is single buffered only
-    io_rw_32 *ep_reg  = &usbh_dpram->int_ep_ctrl[ep->interrupt_num - 1].ctrl;
-    io_rw_32 *buf_reg = &usbh_dpram->int_ep_buffer_ctrl[ep->interrupt_num - 1].ctrl;
+    io_rw_32 *ep_reg  = dpram_int_ep_ctrl(ep->interrupt_num);
+    io_rw_32 *buf_reg = dpram_int_ep_buffer_ctrl(ep->interrupt_num);
     hw_endpoint_xfer_start(ep, ep_reg, buf_reg, buffer, ff, total_len);
   } else {
     const uint8_t    ep_num = tu_edpt_number(ep->ep_addr);
diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c
index ac1536f5b..1e2c4f211 100644
--- a/src/portable/raspberrypi/rp2040/rp2040_usb.c
+++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c
@@ -124,8 +124,10 @@ void __tusb_irq_path_func(hwbuf_ctrl_update)(io_rw_32 *buf_ctrl_reg, uint32_t an
 
       // Section 4.1.2.7.1 (rp2040) / 12.7.3.7.1 (rp2350) Concurrent access: after write to buffer control,
       // wait for USB controller to see the update before setting AVAILABLE.
-      // Host also needs this for continuation buffers in multi-packet transfers.
-      busy_wait_at_least_cycles(12);
+      // Don't need delay in host mode as host is in charge of when to start the transaction.
+      if (!is_host) {
+        busy_wait_at_least_cycles(12);
+      }
     }
   }
 
@@ -193,13 +195,13 @@ void __tusb_irq_path_func(hw_endpoint_buffer_xact)(struct hw_endpoint *ep, io_rw
     // If both complete simultaneously, buf_status re-sets on next clock (datasheet Table 406).
     uint32_t ep_ctrl = *ep_reg | EP_CTRL_INTERRUPT_PER_BUFFER;
 
-    // Since short packet on buf0 in double-buffered RX: buf1 may already contain data from the
-    // NEXT transfer (host sent it before CPU processed this IRQ). Cannot safely recover. Avoid by not using double
-    // buffering for rx transfer
-    bool force_single = is_rx;
-  #if CFG_TUH_ENABLED
-    force_single |= (is_host && ep->interrupt_num != 0); // host interrupt is single only
-  #endif
+    // For now: skip double buffered for RX e.g OUT endpoint in Device mode, since host could send < 64 bytes and cause
+    // short packet on buffer0
+    // NOTE: this could happen to Host mode IN endpoint Also, Host mode "interrupt" endpoint hardware is only single
+    // buffered,
+    // NOTE2: Currently Host bulk is implemented using "interrupt" endpoint
+    const bool force_single = (!is_host && is_rx) || (is_host && tu_edpt_number(ep->ep_addr) != 0);
+    // bool force_single = is_rx || (is_host && ep->interrupt_num != 0);
 
     if (ep->remaining_len && !force_single) {
       // Use buffer 1 (double buffered) if there is still data
@@ -318,14 +320,11 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint *ep, io_
   const bool is_rx       = is_host ? (dir == TUSB_DIR_IN) : (dir == TUSB_DIR_OUT);
   const bool is_double   = ep_reg != NULL && ((*ep_reg) & EP_CTRL_DOUBLE_BUFFERED_BITS);
 
-  const uint16_t xferred = hwbuf_sync(ep, buf_reg, is_double ? buf_id : 0, is_rx);
-  bool is_done = (ep->remaining_len == 0);
+  hwbuf_sync(ep, buf_reg, buf_id, is_rx);
+  const bool is_done = (ep->remaining_len == 0);
 
   if (is_double) {
-    if (xferred < ep->max_packet_size) {
-      // Short packet
-      is_done = true;
-    } else if (buf_id == 0) {
+    if (buf_id == 0) {
       // buf0 done: wait for buf1, don't start new buffers
       hw_endpoint_lock_update(ep, -1);
       return false;