dcd(rusb2): iso alloc/activate; bound the FIFO-ready wait

An unpolled full iso-IN pipe keeps FRDY low forever and froze the stack
with IRQs masked; bound the spin and abort the FIFO access.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HeF2gZ1M7GWkz6Av4BpKPg

1 files changed, 143 insertions, 32 deletions

diff --git a/src/portable/renesas/rusb2/dcd_rusb2.c b/src/portable/renesas/rusb2/dcd_rusb2.c
index f0ef9738b..5e42f63f5 100644
--- a/src/portable/renesas/rusb2/dcd_rusb2.c
+++ b/src/portable/renesas/rusb2/dcd_rusb2.c
@@ -28,6 +28,9 @@ typedef struct {
 
   uint8_t ep; /* an assigned endpoint address */
   uint8_t ff; /* `buf` is TU_FUFO or POD */
+  bool queued;      /* a transfer is submitted and not yet completed (independent of `buf`, which is
+                       NULL for a zero-length read) -- used to decide clear-stall re-arm */
+  bool zlp_pending; /* a zero-length IN packet couldn't be queued at submit (FIFO full); retry on BRDY */
 } pipe_state_t;
 
 typedef struct
@@ -121,9 +124,19 @@ static uint16_t edpt_max_packet_size(rusb2_reg_t *rusb, unsigned num) {
   return rusb->PIPEMAXP;
 }
 
