fix(enum): Fixed STALL on descriptor request, removed unused value

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
+    if (ctrl_xfer->status != USB_TRANSFER_STATUS_COMPLETED) {
-    // We already checked the transfer status for control transfer
+        ESP_LOGE(ENUM_TAG, "Bad transfer status %d: %s",
-    assert(ctrl_xfer->status == USB_TRANSFER_STATUS_COMPLETED);
+                 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.
+                // iSerialNumber string failed. Jump to Set Configuration and complete enumeration process.
-                next_stage = ENUM_STAGE_COMPLETE;
+                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 nest stage of enumeration process, based on the stage result
-    set_next_stage(ret == ESP_OK);
+    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;
 }