Merge pull request #868 from hathach/host-hcd

Clean up Host HCD API

5 files changed, 158 insertions, 156 deletions

diff --git a/src/host/hcd.h b/src/host/hcd.h
index ba6a9c5ca..46209dc45 100644
--- a/src/host/hcd.h
+++ b/src/host/hcd.h
@@ -28,6 +28,8 @@
 #define _TUSB_HCD_H_
 
 #include "common/tusb_common.h"
+#include "osal/osal.h"
+#include "common/tusb_fifo.h"
 
 #ifdef __cplusplus
  extern "C" {
@@ -106,15 +108,8 @@ void hcd_int_enable (uint8_t rhport);
 // Disable USB interrupt
 void hcd_int_disable(uint8_t rhport);
 
-// Get micro frame number (125 us)
-uint32_t hcd_uframe_number(uint8_t rhport);
-
 // Get frame number (1ms)
-TU_ATTR_ALWAYS_INLINE static inline
-uint32_t hcd_frame_number(uint8_t rhport)
-{
-  return hcd_uframe_number(rhport) >> 3;
-}
+uint32_t hcd_frame_number(uint8_t rhport);
 
 //--------------------------------------------------------------------+
 // Port API
@@ -141,21 +136,12 @@ void hcd_device_close(uint8_t rhport, uint8_t dev_addr);
 
 bool hcd_setup_send(uint8_t rhport, uint8_t dev_addr, uint8_t const setup_packet[8]);
 bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_endpoint_t const * ep_desc);
+bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * buffer, uint16_t buflen);
 
 bool hcd_edpt_busy(uint8_t dev_addr, uint8_t ep_addr);
 bool hcd_edpt_stalled(uint8_t dev_addr, uint8_t ep_addr);
 bool hcd_edpt_clear_stall(uint8_t dev_addr, uint8_t ep_addr);
 
-// TODO merge with pipe_xfer
-bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * buffer, uint16_t buflen);
-
-//--------------------------------------------------------------------+
-// PIPE API - TODO remove later
-//--------------------------------------------------------------------+
-// TODO control xfer should be used via usbh layer
-bool hcd_pipe_queue_xfer(uint8_t dev_addr, uint8_t ep_addr, uint8_t buffer[], uint16_t total_bytes); // only queue, not transferring yet
-bool hcd_pipe_xfer(uint8_t dev_addr, uint8_t ep_addr, uint8_t buffer[], uint16_t total_bytes, bool int_on_complete);
-
 //--------------------------------------------------------------------+
 // Event API (implemented by stack)
 //--------------------------------------------------------------------+
diff --git a/src/host/hub.c b/src/host/hub.c
index 4db680f9e..f921027c7 100644
--- a/src/host/hub.c
+++ b/src/host/hub.c
@@ -42,13 +42,30 @@ typedef struct
   uint8_t status_change; // data from status change interrupt endpoint
 
   hub_port_status_response_t port_status;
-}usbh_hub_t;
+} hub_interface_t;
 
-CFG_TUSB_MEM_SECTION static usbh_hub_t hub_data[CFG_TUSB_HOST_DEVICE_MAX];
+CFG_TUSB_MEM_SECTION static hub_interface_t hub_data[CFG_TUSB_HOST_DEVICE_MAX];
 TU_ATTR_ALIGNED(4) CFG_TUSB_MEM_SECTION static uint8_t _hub_buffer[sizeof(descriptor_hub_desc_t)];
 
