From 18d566b94c8f17d9df04914979997d589b905290 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 17 Feb 2025 17:06:51 +0700 Subject: [PATCH] improve hub: retry if hub interrupt endpoint failed bump up pio-usb to latest --- src/host/hub.c | 184 ++++++++++++++++++++++------------------------ src/host/hub.h | 51 ++++++------- src/host/usbh.c | 8 +- tools/get_deps.py | 9 ++- 4 files changed, 120 insertions(+), 132 deletions(-) diff --git a/src/host/hub.c b/src/host/hub.c index e97014443..611bf71be 100644 --- a/src/host/hub.c +++ b/src/host/hub.c @@ -40,29 +40,37 @@ //--------------------------------------------------------------------+ // MACRO CONSTANT TYPEDEF //--------------------------------------------------------------------+ -typedef struct -{ +typedef struct { uint8_t itf_num; uint8_t ep_in; - uint8_t port_count; + + // from hub descriptor + uint8_t bNbrPorts; + // uint8_t bPwrOn2PwrGood; + // uint16_t wHubCharacteristics; CFG_TUH_MEM_ALIGN uint8_t status_change; CFG_TUH_MEM_ALIGN hub_port_status_response_t port_status; CFG_TUH_MEM_ALIGN hub_status_response_t hub_status; } hub_interface_t; -CFG_TUH_MEM_SECTION static hub_interface_t hub_data[CFG_TUH_HUB]; -CFG_TUH_MEM_SECTION CFG_TUH_MEM_ALIGN static uint8_t _hub_buffer[sizeof(descriptor_hub_desc_t)]; +typedef struct { + TUH_EPBUF_DEF(buf, CFG_TUH_HUB_BUFSIZE); +} hub_epbuf_t; -TU_ATTR_ALWAYS_INLINE -static inline hub_interface_t* get_itf(uint8_t dev_addr) -{ - return &hub_data[dev_addr-1-CFG_TUH_DEVICE_MAX]; +CFG_TUH_MEM_SECTION static hub_interface_t hub_itfs[CFG_TUH_HUB]; +CFG_TUH_MEM_SECTION static hub_epbuf_t hub_epbufs[CFG_TUH_HUB]; + +TU_ATTR_ALWAYS_INLINE static inline hub_interface_t* get_hub_itf(uint8_t daddr) { + return &hub_itfs[daddr-1-CFG_TUH_DEVICE_MAX]; } -#if CFG_TUSB_DEBUG >= 2 -static char const* const _hub_feature_str[] = -{ +TU_ATTR_ALWAYS_INLINE static inline uint8_t* get_hub_epbuf(uint8_t daddr) { + return hub_epbufs[daddr-1-CFG_TUH_DEVICE_MAX].buf; +} + +#if CFG_TUSB_DEBUG >= HUB_DEBUG +static char const* const _hub_feature_str[] = { [HUB_FEATURE_PORT_CONNECTION ] = "PORT_CONNECTION", [HUB_FEATURE_PORT_ENABLE ] = "PORT_ENABLE", [HUB_FEATURE_PORT_SUSPEND ] = "PORT_SUSPEND", @@ -84,12 +92,9 @@ static char const* const _hub_feature_str[] = // HUB //--------------------------------------------------------------------+ bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, - tuh_xfer_cb_t complete_cb, uintptr_t user_data) -{ - tusb_control_request_t const request = - { - .bmRequestType_bit = - { + tuh_xfer_cb_t complete_cb, uintptr_t user_data) { + tusb_control_request_t const request = { + .bmRequestType_bit = { .recipient = (hub_port == 0) ? TUSB_REQ_RCPT_DEVICE : TUSB_REQ_RCPT_OTHER, .type = TUSB_REQ_TYPE_CLASS, .direction = TUSB_DIR_OUT @@ -100,8 +105,7 @@ bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, .wLength = 0 }; - tuh_xfer_t xfer = - { + tuh_xfer_t xfer = { .daddr = hub_addr, .ep_addr = 0, .setup = &request, @@ -110,7 +114,7 @@ bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, .user_data = user_data }; - TU_LOG2("HUB Clear Feature: %s, addr = %u port = %u\r\n", _hub_feature_str[feature], hub_addr, hub_port); + TU_LOG_DRV("HUB Clear Feature: %s, addr = %u port = %u\r\n", _hub_feature_str[feature], hub_addr, hub_port); TU_ASSERT( tuh_control_xfer(&xfer) ); return true; } @@ -142,7 +146,7 @@ bool hub_port_set_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, .user_data = user_data }; - TU_LOG2("HUB Set Feature: %s, addr = %u port = %u\r\n", _hub_feature_str[feature], hub_addr, hub_port); + TU_LOG_DRV("HUB Set Feature: %s, addr = %u port = %u\r\n", _hub_feature_str[feature], hub_addr, hub_port); TU_ASSERT( tuh_control_xfer(&xfer) ); return true; } @@ -174,7 +178,7 @@ bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void* resp, .user_data = user_data }; - TU_LOG2("HUB Get Port Status: addr = %u port = %u\r\n", hub_addr, hub_port); + TU_LOG_DRV("HUB Get Port Status: addr = %u port = %u\r\n", hub_addr, hub_port); TU_VERIFY( tuh_control_xfer(&xfer) ); return true; } @@ -183,7 +187,7 @@ bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void* resp, // CLASS-USBH API (don't require to verify parameters) //--------------------------------------------------------------------+ bool hub_init(void) { - tu_memclr(hub_data, sizeof(hub_data)); + tu_memclr(hub_itfs, sizeof(hub_itfs)); return true; } @@ -191,40 +195,32 @@ bool hub_deinit(void) { return true; } -bool hub_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *itf_desc, uint16_t max_len) -{ +bool hub_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *itf_desc, uint16_t max_len) { (void) rhport; TU_VERIFY(TUSB_CLASS_HUB == itf_desc->bInterfaceClass && 0 == itf_desc->bInterfaceSubClass); + TU_VERIFY(itf_desc->bInterfaceProtocol <= 1); // not support multiple TT yet - // hub driver does not support multiple TT yet - TU_VERIFY(itf_desc->bInterfaceProtocol <= 1); - - // msc driver length is fixed uint16_t const drv_len = sizeof(tusb_desc_interface_t) + sizeof(tusb_desc_endpoint_t); TU_ASSERT(drv_len <= max_len); - //------------- Interrupt Status endpoint -------------// + // Interrupt Status endpoint tusb_desc_endpoint_t const *desc_ep = (tusb_desc_endpoint_t const *) tu_desc_next(itf_desc); - TU_ASSERT(TUSB_DESC_ENDPOINT == desc_ep->bDescriptorType && TUSB_XFER_INTERRUPT == desc_ep->bmAttributes.xfer, 0); - TU_ASSERT(tuh_edpt_open(dev_addr, desc_ep)); - hub_interface_t* p_hub = get_itf(dev_addr); - + hub_interface_t* p_hub = get_hub_itf(dev_addr); p_hub->itf_num = itf_desc->bInterfaceNumber; p_hub->ep_in = desc_ep->bEndpointAddress; return true; } -void hub_close(uint8_t dev_addr) -{ +void hub_close(uint8_t dev_addr) { TU_VERIFY(dev_addr > CFG_TUH_DEVICE_MAX, ); - hub_interface_t* p_hub = get_itf(dev_addr); + hub_interface_t* p_hub = get_hub_itf(dev_addr); if (p_hub->ep_in) { TU_LOG_DRV(" HUB close addr = %d\r\n", dev_addr); @@ -232,30 +228,24 @@ void hub_close(uint8_t dev_addr) } } -bool hub_edpt_status_xfer(uint8_t dev_addr) -{ - hub_interface_t* hub_itf = get_itf(dev_addr); +bool hub_edpt_status_xfer(uint8_t dev_addr) { + hub_interface_t* hub_itf = get_hub_itf(dev_addr); return usbh_edpt_xfer(dev_addr, hub_itf->ep_in, &hub_itf->status_change, 1); } - //--------------------------------------------------------------------+ // Set Configure //--------------------------------------------------------------------+ - static void config_set_port_power (tuh_xfer_t* xfer); static void config_port_power_complete (tuh_xfer_t* xfer); -bool hub_set_config(uint8_t dev_addr, uint8_t itf_num) -{ - hub_interface_t* p_hub = get_itf(dev_addr); +bool hub_set_config(uint8_t dev_addr, uint8_t itf_num) { + hub_interface_t* p_hub = get_hub_itf(dev_addr); TU_ASSERT(itf_num == p_hub->itf_num); // Get Hub Descriptor - tusb_control_request_t const request = - { - .bmRequestType_bit = - { + tusb_control_request_t const request = { + .bmRequestType_bit = { .recipient = TUSB_REQ_RCPT_DEVICE, .type = TUSB_REQ_TYPE_CLASS, .direction = TUSB_DIR_IN @@ -263,34 +253,31 @@ bool hub_set_config(uint8_t dev_addr, uint8_t itf_num) .bRequest = HUB_REQUEST_GET_DESCRIPTOR, .wValue = 0, .wIndex = 0, - .wLength = sizeof(descriptor_hub_desc_t) + .wLength = sizeof(hub_desc_cs_t) }; - tuh_xfer_t xfer = - { + tuh_xfer_t xfer = { .daddr = dev_addr, .ep_addr = 0, .setup = &request, - .buffer = _hub_buffer, + .buffer = get_hub_epbuf(dev_addr), .complete_cb = config_set_port_power, .user_data = 0 }; - TU_ASSERT( tuh_control_xfer(&xfer) ); - + TU_ASSERT(tuh_control_xfer(&xfer)); return true; } -static void config_set_port_power (tuh_xfer_t* xfer) -{ +static void config_set_port_power (tuh_xfer_t* xfer) { TU_ASSERT(XFER_RESULT_SUCCESS == xfer->result, ); uint8_t const daddr = xfer->daddr; - hub_interface_t* p_hub = get_itf(daddr); + hub_interface_t* p_hub = get_hub_itf(daddr); // only use number of ports in hub descriptor - descriptor_hub_desc_t const* desc_hub = (descriptor_hub_desc_t const*) _hub_buffer; - p_hub->port_count = desc_hub->bNbrPorts; + hub_desc_cs_t const* desc_hub = (hub_desc_cs_t const*) get_hub_epbuf(daddr); + p_hub->bNbrPorts = desc_hub->bNbrPorts; // May need to GET_STATUS @@ -299,14 +286,13 @@ static void config_set_port_power (tuh_xfer_t* xfer) hub_port_set_feature(daddr, hub_port, HUB_FEATURE_PORT_POWER, config_port_power_complete, 0); } -static void config_port_power_complete (tuh_xfer_t* xfer) -{ +static void config_port_power_complete (tuh_xfer_t* xfer) { TU_ASSERT(XFER_RESULT_SUCCESS == xfer->result, ); uint8_t const daddr = xfer->daddr; - hub_interface_t* p_hub = get_itf(daddr); + hub_interface_t* p_hub = get_hub_itf(daddr); - if (xfer->setup->wIndex == p_hub->port_count) + if (xfer->setup->wIndex == p_hub->bNbrPorts) { // All ports are power -> queue notification status endpoint and // complete the SET CONFIGURATION @@ -334,46 +320,48 @@ static void connection_port_reset_complete (tuh_xfer_t* xfer); bool hub_xfer_cb(uint8_t dev_addr, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) { (void) xferred_bytes; // TODO can be more than 1 for hub with lots of ports (void) ep_addr; - TU_VERIFY(result == XFER_RESULT_SUCCESS); - hub_interface_t* p_hub = get_itf(dev_addr); + if (result == XFER_RESULT_SUCCESS) { + hub_interface_t* p_hub = get_hub_itf(dev_addr); - uint8_t const status_change = p_hub->status_change; - TU_LOG2(" Hub Status Change = 0x%02X\r\n", status_change); + uint8_t const status_change = p_hub->status_change; + TU_LOG_DRV(" Hub Status Change = 0x%02X\r\n", status_change); - if ( status_change == 0 ) { - // The status change event was neither for the hub, nor for any of its ports. - // This shouldn't happen, but it does with some devices. - // Initiate the next interrupt poll here. - return hub_edpt_status_xfer(dev_addr); - } - - if (tu_bit_test(status_change, 0)) { - // Hub bit 0 is for the hub device events - if (hub_port_get_status(dev_addr, 0, &p_hub->hub_status, hub_get_status_complete, 0) == false) { - //Hub status control transfer failed, retry - hub_edpt_status_xfer(dev_addr); + if ( status_change == 0 ) { + // The status change event was neither for the hub, nor for any of its ports. + // This shouldn't happen, but it does with some devices. Initiate the next interrupt poll here. + return hub_edpt_status_xfer(dev_addr); } - } - else { - // Hub bits 1 to n are hub port events - for (uint8_t port=1; port <= p_hub->port_count; port++) { - if ( tu_bit_test(status_change, port) ) { - if (hub_port_get_status(dev_addr, port, &p_hub->port_status, hub_port_get_status_complete, 0) == false) { - //Hub status control transfer failed, retry - hub_edpt_status_xfer(dev_addr); - } - break; + + if (tu_bit_test(status_change, 0)) { + // Hub bit 0 is for the hub device events + if (hub_port_get_status(dev_addr, 0, &p_hub->hub_status, hub_get_status_complete, 0) == false) { + //Hub status control transfer failed, retry + hub_edpt_status_xfer(dev_addr); } } + else { + // Hub bits 1 to n are hub port events + for (uint8_t port=1; port <= p_hub->bNbrPorts; port++) { + if ( tu_bit_test(status_change, port) ) { + if (hub_port_get_status(dev_addr, port, &p_hub->port_status, hub_port_get_status_complete, 0) == false) { + //Hub status control transfer failed, retry + hub_edpt_status_xfer(dev_addr); + } + break; + } + } + } + } else { + TU_LOG_DRV(" Hub Status Change: failed, retry\r\n"); + hub_edpt_status_xfer(dev_addr); // retry } // NOTE: next status transfer is queued by usbh.c after handling this request return true; } -static void hub_clear_feature_complete_stub(tuh_xfer_t* xfer) -{ +static void hub_clear_feature_complete_stub(tuh_xfer_t* xfer) { TU_ASSERT(xfer->result == XFER_RESULT_SUCCESS, ); hub_edpt_status_xfer(xfer->daddr); } @@ -383,15 +371,15 @@ static void hub_get_status_complete (tuh_xfer_t* xfer) TU_ASSERT(xfer->result == XFER_RESULT_SUCCESS, ); uint8_t const daddr = xfer->daddr; - hub_interface_t* p_hub = get_itf(daddr); + hub_interface_t* p_hub = get_hub_itf(daddr); uint8_t const port_num = (uint8_t) tu_le16toh(xfer->setup->wIndex); TU_ASSERT(port_num == 0 , ); - TU_LOG2("HUB Got hub status, addr = %u, status = %04x\r\n", daddr, p_hub->hub_status.change.value); + TU_LOG_DRV("HUB Got hub status, addr = %u, status = %04x\r\n", daddr, p_hub->hub_status.change.value); if (p_hub->hub_status.change.local_power_source) { - TU_LOG2("HUB Local Power Change, addr = %u\r\n", daddr); + TU_LOG_DRV("HUB Local Power Change, addr = %u\r\n", daddr); hub_port_clear_feature(daddr, port_num, HUB_FEATURE_HUB_LOCAL_POWER_CHANGE, hub_clear_feature_complete_stub, 0); } else if (p_hub->hub_status.change.over_current) @@ -406,7 +394,7 @@ static void hub_port_get_status_complete (tuh_xfer_t* xfer) TU_ASSERT(xfer->result == XFER_RESULT_SUCCESS, ); uint8_t const daddr = xfer->daddr; - hub_interface_t* p_hub = get_itf(daddr); + hub_interface_t* p_hub = get_hub_itf(daddr); uint8_t const port_num = (uint8_t) tu_le16toh(xfer->setup->wIndex); // Connection change @@ -453,7 +441,7 @@ static void connection_clear_conn_change_complete (tuh_xfer_t* xfer) TU_ASSERT(xfer->result == XFER_RESULT_SUCCESS, ); uint8_t const daddr = xfer->daddr; - hub_interface_t* p_hub = get_itf(daddr); + hub_interface_t* p_hub = get_hub_itf(daddr); uint8_t const port_num = (uint8_t) tu_le16toh(xfer->setup->wIndex); if ( p_hub->port_status.status.connection ) diff --git a/src/host/hub.h b/src/host/hub.h index 385efe6b2..59097fb0a 100644 --- a/src/host/hub.h +++ b/src/host/hub.h @@ -24,17 +24,8 @@ * This file is part of the TinyUSB stack. */ -/** \ingroup group_class - * \defgroup ClassDriver_Hub Hub (Host only) - * \details Like most PC's OS, Hub support is completely hidden from Application. In fact, application cannot determine whether - * a device is mounted directly via roothub or via a hub's port. All Hub-related procedures are performed and managed - * by tinyusb stack. Unless you are trying to develop the stack itself, there are nothing else can be used by Application. - * \note Due to my laziness, only 1-level of Hub is supported. In other way, the stack cannot mount a hub via another hub. - * @{ - */ - -#ifndef _TUSB_HUB_H_ -#define _TUSB_HUB_H_ +#ifndef TUSB_HUB_H_ +#define TUSB_HUB_H_ #include "common/tusb_common.h" @@ -42,6 +33,14 @@ extern "C" { #endif +//--------------------------------------------------------------------+ +// Configuration +//--------------------------------------------------------------------+ + +#ifndef CFG_TUH_HUB_BUFSIZE + #define CFG_TUH_HUB_BUFSIZE 12 +#endif + //D1...D0: Logical Power Switching Mode //00: Ganged power switching (all ports’power at //once) @@ -89,16 +88,17 @@ typedef struct TU_ATTR_PACKED{ uint8_t bHubContrCurrent; uint8_t DeviceRemovable; // bitmap each bit for a port (from bit1) uint8_t PortPwrCtrlMask; // just for compatibility, should be 0xff -} descriptor_hub_desc_t; +} hub_desc_cs_t; -TU_VERIFY_STATIC( sizeof(descriptor_hub_desc_t) == 9, "size is not correct"); +TU_VERIFY_STATIC(sizeof(hub_desc_cs_t) == 9, "size is not correct"); +TU_VERIFY_STATIC(CFG_TUH_HUB_BUFSIZE >= sizeof(hub_desc_cs_t), "buffer is not big enough"); enum { HUB_REQUEST_GET_STATUS = 0 , HUB_REQUEST_CLEAR_FEATURE = 1 , - + // 2 is reserved HUB_REQUEST_SET_FEATURE = 3 , - + // 4-5 are reserved HUB_REQUEST_GET_DESCRIPTOR = 6 , HUB_REQUEST_SET_DESCRIPTOR = 7 , HUB_REQUEST_CLEAR_TT_BUFFER = 8 , @@ -118,10 +118,10 @@ enum{ HUB_FEATURE_PORT_SUSPEND = 2, HUB_FEATURE_PORT_OVER_CURRENT = 3, HUB_FEATURE_PORT_RESET = 4, - + // 5-7 are reserved HUB_FEATURE_PORT_POWER = 8, HUB_FEATURE_PORT_LOW_SPEED = 9, - + // 10-15 are reserved HUB_FEATURE_PORT_CONNECTION_CHANGE = 16, HUB_FEATURE_PORT_ENABLE_CHANGE = 17, HUB_FEATURE_PORT_SUSPEND_CHANGE = 18, @@ -172,16 +172,16 @@ typedef struct { TU_VERIFY_STATIC( sizeof(hub_port_status_response_t) == 4, "size is not correct"); // Clear feature -bool hub_port_clear_feature (uint8_t hub_addr, uint8_t hub_port, uint8_t feature, - tuh_xfer_cb_t complete_cb, uintptr_t user_data); +bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, + tuh_xfer_cb_t complete_cb, uintptr_t user_data); // Set feature -bool hub_port_set_feature (uint8_t hub_addr, uint8_t hub_port, uint8_t feature, - tuh_xfer_cb_t complete_cb, uintptr_t user_data); +bool hub_port_set_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, + tuh_xfer_cb_t complete_cb, uintptr_t user_data); // Get port status -bool hub_port_get_status (uint8_t hub_addr, uint8_t hub_port, void* resp, - tuh_xfer_cb_t complete_cb, uintptr_t user_data); +bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void *resp, + tuh_xfer_cb_t complete_cb, uintptr_t user_data); // Get status from Interrupt endpoint bool hub_edpt_status_xfer(uint8_t dev_addr); @@ -198,7 +198,6 @@ bool hub_port_clear_reset_change(uint8_t hub_addr, uint8_t hub_port, tuh_xfer_cb return hub_port_clear_feature(hub_addr, hub_port, HUB_FEATURE_PORT_RESET_CHANGE, complete_cb, user_data); } - //--------------------------------------------------------------------+ // Internal Class Driver API //--------------------------------------------------------------------+ @@ -213,6 +212,4 @@ void hub_close (uint8_t dev_addr); } #endif -#endif /* _TUSB_HUB_H_ */ - -/** @} */ +#endif diff --git a/src/host/usbh.c b/src/host/usbh.c index a2994cde7..5f4f17167 100644 --- a/src/host/usbh.c +++ b/src/host/usbh.c @@ -467,13 +467,11 @@ bool tuh_task_event_ready(void) { * This should be called periodically within the mainloop or rtos thread. * @code - int main(void) - { + int main(void) { application_init(); tusb_init(0, TUSB_ROLE_HOST); - while(1) // the mainloop - { + while(1) { // the mainloop application_code(); tuh_task(); // tinyusb host task } @@ -1657,7 +1655,7 @@ static bool _parse_configuration_descriptor(uint8_t dev_addr, tusb_desc_configur if ( 0 == tu_desc_len(p_desc) ) { // A zero length descriptor indicates that the device is off spec (e.g. wrong wTotalLength). // Parsed interfaces should still be usable - TU_LOG_USBH("Encountered a zero-length descriptor after %u bytes\r\n", (uint32_t)p_desc - (uint32_t)desc_cfg); + TU_LOG_USBH("Encountered a zero-length descriptor after %" PRIu32 "bytes\r\n", (uint32_t)p_desc - (uint32_t)desc_cfg); break; } diff --git a/tools/get_deps.py b/tools/get_deps.py index c8459c1f1..3665904b3 100755 --- a/tools/get_deps.py +++ b/tools/get_deps.py @@ -59,7 +59,7 @@ deps_optional = { '144f1eb7ea8c06512e12f12b27383601c0272410', 'kinetis_k kinetis_k32l2 kinetis_kl lpc51 lpc54 lpc55 mcx imxrt'], 'hw/mcu/raspberry_pi/Pico-PIO-USB': ['https://github.com/sekigon-gonnoc/Pico-PIO-USB.git', - 'fe9133fc513b82cc3dc62c67cb51f2339cf29ef7', + '442af432568f25fb72c5784d9d67b8e85465cb28', 'rp2040'], 'hw/mcu/renesas/fsp': ['https://github.com/renesas/fsp.git', 'edcc97d684b6f716728a60d7a6fea049d9870bd6', @@ -217,7 +217,12 @@ TOP = Path(__file__).parent.parent.resolve() def run_cmd(cmd): - return subprocess.run(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + r = subprocess.run(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + title = f'Command Error: {cmd}' + if r.returncode != 0: + print(title) + print(r.stdout.decode("utf-8")) + return r def get_a_dep(d):