fix(dwc2): enable periodic channels in the selected frame

Periodic IN and DMA-backed transfers selected ODDFRM before waiting for request-queue space. A DWC2 interrupt could also run between reading HFNUM and writing HCCHAR.CHENA, allowing the selected frame to pass while the transfer still appeared active.

Wait for request-queue capacity with controller interrupts enabled, then mask only GAHBCFG.GINT while sampling HFNUM and enabling a new periodic channel. Record the periodic phase from that same HFNUM sample so a boundary after channel enable cannot shift later interval calculations. The bounded critical section contains no queue wait, callback, disable, or allocation loop. Retries that already selected their frame bypass the new selection step.

Also clear a retained HCCHAR.CHDIS before every channel enable. A halted channel can otherwise be re-enabled as CHENA|CHDIS and wait for a terminal interrupt that never arrives.

Hardware traces captured periodic IN selections at frames 0x3303 and 0x3266 but activation only after 0x330c and 0x3273, respectively.

1 files changed, 54 insertions, 31 deletions

diff --git a/src/portable/synopsys/dwc2/hcd_dwc2.c b/src/portable/synopsys/dwc2/hcd_dwc2.c
index 55a15b326..31ea2ae20 100644
--- a/src/portable/synopsys/dwc2/hcd_dwc2.c
+++ b/src/portable/synopsys/dwc2/hcd_dwc2.c
@@ -204,13 +204,37 @@ TU_ATTR_ALWAYS_INLINE static inline bool channel_disable(const dwc2_regs_t* dwc2
   return true;
 }
 
-// attempt to send IN token to receive data
-TU_ATTR_ALWAYS_INLINE static inline bool channel_send_in_token(const dwc2_regs_t* dwc2, dwc2_channel_t* channel) {
+// Enable a channel, selecting the following frame for a new periodic transfer.
+// Return that frame from the same HFNUM sample used for ODDFRM selection.
+// Clear CHDIS explicitly: a halted channel may retain it in HCCHAR.
+TU_ATTR_ALWAYS_INLINE static inline uint16_t channel_enable(dwc2_regs_t* dwc2, dwc2_channel_t* channel,
+                                                            bool next_periodic_frame) {
+  uint32_t hcchar = channel->hcchar & ~HCCHAR_CHDIS;
+  uint16_t periodic_frame = 0;
+  if (next_periodic_frame) {
+    // Prevent the USB interrupt from consuming the selected frame before
+    // HCCHAR.CHENA is written. Queue-space waits happen before this helper.
+    const uint32_t gahbcfg = dwc2->gahbcfg;
+    dwc2->gahbcfg          = gahbcfg & ~GAHBCFG_GINT;
+    const uint32_t hfnum   = dwc2->hfnum;
+    hcchar                 = (hcchar & ~HCCHAR_ODDFRM) | (((hfnum & 1u) ^ 1u) << HCCHAR_ODDFRM_Pos);
+    channel->hcchar        = hcchar | HCCHAR_CHENA;
+    periodic_frame         = (uint16_t) ((hfnum + 1u) & HCD_FRAME_NUMBER_MASK);
+    dwc2->gahbcfg          = gahbcfg;
+  } else {
+    channel->hcchar = hcchar | HCCHAR_CHENA;
+  }
+  return periodic_frame;
+}
+
+// Attempt to send an IN token to receive data. For a new periodic transfer,
+// select its frame only after request-queue space is available.
+TU_ATTR_ALWAYS_INLINE static inline uint16_t channel_send_in_token(dwc2_regs_t* dwc2, dwc2_channel_t* channel,
+                                                                   bool next_periodic_frame) {
   while (0 == req_queue_avail(dwc2, channel_is_periodic(channel->hcchar))) {
     // blocking wait for request queue available
   }
-  channel->hcchar |= HCCHAR_CHENA;
-  return true;
+  return channel_enable(dwc2, channel, next_periodic_frame);
 }
 
 // Find currently enabled channel. Note: EP0 is bidirectional
@@ -612,20 +636,17 @@ static void channel_xfer_out_wrapup(dwc2_regs_t* dwc2, uint8_t ch_id) {
   edpt->buflen -= actual_bytes;
 }
 
-static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) {
+static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id, bool new_periodic_xfer) {
   hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id];
   hcd_endpoint_t* edpt = &_hcd_data.edpt[xfer->ep_id];
   dwc2_channel_char_t* hcchar_bm = &edpt->hcchar_bm;
   dwc2_channel_t* channel = &dwc2->channel[ch_id];
   bool const is_period = channel_is_periodic(edpt->hcchar);
