Merge pull request #3371 from hathach/enum_catch

Hub enumeration reliability improvements

1 files changed, 55 insertions, 30 deletions

diff --git a/src/host/usbh.c b/src/host/usbh.c
index 5b14a15cb..e99d9e977 100644
--- a/src/host/usbh.c
+++ b/src/host/usbh.c
@@ -312,7 +312,8 @@ TU_ATTR_ALWAYS_INLINE static inline usbh_class_driver_t const *get_driver(uint8_
 // Function Inline and Prototypes
 //--------------------------------------------------------------------+
 static bool enum_new_device(hcd_event_t* event);
-static void process_removed_device(uint8_t rhport, uint8_t hub_addr, uint8_t hub_port);
+static void process_remove_event(hcd_event_t *event);
+static void remove_device_tree(uint8_t rhport, uint8_t hub_addr, uint8_t hub_port);
 static bool usbh_edpt_control_open(uint8_t dev_addr, uint8_t max_packet_size);
 static bool usbh_control_xfer_cb (uint8_t daddr, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes);
 
@@ -540,8 +541,8 @@ bool tuh_deinit(uint8_t rhport) {
   hcd_deinit(rhport);
   _usbh_data.controller_id = TUSB_INDEX_INVALID_8;
 
-  // "unplug" all devices on this rhport (hub_addr = 0, hub_port = 0)
-  process_removed_device(rhport, 0, 0);
+  // remove all devices on this rhport (hub_addr = 0, hub_port = 0)
+  remove_device_tree(rhport, 0, 0);
 
   // deinit host stack if no controller is active
   if (!tuh_inited()) {
@@ -605,6 +606,11 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
 
     switch (event.event_id) {
       case HCD_EVENT_DEVICE_ATTACH:
+        // Should we miss the hub detach event due to high traffic, Or due to physical debouncing, some devices can
+        // cause multiple attaches (actually reset) without detach event.
+        // Force remove currently mounted with the same bus info (rhport, hub addr, hub port) if exists
+        process_remove_event(&event);
+
         // due to the shared control buffer, we must fully complete enumerating one device first.
         // TODO better to have an separated queue for newly attached devices
         if (_usbh_data.enumerating_daddr == TUSB_INDEX_INVALID_8) {
@@ -625,15 +631,7 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
 
       case HCD_EVENT_DEVICE_REMOVE:
         TU_LOG_USBH("[%u:%u:%u] USBH DEVICE REMOVED\r\n", event.rhport, event.connection.hub_addr, event.connection.hub_port);
-        if (_usbh_data.enumerating_daddr == 0 &&
-            event.rhport == _usbh_data.dev0_bus.rhport &&
-            event.connection.hub_addr == _usbh_data.dev0_bus.hub_addr &&
-            event.connection.hub_port == _usbh_data.dev0_bus.hub_port) {
-          // dev0 is unplugged while enumerating (not yet assigned an address)
-          usbh_device_close(_usbh_data.dev0_bus.rhport, 0);
-        } else {
-          process_removed_device(event.rhport, event.connection.hub_addr, event.connection.hub_port);
-        }
+        process_remove_event(&event);
         break;
 
       case HCD_EVENT_XFER_COMPLETE: {
@@ -1321,8 +1319,22 @@ bool tuh_interface_set(uint8_t daddr, uint8_t itf_num, uint8_t itf_alt,
 //--------------------------------------------------------------------+
 // Detaching
 //--------------------------------------------------------------------+
-// a device unplugged from rhport:hub_addr:hub_port
-static void process_removed_device(uint8_t rhport, uint8_t hub_addr, uint8_t hub_port) {
+
+// process detach event from rhport:hub_addr:hub_port
+static void process_remove_event(hcd_event_t *event) {
+  if (_usbh_data.enumerating_daddr == 0 &&
+      event->rhport == _usbh_data.dev0_bus.rhport &&
+      event->connection.hub_addr == _usbh_data.dev0_bus.hub_addr &&
+      event->connection.hub_port == _usbh_data.dev0_bus.hub_port) {
+    // dev0 is unplugged while enumerating (not yet assigned an address)
+    usbh_device_close(_usbh_data.dev0_bus.rhport, 0);
+  } else {
+    remove_device_tree(event->rhport, event->connection.hub_addr, event->connection.hub_port);
+  }
+}
+
+// remove a device at rhport:hub_addr:hub_port and all of its downstream
+static void remove_device_tree(uint8_t rhport, uint8_t hub_addr, uint8_t hub_port) {
   // Find the all devices (star-network) under port that is unplugged
   #if CFG_TUH_HUB
   uint8_t removing_hubs[CFG_TUH_HUB] = { 0 };
@@ -1427,7 +1439,7 @@ enum {
 
 static uint8_t enum_get_new_address(bool is_hub);
 static bool enum_parse_configuration_desc (uint8_t dev_addr, tusb_desc_configuration_t const* desc_cfg);
-static void enum_full_complete(void);
+static void enum_full_complete(bool success);
 static void process_enumeration(tuh_xfer_t* xfer);
 
 // start a new enumeration process
@@ -1449,7 +1461,7 @@ static bool enum_new_device(hcd_event_t* event) {
 
     if (!hcd_port_connect_status(dev0_bus->rhport)) {
       TU_LOG_USBH("Device unplugged while debouncing\r\n");
-      enum_full_complete();
+      enum_full_complete(false);
       return true;
     }
 
@@ -1460,7 +1472,7 @@ static bool enum_new_device(hcd_event_t* event) {
 
     if (!hcd_port_connect_status(dev0_bus->rhport)) {
       // device unplugged while delaying
-      enum_full_complete();
+      enum_full_complete(false);
       return true;
     }
 
@@ -1506,7 +1518,7 @@ static void process_enumeration(tuh_xfer_t* xfer) {
     }
 
     if (!retry) {
-      enum_full_complete(); // complete as failed
+      enum_full_complete(false); // complete as failed
     }
     return;
   }
@@ -1529,7 +1541,7 @@ static void process_enumeration(tuh_xfer_t* xfer) {
 
       if (0 == port_status.status.connection) {
         TU_LOG_USBH("Device unplugged from hub while debouncing\r\n");
-        enum_full_complete();
+        enum_full_complete(false);
         return;
       }
 
@@ -1566,7 +1578,7 @@ static void process_enumeration(tuh_xfer_t* xfer) {
 
       if (0 == port_status.status.connection) {
         TU_LOG_USBH("Device unplugged from hub (not addressed yet)\r\n");
-        enum_full_complete();
+        enum_full_complete(false);
         return;
       }
 
@@ -1582,7 +1594,11 @@ static void process_enumeration(tuh_xfer_t* xfer) {
 
       // TODO probably doesn't need to open/close each enumeration
       uint8_t const addr0 = 0;
-      TU_ASSERT(usbh_edpt_control_open(addr0, 8),);
+      if (!usbh_edpt_control_open(addr0, 8)) {
+        // Stop enumeration gracefully
+        enum_full_complete(false);
+        TU_ASSERT(false,);
+      }
 
       // Get first 8 bytes of device descriptor for control endpoint size
       TU_LOG_USBH("Get 8 byte of Device Descriptor\r\n");
@@ -1592,10 +1608,6 @@ static void process_enumeration(tuh_xfer_t* xfer) {
     }
 
     case ENUM_SET_ADDR: {
-      // Due to physical debouncing, some devices can cause multiple attaches (actually reset) without detach event
-      // Force remove currently mounted with the same bus info (rhport, hub addr, hub port) if exists
-      process_removed_device(dev0_bus->rhport, dev0_bus->hub_addr, dev0_bus->hub_port);
-
       const tusb_desc_device_t *desc_device = (const tusb_desc_device_t *) _usbh_epbuf.ctrl;
       const uint8_t new_addr = enum_get_new_address(desc_device->bDeviceClass == TUSB_CLASS_HUB);
       TU_ASSERT(new_addr != 0,);
@@ -1620,7 +1632,12 @@ static void process_enumeration(tuh_xfer_t* xfer) {
 
       usbh_device_close(dev0_bus->rhport, 0); // close dev0
 
-      TU_ASSERT(usbh_edpt_control_open(new_addr, new_dev->bMaxPacketSize0),); // open new control endpoint
+      if (!usbh_edpt_control_open(new_addr, new_dev->bMaxPacketSize0)) { // open new control endpoint
+        // Stop enumeration gracefully
+        clear_device(new_dev);
+        enum_full_complete(false);
+        TU_ASSERT(false,);
+      }
 
       TU_LOG_USBH("Get Device Descriptor\r\n");
       TU_ASSERT(tuh_descriptor_get_device(new_addr, _usbh_epbuf.ctrl, sizeof(tusb_desc_device_t),
@@ -1774,6 +1791,12 @@ static void process_enumeration(tuh_xfer_t* xfer) {
       TU_LOG_USBH("Device configured\r\n");
       dev->configured = 1;
 
+    #if CFG_TUH_HUB
+      if (_usbh_data.dev0_bus.hub_addr != 0) {
+        hub_edpt_status_xfer(_usbh_data.dev0_bus.hub_addr); // get next hub status
+      }
+    #endif
+
       // Parse configuration & set up drivers
       // driver_open() must not make any usb transfer
       TU_ASSERT(enum_parse_configuration_desc(daddr, (tusb_desc_configuration_t*) _usbh_epbuf.ctrl),);
@@ -1787,7 +1810,7 @@ static void process_enumeration(tuh_xfer_t* xfer) {
     }
 
     default:
-      enum_full_complete(); // stop enumeration if unknown state
+      enum_full_complete(false); // stop enumeration if unknown state
       break;
   }
 }
@@ -1890,7 +1913,7 @@ void usbh_driver_set_config_complete(uint8_t dev_addr, uint8_t itf_num) {
 
   // all interface are configured
   if (itf_num == CFG_TUH_INTERFACE_MAX) {
-    enum_full_complete();
+    enum_full_complete(true);
 
     if (is_hub_addr(dev_addr)) {
       TU_LOG_USBH("HUB address = %u is mounted\r\n", dev_addr);
@@ -1901,12 +1924,14 @@ void usbh_driver_set_config_complete(uint8_t dev_addr, uint8_t itf_num) {
   }
 }
 
-static void enum_full_complete(void) {
+static void enum_full_complete(bool success) {
+  (void)success;
   // mark enumeration as complete
   _usbh_data.enumerating_daddr = TUSB_INDEX_INVALID_8;
 
 #if CFG_TUH_HUB
-  if (_usbh_data.dev0_bus.hub_addr != 0) {
+  // Hub status is already requested in case of successful enumeration
+  if (_usbh_data.dev0_bus.hub_addr != 0 && !success) {
     hub_edpt_status_xfer(_usbh_data.dev0_bus.hub_addr); // get next hub status
   }
 #endif