fix(usb/host): Correctly handle unpowered port in HUB

components/usb/hub.c

@@ -369,8 +369,8 @@ static void root_port_handle_events(hcd_port_handle_t root_port_hdl)
             p_hub_driver_obj->dynamic.port_reqs |= PORT_REQ_RECOVER;
             p_hub_driver_obj->dynamic.flags.actions |= HUB_DRIVER_ACTION_ROOT_REQ;
             break;
-        case ROOT_PORT_STATE_ENABLED:
-            // There is an enabled (active) device. We need to indicate to USBH that the device is gone
+        case ROOT_PORT_STATE_NOT_POWERED: // The user turned off ports' power. Indicate to USBH that the device is gone
+        case ROOT_PORT_STATE_ENABLED: // There is an enabled (active) device. Indicate to USBH that the device is gone
             port_has_device = true;
             break;
         default:
@@ -408,10 +408,19 @@ static void root_port_req(hcd_port_handle_t root_port_hdl)
     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_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_POWER_ON));
+
+        // 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();
-        p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_POWERED;
+        const root_port_state_t root_state = p_hub_driver_obj->dynamic.root_port_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));
+            HUB_DRIVER_ENTER_CRITICAL();
+            p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_POWERED;
+            HUB_DRIVER_EXIT_CRITICAL();
+        }
     }
 }
 
@@ -574,15 +583,18 @@ esp_err_t hub_root_stop(void)
 {
     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);
-    HUB_DRIVER_EXIT_CRITICAL();
-    esp_err_t ret;
-    ret = hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_POWER_OFF);
-    if (ret == ESP_OK) {
-        HUB_DRIVER_ENTER_CRITICAL();
-        p_hub_driver_obj->dynamic.root_port_state = ROOT_PORT_STATE_NOT_POWERED;
+    if (p_hub_driver_obj->dynamic.root_port_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;
+    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);
+    assert(ret == ESP_OK);
     return ret;
 }
 

components/usb/include/usb/usb_host.h

@@ -219,8 +219,10 @@ esp_err_t usb_host_lib_info(usb_host_lib_info_t *info_ret);
  * @note If 'usb_host_config_t.root_port_unpowered' was set on USB Host Library installation, users must call this
  *       function to power ON the root port before any device connections can occur.
  *
- * @param enable True to power the root port ON, false to power OFF
- * @return esp_err_t
+ * @param[in] enable True to power the root port ON, false to power OFF
+ * @return
+ *    - ESP_OK: Root port power enabled/disabled
+ *    - ESP_ERR_INVALID_STATE: Root port already powered or HUB driver not installed
  */
 esp_err_t usb_host_lib_set_root_port_power(bool enable);
 

components/usb/test_apps/usb_host/main/msc_client_async_dconn.c

@@ -88,8 +88,10 @@ static void msc_data_transfer_cb(usb_transfer_t *transfer)
     // The data stage should have either completed, or failed due to the disconnection.
     TEST_ASSERT(transfer->status == USB_TRANSFER_STATUS_COMPLETED || transfer->status == USB_TRANSFER_STATUS_NO_DEVICE);
     if (transfer->status == USB_TRANSFER_STATUS_COMPLETED) {
+        printf("Data transfer completed\n");
         TEST_ASSERT_EQUAL(transfer->num_bytes, transfer->actual_num_bytes);
     } else {
+        printf("Data transfer NOT completed: No device\n");
         TEST_ASSERT_EQUAL(0, transfer->actual_num_bytes);
     }
     msc_client_obj_t *msc_obj = (msc_client_obj_t *)transfer->context;
@@ -238,7 +240,7 @@ void msc_client_async_dconn_task(void *arg)
             break;
         }
         case TEST_STAGE_MSC_DATA_DCONN: {
-            ESP_LOGD(MSC_CLIENT_TAG, "Data and disconnect");
+            ESP_LOGD(MSC_CLIENT_TAG, "Data (%d transfers) and disconnect", msc_obj.num_data_transfers);
             // Setup the Data IN transfers
             const usb_ep_desc_t *in_ep_desc = dev_msc_get_in_ep_desc(msc_obj.dev_speed);
             const int bulk_ep_mps = USB_EP_DESC_GET_MPS(in_ep_desc);
@@ -259,8 +261,11 @@ void msc_client_async_dconn_task(void *arg)
             ESP_LOGD(MSC_CLIENT_TAG, "Close");
             TEST_ASSERT_EQUAL(ESP_OK, usb_host_interface_release(msc_obj.client_hdl, msc_obj.dev_hdl, msc_obj.dev_info->bInterfaceNumber));
             TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_close(msc_obj.client_hdl, msc_obj.dev_hdl));
-            dconn_iter++;
-            if (dconn_iter < TEST_DCONN_ITERATIONS) {
+            vTaskDelay(10); // Yield to USB Host task so it can handle the disconnection
+
+            // The device has disconnected and it's disconnection has been handled
+            printf("Dconn iter %d done\n", dconn_iter);
+            if (++dconn_iter < TEST_DCONN_ITERATIONS) {
                 // Start the next test iteration by going back to TEST_STAGE_WAIT_CONN and reenabling connections
                 msc_obj.next_stage = TEST_STAGE_WAIT_CONN;
                 skip_event_handling = true; // Need to execute TEST_STAGE_WAIT_CONN

components/usb/test_apps/usb_host/main/msc_client_async_enum.c

@@ -177,6 +177,7 @@ void msc_client_async_enum_task(void *arg)
             if (enum_iter < TEST_ENUM_ITERATIONS) {
                 // Start the next test iteration by disconnecting the device, then going back to TEST_STAGE_WAIT_CONN stage
                 usb_host_lib_set_root_port_power(false);
+                vTaskDelay(10); // Yield to USB Host task so it can handle the disconnection
                 usb_host_lib_set_root_port_power(true);
                 msc_obj.next_stage = TEST_STAGE_WAIT_CONN;
                 skip_event_handling = true; // Need to execute TEST_STAGE_WAIT_CONN

components/usb/test_apps/usb_host/main/test_app_main.c

@@ -31,6 +31,7 @@ void tearDown(void)
     // Short delay to allow task to be cleaned up
     vTaskDelay(10);
     // Clean up USB Host
+    printf("USB Host uninstall\n");
     ESP_ERROR_CHECK(usb_host_uninstall());
     // Short delay to allow task to be cleaned up after client uninstall
     vTaskDelay(10);