-//OSAL_SEM_DEF(hub_enum_semaphore);
-//static osal_semaphore_handle_t hub_enum_sem_hdl;
+#if CFG_TUSB_DEBUG
+static char const* const _hub_feature_str[] =
+{
+  [HUB_FEATURE_PORT_CONNECTION          ] = "PORT_CONNECTION",
+  [HUB_FEATURE_PORT_ENABLE              ] = "PORT_ENABLE",
+  [HUB_FEATURE_PORT_SUSPEND             ] = "PORT_SUSPEND",
+  [HUB_FEATURE_PORT_OVER_CURRENT        ] = "PORT_OVER_CURRENT",
+  [HUB_FEATURE_PORT_RESET               ] = "PORT_RESET",
+  [HUB_FEATURE_PORT_POWER               ] = "PORT_POWER",
+  [HUB_FEATURE_PORT_LOW_SPEED           ] = "PORT_LOW_SPEED",
+  [HUB_FEATURE_PORT_CONNECTION_CHANGE   ] = "PORT_CONNECTION_CHANGE",
+  [HUB_FEATURE_PORT_ENABLE_CHANGE       ] = "PORT_ENABLE_CHANGE",
+  [HUB_FEATURE_PORT_SUSPEND_CHANGE      ] = "PORT_SUSPEND_CHANGE",
+  [HUB_FEATURE_PORT_OVER_CURRENT_CHANGE ] = "PORT_OVER_CURRENT_CHANGE",
+  [HUB_FEATURE_PORT_RESET_CHANGE        ] = "PORT_RESET_CHANGE",
+  [HUB_FEATURE_PORT_TEST                ] = "PORT_TEST",
+  [HUB_FEATURE_PORT_INDICATOR           ] = "PORT_INDICATOR",
+};
+#endif
 
 //--------------------------------------------------------------------+
 // HUB
@@ -69,12 +86,12 @@ bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature,
     .wLength  = 0
   };
 
-  TU_LOG2("HUB Clear Port Feature: addr = %u port = %u, feature = %u\r\n", hub_addr, hub_port, feature);
+  TU_LOG2("HUB Clear Feature: %s, addr = %u port = %u\r\n", _hub_feature_str[feature], hub_addr, hub_port);
   TU_ASSERT( tuh_control_xfer(hub_addr, &request, NULL, complete_cb) );
   return true;
 }
 
-bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void* resp, tuh_control_complete_cb_t complete_cb)
+bool hub_port_set_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, tuh_control_complete_cb_t complete_cb)
 {
   tusb_control_request_t const request =
   {
@@ -82,37 +99,42 @@ bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void* resp, tuh_con
     {
       .recipient = TUSB_REQ_RCPT_OTHER,
       .type      = TUSB_REQ_TYPE_CLASS,
-      .direction = TUSB_DIR_IN
+      .direction = TUSB_DIR_OUT
     },
-    .bRequest = HUB_REQUEST_GET_STATUS,
-    .wValue   = 0,
+    .bRequest = HUB_REQUEST_SET_FEATURE,
+    .wValue   = feature,
     .wIndex   = hub_port,
-    .wLength  = 4
+    .wLength  = 0
   };
 
-  TU_LOG2("HUB Get Port Status: addr = %u port = %u\r\n", hub_addr, hub_port);
-  TU_ASSERT( tuh_control_xfer( hub_addr, &request, resp, complete_cb) );
+  TU_LOG2("HUB Set Feature: %s, addr = %u port = %u\r\n", _hub_feature_str[feature], hub_addr, hub_port);
+  TU_ASSERT( tuh_control_xfer(hub_addr, &request, NULL, complete_cb) );
   return true;
 }
 
 bool hub_port_reset(uint8_t hub_addr, uint8_t hub_port, tuh_control_complete_cb_t complete_cb)
 {
+  return hub_port_set_feature(hub_addr, hub_port, HUB_FEATURE_PORT_RESET, complete_cb);
+}
+
+bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void* resp, tuh_control_complete_cb_t complete_cb)
+{
   tusb_control_request_t const request =
   {
     .bmRequestType_bit =
     {
       .recipient = TUSB_REQ_RCPT_OTHER,
       .type      = TUSB_REQ_TYPE_CLASS,
-      .direction = TUSB_DIR_OUT
+      .direction = TUSB_DIR_IN
     },
-    .bRequest = HUB_REQUEST_SET_FEATURE,
-    .wValue   = HUB_FEATURE_PORT_RESET,
+    .bRequest = HUB_REQUEST_GET_STATUS,
+    .wValue   = 0,
     .wIndex   = hub_port,
-    .wLength  = 0
+    .wLength  = 4
   };
 
-  TU_LOG2("HUB Reset Port: addr = %u port = %u\r\n", hub_addr, hub_port);
-  TU_ASSERT( tuh_control_xfer(hub_addr, &request, NULL, complete_cb) );
+  TU_LOG2("HUB Get Port Status: addr = %u port = %u\r\n", hub_addr, hub_port);
+  TU_ASSERT( tuh_control_xfer( hub_addr, &request, resp, complete_cb) );
   return true;
 }
 