-
+  uint16_t periodic_frame = 0;
   // clear previous state
   xfer->fifo_bytes = 0;
 
   // hchar: restore but don't enable yet
-  if (is_period) {
-    hcchar_bm->odd_frame = 1 - (dwc2->hfnum & 1);   // transfer on next frame
-  }
   channel->hcchar = (edpt->hcchar & ~HCCHAR_CHENA);
 
   // hctsiz: zero length packet still count as 1
@@ -659,10 +680,10 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) {
     channel->hcdma = (uint32_t) edpt->buffer;
 
     if (hcchar_bm->ep_dir == TUSB_DIR_IN) {
-      channel_send_in_token(dwc2, channel);
+      periodic_frame = channel_send_in_token(dwc2, channel, is_period);
     } else {
       hcd_dcache_clean(edpt->buffer, edpt->buflen);
-      channel->hcchar |= HCCHAR_CHENA;
+      periodic_frame = channel_enable(dwc2, channel, is_period);
     }
   } else {
     uint32_t hcintmsk = HCINT_NAK | HCINT_XACT_ERR | HCINT_STALL | HCINT_XFER_COMPLETE | HCINT_DATATOGGLE_ERR;
@@ -686,9 +707,9 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) {
     // IN Token. If we got NAK, we have to re-enable the channel again in the interrupt. Due to the way usbh stack only
     // call hcd_edpt_xfer() once, we will need to manage de-allocate/re-allocate IN channel dynamically.
     if (hcchar_bm->ep_dir == TUSB_DIR_IN) {
-      channel_send_in_token(dwc2, channel);
+      periodic_frame = channel_send_in_token(dwc2, channel, is_period);
     } else {
-      channel->hcchar |= HCCHAR_CHENA;
+      periodic_frame = channel_enable(dwc2, channel, is_period);
       if (edpt->buflen > 0) {
         // To prevent conflict with other channel, we will enable periodic/non-periodic FIFO empty interrupt accordingly
         // And write packet in the interrupt handler
@@ -697,6 +718,10 @@ static bool channel_xfer_start(dwc2_regs_t* dwc2, uint8_t ch_id) {
     }
   }
 
+  if (is_period && new_periodic_xfer) {
+    edpt->periodic_frame = periodic_frame;
+  }
+
   return true;
 }
 
@@ -707,11 +732,9 @@ static bool edpt_xfer_kickoff(dwc2_regs_t* dwc2, uint8_t ep_id) {
   hcd_xfer_t* xfer = &_hcd_data.xfer[ch_id];
   xfer->ep_id = ep_id;
   xfer->result = XFER_RESULT_INVALID;
-
-  const bool result = channel_xfer_start(dwc2, ch_id);
-  if (result && channel_is_periodic(_hcd_data.edpt[ep_id].hcchar)) {
-    hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id];
-    edpt->periodic_frame = (uint16_t) ((dwc2->hfnum + 1u) & HCD_FRAME_NUMBER_MASK);
+  hcd_endpoint_t* edpt = &_hcd_data.edpt[ep_id];
+  const bool result = channel_xfer_start(dwc2, ch_id, true);
+  if (result && channel_is_periodic(edpt->hcchar)) {
     edpt->periodic_phase = 1;
     edpt->xfer_pending = 0;
   }
@@ -858,7 +881,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci
       if (xfer->period_split_nyet_count < HCD_XFER_PERIOD_SPLIT_NYET_MAX) {
         hcchar.odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame
         channel->hcchar = hcchar.value;
-        channel_send_in_token(dwc2, channel);
+        channel_send_in_token(dwc2, channel, false);
         return;
       } else {
         // too many NYET, de-allocate channel with below code
@@ -871,7 +894,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci
       // retry on next frame if bInterval is 1
       hcchar.odd_frame = 1 - (dwc2->hfnum & 1);
       channel->hcchar = hcchar.value;
-      channel_send_in_token(dwc2, channel);
+      channel_send_in_token(dwc2, channel, false);
     } else {
       // otherwise, de-allocate channel, enable SOF set frame counter for later transfer
       const dwc2_channel_tsize_t hctsiz = {.value = channel->hctsiz};
@@ -884,7 +907,7 @@ static void channel_xfer_in_retry(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci
     }
   } else {
     // for control/bulk: retry immediately
-    channel_send_in_token(dwc2, channel);
+    channel_send_in_token(dwc2, channel, false);
   }
 }
 
@@ -1059,7 +1082,7 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h
         channel->hcintmsk |= HCINT_NYET;
         hcsplt.split_compl = 1;
         channel->hcsplt = hcsplt.value;
-        channel_send_in_token(dwc2, channel);
+        channel_send_in_token(dwc2, channel, false);
       } else {
         // do nothing for complete split with DATA, this will trigger XferComplete and handled there
       }
