usbd: disable xfer_isr path for ch32 usbfs/usbhs

docs/reference/device_issues.rst

@@ -33,3 +33,8 @@ Reference: `CH32V30X Reference Manual`_ USBFS/USBHS controller chapter
 Data corruption may occur on isochronous endpoints. Due to the lacking of FIFO for interrupt status registers, later completed transfer will overwrite `INT_ST` and `RX_LEN` register if previous transfer processing is not completed.
 
 Other types of transfers are not affected.
+
+**Partial workaround:**
+  - Keep USB interrupt as the highest priority interrupt.
+  - Force the device to full-speed mode by setting dev_init.speed = TUSB_SPEED_FULL when calling tusb_init() to give more time for the device to process the transfer before the next transfer is completed.
+  - Keep the device configuration simple, e.g. only one isochronous endpoint, to reduce the processing time of each transfer.

src/class/audio/audio_device.c

@@ -392,11 +392,11 @@
 tu_static CFG_TUD_MEM_SECTION audiod_function_t _audiod_fct[CFG_TUD_AUDIO];
 
 #if CFG_TUD_AUDIO_ENABLE_EP_OUT
-static bool audiod_rx_xfer_isr(uint8_t rhport, audiod_function_t* audio, uint16_t n_bytes_received);
+static bool audiod_rx_done(uint8_t rhport, audiod_function_t* audio, uint16_t n_bytes_received, bool is_isr);
 #endif
 
 #if CFG_TUD_AUDIO_ENABLE_EP_IN
-static bool audiod_tx_xfer_isr(uint8_t rhport, audiod_function_t* audio, uint16_t n_bytes_sent);
+static bool audiod_tx_done(uint8_t rhport, audiod_function_t* audio, uint16_t n_bytes_sent, bool is_isr);
 #endif
 
 static bool audiod_get_interface(uint8_t rhport, tusb_control_request_t const *p_request);
@@ -463,18 +463,18 @@
   return NULL;
 }
 
-static bool audiod_rx_xfer_isr(uint8_t rhport, audiod_function_t* audio, uint16_t n_bytes_received) {
+static bool audiod_rx_done(uint8_t rhport, audiod_function_t* audio, uint16_t n_bytes_received, bool is_isr) {
   uint8_t idx_audio_fct = audiod_get_audio_fct_idx(audio);
 
   #if !CFG_TUD_EDPT_DEDICATED_HWFIFO
   // Data currently is in linear buffer, copy into EP OUT FIFO
   TU_VERIFY(0 < tu_fifo_write_n(&audio->ep_out_ff, audio->lin_buf_out, n_bytes_received));
 
   // Schedule for next receive
-  TU_VERIFY(usbd_edpt_xfer(rhport, audio->ep_out, audio->lin_buf_out, audio->ep_out_sz, true));
+  TU_VERIFY(usbd_edpt_xfer(rhport, audio->ep_out, audio->lin_buf_out, audio->ep_out_sz, is_isr));
   #else
   // Data is already placed in EP FIFO, schedule for next receive
-  TU_VERIFY(usbd_edpt_xfer_fifo(rhport, audio->ep_out, &audio->ep_out_ff, audio->ep_out_sz, true));
+  TU_VERIFY(usbd_edpt_xfer_fifo(rhport, audio->ep_out, &audio->ep_out_ff, audio->ep_out_sz, is_isr));
   #endif
 
   #if CFG_TUD_AUDIO_ENABLE_FEEDBACK_EP
@@ -526,7 +526,7 @@
   }
 }
 
