refactor hub class

- separate connect/disconnect handling
- hub work with full speed, but doesn't seem to work with Low speed
device (with mcb1800)
- need to update msc host after migrating from isr to xfer_cb (blocked
at inquiry)

examples/host/cdc_msc_hid/src/tusb_config.h

@@ -69,7 +69,7 @@
 // CONFIGURATION
 //--------------------------------------------------------------------
 
-#define CFG_TUH_HUB                 0
+#define CFG_TUH_HUB                 1
 #define CFG_TUH_CDC                 1
 #define CFG_TUH_HID_KEYBOARD        1
 #define CFG_TUH_HID_MOUSE           1

src/host/ehci/ehci.c

@@ -533,7 +533,7 @@ static void async_list_xfer_complete_isr(ehci_qhd_t * const async_head)
 
 static void period_list_xfer_complete_isr(uint8_t hostid, uint8_t interval_ms)
 {
-  uint8_t max_loop = 0;
+  uint16_t max_loop = 0;
   uint32_t const period_1ms_addr = (uint32_t) get_period_head(hostid, 1);
   ehci_link_t next_item = * get_period_head(hostid, interval_ms);
 

src/host/hcd.h

@@ -55,10 +55,11 @@ typedef struct
 
   union
   {
+    // Attach, Remove
     struct {
       uint8_t hub_addr;
       uint8_t hub_port;
-    } attach, remove;
+    } connection;
 
     // XFER_COMPLETE
     struct {

src/host/hub.c

@@ -33,6 +33,8 @@
 //--------------------------------------------------------------------+
 #include "hub.h"
 
+extern void osal_task_delay(uint32_t msec); // TODO remove
+
 //--------------------------------------------------------------------+
 // MACRO CONSTANT TYPEDEF
 //--------------------------------------------------------------------+
@@ -53,7 +55,7 @@ TU_ATTR_ALIGNED(4) CFG_TUSB_MEM_SECTION static uint8_t hub_enum_buffer[sizeof(de
 //--------------------------------------------------------------------+
 // HUB
 //--------------------------------------------------------------------+
-bool hub_port_clear_feature_subtask(uint8_t hub_addr, uint8_t hub_port, uint8_t feature)
+bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature)
 {
   TU_ASSERT(HUB_FEATURE_PORT_CONNECTION_CHANGE <= feature && feature <= HUB_FEATURE_PORT_RESET_CHANGE);
 
@@ -65,33 +67,35 @@ bool hub_port_clear_feature_subtask(uint8_t hub_addr, uint8_t hub_port, uint8_t
           .wLength = 0
   };
 
-  //------------- Clear Port Feature request -------------//
   TU_ASSERT( usbh_control_xfer( hub_addr, &request, NULL ) );
+  return true;
+}
 
-  //------------- Get Port Status to check if feature is cleared -------------//
+bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, hub_port_status_response_t* resp)
-  request = (tusb_control_request_t ) {
+{
-        .bmRequestType_bit = { .recipient = TUSB_REQ_RCPT_OTHER, .type = TUSB_REQ_TYPE_CLASS, .direction = TUSB_DIR_IN },
+  tusb_control_request_t request =
-        .bRequest = HUB_REQUEST_GET_STATUS,
+  {
-        .wValue = 0,
+    .bmRequestType_bit =
-        .wIndex = hub_port,
+    {
-        .wLength = 4
+      .recipient = TUSB_REQ_RCPT_OTHER,
+      .type      = TUSB_REQ_TYPE_CLASS,
+      .direction = TUSB_DIR_IN
+    },
+
+    .bRequest = HUB_REQUEST_GET_STATUS,
+    .wValue   = 0,
+    .wIndex   = hub_port,
+    .wLength  = 4
   };
 
   TU_ASSERT( usbh_control_xfer( hub_addr, &request, hub_enum_buffer ) );
 
-  //------------- Check if feature is cleared -------------//
+  memcpy(resp, hub_enum_buffer, sizeof(hub_port_status_response_t));
-  hub_port_status_response_t * p_port_status;
-  p_port_status = (hub_port_status_response_t *) hub_enum_buffer;
-
-  TU_ASSERT( !tu_bit_test(p_port_status->status_change.value, feature-16)  );
-
   return true;
 }
 
-bool hub_port_reset_subtask(uint8_t hub_addr, uint8_t hub_port)
+bool hub_port_reset(uint8_t hub_addr, uint8_t hub_port)
 {
-  enum { RESET_DELAY = 200 }; // USB specs say only 50ms but many devices require much longer
-
   //------------- Set Port Reset -------------//
   tusb_control_request_t request = {
           .bmRequestType_bit = { .recipient = TUSB_REQ_RCPT_OTHER, .type = TUSB_REQ_TYPE_CLASS, .direction = TUSB_DIR_OUT },
@@ -102,37 +106,9 @@ bool hub_port_reset_subtask(uint8_t hub_addr, uint8_t hub_port)
   };
 
   TU_ASSERT( usbh_control_xfer( hub_addr, &request, NULL ) );
-
-  osal_task_delay(RESET_DELAY); // TODO Hub wait for Status Endpoint on Reset Change
-
-  //------------- Get Port Status to check if port is enabled, powered and reset_change -------------//
-  request = (tusb_control_request_t ) {
-        .bmRequestType_bit = { .recipient = TUSB_REQ_RCPT_OTHER, .type = TUSB_REQ_TYPE_CLASS, .direction = TUSB_DIR_IN },
-        .bRequest = HUB_REQUEST_GET_STATUS,
-        .wValue = 0,
-        .wIndex = hub_port,
-        .wLength = 4
-  };
-
-  TU_ASSERT( usbh_control_xfer( hub_addr, &request, hub_enum_buffer ) );
-
-  hub_port_status_response_t * p_port_status;
-  p_port_status = (hub_port_status_response_t *) hub_enum_buffer;
-
-  TU_ASSERT ( p_port_status->status_change.reset && p_port_status->status_current.connect_status &&
-              p_port_status->status_current.port_power && p_port_status->status_current.port_enable);
-
   return true;
 }
 
-// can only get the speed RIGHT AFTER hub_port_reset_subtask call
-tusb_speed_t hub_port_get_speed(void)
-{
-  hub_port_status_response_t * p_port_status = (hub_port_status_response_t *) hub_enum_buffer;
-  return (p_port_status->status_current.high_speed_device_attached) ? TUSB_SPEED_HIGH :
-         (p_port_status->status_current.low_speed_device_attached ) ? TUSB_SPEED_LOW  : TUSB_SPEED_FULL;
-}
-
 //--------------------------------------------------------------------+
 // CLASS-USBH API (don't require to verify parameters)
 //--------------------------------------------------------------------+
@@ -199,31 +175,49 @@ bool hub_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *itf
 
 // is the response of interrupt endpoint polling
 #include "usbh_hcd.h" // FIXME remove
-void hub_xfer_cb(uint8_t dev_addr, uint8_t ep_addr, xfer_result_t event, uint32_t xferred_bytes)
+void hub_xfer_cb(uint8_t dev_addr, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes)
 {
   (void) xferred_bytes; // TODO can be more than 1 for hub with lots of ports
   (void) ep_addr;
 
   usbh_hub_t * p_hub = &hub_data[dev_addr-1];
 
-  if ( event == XFER_RESULT_SUCCESS )
+  if ( result == XFER_RESULT_SUCCESS )
   {
+    TU_LOG2("Port Status Change = 0x%02X\r\n", p_hub->status_change);
     for (uint8_t port=1; port <= p_hub->port_number; port++)
     {
       // TODO HUB ignore bit0 hub_status_change
       if ( tu_bit_test(p_hub->status_change, port) )
       {
-        hcd_event_t event =
+        hub_port_status_response_t port_status;
+        hub_port_get_status(dev_addr, port, &port_status);
+
+        // Connection change
+        if (port_status.change.connection)
         {
-          .rhport = _usbh_devices[dev_addr].rhport,
+          // Port is powered and enabled
-          .event_id = HCD_EVENT_DEVICE_ATTACH
+          //TU_VERIFY(port_status.status_current.port_power && port_status.status_current.port_enable, );
-        };
 
-        event.attach.hub_addr = dev_addr;
+          // Acknowledge Port Connection Change
-        event.attach.hub_port = port;
+          hub_port_clear_feature(dev_addr, port, HUB_FEATURE_PORT_CONNECTION_CHANGE);
 
-        hcd_event_handler(&event, true);
+          // Reset port if attach event
-        break; // TODO handle one port at a time, next port if any will be handled in the next cycle
+          if ( port_status.status.connection ) hub_port_reset(dev_addr, port);
+
+          hcd_event_t event =
+          {
+            .rhport     = _usbh_devices[dev_addr].rhport,
+            .event_id   = port_status.status.connection ? HCD_EVENT_DEVICE_ATTACH : HCD_EVENT_DEVICE_REMOVE,
+            .connection =
+            {
+              .hub_addr = dev_addr,
+              .hub_port = port
+            }
+          };
+
+          hcd_event_handler(&event, true);
+        }
       }
     }
     // NOTE: next status transfer is queued by usbh.c after handling this request

src/host/hub.h

@@ -142,7 +142,7 @@ typedef struct {
     };
 
     uint16_t value;
-  } status, status_change;
+  } status, change;
 } hub_status_response_t;
 
 TU_VERIFY_STATIC( sizeof(hub_status_response_t) == 4, "size is not correct");
@@ -151,30 +151,30 @@ TU_VERIFY_STATIC( sizeof(hub_status_response_t) == 4, "size is not correct");
 typedef struct {
   union {
     struct TU_ATTR_PACKED {
-      uint16_t connect_status             : 1;
+      uint16_t connection             : 1;
-      uint16_t port_enable                : 1;
+      uint16_t port_enable            : 1;
-      uint16_t suspend                    : 1;
+      uint16_t suspend                : 1;
-      uint16_t over_current               : 1;
+      uint16_t over_current           : 1;
-      uint16_t reset                      : 1;
+      uint16_t reset                  : 1;
 
-      uint16_t                            : 3;
+      uint16_t                        : 3;
-      uint16_t port_power                 : 1;
+      uint16_t port_power             : 1;
-      uint16_t low_speed_device_attached  : 1;
+      uint16_t low_speed              : 1;
-      uint16_t high_speed_device_attached : 1;
+      uint16_t high_speed             : 1;
-      uint16_t port_test_mode             : 1;
+      uint16_t port_test_mode         : 1;
-      uint16_t port_indicator_control     : 1;
+      uint16_t port_indicator_control : 1;
       uint16_t : 0;
     };
 
     uint16_t value;
-  } status_current, status_change;
+  } status, change;
 } hub_port_status_response_t;
 
 TU_VERIFY_STATIC( sizeof(hub_port_status_response_t) == 4, "size is not correct");
 