-static inline void pipe_wait_for_ready(rusb2_reg_t * rusb, unsigned num) {
-  while ( rusb->D0FIFOSEL_b.CURPIPE != num ) {}
-  while ( !rusb->D0FIFOCTR_b.FRDY ) {}
+// Select the D0FIFO for `num` and wait until its buffer is ready for CPU access. Both flags
+// normally settle within a few cycles (the pipe was just armed, or a BRDY freed a plane). But an
+// IN pipe whose double buffer is already full stalls FRDY until the host drains it, and a
+// no-handshake iso IN endpoint the host has stopped polling never drains at all — so FRDY would
+// hang forever. This runs with the USB IRQ masked, so a naked spin freezes the whole stack; bound
+// it and let the caller abort the FIFO access. Returns false on timeout.
+#define RUSB2_FIFO_READY_SPIN 100000u
+static inline bool pipe_wait_for_ready(rusb2_reg_t *rusb, unsigned num) {
+  uint32_t spin = RUSB2_FIFO_READY_SPIN;
+  while ( rusb->D0FIFOSEL_b.CURPIPE != num ) { if (!spin--) return false; }
+  spin = RUSB2_FIFO_READY_SPIN;
+  while ( !rusb->D0FIFOCTR_b.FRDY ) { if (!spin--) return false; }
+  return true;
 }
 
 //--------------------------------------------------------------------+
@@ -201,6 +214,12 @@ static bool pipe0_xfer_out(rusb2_reg_t *rusb) {
   pipe->remaining = rem - len;
   if ((len < mps) || (rem == len)) {
     pipe->buf = NULL;
+    // Flow-control the single-buffer control pipe: NAK further OUT until usbd arms the next
+    // data-stage chunk. usbd receives a multi-packet control-OUT one CFG_TUD_ENDPOINT0_SIZE
+    // packet per submit; without this the DCP auto-accepts the next back-to-back packet into the
+    // just-emptied buffer and the following BRDY (remaining==0) BCLR-discards it, dropping 64
+    // bytes mid-transfer (e.g. usbtest ctrl_out 512B). RA4M1 UM R01UH0887 DCPCTR.PID.
+    rusb->DCPCTR = RUSB2_PIPE_CTR_PID_NAK;
     return true;
   }
 
@@ -226,7 +245,12 @@ static bool pipe_xfer_in(rusb2_reg_t* rusb, unsigned num)
   }
 
   const uint16_t mps = edpt_max_packet_size(rusb, num);
-  pipe_wait_for_ready(rusb, num);
+  if (!pipe_wait_for_ready(rusb, num)) {
+    // Buffer never came ready (double-buffered IN pipe full, host not draining). Drop this load;
+    // the transfer stays pending and is retried when a BRDY frees a plane or the pipe is re-armed.
+    rusb->D0FIFOSEL = 0;
+    return false;
+  }
   uint16_t len = tu_min16(rem, mps);
   void    *buf = pipe->buf;
 
@@ -267,7 +291,10 @@ static bool pipe_xfer_out(rusb2_reg_t* rusb, unsigned num)
   rusb->D0FIFOSEL = fifo_sel;
 
   const uint16_t mps = edpt_max_packet_size(rusb, num);
-  pipe_wait_for_ready(rusb, num);
+  if (!pipe_wait_for_ready(rusb, num)) {
+    rusb->D0FIFOSEL = 0;
+    return false; // FIFO not ready; leave the receive pending (BRDY re-enters when data arrives)
+  }
 
   const uint16_t vld  = (uint16_t)rusb->D0FIFOCTR_b.DTLN;
   const uint16_t len  = tu_min16(tu_min16(rem, mps), vld);
@@ -370,6 +397,24 @@ static bool process_pipe0_xfer(rusb2_reg_t *rusb, int buffer_type, uint8_t ep_ad
   return true;
 }
 
+// Queue a zero-length IN packet. Returns false if the FIFO buffer wasn't free (double-buffered pipe
+// full, host not draining) so BVAL couldn't be written -- the caller retries on the next BRDY.
+static bool pipe_zlp_in(rusb2_reg_t *rusb, unsigned num) {
+  rusb->D0FIFOSEL = (uint16_t) num;
+  const bool ready = pipe_wait_for_ready(rusb, num);
+  if (ready) {
+    rusb->D0FIFOCTR = RUSB2_CFIFOCTR_BVAL_Msk;
+  }
+  rusb->D0FIFOSEL = 0;
+  // deselect completes within a few bus cycles (not host-dependent), but bound it anyway: this
+  // runs with the USB IRQ masked, where any stuck spin freezes the whole stack
+  uint32_t spin = RUSB2_FIFO_READY_SPIN;
+  while (rusb->D0FIFOSEL_b.CURPIPE) {
+    if (!spin--) { break; }
+  }
+  return ready;
+}
+
 static bool process_pipe_xfer(rusb2_reg_t* rusb, int buffer_type, uint8_t ep_addr, void* buffer, uint16_t total_bytes)
 {
   const unsigned epn = tu_edpt_number(ep_addr);
@@ -379,23 +424,20 @@ static bool process_pipe_xfer(rusb2_reg_t* rusb, int buffer_type, uint8_t ep_add
   TU_ASSERT(num);
 
   pipe_state_t *pipe  = &_dcd.pipe[num];
-  pipe->ff        = buffer_type;
-  pipe->buf       = buffer;
-  pipe->length    = total_bytes;
-  pipe->remaining = total_bytes;
+  pipe->ff          = buffer_type;
+  pipe->buf         = buffer;
+  pipe->length      = total_bytes;
+  pipe->remaining   = total_bytes;
+  pipe->queued      = true;
+  pipe->zlp_pending = false;
 
   if (dir) {
     /* IN */
     if (total_bytes) {
       pipe_xfer_in(rusb, num);
     } else {
-      /* ZLP */
-      rusb->D0FIFOSEL = num;
-      pipe_wait_for_ready(rusb, num);
-      rusb->D0FIFOCTR = RUSB2_CFIFOCTR_BVAL_Msk;
-      rusb->D0FIFOSEL = 0;
-      /* if CURPIPE bits changes, check written value */
-      while (rusb->D0FIFOSEL_b.CURPIPE) {}
+      /* ZLP: if the FIFO buffer isn't free yet, defer the queue to the next BRDY (see process_pipe_brdy) */
+      pipe->zlp_pending = !pipe_zlp_in(rusb, num);
     }
   } else {
     // OUT
@@ -448,7 +490,15 @@ static void process_pipe_brdy(uint8_t rhport, unsigned num)
 
   if (dir) {
     /* IN */
-    completed = pipe_xfer_in(rusb, num);
+    if (pipe->zlp_pending) {
+      // The submit-time ZLP couldn't be queued (FIFO full); a freed buffer plane lets us queue it
+      // now. Don't report completion until the ZLP is actually queued (and then sent, next BRDY),
+      // otherwise a spurious BRDY would complete a zero-length IN the host never received.
+      pipe->zlp_pending = !pipe_zlp_in(rusb, num);
+      completed = false;
+    } else {
+      completed = pipe_xfer_in(rusb, num);
+    }
   } else {
     // OUT
     if (num) {
@@ -458,6 +508,7 @@ static void process_pipe_brdy(uint8_t rhport, unsigned num)
     }
   }
   if (completed) {
+    pipe->queued = false;
     dcd_event_xfer_complete(rhport, pipe->ep,
                             pipe->length - pipe->remaining,
                             XFER_RESULT_SUCCESS, true);
@@ -704,8 +755,14 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * ep_desc)
     }
   }
 
-  const unsigned num = find_pipe(xfer);
-  TU_ASSERT(num);
+  // Re-opening an endpoint must reuse its pipe: usbd_edpt_close() is a no-op on ISO_ALLOC ports,
+  // so a class's close/open across SET_INTERFACE (e.g. video's notification endpoint) would
+  // otherwise allocate a second pipe with the same EPNUM and leak pipes until exhaustion.
+  unsigned num = _dcd.ep[dir][epn];
+  if (num == 0) {
+    num = find_pipe(xfer);
+    TU_ASSERT(num);
+  }
 
   _dcd.pipe[num].ep = ep_addr;
   _dcd.ep[dir][epn] = num;
@@ -748,6 +805,8 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * ep_desc)
   return true;
 }
 