-static bool audiod_tx_xfer_isr(uint8_t rhport, audiod_function_t * audio, uint16_t n_bytes_sent) {
+static bool audiod_tx_done(uint8_t rhport, audiod_function_t * audio, uint16_t n_bytes_sent, bool is_isr) {
   uint8_t idx_audio_fct = audiod_get_audio_fct_idx(audio);
 
   // Only send something if current alternate interface is not 0 as in this case nothing is to be sent due to UAC2 specifications
@@ -543,10 +543,10 @@
   #endif
   #if !CFG_TUD_EDPT_DEDICATED_HWFIFO
   tu_fifo_read_n(&audio->ep_in_ff, audio->lin_buf_in, n_bytes_tx);
-  TU_VERIFY(usbd_edpt_xfer(rhport, audio->ep_in, audio->lin_buf_in, n_bytes_tx, true));
+  TU_VERIFY(usbd_edpt_xfer(rhport, audio->ep_in, audio->lin_buf_in, n_bytes_tx, is_isr));
   #else
   // Send everything in ISO EP FIFO
-  TU_VERIFY(usbd_edpt_xfer_fifo(rhport, audio->ep_in, &audio->ep_in_ff, n_bytes_tx, true));
+  TU_VERIFY(usbd_edpt_xfer_fifo(rhport, audio->ep_in, &audio->ep_in_ff, n_bytes_tx, is_isr));
   #endif
 
   // Call a weak callback here - a possibility for user to get informed former TX was completed and data gets now loaded into EP in buffer
@@ -1463,38 +1463,53 @@
 
 bool audiod_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) {
   (void) result;
-  (void) xferred_bytes;
 
-  #if CFG_TUD_AUDIO_ENABLE_INTERRUPT_EP
-  // Search for interface belonging to given end point address and proceed as required
   for (uint8_t func_id = 0; func_id < CFG_TUD_AUDIO; func_id++) {
     audiod_function_t *audio = &_audiod_fct[func_id];
+    (void) func_id;
 
-    // Data transmission of control interrupt finished
-    if (audio->ep_int == ep_addr) {
-      // According to USB2 specification, maximum payload of interrupt EP is 8 bytes on low speed, 64 bytes on full speed, and 1024 bytes on high speed (but only if an alternate interface other than 0 is used - see specification p. 49)
-      // In case there is nothing to send we have to return a NAK - this is taken care of by PHY ???
-      // In case of an erroneous transmission a retransmission is conducted - this is taken care of by PHY ???
+#if !TUP_USBD_XFER_ISR
+  #if CFG_TUD_AUDIO_ENABLE_EP_IN
+    if (audio->ep_in == ep_addr) {
+      audiod_tx_done(rhport, audio, (uint16_t) xferred_bytes, false);
+      return true;
+    }
+  #endif
 
-      // I assume here, that things above are handled by PHY
-      // All transmission is done - what remains to do is to inform job was completed
+  #if CFG_TUD_AUDIO_ENABLE_EP_OUT
+    if (audio->ep_out == ep_addr) {
+      audiod_rx_done(rhport, audio, (uint16_t) xferred_bytes, false);
+      return true;
+    }
+    #if CFG_TUD_AUDIO_ENABLE_FEEDBACK_EP
+    if (audio->ep_fb == ep_addr) {
+      audiod_fb_send(func_id, false);
+      return true;
+    }
+    #endif
+  #endif
+#endif // !TUP_USBD_XFER_ISR
 
+  #if CFG_TUD_AUDIO_ENABLE_INTERRUPT_EP
+    // Data transmission of control interrupt finished
+    if (audio->ep_int == ep_addr) {
       tud_audio_int_done_cb(rhport);
       return true;
     }
+  #endif
 
+    (void) audio;
   }
-  #else
+
   (void) rhport;
   (void) ep_addr;
-  #endif
-
+  (void) xferred_bytes;
   return false;
 }
 
+#if TUP_USBD_XFER_ISR
 bool audiod_xfer_isr(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) {
   (void) result;
-  (void) xferred_bytes;
 
   // Search for interface belonging to given end point address and proceed as required
   for (uint8_t func_id = 0; func_id < CFG_TUD_AUDIO; func_id++)
@@ -1514,15 +1529,15 @@
       // This is the only place where we can fill something into the EPs buffer!
 
       // Load new data
-      audiod_tx_xfer_isr(rhport, audio, (uint16_t) xferred_bytes);
+      audiod_tx_done(rhport, audio, (uint16_t) xferred_bytes, true);
       return true;
     }
 #endif
 
 #if CFG_TUD_AUDIO_ENABLE_EP_OUT
     // New audio packet received
     if (audio->ep_out == ep_addr) {
-      audiod_rx_xfer_isr(rhport, audio, (uint16_t) xferred_bytes);
+      audiod_rx_done(rhport, audio, (uint16_t) xferred_bytes, true);
       return true;
     }
   #if CFG_TUD_AUDIO_ENABLE_FEEDBACK_EP
@@ -1539,6 +1554,7 @@
 
   return false;
 }
+#endif // TUP_USBD_XFER_ISR
 
 #if CFG_TUD_AUDIO_ENABLE_EP_OUT && CFG_TUD_AUDIO_ENABLE_FEEDBACK_EP
 

src/class/audio/audio_device.h

@@ -461,7 +461,9 @@ void     audiod_reset          (uint8_t rhport);
 uint16_t audiod_open           (uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len);
 bool     audiod_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request);
 bool     audiod_xfer_cb        (uint8_t rhport, uint8_t edpt_addr, xfer_result_t result, uint32_t xferred_bytes);