@@ -121,8 +143,7 @@ bool hub_port_reset(uint8_t hub_addr, uint8_t hub_port, tuh_control_complete_cb_
 //--------------------------------------------------------------------+
 void hub_init(void)
 {
-  tu_memclr(hub_data, CFG_TUSB_HOST_DEVICE_MAX*sizeof(usbh_hub_t));
-//  hub_enum_sem_hdl = osal_semaphore_create( OSAL_SEM_REF(hub_enum_semaphore) );
+  tu_memclr(hub_data, CFG_TUSB_HOST_DEVICE_MAX*sizeof( hub_interface_t));
 }
 
 bool hub_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *itf_desc, uint16_t *p_length)
@@ -147,46 +168,72 @@ bool hub_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_interface_t const *itf
   return true;
 }
 
-static bool config_get_hub_desc_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
-static bool config_port_power_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
+void hub_close(uint8_t dev_addr)
+{
+  tu_memclr(&hub_data[dev_addr-1], sizeof( hub_interface_t));
+}
 
-static bool config_get_hub_desc_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result)
+bool hub_status_pipe_queue(uint8_t dev_addr)
 {
-  (void) request;
-  TU_ASSERT(XFER_RESULT_SUCCESS == result);
+  hub_interface_t * p_hub = &hub_data[dev_addr-1];
+  return usbh_edpt_xfer(dev_addr, p_hub->ep_in, &p_hub->status_change, 1);
+}
 
-  usbh_hub_t* p_hub = &hub_data[dev_addr-1];
 
-  // only use number of ports in hub descriptor
-  descriptor_hub_desc_t const* desc_hub = (descriptor_hub_desc_t const*) _hub_buffer;
-  p_hub->port_count = desc_hub->bNbrPorts;
+//--------------------------------------------------------------------+
+// Set Configure
+//--------------------------------------------------------------------+
 
