From 46952c601d804083faf5ee69dcded10fe5b6fa8c Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 31 Jan 2025 13:05:29 +0100 Subject: [PATCH] feat(usb_host): Added uid presence check in USBH device object list --- components/usb/hub.c | 52 +++++++++++++-------------- components/usb/private_include/usbh.h | 14 +++++++- components/usb/usbh.c | 10 +++++- 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/components/usb/hub.c b/components/usb/hub.c index e39e741d69..ac2ba34fe8 100644 --- a/components/usb/hub.c +++ b/components/usb/hub.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -28,7 +28,6 @@ implement the bare minimum to control the root HCD port. */ #define HUB_ROOT_PORT_NUM 1 // HCD only supports one port -#define HUB_ROOT_DEV_UID 1 // Unique device ID #ifdef CONFIG_USB_HOST_HW_BUFFER_BIAS_IN #define HUB_ROOT_HCD_PORT_FIFO_BIAS HCD_PORT_FIFO_BIAS_RX @@ -93,7 +92,6 @@ typedef struct { struct { TAILQ_HEAD(tailhead_devs, dev_tree_node_s) dev_nodes_tailq; /**< Tailq of attached devices */ - unsigned int next_uid; /**< Unique ID for next upcoming device */ } single_thread; /**< Single thread members don't require a critical section so long as they are never accessed from multiple threads */ struct { @@ -153,53 +151,52 @@ static bool root_port_callback(hcd_port_handle_t port_hdl, hcd_port_event_t port * * @return esp_err_t */ -static esp_err_t new_dev_tree_node(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num, usb_speed_t speed) +static esp_err_t dev_tree_node_new(usb_device_handle_t parent_dev_hdl, uint8_t parent_port_num, usb_speed_t speed) { esp_err_t ret; - unsigned int node_uid = p_hub_driver_obj->single_thread.next_uid; - + // 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) { return ESP_ERR_NO_MEM; } + // Assign initial UID based on the current number of registered devices + int device_num = 0; + ESP_ERROR_CHECK(usbh_devs_num(&device_num)); + dev_tree_node->uid = device_num + 1; + // Ensure the UID is unique + while (usbh_devs_is_uid_in_use(dev_tree_node->uid)) { + dev_tree_node->uid++; + assert(dev_tree_node->uid != 0); // No overflow possible + } - // Allocate a new USBH device + dev_tree_node->parent_dev_hdl = parent_dev_hdl; + dev_tree_node->parent_port_num = parent_port_num; + + // Initialize and register a new USBH Device with the assigned UID usbh_dev_params_t params = { - .uid = node_uid, + .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 responsibility of parent-child tree building to Hub Driver instead of USBH + // TODO: IDF-10023 Move parent-child tree management responsibility to Hub Driver .parent_dev_hdl = parent_dev_hdl, .parent_port_num = parent_port_num, }; ret = usbh_devs_add(¶ms); if (ret != ESP_OK) { - // USBH devs add could failed due to lack of free hcd channels - // TODO: IDF-10044 Hub should recover after running out of hcd channels + // Device registration may fail if there are no available HCD channels. + // TODO: IDF-10044 Implement hub recovery mechanism for running out of HCD channels. goto fail; } - dev_tree_node->uid = node_uid; - dev_tree_node->parent_dev_hdl = parent_dev_hdl; - dev_tree_node->parent_port_num = parent_port_num; TAILQ_INSERT_TAIL(&p_hub_driver_obj->single_thread.dev_nodes_tailq, dev_tree_node, tailq_entry); - p_hub_driver_obj->single_thread.next_uid++; - if (p_hub_driver_obj->single_thread.next_uid == 0) { - ESP_LOGW(HUB_DRIVER_TAG, "Counter overflowed, possibility of uid collisions"); - p_hub_driver_obj->single_thread.next_uid = HUB_ROOT_DEV_UID; - } - // Verify presence of a device with newly prepared uid in USBH - // TODO: IDF-10022 Provide a mechanism to request presence status of a device with uid in USBH device object list - // Return if device uid is not in USBH device object list, repeat until uid will be founded - - ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): new", node_uid); + ESP_LOGD(HUB_DRIVER_TAG, "Device tree node (uid=%d): new", dev_tree_node->uid); hub_event_data_t event_data = { .event = HUB_EVENT_CONNECTED, .connected = { - .uid = node_uid, + .uid = dev_tree_node->uid, }, }; p_hub_driver_obj->constant.event_cb(&event_data, p_hub_driver_obj->constant.event_cb_arg); @@ -364,7 +361,7 @@ static void ext_port_event_callback(ext_port_event_data_t *event_data, void *arg } #endif // CONFIG_USB_HOST_EXT_PORT_SUPPORT_LS - if (new_dev_tree_node(event_data->connected.parent_dev_hdl, event_data->connected.parent_port_num, port_speed) != ESP_OK) { + if (dev_tree_node_new(event_data->connected.parent_dev_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; } @@ -406,7 +403,7 @@ static void root_port_handle_events(hcd_port_handle_t root_port_hdl) goto new_dev_err; } - if (new_dev_tree_node(NULL, 0, speed) != ESP_OK) { + if (dev_tree_node_new(NULL, 0, speed) != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Failed to add new device"); goto new_dev_err; } @@ -589,7 +586,6 @@ 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; - hub_driver_obj->single_thread.next_uid = HUB_ROOT_DEV_UID; TAILQ_INIT(&hub_driver_obj->single_thread.dev_nodes_tailq); // Driver is not installed, we can modify dynamic section outside of the critical section hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_NOT_POWERED; diff --git a/components/usb/private_include/usbh.h b/components/usb/private_include/usbh.h index 7777acd6e2..a07ff44767 100644 --- a/components/usb/private_include/usbh.h +++ b/components/usb/private_include/usbh.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 */ @@ -194,6 +194,18 @@ esp_err_t usbh_uninstall(void); esp_err_t usbh_process(void); // ---------------------- Device Pool Functions -------------------------------- +/** + * @brief Determines whether a UID is currently assigned in the USBH device list + * + * @note This function may block execution while checking the device list. + * + * @param[in] uid Unique ID to check + * + * @return + * - true if UID is already in use. + * - false if UID is available for assignment. + */ +bool usbh_devs_is_uid_in_use(uint32_t uid); /** * @brief Get the current number of devices diff --git a/components/usb/usbh.c b/components/usb/usbh.c index 54355ace68..cb047b4279 100644 --- a/components/usb/usbh.c +++ b/components/usb/usbh.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -781,6 +781,14 @@ esp_err_t usbh_process(void) // ----------------------------------------------------------------------------- // ------------------------- Device Pool Functions ----------------------------- // ----------------------------------------------------------------------------- +bool usbh_devs_is_uid_in_use(uint32_t uid) +{ + bool uid_in_use; + USBH_ENTER_CRITICAL(); + uid_in_use = (_find_dev_from_uid(uid) != NULL); // Check if UID exists + USBH_EXIT_CRITICAL(); + return uid_in_use; +} esp_err_t usbh_devs_num(int *num_devs_ret) {