From 59aa1cbaf82f7063d20d93954cbc225bbe9850f6 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Mon, 2 Jun 2025 17:39:46 +0800 Subject: [PATCH 1/2] refactor(ext_hub): Device release (allows to run usb_host test with ext hub) 754d357f refactor(ext_hub): Fixed device release, optimized the order of closing usbh device fc61875a refactor(ext_hub): Pospone the device release, if device is not IDLE 3fd17b8b refactor(hub): Applied new ext_hub api, refactor func names 3003362b refactor(usb_host): Cancel hub porpagation to the user, rename non-critical func 19ce9ed6 refactor(test_usb_host_async): Added host_lib_task finish notification f238d75b refactor(ext_port): Remove the error verification, as error will be handled in ext hub driver Co-authored-by: Roman Leonov --- components/usb/ext_hub.c | 196 +++++++----------- components/usb/ext_port.c | 2 +- components/usb/hub.c | 16 +- components/usb/private_include/ext_hub.h | 5 +- components/usb/private_include/hub.h | 4 +- .../usb_host/main/test_usb_host_async.c | 14 +- components/usb/usb_host.c | 22 +- 7 files changed, 113 insertions(+), 146 deletions(-) diff --git a/components/usb/ext_hub.c b/components/usb/ext_hub.c index 6dcf7d95fd..55d4205b84 100644 --- a/components/usb/ext_hub.c +++ b/components/usb/ext_hub.c @@ -313,13 +313,32 @@ static bool _device_set_actions(ext_hub_dev_t *ext_hub_dev, uint32_t action_flag static esp_err_t device_enable_int_ep(ext_hub_dev_t *ext_hub_dev) { - ESP_LOGD(EXT_HUB_TAG, "[%d] Enable EP IN", ext_hub_dev->constant.dev_addr); - esp_err_t ret = usbh_ep_enqueue_urb(ext_hub_dev->constant.ep_in_hdl, ext_hub_dev->constant.in_urb); - if (ret != ESP_OK) { - ESP_LOGE(EXT_HUB_TAG, "[%d] Failed to submit in urb: %s", ext_hub_dev->constant.dev_addr, esp_err_to_name(ret)); - device_error(ext_hub_dev); + bool call_proc_req_cb = false; + bool is_active = true; + ESP_LOGD(EXT_HUB_TAG, "[%d] Device ports handle complete", ext_hub_dev->constant.dev_addr); + + // Check if the device should be released + EXT_HUB_ENTER_CRITICAL(); + if (ext_hub_dev->dynamic.flags.waiting_release) { + is_active = false; + call_proc_req_cb = _device_set_actions(ext_hub_dev, DEV_ACTION_RELEASE); } - return ret; + EXT_HUB_EXIT_CRITICAL(); + + if (is_active) { + ESP_LOGD(EXT_HUB_TAG, "[%d] Enable IN EP", ext_hub_dev->constant.dev_addr); + esp_err_t ret = usbh_ep_enqueue_urb(ext_hub_dev->constant.ep_in_hdl, ext_hub_dev->constant.in_urb); + if (ret != ESP_OK) { + ESP_LOGE(EXT_HUB_TAG, "[%d] Failed to submit in urb: %s", ext_hub_dev->constant.dev_addr, esp_err_to_name(ret)); + device_error(ext_hub_dev); + } + return ret; + } + + if (call_proc_req_cb) { + p_ext_hub_driver->constant.proc_req_cb(false, p_ext_hub_driver->constant.proc_req_cb_arg); + } + return ESP_OK; } static void device_has_changed(ext_hub_dev_t *ext_hub_dev) @@ -421,7 +440,6 @@ static esp_err_t device_port_free(ext_hub_dev_t *ext_hub_dev, uint8_t port_idx) EXT_HUB_CHECK(ext_hub_dev->constant.ports[port_idx] != NULL, ESP_ERR_INVALID_STATE); bool call_proc_req_cb = false; - bool all_ports_freed = false; assert(ext_hub_dev->single_thread.maxchild != 0); assert(p_ext_hub_driver->constant.port_driver); @@ -436,25 +454,11 @@ static esp_err_t device_port_free(ext_hub_dev_t *ext_hub_dev, uint8_t port_idx) ext_hub_dev->single_thread.maxchild--; EXT_HUB_ENTER_CRITICAL(); - if (ext_hub_dev->dynamic.flags.is_gone) { - all_ports_freed = (ext_hub_dev->single_thread.maxchild == 0); - - if (all_ports_freed) { - ext_hub_dev->dynamic.flags.waiting_children = 0; - ext_hub_dev->dynamic.flags.waiting_free = 1; - call_proc_req_cb = _device_set_actions(ext_hub_dev, DEV_ACTION_FREE); - } else { - ext_hub_dev->dynamic.flags.waiting_children = 1; - } + if (ext_hub_dev->dynamic.flags.waiting_free && ext_hub_dev->single_thread.maxchild == 0) { + call_proc_req_cb = _device_set_actions(ext_hub_dev, DEV_ACTION_FREE); } EXT_HUB_EXIT_CRITICAL(); - // Close the device if all children were freed - if (all_ports_freed) { - ESP_LOGD(EXT_HUB_TAG, "[%d] Release USBH device object", ext_hub_dev->constant.dev_addr); - ESP_ERROR_CHECK(usbh_dev_close(ext_hub_dev->constant.dev_hdl)); - } - if (call_proc_req_cb) { p_ext_hub_driver->constant.proc_req_cb(false, p_ext_hub_driver->constant.proc_req_cb_arg); } @@ -464,71 +468,60 @@ exit: return ret; } -static esp_err_t device_release_ports(ext_hub_dev_t *ext_hub_dev) +static esp_err_t device_release_port(ext_hub_dev_t *ext_hub_dev, uint8_t port_idx) { - esp_err_t ret = ESP_OK; - // Mark all ports as gone - for (uint8_t i = 0; i < ext_hub_dev->constant.hub_desc->bNbrPorts; i++) { - // Only for ports, that were created - if (ext_hub_dev->constant.ports[i] != NULL) { - // Mark port as gone - ret = p_ext_hub_driver->constant.port_driver->gone(ext_hub_dev->constant.ports[i]); - if (ret == ESP_OK) { - // Port doesn't have a device and can be recycled right now - ESP_ERROR_CHECK(device_port_free(ext_hub_dev, i)); - } else if (ret == ESP_ERR_NOT_FINISHED) { - // Port has a device and will be recycled after USBH device will be released by all clients and freed - ESP_LOGE(EXT_HUB_TAG, "[%d:%d] Port is gone", ext_hub_dev->constant.dev_addr, i + 1); - // Not an error case, but it's good to notify this with the error message - // TODO: IDF-12173 remove the error, instead set up the flag and verify the flag while recycle - ret = ESP_OK; - } else { - ESP_LOGE(EXT_HUB_TAG, "[%d:%d] Unable to mark port as gone: %s", - ext_hub_dev->constant.dev_addr, i + 1, esp_err_to_name(ret)); - return ret; - } - } else { - ESP_LOGE(EXT_HUB_TAG, "[%d:%d] Port was not created", ext_hub_dev->constant.dev_addr, i + 1); - } + assert(ext_hub_dev->constant.ports[port_idx] != NULL); + assert(ext_hub_dev->single_thread.maxchild != 0); + assert(p_ext_hub_driver->constant.port_driver); + + // Mark port as gone + esp_err_t ret = p_ext_hub_driver->constant.port_driver->gone(ext_hub_dev->constant.ports[port_idx]); + if (ret == ESP_OK) { + // Port doesn't have a device and can be freed right now + ESP_ERROR_CHECK(device_port_free(ext_hub_dev, port_idx)); + } else if (ret == ESP_ERR_NOT_FINISHED) { + // Port has a device and will be freed on recycle + EXT_HUB_ENTER_CRITICAL(); + ext_hub_dev->dynamic.flags.waiting_children = 1; + EXT_HUB_EXIT_CRITICAL(); + ret = ESP_OK; + } else { + ESP_LOGE(EXT_HUB_TAG, "[%d:%d] Unable to mark port as gone: %s", + ext_hub_dev->constant.dev_addr, port_idx + 1, esp_err_to_name(ret)); + return ret; } + return ret; } +static void device_release_ports(ext_hub_dev_t *ext_hub_dev) +{ + assert(ext_hub_dev->constant.hub_desc->bNbrPorts); // Sanity check + for (uint8_t i = 0; i < ext_hub_dev->constant.hub_desc->bNbrPorts; i++) { + ESP_ERROR_CHECK(device_release_port(ext_hub_dev, i)); + } +} + static void device_release(ext_hub_dev_t *ext_hub_dev) { ESP_LOGD(EXT_HUB_TAG, "[%d] Device release", ext_hub_dev->constant.dev_addr); EXT_HUB_ENTER_CRITICAL(); - assert(ext_hub_dev->dynamic.flags.waiting_release); // Device should waiting the release - ext_hub_dev->dynamic.flags.is_gone = 1; + assert(ext_hub_dev->dynamic.flags.waiting_release); // Sanity check ext_hub_dev->dynamic.flags.waiting_release = 0; + ext_hub_dev->dynamic.flags.waiting_free = 1; EXT_HUB_EXIT_CRITICAL(); - // Release IN EP - ESP_ERROR_CHECK(usbh_ep_command(ext_hub_dev->constant.ep_in_hdl, USBH_EP_CMD_HALT)); - switch (ext_hub_dev->single_thread.state) { - case EXT_HUB_STATE_ATTACHED: - // Device has no configured ports, release the USBH device object - ESP_LOGD(EXT_HUB_TAG, "[%d] Release USBH device object", ext_hub_dev->constant.dev_addr); - ESP_ERROR_CHECK(usbh_dev_close(ext_hub_dev->constant.dev_hdl)); - break; case EXT_HUB_STATE_CONFIGURED: case EXT_HUB_STATE_SUSPENDED: assert(ext_hub_dev->constant.hub_desc != NULL); // Device should have a Hub descriptor assert(p_ext_hub_driver->constant.port_driver); // Port driver should be available - - // Release ports if device has them - if (ext_hub_dev->constant.hub_desc->bNbrPorts) { - ESP_ERROR_CHECK(device_release_ports(ext_hub_dev)); - } + device_release_ports(ext_hub_dev); break; default: - // Should never occur - abort(); break; } - ext_hub_dev->single_thread.state = EXT_HUB_STATE_RELEASED; } @@ -772,6 +765,9 @@ static void device_free(ext_hub_dev_t *ext_hub_dev) TAILQ_REMOVE(&p_ext_hub_driver->dynamic.ext_hubs_tailq, ext_hub_dev, dynamic.tailq_entry); EXT_HUB_EXIT_CRITICAL(); + ESP_ERROR_CHECK(usbh_ep_free(ext_hub_dev->constant.ep_in_hdl)); + ESP_ERROR_CHECK(usbh_dev_close(ext_hub_dev->constant.dev_hdl)); + if (ext_hub_dev->constant.hub_desc) { heap_caps_free(ext_hub_dev->constant.hub_desc); } @@ -779,10 +775,8 @@ static void device_free(ext_hub_dev_t *ext_hub_dev) heap_caps_free(ext_hub_dev->constant.ports); } - ESP_ERROR_CHECK(usbh_ep_free(ext_hub_dev->constant.ep_in_hdl)); urb_free(ext_hub_dev->constant.ctrl_urb); urb_free(ext_hub_dev->constant.in_urb); - heap_caps_free(ext_hub_dev); } @@ -1388,6 +1382,7 @@ esp_err_t ext_hub_dev_gone(uint8_t dev_addr) ESP_LOGD(EXT_HUB_TAG, "[%d] Device gone", ext_hub_dev->constant.dev_addr); EXT_HUB_ENTER_CRITICAL(); + ext_hub_dev->dynamic.flags.is_gone = 1; if (ext_hub_dev->dynamic.flags.waiting_release || ext_hub_dev->dynamic.flags.waiting_children) { EXT_HUB_EXIT_CRITICAL(); @@ -1397,7 +1392,7 @@ esp_err_t ext_hub_dev_gone(uint8_t dev_addr) } if ((ext_hub_dev->single_thread.maxchild == 0) && !ext_hub_dev->dynamic.flags.in_pending_list) { - // Not in pending list and doesn't have any children + // Not in pending list and doesn't have any children release and free right away ext_hub_dev->dynamic.flags.waiting_free = 1; ext_hub_dev->dynamic.flags.waiting_release = 1; call_proc_req_cb = _device_set_actions(ext_hub_dev, DEV_ACTION_RELEASE | @@ -1421,10 +1416,9 @@ exit: return ret; } -esp_err_t ext_hub_all_free(void) +void ext_hub_mark_all_free(void) { bool call_proc_req_cb = false; - bool wait_for_free = false; EXT_HUB_ENTER_CRITICAL(); for (int i = 0; i < 2; i++) { @@ -1439,9 +1433,13 @@ esp_err_t ext_hub_all_free(void) while (hub_curr != NULL) { hub_next = TAILQ_NEXT(hub_curr, dynamic.tailq_entry); hub_curr->dynamic.flags.waiting_release = 1; - call_proc_req_cb = _device_set_actions(hub_curr, DEV_ACTION_RELEASE); - // At least one hub should be released - wait_for_free = true; + uint32_t action_flags = DEV_ACTION_EP1_FLUSH | DEV_ACTION_EP1_DEQUEUE; + // If device in the IDLE stage, add the release action + // otherwise, device will be released when the stage changed to IDLE + if (hub_curr->single_thread.stage == EXT_HUB_STAGE_IDLE) { + action_flags |= DEV_ACTION_RELEASE; + } + call_proc_req_cb = _device_set_actions(hub_curr, action_flags); hub_curr = hub_next; } } @@ -1450,8 +1448,6 @@ esp_err_t ext_hub_all_free(void) if (call_proc_req_cb) { p_ext_hub_driver->constant.proc_req_cb(false, p_ext_hub_driver->constant.proc_req_cb_arg); } - - return (wait_for_free) ? ESP_ERR_NOT_FINISHED : ESP_OK; } esp_err_t ext_hub_status_handle_complete(ext_hub_handle_t ext_hub_hdl) @@ -1576,66 +1572,30 @@ esp_err_t ext_hub_port_recycle(ext_hub_handle_t ext_hub_hdl, uint8_t port_num) EXT_HUB_CHECK(ext_hub_hdl != NULL, ESP_ERR_INVALID_ARG); ext_hub_dev_t *ext_hub_dev = (ext_hub_dev_t *)ext_hub_hdl; - esp_err_t ret; uint8_t port_idx = port_num - 1; - bool free_port = false; bool release_port = false; - EXT_HUB_CHECK(port_idx < ext_hub_dev->constant.hub_desc->bNbrPorts, ESP_ERR_INVALID_SIZE); EXT_HUB_CHECK(p_ext_hub_driver->constant.port_driver != NULL, ESP_ERR_NOT_SUPPORTED); EXT_HUB_CHECK(ext_hub_dev->single_thread.state == EXT_HUB_STATE_CONFIGURED || ext_hub_dev->single_thread.state == EXT_HUB_STATE_RELEASED, ESP_ERR_INVALID_STATE); EXT_HUB_ENTER_CRITICAL(); - if (ext_hub_dev->dynamic.flags.waiting_release) { + if (ext_hub_dev->dynamic.flags.waiting_release || + ext_hub_dev->dynamic.flags.waiting_children) { release_port = true; - } else if (ext_hub_dev->dynamic.flags.waiting_children) { - assert(ext_hub_dev->dynamic.flags.waiting_release == 0); // Device should be already released - assert(ext_hub_dev->dynamic.flags.is_gone == 1); // Device should be gone by now - free_port = true; } EXT_HUB_EXIT_CRITICAL(); - if (!release_port && !free_port) { + esp_err_t ret; + if (release_port) { + ret = device_release_port(ext_hub_dev, port_idx); + } else { // Parent still present, recycle the port ret = p_ext_hub_driver->constant.port_driver->recycle(ext_hub_dev->constant.ports[port_idx]); if (ret != ESP_OK) { ESP_LOGE(EXT_HUB_TAG, "[%d:%d] Unable to recycle the port: %s", ext_hub_dev->constant.dev_addr, port_num, esp_err_to_name(ret)); - goto exit; - } - } else { - if (release_port) { - // Notify the port that parent is not available anymore and port should be recycled then freed - ret = p_ext_hub_driver->constant.port_driver->gone(ext_hub_dev->constant.ports[port_idx]); - if (ret == ESP_OK) { - ESP_LOGD(EXT_HUB_TAG, "[%d:%d] Port doesn't have a device and can be freed right now", - ext_hub_dev->constant.dev_addr, - port_num); - assert(free_port == false); - free_port = true; - } else if (ret == ESP_ERR_NOT_FINISHED) { - // Port has a device and will be recycled after USBH device will be released by all clients and freed - ESP_LOGE(EXT_HUB_TAG, "[%d:%d] Port is gone", - ext_hub_dev->constant.dev_addr, - port_num); - // Logically, recycling logic are finished for the Hub Driver, return ESP_OK to free the node - assert(free_port == false); - } else { - ESP_LOGE(EXT_HUB_TAG, "[%d:%d] Unable to mark port as gone: %s", - ext_hub_dev->constant.dev_addr, port_num, esp_err_to_name(ret)); - } - } - - if (free_port) { - ret = device_port_free(ext_hub_dev, port_idx); - if (ret != ESP_OK) { - goto exit; - } } } - - ret = ESP_OK; -exit: return ret; } diff --git a/components/usb/ext_port.c b/components/usb/ext_port.c index 5fedc49ae3..eb5f40d3ea 100644 --- a/components/usb/ext_port.c +++ b/components/usb/ext_port.c @@ -717,7 +717,7 @@ static void handle_port_state(ext_port_t *ext_port) switch (curr_state) { case USB_PORT_STATE_NOT_CONFIGURED: new_state = USB_PORT_STATE_POWERED_OFF; - ESP_ERROR_CHECK(port_request_status(ext_port)); + port_request_status(ext_port); need_handling = true; break; case USB_PORT_STATE_POWERED_OFF: diff --git a/components/usb/hub.c b/components/usb/hub.c index 38d51c525e..21a6e608ad 100644 --- a/components/usb/hub.c +++ b/components/usb/hub.c @@ -768,7 +768,7 @@ esp_err_t hub_port_disable(usb_device_handle_t parent_dev_hdl, uint8_t parent_po return ret; } -esp_err_t hub_notify_new_dev(uint8_t dev_addr) +esp_err_t hub_dev_new(uint8_t dev_addr) { HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); @@ -791,15 +791,15 @@ esp_err_t hub_notify_new_dev(uint8_t dev_addr) } } // Close device - usbh_dev_close(dev_hdl); + ESP_ERROR_CHECK(usbh_dev_close(dev_hdl)); } - // Logic should not stop the flow, so no error to return - ret = ESP_OK; + // Nothing to do, while Hubs support is not enabled + ret = ESP_ERR_NOT_SUPPORTED; #endif // ENABLE_USB_HUBS return ret; } -esp_err_t hub_notify_dev_gone(uint8_t dev_addr) +esp_err_t hub_dev_gone(uint8_t dev_addr) { HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); @@ -810,7 +810,7 @@ esp_err_t hub_notify_dev_gone(uint8_t dev_addr) ret = ext_hub_dev_gone(dev_addr); #else // Nothing to do, while Hubs support is not enabled - ret = ESP_OK; + ret = ESP_ERR_NOT_SUPPORTED; #endif // ENABLE_USB_HUBS return ret; } @@ -822,7 +822,9 @@ esp_err_t hub_notify_all_free(void) HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); HUB_DRIVER_EXIT_CRITICAL(); - return ext_hub_all_free(); + ext_hub_mark_all_free(); + + return ESP_OK; } #endif // ENABLE_USB_HUBS diff --git a/components/usb/private_include/ext_hub.h b/components/usb/private_include/ext_hub.h index be95dbefe3..f58b9a3383 100644 --- a/components/usb/private_include/ext_hub.h +++ b/components/usb/private_include/ext_hub.h @@ -131,11 +131,8 @@ esp_err_t ext_hub_dev_gone(uint8_t dev_addr); * Entry: * - should be called within Hub Driver when USB Host library need to be uninstalled * - * @return - * - ESP_OK: All devices freed - * - ESP_ERR_NOT_FINISHED: Operation not finished: devices waiting children to be freed. */ -esp_err_t ext_hub_all_free(void); +void ext_hub_mark_all_free(void); /** * @brief The External Hub or Ports statuses change completed diff --git a/components/usb/private_include/hub.h b/components/usb/private_include/hub.h index f563a56f28..c5ca0aec88 100644 --- a/components/usb/private_include/hub.h +++ b/components/usb/private_include/hub.h @@ -205,7 +205,7 @@ esp_err_t hub_port_disable(usb_device_handle_t parent_dev_hdl, uint8_t parent_po * - ESP_OK: New device added successfully * - ESP_ERR_INVALID_STATE: Hub driver is not in a correct state */ -esp_err_t hub_notify_new_dev(uint8_t dev_addr); +esp_err_t hub_dev_new(uint8_t dev_addr); /** * @brief Notify Hub driver that device has been removed @@ -221,7 +221,7 @@ esp_err_t hub_notify_new_dev(uint8_t dev_addr); * - ESP_OK: A device removed successfully * - ESP_ERR_INVALID_STATE: Hub driver is not in a correct state */ -esp_err_t hub_notify_dev_gone(uint8_t dev_addr); +esp_err_t hub_dev_gone(uint8_t dev_addr); #if ENABLE_USB_HUBS /** diff --git a/components/usb/test_apps/usb_host/main/test_usb_host_async.c b/components/usb/test_apps/usb_host/main/test_usb_host_async.c index e691708670..3d68b06495 100644 --- a/components/usb/test_apps/usb_host/main/test_usb_host_async.c +++ b/components/usb/test_apps/usb_host/main/test_usb_host_async.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -297,6 +297,7 @@ Procedure: */ static void host_lib_task(void *arg) { + TaskHandle_t pending_task = (TaskHandle_t)arg; while (1) { // Start handling system events uint32_t event_flags; @@ -306,10 +307,12 @@ static void host_lib_task(void *arg) TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_device_free_all()); } if (event_flags & USB_HOST_LIB_EVENT_FLAGS_ALL_FREE) { + printf("All devices freed\n"); break; } } - + // Notify the main task that the host library task is done + xTaskNotifyGive(pending_task); printf("Deleting host_lib_task\n"); vTaskDelete(NULL); } @@ -330,13 +333,16 @@ TEST_CASE("Test USB Host multiconfig client (single client)", "[usb_host][full_s xTaskNotifyGive(client_task); TaskHandle_t host_lib_task_hdl; - xTaskCreatePinnedToCore(host_lib_task, "host lib", 4096, NULL, 2, &host_lib_task_hdl, 0); + // Get Current task handle + TaskHandle_t pending_task = xTaskGetCurrentTaskHandle(); + xTaskCreatePinnedToCore(host_lib_task, "host lib", 4096, (void*)pending_task, 2, &host_lib_task_hdl, 0); TEST_ASSERT_NOT_NULL_MESSAGE(host_lib_task_hdl, "Failed to create host lib task"); // Wait for the device to be open xSemaphoreTake(dev_open_smp, portMAX_DELAY); multiconf_client_get_conf_desc(); - + // Wait for the host library task to finish + ulTaskNotifyTake(pdTRUE, portMAX_DELAY); // Cleanup vSemaphoreDelete(dev_open_smp); } diff --git a/components/usb/usb_host.c b/components/usb/usb_host.c index 24d8838342..d4c38895ea 100644 --- a/components/usb/usb_host.c +++ b/components/usb/usb_host.c @@ -221,7 +221,7 @@ static bool _unblock_lib(bool in_isr) return yield; } -static inline bool _is_internal_client(void *client) +static inline bool is_internal_client(void *client) { if (p_host_lib_obj->constant.enum_client && (client == p_host_lib_obj->constant.enum_client)) { return true; @@ -295,7 +295,7 @@ static void usbh_event_callback(usbh_event_data_t *event_data, void *arg) assert(event_data->ctrl_xfer_data.urb != NULL); assert(event_data->ctrl_xfer_data.urb->usb_host_client != NULL); // Redistribute completed control transfers to the clients that submitted them - if (_is_internal_client(event_data->ctrl_xfer_data.urb->usb_host_client)) { + if (is_internal_client(event_data->ctrl_xfer_data.urb->usb_host_client)) { // Simply call the transfer callback event_data->ctrl_xfer_data.urb->transfer.callback(&event_data->ctrl_xfer_data.urb->transfer); } else { @@ -309,10 +309,11 @@ static void usbh_event_callback(usbh_event_data_t *event_data, void *arg) break; } case USBH_EVENT_NEW_DEV: { - // Internal client - hub_notify_new_dev(event_data->new_dev_data.dev_addr); - // External clients - // Prepare a NEW_DEV client event message, the send it to all clients + if (hub_dev_new(event_data->new_dev_data.dev_addr) == ESP_OK) { + // Device is a hub, we do not need to propagate the event to the clients + break; + } + // Prepare a NEW_DEV client event message, and send it to all clients usb_host_client_event_msg_t event_msg = { .event = USB_HOST_CLIENT_EVENT_NEW_DEV, .new_dev.address = event_data->new_dev_data.dev_addr, @@ -321,10 +322,11 @@ static void usbh_event_callback(usbh_event_data_t *event_data, void *arg) break; } case USBH_EVENT_DEV_GONE: { - // Internal client - hub_notify_dev_gone(event_data->new_dev_data.dev_addr); - // External clients - // Prepare event msg, send only to clients that have opened the device + if (hub_dev_gone(event_data->new_dev_data.dev_addr) == ESP_OK) { + // Device is a hub, we do not need to propagate the event to the clients + break; + } + // Prepare a DEV GONE event, send only to clients that have opened the device usb_host_client_event_msg_t event_msg = { .event = USB_HOST_CLIENT_EVENT_DEV_GONE, .dev_gone.dev_hdl = event_data->dev_gone_data.dev_hdl, From e550fb11c395dc391eca3b410260de33af9e5b62 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Mon, 2 Jun 2025 17:40:04 +0800 Subject: [PATCH 2/2] fix(ext_hub): Added processing waiting_release flag while dev changed to IDLE 11382a2a fix(ext_hub): Added processing waiting_release flag while dev changed to IDLE Co-authored-by: Roman Leonov --- components/usb/ext_hub.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/usb/ext_hub.c b/components/usb/ext_hub.c index 55d4205b84..0260f8cd7c 100644 --- a/components/usb/ext_hub.c +++ b/components/usb/ext_hub.c @@ -1065,6 +1065,11 @@ static bool device_set_next_stage(ext_hub_dev_t *ext_hub_dev, bool last_stage_pa } else { // Terminal stages, move to IDLE next_stage = EXT_HUB_STAGE_IDLE; + EXT_HUB_ENTER_CRITICAL(); + if (ext_hub_dev->dynamic.flags.waiting_release) { + call_proc_req_cb = _device_set_actions(ext_hub_dev, DEV_ACTION_RELEASE); + } + EXT_HUB_EXIT_CRITICAL(); } ext_hub_dev->single_thread.stage = next_stage; call_proc_req_cb = stage_need_process(next_stage);