update tinyusb to fix race condition hathach/tinyusb#2310

adafruit · Nov 3, 2023 · 6a87063 · 6a87063
1 parent 1708cff
commit 6a87063
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 62 deletions.
diff --git a/src/arduino/Adafruit_TinyUSB_API.cpp b/src/arduino/Adafruit_TinyUSB_API.cpp
@@ -57,22 +57,16 @@ void TinyUSB_Device_FlushCDC(void) {
 
 // Debug log with Serial1
 #if CFG_TUSB_DEBUG && defined(CFG_TUSB_DEBUG_PRINTF)
+
 // #define USE_SEGGER_RTT
+#define SERIAL_TUSB_DEBUG Serial1
 
 #ifdef USE_SEGGER_RTT
 #include "SEGGER_RTT/RTT/SEGGER_RTT.h"
 #endif
 
 __attribute__((used)) int CFG_TUSB_DEBUG_PRINTF(const char *__restrict format,
                                                 ...) {
-#ifndef USE_SEGGER_RTT
-  static bool ser1_inited = false;
-  if (!ser1_inited) {
-    ser1_inited = true;
-    Serial1.begin(115200);
-  }
-#endif
-
   char buf[256];
   int len;
   va_list ap;
@@ -82,7 +76,13 @@ __attribute__((used)) int CFG_TUSB_DEBUG_PRINTF(const char *__restrict format,
 #ifdef USE_SEGGER_RTT
   SEGGER_RTT_Write(0, buf, len);
 #else
-  Serial1.write(buf);
+  static volatile bool ser_inited = false;
+  if (!ser_inited) {
+    ser_inited = true;
+    SERIAL_TUSB_DEBUG.begin(115200);
+    // SERIAL_TUSB_DEBUG.begin(921600);
+  }
+  SERIAL_TUSB_DEBUG.write(buf);
 #endif
 
   va_end(ap);

diff --git a/src/arduino/ports/rp2040/tusb_config_rp2040.h b/src/arduino/ports/rp2040/tusb_config_rp2040.h
@@ -54,6 +54,7 @@ extern "C" {
 
 // For selectively disable device log (when > CFG_TUSB_DEBUG)
 // #define CFG_TUD_LOG_LEVEL 3
+// #define CFG_TUH_LOG_LEVEL 3
 
 #define CFG_TUSB_MEM_SECTION
 #define CFG_TUSB_MEM_ALIGN TU_ATTR_ALIGNED(4)

diff --git a/src/common/tusb_mcu.h b/src/common/tusb_mcu.h
@@ -217,9 +217,7 @@
 
   // TypeC controller
   #define TUP_USBIP_TYPEC_STM32
-
   #define TUP_DCD_ENDPOINT_MAX    8
-
   #define TUP_TYPEC_RHPORTS_NUM 1
 
 #elif TU_CHECK_MCU(OPT_MCU_STM32G0)
@@ -261,14 +259,21 @@
 #elif TU_CHECK_MCU(OPT_MCU_STM32U5)
   #define TUP_USBIP_DWC2
   #define TUP_USBIP_DWC2_STM32
-  #define TUP_DCD_ENDPOINT_MAX    6
+
+  // U59x/5Ax/5Fx/5Gx are highspeed with built-in HS PHY
+  #if defined(STM32U595xx) || defined(STM32U599xx) || defined(STM32U5A5xx) || defined(STM32U5A9xx) || \
+      defined(STM32U5F7xx) || defined(STM32U5F9xx) || defined(STM32U5G7xx) || defined(STM32U5G9xx)
+    #define TUP_DCD_ENDPOINT_MAX  9
+    #define TUP_RHPORT_HIGHSPEED  1
+  #else
+    #define TUP_DCD_ENDPOINT_MAX  6
+  #endif
 
 #elif TU_CHECK_MCU(OPT_MCU_STM32L5)
   #define TUP_USBIP_FSDEV
   #define TUP_USBIP_FSDEV_STM32
   #define TUP_DCD_ENDPOINT_MAX    8
 
-
 //--------------------------------------------------------------------+
 // Sony
 //--------------------------------------------------------------------+

diff --git a/src/device/usbd.h b/src/device/usbd.h
@@ -465,7 +465,7 @@ TU_ATTR_WEAK bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb
   /* Type I Format Type Descriptor(2.3.1.6 - Audio Formats) */\
   TUD_AUDIO_DESC_TYPE_I_FORMAT(_nBytesPerSample, _nBitsUsedPerSample),\
   /* Standard AS Isochronous Audio Data Endpoint Descriptor(4.10.1.1) */\
-  TUD_AUDIO_DESC_STD_AS_ISO_EP(/*_ep*/ _epin, /*_attr*/ (uint8_t) (TUSB_XFER_ISOCHRONOUS | TUSB_ISO_EP_ATT_ASYNCHRONOUS | TUSB_ISO_EP_ATT_DATA), /*_maxEPsize*/ _epsize, /*_interval*/ TUD_OPT_HIGH_SPEED ? 0x04 : 0x01),\
+  TUD_AUDIO_DESC_STD_AS_ISO_EP(/*_ep*/ _epin, /*_attr*/ (uint8_t) (TUSB_XFER_ISOCHRONOUS | TUSB_ISO_EP_ATT_ASYNCHRONOUS | TUSB_ISO_EP_ATT_DATA), /*_maxEPsize*/ _epsize, /*_interval*/ 0x01),\
   /* Class-Specific AS Isochronous Audio Data Endpoint Descriptor(4.10.1.2) */\
   TUD_AUDIO_DESC_CS_AS_ISO_EP(/*_attr*/ AUDIO_CS_AS_ISO_DATA_EP_ATT_NON_MAX_PACKETS_OK, /*_ctrl*/ AUDIO_CTRL_NONE, /*_lockdelayunit*/ AUDIO_CS_AS_ISO_DATA_EP_LOCK_DELAY_UNIT_UNDEFINED, /*_lockdelay*/ 0x0000)
 
@@ -514,7 +514,7 @@ TU_ATTR_WEAK bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb
   /* Type I Format Type Descriptor(2.3.1.6 - Audio Formats) */\
   TUD_AUDIO_DESC_TYPE_I_FORMAT(_nBytesPerSample, _nBitsUsedPerSample),\
   /* Standard AS Isochronous Audio Data Endpoint Descriptor(4.10.1.1) */\
-  TUD_AUDIO_DESC_STD_AS_ISO_EP(/*_ep*/ _epin, /*_attr*/ (uint8_t) (TUSB_XFER_ISOCHRONOUS | TUSB_ISO_EP_ATT_ASYNCHRONOUS | TUSB_ISO_EP_ATT_DATA), /*_maxEPsize*/ _epsize, /*_interval*/ TUD_OPT_HIGH_SPEED ? 0x04 : 0x01),\
+  TUD_AUDIO_DESC_STD_AS_ISO_EP(/*_ep*/ _epin, /*_attr*/ (uint8_t) (TUSB_XFER_ISOCHRONOUS | TUSB_ISO_EP_ATT_ASYNCHRONOUS | TUSB_ISO_EP_ATT_DATA), /*_maxEPsize*/ _epsize, /*_interval*/ 0x01),\
   /* Class-Specific AS Isochronous Audio Data Endpoint Descriptor(4.10.1.2) */\
   TUD_AUDIO_DESC_CS_AS_ISO_EP(/*_attr*/ AUDIO_CS_AS_ISO_DATA_EP_ATT_NON_MAX_PACKETS_OK, /*_ctrl*/ AUDIO_CTRL_NONE, /*_lockdelayunit*/ AUDIO_CS_AS_ISO_DATA_EP_LOCK_DELAY_UNIT_UNDEFINED, /*_lockdelay*/ 0x0000)
 
@@ -562,7 +562,7 @@ TU_ATTR_WEAK bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb
   /* Type I Format Type Descriptor(2.3.1.6 - Audio Formats) */\
   TUD_AUDIO_DESC_TYPE_I_FORMAT(_nBytesPerSample, _nBitsUsedPerSample),\
   /* Standard AS Isochronous Audio Data Endpoint Descriptor(4.10.1.1) */\
-  TUD_AUDIO_DESC_STD_AS_ISO_EP(/*_ep*/ _epout, /*_attr*/ (uint8_t) (TUSB_XFER_ISOCHRONOUS | TUSB_ISO_EP_ATT_ASYNCHRONOUS | TUSB_ISO_EP_ATT_DATA), /*_maxEPsize*/ _epsize, /*_interval*/ TUD_OPT_HIGH_SPEED ? 0x04 : 0x01),\
+  TUD_AUDIO_DESC_STD_AS_ISO_EP(/*_ep*/ _epout, /*_attr*/ (uint8_t) (TUSB_XFER_ISOCHRONOUS | TUSB_ISO_EP_ATT_ASYNCHRONOUS | TUSB_ISO_EP_ATT_DATA), /*_maxEPsize*/ _epsize, /*_interval*/ 0x01),\
   /* Class-Specific AS Isochronous Audio Data Endpoint Descriptor(4.10.1.2) */\
   TUD_AUDIO_DESC_CS_AS_ISO_EP(/*_attr*/ AUDIO_CS_AS_ISO_DATA_EP_ATT_NON_MAX_PACKETS_OK, /*_ctrl*/ AUDIO_CTRL_NONE, /*_lockdelayunit*/ AUDIO_CS_AS_ISO_DATA_EP_LOCK_DELAY_UNIT_UNDEFINED, /*_lockdelay*/ 0x0000),\
   /* Standard AS Isochronous Feedback Endpoint Descriptor(4.10.2.1) */\

diff --git a/src/host/usbh.c b/src/host/usbh.c
@@ -86,16 +86,12 @@ typedef struct
   uint8_t rhport;
   uint8_t hub_addr;
   uint8_t hub_port;
-  uint8_t speed;
-
-  // enumeration is in progress, done when all interfaces are configured
-  volatile uint8_t enumerating;
 
-//  struct TU_ATTR_PACKED {
-//    uint8_t          speed       : 4; // packed speed to save footprint
-//    volatile uint8_t enumerating : 1;
-//    uint8_t TU_RESERVED : 3;
-//  };
+  struct TU_ATTR_PACKED {
+             uint8_t speed       : 4; // packed speed to save footprint
+    volatile uint8_t enumerating : 1; // enumeration is in progress, false if not connected or all interfaces are configured
+    uint8_t TU_RESERVED : 3;
+  };
 } usbh_dev0_t;
 
 typedef struct {
@@ -462,8 +458,8 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
     switch (event.event_id)
     {
       case HCD_EVENT_DEVICE_ATTACH:
-        // due to the shared _usbh_ctrl_buf, we must complete enumerating
-        // one device before enumerating another one.
+        // due to the shared _usbh_ctrl_buf, we must complete enumerating one device before enumerating another one.
+        // TODO better to have an separated queue for newly attached devices
         if ( _dev0.enumerating ) {
           TU_LOG_USBH("[%u:] USBH Defer Attach until current enumeration complete\r\n", event.rhport);
 
@@ -587,11 +583,17 @@ bool tuh_control_xfer (tuh_xfer_t* xfer) {
   // EP0 with setup packet
   TU_VERIFY(xfer->ep_addr == 0 && xfer->setup);
 
-  // pre-check to help reducing mutex lock
-  TU_VERIFY(_ctrl_xfer.stage == CONTROL_STAGE_IDLE);
-
+  // Check if device is still connected (enumerating for dev0)
   uint8_t const daddr = xfer->daddr;
+  if ( daddr == 0 ) {
+    if (!_dev0.enumerating) return false;
+  } else {
+    usbh_device_t const* dev = get_device(daddr);
+    if (dev && dev->connected == 0) return false;
+  }
 
+  // pre-check to help reducing mutex lock
+  TU_VERIFY(_ctrl_xfer.stage == CONTROL_STAGE_IDLE);
   (void) osal_mutex_lock(_usbh_mutex, OSAL_TIMEOUT_WAIT_FOREVER);
 
   bool const is_idle = (_ctrl_xfer.stage == CONTROL_STAGE_IDLE);
@@ -691,7 +693,7 @@ static bool usbh_control_xfer_cb (uint8_t dev_addr, uint8_t ep_addr, xfer_result
 
   if (XFER_RESULT_SUCCESS != result) {
     TU_LOG1("[%u:%u] Control %s, xferred_bytes = %lu\r\n", rhport, dev_addr, result == XFER_RESULT_STALLED ? "STALLED" : "FAILED", xferred_bytes);
-    TU_LOG_BUF(1, request, 8);
+    TU_LOG1_BUF(request, 8);
     TU_LOG1("\r\n");
 
     // terminate transfer if any stage failed
@@ -948,19 +950,23 @@ void hcd_devtree_get_info(uint8_t dev_addr, hcd_devtree_info_t* devtree_info)
   }
 }
 
-TU_ATTR_FAST_FUNC void hcd_event_handler(hcd_event_t const* event, bool in_isr)
-{
-  switch (event->event_id)
-  {
-//    case HCD_EVENT_DEVICE_REMOVE:
-//      // FIXME device remove from a hub need an HCD API for hcd to free up endpoint
-//      // mark device as removing to prevent further xfer before the event is processed in usbh task
-//      break;
+TU_ATTR_FAST_FUNC void hcd_event_handler(hcd_event_t const* event, bool in_isr) {
+  switch (event->event_id) {
+    case HCD_EVENT_DEVICE_REMOVE:
+      // FIXME device remove from a hub need an HCD API for hcd to free up endpoint
+      // mark device as removing to prevent further xfer before the event is processed in usbh task
 
-    default:
-      osal_queue_send(_usbh_q, event, in_isr);
-    break;
+      // Check if dev0 is removed
+      if ((event->rhport == _dev0.rhport) && (event->connection.hub_addr == _dev0.hub_addr) &&
+          (event->connection.hub_port == _dev0.hub_port)) {
+        _dev0.enumerating = 0;
+      }
+      break;
+
+    default: break;
   }
+
+  osal_queue_send(_usbh_q, event, in_isr);
 }
 
 //--------------------------------------------------------------------+
@@ -1325,28 +1331,28 @@ static bool _parse_configuration_descriptor (uint8_t dev_addr, tusb_desc_configu
 static void enum_full_complete(void);
 
 // process device enumeration
-static void process_enumeration(tuh_xfer_t* xfer)
-{
+static void process_enumeration(tuh_xfer_t* xfer) {
   // Retry a few times with transfers in enumeration since device can be unstable when starting up
   enum {
     ATTEMPT_COUNT_MAX = 3,
     ATTEMPT_DELAY_MS = 100
   };
   static uint8_t failed_count = 0;
 
-  if (XFER_RESULT_SUCCESS != xfer->result)
-  {
+  if (XFER_RESULT_SUCCESS != xfer->result) {
     // retry if not reaching max attempt
-    if ( failed_count < ATTEMPT_COUNT_MAX )
-    {
+    bool retry = _dev0.enumerating && (failed_count < ATTEMPT_COUNT_MAX);
+    if ( retry ) {
       failed_count++;
       osal_task_delay(ATTEMPT_DELAY_MS); // delay a bit
       TU_LOG1("Enumeration attempt %u\r\n", failed_count);
-      TU_ASSERT(tuh_control_xfer(xfer), );
-    }else
-    {
+      retry = tuh_control_xfer(xfer);
+    }
+
+    if (!retry) {
       enum_full_complete();
     }
+
     return;
   }
   failed_count = 0;

diff --git a/src/portable/raspberrypi/pio_usb/hcd_pio_usb.c b/src/portable/raspberrypi/pio_usb/hcd_pio_usb.c
@@ -182,14 +182,6 @@ void __no_inline_not_in_flash_func(pio_usb_host_irq_handler)(uint8_t root_id) {
   root_port_t *rport = PIO_USB_ROOT_PORT(root_id);
   uint32_t const ints = rport->ints;
 
-  if ( ints & PIO_USB_INTS_CONNECT_BITS ) {
-    hcd_event_device_attach(tu_rhport, true);
-  }
-
-  if ( ints & PIO_USB_INTS_DISCONNECT_BITS ) {
-    hcd_event_device_remove(tu_rhport, true);
-  }
-
   if ( ints & PIO_USB_INTS_ENDPOINT_COMPLETE_BITS ) {
     handle_endpoint_irq(rport, XFER_RESULT_SUCCESS, &rport->ep_complete);
   }
@@ -202,6 +194,14 @@ void __no_inline_not_in_flash_func(pio_usb_host_irq_handler)(uint8_t root_id) {
     handle_endpoint_irq(rport, XFER_RESULT_FAILED, &rport->ep_error);
   }
 
+  if ( ints & PIO_USB_INTS_CONNECT_BITS ) {
+    hcd_event_device_attach(tu_rhport, true);
+  }
+
+  if ( ints & PIO_USB_INTS_DISCONNECT_BITS ) {
+    hcd_event_device_remove(tu_rhport, true);
+  }
+
   // clear all
   rport->ints &= ~ints;
 }

diff --git a/src/tusb_option.h b/src/tusb_option.h
@@ -174,10 +174,10 @@
 // NXP LPC MCX
 #define OPT_MCU_MCXN9            2300  ///< NXP MCX N9 Series
 
-// Helper to check if configured MCU is one of listed
+// Check if configured MCU is one of listed
 // Apply _TU_CHECK_MCU with || as separator to list of input
-#define _TU_CHECK_MCU(_m)   (CFG_TUSB_MCU == _m)
-#define TU_CHECK_MCU(...)   (TU_ARGS_APPLY(_TU_CHECK_MCU, ||, __VA_ARGS__))
+#define _TU_CHECK_MCU(_m)    (CFG_TUSB_MCU == _m)
+#define TU_CHECK_MCU(...)    (TU_ARGS_APPLY(_TU_CHECK_MCU, ||, __VA_ARGS__))
 
 //--------------------------------------------------------------------+
 // Supported OS
@@ -273,6 +273,7 @@
 // highspeed support indicator
 #define TUH_OPT_HIGH_SPEED    (CFG_TUH_MAX_SPEED ? (CFG_TUH_MAX_SPEED & OPT_MODE_HIGH_SPEED) : TUP_RHPORT_HIGHSPEED)
 
+
 //--------------------------------------------------------------------+
 // TODO move later
 //--------------------------------------------------------------------+