-  // May need to GET_STATUS
+static bool config_set_port_power (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
+static bool config_port_power_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
 
-  // Ports must be powered on to be able to detect connection
-  tusb_control_request_t const new_request =
+bool hub_set_config(uint8_t dev_addr, uint8_t itf_num)
+{
+   hub_interface_t* p_hub = &hub_data[dev_addr-1];
+  TU_ASSERT(itf_num == p_hub->itf_num);
+
+  // Get Hub Descriptor
+  tusb_control_request_t const request =
   {
     .bmRequestType_bit =
     {
-      .recipient = TUSB_REQ_RCPT_OTHER,
+      .recipient = TUSB_REQ_RCPT_DEVICE,
       .type      = TUSB_REQ_TYPE_CLASS,
-      .direction = TUSB_DIR_OUT
+      .direction = TUSB_DIR_IN
     },
-    .bRequest = HUB_REQUEST_SET_FEATURE,
-    .wValue   = HUB_FEATURE_PORT_POWER,
-    .wIndex   = 1, // starting with port 1
-    .wLength  = 0
+    .bRequest = HUB_REQUEST_GET_DESCRIPTOR,
+    .wValue   = 0,
+    .wIndex   = 0,
+    .wLength  = sizeof(descriptor_hub_desc_t)
   };
 
-  TU_ASSERT( tuh_control_xfer(dev_addr, &new_request, NULL, config_port_power_complete) );
+  TU_ASSERT( tuh_control_xfer(dev_addr, &request, _hub_buffer, config_set_port_power) );
 
   return true;
 }
 
+static bool config_set_port_power (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result)
+{
+  (void) request;
+  TU_ASSERT(XFER_RESULT_SUCCESS == result);
+
+  hub_interface_t* p_hub = &hub_data[dev_addr-1];
+
+  // only use number of ports in hub descriptor
+  descriptor_hub_desc_t const* desc_hub = (descriptor_hub_desc_t const*) _hub_buffer;
+  p_hub->port_count = desc_hub->bNbrPorts;
+
+  // May need to GET_STATUS
+
+  // Set Port Power to be able to detect connection, starting with port 1
+  uint8_t const hub_port = 1;
+  return hub_port_set_feature(dev_addr, hub_port, HUB_FEATURE_PORT_POWER, config_port_power_complete);
+}
+
 static bool config_port_power_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result)
 {
   TU_ASSERT(XFER_RESULT_SUCCESS == result);
-  usbh_hub_t* p_hub = &hub_data[dev_addr-1];
+   hub_interface_t* p_hub = &hub_data[dev_addr-1];
 
   if (request->wIndex == p_hub->port_count)
   {
@@ -197,64 +244,71 @@ static bool config_port_power_complete (uint8_t dev_addr, tusb_control_request_t
     usbh_driver_set_config_complete(dev_addr, p_hub->itf_num);
   }else
   {
-    tusb_control_request_t new_request = *request;
-    new_request.wIndex++; // power next port
-
-    TU_ASSERT( tuh_control_xfer(dev_addr, &new_request, NULL, config_port_power_complete) );
+    // power next port
+    uint8_t const hub_port = (uint8_t) (request->wIndex + 1);
+    return hub_port_set_feature(dev_addr, hub_port, HUB_FEATURE_PORT_POWER, config_port_power_complete);
   }
 
   return true;
 }
 
-bool hub_set_config(uint8_t dev_addr, uint8_t itf_num)
+//--------------------------------------------------------------------+
+// Connection Changes
+//--------------------------------------------------------------------+
+
+static bool connection_get_status_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
+static bool connection_clear_conn_change_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
+static bool connection_port_reset_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
+
+// callback as response of interrupt endpoint polling
+bool hub_xfer_cb(uint8_t dev_addr, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes)
 {
-  usbh_hub_t* p_hub = &hub_data[dev_addr-1];
-  TU_ASSERT(itf_num == p_hub->itf_num);
+  (void) xferred_bytes; // TODO can be more than 1 for hub with lots of ports
+  (void) ep_addr;
+  TU_ASSERT(result == XFER_RESULT_SUCCESS);
 
-  //------------- Get Hub Descriptor -------------//
-  tusb_control_request_t request =
+  hub_interface_t * p_hub = &hub_data[dev_addr-1];
+
+  TU_LOG2("  Port Status Change = 0x%02X\r\n", p_hub->status_change);
+
+  // Hub ignore bit0 in status change
+  for (uint8_t port=1; port <= p_hub->port_count; port++)
   {
-    .bmRequestType_bit =
+    if ( tu_bit_test(p_hub->status_change, port) )
     {
-      .recipient = TUSB_REQ_RCPT_DEVICE,
-      .type      = TUSB_REQ_TYPE_CLASS,
-      .direction = TUSB_DIR_IN
-    },
-    .bRequest = HUB_REQUEST_GET_DESCRIPTOR,
-    .wValue   = 0,
-    .wIndex   = 0,
-    .wLength  = sizeof(descriptor_hub_desc_t)
-  };
+      hub_port_get_status(dev_addr, port, &p_hub->port_status, connection_get_status_complete);
+      break;
+    }
+  }
 
-  TU_ASSERT( tuh_control_xfer(dev_addr, &request, _hub_buffer, config_get_hub_desc_complete) );
+  // NOTE: next status transfer is queued by usbh.c after handling this request
 
   return true;
 }
 
