From 6ae151db8c6f06158aba510b4f9c62ed6e4bc01a Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Tue, 22 Nov 2022 00:41:15 +0800 Subject: [PATCH 1/2] usb_host: Fix error when fetching LANGID table USB devices may support string descriptors in multiple languages. The supported languages are stored in a LANGID table, which itself is a string descriptor at index 0. When fetching the LANGID table itself, the USB 2.0 specification does not specify what LANGID to use, thus the Hub driver would use the default LANGID "ENUM_LANGID". However, this would cause some devices to stall. This commit fixes the issue by always requesting the LANGID table itself using a LANGID of 0. --- components/usb/hub.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/components/usb/hub.c b/components/usb/hub.c index e27dc836d02..226e2866cf2 100644 --- a/components/usb/hub.c +++ b/components/usb/hub.c @@ -288,33 +288,34 @@ static bool enum_stage_second_reset(enum_ctrl_t *enum_ctrl) return true; } -static uint8_t get_string_desc_index(enum_ctrl_t *enum_ctrl) +static void get_string_desc_index_and_langid(enum_ctrl_t *enum_ctrl, uint8_t *index, uint16_t *langid) { - uint8_t index; switch (enum_ctrl->stage) { case ENUM_STAGE_GET_SHORT_LANGID_TABLE: case ENUM_STAGE_GET_FULL_LANGID_TABLE: - index = 0; //The LANGID table uses an index of 0 + *index = 0; //The LANGID table uses an index of 0 + *langid = 0; //Getting the LANGID table itself should use a LANGID of 0 break; case ENUM_STAGE_GET_SHORT_MANU_STR_DESC: case ENUM_STAGE_GET_FULL_MANU_STR_DESC: - index = enum_ctrl->iManufacturer; + *index = enum_ctrl->iManufacturer; + *langid = ENUM_LANGID; //Use the default LANGID break; case ENUM_STAGE_GET_SHORT_PROD_STR_DESC: case ENUM_STAGE_GET_FULL_PROD_STR_DESC: - index = enum_ctrl->iProduct; + *index = enum_ctrl->iProduct; + *langid = ENUM_LANGID; //Use the default LANGID break; case ENUM_STAGE_GET_SHORT_SER_STR_DESC: case ENUM_STAGE_GET_FULL_SER_STR_DESC: - index = enum_ctrl->iSerialNumber; + *index = enum_ctrl->iSerialNumber; + *langid = ENUM_LANGID; //Use the default LANGID break; default: //Should not occur - index = 0; abort(); break; } - return index; } static bool enum_stage_transfer(enum_ctrl_t *enum_ctrl) @@ -369,11 +370,13 @@ static bool enum_stage_transfer(enum_ctrl_t *enum_ctrl) case ENUM_STAGE_GET_SHORT_MANU_STR_DESC: case ENUM_STAGE_GET_SHORT_PROD_STR_DESC: case ENUM_STAGE_GET_SHORT_SER_STR_DESC: { - uint8_t index = get_string_desc_index(enum_ctrl); + uint8_t index; + uint16_t langid; + get_string_desc_index_and_langid(enum_ctrl, &index, &langid); //Get only the header of the string descriptor USB_SETUP_PACKET_INIT_GET_STR_DESC((usb_setup_packet_t *)transfer->data_buffer, index, - ENUM_LANGID, + langid, sizeof(usb_str_desc_t)); transfer->num_bytes = sizeof(usb_setup_packet_t) + usb_round_up_to_mps(sizeof(usb_str_desc_t), enum_ctrl->bMaxPacketSize0); //IN data stage should return exactly sizeof(usb_str_desc_t) bytes @@ -384,11 +387,13 @@ static bool enum_stage_transfer(enum_ctrl_t *enum_ctrl) case ENUM_STAGE_GET_FULL_MANU_STR_DESC: case ENUM_STAGE_GET_FULL_PROD_STR_DESC: case ENUM_STAGE_GET_FULL_SER_STR_DESC: { - uint8_t index = get_string_desc_index(enum_ctrl); + uint8_t index; + uint16_t langid; + get_string_desc_index_and_langid(enum_ctrl, &index, &langid); //Get the full string descriptor at a particular index, requesting the descriptors exact length USB_SETUP_PACKET_INIT_GET_STR_DESC((usb_setup_packet_t *)transfer->data_buffer, index, - ENUM_LANGID, + langid, enum_ctrl->str_desc_bLength); transfer->num_bytes = sizeof(usb_setup_packet_t) + usb_round_up_to_mps(enum_ctrl->str_desc_bLength, enum_ctrl->bMaxPacketSize0); //IN data stage should return exactly str_desc_bLength bytes @@ -415,12 +420,12 @@ static bool enum_stage_transfer_check(enum_ctrl_t *enum_ctrl) //Check transfer status usb_transfer_t *transfer = &dequeued_enum_urb->transfer; if (transfer->status != USB_TRANSFER_STATUS_COMPLETED) { - ESP_LOGE(HUB_DRIVER_TAG, "Bad transfer status: %s", enum_stage_strings[enum_ctrl->stage]); + ESP_LOGE(HUB_DRIVER_TAG, "Bad transfer status %d: %s", transfer->status, enum_stage_strings[enum_ctrl->stage]); return false; } //Check IN transfer returned the expected correct number of bytes if (enum_ctrl->expect_num_bytes != 0 && enum_ctrl->expect_num_bytes != transfer->actual_num_bytes) { - ESP_LOGE(HUB_DRIVER_TAG, "Incorrect number of bytes returned: %s", enum_stage_strings[enum_ctrl->stage]); + ESP_LOGE(HUB_DRIVER_TAG, "Incorrect number of bytes returned %d: %s", transfer->actual_num_bytes, enum_stage_strings[enum_ctrl->stage]); return false; } From 7010349a9aa22785b7ddc1f80df982185bedd6e5 Mon Sep 17 00:00:00 2001 From: Saurabh Kumar Bansal Date: Wed, 25 Jan 2023 19:38:52 +0530 Subject: [PATCH 2/2] usb_host: Hub driver skips fetching string descriptors if their index is 0 When a USB does not support a particular string dsecriptor (e.g., manufacturer, product, and serial number), the string descriptors corresponding index will be set to 0 in the device descriptor (e.g., iManufacturer, iProduct, iString). Previously, the Hub driver would always attempt to fetch the all three string descriptors, thus leading an error in CHECK_SHORT_SER_STR_DESC if the device did not support the descriptor. This commit fixes the Hub drvier by skipping the enumeration stages of a particular descriptor if its index is 0 (i.e., not supported by the device). --- components/usb/hub.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/components/usb/hub.c b/components/usb/hub.c index 226e2866cf2..be09ffdf952 100644 --- a/components/usb/hub.c +++ b/components/usb/hub.c @@ -421,6 +421,10 @@ static bool enum_stage_transfer_check(enum_ctrl_t *enum_ctrl) usb_transfer_t *transfer = &dequeued_enum_urb->transfer; if (transfer->status != USB_TRANSFER_STATUS_COMPLETED) { ESP_LOGE(HUB_DRIVER_TAG, "Bad transfer status %d: %s", transfer->status, enum_stage_strings[enum_ctrl->stage]); + if (transfer->status == USB_TRANSFER_STATUS_STALL) { + //EP stalled, clearing the pipe to execute further stages + ESP_ERROR_CHECK(hcd_pipe_command(enum_ctrl->pipe, HCD_PIPE_CMD_CLEAR)); + } return false; } //Check IN transfer returned the expected correct number of bytes @@ -622,6 +626,27 @@ static void enum_stage_cleanup_failed(enum_ctrl_t *enum_ctrl) HUB_DRIVER_EXIT_CRITICAL(); } +static enum_stage_t get_next_stage(enum_stage_t old_stage, enum_ctrl_t *enum_ctrl) +{ + enum_stage_t new_stage = old_stage + 1; + //Skip the GET_DESCRIPTOR string type corresponding stages if a particular index is 0. + while(((new_stage == ENUM_STAGE_GET_SHORT_MANU_STR_DESC || + new_stage == ENUM_STAGE_CHECK_SHORT_MANU_STR_DESC || + new_stage == ENUM_STAGE_GET_FULL_MANU_STR_DESC || + new_stage == ENUM_STAGE_CHECK_FULL_MANU_STR_DESC) && enum_ctrl->iManufacturer == 0) || + ((new_stage == ENUM_STAGE_GET_SHORT_PROD_STR_DESC || + new_stage == ENUM_STAGE_CHECK_SHORT_PROD_STR_DESC || + new_stage == ENUM_STAGE_GET_FULL_PROD_STR_DESC || + new_stage == ENUM_STAGE_CHECK_FULL_PROD_STR_DESC) && enum_ctrl->iProduct == 0) || + ((new_stage == ENUM_STAGE_GET_SHORT_SER_STR_DESC || + new_stage == ENUM_STAGE_CHECK_SHORT_SER_STR_DESC || + new_stage == ENUM_STAGE_GET_FULL_SER_STR_DESC || + new_stage == ENUM_STAGE_CHECK_FULL_SER_STR_DESC) && enum_ctrl->iSerialNumber == 0)) { + new_stage++; + } + return new_stage; +} + static void enum_set_next_stage(enum_ctrl_t *enum_ctrl, bool last_stage_pass) { //Set next stage @@ -629,7 +654,7 @@ static void enum_set_next_stage(enum_ctrl_t *enum_ctrl, bool last_stage_pass) if (enum_ctrl->stage != ENUM_STAGE_NONE && enum_ctrl->stage != ENUM_STAGE_CLEANUP && enum_ctrl->stage != ENUM_STAGE_CLEANUP_FAILED) { - enum_ctrl->stage++; //Go to next stage + enum_ctrl->stage = get_next_stage(enum_ctrl->stage, enum_ctrl); } else { enum_ctrl->stage = ENUM_STAGE_NONE; }