Merge branch 'fix/usb_host_ext_port_recovery_delay' into 'master'

fix(ext_port): Added port recovery delay Closes IDF-11357 See merge request espressif/esp-idf!33984
espressif · Oct 21, 2024 · e26ad72 · e26ad72
2 parents e8a6f41 + 4677cce
commit e26ad72
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 19 deletions.
diff --git a/components/usb/Kconfig b/components/usb/Kconfig
@@ -133,7 +133,19 @@ menu "USB-OTG"
 
                     The default value is 1.
 
-            config USB_HOST_EXT_PORT_CUSTOM_RESET_ENABLE
+            config USB_HOST_EXT_PORT_RESET_RECOVERY_DELAY_MS
+                int "Reset recovery delay in ms"
+                default 30
+                help
+                    After a port stops driving the reset signal, the USB 2.0 specification requires that
+                    the "USB System Software guarantees a minimum of 10 ms for reset recovery" before the
+                    attached device is expected to respond to data transfers (see USB 2.0 chapter 7.1.7.3 for
+                    more details).
+                    The device may ignore any data transfers during the recovery interval.
+
+                    The default value is set to 30 ms to be safe.
+
+            config USB_HOST_EXT_PORT_CUSTOM_POWER_ON_DELAY_ENABLE
                 bool "Custom bPwrOn2PwrGood value"
                 default n
                 help
@@ -143,8 +155,8 @@ menu "USB-OTG"
                     When enabled, applies the custom PwrOn2PwrGood delay.
                     When disabled, applies the PwrOn2PwrGood value from the Hub Descriptor.
 
-            config USB_HOST_EXT_PORT_CUSTOM_RESET_MS
-                depends on USB_HOST_EXT_PORT_CUSTOM_RESET_ENABLE
+            config USB_HOST_EXT_PORT_CUSTOM_POWER_ON_DELAY_MS
+                depends on USB_HOST_EXT_PORT_CUSTOM_POWER_ON_DELAY_ENABLE
                 int "PwrOn2PwrGood delay in ms"
                 default 100
                 range 0 5000

diff --git a/components/usb/ext_port.c b/components/usb/ext_port.c
@@ -25,9 +25,9 @@
 #define EXT_PORT_RESET_ATTEMPTS     1
 #endif
 // Delay in ms after sending the SetFeature() class specific request
-#define EXT_PORT_RESET_CUSTOM_DELAY         CONFIG_USB_HOST_EXT_PORT_CUSTOM_RESET_ENABLE
-#define EXT_PORT_RESET_CUSTOM_DELAY_MS      CONFIG_USB_HOST_EXT_PORT_CUSTOM_RESET_MS
-#define EXT_PORT_RESET_DEFAULT_DELAY_MS     100
+#define EXT_PORT_RESET_RECOVERY_DELAY_MS       CONFIG_USB_HOST_EXT_PORT_RESET_RECOVERY_DELAY_MS
+#define EXT_PORT_POWER_ON_CUSTOM_DELAY         CONFIG_USB_HOST_EXT_PORT_CUSTOM_POWER_ON_DELAY_ENABLE
+#define EXT_PORT_POWER_ON_CUSTOM_DELAY_MS      CONFIG_USB_HOST_EXT_PORT_CUSTOM_POWER_ON_DELAY_MS
 
 /**
  * @brief External Port driver action flags
@@ -80,7 +80,7 @@ struct ext_port_s {
         // Port related constant members
         ext_hub_handle_t ext_hub_hdl;           /**< Ports' parent External Hub handle */
         uint8_t port_num;                       /**< Ports' parent External Hub Port number */
-        int reset_delay_ms;                     /**< Ports' Power on time to Power Good, ms */
+        int power_on_delay_ms;                  /**< Ports' Power on time to Power Good, ms */
     } constant;                                 /**< Constant members. Do not change after installation thus do not require a critical section or mutex */
 };
 
@@ -262,9 +262,17 @@ static esp_err_t port_set_feature(ext_port_t *ext_port, const usb_hub_port_featu
     }
     // Every set feature requires status update
     ext_port->flags.status_outdated = 1;
-    // PowerOn to PowerGood delay for port
-    if (feature == USB_FEATURE_PORT_RESET) {
-        vTaskDelay(pdMS_TO_TICKS(ext_port->constant.reset_delay_ms));
+    switch (feature) {
+    case USB_FEATURE_PORT_POWER:
+        // PowerOn to PowerGood delay for port
+        vTaskDelay(pdMS_TO_TICKS(ext_port->constant.power_on_delay_ms));
+        break;
+    case USB_FEATURE_PORT_RESET:
+        // Port has reset, give the port some time to recover
+        vTaskDelay(pdMS_TO_TICKS(EXT_PORT_RESET_RECOVERY_DELAY_MS));
+        break;
+    default:
+        break;
     }
     return ret;
 }
@@ -451,21 +459,21 @@ static esp_err_t port_alloc(ext_hub_handle_t ext_hub_hdl, usb_device_handle_t pa
     ext_port->constant.parent_dev_addr = parent_dev_addr;
     ext_port->constant.ext_hub_hdl = ext_hub_hdl;
     ext_port->constant.port_num = parent_port_num;
-#if (EXT_PORT_RESET_CUSTOM_DELAY)
-    ext_port->constant.reset_delay_ms = EXT_PORT_RESET_CUSTOM_DELAY_MS;
+#if (EXT_PORT_POWER_ON_CUSTOM_DELAY)
+    ext_port->constant.power_on_delay_ms = EXT_PORT_POWER_ON_CUSTOM_DELAY_MS;
 #else
-    ext_port->constant.reset_delay_ms = (port_delay_ms == 0)
-                                        ? EXT_PORT_RESET_DEFAULT_DELAY_MS
-                                        : port_delay_ms;
-#endif // EXT_PORT_POWER_ON_CUSTOM
+    // We don't need any additional delay in case port_delay_ms == 0, because this usually means
+    // that parent Hub device has no power switches
+    ext_port->constant.power_on_delay_ms = port_delay_ms;
+#endif // EXT_PORT_POWER_ON_CUSTOM_DELAY
 
     ext_port->state = USB_PORT_STATE_NOT_CONFIGURED;
     ext_port->dev_state = PORT_DEV_NOT_PRESENT;
 
     ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port has been added (PwrOn2PwrGood=%d ms)",
              ext_port->constant.parent_dev_addr,
              ext_port->constant.port_num,
-             ext_port->constant.reset_delay_ms);
+             ext_port->constant.power_on_delay_ms);
 
     *port_obj = ext_port;
     return ESP_OK;
@@ -555,8 +563,6 @@ static bool handle_port_status(ext_port_t *ext_port)
         ESP_LOGD(EXT_PORT_TAG, "[%d:%d] Port still in reset, wait and repeat get status...",
                  ext_port->constant.parent_dev_addr,
                  ext_port->constant.port_num);
-        // PowerOn to PowerGood delay for port
-        vTaskDelay(pdMS_TO_TICKS(ext_port->constant.reset_delay_ms));
         port_request_status(ext_port);
         need_processing = true;
     }