-bool hub_port_reset_subtask(uint8_t hub_addr, uint8_t hub_port);
+bool hub_port_reset(uint8_t hub_addr, uint8_t hub_port);
-bool hub_port_clear_feature_subtask(uint8_t hub_addr, uint8_t hub_port, uint8_t feature);
+bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, hub_port_status_response_t* resp);
-tusb_speed_t hub_port_get_speed(void);
+bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature);
 bool hub_status_pipe_queue(uint8_t dev_addr);
 
 //--------------------------------------------------------------------+

src/host/usbh.c

@@ -137,7 +137,7 @@ tusb_device_state_t tuh_device_get_state (uint8_t const dev_addr)
   return (tusb_device_state_t) _usbh_devices[dev_addr].state;
 }
 
-static inline void osal_task_delay(uint32_t msec)
+void osal_task_delay(uint32_t msec)
 {
   (void) msec;
 
@@ -305,8 +305,8 @@ void hcd_event_device_attach(uint8_t rhport, bool in_isr)
     .event_id = HCD_EVENT_DEVICE_ATTACH
   };
 
-  event.attach.hub_addr = 0;
+  event.connection.hub_addr = 0;
-  event.attach.hub_port = 0;
+  event.connection.hub_port = 0;
 
   hcd_event_handler(&event, in_isr);
 }