@@ -1070,7 +1093,7 @@ static bool handle_channel_in_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t h
         // still more packet to receive, also reset to start split
         hcsplt.split_compl = 0;
         channel->hcsplt = hcsplt.value;
-        channel_send_in_token(dwc2, channel);
+        channel_send_in_token(dwc2, channel, false);
       }
     }
   } else if (hcint & HCINT_HALTED) {
@@ -1157,7 +1180,7 @@ static bool handle_channel_out_slave(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t
       is_done = true;
     } else {
       // Got here due to NAK or NYET
-      TU_ASSERT(channel_xfer_start(dwc2, ch_id));
+      TU_ASSERT(channel_xfer_start(dwc2, ch_id, false));
     }
   } else if (hcint & HCINT_ACK) {
     xfer->err_count = 0;
@@ -1209,7 +1232,7 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci
       if (xfer->closing) {
         is_done = true;
       } else {
-        channel_send_in_token(dwc2, channel);
+        channel_send_in_token(dwc2, channel, false);
       }
     } else if (hcint & (HCINT_XFER_COMPLETE | HCINT_STALL | HCINT_BABBLE_ERR)) {
       const uint16_t remain_bytes = (uint16_t) hctsiz.xfer_size;
@@ -1270,7 +1293,7 @@ static bool handle_channel_in_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hci
           hcchar.odd_frame = 1 - (dwc2->hfnum & 1); // transfer on next frame
           channel->hcchar = hcchar.value;
         }
-        channel_send_in_token(dwc2, channel);
+        channel_send_in_token(dwc2, channel, false);
       }
     } else if (hcint & (HCINT_NAK | HCINT_DATATOGGLE_ERR)) {
       xfer->err_count = 0;
@@ -1321,7 +1344,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc
       if (xfer->closing) {
         is_done = true;
       } else {
-        channel_xfer_start(dwc2, ch_id);
+        channel_xfer_start(dwc2, ch_id, false);
       }
     } else if (hcint & (HCINT_XFER_COMPLETE | HCINT_STALL)) {
       is_done = true;
@@ -1339,7 +1362,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc
        xfer->err_count = 0;
        // clean up transfer so far and start again
        channel_xfer_out_wrapup(dwc2, ch_id);
-       channel_xfer_start(dwc2, ch_id);
+       channel_xfer_start(dwc2, ch_id, false);
      } else {
        xfer->err_count++;
        if (xfer->err_count >= HCD_XFER_ERROR_MAX) {
@@ -1355,7 +1378,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc
            xfer->retry_disabled = 1;
            channel_disable(dwc2, channel);
          } else {
-           channel_xfer_start(dwc2, ch_id);
+           channel_xfer_start(dwc2, ch_id, false);
          }
        }
      }
@@ -1365,7 +1388,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc
         xfer->result = XFER_RESULT_FAILED;
         is_done      = true;
       } else {
-        channel_xfer_start(dwc2, ch_id);
+        channel_xfer_start(dwc2, ch_id, false);
       }
     } else if (hcint & HCINT_NYET) {
       if (hcsplt.split_en && hcsplt.split_compl) {
@@ -1387,7 +1410,7 @@ static bool handle_channel_out_dma(dwc2_regs_t* dwc2, uint8_t ch_id, uint32_t hc
       // Non-split OUT NAK is core-handled (5.1.2.2), so this is split-only.
       xfer->err_count = 0;
       channel_xfer_out_wrapup(dwc2, ch_id);
-      channel_xfer_start(dwc2, ch_id);
+      channel_xfer_start(dwc2, ch_id, false);
     }
 
     if (xfer->closing == 1) {