-static bool connection_clear_conn_change_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
-static bool connection_get_status_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
-static bool connection_port_reset_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result);
-
-static bool connection_port_reset_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result)
+static bool connection_get_status_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result)
 {
   TU_ASSERT(result == XFER_RESULT_SUCCESS);
-
-  // usbh_hub_t * p_hub = &hub_data[dev_addr-1];
+  hub_interface_t * p_hub = &hub_data[dev_addr-1];
   uint8_t const port_num = (uint8_t) request->wIndex;
 
-  // submit attach event
-  hcd_event_t event =
+  // Connection change
+  if (p_hub->port_status.change.connection)
   {
-    .rhport     = usbh_get_rhport(dev_addr),
-    .event_id   = HCD_EVENT_DEVICE_ATTACH,
-    .connection =
-    {
-      .hub_addr = dev_addr,
-      .hub_port = port_num
-    }
-  };
+    // Port is powered and enabled
+    //TU_VERIFY(port_status.status_current.port_power && port_status.status_current.port_enable, );
 
-  hcd_event_handler(&event, false);
+    // Acknowledge Port Connection Change
+    hub_port_clear_feature(dev_addr, port_num, HUB_FEATURE_PORT_CONNECTION_CHANGE, connection_clear_conn_change_complete);
+  }else
+  {
+    // Other changes are: Enable, Suspend, Over Current, Reset, L1 state
+    // TODO clear change
+
+    // prepare for next hub status
+    // TODO continue with status_change, or maybe we can do it again with status
+    hub_status_pipe_queue(dev_addr);
+  }
 
   return true;
 }
@@ -263,7 +317,7 @@ static bool connection_clear_conn_change_complete (uint8_t dev_addr, tusb_contro
 {
   TU_ASSERT(result == XFER_RESULT_SUCCESS);
 
-  usbh_hub_t * p_hub = &hub_data[dev_addr-1];
+  hub_interface_t * p_hub = &hub_data[dev_addr-1];
   uint8_t const port_num = (uint8_t) request->wIndex;
 
   if ( p_hub->port_status.status.connection )
@@ -290,70 +344,28 @@ static bool connection_clear_conn_change_complete (uint8_t dev_addr, tusb_contro
   return true;
 }
 
-static bool connection_get_status_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result)
+static bool connection_port_reset_complete (uint8_t dev_addr, tusb_control_request_t const * request, xfer_result_t result)
 {
   TU_ASSERT(result == XFER_RESULT_SUCCESS);
-  usbh_hub_t * p_hub = &hub_data[dev_addr-1];
-  uint8_t const port_num = (uint8_t) request->wIndex;
-
-  // Connection change
-  if (p_hub->port_status.change.connection)
-  {
-    // Port is powered and enabled
-    //TU_VERIFY(port_status.status_current.port_power && port_status.status_current.port_enable, );
-
-    // Acknowledge Port Connection Change
-    hub_port_clear_feature(dev_addr, port_num, HUB_FEATURE_PORT_CONNECTION_CHANGE, connection_clear_conn_change_complete);
-  }else
-  {
-    // Other changes are: Enable, Suspend, Over Current, Reset, L1 state
-    // TODO clear change
-
-    // prepare for next hub status
-    // TODO continue with status_change, or maybe we can do it again with status
-    hub_status_pipe_queue(dev_addr);
-  }
-
-  return true;
-}
-
-// is the response of interrupt endpoint polling
-#include "usbh_hcd.h" // FIXME remove
-bool 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;
-  TU_ASSERT( result == XFER_RESULT_SUCCESS);
 
-  usbh_hub_t * p_hub = &hub_data[dev_addr-1];
+  // usbh_hub_t * p_hub = &hub_data[dev_addr-1];
+  uint8_t const port_num = (uint8_t) request->wIndex;
 
-  TU_LOG2("Port Status Change = 0x%02X\r\n", p_hub->status_change);
-  for (uint8_t port=1; port <= p_hub->port_count; port++)
+  // submit attach event
+  hcd_event_t event =
   {
-    // TODO HUB ignore bit0 hub_status_change
-    if ( tu_bit_test(p_hub->status_change, port) )
+    .rhport     = usbh_get_rhport(dev_addr),
+    .event_id   = HCD_EVENT_DEVICE_ATTACH,
+    .connection =
     {
-      hub_port_get_status(dev_addr, port, &p_hub->port_status, connection_get_status_complete);
-      break;
+      .hub_addr = dev_addr,
+      .hub_port = port_num
     }
-  }
+  };
 
-  // NOTE: next status transfer is queued by usbh.c after handling this request
+  hcd_event_handler(&event, false);
 
   return true;
 }
 