+static void edpt_close(uint8_t rhport, uint8_t ep_addr);
+
 void dcd_edpt_close_all(uint8_t rhport)
 {
   unsigned i = TU_ARRAY_SIZE(_dcd.pipe);
@@ -757,12 +816,14 @@ void dcd_edpt_close_all(uint8_t rhport)
     if (!ep_addr) {
       continue;
     }
-    dcd_edpt_close(rhport, (uint8_t)ep_addr);
+    edpt_close(rhport, (uint8_t)ep_addr);
   }
   dcd_int_enable(rhport);
 }
 
-void dcd_edpt_close(uint8_t rhport, uint8_t ep_addr)
+// Internal helper: on this (ISO_ALLOC) IP the stack no longer calls dcd_edpt_close(); only
+// dcd_edpt_close_all() uses it to tear down each pipe.
+static void edpt_close(uint8_t rhport, uint8_t ep_addr)
 {
   rusb2_reg_t * rusb = RUSB2_REG(rhport);
   const unsigned epn = tu_edpt_number(ep_addr);
@@ -774,24 +835,68 @@ void dcd_edpt_close(uint8_t rhport, uint8_t ep_addr)
   *ctr = 0;
   rusb->PIPESEL = (uint16_t)num;
   rusb->PIPECFG = 0;
-  _dcd.pipe[num].ep = 0;
+  _dcd.pipe[num].ep          = 0;
+  _dcd.pipe[num].queued      = false;
+  _dcd.pipe[num].zlp_pending = false;
   _dcd.ep[dir][epn] = 0;
 }
 