@@ -319,8 +319,8 @@ void hcd_event_device_remove(uint8_t hostid, bool in_isr)
     .event_id = HCD_EVENT_DEVICE_REMOVE
   };
 
-  event.attach.hub_addr = 0;
+  event.connection.hub_addr = 0;
-  event.attach.hub_port = 0;
+  event.connection.hub_port = 0;
 
   hcd_event_handler(&event, in_isr);
 }
@@ -434,78 +434,44 @@ bool enum_task(hcd_event_t* event)
   tusb_control_request_t request;
 
   dev0->rhport   = event->rhport; // TODO refractor integrate to device_pool
-  dev0->hub_addr = event->attach.hub_addr;
+  dev0->hub_addr = event->connection.hub_addr;
-  dev0->hub_port = event->attach.hub_port;
+  dev0->hub_port = event->connection.hub_port;
   dev0->state    = TUSB_DEVICE_STATE_UNPLUG;
 
   //------------- connected/disconnected directly with roothub -------------//
   if ( dev0->hub_addr == 0)
   {
-    if( hcd_port_connect_status(dev0->rhport) )
+    // wait until device is stable. Increase this if the first 8 bytes is failed to get
-    {
+    osal_task_delay(POWER_STABLE_DELAY);
-      TU_LOG2("Device connect \r\n");
 
-      // connection event
+    // device unplugged while delaying
-      osal_task_delay(POWER_STABLE_DELAY); // wait until device is stable. Increase this if the first 8 bytes is failed to get
+    if ( !hcd_port_connect_status(dev0->rhport) ) return true;
 
-      // exit if device unplugged while delaying
+    hcd_port_reset( dev0->rhport ); // port must be reset to have correct speed operation
-      if ( !hcd_port_connect_status(dev0->rhport) ) return true;
+    osal_task_delay(RESET_DELAY);
 
-      hcd_port_reset( dev0->rhport ); // port must be reset to have correct speed operation
+    dev0->speed = hcd_port_speed_get( dev0->rhport );
-      osal_task_delay(RESET_DELAY);
-
-      dev0->speed = hcd_port_speed_get( dev0->rhport );
-    }
-    else
-    {
-      TU_LOG2("Device disconnect \r\n");
-
-      // disconnection event
-      usbh_device_unplugged(dev0->rhport, 0, 0);
-      return true; // restart task
-    }
   }
 #if CFG_TUH_HUB
   //------------- connected/disconnected via hub -------------//
   else
   {
-    //------------- Get Port Status -------------//
+    // TODO wait for PORT reset change instead
-    request = (tusb_control_request_t ) {
+    osal_task_delay(POWER_STABLE_DELAY);
-          .bmRequestType_bit = { .recipient = TUSB_REQ_RCPT_OTHER, .type = TUSB_REQ_TYPE_CLASS, .direction = TUSB_DIR_IN },
-          .bRequest = HUB_REQUEST_GET_STATUS,
-          .wValue = 0,
-          .wIndex = dev0->hub_port,
-          .wLength = 4
-    };
-    // TODO hub refractor
-    TU_VERIFY_HDLR( usbh_control_xfer( dev0->hub_addr, &request, _usbh_ctrl_buf ), hub_status_pipe_queue( dev0->hub_addr) );
 
-    // Acknowledge Port Connection Change
+    hub_port_status_response_t port_status;
-    hub_port_clear_feature_subtask(dev0->hub_addr, dev0->hub_port, HUB_FEATURE_PORT_CONNECTION_CHANGE);
+    TU_VERIFY_HDLR( hub_port_get_status(dev0->hub_addr, dev0->hub_port, &port_status), hub_status_pipe_queue( dev0->hub_addr) );
 
-    hub_port_status_response_t * p_port_status;
+    // device unplugged while delaying
-    p_port_status = ((hub_port_status_response_t *) _usbh_ctrl_buf);
+    if ( !port_status.status.connection ) return true;
 
-    if ( ! p_port_status->status_change.connect_status )   return true; // only handle connection change
+    dev0->speed = (port_status.status.high_speed) ? TUSB_SPEED_HIGH :
+                  (port_status.status.low_speed ) ? TUSB_SPEED_LOW  : TUSB_SPEED_FULL;
 
-    if ( ! p_port_status->status_current.connect_status )
+    // Acknowledge Port Reset Change
+    if (port_status.change.reset)
     {
-      // Disconnection event
+      hub_port_clear_feature(dev0->hub_addr, dev0->hub_port, HUB_FEATURE_PORT_RESET_CHANGE);
-      usbh_device_unplugged(dev0->rhport, dev0->hub_addr, dev0->hub_port);
-
-      (void) hub_status_pipe_queue( dev0->hub_addr ); // done with hub, waiting for next data on status pipe
-      return true; // restart task
-    }
-    else
-    {
-      // Connection Event
-      TU_VERIFY_HDLR(hub_port_reset_subtask(dev0->hub_addr, dev0->hub_port),
-                     hub_status_pipe_queue( dev0->hub_addr) ); // TODO hub refractor
-
-      dev0->speed = hub_port_get_speed();
-
-      // Acknowledge Port Reset Change
-      hub_port_clear_feature_subtask(dev0->hub_addr, dev0->hub_port, HUB_FEATURE_PORT_RESET_CHANGE);
     }
   }
 #endif // CFG_TUH_HUB
