Merge pull request #3295 from hathach/dwc2_ep0

dcd/dwc2: fix EP0 multi-packet transfer logic

4 files changed, 75 insertions, 46 deletions

diff --git a/src/class/dfu/dfu_device.c b/src/class/dfu/dfu_device.c
index 0d2b63b57..d3cc53918 100644
--- a/src/class/dfu/dfu_device.c
+++ b/src/class/dfu/dfu_device.c
@@ -50,21 +50,17 @@
 typedef struct {
   uint8_t attrs;
   uint8_t alt;
+  uint8_t state;
+  uint8_t status;
 
-  dfu_state_t state;
-  dfu_status_t status;
-
-  bool flashing_in_progress;
+  bool     flashing_in_progress;
   uint16_t block;
   uint16_t length;
 } dfu_state_ctx_t;
 
-// Only a single dfu state is allowed
 static dfu_state_ctx_t _dfu_ctx;
 
-CFG_TUD_MEM_SECTION static struct {
-  TUD_EPBUF_DEF(transfer_buf, CFG_TUD_DFU_XFER_BUFSIZE);
-} _dfu_epbuf;
+TU_ATTR_ALIGNED(4) uint8_t _transfer_buf[CFG_TUD_DFU_XFER_BUFSIZE];
 
 static void reset_state(void) {
   _dfu_ctx.state = DFU_IDLE;
@@ -253,6 +249,8 @@ bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, const tusb_control
           tud_control_status(rhport, request);
         } else if (stage == CONTROL_STAGE_ACK) {
           tud_dfu_detach_cb();
+        } else {
+          // nothing to do
         }
         break;
 
@@ -275,6 +273,8 @@ bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, const tusb_control
           tud_control_status(rhport, request);
         } else if (stage == CONTROL_STAGE_ACK) {
           tud_dfu_abort_cb(_dfu_ctx.alt);
+        } else {
+          // nothing to do
         }
         break;
 
@@ -283,10 +283,10 @@ bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, const tusb_control
           TU_VERIFY(_dfu_ctx.attrs & DFU_ATTR_CAN_UPLOAD);
           TU_VERIFY(request->wLength <= CFG_TUD_DFU_XFER_BUFSIZE);
 
-          const uint16_t xfer_len = tud_dfu_upload_cb(_dfu_ctx.alt, request->wValue, _dfu_epbuf.transfer_buf,
+          const uint16_t xfer_len = tud_dfu_upload_cb(_dfu_ctx.alt, request->wValue, _transfer_buf,
                                                       request->wLength);
 
-          return tud_control_xfer(rhport, request, _dfu_epbuf.transfer_buf, xfer_len);
+          return tud_control_xfer(rhport, request, _transfer_buf, xfer_len);
         }
         break;
 
@@ -303,10 +303,10 @@ bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, const tusb_control
           _dfu_ctx.block = request->wValue;
           _dfu_ctx.length = request->wLength;
 