-#if 0
 bool dcd_edpt_iso_alloc(uint8_t rhport, uint8_t ep_addr, uint16_t largest_packet_size) {
-  (void)rhport;
-  (void)ep_addr;
-  (void)largest_packet_size;
-  return false;
+  rusb2_reg_t * rusb = RUSB2_REG(rhport);
+  const unsigned epn = tu_edpt_number(ep_addr);
+  const unsigned dir = tu_edpt_dir(ep_addr);
+
+  // Fullspeed ISO is limited to 256 bytes
+  if (!rusb2_is_highspeed_rhport(rhport) && largest_packet_size > 256) {
+    return false;
+  }
+
+  // Reserve an ISO-capable pipe (1 or 2) once; it persists across altsetting changes so
+  // dcd_edpt_iso_activate() only has to re-arm it in place (no pipe free/realloc, which on this
+  // shared-register IP would churn PIPESEL/PIPECFG and disturb the other pipes).
+  const unsigned num = find_pipe(TUSB_XFER_ISOCHRONOUS);
+  TU_ASSERT(num);
+  _dcd.pipe[num].ep = ep_addr;
+  _dcd.ep[dir][epn] = num;
+
+  dcd_int_disable(rhport);
+  if (rusb2_is_highspeed_rhport(rhport)) {
+    // FIXME (as in dcd_edpt_open): PIPEBUF is a PIPESEL-windowed register (RA6M5 UM §29.2.35) so it
+    //       must be written AFTER PIPESEL selects this pipe, and the fixed BUFNMB=0x08 overlaps every
+    //       HS pipe — a real per-pipe buffer allocator is needed. Left as-is: no RA6M5 HS board on
+    //       the HIL rig to validate a change, and the current mis-ordered write is inert on FS/RA4M1.
+    rusb->PIPEBUF = 0x7C08;
+  }
+  rusb->PIPESEL = (uint16_t) num;
+  rusb->PIPEMAXP = largest_packet_size;
+  volatile uint16_t *ctr = get_pipectr(rusb, num);
+  *ctr = RUSB2_PIPE_CTR_ACLRM_Msk | RUSB2_PIPE_CTR_SQCLR_Msk;
+  *ctr = 0; // leave the pipe NAKing until activated
+  rusb->PIPECFG = (uint16_t) ((dir << 4) | epn | RUSB2_PIPECFG_TYPE_ISO | RUSB2_PIPECFG_DBLB_Msk);
+  rusb->BRDYSTS = (uint16_t) (0x3FFu ^ TU_BIT(num));
+  rusb->BRDYENB |= TU_BIT(num);
+  dcd_int_enable(rhport);
+  return true;
 }
 
 bool dcd_edpt_iso_activate(uint8_t rhport, const tusb_desc_endpoint_t *desc_ep) {
-  (void)rhport;
-  (void)desc_ep;
-  return false;
+  rusb2_reg_t * rusb = RUSB2_REG(rhport);
+  const uint8_t ep_addr = desc_ep->bEndpointAddress;
+  const unsigned epn = tu_edpt_number(ep_addr);
+  const unsigned dir = tu_edpt_dir(ep_addr);
+  const unsigned num = _dcd.ep[dir][epn];
+  TU_ASSERT(num); // must have been iso-alloc'd
+
+  dcd_int_disable(rhport);
+  rusb->PIPESEL = (uint16_t) num;
+  rusb->PIPEMAXP = tu_edpt_packet_size(desc_ep);
+  volatile uint16_t *ctr = get_pipectr(rusb, num);
+  *ctr = RUSB2_PIPE_CTR_ACLRM_Msk | RUSB2_PIPE_CTR_SQCLR_Msk; // abort in-flight + reset data toggle
+  *ctr = 0;
+  *ctr = RUSB2_PIPE_CTR_PID_BUF; // enable
+  dcd_int_enable(rhport);
+  return true;
 }
-#endif
 
 bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes, bool is_isr)
 {
@@ -847,7 +952,13 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr)
   } else {
     const unsigned num = _dcd.ep[0][tu_edpt_number(ep_addr)];
     rusb->PIPESEL = (uint16_t)num;
-    if (rusb->PIPECFG_b.TYPE != 1) {
+    // Non-bulk OUT re-enables straight away. Bulk OUT is normally armed together with its transaction
+    // counter (TRE) by process_pipe_xfer(), so we don't blindly re-enable it here — but if a receive
+    // was already armed (still queued), SQCLR above just left it NAKing. Re-assert BUF so it keeps
+    // receiving; the class driver still considers that read submitted and never re-arms it, so
+    // otherwise the endpoint NAKs forever (usbtest toggle test 29 clears the halt on an armed pipe).
+    // `queued` (not `buf`) is the armed test: a zero-length OUT read has buf==NULL yet is armed.
+    if (rusb->PIPECFG_b.TYPE != 1 || _dcd.pipe[num].queued) {
       *ctr = RUSB2_PIPE_CTR_PID_BUF;
     }
   }