-void hub_close(uint8_t dev_addr)
-{
-  tu_memclr(&hub_data[dev_addr-1], sizeof(usbh_hub_t));
-//  osal_semaphore_reset(hub_enum_sem_hdl);
-}
-
-bool hub_status_pipe_queue(uint8_t dev_addr)
-{
-  usbh_hub_t * p_hub = &hub_data[dev_addr-1];
-  return hcd_pipe_xfer(dev_addr, p_hub->ep_in, &p_hub->status_change, 1, true);
-}
-
-
 #endif
diff --git a/src/host/hub.h b/src/host/hub.h
index 2b2f39ee1..a5111b8e7 100644
--- a/src/host/hub.h
+++ b/src/host/hub.h
@@ -171,9 +171,11 @@ typedef struct {
 
 TU_VERIFY_STATIC( sizeof(hub_port_status_response_t) == 4, "size is not correct");
 
+bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, tuh_control_complete_cb_t complete_cb);
+bool hub_port_set_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, tuh_control_complete_cb_t complete_cb);
+
 bool hub_port_reset(uint8_t hub_addr, uint8_t hub_port, tuh_control_complete_cb_t complete_cb);
 bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void* resp, tuh_control_complete_cb_t complete_cb);
-bool hub_port_clear_feature(uint8_t hub_addr, uint8_t hub_port, uint8_t feature, tuh_control_complete_cb_t complete_cb);
 bool hub_status_pipe_queue(uint8_t dev_addr);
 
 //--------------------------------------------------------------------+
diff --git a/src/host/usbh.c b/src/host/usbh.c
index ac0b654ef..ca0653409 100644
--- a/src/host/usbh.c
+++ b/src/host/usbh.c
@@ -378,6 +378,8 @@ bool usbh_edpt_xfer(uint8_t dev_addr, uint8_t ep_addr, uint8_t * buffer, uint16_
 
 bool usbh_edpt_control_open(uint8_t dev_addr, uint8_t max_packet_size)
 {
+  TU_LOG2("Open EP Control with Size = %u\r\n", max_packet_size);
+
   tusb_desc_endpoint_t ep0_desc =
   {
     .bLength          = sizeof(tusb_desc_endpoint_t),
@@ -455,7 +457,7 @@ void hcd_event_device_attach(uint8_t rhport, bool in_isr)
 {
   hcd_event_t event =
   {
-    .rhport = rhport,
+    .rhport   = rhport,
     .event_id = HCD_EVENT_DEVICE_ATTACH
   };
 
@@ -469,7 +471,7 @@ void hcd_event_device_remove(uint8_t hostid, bool in_isr)
 {
   hcd_event_t event =
   {
-    .rhport = hostid,
+    .rhport   = hostid,
     .event_id = HCD_EVENT_DEVICE_REMOVE
   };
 
diff --git a/src/host/usbh_control.c b/src/host/usbh_control.c
index 974d10662..4dbf8592a 100644
--- a/src/host/usbh_control.c
+++ b/src/host/usbh_control.c
@@ -108,7 +108,7 @@ bool usbh_control_xfer_cb (uint8_t dev_addr, uint8_t ep_addr, xfer_result_t resu
         _ctrl_xfer.stage = STAGE_DATA;
         if (request->wLength)
         {
-          // Note: initial data toggle is always 1
+          // DATA stage: initial data toggle is always 1
           hcd_edpt_xfer(rhport, dev_addr, tu_edpt_addr(0, request->bmRequestType_bit.direction), _ctrl_xfer.buffer, request->wLength);
           return true;
         }
@@ -123,7 +123,7 @@ bool usbh_control_xfer_cb (uint8_t dev_addr, uint8_t ep_addr, xfer_result_t resu
           TU_LOG2_MEM(_ctrl_xfer.buffer, request->wLength, 2);
         }
 
-        // data toggle is always 1
+        // ACK stage: toggle is always 1
         hcd_edpt_xfer(rhport, dev_addr, tu_edpt_addr(0, 1-request->bmRequestType_bit.direction), NULL, 0);
       break;