+#if TUP_USBD_XFER_ISR
 bool     audiod_xfer_isr       (uint8_t rhport, uint8_t edpt_addr, xfer_result_t result, uint32_t xferred_bytes);
+#endif
 void     audiod_sof_isr        (uint8_t rhport, uint32_t frame_count);
 
 #ifdef __cplusplus

src/common/tusb_mcu.h

@@ -589,6 +589,7 @@
 #elif TU_CHECK_MCU(OPT_MCU_CH32F20X)
   #define TUP_USBIP_WCH_USBHS
   #define TUP_USBIP_WCH_USBFS
+  #define TUP_USBD_XFER_ISR 0
 
   #if !defined(CFG_TUD_WCH_USBIP_USBFS)
     #define CFG_TUD_WCH_USBIP_USBFS 0
@@ -607,6 +608,7 @@
 
 #elif TU_CHECK_MCU(OPT_MCU_CH32V103)
   #define TUP_USBIP_WCH_USBFS
+  #define TUP_USBD_XFER_ISR 0
 
   #if !defined(CFG_TUD_WCH_USBIP_USBFS)
     #define CFG_TUD_WCH_USBIP_USBFS 1
@@ -617,6 +619,7 @@
 #elif TU_CHECK_MCU(OPT_MCU_CH32V20X)
   // v20x support both port0 FSDEV (USBD) and port1 USBFS
   #define TUP_USBIP_WCH_USBFS
+  #define TUP_USBD_XFER_ISR 0
 
   #ifndef CFG_TUH_WCH_USBIP_USBFS
     #define CFG_TUH_WCH_USBIP_USBFS 1
@@ -643,6 +646,7 @@
   // v307 support both FS and HS, default to HS
   #define TUP_USBIP_WCH_USBHS
   #define TUP_USBIP_WCH_USBFS
+  #define TUP_USBD_XFER_ISR 0
 
   #if !defined(CFG_TUD_WCH_USBIP_USBFS)
     #define CFG_TUD_WCH_USBIP_USBFS 0
@@ -665,6 +669,7 @@
   // the shared hcd_ch32_usbfs.c is CH32V20x-specific and does not support CH58x, so host /
   // USB2 (rhport 1) is not provided here.
   #define TUP_USBIP_WCH_USBFS
+  #define TUP_USBD_XFER_ISR 0
 
   #ifndef CFG_TUD_WCH_USBIP_USBFS
     #define CFG_TUD_WCH_USBIP_USBFS 1
@@ -775,3 +780,9 @@
 #if CFG_TUD_ENDPOINT_ONE_DIRECTION_ONLY && !defined(TUD_ENDPOINT_ONE_DIRECTION_ONLY)
   #define TUD_ENDPOINT_ONE_DIRECTION_ONLY
 #endif
+
+// Enable xfer_isr callback in usbd for handling transfer completion in ISR context.
+// Set to 0 for DCDs that cannot safely re-arm transfers from within their own ISR (e.g. WCH).
+#ifndef TUP_USBD_XFER_ISR
+  #define TUP_USBD_XFER_ISR 1
+#endif

src/device/usbd.c

