Merge pull request #3007 from hathach/fix-8bitdo-enum

Fix(host) enumeration with 8bitdo devices

examples/host/cdc_msc_hid/src/hid_app.c

@@ -39,18 +39,16 @@
 static uint8_t const keycode2ascii[128][2] =  { HID_KEYCODE_TO_ASCII };
 
 // Each HID instance can has multiple reports
-static struct
+static struct {
-{
   uint8_t report_count;
   tuh_hid_report_info_t report_info[MAX_REPORT];
-}hid_info[CFG_TUH_HID];
+} hid_info[CFG_TUH_HID];
 
 static void process_kbd_report(hid_keyboard_report_t const *report);
 static void process_mouse_report(hid_mouse_report_t const * report);
 static void process_generic_report(uint8_t dev_addr, uint8_t instance, uint8_t const* report, uint16_t len);
 
-void hid_app_task(void)
+void hid_app_task(void) {
-{
   // nothing to do
 }
 
@@ -63,64 +61,57 @@ void hid_app_task(void)
 // can be used to parse common/simple enough descriptor.
 // Note: if report descriptor length > CFG_TUH_ENUMERATION_BUFSIZE, it will be skipped
 // therefore report_desc = NULL, desc_len = 0
-void tuh_hid_mount_cb(uint8_t dev_addr, uint8_t instance, uint8_t const* desc_report, uint16_t desc_len)
+void tuh_hid_mount_cb(uint8_t dev_addr, uint8_t instance, uint8_t const *desc_report, uint16_t desc_len) {
-{
   printf("HID device address = %d, instance = %d is mounted\r\n", dev_addr, instance);
 
   // Interface protocol (hid_interface_protocol_enum_t)
-  const char* protocol_str[] = { "None", "Keyboard", "Mouse" };
+  const char *protocol_str[] = {"None", "Keyboard", "Mouse"};
   uint8_t const itf_protocol = tuh_hid_interface_protocol(dev_addr, instance);
 
   printf("HID Interface Protocol = %s\r\n", protocol_str[itf_protocol]);
 
   // By default host stack will use activate boot protocol on supported interface.
   // Therefore for this simple example, we only need to parse generic report descriptor (with built-in parser)
-  if ( itf_protocol == HID_ITF_PROTOCOL_NONE )
+  if (itf_protocol == HID_ITF_PROTOCOL_NONE) {
-  {
     hid_info[instance].report_count = tuh_hid_parse_report_descriptor(hid_info[instance].report_info, MAX_REPORT, desc_report, desc_len);
     printf("HID has %u reports \r\n", hid_info[instance].report_count);
   }
 
   // request to receive report
   // tuh_hid_report_received_cb() will be invoked when report is available
-  if ( !tuh_hid_receive_report(dev_addr, instance) )
+  if (!tuh_hid_receive_report(dev_addr, instance)) {
-  {
     printf("Error: cannot request to receive report\r\n");
   }
 }
 
 // Invoked when device with hid interface is un-mounted
-void tuh_hid_umount_cb(uint8_t dev_addr, uint8_t instance)
+void tuh_hid_umount_cb(uint8_t dev_addr, uint8_t instance) {
-{
   printf("HID device address = %d, instance = %d is unmounted\r\n", dev_addr, instance);
 }
 
 // Invoked when received report from device via interrupt endpoint
-void tuh_hid_report_received_cb(uint8_t dev_addr, uint8_t instance, uint8_t const* report, uint16_t len)
+void tuh_hid_report_received_cb(uint8_t dev_addr, uint8_t instance, uint8_t const *report, uint16_t len) {
-{
   uint8_t const itf_protocol = tuh_hid_interface_protocol(dev_addr, instance);
 
-  switch (itf_protocol)
+  switch (itf_protocol) {
-  {
     case HID_ITF_PROTOCOL_KEYBOARD:
       TU_LOG2("HID receive boot keyboard report\r\n");
-      process_kbd_report( (hid_keyboard_report_t const*) report );
+      process_kbd_report((hid_keyboard_report_t const *) report);
-    break;
+      break;
 
     case HID_ITF_PROTOCOL_MOUSE:
       TU_LOG2("HID receive boot mouse report\r\n");
-      process_mouse_report( (hid_mouse_report_t const*) report );
+      process_mouse_report((hid_mouse_report_t const *) report);
-    break;
+      break;
 
     default:
       // Generic report requires matching ReportID and contents with previous parsed report info
       process_generic_report(dev_addr, instance, report, len);
-    break;
+      break;
   }
 
   // continue to request to receive report
-  if ( !tuh_hid_receive_report(dev_addr, instance) )
+  if (!tuh_hid_receive_report(dev_addr, instance)) {
-  {
     printf("Error: cannot request to receive report\r\n");
   }
 }
@@ -231,29 +222,24 @@ static void process_mouse_report(hid_mouse_report_t const * report)
 //--------------------------------------------------------------------+
 // Generic Report
 //--------------------------------------------------------------------+
-static void process_generic_report(uint8_t dev_addr, uint8_t instance, uint8_t const* report, uint16_t len)
+static void process_generic_report(uint8_t dev_addr, uint8_t instance, uint8_t const *report, uint16_t len) {
-{
   (void) dev_addr;
   (void) len;
 
   uint8_t const rpt_count = hid_info[instance].report_count;
-  tuh_hid_report_info_t* rpt_info_arr = hid_info[instance].report_info;
+  tuh_hid_report_info_t *rpt_info_arr = hid_info[instance].report_info;
-  tuh_hid_report_info_t* rpt_info = NULL;
+  tuh_hid_report_info_t *rpt_info = NULL;
 
-  if ( rpt_count == 1 && rpt_info_arr[0].report_id == 0)
+  if (rpt_count == 1 && rpt_info_arr[0].report_id == 0) {
-  {
     // Simple report without report ID as 1st byte
     rpt_info = &rpt_info_arr[0];
-  }else
+  } else {
-  {
     // Composite report, 1st byte is report ID, data starts from 2nd byte
     uint8_t const rpt_id = report[0];
 
     // Find report id in the array
-    for(uint8_t i=0; i<rpt_count; i++)
+    for (uint8_t i = 0; i < rpt_count; i++) {
-    {
+      if (rpt_id == rpt_info_arr[i].report_id) {
-      if (rpt_id == rpt_info_arr[i].report_id )
-      {
         rpt_info = &rpt_info_arr[i];
         break;
       }
@@ -263,8 +249,7 @@ static void process_generic_report(uint8_t dev_addr, uint8_t instance, uint8_t c
     len--;
   }
 
-  if (!rpt_info)
+  if (!rpt_info) {
-  {
     printf("Couldn't find report info !\r\n");
     return;
   }
@@ -276,23 +261,22 @@ static void process_generic_report(uint8_t dev_addr, uint8_t instance, uint8_t c
   // - Consumer Control (Media Key) : Consumer, Consumer Control
   // - System Control (Power key)   : Desktop, System Control
   // - Generic (vendor)             : 0xFFxx, xx
-  if ( rpt_info->usage_page == HID_USAGE_PAGE_DESKTOP )
+  if (rpt_info->usage_page == HID_USAGE_PAGE_DESKTOP) {
-  {
+    switch (rpt_info->usage) {
-    switch (rpt_info->usage)
-    {
       case HID_USAGE_DESKTOP_KEYBOARD:
         TU_LOG1("HID receive keyboard report\r\n");
         // Assume keyboard follow boot report layout
-        process_kbd_report( (hid_keyboard_report_t const*) report );
+        process_kbd_report((hid_keyboard_report_t const *) report);
-      break;
+        break;
 
       case HID_USAGE_DESKTOP_MOUSE:
         TU_LOG1("HID receive mouse report\r\n");
         // Assume mouse follow boot report layout
-        process_mouse_report( (hid_mouse_report_t const*) report );
+        process_mouse_report((hid_mouse_report_t const *) report);
-      break;
+        break;
 
-      default: break;
+      default:
+        break;
     }
   }
 }

src/common/tusb_types.h

@@ -462,7 +462,7 @@ TU_VERIFY_STATIC( sizeof(tusb_desc_interface_assoc_t) == 8, "size is not correct
 typedef struct TU_ATTR_PACKED {
   uint8_t  bLength         ; ///< Size of this descriptor in bytes
   uint8_t  bDescriptorType ; ///< Descriptor Type
-  uint16_t unicode_string[];
+  uint16_t utf16le[];
 } tusb_desc_string_t;
 
 // USB Binary Device Object Store (BOS)

src/host/hub.c

@@ -46,7 +46,7 @@ typedef struct {
 
   // from hub descriptor
   uint8_t bNbrPorts;
-  uint8_t bPwrOn2PwrGood; // port power on to good, in 2ms unit
+  uint8_t bPwrOn2PwrGood_2ms; // port power on to good, in 2ms unit
   // uint16_t wHubCharacteristics;
 
   hub_port_status_response_t port_status;
@@ -279,7 +279,7 @@ static void config_set_port_power (tuh_xfer_t* xfer) {
   // only use number of ports in hub descriptor
   hub_desc_cs_t const* desc_hub = (hub_desc_cs_t const*) p_epbuf->ctrl_buf;
   p_hub->bNbrPorts = desc_hub->bNbrPorts;
-  p_hub->bPwrOn2PwrGood = desc_hub->bPwrOn2PwrGood;
+  p_hub->bPwrOn2PwrGood_2ms = desc_hub->bPwrOn2PwrGood;
 
   // May need to GET_STATUS
 
@@ -301,7 +301,6 @@ static void config_port_power_complete (tuh_xfer_t* xfer) {
       TU_MESS_FAILED();
       TU_BREAKPOINT();
     }
-    // delay bPwrOn2PwrGood * 2 ms before set configuration complete
     usbh_driver_set_config_complete(daddr, p_hub->itf_num);
   } else {
     // power next port

src/host/usbh.c

@@ -1282,13 +1282,17 @@ static void process_removing_device(uint8_t rhport, uint8_t hub_addr, uint8_t hu
           removing_hubs |= TU_BIT(dev_id - CFG_TUH_DEVICE_MAX);
         } else {
           // Invoke callback before closing driver (maybe call it later ?)
-          if (tuh_umount_cb) tuh_umount_cb(daddr);
+          if (tuh_umount_cb) {
+            tuh_umount_cb(daddr);
+          }
         }
 
         // Close class driver
         for (uint8_t drv_id = 0; drv_id < TOTAL_DRIVER_COUNT; drv_id++) {
           usbh_class_driver_t const* driver = get_driver(drv_id);
-          if (driver) driver->close(daddr);
+          if (driver) {
+            driver->close(daddr);
+          }
         }
 
         hcd_device_close(rhport, daddr);
@@ -1345,8 +1349,11 @@ enum {
   ENUM_HUB_GET_STATUS_2,
   ENUM_HUB_CLEAR_RESET_2,
   ENUM_SET_ADDR,
-
   ENUM_GET_DEVICE_DESC,
+  ENUM_GET_STRING_LANGUAGE_ID,
+  ENUM_GET_STRING_MANUFACTURER,
+  ENUM_GET_STRING_PRODUCT,
+  ENUM_GET_STRING_SERIAL,
   ENUM_GET_9BYTE_CONFIG_DESC,
   ENUM_GET_FULL_CONFIG_DESC,
   ENUM_SET_CONFIG,
@@ -1359,25 +1366,25 @@ static void enum_full_complete(void);
 
 // process device enumeration
 static void process_enumeration(tuh_xfer_t* xfer) {
-  // Retry a few times with transfers in enumeration since device can be unstable when starting up
+  // Retry a few times while enumerating since device can be unstable when starting up
-  enum {
-    ATTEMPT_COUNT_MAX = 3,
-    ATTEMPT_DELAY_MS = 100
-  };
   static uint8_t failed_count = 0;
+  if (XFER_RESULT_FAILED == xfer->result) {
+    enum {
+      ATTEMPT_COUNT_MAX = 3,
+      ATTEMPT_DELAY_MS = 100
+    };
 
-  if (XFER_RESULT_SUCCESS != xfer->result) {
     // retry if not reaching max attempt
+    failed_count++;
     bool retry = _dev0.enumerating && (failed_count < ATTEMPT_COUNT_MAX);
-    if ( retry ) {
+    if (retry) {
-      failed_count++;
       tusb_time_delay_ms_api(ATTEMPT_DELAY_MS); // delay a bit
-      TU_LOG1("Enumeration attempt %u\r\n", failed_count);
+      TU_LOG1("Enumeration attempt %u/%u\r\n", failed_count+1, ATTEMPT_COUNT_MAX);
       retry = tuh_control_xfer(xfer);
     }
 
     if (!retry) {
-      enum_full_complete();
+      enum_full_complete(); // complete as failed
     }
 
     return;
@@ -1386,6 +1393,8 @@ static void process_enumeration(tuh_xfer_t* xfer) {
 
   uint8_t const daddr = xfer->daddr;
   uintptr_t const state = xfer->user_data;
+  usbh_device_t* dev = get_device(daddr);
+  uint16_t langid = 0x0409; // default is English
 
   switch (state) {
     #if CFG_TUH_HUB
@@ -1476,7 +1485,6 @@ static void process_enumeration(tuh_xfer_t* xfer) {
       tusb_time_delay_ms_api(2);
 
       const uint8_t new_addr = (uint8_t) tu_le16toh(xfer->setup->wValue);
-
       usbh_device_t* new_dev = get_device(new_addr);
       TU_ASSERT(new_dev,);
       new_dev->addressed = 1;
@@ -1490,21 +1498,69 @@ static void process_enumeration(tuh_xfer_t* xfer) {
       // Get full device descriptor
       TU_LOG_USBH("Get Device Descriptor\r\n");
       TU_ASSERT(tuh_descriptor_get_device(new_addr, _usbh_epbuf.ctrl, sizeof(tusb_desc_device_t),
-                                          process_enumeration, ENUM_GET_9BYTE_CONFIG_DESC),);
+                                          process_enumeration, ENUM_GET_STRING_LANGUAGE_ID),);
       break;
     }
 
-    case ENUM_GET_9BYTE_CONFIG_DESC: {
+    case ENUM_GET_STRING_LANGUAGE_ID: {
-      tusb_desc_device_t const* desc_device = (tusb_desc_device_t const*) _usbh_epbuf.ctrl;
+      // save the received device descriptor
-      usbh_device_t* dev = get_device(daddr);
       TU_ASSERT(dev,);
-
+      tusb_desc_device_t const* desc_device = (tusb_desc_device_t const*) _usbh_epbuf.ctrl;
       dev->vid = desc_device->idVendor;
       dev->pid = desc_device->idProduct;
       dev->i_manufacturer = desc_device->iManufacturer;
       dev->i_product = desc_device->iProduct;
       dev->i_serial = desc_device->iSerialNumber;
 
+      tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, CFG_TUH_ENUMERATION_BUFSIZE,
+                                       process_enumeration, ENUM_GET_STRING_MANUFACTURER);
+      break;
+    }
+
+    case ENUM_GET_STRING_MANUFACTURER: {
+      TU_ASSERT(dev,);
+      const tusb_desc_string_t* desc_langid = (tusb_desc_string_t const*) _usbh_epbuf.ctrl;
+      if (desc_langid->bLength >= 4) {
+        langid = tu_le16toh(desc_langid->utf16le[0]);
+      }
+      if (dev->i_manufacturer != 0) {
+        tuh_descriptor_get_string(daddr, dev->i_manufacturer, langid, _usbh_epbuf.ctrl, CFG_TUH_ENUMERATION_BUFSIZE,
+                                  process_enumeration, ENUM_GET_STRING_PRODUCT);
+        break;
+      } else {
+        TU_ATTR_FALLTHROUGH;
+      }
+    }
+
+    case ENUM_GET_STRING_PRODUCT: {
+      TU_ASSERT(dev,);
+      if (state == ENUM_GET_STRING_PRODUCT) {
+        langid = tu_le16toh(xfer->setup->wIndex); // if not fall through, get langid from previous setup packet
+      }
+      if (dev->i_product != 0) {
+        tuh_descriptor_get_string(daddr, dev->i_product, 0x0409, _usbh_epbuf.ctrl, CFG_TUH_ENUMERATION_BUFSIZE,
+                            process_enumeration, ENUM_GET_STRING_SERIAL);
+        break;
+      } else {
+        TU_ATTR_FALLTHROUGH;
+      }
+    }
+
+    case ENUM_GET_STRING_SERIAL: {
+      TU_ASSERT(dev,);
+      if (state == ENUM_GET_STRING_SERIAL) {
+        langid = tu_le16toh(xfer->setup->wIndex); // if not fall through, get langid from previous setup packet
+      }
+      if (dev->i_serial != 0) {
+        tuh_descriptor_get_string(daddr, dev->i_serial, langid, _usbh_epbuf.ctrl, CFG_TUH_ENUMERATION_BUFSIZE,
+                            process_enumeration, ENUM_GET_9BYTE_CONFIG_DESC);
+        break;
+      } else {
+        TU_ATTR_FALLTHROUGH;
+      }
+    }
+
+    case ENUM_GET_9BYTE_CONFIG_DESC: {
       // Get 9-byte for total length
       uint8_t const config_idx = CONFIG_NUM - 1;
       TU_LOG_USBH("Get Configuration[0] Descriptor (9 bytes)\r\n");
@@ -1537,7 +1593,6 @@ static void process_enumeration(tuh_xfer_t* xfer) {
 
     case ENUM_CONFIG_DRIVER: {
       TU_LOG_USBH("Device configured\r\n");
-      usbh_device_t* dev = get_device(daddr);
       TU_ASSERT(dev,);
 
       dev->configured = 1;

src/host/usbh.h

@@ -265,6 +265,13 @@ bool tuh_descriptor_get_hid_report(uint8_t daddr, uint8_t itf_num, uint8_t desc_
 bool tuh_descriptor_get_string(uint8_t daddr, uint8_t index, uint16_t language_id, void* buffer, uint16_t len,
                                tuh_xfer_cb_t complete_cb, uintptr_t user_data);
 
+// Get language id string descriptor (control transfer)
+TU_ATTR_ALWAYS_INLINE static inline
+bool tuh_descriptor_get_string_langid(uint8_t daddr, void* buffer, uint16_t len,
+                               tuh_xfer_cb_t complete_cb, uintptr_t user_data) {
+  return tuh_descriptor_get_string(daddr, 0, 0, buffer, len, complete_cb, user_data);
+}
+
 // Get manufacturer string descriptor (control transfer)
 // true on success, false if there is on-going control transfer or incorrect parameters
 bool tuh_descriptor_get_manufacturer_string(uint8_t daddr, uint16_t language_id, void* buffer, uint16_t len,
@@ -304,6 +311,12 @@ uint8_t tuh_descriptor_get_hid_report_sync(uint8_t daddr, uint8_t itf_num, uint8
 // return transfer result
 uint8_t tuh_descriptor_get_string_sync(uint8_t daddr, uint8_t index, uint16_t language_id, void* buffer, uint16_t len);
 
+// Sync (blocking) version of tuh_descriptor_get_string_langid()
+TU_ATTR_ALWAYS_INLINE static inline
+uint8_t tuh_descriptor_get_string_langid_sync(uint8_t daddr, void* buffer, uint16_t len) {
+  return tuh_descriptor_get_string_sync(daddr, 0, 0, buffer, len);
+}
+
 // Sync (blocking) version of tuh_descriptor_get_manufacturer_string()
 // return transfer result
 uint8_t tuh_descriptor_get_manufacturer_string_sync(uint8_t daddr, uint16_t language_id, void* buffer, uint16_t len);

src/portable/raspberrypi/rp2040/hcd_rp2040.c

@@ -459,28 +459,33 @@ tusb_speed_t hcd_port_speed_get(uint8_t rhport)
 }
 
 // Close all opened endpoint belong to this device
-void hcd_device_close(uint8_t rhport, uint8_t dev_addr)
+void hcd_device_close(uint8_t rhport, uint8_t dev_addr) {
-{
   pico_trace("hcd_device_close %d\n", dev_addr);
   (void) rhport;
 
-  if (dev_addr == 0) return;
+  // reset epx if it is currently active with unplugged device
+  if (epx.configured && epx.active && epx.dev_addr == dev_addr) {
+    epx.configured = false;
+    *epx.endpoint_control = 0;
+    *epx.buffer_control = 0;
+    hw_endpoint_reset_transfer(&epx);
+  }
 
-  for (size_t i = 1; i < TU_ARRAY_SIZE(ep_pool); i++)
+  // dev0 only has ep0
-  {
+  if (dev_addr != 0) {
-    hw_endpoint_t* ep = &ep_pool[i];
+    for (size_t i = 1; i < TU_ARRAY_SIZE(ep_pool); i++) {
+      hw_endpoint_t *ep = &ep_pool[i];
+      if (ep->dev_addr == dev_addr && ep->configured) {
+        // in case it is an interrupt endpoint, disable it
+        usb_hw_clear->int_ep_ctrl = (1 << (ep->interrupt_num + 1));
+        usb_hw->int_ep_addr_ctrl[ep->interrupt_num] = 0;
 
-    if (ep->dev_addr == dev_addr && ep->configured)
+        // unconfigure the endpoint
-    {
+        ep->configured = false;
-      // in case it is an interrupt endpoint, disable it
+        *ep->endpoint_control = 0;
-      usb_hw_clear->int_ep_ctrl = (1 << (ep->interrupt_num + 1));
+        *ep->buffer_control = 0;
-      usb_hw->int_ep_addr_ctrl[ep->interrupt_num] = 0;
+        hw_endpoint_reset_transfer(ep);
-
+      }
-      // unconfigure the endpoint
-      ep->configured = false;
-      *ep->endpoint_control = 0;
-      *ep->buffer_control = 0;
-      hw_endpoint_reset_transfer(ep);
     }
   }
 }
@@ -557,7 +562,7 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t *
   }
 
   // If a normal transfer (non-interrupt) then initiate using
-  // sie ctrl registers. Otherwise interrupt ep registers should
+  // sie ctrl registers. Otherwise, interrupt ep registers should
   // already be configured
   if ( ep == &epx )
   {
@@ -597,13 +602,12 @@ bool hcd_setup_send(uint8_t rhport, uint8_t dev_addr, uint8_t const setup_packet
   (void) rhport;
 
   // Copy data into setup packet buffer
-  for ( uint8_t i = 0; i < 8; i++ )
+  for (uint8_t i = 0; i < 8; i++) {
-  {
     usbh_dpram->setup_packet[i] = setup_packet[i];
   }
 
   // Configure EP0 struct with setup info for the trans complete
-  struct hw_endpoint * ep = _hw_endpoint_allocate(0);
+  struct hw_endpoint * ep = _hw_endpoint_allocate( (uint8_t) TUSB_XFER_CONTROL);
   TU_ASSERT(ep);
 
   // EPX should be inactive

src/portable/raspberrypi/rp2040/rp2040_usb.c

@@ -110,7 +110,7 @@ void __tusb_irq_path_func(_hw_endpoint_buffer_control_update32)(struct hw_endpoi
       *ep->buffer_control = value & ~USB_BUF_CTRL_AVAIL;
       // 4.1.2.5.1 Con-current access: 12 cycles (should be good for 48*12Mhz = 576Mhz) after write to buffer control
       // Don't need delay in host mode as host is in charge
-      if ( !is_host_mode()) {
+      if (!is_host_mode()) {
         busy_wait_at_least_cycles(12);
       }
     }