@@ -540,10 +506,12 @@ bool enum_task(hcd_event_t* event)
     // connected via a hub
     TU_VERIFY_HDLR(is_ok, hub_status_pipe_queue( dev0->hub_addr) ); // TODO hub refractor
 
-    if ( hub_port_reset_subtask(dev0->hub_addr, dev0->hub_port) )
+    if ( hub_port_reset(dev0->hub_addr, dev0->hub_port) )
     {
+      osal_task_delay(RESET_DELAY);
+
       // Acknowledge Port Reset Change if Reset Successful
-      hub_port_clear_feature_subtask(dev0->hub_addr, dev0->hub_port, HUB_FEATURE_PORT_RESET_CHANGE);
+      hub_port_clear_feature(dev0->hub_addr, dev0->hub_port, HUB_FEATURE_PORT_RESET_CHANGE);
     }
 
     (void) hub_status_pipe_queue( dev0->hub_addr ); // done with hub, waiting for next data on status pipe
@@ -676,10 +644,24 @@ void tuh_task(void)
     switch (event.event_id)
     {
       case HCD_EVENT_DEVICE_ATTACH:
-      case HCD_EVENT_DEVICE_REMOVE:
+        TU_LOG2("USBH DEVICE ATTACH\r\n");
         enum_task(&event);
       break;
 
+      case HCD_EVENT_DEVICE_REMOVE:
+        TU_LOG2("USBH DEVICE REMOVED\r\n");
+        usbh_device_unplugged(event.rhport, event.connection.hub_addr, event.connection.hub_port);
+
+        #if CFG_TUH_HUB
+        // TODO remove
+        if ( event.connection.hub_addr != 0)
+        {
+          // done with hub, waiting for next data on status pipe
+          (void) hub_status_pipe_queue( event.connection.hub_addr );
+        }
+        #endif
+      break;
+
       case HCD_EVENT_XFER_COMPLETE:
       {
         usbh_device_t* dev = &_usbh_devices[event.dev_addr];