@@ -179,7 +179,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = cdcd_open,
         .control_xfer_cb  = cdcd_control_xfer_cb,
         .xfer_cb          = cdcd_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -193,7 +195,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = mscd_open,
         .control_xfer_cb  = mscd_control_xfer_cb,
         .xfer_cb          = mscd_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -207,7 +211,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = hidd_open,
         .control_xfer_cb  = hidd_control_xfer_cb,
         .xfer_cb          = hidd_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -221,7 +227,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = audiod_open,
         .control_xfer_cb  = audiod_control_xfer_cb,
         .xfer_cb          = audiod_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = audiod_xfer_isr,
+#endif
         .sof              = audiod_sof_isr
     },
     #endif
@@ -235,7 +243,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = videod_open,
         .control_xfer_cb  = videod_control_xfer_cb,
         .xfer_cb          = videod_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -249,7 +259,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .reset            = midid_reset,
         .control_xfer_cb  = midid_control_xfer_cb,
         .xfer_cb          = midid_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -263,7 +275,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .reset            = midi2d_reset,
         .control_xfer_cb  = midi2d_control_xfer_cb,
         .xfer_cb          = midi2d_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -277,7 +291,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = vendord_open,
         .control_xfer_cb  = tud_vendor_control_xfer_cb,
         .xfer_cb          = vendord_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -291,7 +307,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = usbtmcd_open_cb,
         .control_xfer_cb  = usbtmcd_control_xfer_cb,
         .xfer_cb          = usbtmcd_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -305,7 +323,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = dfu_rtd_open,
         .control_xfer_cb  = dfu_rtd_control_xfer_cb,
         .xfer_cb          = NULL,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -319,7 +339,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = dfu_moded_open,
         .control_xfer_cb  = dfu_moded_control_xfer_cb,
         .xfer_cb          = NULL,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -333,7 +355,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = netd_open,
         .control_xfer_cb  = netd_control_xfer_cb,
         .xfer_cb          = netd_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL,
     },
     #endif
@@ -347,7 +371,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = btd_open,
         .control_xfer_cb  = btd_control_xfer_cb,
         .xfer_cb          = btd_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -361,7 +387,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = mtpd_open,
         .control_xfer_cb  = mtpd_control_xfer_cb,
         .xfer_cb          = mtpd_xfer_cb,
+#if TUP_USBD_XFER_ISR
         .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -375,6 +403,9 @@ static const usbd_class_driver_t _usbd_driver[] = {
         .open             = printerd_open,
         .control_xfer_cb  = printerd_control_xfer_cb,
         .xfer_cb          = printerd_xfer_cb,
+#if TUP_USBD_XFER_ISR
+        .xfer_isr         = NULL,
+#endif
         .sof              = NULL
     },
     #endif
@@ -1452,6 +1483,7 @@ TU_ATTR_FAST_FUNC void dcd_event_handler(dcd_event_t const* event, bool in_isr)
       uint8_t const ep_dir = tu_edpt_dir(ep_addr);
 
       send = true;
+#if TUP_USBD_XFER_ISR
       if(epnum > 0) {
         usbd_class_driver_t const* driver = get_driver(_usbd_dev.ep2drv[epnum][ep_dir]);
 
@@ -1468,6 +1500,10 @@ TU_ATTR_FAST_FUNC void dcd_event_handler(dcd_event_t const* event, bool in_isr)
           }
         }
       }
+#else
+      (void) epnum;
+      (void) ep_dir;
+#endif
       break;
     }
 

src/device/usbd_pvt.h

@@ -57,7 +57,9 @@ typedef struct {
   uint16_t (* open             ) (uint8_t rhport, tusb_desc_interface_t const * desc_intf, uint16_t max_len);
   bool     (* control_xfer_cb  ) (uint8_t rhport, uint8_t stage, tusb_control_request_t const * request);
   bool     (* xfer_cb          ) (uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes);
+#if TUP_USBD_XFER_ISR
   bool     (* xfer_isr         ) (uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes); // optional, return false to defer to xfer_cb()
+#endif
   void     (* sof              ) (uint8_t rhport, uint32_t frame_count); // optional
 } usbd_class_driver_t;