From 508a1b4c74c3aac646f20470a9743bedd3310367 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 14 Feb 2025 11:47:13 +0100 Subject: [PATCH 01/17] refactor(enum): Removed parent_dev_hdl and parent_port_num, replaced with node uid --- host/usb/private_include/enum.h | 30 ++--------- host/usb/private_include/hub.h | 43 +++++++-------- host/usb/src/enum.c | 96 +++++++-------------------------- host/usb/src/hub.c | 60 +++++++++++++++------ host/usb/src/usb_host.c | 16 +++--- 5 files changed, 96 insertions(+), 149 deletions(-) diff --git a/host/usb/private_include/enum.h b/host/usb/private_include/enum.h index 59b708d5..e4adc382 100644 --- a/host/usb/private_include/enum.h +++ b/host/usb/private_include/enum.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -44,31 +44,9 @@ typedef enum { } enum_event_t; typedef struct { - enum_event_t event; /**< Enumerator driver event */ - union { - struct { - unsigned int uid; /**< Device unique ID */ - usb_device_handle_t parent_dev_hdl; /**< Parent of the enumerating device */ - uint8_t parent_port_num; /**< Parent port number of the enumerating device */ - } started; /**< ENUM_EVENT_STARTED specific data */ - - struct { - usb_device_handle_t parent_dev_hdl; /**< Parent of the enumerating device */ - uint8_t parent_port_num; /**< Parent port number of the enumerating device */ - } reset_req; /**< ENUM_EVENT_RESET_REQUIRED specific data */ - - struct { - usb_device_handle_t parent_dev_hdl; /**< Parent of the enumerating device */ - uint8_t parent_port_num; /**< Parent port number of the enumerating device */ - usb_device_handle_t dev_hdl; /**< Handle of the enumerating device */ - uint8_t dev_addr; /**< Address of the enumerating device */ - } complete; /**< ENUM_EVENT_COMPLETED specific data */ - - struct { - usb_device_handle_t parent_dev_hdl; /**< Parent of the enumerating device */ - uint8_t parent_port_num; /**< Parent port number of the enumerating device */ - } canceled; /**< ENUM_EVENT_CANCELED specific data */ - }; + enum_event_t event; /**< Enumerator driver event */ + unsigned int node_uid; /**< Unique node ID */ + usb_device_handle_t dev_hdl; /**< Handle of the enumerating device */ } enum_event_data_t; // ---------------------------- Callbacks -------------------------------------- diff --git a/host/usb/private_include/hub.h b/host/usb/private_include/hub.h index e47e11c1..e3fe727f 100644 --- a/host/usb/private_include/hub.h +++ b/host/usb/private_include/hub.h @@ -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 */ @@ -131,66 +131,67 @@ esp_err_t hub_root_stop(void); /** * @brief Indicate to the Hub driver that a device's port can be recycled * - * The device connected to the port has been freed. The Hub driver can now recycled the port + * The device connected to the port has been freed. + * The Hub driver can now recycled the node and re-enable the port while it it still present. * * @note This function should only be called from the Host Library task * - * @param[in] parent_dev_hdl Parent device handle (is used to get the External Hub handle) - * @param[in] parent_port_num Parent number (is used to specify the External Port) - * @param[in] dev_uid Device's unique ID + * @param[in] node_uid Device's node unique ID * * @return * - ESP_OK: device's port can be recycled * - ESP_ERR_INVALID_STATE: Hub driver is not installed * - ESP_ERR_NOT_SUPPORTED: Recycling External Port is not available (External Hub support disabled), * or ext hub port error + * - ESP_ERR_NOT_FOUND: Device's node is not found */ -esp_err_t hub_port_recycle(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num, unsigned int dev_uid); +esp_err_t hub_node_recycle(unsigned int node_uid); /** - * @brief Reset the port + * @brief Reset the device in the port, related to the specific Device Tree node * * @note This function should only be called from the Host Library task * - * @param[in] parent_dev_hdl Parent device handle (is used to get the External Hub handle) - * @param[in] parent_port_num Parent number (is used to specify the External Port) + * @param[in] node_uid Device's node unique ID + * * @return - * - ESP_OK: Port reset successful + * - ESP_OK if device in port reset successful * - ESP_ERR_INVALID_STATE: Hub driver is not installed * - ESP_ERR_NOT_SUPPORTED: Resetting External Port is not available (External Hub support disabled), * or ext hub port error + * - ESP_ERR_NOT_FOUND: Device's node is not found */ -esp_err_t hub_port_reset(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num); +esp_err_t hub_node_reset(unsigned int node_uid); /** - * @brief Activate the port + * @brief Port, related to the specific Device Tree node, has an active device * * @note This function should only be called from the Host Library task * - * @param[in] parent_dev_hdl Parent device handle (is used to get the External Hub handle) - * @param[in] parent_port_num Parent number (is used to specify the External Port) + * @param[in] node_uid Device's node unique ID * * @return - * - ESP_OK: Port activated successfully - * - ESP_ERR_NOT_SUPPORTED: Activating External Port is not available (External Hub support disabled), + * - ESP_OK if Port, related to the Device Tree node was activated successfully + * - ESP_ERR_NOT_SUPPORTED if activating Port is not available (External Hub support disabled), * or ext hub port error + * - ESP_ERR_NOT_FOUND if Device's node is not found */ -esp_err_t hub_port_active(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num); +esp_err_t hub_node_active(unsigned int node_uid); /** - * @brief Disable the port + * @brief Disable the port, related to the specific Device Tree node * * @note This function should only be called from the Host Library task * - * @param[in] parent_dev_hdl Parent device handle (is used to get the External Hub handle) - * @param[in] parent_port_num Parent number (is used to specify the External Port) + * @param[in] node_uid Device's node unique ID * * @return * - ESP_OK: Port has been disabled without error * - ESP_ERR_INVALID_STATE: Port can't be disabled in current state * - ESP_ERR_NOT_SUPPORTED: This function is not support by the selected port + * - ESP_ERR_NOT_FOUND: Device's node is not found */ -esp_err_t hub_port_disable(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num); +esp_err_t hub_node_disable(unsigned int node_uid); /** * @brief Notify Hub driver that new device has been attached diff --git a/host/usb/src/enum.c b/host/usb/src/enum.c index 7d76e3ae..2c1b9159 100644 --- a/host/usb/src/enum.c +++ b/host/usb/src/enum.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -132,12 +132,8 @@ typedef struct { typedef struct { struct { // Device related objects, initialized at start of a particular enumeration - unsigned int dev_uid; /**< Unique device ID being enumerated */ + unsigned int node_uid; /**< Unique node ID of device being enumerated */ usb_device_handle_t dev_hdl; /**< Handle of device being enumerated */ - // Parent info for optimization and more clean debug output - usb_device_handle_t parent_dev_hdl; /**< Device's parent handle */ - uint8_t parent_dev_addr; /**< Device's parent address */ - uint8_t parent_port_num; /**< Device's parent port number */ // Parameters, updated during enumeration enum_stage_t stage; /**< Current enumeration stage */ enum_device_params_t dev_params; /**< Parameters of device under enumeration */ @@ -279,13 +275,9 @@ static esp_err_t second_reset_request(void) // Notify USB Host enum_event_data_t event_data = { .event = ENUM_EVENT_RESET_REQUIRED, - .reset_req = { - .parent_dev_hdl = p_enum_driver->single_thread.parent_dev_hdl, - .parent_port_num = p_enum_driver->single_thread.parent_port_num, - }, + .node_uid = p_enum_driver->single_thread.node_uid, }; p_enum_driver->constant.enum_event_cb(&event_data, p_enum_driver->constant.enum_event_cb_arg); - return ESP_OK; } @@ -739,10 +731,8 @@ static esp_err_t control_request(enum_stage_t stage) ret = usbh_dev_submit_ctrl_urb(p_enum_driver->single_thread.dev_hdl, p_enum_driver->constant.urb); if (ret != ESP_OK) { - ESP_LOGE(ENUM_TAG, "[%d:%d] Control transfer submit error (%#x), stage '%s'", - p_enum_driver->single_thread.parent_dev_addr, - p_enum_driver->single_thread.parent_port_num, - ret, + ESP_LOGE(ENUM_TAG, "Control transfer submit error %s, stage '%s'", + esp_err_to_name(ret), enum_stage_strings[stage]); } @@ -781,9 +771,7 @@ static esp_err_t control_response_handling(enum_stage_t stage) // Check Control IN transfer returned the expected correct number of bytes if (expected_num_bytes != 0 && expected_num_bytes != ctrl_xfer->actual_num_bytes) { - ESP_LOGW(ENUM_TAG, "[%d:%d] Unexpected (%d) device response length (expected %d)", - p_enum_driver->single_thread.parent_dev_addr, - p_enum_driver->single_thread.parent_port_num, + ESP_LOGW(ENUM_TAG, "Unexpected (%d) device response length (expected %d)", ctrl_xfer->actual_num_bytes, expected_num_bytes); if (ctrl_xfer->actual_num_bytes < expected_num_bytes) { @@ -847,9 +835,8 @@ static esp_err_t control_response_handling(enum_stage_t stage) static esp_err_t stage_cancel(void) { // There should be device under enumeration + const unsigned int node_uid = p_enum_driver->single_thread.node_uid; usb_device_handle_t dev_hdl = p_enum_driver->single_thread.dev_hdl; - usb_device_handle_t parent_dev_hdl = p_enum_driver->single_thread.parent_dev_hdl; - uint8_t parent_port_num = p_enum_driver->single_thread.parent_port_num; if (dev_hdl) { ESP_ERROR_CHECK(usbh_dev_enum_unlock(dev_hdl)); @@ -857,21 +844,15 @@ static esp_err_t stage_cancel(void) } // Clean up variables device from enumerator - p_enum_driver->single_thread.dev_uid = 0; + p_enum_driver->single_thread.node_uid = 0; p_enum_driver->single_thread.dev_hdl = NULL; - p_enum_driver->single_thread.parent_dev_hdl = NULL; - p_enum_driver->single_thread.parent_dev_addr = 0; - p_enum_driver->single_thread.parent_port_num = 0; p_enum_driver->constant.urb->transfer.context = NULL; // Propagate the event enum_event_data_t event_data = { .event = ENUM_EVENT_CANCELED, - .canceled = { - .parent_dev_hdl = parent_dev_hdl, - .parent_port_num = parent_port_num, - }, + .node_uid = node_uid, }; p_enum_driver->constant.enum_event_cb(&event_data, p_enum_driver->constant.enum_event_cb_arg); return ESP_OK; @@ -884,10 +865,8 @@ static esp_err_t stage_cancel(void) */ static esp_err_t stage_complete(void) { + unsigned int node_uid = p_enum_driver->single_thread.node_uid; usb_device_handle_t dev_hdl = p_enum_driver->single_thread.dev_hdl; - usb_device_handle_t parent_dev_hdl = p_enum_driver->single_thread.parent_dev_hdl; - uint8_t parent_dev_addr = p_enum_driver->single_thread.parent_dev_addr; - uint8_t parent_port_num = p_enum_driver->single_thread.parent_port_num; uint8_t dev_addr = 0; ESP_ERROR_CHECK(usbh_dev_get_addr(dev_hdl, &dev_addr)); @@ -896,11 +875,8 @@ static esp_err_t stage_complete(void) ESP_ERROR_CHECK(usbh_dev_close(dev_hdl)); // Release device from enumerator - p_enum_driver->single_thread.dev_uid = 0; + p_enum_driver->single_thread.node_uid = 0; p_enum_driver->single_thread.dev_hdl = NULL; - p_enum_driver->single_thread.parent_dev_hdl = NULL; - p_enum_driver->single_thread.parent_dev_addr = 0; - p_enum_driver->single_thread.parent_port_num = 0; // Release device from enumerator p_enum_driver->constant.urb->transfer.context = NULL; @@ -912,19 +888,12 @@ static esp_err_t stage_complete(void) // Increase device address to use new value during the next enumeration process get_next_dev_addr(); - ESP_LOGD(ENUM_TAG, "[%d:%d] Processing complete, new device address %d", - parent_dev_addr, - parent_port_num, - dev_addr); + ESP_LOGD(ENUM_TAG, "Processing complete, new device address %d", dev_addr); enum_event_data_t event_data = { .event = ENUM_EVENT_COMPLETED, - .complete = { - .dev_hdl = dev_hdl, - .dev_addr = dev_addr, - .parent_dev_hdl = parent_dev_hdl, - .parent_port_num = parent_port_num, - }, + .node_uid = node_uid, + .dev_hdl = dev_hdl, }; p_enum_driver->constant.enum_event_cb(&event_data, p_enum_driver->constant.enum_event_cb_arg); return ESP_OK; @@ -1005,10 +974,7 @@ static bool set_next_stage(bool last_stage_pass) // Find the next stage if (last_stage_pass) { // Last stage was successful - ESP_LOGD(ENUM_TAG, "[%d:%d] %s OK", - p_enum_driver->single_thread.parent_dev_addr, - p_enum_driver->single_thread.parent_port_num, - enum_stage_strings[last_stage]); + ESP_LOGD(ENUM_TAG, "%s OK", enum_stage_strings[last_stage]); // Get next stage if (last_stage == ENUM_STAGE_COMPLETE || last_stage == ENUM_STAGE_CANCEL) { @@ -1055,10 +1021,7 @@ static bool set_next_stage(bool last_stage_pass) break; default: // Stage is not allowed to failed. Cancel enumeration. - ESP_LOGE(ENUM_TAG, "[%d:%d] %s FAILED", - p_enum_driver->single_thread.parent_dev_addr, - p_enum_driver->single_thread.parent_port_num, - enum_stage_strings[last_stage]); + ESP_LOGE(ENUM_TAG, "%s FAILED", enum_stage_strings[last_stage]); next_stage = ENUM_STAGE_CANCEL; break; } @@ -1211,25 +1174,14 @@ esp_err_t enum_start(unsigned int uid) // Get device info usb_device_info_t dev_info; - uint8_t parent_dev_addr = 0; ESP_ERROR_CHECK(usbh_dev_get_info(dev_hdl, &dev_info)); - if (dev_info.parent.dev_hdl) { - ESP_ERROR_CHECK(usbh_dev_get_addr(dev_info.parent.dev_hdl, &parent_dev_addr)); - } - // Stage ENUM_STAGE_GET_SHORT_DEV_DESC - ESP_LOGD(ENUM_TAG, "[%d:%d] Start processing, device address %d", - parent_dev_addr, - dev_info.parent.port_num, - 0); + ESP_LOGD(ENUM_TAG, "Start processing device with address %d", 0); p_enum_driver->single_thread.stage = ENUM_STAGE_GET_SHORT_DEV_DESC; - p_enum_driver->single_thread.dev_uid = uid; + p_enum_driver->single_thread.node_uid = uid; p_enum_driver->single_thread.dev_hdl = dev_hdl; - p_enum_driver->single_thread.parent_dev_hdl = dev_info.parent.dev_hdl; - p_enum_driver->single_thread.parent_dev_addr = parent_dev_addr; - p_enum_driver->single_thread.parent_port_num = dev_info.parent.port_num; // Save device handle to the URB transfer context p_enum_driver->constant.urb->transfer.context = (void *) dev_hdl; // Device params @@ -1241,14 +1193,9 @@ esp_err_t enum_start(unsigned int uid) // Notify USB Host about starting enumeration process enum_event_data_t event_data = { .event = ENUM_EVENT_STARTED, - .started = { - .uid = uid, - .parent_dev_hdl = dev_info.parent.dev_hdl, - .parent_port_num = dev_info.parent.port_num, - }, + .node_uid = uid, }; p_enum_driver->constant.enum_event_cb(&event_data, p_enum_driver->constant.enum_event_cb_arg); - // Request processing p_enum_driver->constant.proc_req_cb(USB_PROC_REQ_SOURCE_ENUM, false, p_enum_driver->constant.proc_req_cb_arg); return ret; @@ -1276,10 +1223,7 @@ esp_err_t enum_cancel(unsigned int uid) p_enum_driver->single_thread.stage = ENUM_STAGE_CANCEL; - ESP_LOGV(ENUM_TAG, "[%d:%d] Cancel at %s", - p_enum_driver->single_thread.parent_dev_addr, - p_enum_driver->single_thread.parent_port_num, - enum_stage_strings[old_stage]); + ESP_LOGV(ENUM_TAG, "Cancel at %s", enum_stage_strings[old_stage]); if (stage_need_process(old_stage)) { // These stages are required to trigger processing in the enum_process() diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index 8af9d09b..8ce34f13 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -145,6 +145,20 @@ static bool root_port_callback(hcd_port_handle_t port_hdl, hcd_port_event_t port // ---------------------- Internal Logic ------------------------ +dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid) +{ + dev_tree_node_t *dev_tree_node = NULL; + dev_tree_node_t *dev_tree_iter; + // Search the device tree nodes list for a device node with the specified parent + TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { + if (dev_tree_iter->uid == node_uid) { + dev_tree_node = dev_tree_iter; + break; + } + } + return dev_tree_node; +} + /** * @brief Creates new device tree node and propagate HUB_EVENT_CONNECTED event * @@ -667,20 +681,23 @@ esp_err_t hub_root_stop(void) return ret; } -esp_err_t hub_port_recycle(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num, unsigned int dev_uid) +esp_err_t hub_node_recycle(unsigned int node_uid) { HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); HUB_DRIVER_EXIT_CRITICAL(); esp_err_t ret; - if (parent_port_num == 0) { + dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); + HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); + + if (dev_tree_node->parent_dev_hdl == NULL) { ret = root_port_recycle(); } else { #if ENABLE_USB_HUBS ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_recycle(ext_hub_hdl, parent_port_num); + ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); + ret = ext_hub_port_recycle(ext_hub_hdl, dev_tree_node->parent_port_num); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port recycle error: %s", esp_err_to_name(ret)); } @@ -691,20 +708,23 @@ esp_err_t hub_port_recycle(usb_device_handle_t parent_dev_hdl, uint8_t parent_po } if (ret == ESP_OK) { - ESP_ERROR_CHECK(dev_tree_node_remove_by_parent(parent_dev_hdl, parent_port_num)); + ESP_ERROR_CHECK(dev_tree_node_remove_by_parent(dev_tree_node->parent_dev_hdl, dev_tree_node->parent_port_num)); } return ret; } -esp_err_t hub_port_reset(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num) +esp_err_t hub_node_reset(unsigned int node_uid) { HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); HUB_DRIVER_EXIT_CRITICAL(); esp_err_t ret; - if (parent_port_num == 0) { + dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); + HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); + + if (dev_tree_node->parent_dev_hdl == NULL) { ret = hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_RESET); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Failed to issue root port reset"); @@ -714,8 +734,8 @@ esp_err_t hub_port_reset(usb_device_handle_t parent_dev_hdl, uint8_t parent_port } else { #if ENABLE_USB_HUBS ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_reset(ext_hub_hdl, parent_port_num); + ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); + ret = ext_hub_port_reset(ext_hub_hdl, dev_tree_node->parent_port_num); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port reset error: %s", esp_err_to_name(ret)); } @@ -727,19 +747,22 @@ esp_err_t hub_port_reset(usb_device_handle_t parent_dev_hdl, uint8_t parent_port return ret; } -esp_err_t hub_port_active(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num) +esp_err_t hub_node_active(unsigned int node_uid) { esp_err_t ret; - if (parent_port_num == 0) { + dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); + HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); + + if (dev_tree_node->parent_dev_hdl == NULL) { // Root port no need to be activated ret = ESP_OK; } else { #if ENABLE_USB_HUBS // External Hub port ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_active(ext_hub_hdl, parent_port_num); + ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); + ret = ext_hub_port_active(ext_hub_hdl, dev_tree_node->parent_port_num); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port activation error: %s", esp_err_to_name(ret)); } @@ -751,18 +774,21 @@ esp_err_t hub_port_active(usb_device_handle_t parent_dev_hdl, uint8_t parent_por return ret; } -esp_err_t hub_port_disable(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num) +esp_err_t hub_node_disable(unsigned int node_uid) { esp_err_t ret; - if (parent_port_num == 0) { + dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); + HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); + + if (dev_tree_node->parent_dev_hdl == NULL) { ret = root_port_disable(); } else { #if ENABLE_USB_HUBS // External Hub port ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_disable(ext_hub_hdl, parent_port_num); + ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); + ret = ext_hub_port_disable(ext_hub_hdl, dev_tree_node->parent_port_num); #else ESP_LOGW(HUB_DRIVER_TAG, "Activating External Port is not available (External Hub support disabled)"); ret = ESP_ERR_NOT_SUPPORTED; diff --git a/host/usb/src/usb_host.c b/host/usb/src/usb_host.c index beecebee..656dd5da 100644 --- a/host/usb/src/usb_host.c +++ b/host/usb/src/usb_host.c @@ -344,11 +344,9 @@ static void usbh_event_callback(usbh_event_data_t *event_data, void *arg) break; } case USBH_EVENT_DEV_FREE: { - // Let the Hub driver know that the device is free and its port can be recycled - // Port could be absent, no need to verify - hub_port_recycle(event_data->dev_free_data.parent_dev_hdl, - event_data->dev_free_data.port_num, - event_data->dev_free_data.dev_uid); + // Let the Hub driver know that the device is free and its node can be free and port re-enabled + // Node could be already freed on device disconnect (when clients still holding the device opened), no need to verify result + hub_node_recycle(event_data->dev_free_data.dev_uid); break; } case USBH_EVENT_ALL_FREE: { @@ -397,16 +395,16 @@ static void enum_event_callback(enum_event_data_t *event_data, void *arg) break; case ENUM_EVENT_RESET_REQUIRED: // Device may be gone, don't need to verify result - hub_port_reset(event_data->reset_req.parent_dev_hdl, event_data->reset_req.parent_port_num); + hub_node_reset(event_data->node_uid); break; case ENUM_EVENT_COMPLETED: // Notify port that device completed enumeration - hub_port_active(event_data->complete.parent_dev_hdl, event_data->complete.parent_port_num); + hub_node_active(event_data->node_uid); // Propagate a new device event - ESP_ERROR_CHECK(usbh_devs_new_dev_event(event_data->complete.dev_hdl)); + ESP_ERROR_CHECK(usbh_devs_new_dev_event(event_data->dev_hdl)); break; case ENUM_EVENT_CANCELED: - hub_port_disable(event_data->canceled.parent_dev_hdl, event_data->canceled.parent_port_num); + hub_node_disable(event_data->node_uid); break; default: abort(); // Should never occur From 663be6af2631d568101c7e1ede9f8a60c90895ea Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 10:16:13 +0100 Subject: [PATCH 02/17] refactor(hub): Removed parent_dev_hdl from dev tree node structure --- host/usb/src/hub.c | 87 +++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index 8ce34f13..cb2ea041 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -69,9 +69,9 @@ typedef enum { */ struct dev_tree_node_s { TAILQ_ENTRY(dev_tree_node_s) tailq_entry; /**< Entry for the device tree node object tailq */ - unsigned int uid; /**< Device's unique ID */ - usb_device_handle_t parent_dev_hdl; /**< Device's parent handle */ - uint8_t parent_port_num; /**< Device's parent port number */ + unsigned int uid; /**< Device's unique ID */ + void* parent; /**< Device's parent context */ + uint8_t port_num; /**< Device's parent port number */ }; typedef struct dev_tree_node_s dev_tree_node_t; @@ -145,7 +145,7 @@ static bool root_port_callback(hcd_port_handle_t port_hdl, hcd_port_event_t port // ---------------------- Internal Logic ------------------------ -dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid) +static dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid) { dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; @@ -164,7 +164,7 @@ dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid) * * @return esp_err_t */ -static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num, usb_speed_t speed) +static esp_err_t dev_tree_node_new(void* parent, uint8_t port_num, usb_speed_t speed) { esp_err_t ret; // Allocate memory for a new device tree node @@ -182,8 +182,8 @@ static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t p assert(dev_tree_node->uid != 0); // No overflow possible } - dev_tree_node->parent_dev_hdl = parent_dev_hdl; - dev_tree_node->parent_port_num = parent_port_num; + dev_tree_node->parent = parent; + dev_tree_node->port_num = port_num; // Initialize and register a new USBH Device with the assigned UID usbh_dev_params_t params = { @@ -191,8 +191,8 @@ static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t p .speed = speed, .root_port_hdl = p_hub_driver_obj->constant.root_port_hdl, // Always the same for all devices // TODO: IDF-10023 Move parent-child tree management responsibility to Hub Driver - .parent_dev_hdl = parent_dev_hdl, - .parent_port_num = parent_port_num, + .parent_dev_hdl = NULL, + .parent_port_num = port_num, }; ret = usbh_devs_add(¶ms); @@ -203,7 +203,7 @@ static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t p TAILQ_INSERT_TAIL(&p_hub_driver_obj->single_thread.dev_nodes_tailq, dev_tree_node, tailq_entry); - ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): new", dev_tree_node->uid); + ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid %d): new", dev_tree_node->port_num, dev_tree_node->uid); hub_event_data_t event_data = { .event = HUB_EVENT_CONNECTED, @@ -220,21 +220,21 @@ static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t p return ret; } -static esp_err_t dev_tree_node_reset_completed(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num) +static esp_err_t dev_tree_node_reset_completed(void* parent, uint8_t port_num) { dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; // Search the device tree nodes list for a device node with the specified parent TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { - if (dev_tree_iter->parent_dev_hdl == parent_dev_hdl && - dev_tree_iter->parent_port_num == parent_port_num) { + if (dev_tree_iter->parent == parent && + dev_tree_iter->port_num == port_num) { dev_tree_node = dev_tree_iter; break; } } if (dev_tree_node == NULL) { - ESP_LOGE(HUB_DRIVER_TAG, "Reset completed, but device tree node with port=%d not found", parent_port_num); + ESP_LOGE(HUB_DRIVER_TAG, "Reset completed, but device tree node (port %d) not found", port_num); return ESP_ERR_NOT_FOUND; } @@ -248,25 +248,25 @@ static esp_err_t dev_tree_node_reset_completed(usb_device_handle_t parent_dev_hd return ESP_OK; } -static esp_err_t dev_tree_node_dev_gone(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num) +static esp_err_t dev_tree_node_dev_gone(void* parent, uint8_t port_num) { dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; // Search the device tree nodes list for a device node with the specified parent TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { - if (dev_tree_iter->parent_dev_hdl == parent_dev_hdl && - dev_tree_iter->parent_port_num == parent_port_num) { + if (dev_tree_iter->parent == parent && + dev_tree_iter->port_num == port_num) { dev_tree_node = dev_tree_iter; break; } } if (dev_tree_node == NULL) { - ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (parent_port=%d): not found", parent_port_num); + ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (port %d): not found", port_num); return ESP_ERR_NOT_FOUND; } - ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): device gone", dev_tree_node->uid); + ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid=%d): device gone", port_num, dev_tree_node->uid); hub_event_data_t event_data = { .event = HUB_EVENT_DISCONNECTED, @@ -284,25 +284,25 @@ static esp_err_t dev_tree_node_dev_gone(usb_device_handle_t parent_dev_hdl, uint * * @return esp_err_t */ -static esp_err_t dev_tree_node_remove_by_parent(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num) +static esp_err_t dev_tree_node_remove_by_parent(void* parent, uint8_t port_num) { dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; // Search the device tree nodes list for a device node with the specified parent TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { - if (dev_tree_iter->parent_dev_hdl == parent_dev_hdl && - dev_tree_iter->parent_port_num == parent_port_num) { + if (dev_tree_iter->parent == parent && + dev_tree_iter->port_num == port_num) { dev_tree_node = dev_tree_iter; break; } } if (dev_tree_node == NULL) { - ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (parent_port=%d): not found", parent_port_num); + ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (port %d): not found", port_num); return ESP_ERR_NOT_FOUND; } - ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): freeing", dev_tree_node->uid); + ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid %d): freeing", port_num, dev_tree_node->uid); TAILQ_REMOVE(&p_hub_driver_obj->single_thread.dev_nodes_tailq, dev_tree_node, tailq_entry); heap_caps_free(dev_tree_node); @@ -365,7 +365,7 @@ static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data } } - if (dev_tree_node_new(event_data->connected.parent_dev_hdl, event_data->connected.parent_port_num, port_speed) != ESP_OK) { + if (dev_tree_node_new(ext_hub_hdl, event_data->connected.parent_port_num, port_speed) != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Failed to add new downstream device"); goto new_ds_dev_err; } @@ -374,11 +374,11 @@ static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data ext_hub_port_disable(ext_hub_hdl, event_data->connected.parent_port_num); break; case EXT_PORT_RESET_COMPLETED: - ESP_ERROR_CHECK(dev_tree_node_reset_completed(event_data->reset_completed.parent_dev_hdl, event_data->reset_completed.parent_port_num)); + ESP_ERROR_CHECK(dev_tree_node_reset_completed(ext_hub_hdl, event_data->reset_completed.parent_port_num)); break; case EXT_PORT_DISCONNECTED: // The node could be freed by now, no need to verify the result here - dev_tree_node_dev_gone(event_data->disconnected.parent_dev_hdl, event_data->disconnected.parent_port_num); + dev_tree_node_dev_gone(ext_hub_hdl, event_data->disconnected.parent_port_num); break; default: // Should never occur @@ -691,13 +691,12 @@ esp_err_t hub_node_recycle(unsigned int node_uid) dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); - if (dev_tree_node->parent_dev_hdl == NULL) { + if (dev_tree_node->parent == NULL) { + // Root Hub Ports ret = root_port_recycle(); } else { #if ENABLE_USB_HUBS - ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_recycle(ext_hub_hdl, dev_tree_node->parent_port_num); + ret = ext_hub_port_recycle((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port recycle error: %s", esp_err_to_name(ret)); } @@ -708,7 +707,7 @@ esp_err_t hub_node_recycle(unsigned int node_uid) } if (ret == ESP_OK) { - ESP_ERROR_CHECK(dev_tree_node_remove_by_parent(dev_tree_node->parent_dev_hdl, dev_tree_node->parent_port_num)); + ESP_ERROR_CHECK(dev_tree_node_remove_by_parent(dev_tree_node->parent, dev_tree_node->port_num)); } return ret; @@ -724,7 +723,7 @@ esp_err_t hub_node_reset(unsigned int node_uid) dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); - if (dev_tree_node->parent_dev_hdl == NULL) { + if (dev_tree_node->parent == NULL) { ret = hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_RESET); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Failed to issue root port reset"); @@ -733,9 +732,7 @@ esp_err_t hub_node_reset(unsigned int node_uid) } } else { #if ENABLE_USB_HUBS - ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_reset(ext_hub_hdl, dev_tree_node->parent_port_num); + ret = ext_hub_port_reset((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port reset error: %s", esp_err_to_name(ret)); } @@ -749,20 +746,21 @@ esp_err_t hub_node_reset(unsigned int node_uid) esp_err_t hub_node_active(unsigned int node_uid) { + HUB_DRIVER_ENTER_CRITICAL(); + HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_NOT_ALLOWED); + HUB_DRIVER_EXIT_CRITICAL(); esp_err_t ret; dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); - if (dev_tree_node->parent_dev_hdl == NULL) { + if (dev_tree_node->parent == NULL) { // Root port no need to be activated ret = ESP_OK; } else { #if ENABLE_USB_HUBS // External Hub port - ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_active(ext_hub_hdl, dev_tree_node->parent_port_num); + ret = ext_hub_port_active((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num); if (ret != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Ext hub port activation error: %s", esp_err_to_name(ret)); } @@ -776,19 +774,20 @@ esp_err_t hub_node_active(unsigned int node_uid) esp_err_t hub_node_disable(unsigned int node_uid) { + HUB_DRIVER_ENTER_CRITICAL(); + HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_NOT_ALLOWED); + HUB_DRIVER_EXIT_CRITICAL(); esp_err_t ret; dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); - if (dev_tree_node->parent_dev_hdl == NULL) { + if (dev_tree_node->parent == NULL) { ret = root_port_disable(); } else { #if ENABLE_USB_HUBS // External Hub port - ext_hub_handle_t ext_hub_hdl = NULL; - ext_hub_get_handle(dev_tree_node->parent_dev_hdl, &ext_hub_hdl); - ret = ext_hub_port_disable(ext_hub_hdl, dev_tree_node->parent_port_num); + ret = ext_hub_port_disable((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num); #else ESP_LOGW(HUB_DRIVER_TAG, "Activating External Port is not available (External Hub support disabled)"); ret = ESP_ERR_NOT_SUPPORTED; From d23d9fbfedccc8405c994b126cc7ed904199e3c0 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 10:19:10 +0100 Subject: [PATCH 03/17] refactor(ext_hub): Removed unused calls as hub driver doesn't have parent_dev_hdl --- host/usb/private_include/ext_hub.h | 13 ------------- host/usb/src/ext_hub.c | 8 -------- 2 files changed, 21 deletions(-) diff --git a/host/usb/private_include/ext_hub.h b/host/usb/private_include/ext_hub.h index a5f463e7..e78b219f 100644 --- a/host/usb/private_include/ext_hub.h +++ b/host/usb/private_include/ext_hub.h @@ -82,19 +82,6 @@ void *ext_hub_get_client(void); // -------------------------- External Hub API --------------------------------- -/** - * @brief Get External Hub device handle by USBH device handle - * - * @param[in] dev_hdl USBH device handle - * @param[out] ext_hub_hdl External Hub device handle - * - * @return - * - ESP_OK: External Hub device handle successfully obtained - * - ESP_ERR_INVALID_STATE: External Hub driver is not installed - * - ESP_ERR_NOT_FOUND: Device not found - */ -esp_err_t ext_hub_get_handle(usb_device_handle_t dev_hdl, ext_hub_handle_t *ext_hub_hdl); - /** * @brief Add new device * diff --git a/host/usb/src/ext_hub.c b/host/usb/src/ext_hub.c index 9ba5f7b9..4f804817 100644 --- a/host/usb/src/ext_hub.c +++ b/host/usb/src/ext_hub.c @@ -1215,14 +1215,6 @@ void *ext_hub_get_client(void) // -------------------------- External Hub API --------------------------------- // ----------------------------------------------------------------------------- -esp_err_t ext_hub_get_handle(usb_device_handle_t dev_hdl, ext_hub_handle_t *ext_hub_hdl) -{ - EXT_HUB_ENTER_CRITICAL(); - EXT_HUB_CHECK_FROM_CRIT(p_ext_hub_driver != NULL, ESP_ERR_INVALID_STATE); - EXT_HUB_EXIT_CRITICAL(); - return get_dev_by_hdl(dev_hdl, ext_hub_hdl); -} - static esp_err_t find_first_intf_desc(const usb_config_desc_t *config_desc, device_config_t *hub_config) { bool iface_found = false; From 24e14ffa2b3807a35f258d5f54d69e20af8612d1 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 10:45:48 +0100 Subject: [PATCH 04/17] refactor(ext_hub): Added device speed getter to stop using parent_dev_hdl --- host/usb/private_include/ext_hub.h | 15 ++++++++++++ host/usb/src/ext_hub.c | 37 +++++++++++++++++------------- host/usb/src/hub.c | 9 ++++---- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/host/usb/private_include/ext_hub.h b/host/usb/private_include/ext_hub.h index e78b219f..cd96d5e6 100644 --- a/host/usb/private_include/ext_hub.h +++ b/host/usb/private_include/ext_hub.h @@ -82,6 +82,21 @@ void *ext_hub_get_client(void); // -------------------------- External Hub API --------------------------------- +/** + * @brief Get the External Hub device USB speed + * + * @note Device should be in list of devices + * + * @paramp[in] ext_hub_hdl External Hub device handle + * @param[out] speed USB speed + * @return + * - ESP_ERR_NOT_ALLOWED if the External Hub driver is not installed + * - ESP_ERR_INVALID_ARG if the handle or speed is NULL + * - ESP_ERR_NOT_FOUND if the device is not found + * - ESP_OK if the speed was obtained + */ +esp_err_t ext_hub_get_speed(ext_hub_handle_t ext_hub_hdl, usb_speed_t *speed); + /** * @brief Add new device * diff --git a/host/usb/src/ext_hub.c b/host/usb/src/ext_hub.c index 4f804817..087184d4 100644 --- a/host/usb/src/ext_hub.c +++ b/host/usb/src/ext_hub.c @@ -779,36 +779,27 @@ static void device_free(ext_hub_dev_t *ext_hub_dev) heap_caps_free(ext_hub_dev); } -static esp_err_t get_dev_by_hdl(usb_device_handle_t dev_hdl, ext_hub_dev_t **ext_hub_hdl) +static bool dev_is_in_list(ext_hub_dev_t *ext_hub_dev) { - esp_err_t ret = ESP_OK; - // Go through the Hubs lists to find the hub with the specified device address - ext_hub_dev_t *found_hub = NULL; + bool is_in_list = false; ext_hub_dev_t *hub = NULL; EXT_HUB_ENTER_CRITICAL(); TAILQ_FOREACH(hub, &p_ext_hub_driver->dynamic.ext_hubs_pending_tailq, dynamic.tailq_entry) { - if (hub->constant.dev_hdl == dev_hdl) { - found_hub = hub; + if (hub == ext_hub_dev) { + is_in_list = true; goto exit; } } - TAILQ_FOREACH(hub, &p_ext_hub_driver->dynamic.ext_hubs_tailq, dynamic.tailq_entry) { - if (hub->constant.dev_hdl == dev_hdl) { - found_hub = hub; + if (hub == ext_hub_dev) { + is_in_list = true; goto exit; } } - exit: - if (found_hub == NULL) { - ret = ESP_ERR_NOT_FOUND; - } EXT_HUB_EXIT_CRITICAL(); - - *ext_hub_hdl = found_hub; - return ret; + return is_in_list; } static esp_err_t get_dev_by_addr(uint8_t dev_addr, ext_hub_dev_t **ext_hub_hdl) @@ -1294,6 +1285,20 @@ static esp_err_t find_first_intf_desc(const usb_config_desc_t *config_desc, devi return (iface_found) ? ESP_OK : ESP_ERR_NOT_FOUND; } +esp_err_t ext_hub_get_speed(ext_hub_handle_t ext_hub_hdl, usb_speed_t *speed) +{ + EXT_HUB_ENTER_CRITICAL(); + EXT_HUB_CHECK_FROM_CRIT(p_ext_hub_driver != NULL, ESP_ERR_NOT_ALLOWED); + EXT_HUB_EXIT_CRITICAL(); + EXT_HUB_CHECK(ext_hub_hdl != NULL && speed != NULL, ESP_ERR_INVALID_ARG); + EXT_HUB_CHECK(dev_is_in_list(ext_hub_hdl), ESP_ERR_NOT_FOUND); + ext_hub_dev_t *ext_hub_dev = (ext_hub_dev_t *)ext_hub_hdl; + usb_device_info_t dev_info; + ESP_ERROR_CHECK(usbh_dev_get_info(ext_hub_dev->constant.dev_hdl, &dev_info)); + *speed = dev_info.speed; + return ESP_OK; +} + esp_err_t ext_hub_new_dev(uint8_t dev_addr) { EXT_HUB_ENTER_CRITICAL(); diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index cb2ea041..baa0516b 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -352,11 +352,10 @@ static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data goto new_ds_dev_err; } - // TODO: IDF-10023 Move responsibility of parent-child tree building to Hub Driver instead of USBH - usb_device_info_t parent_dev_info; - ESP_ERROR_CHECK(usbh_dev_get_info(event_data->connected.parent_dev_hdl, &parent_dev_info)); - if (parent_dev_info.speed == USB_SPEED_HIGH) { - if (port_speed != parent_dev_info.speed) { + usb_speed_t parent_speed; + ESP_ERROR_CHECK(ext_hub_get_speed(ext_hub_hdl, &parent_speed)); + if (parent_speed == USB_SPEED_HIGH) { + if (port_speed != parent_speed) { ESP_LOGE(HUB_DRIVER_TAG, "Connected device is %s, transaction translator (TT) is not supported", (char *[]) { "LS", "FS", "HS" From 173aa63f1893fdb9da2e5a01b7e183fbda939730 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 12:24:16 +0100 Subject: [PATCH 05/17] refactor(ext_port): Removed parent_dev_hdl and port_num from event --- host/usb/private_include/ext_port.h | 25 +------------------------ host/usb/src/ext_port.c | 24 +----------------------- host/usb/src/hub.c | 18 +++++++++--------- 3 files changed, 11 insertions(+), 56 deletions(-) diff --git a/host/usb/private_include/ext_port.h b/host/usb/private_include/ext_port.h index 791b58fd..64a17307 100644 --- a/host/usb/private_include/ext_port.h +++ b/host/usb/private_include/ext_port.h @@ -47,29 +47,6 @@ typedef enum { EXT_PORT_DISCONNECTED, /**< Port has a device disconnection event */ } ext_port_event_t; -/** - * @brief Event data object for External Port driver events - */ -typedef struct { - ext_port_event_t event; - union { - struct { - usb_device_handle_t parent_dev_hdl; /**< Ports' parent device handle */ - uint8_t parent_port_num; /**< Ports' parent port number */ - } connected; /**< EXT_PORT_CONNECTED event specific data */ - - struct { - usb_device_handle_t parent_dev_hdl; /**< Ports' parent device handle */ - uint8_t parent_port_num; /**< Ports' parent port number */ - } reset_completed; /**< EXT_PORT_RESET_COMPLETED event specific data */ - - struct { - usb_device_handle_t parent_dev_hdl; /**< Ports' parent device handle */ - uint8_t parent_port_num; /**< Ports' parent port number */ - } disconnected; /**< EXT_PORT_DISCONNECTED event specific data */ - }; -} ext_port_event_data_t; - typedef enum { EXT_PORT_PARENT_REQ_CONTROL = 0x01, /** Port requires action from the parent Hub */ EXT_PORT_PARENT_REQ_PROC_COMPLETED /** All ports were handled */ @@ -101,7 +78,7 @@ typedef void (*ext_port_cb_t)(void *user_arg); * * @note For the Hub Driver only */ -typedef void (*ext_port_event_cb_t)(ext_port_hdl_t port_hdl, ext_port_event_data_t *event_data, void *arg); +typedef void (*ext_port_event_cb_t)(ext_port_hdl_t port_hdl, ext_port_event_t event, void *arg); /** * @brief Callback used to indicate that the External Port driver requires a Hub class specific request diff --git a/host/usb/src/ext_port.c b/host/usb/src/ext_port.c index 603595a9..26157ae2 100644 --- a/host/usb/src/ext_port.c +++ b/host/usb/src/ext_port.c @@ -411,29 +411,7 @@ static void port_stop_handling(ext_port_t *ext_port) */ static void port_event(ext_port_t *ext_port, ext_port_event_t event) { - ext_port_event_data_t event_data = { - .event = event, - }; - switch (event) { - case EXT_PORT_CONNECTED: - event_data.connected.parent_dev_hdl = ext_port->constant.parent_dev_hdl; - event_data.connected.parent_port_num = ext_port->constant.port_num; - break; - case EXT_PORT_RESET_COMPLETED: - event_data.reset_completed.parent_dev_hdl = ext_port->constant.parent_dev_hdl; - event_data.reset_completed.parent_port_num = ext_port->constant.port_num; - break; - case EXT_PORT_DISCONNECTED: - event_data.disconnected.parent_dev_hdl = ext_port->constant.parent_dev_hdl; - event_data.disconnected.parent_port_num = ext_port->constant.port_num; - break; - default: - // Should never occur - abort(); - break; - } - - p_ext_port_driver->constant.event_cb((ext_port_hdl_t)ext_port, &event_data, p_ext_port_driver->constant.event_cb_arg); + p_ext_port_driver->constant.event_cb((ext_port_hdl_t)ext_port, event, p_ext_port_driver->constant.event_cb_arg); } /** diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index baa0516b..daec698a 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -337,18 +337,18 @@ static void ext_port_callback(void *user_arg) p_hub_driver_obj->constant.proc_req_cb(USB_PROC_REQ_SOURCE_HUB, false, p_hub_driver_obj->constant.proc_req_cb_arg); } -static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data_t *event_data, void *arg) +static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_t event, void *arg) { + uint8_t port_num; ext_hub_handle_t ext_hub_hdl = (ext_hub_handle_t) ext_port_get_context(port_hdl); + ESP_ERROR_CHECK(ext_port_get_port_num(port_hdl, &port_num)); - switch (event_data->event) { + switch (event) { case EXT_PORT_CONNECTED: // First reset is done by ext_port logic usb_speed_t port_speed; - if (ext_hub_port_get_speed(ext_hub_hdl, - event_data->connected.parent_port_num, - &port_speed) != ESP_OK) { + if (ext_hub_port_get_speed(ext_hub_hdl, port_num, &port_speed) != ESP_OK) { goto new_ds_dev_err; } @@ -364,20 +364,20 @@ static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data } } - if (dev_tree_node_new(ext_hub_hdl, event_data->connected.parent_port_num, port_speed) != ESP_OK) { + if (dev_tree_node_new(ext_hub_hdl, port_num, port_speed) != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Failed to add new downstream device"); goto new_ds_dev_err; } break; new_ds_dev_err: - ext_hub_port_disable(ext_hub_hdl, event_data->connected.parent_port_num); + ext_hub_port_disable(ext_hub_hdl, port_num); break; case EXT_PORT_RESET_COMPLETED: - ESP_ERROR_CHECK(dev_tree_node_reset_completed(ext_hub_hdl, event_data->reset_completed.parent_port_num)); + ESP_ERROR_CHECK(dev_tree_node_reset_completed(ext_hub_hdl, port_num)); break; case EXT_PORT_DISCONNECTED: // The node could be freed by now, no need to verify the result here - dev_tree_node_dev_gone(ext_hub_hdl, event_data->disconnected.parent_port_num); + dev_tree_node_dev_gone(ext_hub_hdl, port_num); break; default: // Should never occur From 4ae9d2492f17ac8a546cd1671d17c3b3031ceb86 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 12:36:26 +0100 Subject: [PATCH 06/17] change(ext_port_test): Applied new ext_port_event callback --- .../ext_port/main/ext_port_common.c | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/host/usb/test/target_test/ext_port/main/ext_port_common.c b/host/usb/test/target_test/ext_port/main/ext_port_common.c index d1e0df57..7ca8e72e 100644 --- a/host/usb/test/target_test/ext_port/main/ext_port_common.c +++ b/host/usb/test/target_test/ext_port/main/ext_port_common.c @@ -72,6 +72,11 @@ const char *const ext_port_event_string[] = { "-> Disconnected", }; +typedef struct { + ext_port_hdl_t port_hdl; + ext_port_event_t event; +} ext_port_event_msg_t; + typedef struct { ext_port_hdl_t port_hdl; ext_port_parent_request_data_t data; @@ -89,10 +94,13 @@ static void test_ext_port_callback(void *user_arg) xSemaphoreGive(process_req_cb); } -static void test_ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_data_t *event_data, void *arg) +static void test_ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_t event, void *arg) { QueueHandle_t ext_port_event_queue = (QueueHandle_t)arg; - ext_port_event_data_t msg = *event_data; + ext_port_event_msg_t msg = { + .port_hdl = port_hdl, + .event = event, + }; xQueueSend(ext_port_event_queue, &msg, portMAX_DELAY); } @@ -118,17 +126,20 @@ static void test_wait_ext_port_process_request(void) TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(_process_cd_req, pdMS_TO_TICKS(EXT_PORT_PROC_CB_TIMEOUT_MS))); } -static void test_wait_ext_port_event(ext_port_event_t event) +static void test_wait_ext_port_event(ext_port_hdl_t port_hdl, ext_port_event_t event) { + uint8_t port_num; // Get the port event queue from the port's context variable QueueHandle_t ext_port_evt_queue = _ext_port_event_queue; TEST_ASSERT_NOT_NULL(ext_port_evt_queue); // Wait for port callback to send an event message - ext_port_event_data_t msg; + ext_port_event_msg_t msg; BaseType_t ret = xQueueReceive(ext_port_evt_queue, &msg, pdMS_TO_TICKS(EXT_PORT_EVENT_TIMEOUT_MS)); TEST_ASSERT_EQUAL_MESSAGE(pdPASS, ret, "External Hub port event not generated on time"); + TEST_ASSERT_EQUAL_MESSAGE(ESP_OK, ext_port_get_port_num(port_hdl, &port_num), "Port number not equal"); // Check the contents of that event message - printf("\tHub port event: %s\n", ext_port_event_string[msg.event]); + printf("\tHub port %d event: %s\n", port_num, ext_port_event_string[msg.event]); + TEST_ASSERT_EQUAL_MESSAGE(port_hdl, msg.port_hdl, "Unexpected External Hub port handle"); TEST_ASSERT_EQUAL_MESSAGE(event, msg.event, "Unexpected External Hub port event"); } @@ -175,7 +186,7 @@ void test_ext_port_setup(void) _ext_port_hub_req_queue = xQueueCreate(TEST_EXT_PORT_QUEUE_LEN, sizeof(ext_port_hub_request_msg_t)); TEST_ASSERT_NOT_NULL(_ext_port_hub_req_queue); // Create a queue for ext port event - _ext_port_event_queue = xQueueCreate(TEST_EXT_PORT_QUEUE_LEN, sizeof(ext_port_event_data_t)); + _ext_port_event_queue = xQueueCreate(TEST_EXT_PORT_QUEUE_LEN, sizeof(ext_port_event_msg_t)); TEST_ASSERT_NOT_NULL(_ext_port_event_queue); // Install External Port driver ext_port_driver_config_t ext_port_config = { @@ -379,7 +390,7 @@ usb_speed_t test_ext_port_connected(uint8_t port1, ext_port_hdl_t port_hdl) // Process the port TEST_ASSERT_EQUAL(ESP_OK, ext_port_process()); // Processing the port after clear reset should generate the connection event - test_wait_ext_port_event(EXT_PORT_CONNECTED); + test_wait_ext_port_event(port_hdl, EXT_PORT_CONNECTED); // Get device speed usb_speed_t dev_speed; TEST_ASSERT_EQUAL(ESP_OK, port_api->get_speed(port_hdl, &dev_speed)); @@ -440,7 +451,7 @@ void test_ext_port_imitate_disconnection(uint8_t port1, ext_port_hdl_t port_hdl) // Process the port TEST_ASSERT_EQUAL(ESP_OK, ext_port_process()); // The External Port driver should indicate that device has been disconnected and port disconnected - test_wait_ext_port_event(EXT_PORT_DISCONNECTED); + test_wait_ext_port_event(port_hdl, EXT_PORT_DISCONNECTED); } void test_ext_port_disable(uint8_t port1, ext_port_hdl_t port_hdl) @@ -463,7 +474,7 @@ void test_ext_port_disable(uint8_t port1, ext_port_hdl_t port_hdl) EXT_PORT_PARENT_REQ_CONTROL, USB_B_REQUEST_HUB_CLEAR_FEATURE)); // When port is not gone, it should create a DISCONNECTION event - test_wait_ext_port_event(EXT_PORT_DISCONNECTED); + test_wait_ext_port_event(port_hdl, EXT_PORT_DISCONNECTED); // Disable the port hub_port_disable(port1); // After completion, trigger the port processing @@ -500,7 +511,7 @@ void test_ext_port_gone(ext_port_hdl_t port_hdl, test_gone_flag_t flag) // Port is enabled, port gone returns ESP_ERR_NOT_FINISHED TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, port_api->gone(port_hdl)); // Mark the connected port as gone should trigger disconnect event if the port has a device - test_wait_ext_port_event(EXT_PORT_DISCONNECTED); + test_wait_ext_port_event(port_hdl, EXT_PORT_DISCONNECTED); return; } // Port doesn't have a device, port gone returns ESP_OK From ad2f788dbe4bb3c5195277b2de30ba013e379990 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 12:53:20 +0100 Subject: [PATCH 07/17] refactor(ext_port): Removed parent_dev_hdl from the port object --- host/usb/private_include/ext_port.h | 3 +- host/usb/src/ext_hub.c | 3 +- host/usb/src/ext_port.c | 171 ++++++++-------------------- 3 files changed, 47 insertions(+), 130 deletions(-) diff --git a/host/usb/private_include/ext_port.h b/host/usb/private_include/ext_port.h index 64a17307..9a66e903 100644 --- a/host/usb/private_include/ext_port.h +++ b/host/usb/private_include/ext_port.h @@ -106,8 +106,7 @@ typedef struct { */ typedef struct { void *context; /**< Ports' parent external Hub handle */ - usb_device_handle_t parent_dev_hdl; /**< Ports' parent device handle */ - uint8_t parent_port_num; /**< Ports' parent port number */ + uint8_t port_num; /**< Ports' parent port number */ uint16_t port_power_delay_ms; /**< Ports' Power on time to Power Good, ms */ } ext_port_config_t; diff --git a/host/usb/src/ext_hub.c b/host/usb/src/ext_hub.c index 087184d4..31c39408 100644 --- a/host/usb/src/ext_hub.c +++ b/host/usb/src/ext_hub.c @@ -416,8 +416,7 @@ static esp_err_t device_port_new(ext_hub_dev_t *ext_hub_dev, uint8_t port_idx) { ext_port_config_t port_config = { .context = (void *) ext_hub_dev, - .parent_dev_hdl = ext_hub_dev->constant.dev_hdl, - .parent_port_num = port_idx + 1, + .port_num = port_idx + 1, .port_power_delay_ms = ext_hub_dev->constant.hub_desc->bPwrOn2PwrGood * 2, }; diff --git a/host/usb/src/ext_port.c b/host/usb/src/ext_port.c index 26157ae2..1b491a87 100644 --- a/host/usb/src/ext_port.c +++ b/host/usb/src/ext_port.c @@ -74,9 +74,6 @@ struct ext_port_s { uint8_t dev_reset_attempts; /**< Ports' device reset failure */ struct { - // Ports' parent info for optimisation and better debug output - usb_device_handle_t parent_dev_hdl; /**< Ports' parent device handle */ - uint8_t parent_dev_addr; /**< Ports' parent device bus address */ // Port related constant members void *context; /**< Ports' parent External Hub handle */ uint8_t port_num; /**< Ports' parent External Hub Port number */ @@ -388,8 +385,7 @@ static void port_stop_handling(ext_port_t *ext_port) } if (ext_port->action_flags) { // Port should be freed but has actions - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port stop handling. Dropped actions 0x%"PRIx32"", - ext_port->constant.parent_dev_addr, + ESP_LOGD(EXT_PORT_TAG, "Port%d stop handling. Dropped actions 0x%"PRIx32"", ext_port->constant.port_num, ext_port->action_flags); } @@ -429,61 +425,6 @@ static ext_port_t *get_port_from_pending_list(void) return ext_port; } -/** - * @brief Allocates port object - * - * Allocates new por object with following parameters: - * - Port state: USB_PORT_STATE_NOT_CONFIGURED - * - Port device status: PORT_DEV_NOT_PRESENT - * - * @param[in] context Ports' parent hub handle - * @param[in] parent_dev_hdl Ports' parent device handle - * @param[in] parent_port_num Ports' parent port number - * @param[in] port_delay_ms Ports' Power on time to Power Good, ms - * @param[out] port_obj Port objects' pointer - * @return - * - ESP_ERR_INVALID_ARG: Unable to allocate the port object: parent hub handle and parent device handle must be not NULL - * - ESP_ERR_NO_MEM: Unable to allocate the port object: no memory - * - ESP_ERR_NOT_FINISHED: Unable to allocate the port object: parent device is not available - * - ESP_OK: Port object created successfully - */ -static esp_err_t port_alloc(void *context, usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num, uint16_t port_delay_ms, ext_port_t **port_obj) -{ - uint8_t parent_dev_addr = 0; - EXT_PORT_CHECK(context != NULL && parent_dev_hdl != NULL, ESP_ERR_INVALID_ARG); - // This is the one exception from the requirement to use only the Ext Hub Driver API. - // TODO: IDF-10023 Move responsibility of parent-child tree building to Hub Driver instead of USBH - EXT_PORT_CHECK(usbh_dev_get_addr(parent_dev_hdl, &parent_dev_addr) == ESP_OK, ESP_ERR_NOT_FINISHED); - - ext_port_t *ext_port = heap_caps_calloc(1, sizeof(ext_port_t), MALLOC_CAP_DEFAULT); - if (ext_port == NULL) { - return ESP_ERR_NO_MEM; - } - - ext_port->constant.parent_dev_hdl = parent_dev_hdl; - ext_port->constant.parent_dev_addr = parent_dev_addr; - ext_port->constant.context = context; - ext_port->constant.port_num = parent_port_num; -#if (EXT_PORT_POWER_ON_CUSTOM_DELAY) - ext_port->constant.power_on_delay_ms = EXT_PORT_POWER_ON_CUSTOM_DELAY_MS; -#else - // We don't need any additional delay in case port_delay_ms == 0, because this usually means - // that parent Hub device has no power switches - ext_port->constant.power_on_delay_ms = port_delay_ms; -#endif // EXT_PORT_POWER_ON_CUSTOM_DELAY - - ext_port->state = USB_PORT_STATE_NOT_CONFIGURED; - ext_port->dev_state = PORT_DEV_NOT_PRESENT; - - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port has been added (PwrOn2PwrGood=%d ms)", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num, - ext_port->constant.power_on_delay_ms); - - *port_obj = ext_port; - return ESP_OK; -} - /** * @brief Port object handling complete * @@ -503,8 +444,7 @@ static esp_err_t handle_complete(ext_port_t *ext_port) bool all_ports_were_handled = true; bool has_pending_ports = false; - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port is %s", - ext_port->constant.parent_dev_addr, + ESP_LOGD(EXT_PORT_TAG, "Port%d is %s", ext_port->constant.port_num, ext_port->flags.is_gone ? "gone" : (ext_port->dev_state == PORT_DEV_PRESENT) ? "active" : "idle"); @@ -571,9 +511,7 @@ static bool handle_port_status(ext_port_t *ext_port) { bool need_processing = false; if (port_is_in_reset(ext_port)) { - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port still in reset, wait and repeat get status...", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGD(EXT_PORT_TAG, "Port%d still in reset, wait and repeat get status...", ext_port->constant.port_num); port_request_status(ext_port); need_processing = true; } @@ -601,9 +539,7 @@ static void handle_port_connection(ext_port_t *ext_port) case USB_PORT_STATE_DISABLED: if (port_has_connection(ext_port)) { if (ext_port->dev_state == PORT_DEV_PRESENT) { - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Mismatch port state and device status", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGE(EXT_PORT_TAG, "Port%d mismatch state and device status", ext_port->constant.port_num); has_device = true; } else { // New device connected, flush reset attempts @@ -617,9 +553,7 @@ static void handle_port_connection(ext_port_t *ext_port) case USB_PORT_STATE_RESETTING: if (!port_has_connection(ext_port)) { if (ext_port->dev_state == PORT_DEV_PRESENT) { - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Failed to issue downstream port reset", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGE(EXT_PORT_TAG, "Port%d failed to issue reset", ext_port->constant.port_num); has_device = true; } else { ext_port->state = USB_PORT_STATE_DISCONNECTED; @@ -662,8 +596,7 @@ static bool handle_port_changes(ext_port_t *ext_port) need_processing = true; } else if (port_has_changed_from_enable(ext_port)) { // For more information, refer to section 11.8.1 Port Error of usb_2.0 specification - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Error on port (state=%d, dev=%d)", - ext_port->constant.parent_dev_addr, + ESP_LOGE(EXT_PORT_TAG, "Port%d error: state=%d, dev=%d", ext_port->constant.port_num, ext_port->state, ext_port->dev_state == PORT_DEV_PRESENT); @@ -711,9 +644,7 @@ static void handle_port_state(ext_port_t *ext_port) port_set_feature(ext_port, USB_FEATURE_PORT_RESET); need_handling = true; } else { - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Unable to reset the device", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGE(EXT_PORT_TAG, "Port%d unable to reset the device", ext_port->constant.port_num); } } break; @@ -744,9 +675,7 @@ static void handle_port_state(ext_port_t *ext_port) } else { // Port in resetting state and doesn't have connection // Error case, could be, when device was removed during port reset - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Device gone during port reset, recover port", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGE(EXT_PORT_TAG, "Port%d device gone during reset, recover port", ext_port->constant.port_num); new_state = USB_PORT_STATE_DISCONNECTED; } break; @@ -772,9 +701,7 @@ static void handle_port_state(ext_port_t *ext_port) } need_handling = true; } else { - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Enabled, but doesn't have connection", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGE(EXT_PORT_TAG, "Port%d enabled, but doesn't have connection", ext_port->constant.port_num); } } else { // If port was enabled, there should be an active device @@ -784,9 +711,7 @@ static void handle_port_state(ext_port_t *ext_port) ext_port->flags.waiting_recycle = 1; } else { // Error state - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Enabled, but doesn't have a device", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGE(EXT_PORT_TAG, "Port%d enabled, but doesn't have a device", ext_port->constant.port_num); new_state = USB_PORT_STATE_DISCONNECTED; } } @@ -798,7 +723,7 @@ static void handle_port_state(ext_port_t *ext_port) } if (curr_state != new_state) { - ESP_LOGD(EXT_PORT_TAG, "New state: %d", new_state); + ESP_LOGD(EXT_PORT_TAG, "Port%d state change: %d->%d", ext_port->constant.port_num, curr_state, new_state); ext_port->state = new_state; } @@ -821,11 +746,10 @@ static void handle_port_state(ext_port_t *ext_port) */ static void handle_port(ext_port_t *ext_port) { - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] change=0x%04x, status=0x%04x, state=%d", - ext_port->constant.parent_dev_addr, + ESP_LOGD(EXT_PORT_TAG, "Port%d [%04x.%04x], state=%d", ext_port->constant.port_num, - ext_port->status.wPortChange.val, ext_port->status.wPortStatus.val, + ext_port->status.wPortChange.val, ext_port->state); if (handle_port_changes(ext_port)) { @@ -850,8 +774,7 @@ static void handle_recycle(ext_port_t *ext_port) { assert(ext_port->flags.waiting_recycle == 1); - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port recycle (state=%d, dev=%d)", - ext_port->constant.parent_dev_addr, + ESP_LOGD(EXT_PORT_TAG, "Port%d recycle (state=%d, dev=%d)", ext_port->constant.port_num, ext_port->state, ext_port->dev_state); @@ -890,8 +813,7 @@ static void handle_recycle(ext_port_t *ext_port) */ static void handle_disable(ext_port_t *ext_port) { - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Disable (state=%d, dev=%d)", - ext_port->constant.parent_dev_addr, + ESP_LOGD(EXT_PORT_TAG, "Port%d disable (state=%d, dev=%d)", ext_port->constant.port_num, ext_port->state, ext_port->dev_state); @@ -902,8 +824,7 @@ static void handle_disable(ext_port_t *ext_port) if (ext_port->state == USB_PORT_STATE_ENABLED) { if (port_has_connection(ext_port)) { - ESP_LOGE(EXT_PORT_TAG, "[%d:%d] Port disabled, reset attempts=%d", - ext_port->constant.parent_dev_addr, + ESP_LOGE(EXT_PORT_TAG, "Port%d disabled, reset attempts=%d", ext_port->constant.port_num, ext_port->dev_reset_attempts); @@ -950,23 +871,33 @@ static esp_err_t port_new(void *port_cfg, void **port_hdl) EXT_PORT_CHECK(p_ext_port_driver != NULL, ESP_ERR_INVALID_STATE); EXT_PORT_CHECK(port_cfg != NULL && port_hdl != NULL, ESP_ERR_INVALID_ARG); - ext_port_t *port = NULL; ext_port_config_t *config = (ext_port_config_t *)port_cfg; - esp_err_t ret = port_alloc(config->context, - config->parent_dev_hdl, - config->parent_port_num, - config->port_power_delay_ms, - &port); - if (ret != ESP_OK) { - *port_hdl = NULL; - goto exit; + ext_port_t *ext_port = heap_caps_calloc(1, sizeof(ext_port_t), MALLOC_CAP_DEFAULT); + if (ext_port == NULL) { + return ESP_ERR_NO_MEM; } - port_set_actions(port, PORT_ACTION_HANDLE); - *port_hdl = (ext_port_hdl_t) port; -exit: - return ret; + ext_port->constant.context = config->context; + ext_port->constant.port_num = config->port_num; +#if (EXT_PORT_POWER_ON_CUSTOM_DELAY) + ext_port->constant.power_on_delay_ms = EXT_PORT_POWER_ON_CUSTOM_DELAY_MS; +#else + // We don't need any additional delay in case port_delay_ms == 0, because this usually means + // that parent Hub device has no power switches + ext_port->constant.power_on_delay_ms = config->port_power_delay_ms; +#endif // EXT_PORT_POWER_ON_CUSTOM_DELAY + + ext_port->state = USB_PORT_STATE_NOT_CONFIGURED; + ext_port->dev_state = PORT_DEV_NOT_PRESENT; + + ESP_LOGD(EXT_PORT_TAG, "Port%d has been added (PwrOn2PwrGood=%d ms)", + ext_port->constant.port_num, + ext_port->constant.power_on_delay_ms); + + port_set_actions(ext_port, PORT_ACTION_HANDLE); + *port_hdl = (ext_port_hdl_t) ext_port; + return ESP_OK; } /** @@ -1016,9 +947,7 @@ static esp_err_t port_reset(void *port_hdl) EXT_PORT_CHECK(port_has_connection(ext_port), ESP_ERR_INVALID_STATE); - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port reset request", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGD(EXT_PORT_TAG, "Port%d reset request", ext_port->constant.port_num); // Reset can be triggered only when port is enabled EXT_PORT_CHECK(ext_port->state == USB_PORT_STATE_ENABLED, ESP_ERR_INVALID_STATE); @@ -1084,9 +1013,7 @@ static esp_err_t port_active(void *port_hdl) EXT_PORT_CHECK(port_hdl != NULL, ESP_ERR_INVALID_ARG); ext_port_t *ext_port = (ext_port_t *) port_hdl; - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port has an enumerated device", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGD(EXT_PORT_TAG, "Port%d has an enumerated device", ext_port->constant.port_num); ext_port->flags.has_enum_device = 1; @@ -1111,9 +1038,7 @@ static esp_err_t port_disable(void *port_hdl) EXT_PORT_CHECK(port_hdl != NULL, ESP_ERR_INVALID_ARG); ext_port_t *ext_port = (ext_port_t *) port_hdl; - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Disable", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGD(EXT_PORT_TAG, "Port%d disable", ext_port->constant.port_num); EXT_PORT_CHECK(ext_port->state == USB_PORT_STATE_ENABLED, ESP_ERR_INVALID_STATE); @@ -1141,9 +1066,7 @@ static esp_err_t port_delete(void *port_hdl) assert(ext_port->dev_state == PORT_DEV_NOT_PRESENT); // Port should not have a device assert(ext_port->flags.in_pending_list == 0); // Port should not be in pending list - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Freeing", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num); + ESP_LOGD(EXT_PORT_TAG, "Port%d freeing", ext_port->constant.port_num); heap_caps_free(ext_port); @@ -1168,8 +1091,7 @@ static esp_err_t port_gone(void *port_hdl) EXT_PORT_CHECK(port_hdl != NULL, ESP_ERR_INVALID_ARG); ext_port_t *ext_port = (ext_port_t *)port_hdl; - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port is gone (state=%d, dev=%d)", - ext_port->constant.parent_dev_addr, + ESP_LOGD(EXT_PORT_TAG, "Port%d is gone (state=%d, dev=%d)", ext_port->constant.port_num, ext_port->state, ext_port->dev_state); @@ -1368,10 +1290,7 @@ esp_err_t ext_port_process(void) while (action_flags) { // Keep processing until all port's action have been handled - ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Processing actions 0x%"PRIx32"", - ext_port->constant.parent_dev_addr, - ext_port->constant.port_num, - action_flags); + ESP_LOGD(EXT_PORT_TAG, "Port%d actions 0x%"PRIx32"", ext_port->constant.port_num, action_flags); if (action_flags & PORT_ACTION_HANDLE) { handle_port(ext_port); From 3e1333404bf18b0edd6b1830602f91b2bb3cf857 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 12:56:41 +0100 Subject: [PATCH 08/17] change(ext_port_test): Applied new port alloc configuration (without parent_dev_hdl) --- .../target_test/ext_port/main/test_ext_port.c | 45 ++++++++----------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/host/usb/test/target_test/ext_port/main/test_ext_port.c b/host/usb/test/target_test/ext_port/main/test_ext_port.c index a81fe5bb..4cbd95a3 100644 --- a/host/usb/test/target_test/ext_port/main/test_ext_port.c +++ b/host/usb/test/target_test/ext_port/main/test_ext_port.c @@ -92,9 +92,8 @@ TEST_CASE("Port: disconnected", "[low_speed][full_speed][high_speed]") TEST_ASSERT_TRUE(TEST_PORT_NUM_EMPTY <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_EMPTY, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_EMPTY, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -136,9 +135,8 @@ TEST_CASE("Port: enumerate child device Low-speed", "[ext_port][low_speed]") TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_LS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_LS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_LS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -193,9 +191,8 @@ TEST_CASE("Port: enumerate child device Full-speed", "[ext_port][full_speed]") TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_FSHS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_FSHS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_FSHS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -250,9 +247,8 @@ TEST_CASE("Port: enumerate child device High-speed", "[ext_port][high_speed]") TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_FSHS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_FSHS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_FSHS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -307,9 +303,8 @@ TEST_CASE("Port: recycle", "[ext_port][full_speed][high_speed]") TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_FSHS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_FSHS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_FSHS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -352,9 +347,8 @@ TEST_CASE("Port: recycle when port is gone", "[ext_port][low_speed][full_speed][ TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_FSHS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_FSHS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_FSHS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -397,9 +391,8 @@ TEST_CASE("Port: disable", "[ext_port][full_speed][high_speed]") TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_FSHS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_FSHS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_FSHS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -440,9 +433,8 @@ TEST_CASE("Port: gone in state - powered on", "[ext_port][full_speed][high_speed TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_FSHS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_FSHS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_FSHS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); @@ -478,9 +470,8 @@ TEST_CASE("Port: gone in state - enabled", "[ext_port][full_speed][high_speed]") TEST_ASSERT_TRUE(TEST_PORT_NUM_DEVICE_FSHS <= port_num); // Create External Port ext_port_config_t port_config = { - .context = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_dev_hdl = (void *) hub_get_context() /* use any before IDF-10023 */, - .parent_port_num = TEST_PORT_NUM_DEVICE_FSHS, + .context = (void *) hub_get_context(), + .port_num = TEST_PORT_NUM_DEVICE_FSHS, .port_power_delay_ms = hub_get_port_poweron_delay_ms(), }; ext_port_hdl_t port_hdl = test_ext_port_new(&port_config); From c4d4297567ec372dc4a3198d1d9ea927463bf7b2 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 14:00:43 +0100 Subject: [PATCH 09/17] refactor(usbh): Moved parent information from usbh to hub driver --- host/usb/include/usb/usb_types_stack.h | 8 +++--- host/usb/private_include/ext_hub.h | 15 +++++++++++ host/usb/private_include/hub.h | 16 ++++++++++++ host/usb/private_include/usbh.h | 29 +++++++++++---------- host/usb/src/ext_hub.c | 12 +++++++++ host/usb/src/hub.c | 32 +++++++++++++++++++---- host/usb/src/usb_host.c | 9 +++++++ host/usb/src/usbh.c | 35 +++++++++----------------- 8 files changed, 110 insertions(+), 46 deletions(-) diff --git a/host/usb/include/usb/usb_types_stack.h b/host/usb/include/usb/usb_types_stack.h index bb2c2906..002dc0c6 100644 --- a/host/usb/include/usb/usb_types_stack.h +++ b/host/usb/include/usb/usb_types_stack.h @@ -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 */ @@ -74,8 +74,10 @@ typedef bool (*usb_host_enum_filter_cb_t)(const usb_device_desc_t *dev_desc, uin * @brief Parent device information */ typedef struct { - usb_device_handle_t dev_hdl; /**< Device's parent handle */ - uint8_t port_num; /**< Device's parent port number */ + // IDF-12502: Remove the parent dev_hdl from parent_info structure (breaking change) + usb_device_handle_t dev_hdl __attribute__((deprecated)); /**< Device's parent handle: Deprecated, as dev_hdl only valid when device was opened via usbh_devs_open() */ + uint8_t dev_addr; /**< Device's parent bus address */ + uint8_t port_num; /**< Device's parent port number */ } usb_parent_dev_info_t; /** diff --git a/host/usb/private_include/ext_hub.h b/host/usb/private_include/ext_hub.h index cd96d5e6..42e1e1d5 100644 --- a/host/usb/private_include/ext_hub.h +++ b/host/usb/private_include/ext_hub.h @@ -97,6 +97,21 @@ void *ext_hub_get_client(void); */ esp_err_t ext_hub_get_speed(ext_hub_handle_t ext_hub_hdl, usb_speed_t *speed); +/** + * @brief Get the External Hub device USB address + * + * @note Device should be in list of devices + * + * @paramp[in] ext_hub_hdl External Hub device handle + * @param[out] addr USB address + * @return + * - ESP_ERR_NOT_ALLOWED if the External Hub driver is not installed + * - ESP_ERR_INVALID_ARG if the handle or addr is NULL + * - ESP_ERR_NOT_FOUND if the device is not found + * - ESP_OK if the address was obtained + */ +esp_err_t ext_hub_get_dev_addr(ext_hub_handle_t ext_hub_hdl, uint8_t *addr); + /** * @brief Add new device * diff --git a/host/usb/private_include/hub.h b/host/usb/private_include/hub.h index e3fe727f..41bd9ebb 100644 --- a/host/usb/private_include/hub.h +++ b/host/usb/private_include/hub.h @@ -193,6 +193,22 @@ esp_err_t hub_node_active(unsigned int node_uid); */ esp_err_t hub_node_disable(unsigned int node_uid); +/** + * @brief Get the node information of the device + * + * @note This function should only be called from the Host Library task + * + * @param[in] node_uid Device's node unique ID + * @param[out] info Device information + * + * @return + * - ESP_ERR_NOT_ALLOWED if the Hub driver is not installed + * - ESP_ERR_INVALID_ARG if the info pointer is NULL + * - ESP_ERR_NOT_FOUND if the device node is not found + * - ESP_OK if Device's information obtained successfully + */ +esp_err_t hub_node_get_info(unsigned int node_uid, usb_parent_dev_info_t *info); + /** * @brief Notify Hub driver that new device has been attached * diff --git a/host/usb/private_include/usbh.h b/host/usb/private_include/usbh.h index a07ff447..daa942f0 100644 --- a/host/usb/private_include/usbh.h +++ b/host/usb/private_include/usbh.h @@ -269,21 +269,6 @@ esp_err_t usbh_devs_add(usbh_dev_params_t *params); */ esp_err_t usbh_devs_remove(unsigned int uid); -/** - * @brief Get a device's connection information - * - * @note Can be called without opening the device - * - * @param[in] uid Unique ID assigned to the device - * @param[out] parent_info Parent device handle - * - * @return - * - ESP_OK: Device parent info obtained successfully - * - ESP_ERR_INVALID_ARG: Invalid argument - * - ESP_ERR_NOT_FOUND: Device with provided uid not found - */ -esp_err_t usbh_devs_get_parent_info(unsigned int uid, usb_parent_dev_info_t *parent_info); - /** * @brief Mark that all devices should be freed at the next possible opportunity * @@ -342,6 +327,20 @@ esp_err_t usbh_devs_new_dev_event(usb_device_handle_t dev_hdl); esp_err_t usbh_dev_close(usb_device_handle_t dev_hdl); // ------------------------------ Getters -------------------------------------- +/** + * @brief Get a device's UID + * + * @note Callers of this function must have opened the device via usbh_devs_open() + * + * @param[in] dev_hdl Device handle + * @param[out] uid Device's UID + * + * @return + * - ESP_ERR_INVALID_ARG if invalid argument + * - ESP_OK if Device's UID obtained successfully + */ +esp_err_t usbh_dev_get_uid(usb_device_handle_t dev_hdl, unsigned int *uid); + /** * @brief Get a device's address * diff --git a/host/usb/src/ext_hub.c b/host/usb/src/ext_hub.c index 31c39408..08647a2e 100644 --- a/host/usb/src/ext_hub.c +++ b/host/usb/src/ext_hub.c @@ -1298,6 +1298,18 @@ esp_err_t ext_hub_get_speed(ext_hub_handle_t ext_hub_hdl, usb_speed_t *speed) return ESP_OK; } +esp_err_t ext_hub_get_dev_addr(ext_hub_handle_t ext_hub_hdl, uint8_t *addr) +{ + EXT_HUB_ENTER_CRITICAL(); + EXT_HUB_CHECK_FROM_CRIT(p_ext_hub_driver != NULL, ESP_ERR_NOT_ALLOWED); + EXT_HUB_EXIT_CRITICAL(); + EXT_HUB_CHECK(ext_hub_hdl != NULL && addr != NULL, ESP_ERR_INVALID_ARG); + EXT_HUB_CHECK(dev_is_in_list(ext_hub_hdl), ESP_ERR_NOT_FOUND); + ext_hub_dev_t *ext_hub_dev = (ext_hub_dev_t *)ext_hub_hdl; + *addr = ext_hub_dev->constant.dev_addr; + return ESP_OK; +} + esp_err_t ext_hub_new_dev(uint8_t dev_addr) { EXT_HUB_ENTER_CRITICAL(); diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index daec698a..cdd7037e 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -70,7 +70,7 @@ typedef enum { struct dev_tree_node_s { TAILQ_ENTRY(dev_tree_node_s) tailq_entry; /**< Entry for the device tree node object tailq */ unsigned int uid; /**< Device's unique ID */ - void* parent; /**< Device's parent context */ + void *parent; /**< Device's parent context */ uint8_t port_num; /**< Device's parent port number */ }; @@ -164,7 +164,7 @@ static dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid) * * @return esp_err_t */ -static esp_err_t dev_tree_node_new(void* parent, uint8_t port_num, usb_speed_t speed) +static esp_err_t dev_tree_node_new(void *parent, uint8_t port_num, usb_speed_t speed) { esp_err_t ret; // Allocate memory for a new device tree node @@ -220,7 +220,7 @@ static esp_err_t dev_tree_node_new(void* parent, uint8_t port_num, usb_speed_t s return ret; } -static esp_err_t dev_tree_node_reset_completed(void* parent, uint8_t port_num) +static esp_err_t dev_tree_node_reset_completed(void *parent, uint8_t port_num) { dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; @@ -248,7 +248,7 @@ static esp_err_t dev_tree_node_reset_completed(void* parent, uint8_t port_num) return ESP_OK; } -static esp_err_t dev_tree_node_dev_gone(void* parent, uint8_t port_num) +static esp_err_t dev_tree_node_dev_gone(void *parent, uint8_t port_num) { dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; @@ -284,7 +284,7 @@ static esp_err_t dev_tree_node_dev_gone(void* parent, uint8_t port_num) * * @return esp_err_t */ -static esp_err_t dev_tree_node_remove_by_parent(void* parent, uint8_t port_num) +static esp_err_t dev_tree_node_remove_by_parent(void *parent, uint8_t port_num) { dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; @@ -795,6 +795,28 @@ esp_err_t hub_node_disable(unsigned int node_uid) return ret; } +esp_err_t hub_node_get_info(unsigned int node_uid, usb_parent_dev_info_t *parent_info) +{ + HUB_DRIVER_ENTER_CRITICAL(); + HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_NOT_ALLOWED); + HUB_DRIVER_EXIT_CRITICAL(); + HUB_DRIVER_CHECK(parent_info != NULL, ESP_ERR_INVALID_ARG); + + dev_tree_node_t *dev_tree_node = dev_tree_node_get_by_uid(node_uid); + HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); + uint8_t parent_dev_addr = 0; +#if (ENABLE_USB_HUBS) + if (dev_tree_node->parent) { + ESP_ERROR_CHECK(ext_hub_get_dev_addr((ext_hub_handle_t) dev_tree_node->parent, &parent_dev_addr)); + } +#endif // ENABLE_USB_HUBS + + memset(parent_info, 0, sizeof(usb_parent_dev_info_t)); + parent_info->dev_addr = parent_dev_addr; + parent_info->port_num = dev_tree_node->port_num; + return ESP_OK; +} + esp_err_t hub_dev_new(uint8_t dev_addr) { HUB_DRIVER_ENTER_CRITICAL(); diff --git a/host/usb/src/usb_host.c b/host/usb/src/usb_host.c index 656dd5da..0476a1af 100644 --- a/host/usb/src/usb_host.c +++ b/host/usb/src/usb_host.c @@ -1093,7 +1093,16 @@ esp_err_t usb_host_device_addr_list_fill(int list_len, uint8_t *dev_addr_list, i esp_err_t usb_host_device_info(usb_device_handle_t dev_hdl, usb_device_info_t *dev_info) { + unsigned int uid; HOST_CHECK(dev_hdl != NULL && dev_info != NULL, ESP_ERR_INVALID_ARG); + esp_err_t ret; + + HOST_CHECK(usbh_dev_get_uid(dev_hdl, &uid) == ESP_OK, ESP_ERR_INVALID_STATE); // Should never fail + ret = hub_node_get_info(uid, &dev_info->parent); + if (ret != ESP_OK) { + ESP_LOGE(USB_HOST_TAG, "Unable to get dev tree node info, parent information not available"); + return ret; + } return usbh_dev_get_info(dev_hdl, dev_info); } diff --git a/host/usb/src/usbh.c b/host/usb/src/usbh.c index cb047b42..f45a0e09 100644 --- a/host/usb/src/usbh.c +++ b/host/usb/src/usbh.c @@ -929,27 +929,6 @@ esp_err_t usbh_devs_remove(unsigned int uid) return ret; } -esp_err_t usbh_devs_get_parent_info(unsigned int uid, usb_parent_dev_info_t *parent_info) -{ - USBH_CHECK(parent_info, ESP_ERR_INVALID_ARG); - esp_err_t ret = ESP_FAIL; - device_t *dev_obj = NULL; - - USBH_ENTER_CRITICAL(); - dev_obj = _find_dev_from_uid(uid); - if (dev_obj == NULL) { - ret = ESP_ERR_NOT_FOUND; - goto exit; - } else { - parent_info->dev_hdl = dev_obj->constant.parent_dev_hdl; - parent_info->port_num = dev_obj->constant.parent_port_num; - ret = ESP_OK; - } -exit: - USBH_EXIT_CRITICAL(); - return ret; -} - esp_err_t usbh_devs_mark_all_free(void) { USBH_ENTER_CRITICAL(); @@ -1075,6 +1054,18 @@ esp_err_t usbh_dev_close(usb_device_handle_t dev_hdl) // ---------------------------- Getters ---------------------------------------- // ----------------------------------------------------------------------------- +esp_err_t usbh_dev_get_uid(usb_device_handle_t dev_hdl, unsigned int *uid) +{ + USBH_CHECK(dev_hdl != NULL && uid != NULL, ESP_ERR_INVALID_ARG); + device_t *dev_obj = (device_t *)dev_hdl; + + USBH_ENTER_CRITICAL(); + *uid = dev_obj->constant.uid; + USBH_EXIT_CRITICAL(); + + return ESP_OK; +} + esp_err_t usbh_dev_get_addr(usb_device_handle_t dev_hdl, uint8_t *dev_addr) { USBH_CHECK(dev_hdl != NULL && dev_addr != NULL, ESP_ERR_INVALID_ARG); @@ -1092,8 +1083,6 @@ esp_err_t usbh_dev_get_info(usb_device_handle_t dev_hdl, usb_device_info_t *dev_ USBH_CHECK(dev_hdl != NULL && dev_info != NULL, ESP_ERR_INVALID_ARG); device_t *dev_obj = (device_t *)dev_hdl; - dev_info->parent.dev_hdl = dev_obj->constant.parent_dev_hdl; - dev_info->parent.port_num = dev_obj->constant.parent_port_num; dev_info->speed = dev_obj->constant.speed; dev_info->dev_addr = dev_obj->constant.address; // Device descriptor might not have been set yet From 838c260be46d2b41d02a94897b9240ffbd29650a Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 14:11:43 +0100 Subject: [PATCH 10/17] refactor(hub): Moved list to dymanic member as the get parent info could come any moment --- host/usb/src/hub.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index cdd7037e..f0b801d7 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -87,11 +87,8 @@ typedef struct { } flags; /**< Hub flags */ root_port_state_t root_port_state; /**< Root port state */ unsigned int port_reqs; /**< Root port request flag */ - } dynamic; /**< Dynamic members. Require a critical section */ - - struct { TAILQ_HEAD(tailhead_devs, dev_tree_node_s) dev_nodes_tailq; /**< Tailq of attached devices */ - } single_thread; /**< Single thread members don't require a critical section so long as they are never accessed from multiple threads */ + } dynamic; /**< Dynamic members. Require a critical section */ struct { hcd_port_handle_t root_port_hdl; /**< Root port HCD handle */ @@ -150,12 +147,14 @@ static dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid) dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; // Search the device tree nodes list for a device node with the specified parent - TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { + HUB_DRIVER_ENTER_CRITICAL(); + TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->dynamic.dev_nodes_tailq, tailq_entry) { if (dev_tree_iter->uid == node_uid) { dev_tree_node = dev_tree_iter; break; } } + HUB_DRIVER_EXIT_CRITICAL(); return dev_tree_node; } @@ -200,9 +199,9 @@ static esp_err_t dev_tree_node_new(void *parent, uint8_t port_num, usb_speed_t s // Device registration may fail if there are no available HCD channels. goto fail; } - - TAILQ_INSERT_TAIL(&p_hub_driver_obj->single_thread.dev_nodes_tailq, dev_tree_node, tailq_entry); - + HUB_DRIVER_ENTER_CRITICAL(); + TAILQ_INSERT_TAIL(&p_hub_driver_obj->dynamic.dev_nodes_tailq, dev_tree_node, tailq_entry); + HUB_DRIVER_EXIT_CRITICAL(); ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid %d): new", dev_tree_node->port_num, dev_tree_node->uid); hub_event_data_t event_data = { @@ -225,13 +224,15 @@ static esp_err_t dev_tree_node_reset_completed(void *parent, uint8_t port_num) dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; // Search the device tree nodes list for a device node with the specified parent - TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { + HUB_DRIVER_ENTER_CRITICAL(); + TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->dynamic.dev_nodes_tailq, tailq_entry) { if (dev_tree_iter->parent == parent && dev_tree_iter->port_num == port_num) { dev_tree_node = dev_tree_iter; break; } } + HUB_DRIVER_EXIT_CRITICAL(); if (dev_tree_node == NULL) { ESP_LOGE(HUB_DRIVER_TAG, "Reset completed, but device tree node (port %d) not found", port_num); @@ -253,13 +254,15 @@ static esp_err_t dev_tree_node_dev_gone(void *parent, uint8_t port_num) dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; // Search the device tree nodes list for a device node with the specified parent - TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { + HUB_DRIVER_ENTER_CRITICAL(); + TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->dynamic.dev_nodes_tailq, tailq_entry) { if (dev_tree_iter->parent == parent && dev_tree_iter->port_num == port_num) { dev_tree_node = dev_tree_iter; break; } } + HUB_DRIVER_EXIT_CRITICAL(); if (dev_tree_node == NULL) { ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (port %d): not found", port_num); @@ -289,13 +292,18 @@ static esp_err_t dev_tree_node_remove_by_parent(void *parent, uint8_t port_num) dev_tree_node_t *dev_tree_node = NULL; dev_tree_node_t *dev_tree_iter; // Search the device tree nodes list for a device node with the specified parent - TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->single_thread.dev_nodes_tailq, tailq_entry) { + HUB_DRIVER_ENTER_CRITICAL(); + TAILQ_FOREACH(dev_tree_iter, &p_hub_driver_obj->dynamic.dev_nodes_tailq, tailq_entry) { if (dev_tree_iter->parent == parent && dev_tree_iter->port_num == port_num) { dev_tree_node = dev_tree_iter; break; } } + if (dev_tree_node) { + TAILQ_REMOVE(&p_hub_driver_obj->dynamic.dev_nodes_tailq, dev_tree_node, tailq_entry); + } + HUB_DRIVER_EXIT_CRITICAL(); if (dev_tree_node == NULL) { ESP_LOGW(HUB_DRIVER_TAG, "Device tree node (port %d): not found", port_num); @@ -303,8 +311,6 @@ static esp_err_t dev_tree_node_remove_by_parent(void *parent, uint8_t port_num) } ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (port %d, uid %d): freeing", port_num, dev_tree_node->uid); - - TAILQ_REMOVE(&p_hub_driver_obj->single_thread.dev_nodes_tailq, dev_tree_node, tailq_entry); heap_caps_free(dev_tree_node); return ESP_OK; } @@ -596,8 +602,8 @@ esp_err_t hub_install(hub_config_t *hub_config, void **client_ret) hub_driver_obj->constant.proc_req_cb_arg = hub_config->proc_req_cb_arg; hub_driver_obj->constant.event_cb = hub_config->event_cb; hub_driver_obj->constant.event_cb_arg = hub_config->event_cb_arg; - TAILQ_INIT(&hub_driver_obj->single_thread.dev_nodes_tailq); // Driver is not installed, we can modify dynamic section outside of the critical section + TAILQ_INIT(&hub_driver_obj->dynamic.dev_nodes_tailq); hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_NOT_POWERED; HUB_DRIVER_ENTER_CRITICAL(); From e928ef9a2707fea0eb01179f91d64172e471615f Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 7 Mar 2025 14:03:21 +0100 Subject: [PATCH 11/17] refactor(usbh): Cleaned up parent_dev_hdl from the usbh object --- host/usb/private_include/usbh.h | 4 ---- host/usb/src/hub.c | 3 --- host/usb/src/usbh.c | 8 -------- 3 files changed, 15 deletions(-) diff --git a/host/usb/private_include/usbh.h b/host/usb/private_include/usbh.h index daa942f0..6603fdbf 100644 --- a/host/usb/private_include/usbh.h +++ b/host/usb/private_include/usbh.h @@ -59,8 +59,6 @@ typedef struct { } dev_gone_data; struct { unsigned int dev_uid; - usb_device_handle_t parent_dev_hdl; - uint8_t port_num; } dev_free_data; }; } usbh_event_data_t; @@ -139,8 +137,6 @@ typedef struct { unsigned int uid; /**< Unique ID assigned to the device */ usb_speed_t speed; /**< Device's speed */ hcd_port_handle_t root_port_hdl; /**< Handle of the port that the device is connected to */ - usb_device_handle_t parent_dev_hdl; /**< Parent's device handle */ - uint8_t parent_port_num; /**< Parent's port number */ } usbh_dev_params_t; // ---------------------- USBH Processing Functions ---------------------------- diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index f0b801d7..2417ff5e 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -189,9 +189,6 @@ static esp_err_t dev_tree_node_new(void *parent, uint8_t port_num, usb_speed_t s .uid = dev_tree_node->uid, .speed = speed, .root_port_hdl = p_hub_driver_obj->constant.root_port_hdl, // Always the same for all devices - // TODO: IDF-10023 Move parent-child tree management responsibility to Hub Driver - .parent_dev_hdl = NULL, - .parent_port_num = port_num, }; ret = usbh_devs_add(¶ms); diff --git a/host/usb/src/usbh.c b/host/usb/src/usbh.c index f45a0e09..671734d2 100644 --- a/host/usb/src/usbh.c +++ b/host/usb/src/usbh.c @@ -79,8 +79,6 @@ struct device_s { // Assigned on device allocation and remain constant for the device's lifetime hcd_pipe_handle_t default_pipe; /**< Pipe handle for Control EP0 */ hcd_port_handle_t port_hdl; /**< HCD port handle */ - usb_device_handle_t parent_dev_hdl; /**< Device's parent device handle. NULL if device is connected to the root port */ - uint8_t parent_port_num; /**< Device's parent port number. 0 if device connected to the root port */ usb_speed_t speed; /**< Device's speed */ unsigned int uid; /**< Device's Unique ID */ /* @@ -388,8 +386,6 @@ static esp_err_t device_alloc(usbh_dev_params_t *params, device_t **dev_obj_ret) dev_obj->dynamic.state = USB_DEVICE_STATE_DEFAULT; dev_obj->constant.default_pipe = default_pipe_hdl; dev_obj->constant.port_hdl = params->root_port_hdl; - dev_obj->constant.parent_dev_hdl = params->parent_dev_hdl; - dev_obj->constant.parent_port_num = params->parent_port_num; dev_obj->constant.speed = params->speed; dev_obj->constant.uid = params->uid; // Note: Enumeration related dev_obj->constant fields are initialized later using usbh_dev_set_...() functions @@ -587,8 +583,6 @@ static inline void handle_free(device_t *dev_obj) { // Cache a copy of the device's address as we are about to free the device object const unsigned int dev_uid = dev_obj->constant.uid; - usb_device_handle_t parent_dev_hdl = dev_obj->constant.parent_dev_hdl; - const uint8_t parent_port_num = dev_obj->constant.parent_port_num; bool all_free; ESP_LOGD(USBH_TAG, "Freeing device %d", dev_obj->constant.address); @@ -613,8 +607,6 @@ static inline void handle_free(device_t *dev_obj) .event = USBH_EVENT_DEV_FREE, .dev_free_data = { .dev_uid = dev_uid, - .parent_dev_hdl = parent_dev_hdl, - .port_num = parent_port_num, } }; p_usbh_obj->constant.event_cb(&event_data, p_usbh_obj->constant.event_cb_arg); From 1e2be8f02ced1cf2d53c51a1c22cb590b7866bf8 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 26 Sep 2025 11:23:22 +0200 Subject: [PATCH 12/17] refactor(ext_hub): Added hcd port handle getter for external hub --- host/usb/private_include/ext_hub.h | 11 +++++++++++ host/usb/src/ext_hub.c | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/host/usb/private_include/ext_hub.h b/host/usb/private_include/ext_hub.h index 42e1e1d5..74207b1f 100644 --- a/host/usb/private_include/ext_hub.h +++ b/host/usb/private_include/ext_hub.h @@ -262,6 +262,17 @@ esp_err_t ext_hub_port_disable(ext_hub_handle_t ext_hub_hdl, uint8_t port_num); */ esp_err_t ext_hub_port_get_speed(ext_hub_handle_t ext_hub_hdl, uint8_t port_num, usb_speed_t *speed); +/** + * @brief Returns root port handle of the External Hub + * + * @param[in] ext_hub_hdl External Hub handle + * + * @return + * - Root port handle of the External Hub if the External Hub is installed + * - NULL if External Hub is not installed + */ +hcd_port_handle_t ext_hub_get_root_port_handle(ext_hub_handle_t ext_hub_hdl); + // --------------------------- USB Chapter 11 ---------------------------------- /** diff --git a/host/usb/src/ext_hub.c b/host/usb/src/ext_hub.c index 08647a2e..1dd93c56 100644 --- a/host/usb/src/ext_hub.c +++ b/host/usb/src/ext_hub.c @@ -1675,6 +1675,20 @@ esp_err_t ext_hub_port_get_speed(ext_hub_handle_t ext_hub_hdl, uint8_t port_num, return p_ext_hub_driver->constant.port_driver->get_speed(ext_hub_dev->constant.ports[port_idx], speed); } + +hcd_port_handle_t ext_hub_get_root_port_handle(ext_hub_handle_t ext_hub_hdl) +{ + EXT_HUB_ENTER_CRITICAL(); + EXT_HUB_CHECK_FROM_CRIT(p_ext_hub_driver != NULL, NULL); + EXT_HUB_EXIT_CRITICAL(); + EXT_HUB_CHECK(ext_hub_hdl != NULL, NULL); + ext_hub_dev_t *ext_hub_dev = (ext_hub_dev_t *)ext_hub_hdl; + + hcd_port_handle_t hcd_port_hdl; + usbh_dev_get_root_port_handle(ext_hub_dev->constant.dev_hdl, &hcd_port_hdl); + return hcd_port_hdl; +} + // ------------------------------------------------------------------------------------------------- // ---------------------------------- USB Chapter 11 ----------------------------------------------- // ------------------------------------------------------------------------------------------------- From 4cbacdbeb3fc22aab2454d0efe97ea5fa47f0802 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 26 Sep 2025 11:23:46 +0200 Subject: [PATCH 13/17] refactor(usbh): Added hcd port handle getter for external hub --- host/usb/private_include/usbh.h | 14 ++++++++++++++ host/usb/src/usbh.c | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/host/usb/private_include/usbh.h b/host/usb/private_include/usbh.h index 6603fdbf..f63eb221 100644 --- a/host/usb/private_include/usbh.h +++ b/host/usb/private_include/usbh.h @@ -351,6 +351,20 @@ esp_err_t usbh_dev_get_uid(usb_device_handle_t dev_hdl, unsigned int *uid); */ esp_err_t usbh_dev_get_addr(usb_device_handle_t dev_hdl, uint8_t *dev_addr); +/** + * @brief Get the root port handle of a device + * + * @note Callers of this function must have opened the device via usbh_devs_open() + * + * @param[in] dev_hdl Device handle + * @param[out] port_hdl Root port handle + * + * @return + * - ESP_ERR_INVALID_ARG if invalid argument + * - ESP_OK if Device's root port handle obtained successfully + */ +esp_err_t usbh_dev_get_root_port_handle(usb_device_handle_t dev_hdl, hcd_port_handle_t *port_hdl); + /** * @brief Get a device's information * diff --git a/host/usb/src/usbh.c b/host/usb/src/usbh.c index 671734d2..7e735bef 100644 --- a/host/usb/src/usbh.c +++ b/host/usb/src/usbh.c @@ -1070,6 +1070,14 @@ esp_err_t usbh_dev_get_addr(usb_device_handle_t dev_hdl, uint8_t *dev_addr) return ESP_OK; } +esp_err_t usbh_dev_get_root_port_handle(usb_device_handle_t dev_hdl, hcd_port_handle_t *port_hdl) +{ + USBH_CHECK(dev_hdl != NULL && port_hdl != NULL, ESP_ERR_INVALID_ARG); + device_t *dev_obj = (device_t *)dev_hdl; + *port_hdl = dev_obj->constant.port_hdl; + return ESP_OK; +} + esp_err_t usbh_dev_get_info(usb_device_handle_t dev_hdl, usb_device_info_t *dev_info) { USBH_CHECK(dev_hdl != NULL && dev_info != NULL, ESP_ERR_INVALID_ARG); From 5eecf459164df5f488cf357748d1057465b02d0e Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 26 Sep 2025 11:24:13 +0200 Subject: [PATCH 14/17] refactor(hcd): Added getter for the port number --- host/usb/private_include/hcd.h | 9 +++++++++ host/usb/src/hcd_dwc.c | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/host/usb/private_include/hcd.h b/host/usb/private_include/hcd.h index eeb1f227..9a742ef0 100644 --- a/host/usb/private_include/hcd.h +++ b/host/usb/private_include/hcd.h @@ -303,6 +303,15 @@ hcd_port_state_t hcd_port_get_state(hcd_port_handle_t port_hdl); */ esp_err_t hcd_port_get_speed(hcd_port_handle_t port_hdl, usb_speed_t *speed); +/** + * @brief Get the port number + * + * @param[in] port_hdl Port handle + * + * @return uint8_t Port number + */ +uint8_t hcd_port_get_number(hcd_port_handle_t port_hdl); + /** * @brief Handle a ports event * diff --git a/host/usb/src/hcd_dwc.c b/host/usb/src/hcd_dwc.c index 69dd8222..8a6fb1ba 100644 --- a/host/usb/src/hcd_dwc.c +++ b/host/usb/src/hcd_dwc.c @@ -256,6 +256,7 @@ struct port_obj { uint32_t val; } flags; bool initialized; + uint8_t port_num; // FIFO related usb_dwc_hal_fifo_config_t fifo_config; // FIFO config to be applied at HAL level // Port callback and context @@ -1418,6 +1419,7 @@ esp_err_t hcd_port_init(int port_number, const hcd_port_config_t *port_config, h HCD_CHECK_FROM_CRIT(port_obj->hal->channels.hdls != NULL, ESP_ERR_NO_MEM); port_obj->initialized = true; + port_obj->port_num = port_number; // Clear the frame list. We will set the frame list register and enable periodic scheduling after a successful reset memset(port_obj->frame_list, 0, FRAME_LIST_LEN * sizeof(uint32_t)); // If FIFO config is zeroed -> calculate from bias @@ -1518,6 +1520,12 @@ esp_err_t hcd_port_get_speed(hcd_port_handle_t port_hdl, usb_speed_t *speed) return ESP_OK; } +uint8_t hcd_port_get_number(hcd_port_handle_t port_hdl) +{ + port_t *port = (port_t *)port_hdl; + return port->port_num; +} + hcd_port_event_t hcd_port_handle_event(hcd_port_handle_t port_hdl) { port_t *port = (port_t *)port_hdl; From 82402a03e11c92bcb480b4b1d1dd12eceebe57f9 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 26 Sep 2025 12:14:08 +0200 Subject: [PATCH 15/17] refactor(hub): Isolated root_port object to have multiple hcd handles --- host/usb/src/hub.c | 260 +++++++++++++++++++++++++++++++++------------ 1 file changed, 194 insertions(+), 66 deletions(-) diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index 2417ff5e..6617b6c0 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -23,10 +23,8 @@ #include "ext_hub.h" #endif // ENABLE_USB_HUBS -/* -Implementation of the HUB driver that only supports the Root Hub with a single port. Therefore, we currently don't -implement the bare minimum to control the root HCD port. -*/ +/* Max support value based on the implementation: 4 */ +#define HUB_ROOT_PORTS 2 #ifdef CONFIG_USB_HOST_HW_BUFFER_BIAS_IN #define HUB_ROOT_HCD_PORT_FIFO_BIAS HCD_PORT_FIFO_BIAS_RX @@ -62,6 +60,21 @@ typedef enum { ROOT_PORT_STATE_ENABLED, /**< A device is connected, port has been reset, SOFs are sent */ } root_port_state_t; +/** + * @brief Root port object + * Root port represent the USB OTG Peripheral + */ +typedef struct { + struct { + root_port_state_t state; /**< Root port state */ + unsigned int reqs; /**< Root port request flag */ + } dynamic; + + struct { + hcd_port_handle_t hdl; /**< Root port HCD handle */ + } constant; +} root_port_t; + /** * @brief Hub device tree node * @@ -81,17 +94,16 @@ typedef struct { union { struct { hub_flag_action_t actions: 8; /**< Hub actions */ - uint32_t reserved24: 24; /**< Reserved */ + uint32_t pending_port: 4; /**< Root port number that has a pending request */ + uint32_t reserved20: 20; /**< Reserved */ }; uint32_t val; /**< Hub flag action value */ } flags; /**< Hub flags */ - root_port_state_t root_port_state; /**< Root port state */ - unsigned int port_reqs; /**< Root port request flag */ TAILQ_HEAD(tailhead_devs, dev_tree_node_s) dev_nodes_tailq; /**< Tailq of attached devices */ } dynamic; /**< Dynamic members. Require a critical section */ struct { - hcd_port_handle_t root_port_hdl; /**< Root port HCD handle */ + root_port_t root_port_hdls[HUB_ROOT_PORTS]; /**< Root port HCD handles */ usb_proc_req_cb_t proc_req_cb; /**< Process request callback */ void *proc_req_cb_arg; /**< Process request callback argument */ hub_event_cb_t event_cb; /**< Hub Driver event callback */ @@ -166,6 +178,19 @@ static dev_tree_node_t *dev_tree_node_get_by_uid(unsigned int node_uid) static esp_err_t dev_tree_node_new(void *parent, uint8_t port_num, usb_speed_t speed) { esp_err_t ret; + hcd_port_handle_t root_port_hdl; + + if (parent == NULL) { + // For root port, port_idx == port_num + root_port_hdl = p_hub_driver_obj->constant.root_port_hdls[port_num].constant.hdl; + } else { +#if (ENABLE_USB_HUBS) + root_port_hdl = ext_hub_get_root_port_handle((ext_hub_handle_t)parent); +#else + return ESP_ERR_NOT_SUPPORTED; +#endif + } + // Allocate memory for a new device tree node dev_tree_node_t *dev_tree_node = heap_caps_calloc(1, sizeof(dev_tree_node_t), MALLOC_CAP_DEFAULT); if (dev_tree_node == NULL) { @@ -188,7 +213,7 @@ static esp_err_t dev_tree_node_new(void *parent, uint8_t port_num, usb_speed_t s usbh_dev_params_t params = { .uid = dev_tree_node->uid, .speed = speed, - .root_port_hdl = p_hub_driver_obj->constant.root_port_hdl, // Always the same for all devices + .root_port_hdl = root_port_hdl, // Always the same for all devices connected to the one root }; ret = usbh_devs_add(¶ms); @@ -316,8 +341,10 @@ static esp_err_t dev_tree_node_remove_by_parent(void *parent, uint8_t port_num) static bool root_port_callback(hcd_port_handle_t port_hdl, hcd_port_event_t port_event, void *user_arg, bool in_isr) { + uint8_t port_idx = hcd_port_get_number(port_hdl); HUB_DRIVER_ENTER_CRITICAL_SAFE(); p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_EVENT; + p_hub_driver_obj->dynamic.flags.pending_port |= (1 << port_idx); HUB_DRIVER_EXIT_CRITICAL_SAFE(); assert(in_isr); // Currently, this callback should only ever be called from an ISR context return p_hub_driver_obj->constant.proc_req_cb(USB_PROC_REQ_SOURCE_HUB, in_isr, p_hub_driver_obj->constant.proc_req_cb_arg); @@ -391,8 +418,10 @@ static void ext_port_event_callback(ext_port_hdl_t port_hdl, ext_port_event_t ev #endif // ENABLE_USB_HUBS // ---------------------- Handlers ------------------------- -static void root_port_handle_events(hcd_port_handle_t root_port_hdl) +static void root_port_handle_events(root_port_t *root_port) { + hcd_port_handle_t root_port_hdl = root_port->constant.hdl; + uint8_t port_idx = hcd_port_get_number(root_port_hdl); hcd_port_event_t port_event = hcd_port_handle_event(root_port_hdl); switch (port_event) { case HCD_PORT_EVENT_NONE: @@ -405,23 +434,23 @@ static void root_port_handle_events(hcd_port_handle_t root_port_hdl) } ESP_LOGD(HUB_DRIVER_TAG, "Root port reset"); usb_speed_t speed; - if (hcd_port_get_speed(p_hub_driver_obj->constant.root_port_hdl, &speed) != ESP_OK) { + if (hcd_port_get_speed(root_port_hdl, &speed) != ESP_OK) { goto new_dev_err; } - if (dev_tree_node_new(NULL, 0, speed) != ESP_OK) { + if (dev_tree_node_new(NULL, port_idx, speed) != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Failed to add new device"); goto new_dev_err; } // Change Port state HUB_DRIVER_ENTER_CRITICAL(); - p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_ENABLED; + root_port->dynamic.state = ROOT_PORT_STATE_ENABLED; HUB_DRIVER_EXIT_CRITICAL(); break; new_dev_err: // We allow this to fail in case a disconnect/port error happens while disabling. - hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_DISABLE); + hcd_port_command(root_port_hdl, HCD_PORT_CMD_DISABLE); reset_err: break; } @@ -430,11 +459,11 @@ static void root_port_handle_events(hcd_port_handle_t root_port_hdl) case HCD_PORT_EVENT_OVERCURRENT: { bool port_has_device = false; HUB_DRIVER_ENTER_CRITICAL(); - switch (p_hub_driver_obj->dynamic.root_port_state) { + switch (root_port->dynamic.state) { case ROOT_PORT_STATE_POWERED: // This occurred before enumeration case ROOT_PORT_STATE_DISABLED: // This occurred after the device has already been disabled // Therefore, there's no device object to clean up, and we can go straight to port recovery - p_hub_driver_obj->dynamic.port_reqs |= PORT_REQ_RECOVER; + root_port->dynamic.reqs |= PORT_REQ_RECOVER; p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ; break; case ROOT_PORT_STATE_NOT_POWERED: // The user turned off ports' power. Indicate to USBH that the device is gone @@ -448,7 +477,7 @@ static void root_port_handle_events(hcd_port_handle_t root_port_hdl) HUB_DRIVER_EXIT_CRITICAL(); if (port_has_device) { - ESP_ERROR_CHECK(dev_tree_node_dev_gone(NULL, 0)); + ESP_ERROR_CHECK(dev_tree_node_dev_gone(NULL, port_idx)); } break; @@ -459,51 +488,56 @@ static void root_port_handle_events(hcd_port_handle_t root_port_hdl) } } -static void root_port_req(hcd_port_handle_t root_port_hdl) +static void root_port_req(root_port_t *root_port) { unsigned int port_reqs; + hcd_port_handle_t root_port_hdl = root_port->constant.hdl; HUB_DRIVER_ENTER_CRITICAL(); - port_reqs = p_hub_driver_obj->dynamic.port_reqs; - p_hub_driver_obj->dynamic.port_reqs = 0; + port_reqs = root_port->dynamic.reqs; + root_port->dynamic.reqs = 0; HUB_DRIVER_EXIT_CRITICAL(); if (port_reqs & PORT_REQ_DISABLE) { ESP_LOGD(HUB_DRIVER_TAG, "Disabling root port"); // We allow this to fail in case a disconnect/port error happens while disabling. - hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_DISABLE); + hcd_port_command(root_port_hdl, HCD_PORT_CMD_DISABLE); } if (port_reqs & PORT_REQ_RECOVER) { ESP_LOGD(HUB_DRIVER_TAG, "Recovering root port"); - ESP_ERROR_CHECK(hcd_port_recover(p_hub_driver_obj->constant.root_port_hdl)); + ESP_ERROR_CHECK(hcd_port_recover(root_port_hdl)); // In case the port's power was turned off with usb_host_lib_set_root_port_power(false) // we will not turn on the power during port recovery HUB_DRIVER_ENTER_CRITICAL(); - const root_port_state_t root_state = p_hub_driver_obj->dynamic.root_port_state; + const root_port_state_t root_state = root_port->dynamic.state; HUB_DRIVER_EXIT_CRITICAL(); if (root_state != ROOT_PORT_STATE_NOT_POWERED) { - ESP_ERROR_CHECK(hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_POWER_ON)); + ESP_ERROR_CHECK(hcd_port_command(root_port_hdl, HCD_PORT_CMD_POWER_ON)); HUB_DRIVER_ENTER_CRITICAL(); - p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_POWERED; + root_port->dynamic.state = ROOT_PORT_STATE_POWERED; HUB_DRIVER_EXIT_CRITICAL(); } } } -static esp_err_t root_port_recycle(void) +static esp_err_t root_port_recycle(uint8_t port_idx) { + root_port_t *root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; + hcd_port_handle_t root_port_hdl = root_port->constant.hdl; + HUB_DRIVER_CHECK(root_port_hdl != NULL, ESP_ERR_INVALID_STATE); + // Device is free, we can now request its port be recycled - hcd_port_state_t port_state = hcd_port_get_state(p_hub_driver_obj->constant.root_port_hdl); + hcd_port_state_t port_state = hcd_port_get_state(root_port_hdl); HUB_DRIVER_ENTER_CRITICAL(); // How the port is recycled will depend on the port's state switch (port_state) { case HCD_PORT_STATE_ENABLED: - p_hub_driver_obj->dynamic.port_reqs |= PORT_REQ_DISABLE; + root_port->dynamic.reqs |= PORT_REQ_DISABLE; break; case HCD_PORT_STATE_RECOVERY: - p_hub_driver_obj->dynamic.port_reqs |= PORT_REQ_RECOVER; + root_port->dynamic.reqs |= PORT_REQ_RECOVER; break; default: abort(); // Should never occur @@ -517,13 +551,26 @@ static esp_err_t root_port_recycle(void) return ESP_OK; } -static esp_err_t root_port_disable(void) +static esp_err_t root_port_reset(uint8_t port_idx) { - hcd_port_state_t port_state = hcd_port_get_state(p_hub_driver_obj->constant.root_port_hdl); + root_port_t *root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; + esp_err_t ret = hcd_port_command(root_port->constant.hdl, HCD_PORT_CMD_RESET); + if (ret != ESP_OK) { + ESP_LOGE(HUB_DRIVER_TAG, "Failed to issue root port reset"); + } else { + ret = dev_tree_node_reset_completed(NULL, port_idx); + } + return ret; +} + +static esp_err_t root_port_disable(uint8_t port_idx) +{ + root_port_t *root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; + hcd_port_state_t port_state = hcd_port_get_state(root_port->constant.hdl); HUB_DRIVER_CHECK(port_state == HCD_PORT_STATE_ENABLED, ESP_ERR_INVALID_STATE); HUB_DRIVER_ENTER_CRITICAL(); - p_hub_driver_obj->dynamic.port_reqs |= PORT_REQ_DISABLE; + root_port->dynamic.reqs |= PORT_REQ_DISABLE; p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ; HUB_DRIVER_EXIT_CRITICAL(); @@ -573,35 +620,41 @@ esp_err_t hub_install(hub_config_t *hub_config, void **client_ret) *client_ret = NULL; #endif // ENABLE_USB_HUBS - // Install HCD port + // Initialise HCD root ports + root_port_t *root_port; + // All HCD ports will use the same configuration hcd_port_config_t port_config = { .callback = root_port_callback, .callback_arg = NULL, .context = NULL, }; - hcd_port_handle_t root_port_hdl; - // Right now we support only one root port, can be extended in future - int root_port_index = 0; - if (hub_config->port_map & BIT1) { - root_port_index = 1; - } - - ret = hcd_port_init(root_port_index, &port_config, &root_port_hdl); - if (ret != ESP_OK) { - ESP_LOGE(HUB_DRIVER_TAG, "HCD Port init error: %s", esp_err_to_name(ret)); - goto err; + for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { + // Get the root port object + root_port = &hub_driver_obj->constant.root_port_hdls[port_idx]; + // Init values + root_port->dynamic.state = ROOT_PORT_STATE_NOT_POWERED; + root_port->dynamic.reqs = 0; + root_port->constant.hdl = NULL; + + // Init HCD port if enabled in the port map + if (hub_config->port_map & (1 << port_idx)) { + ESP_LOGD(HUB_DRIVER_TAG, "Init HCD port %d", port_idx); + ret = hcd_port_init(port_idx, &port_config, &root_port->constant.hdl); + if (ret != ESP_OK) { + ESP_LOGE(HUB_DRIVER_TAG, "HCD Port%d init error: %s", port_idx, esp_err_to_name(ret)); + goto err; + } + } } // Initialize Hub driver object - hub_driver_obj->constant.root_port_hdl = root_port_hdl; hub_driver_obj->constant.proc_req_cb = hub_config->proc_req_cb; hub_driver_obj->constant.proc_req_cb_arg = hub_config->proc_req_cb_arg; hub_driver_obj->constant.event_cb = hub_config->event_cb; hub_driver_obj->constant.event_cb_arg = hub_config->event_cb_arg; // Driver is not installed, we can modify dynamic section outside of the critical section TAILQ_INIT(&hub_driver_obj->dynamic.dev_nodes_tailq); - hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_NOT_POWERED; HUB_DRIVER_ENTER_CRITICAL(); if (p_hub_driver_obj != NULL) { @@ -615,7 +668,12 @@ esp_err_t hub_install(hub_config_t *hub_config, void **client_ret) return ret; assign_err: - ESP_ERROR_CHECK(hcd_port_deinit(root_port_hdl)); + for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { + root_port = &hub_driver_obj->constant.root_port_hdls[port_idx]; + if (root_port->constant.hdl != NULL) { + ESP_ERROR_CHECK(hcd_port_deinit(root_port->constant.hdl)); + } + } err: #if ENABLE_USB_HUBS ext_hub_uninstall(); @@ -629,9 +687,17 @@ esp_err_t hub_install(hub_config_t *hub_config, void **client_ret) esp_err_t hub_uninstall(void) { + root_port_t *root_port; + HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); - HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj->dynamic.root_port_state == ROOT_PORT_STATE_NOT_POWERED, ESP_ERR_INVALID_STATE); + // All ports must be NOT_POWERED + for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { + root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; + if (root_port->constant.hdl != NULL) { + HUB_DRIVER_CHECK_FROM_CRIT(root_port->dynamic.state == ROOT_PORT_STATE_NOT_POWERED, ESP_ERR_INVALID_STATE); + } + } hub_driver_t *hub_driver_obj = p_hub_driver_obj; p_hub_driver_obj = NULL; HUB_DRIVER_EXIT_CRITICAL(); @@ -641,7 +707,13 @@ esp_err_t hub_uninstall(void) ESP_ERROR_CHECK(ext_port_uninstall()); #endif // ENABLE_USB_HUBS - ESP_ERROR_CHECK(hcd_port_deinit(hub_driver_obj->constant.root_port_hdl)); + // Deinitialize Root Ports + for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { + root_port = &hub_driver_obj->constant.root_port_hdls[port_idx]; + if (root_port->constant.hdl != NULL) { + ESP_ERROR_CHECK(hcd_port_deinit(root_port->constant.hdl)); + } + } // Free Hub driver resources heap_caps_free(hub_driver_obj); return ESP_OK; @@ -649,16 +721,27 @@ esp_err_t hub_uninstall(void) esp_err_t hub_root_start(void) { + /* TODO: Support hub root port selection in USB Host (higher) layer */ + // For now, there is only one root port powered on at any time + root_port_t *root_port; + HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); - HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj->dynamic.root_port_state == ROOT_PORT_STATE_NOT_POWERED, ESP_ERR_INVALID_STATE); + for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { + root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; + if (root_port->constant.hdl != NULL) { + break; + } + } + HUB_DRIVER_CHECK_FROM_CRIT(root_port != NULL, ESP_ERR_NOT_FOUND); + HUB_DRIVER_CHECK_FROM_CRIT(root_port->dynamic.state == ROOT_PORT_STATE_NOT_POWERED, ESP_ERR_INVALID_STATE); HUB_DRIVER_EXIT_CRITICAL(); // Power ON the root port esp_err_t ret; - ret = hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_POWER_ON); + ret = hcd_port_command(root_port->constant.hdl, HCD_PORT_CMD_POWER_ON); if (ret == ESP_OK) { HUB_DRIVER_ENTER_CRITICAL(); - p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_POWERED; + root_port->dynamic.state = ROOT_PORT_STATE_POWERED; HUB_DRIVER_EXIT_CRITICAL(); } return ret; @@ -666,19 +749,29 @@ esp_err_t hub_root_start(void) esp_err_t hub_root_stop(void) { + /* TODO: Support hub root port selection in USB Host (higher) layer */ + // For now, there is only one root port powered on at any time + root_port_t *root_port; + HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); - if (p_hub_driver_obj->dynamic.root_port_state == ROOT_PORT_STATE_NOT_POWERED) { + for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { + root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; + if (root_port->constant.hdl != NULL) { + break; + } + } + if (root_port->dynamic.state == ROOT_PORT_STATE_NOT_POWERED) { // The HUB was already stopped by usb_host_lib_set_root_port_power(false) HUB_DRIVER_EXIT_CRITICAL(); return ESP_OK; } - p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_NOT_POWERED; + root_port->dynamic.state = ROOT_PORT_STATE_NOT_POWERED; HUB_DRIVER_EXIT_CRITICAL(); // HCD_PORT_CMD_POWER_OFF will only fail if the port is already powered_off // This should never happen, so we assert ret == ESP_OK - const esp_err_t ret = hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_POWER_OFF); + const esp_err_t ret = hcd_port_command(root_port->constant.hdl, HCD_PORT_CMD_POWER_OFF); assert(ret == ESP_OK); return ret; } @@ -694,8 +787,8 @@ esp_err_t hub_node_recycle(unsigned int node_uid) HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); if (dev_tree_node->parent == NULL) { - // Root Hub Ports - ret = root_port_recycle(); + // For Root ports number == index + ret = root_port_recycle(dev_tree_node->port_num); } else { #if ENABLE_USB_HUBS ret = ext_hub_port_recycle((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num); @@ -726,12 +819,7 @@ esp_err_t hub_node_reset(unsigned int node_uid) HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); if (dev_tree_node->parent == NULL) { - ret = hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_RESET); - if (ret != ESP_OK) { - ESP_LOGE(HUB_DRIVER_TAG, "Failed to issue root port reset"); - } else { - ret = dev_tree_node_reset_completed(NULL, 0); - } + ret = root_port_reset(dev_tree_node->port_num); } else { #if ENABLE_USB_HUBS ret = ext_hub_port_reset((ext_hub_handle_t) dev_tree_node->parent, dev_tree_node->port_num); @@ -785,7 +873,8 @@ esp_err_t hub_node_disable(unsigned int node_uid) HUB_DRIVER_CHECK(dev_tree_node != NULL, ESP_ERR_NOT_FOUND); if (dev_tree_node->parent == NULL) { - ret = root_port_disable(); + // For Root ports number == index + ret = root_port_disable(dev_tree_node->port_num); } else { #if ENABLE_USB_HUBS // External Hub port @@ -880,6 +969,45 @@ esp_err_t hub_notify_all_free(void) } #endif // ENABLE_USB_HUBS +/** + * @brief Get next pending root port number + * + * @param[out] port_idx Pointer to store the port index + * @return true if a pending port was found, false otherwise + */ +static bool root_get_pending_port(uint8_t *port_idx) +{ + bool is_pending = false; + HUB_DRIVER_ENTER_CRITICAL(); + for (int i = 0; i < 4; i++) { + if (p_hub_driver_obj->dynamic.flags.pending_port & (1 << i)) { + p_hub_driver_obj->dynamic.flags.pending_port &= ~(1 << i); + *port_idx = i; + is_pending = true; + break; + } + } + HUB_DRIVER_EXIT_CRITICAL(); + return is_pending; +} + +static esp_err_t root_port_process(void) +{ + uint8_t port_idx; + while (root_get_pending_port(&port_idx)) { + root_port_handle_events(&p_hub_driver_obj->constant.root_port_hdls[port_idx]); + } + return ESP_OK; +} + +static esp_err_t root_port_req_process(void) +{ + for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { + root_port_req(&p_hub_driver_obj->constant.root_port_hdls[port_idx]); + } + return ESP_OK; +} + esp_err_t hub_process(void) { HUB_DRIVER_ENTER_CRITICAL(); @@ -897,10 +1025,10 @@ esp_err_t hub_process(void) } #endif // ENABLE_USB_HUBS if (action_flags & HUB_DRIVER_ACTION_ROOT_EVENT) { - root_port_handle_events(p_hub_driver_obj->constant.root_port_hdl); + ESP_ERROR_CHECK(root_port_process()); } if (action_flags & HUB_DRIVER_ACTION_ROOT_REQ) { - root_port_req(p_hub_driver_obj->constant.root_port_hdl); + ESP_ERROR_CHECK(root_port_req_process()); } HUB_DRIVER_ENTER_CRITICAL(); From 60465adaea6b6930fab06e6e06432e85e92a81e6 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 26 Sep 2025 15:17:20 +0200 Subject: [PATCH 16/17] feature(usb_host): Added handling of peripheral map during install - HUB: Added periph_idx argument to hub_root_start() and hub_root_stop() --- host/usb/private_include/hub.h | 8 ++- host/usb/src/hub.c | 27 ++------- host/usb/src/usb_host.c | 108 +++++++++++++++++++++------------ 3 files changed, 80 insertions(+), 63 deletions(-) diff --git a/host/usb/private_include/hub.h b/host/usb/private_include/hub.h index 41bd9ebb..f7d32223 100644 --- a/host/usb/private_include/hub.h +++ b/host/usb/private_include/hub.h @@ -111,22 +111,26 @@ esp_err_t hub_uninstall(void); * * @note This function should only be called from the Host Library task * + * @param[in] port_idx Root port index + * * @return * - ESP_OK: Hub driver started successfully * - ESP_ERR_INVALID_STATE: Hub driver is not installed, or root port is in other state than not powered */ -esp_err_t hub_root_start(void); +esp_err_t hub_root_start(uint8_t port_idx); /** * @brief Stops the Hub driver's root port * * This will power OFF the root port * + * @param[in] port_idx Root port index + * * @return * - ESP_OK: Hub driver started successfully * - ESP_ERR_INVALID_STATE: Hub driver is not installed, or root port is in not powered state */ -esp_err_t hub_root_stop(void); +esp_err_t hub_root_stop(uint8_t port_idx); /** * @brief Indicate to the Hub driver that a device's port can be recycled diff --git a/host/usb/src/hub.c b/host/usb/src/hub.c index 6617b6c0..b515ec70 100644 --- a/host/usb/src/hub.c +++ b/host/usb/src/hub.c @@ -719,21 +719,11 @@ esp_err_t hub_uninstall(void) return ESP_OK; } -esp_err_t hub_root_start(void) +esp_err_t hub_root_start(uint8_t port_idx) { - /* TODO: Support hub root port selection in USB Host (higher) layer */ - // For now, there is only one root port powered on at any time - root_port_t *root_port; - HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); - for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { - root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; - if (root_port->constant.hdl != NULL) { - break; - } - } - HUB_DRIVER_CHECK_FROM_CRIT(root_port != NULL, ESP_ERR_NOT_FOUND); + root_port_t *root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; HUB_DRIVER_CHECK_FROM_CRIT(root_port->dynamic.state == ROOT_PORT_STATE_NOT_POWERED, ESP_ERR_INVALID_STATE); HUB_DRIVER_EXIT_CRITICAL(); // Power ON the root port @@ -747,20 +737,11 @@ esp_err_t hub_root_start(void) return ret; } -esp_err_t hub_root_stop(void) +esp_err_t hub_root_stop(uint8_t port_idx) { - /* TODO: Support hub root port selection in USB Host (higher) layer */ - // For now, there is only one root port powered on at any time - root_port_t *root_port; - HUB_DRIVER_ENTER_CRITICAL(); HUB_DRIVER_CHECK_FROM_CRIT(p_hub_driver_obj != NULL, ESP_ERR_INVALID_STATE); - for (uint8_t port_idx = 0; port_idx < HUB_ROOT_PORTS; port_idx++) { - root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; - if (root_port->constant.hdl != NULL) { - break; - } - } + root_port_t *root_port = &p_hub_driver_obj->constant.root_port_hdls[port_idx]; if (root_port->dynamic.state == ROOT_PORT_STATE_NOT_POWERED) { // The HUB was already stopped by usb_host_lib_set_root_port_power(false) HUB_DRIVER_EXIT_CRITICAL(); diff --git a/host/usb/src/usb_host.c b/host/usb/src/usb_host.c index 0476a1af..87aa6f92 100644 --- a/host/usb/src/usb_host.c +++ b/host/usb/src/usb_host.c @@ -36,6 +36,9 @@ Warning: The USB Host Library API is still a beta version and may be subject to #include "soc/usb_periph.h" #endif +// TODOs List: +// 1. Refactor HUB_PORTS_NUM: make them dynamically configurable by hub_config_t::port_map + DEFINE_CRIT_SECTION_LOCK_STATIC(host_lock); #define HOST_ENTER_CRITICAL_ISR() esp_os_enter_critical_isr(&host_lock) #define HOST_EXIT_CRITICAL_ISR() esp_os_exit_critical_isr(&host_lock) @@ -158,7 +161,8 @@ typedef struct { struct { SemaphoreHandle_t event_sem; SemaphoreHandle_t mux_lock; - usb_phy_handle_t phy_handle; // Will be NULL if host library is installed with skip_phy_setup + unsigned peripheral_map; // Peripheral map + usb_phy_handle_t phy_hdls[4 /* TODO: 1 */]; // Will be NULL if host library is installed with skip_phy_setup void *enum_client; // Pointer to Enum driver (acting as a client). Used to reroute completed USBH control transfers void *hub_client; // Pointer to External Hub driver (acting as a client). Used to reroute completed USBH control transfers. NULL, when External Hub Driver not available. } constant; @@ -463,6 +467,8 @@ esp_err_t usb_host_install(const usb_host_config_t *config) TAILQ_INIT(&host_lib_obj->mux_protected.client_tailq); host_lib_obj->constant.event_sem = event_sem; host_lib_obj->constant.mux_lock = mux_lock; + // For backward compatibility accept 0 too + host_lib_obj->constant.peripheral_map = config->peripheral_map == 0 ? BIT0 : config->peripheral_map; /* Install each layer of the Host stack (listed below) from the lowest layer to the highest @@ -473,35 +479,38 @@ esp_err_t usb_host_install(const usb_host_config_t *config) - Hub */ - // For backward compatibility accept 0 too - const unsigned peripheral_map = config->peripheral_map == 0 ? BIT0 : config->peripheral_map; - // Install USB PHY (if necessary). USB PHY driver will also enable the underlying Host Controller if (!config->skip_phy_setup) { - bool init_utmi_phy = false; // Default value for Linux simulation - -#if SOC_USB_OTG_SUPPORTED // In case we run on a real target, select the PHY from usb_dwc_info description structure - // Right now we support only one peripheral, can be extended in future - int peripheral_index = 0; - if (peripheral_map & BIT1) { - peripheral_index = 1; - } - init_utmi_phy = (usb_dwc_info.controllers[peripheral_index].supported_phys == USB_PHY_INST_UTMI_0); -#endif // SOC_USB_OTG_SUPPORTED - - // Host Library defaults to internal PHY + // Host Library defaults to install PHY usb_phy_config_t phy_config = { .controller = USB_PHY_CTRL_OTG, - .target = init_utmi_phy ? USB_PHY_TARGET_UTMI : USB_PHY_TARGET_INT, .otg_mode = USB_OTG_MODE_HOST, .otg_speed = USB_PHY_SPEED_UNDEFINED, // In Host mode, the speed is determined by the connected device - .ext_io_conf = NULL, - .otg_io_conf = NULL, }; - ret = usb_new_phy(&phy_config, &host_lib_obj->constant.phy_handle); - if (ret != ESP_OK) { - ESP_LOGE(USB_HOST_TAG, "PHY install error: %s", esp_err_to_name(ret)); - goto phy_err; + + for (unsigned idx = 0; idx < 4 /* TODO: 1 */; idx++) { + if (host_lib_obj->constant.peripheral_map & (BIT0 << idx)) { +#if SOC_USB_OTG_SUPPORTED // In case we run on a real target, select the PHY from usb_dwc_info description structure + // Index should be less than SOC_USB_OTG_PERIPH_NUM + if (idx >= (SOC_USB_OTG_PERIPH_NUM)) { + ESP_LOGE(USB_HOST_TAG, "Peripheral index %d in peripheral_map is out of range", idx); + ret = ESP_ERR_INVALID_ARG; + goto phy_err; + } + // Right now we support only one peripheral, can be extended in future + if (usb_dwc_info.controllers[idx].supported_phys == USB_PHY_INST_UTMI_0) { + phy_config.target = USB_PHY_TARGET_UTMI; + } else { + phy_config.target = USB_PHY_TARGET_INT; + } +#endif // SOC_USB_OTG_SUPPORTED + ret = usb_new_phy(&phy_config, &host_lib_obj->constant.phy_hdls[idx]); + if (ret != ESP_OK) { + ESP_LOGE(USB_HOST_TAG, "PHY for peripheral %d install error: %s", + idx, esp_err_to_name(ret)); + goto phy_err; + } + } } } @@ -510,7 +519,7 @@ esp_err_t usb_host_install(const usb_host_config_t *config) hcd_fifo_settings_t user_fifo_config; hcd_config_t hcd_config = { .intr_flags = config->intr_flags, - .peripheral_map = peripheral_map, + .peripheral_map = host_lib_obj->constant.peripheral_map, .fifo_config = NULL, // Default: use bias strategy from Kconfig }; @@ -565,7 +574,7 @@ esp_err_t usb_host_install(const usb_host_config_t *config) // Install Hub hub_config_t hub_config = { - .port_map = peripheral_map, // Each USB-OTG peripheral maps to a root port + .port_map = host_lib_obj->constant.peripheral_map, // Each USB-OTG peripheral maps to a root port .proc_req_cb = proc_req_callback, .proc_req_cb_arg = NULL, .event_cb = hub_event_callback, @@ -588,8 +597,12 @@ esp_err_t usb_host_install(const usb_host_config_t *config) HOST_EXIT_CRITICAL(); if (!config->root_port_unpowered) { - // Start the root hub - ESP_ERROR_CHECK(hub_root_start()); + // Start the root hub according to the peripheral map + for (unsigned idx = 0; idx < 4 /* TODO: 1 */; idx++) { + if (p_host_lib_obj->constant.peripheral_map & (BIT0 << idx)) { + ESP_ERROR_CHECK(hub_root_start(idx)); + } + } } ret = ESP_OK; @@ -604,10 +617,13 @@ esp_err_t usb_host_install(const usb_host_config_t *config) usbh_err: ESP_ERROR_CHECK(hcd_uninstall()); hcd_err: - if (host_lib_obj->constant.phy_handle) { - ESP_ERROR_CHECK(usb_del_phy(host_lib_obj->constant.phy_handle)); - } phy_err: + for (unsigned idx = 0; idx < 4 /* TODO: 1*/; idx++) { + if (host_lib_obj->constant.peripheral_map & (BIT0 << idx) && + (host_lib_obj->constant.phy_hdls[idx] != NULL)) { + ESP_ERROR_CHECK(usb_del_phy(host_lib_obj->constant.phy_hdls[idx])); + } + } alloc_err: if (mux_lock) { vSemaphoreDelete(mux_lock); @@ -630,8 +646,12 @@ esp_err_t usb_host_uninstall(void) ESP_ERR_INVALID_STATE); HOST_EXIT_CRITICAL(); - // Stop the root hub - ESP_ERROR_CHECK(hub_root_stop()); + // Stop the root hub according to the peripheral map + for (unsigned idx = 0; idx < 4 /* TODO: 1 */; idx++) { + if (p_host_lib_obj->constant.peripheral_map & (BIT0 << idx)) { + ESP_ERROR_CHECK(hub_root_stop(idx)); + } + } // Unassign the host library object HOST_ENTER_CRITICAL(); @@ -652,8 +672,11 @@ esp_err_t usb_host_uninstall(void) ESP_ERROR_CHECK(usbh_uninstall()); ESP_ERROR_CHECK(hcd_uninstall()); // If the USB PHY was setup, then delete it - if (host_lib_obj->constant.phy_handle) { - ESP_ERROR_CHECK(usb_del_phy(host_lib_obj->constant.phy_handle)); + for (unsigned idx = 0; idx < 4 /* TODO: 1 */; idx++) { + if (host_lib_obj->constant.peripheral_map & (BIT0 << idx) && + (host_lib_obj->constant.phy_hdls[idx] != NULL)) { + ESP_ERROR_CHECK(usb_del_phy(host_lib_obj->constant.phy_hdls[idx])); + } } // Free memory objects @@ -747,10 +770,19 @@ esp_err_t usb_host_lib_info(usb_host_lib_info_t *info_ret) esp_err_t usb_host_lib_set_root_port_power(bool enable) { esp_err_t ret; - if (enable) { - ret = hub_root_start(); - } else { - ret = hub_root_stop(); + + // Set port power for every port according to the peripheral map + for (unsigned idx = 0; idx < 4 /* TODO: 1 */; idx++) { + if (p_host_lib_obj->constant.peripheral_map & (BIT0 << idx)) { + if (enable) { + ret = hub_root_start(idx); + } else { + ret = hub_root_stop(idx); + } + if (ret != ESP_OK) { + return ret; + } + } } return ret; From 85b3cc0791d754c6151662e2fc3f23c17fc77491 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Mon, 29 Sep 2025 16:25:07 +0200 Subject: [PATCH 17/17] refactor(usb_host_mock): Adapted hub_root_start and hub_root_stop api --- .../usb_host_layer_test/main/usb_host_install_unit_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/host/usb/test/host_test/usb_host_layer_test/main/usb_host_install_unit_test.cpp b/host/usb/test/host_test/usb_host_layer_test/main/usb_host_install_unit_test.cpp index 7cc25c79..b0eb2961 100644 --- a/host/usb/test/host_test/usb_host_layer_test/main/usb_host_install_unit_test.cpp +++ b/host/usb/test/host_test/usb_host_layer_test/main/usb_host_install_unit_test.cpp @@ -199,7 +199,7 @@ SCENARIO("USB Host install") hub_install_ExpectAnyArgsAndReturn(ESP_OK); // Make hub_root_start() to pass - hub_root_start_ExpectAndReturn(ESP_OK); + hub_root_start_ExpectAndReturn(0, ESP_OK); // Call the DUT function, expect ESP_OK REQUIRE(ESP_OK == usb_host_install(&usb_host_config)); @@ -228,7 +228,7 @@ SCENARIO("USB Host post-uninstall") SECTION("Successfully uninstall the USB Host driver") { // Make the hub_root_stop() to pass - hub_root_stop_ExpectAndReturn(ESP_OK); + hub_root_stop_ExpectAndReturn(0, ESP_OK); // Make uninstalling of all the drivers to pass hub_uninstall_ExpectAndReturn(ESP_OK);