From c5ee01dd0b7bab1c836f5fa87921673cf7baa1a8 Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Tue, 11 Jun 2024 02:55:27 +0800 Subject: [PATCH] fix(enum): Fixed STALL on descriptor request, removed unused value --- components/usb/enum.c | 57 ++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/components/usb/enum.c b/components/usb/enum.c index eb4b448a93..fd7325e51a 100644 --- a/components/usb/enum.c +++ b/components/usb/enum.c @@ -30,14 +30,16 @@ /** * @brief Stages of device enumeration listed in their order of execution * + * Entry: * - These stages MUST BE LISTED IN THE ORDER OF THEIR EXECUTION as the enumeration will simply increment the current stage * - If an error occurs at any stage, ENUM_STAGE_CANCEL acts as a common exit stage on failure * - Must start with 0 as enum is also used as an index * - The short descriptor stages are used to fetch the start particular descriptors that don't have a fixed length in order to determine the full descriptors length + * - Any state of Get String Descriptor could be STALLed by the device. In that case we just don't fetch them and treat enumeration as successful */ typedef enum { ENUM_STAGE_IDLE = 0, /**< There is no device awaiting enumeration */ - // Basic device enumeration + // Basic Device enumeration ENUM_STAGE_GET_SHORT_DEV_DESC, /**< Getting short dev desc (wLength is ENUM_SHORT_DESC_REQ_LEN) */ ENUM_STAGE_CHECK_SHORT_DEV_DESC, /**< Save bMaxPacketSize0 from the short dev desc. Update the MPS of the enum pipe */ ENUM_STAGE_SECOND_RESET, /**< Reset the device again (Workaround for old USB devices that get confused by the previous short dev desc request). */ @@ -51,7 +53,7 @@ typedef enum { ENUM_STAGE_CHECK_SHORT_CONFIG_DESC, /**< Save wTotalLength of the short config desc */ ENUM_STAGE_GET_FULL_CONFIG_DESC, /**< Get the full config desc (wLength is the saved wTotalLength) */ ENUM_STAGE_CHECK_FULL_CONFIG_DESC, /**< Check the full config desc, fill it into the device object in USBH */ - // Get string descriptors + // Get String Descriptors ENUM_STAGE_GET_SHORT_LANGID_TABLE, /**< Get the header of the LANGID table string descriptor */ ENUM_STAGE_CHECK_SHORT_LANGID_TABLE, /**< Save the bLength of the LANGID table string descriptor */ ENUM_STAGE_GET_FULL_LANGID_TABLE, /**< Get the full LANGID table string descriptor */ @@ -68,7 +70,7 @@ typedef enum { ENUM_STAGE_CHECK_SHORT_SER_STR_DESC, /**< Save the bLength of the iSerialNumber string descriptor */ ENUM_STAGE_GET_FULL_SER_STR_DESC, /**< Get the full iSerialNumber string descriptor */ ENUM_STAGE_CHECK_FULL_SER_STR_DESC, /**< Check and fill the full iSerialNumber string descriptor */ - // Set configuration + // Set Configuration ENUM_STAGE_SET_CONFIG, /**< Send SET_CONFIGURATION request */ ENUM_STAGE_CHECK_CONFIG, /**< Check that SET_CONFIGURATION request was successful */ // Terminal stages @@ -779,9 +781,12 @@ static esp_err_t control_response_handling(void) int expected_num_bytes = p_enum_driver->single_thread.expect_num_bytes; usb_transfer_t *ctrl_xfer = &p_enum_driver->constant.urb->transfer; - // Sanity check - // We already checked the transfer status for control transfer - assert(ctrl_xfer->status == USB_TRANSFER_STATUS_COMPLETED); + if (ctrl_xfer->status != USB_TRANSFER_STATUS_COMPLETED) { + ESP_LOGE(ENUM_TAG, "Bad transfer status %d: %s", + ctrl_xfer->status, + enum_stage_strings[p_enum_driver->dynamic.stage]); + return ret; + } // Check Control IN transfer returned the expected correct number of bytes if (expected_num_bytes != 0 && expected_num_bytes != ctrl_xfer->actual_num_bytes) { @@ -996,11 +1001,11 @@ static void set_next_stage(bool last_stage_pass) // Stages that are allowed to fail skip to the next appropriate stage case ENUM_STAGE_CHECK_SHORT_LANGID_TABLE: case ENUM_STAGE_CHECK_FULL_LANGID_TABLE: - // Couldn't get LANGID, skip the rest of the string descriptors and jump straight to cleanup. + // Couldn't get LANGID, skip the rest of the string descriptors case ENUM_STAGE_CHECK_SHORT_SER_STR_DESC: case ENUM_STAGE_CHECK_FULL_SER_STR_DESC: - // iSerialNumber string failed. Jump to complete. - next_stage = ENUM_STAGE_COMPLETE; + // iSerialNumber string failed. Jump to Set Configuration and complete enumeration process. + next_stage = ENUM_STAGE_SET_CONFIG; break; case ENUM_STAGE_CHECK_SHORT_MANU_STR_DESC: case ENUM_STAGE_CHECK_FULL_MANU_STR_DESC: @@ -1086,14 +1091,21 @@ static void enum_control_transfer_complete(usb_transfer_t *ctrl_xfer) assert(ctrl_xfer->context); assert(p_enum_driver->single_thread.dev_hdl == ctrl_xfer->context); - if (ctrl_xfer->status == USB_TRANSFER_STATUS_COMPLETED) { + switch (ctrl_xfer->status) { + case USB_TRANSFER_STATUS_COMPLETED: ESP_LOG_BUFFER_HEXDUMP(ENUM_TAG, ctrl_xfer->data_buffer, ctrl_xfer->actual_num_bytes, ESP_LOG_VERBOSE); goto process; - } else { + break; + case USB_TRANSFER_STATUS_STALL: + // Device can STALL some requests if it doesn't have the requested descriptors + goto process; + break; + default: ESP_LOGE(ENUM_TAG, "[%d:%d] Control transfer failed, status=%d", p_enum_driver->single_thread.parent_dev_addr, p_enum_driver->single_thread.parent_port_num, ctrl_xfer->status); + break; } // Cancel enumeration process enum_cancel(p_enum_driver->single_thread.dev_uid); @@ -1284,7 +1296,7 @@ esp_err_t enum_process(void) ENUM_CHECK_FROM_CRIT(p_enum_driver->dynamic.flags.processing != 0, ESP_ERR_INVALID_STATE); ENUM_EXIT_CRITICAL(); - esp_err_t ret = ESP_FAIL; + esp_err_t res = ESP_FAIL; bool need_process_cb = true; enum_stage_t stage = p_enum_driver->dynamic.stage; @@ -1305,13 +1317,13 @@ esp_err_t enum_process(void) case ENUM_STAGE_GET_SHORT_SER_STR_DESC: case ENUM_STAGE_GET_FULL_SER_STR_DESC: need_process_cb = false; // Do not need to request process callback, as we need to wait transfer completion - ret = control_request(); + res = control_request(); break; // Recovery interval case ENUM_STAGE_SET_ADDR_RECOVERY: // Need a short delay before device is ready. Todo: IDF-7007 vTaskDelay(pdMS_TO_TICKS(SET_ADDR_RECOVERY_INTERVAL_MS)); - ret = ESP_OK; + res = ESP_OK; break; // Transfer check stages case ENUM_STAGE_CHECK_SHORT_DEV_DESC: @@ -1328,37 +1340,36 @@ esp_err_t enum_process(void) case ENUM_STAGE_CHECK_FULL_PROD_STR_DESC: case ENUM_STAGE_CHECK_SHORT_SER_STR_DESC: case ENUM_STAGE_CHECK_FULL_SER_STR_DESC: - ret = control_response_handling(); + res = control_response_handling(); break; case ENUM_STAGE_SELECT_CONFIG: - ret = select_active_configuration(); + res = select_active_configuration(); break; case ENUM_STAGE_SECOND_RESET: need_process_cb = false; // We need to wait Hub driver to finish port reset - ret = second_reset(); + res = second_reset(); break; case ENUM_STAGE_CANCEL: need_process_cb = false; // Terminal state - ret = stage_cancel(); + res = stage_cancel(); break; case ENUM_STAGE_COMPLETE: need_process_cb = false; // Terminal state - ret = stage_complete(); + res = stage_complete(); break; default: // Should never occur - ret = ESP_ERR_INVALID_STATE; abort(); break; } - // Set nest stage of enumeration process - set_next_stage(ret == ESP_OK); + // Set nest stage of enumeration process, based on the stage result + set_next_stage(res == ESP_OK); // Request process callback is necessary if (need_process_cb) { p_enum_driver->constant.proc_req_cb(USB_PROC_REQ_SOURCE_ENUM, false, p_enum_driver->constant.proc_req_cb_arg); } - return ret; + return ESP_OK; }