-          if (request->wLength) {
+          if (request->wLength > 0) {
             // Download with payload -> transition to DOWNLOAD SYNC
             _dfu_ctx.state = DFU_DNLOAD_SYNC;
-            return tud_control_xfer(rhport, request, _dfu_epbuf.transfer_buf, request->wLength);
+            return tud_control_xfer(rhport, request, _transfer_buf, request->wLength);
           } else {
             // Download is complete -> transition to MANIFEST SYNC
             _dfu_ctx.state = DFU_MANIFEST_SYNC;
@@ -352,6 +352,8 @@ void tud_dfu_finish_flashing(uint8_t status) {
       _dfu_ctx.state = (_dfu_ctx.attrs & DFU_ATTR_MANIFESTATION_TOLERANT)
                          ? DFU_MANIFEST_SYNC
                          : DFU_MANIFEST_WAIT_RESET;
+    } else {
+      // nothing to do
     }
   } else {
     // failed while flashing, move to dfuError
@@ -378,10 +380,12 @@ static bool process_download_get_status(uint8_t rhport, uint8_t stage, const tus
   } else if (stage == CONTROL_STAGE_ACK) {
     if (_dfu_ctx.flashing_in_progress) {
       _dfu_ctx.state = DFU_DNBUSY;
-      tud_dfu_download_cb(_dfu_ctx.alt, _dfu_ctx.block, _dfu_epbuf.transfer_buf, _dfu_ctx.length);
+      tud_dfu_download_cb(_dfu_ctx.alt, _dfu_ctx.block, _transfer_buf, _dfu_ctx.length);
     } else {
       _dfu_ctx.state = DFU_DNLOAD_IDLE;
     }
+  } else {
+    // nothing to do
   }
 
   return true;
@@ -409,6 +413,8 @@ static bool process_manifest_get_status(uint8_t rhport, uint8_t stage, const tus
     } else {
       _dfu_ctx.state = DFU_IDLE;
     }
+  } else {
+    // nothing to do
   }
 
   return true;
diff --git a/src/device/usbd_control.c b/src/device/usbd_control.c
index 2c2ff76b7..996e7387f 100644
--- a/src/device/usbd_control.c
+++ b/src/device/usbd_control.c
@@ -60,7 +60,7 @@ typedef struct {
 static usbd_control_xfer_t _ctrl_xfer;
 
 CFG_TUD_MEM_SECTION static struct {
-  TUD_EPBUF_DEF(buf, CFG_TUD_ENDPOINT0_SIZE);
+  TUD_EPBUF_DEF(buf, CFG_TUD_ENDPOINT0_BUFSIZE);
 } _ctrl_epbuf;
 
 //--------------------------------------------------------------------+
@@ -88,13 +88,13 @@ bool tud_control_status(uint8_t rhport, const tusb_control_request_t* request) {
 // Each transaction has up to Endpoint0's max packet size.
 // This function can also transfer an zero-length packet
 static bool data_stage_xact(uint8_t rhport) {
-  const uint16_t xact_len = tu_min16(_ctrl_xfer.data_len - _ctrl_xfer.total_xferred, CFG_TUD_ENDPOINT0_SIZE);
+  const uint16_t xact_len = tu_min16(_ctrl_xfer.data_len - _ctrl_xfer.total_xferred, CFG_TUD_ENDPOINT0_BUFSIZE);
   uint8_t ep_addr = EDPT_CTRL_OUT;
 
   if (_ctrl_xfer.request.bmRequestType_bit.direction == TUSB_DIR_IN) {
     ep_addr = EDPT_CTRL_IN;
     if (0u != xact_len) {
-      TU_VERIFY(0 == tu_memcpy_s(_ctrl_epbuf.buf, CFG_TUD_ENDPOINT0_SIZE, _ctrl_xfer.buffer, xact_len));
+      TU_VERIFY(0 == tu_memcpy_s(_ctrl_epbuf.buf, CFG_TUD_ENDPOINT0_BUFSIZE, _ctrl_xfer.buffer, xact_len));
     }
   }
 
@@ -179,7 +179,7 @@ bool usbd_control_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result,
   // Data Stage is complete when all request's length are transferred or
   // a short packet is sent including zero-length packet.
   if ((_ctrl_xfer.request.wLength == _ctrl_xfer.total_xferred) ||
-      (xferred_bytes < CFG_TUD_ENDPOINT0_SIZE)) {
+      (xferred_bytes < CFG_TUD_ENDPOINT0_BUFSIZE)) {
     // DATA stage is complete
     bool is_ok = true;
 
diff --git a/src/portable/synopsys/dwc2/dcd_dwc2.c b/src/portable/synopsys/dwc2/dcd_dwc2.c
index 06579fbb3..dc38d7561 100644
--- a/src/portable/synopsys/dwc2/dcd_dwc2.c
+++ b/src/portable/synopsys/dwc2/dcd_dwc2.c
@@ -332,6 +332,40 @@ static void edpt_disable(uint8_t rhport, uint8_t ep_addr, bool stall) {
   }
 }
 
+static uint16_t epin_write_tx_fifo(uint8_t rhport, uint8_t epnum) {
+  dwc2_regs_t* dwc2 = DWC2_REG(rhport);
+  dwc2_dep_t* const epin = &dwc2->ep[0][epnum];
+  xfer_ctl_t* const xfer = XFER_CTL_BASE(epnum, TUSB_DIR_IN);
+
+  dwc2_ep_tsize_t tsiz = {.value = epin->tsiz};
+  const uint16_t remain_packets = tsiz.packet_count;
+
+  uint16_t total_bytes_written = 0;
+  // Process every single packet (only whole packets can be written to fifo)
+  for (uint16_t i = 0; i < remain_packets; i++) {
+    tsiz.value = epin->tsiz;
+    const uint16_t remain_bytes = (uint16_t) tsiz.xfer_size;
+    const uint16_t xact_bytes = tu_min16(remain_bytes, xfer->max_size);
+
+    // Check if dtxfsts has enough space available
+    if (xact_bytes > ((epin->dtxfsts & DTXFSTS_INEPTFSAV_Msk) << 2)) {
+      break;
+    }
+
+    // Push packet to Tx-FIFO
+    if (xfer->ff) {
+      volatile uint32_t* tx_fifo = dwc2->fifo[epnum];
+      tu_fifo_read_n_const_addr_full_words(xfer->ff, (void*)(uintptr_t)tx_fifo, xact_bytes);
+      total_bytes_written += xact_bytes;
+    } else {
+      dfifo_write_packet(dwc2, epnum, xfer->buffer, xact_bytes);
+      xfer->buffer += xact_bytes;
+      total_bytes_written += xact_bytes;
+    }
+  }
+  return total_bytes_written;
+}
+
 // Since this function returns void, it is not possible to return a boolean success message
 // We must make sure that this function is not called when the EP is disabled
 // Must be called from critical section
@@ -345,7 +379,7 @@ static void edpt_schedule_packets(uint8_t rhport, const uint8_t epnum, const uin
 
   // EP0 is limited to one packet per xfer
   if (epnum == 0) {
-    total_bytes = tu_min16(_dcd_data.ep0_pending[dir], xfer->max_size);
+    total_bytes = tu_min16(_dcd_data.ep0_pending[dir], CFG_TUD_ENDPOINT0_SIZE);
     _dcd_data.ep0_pending[dir] -= total_bytes;
     num_packets = 1;
   } else {
@@ -383,12 +417,21 @@ static void edpt_schedule_packets(uint8_t rhport, const uint8_t epnum, const uin
     }
     dep->diepdma = (uintptr_t) xfer->buffer;
     dep->diepctl = depctl.value; // enable endpoint
+    // Advance buffer pointer for EP0
+    if (epnum == 0) {
+      xfer->buffer += total_bytes;
+    }
   } else {
     dep->diepctl = depctl.value; // enable endpoint
 
-    // Enable tx fifo empty interrupt only if there is data. Note must after depctl enable
     if (dir == TUSB_DIR_IN && total_bytes != 0) {
-      dwc2->diepempmsk |= (1u << epnum); //-V629
+      const uint16_t xferred_bytes = epin_write_tx_fifo(rhport, epnum);
+
+      // Enable TXFE interrupt if there are still data to be sent
+      // EP0 only sends one packet at a time, so no need to check for EP0
+      if ((epnum != 0) && (xfer->total_len - xferred_bytes > 0)) {
+         dwc2->diepempmsk |= (1u << epnum);
+      }
     }
   }
 }
@@ -836,11 +879,9 @@ static void handle_rxflvl_irq(uint8_t rhport) {
         const dwc2_ep_tsize_t tsiz = {.value = epout->tsiz};
         xfer->total_len -= tsiz.xfer_size;
         if (epnum == 0) {
-          xfer->total_len -= _dcd_data.ep0_pending[TUSB_DIR_OUT];
           _dcd_data.ep0_pending[TUSB_DIR_OUT] = 0;
         }
       }
-
       break;
     }
 
@@ -900,32 +941,10 @@ static void handle_epin_slave(uint8_t rhport, uint8_t epnum, dwc2_diepint_t diep
   // - 64 bytes or
   // - Half/Empty of TX FIFO size (configured by GAHBCFG.TXFELVL)
   if (diepint_bm.txfifo_empty && tu_bit_test(dwc2->diepempmsk, epnum)) {
-    dwc2_ep_tsize_t tsiz = {.value = epin->tsiz};
-    const uint16_t remain_packets = tsiz.packet_count;
-
-    // Process every single packet (only whole packets can be written to fifo)
-    for (uint16_t i = 0; i < remain_packets; i++) {
-      tsiz.value = epin->tsiz;
-      const uint16_t remain_bytes = (uint16_t) tsiz.xfer_size;
-      const uint16_t xact_bytes = tu_min16(remain_bytes, xfer->max_size);
-
-      // Check if dtxfsts has enough space available
-      if (xact_bytes > ((epin->dtxfsts & DTXFSTS_INEPTFSAV_Msk) << 2)) {
-        break;
-      }
-
-      // Push packet to Tx-FIFO
-      if (xfer->ff != NULL) {
-        volatile uint32_t* tx_fifo = dwc2->fifo[epnum];
-        tu_fifo_read_n_const_addr_full_words(xfer->ff, (void*)(uintptr_t)tx_fifo, xact_bytes);
-      } else {
-        dfifo_write_packet(dwc2, epnum, xfer->buffer, xact_bytes);
-        xfer->buffer += xact_bytes;
-      }
-    }
+    epin_write_tx_fifo(rhport, epnum);
 
     // Turn off TXFE if all bytes are written.
-    tsiz.value = epin->tsiz;
+    dwc2_ep_tsize_t tsiz = {.value = epin->tsiz};
     if (tsiz.xfer_size == 0) {
       dwc2->diepempmsk &= ~(1u << epnum);
     }
diff --git a/src/tusb_option.h b/src/tusb_option.h
index b90df2724..eed14214d 100644
--- a/src/tusb_option.h
+++ b/src/tusb_option.h
@@ -485,6 +485,10 @@
   #define CFG_TUD_ENDPOINT0_SIZE  64
 #endif
 
+#ifndef CFG_TUD_ENDPOINT0_BUFSIZE
+  #define CFG_TUD_ENDPOINT0_BUFSIZE  CFG_TUD_ENDPOINT0_SIZE
+#endif
+
 #ifndef CFG_TUD_INTERFACE_MAX
   #define CFG_TUD_INTERFACE_MAX   16
 #endif