From 6a87063a3bc18c5e14375ce512f4b0c126101209 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 3 Nov 2023 22:39:18 +0700 Subject: [PATCH] update tinyusb to fix race condition https://github.com/hathach/tinyusb/pull/2310 --- src/arduino/Adafruit_TinyUSB_API.cpp | 18 ++--- src/arduino/ports/rp2040/tusb_config_rp2040.h | 1 + src/common/tusb_mcu.h | 13 +++- src/device/usbd.h | 6 +- src/host/usbh.c | 76 ++++++++++--------- .../raspberrypi/pio_usb/hcd_pio_usb.c | 16 ++-- src/tusb_option.h | 7 +- 7 files changed, 75 insertions(+), 62 deletions(-) diff --git a/src/arduino/Adafruit_TinyUSB_API.cpp b/src/arduino/Adafruit_TinyUSB_API.cpp index b0afda8a..a67a66b3 100644 --- a/src/arduino/Adafruit_TinyUSB_API.cpp +++ b/src/arduino/Adafruit_TinyUSB_API.cpp @@ -57,7 +57,9 @@ 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" @@ -65,14 +67,6 @@ void TinyUSB_Device_FlushCDC(void) { __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 index e94d5db3..c3c68e76 100644 --- 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 index f802723a..d90bf945 100644 --- 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 index 0e2f7b97..bd990a6f 100644 --- 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 index abed89c1..43c95161 100644 --- 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,8 +1331,7 @@ 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, @@ -1334,19 +1339,20 @@ static void process_enumeration(tuh_xfer_t* xfer) }; 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 index 88d2e244..6f8240a3 100644 --- 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 index 5e648b08..bfbabe52 100644 --- 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 //--------------------------------------------------------------------+