Merge pull request #3081 from hathach/usbh-enum-get-string-desc-first

usbh enum get string descriptor length first

Author: hathach

.github/workflows/build.yml

@@ -85,7 +85,7 @@ jobs:
           - 'msp430-gcc'
           - 'riscv-gcc'
           - 'rx-gcc'
-          - 'esp-idf' # build-system is ignored
+          - 'esp-idf'
     with:
       build-system: 'make'
       toolchain: ${{ matrix.toolchain }}
@@ -115,7 +115,8 @@ jobs:
   # cmake is built by circle-ci. Due to IAR limit capacity, only build oe per family
   # ---------------------------------------
   arm-iar:
-    if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false
+    if: false # disable for now since we got reach capacity limit too often
+    #if: github.event_name == 'push' && github.repository_owner == 'hathach'
     needs: set-matrix
     uses: ./.github/workflows/build_util.yml
     secrets: inherit

.github/workflows/ci_set_matrix.py

@@ -44,9 +44,10 @@
     "stm32l0 stm32l4": ["arm-gcc", "arm-clang", "arm-iar"],
     "stm32u5 stm32wb": ["arm-gcc", "arm-clang", "arm-iar"],
     "xmc4000": ["arm-gcc"],
-    "-bespressif_kaluga_1": ["esp-idf"],
-    "-bespressif_s3_devkitm": ["esp-idf"],
-    "-bespressif_p4_function_ev": ["esp-idf"],
+    "-bespressif_s2_devkitc": ["esp-idf"],
+    # S3, P4 will be built by hil test
+    # "-bespressif_s3_devkitm": ["esp-idf"],
+    # "-bespressif_p4_function_ev": ["esp-idf"],
 }
 
 

hw/bsp/espressif/boards/espressif_s2_devkitc/board.h

@@ -36,13 +36,19 @@
  extern "C" {
 #endif
 
-// Note: On the production version (v1.2) WS2812 is connected to GPIO 18,
-// however earlier revision v1.1 WS2812 is connected to GPIO 17
 #define NEOPIXEL_PIN          18
 
 #define BUTTON_PIN            0
 #define BUTTON_STATE_ACTIVE   0
 
+// SPI for USB host shield
+#define MAX3421_SPI_HOST  SPI2_HOST
+#define MAX3421_SCK_PIN  36
+#define MAX3421_MOSI_PIN 35
+#define MAX3421_MISO_PIN 37
+#define MAX3421_CS_PIN   15
+#define MAX3421_INTR_PIN 14
+
 #ifdef __cplusplus
  }
 #endif

hw/bsp/espressif/boards/espressif_s3_devkitc/board.h

@@ -36,7 +36,7 @@
  extern "C" {
 #endif
 
-#define NEOPIXEL_PIN          48
+#define NEOPIXEL_PIN          38
 
 #define BUTTON_PIN            0
 #define BUTTON_STATE_ACTIVE   0

hw/bsp/espressif/family.cmake

@@ -34,6 +34,6 @@ endif ()
 set(EXTRA_COMPONENT_DIRS "src" "${CMAKE_CURRENT_LIST_DIR}/boards" "${CMAKE_CURRENT_LIST_DIR}/components")
 
 # set SDKCONFIG for each IDF Target
-set(SDKCONFIG ${CMAKE_SOURCE_DIR}/sdkconfig.${IDF_TARGET})
+set(SDKCONFIG ${CMAKE_BINARY_DIR}/sdkconfig)
 
 include($ENV{IDF_PATH}/tools/cmake/project.cmake)

src/common/tusb_debug.h

@@ -108,15 +108,13 @@ typedef struct {
 } tu_lookup_table_t;
 
 static inline const char* tu_lookup_find(tu_lookup_table_t const* p_table, uint32_t key) {
-  tu_static char not_found[11];
-
   for(uint16_t i=0; i<p_table->count; i++) {
-    if (p_table->items[i].key == key) return p_table->items[i].data;
+    if (p_table->items[i].key == key) { return p_table->items[i].data; }
   }
 
   // not found return the key value in hex
+  static char not_found[11];
   snprintf(not_found, sizeof(not_found), "0x%08lX", (unsigned long) key);
-
   return not_found;
 }
 

src/host/usbh.c

@@ -1372,9 +1372,13 @@ enum {
   ENUM_HUB_CLEAR_RESET_2,
   ENUM_SET_ADDR,
   ENUM_GET_DEVICE_DESC,
+  ENUM_GET_STRING_LANGUAGE_ID_LEN,
   ENUM_GET_STRING_LANGUAGE_ID,
+  ENUM_GET_STRING_MANUFACTURER_LEN,
   ENUM_GET_STRING_MANUFACTURER,
+  ENUM_GET_STRING_PRODUCT_LEN,
   ENUM_GET_STRING_PRODUCT,
+  ENUM_GET_STRING_SERIAL_LEN,
   ENUM_GET_STRING_SERIAL,
   ENUM_GET_9BYTE_CONFIG_DESC,
   ENUM_GET_FULL_CONFIG_DESC,
@@ -1416,6 +1420,9 @@ 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);
+  if (daddr > 0) {
+    TU_ASSERT(dev,);
+  }
   uint16_t langid = 0x0409; // default is English
 
   switch (state) {
@@ -1474,30 +1481,6 @@ static void process_enumeration(tuh_xfer_t* xfer) {
       break;
     }
 
-#if 0
-      case ENUM_RESET_2:
-        // TODO not used by now, but may be needed for some devices !?
-        // Reset device again before Set Address
-        TU_LOG_USBH("Port reset2 \r\n");
-        if (_dev0.hub_addr == 0) {
-          // connected directly to roothub
-          hcd_port_reset( _dev0.rhport );
-          tusb_time_delay_ms_api(RESET_DELAY); // TODO may not work for no-OS on MCU that require reset_end() since
-                                        // sof of controller may not running while resetting
-          hcd_port_reset_end(_dev0.rhport);
-          // TODO: fall through to SET ADDRESS, refactor later
-        }
-#if CFG_TUH_HUB
-        else {
-          // after RESET_DELAY the hub_port_reset() already complete
-          TU_ASSERT( hub_port_reset(_dev0.hub_addr, _dev0.hub_port,
-                                    process_enumeration, ENUM_HUB_GET_STATUS_2), );
-          break;
-        }
-#endif
-        TU_ATTR_FALLTHROUGH;
-#endif
-
     case ENUM_SET_ADDR:
       enum_request_set_addr((tusb_desc_device_t*) _usbh_epbuf.ctrl);
       break;
@@ -1520,65 +1503,104 @@ 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_STRING_LANGUAGE_ID),);
+                                          process_enumeration, ENUM_GET_STRING_LANGUAGE_ID_LEN),);
       break;
     }
 
-    case ENUM_GET_STRING_LANGUAGE_ID: {
+    // For string descriptor (langid, manufacturer, product, serila): always get the first 2 bytes
+    // to determine the length first. otherwise, some device may have buffer overflow.
+    case ENUM_GET_STRING_LANGUAGE_ID_LEN: {
       // save the received device descriptor
-      TU_ASSERT(dev,);
-      tusb_desc_device_t const* desc_device = (tusb_desc_device_t const*) _usbh_epbuf.ctrl;
+      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;
       dev->bNumConfigurations = desc_device->bNumConfigurations;
 
-      tuh_enum_descriptor_device_cb(daddr, desc_device); // callback
+      tuh_enum_descriptor_device_cb(daddr, desc_device);// callback
 
-      tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, CFG_TUH_ENUMERATION_BUFSIZE,
-                                       process_enumeration, ENUM_GET_STRING_MANUFACTURER);
+      tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, 2,
+                                       process_enumeration, ENUM_GET_STRING_LANGUAGE_ID);
       break;
     }
 
-    case ENUM_GET_STRING_MANUFACTURER: {
-      TU_ASSERT(dev,);
-      const tusb_desc_string_t* desc_langid = (tusb_desc_string_t const*) _usbh_epbuf.ctrl;
+    case ENUM_GET_STRING_LANGUAGE_ID: {
+      const uint8_t str_len = xfer->buffer[0];
+      tuh_descriptor_get_string_langid(daddr, _usbh_epbuf.ctrl, str_len,
+                                       process_enumeration, ENUM_GET_STRING_MANUFACTURER_LEN);
+      break;
+    }
+
+    case ENUM_GET_STRING_MANUFACTURER_LEN: {
+      const tusb_desc_string_t* desc_langid = (const tusb_desc_string_t *) _usbh_epbuf.ctrl;
       if (desc_langid->bLength >= 4) {
-        langid = tu_le16toh(desc_langid->utf16le[0]);
+        langid = tu_le16toh(desc_langid->utf16le[0]); // previous request is langid
       }
       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);
+        tuh_descriptor_get_string(daddr, dev->i_manufacturer, langid, _usbh_epbuf.ctrl, 2,
+                                  process_enumeration, ENUM_GET_STRING_MANUFACTURER);
+        break;
+      }else {
+        TU_ATTR_FALLTHROUGH;
+      }
+    }
+
+    case ENUM_GET_STRING_MANUFACTURER: {
+      if (dev->i_manufacturer != 0)  {
+        langid = tu_le16toh(xfer->setup->wIndex); // langid from length's request
+        const uint8_t str_len = xfer->buffer[0];
+        tuh_descriptor_get_string(daddr, dev->i_manufacturer, langid, _usbh_epbuf.ctrl, str_len,
+                                  process_enumeration, ENUM_GET_STRING_PRODUCT_LEN);
         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
+    case ENUM_GET_STRING_PRODUCT_LEN:
+      if (dev->i_product != 0) {
+        if (state == ENUM_GET_STRING_PRODUCT_LEN) {
+          langid = tu_le16toh(xfer->setup->wIndex); // get langid from previous setup packet if not fall through
+        }
+        tuh_descriptor_get_string(daddr, dev->i_product, langid, _usbh_epbuf.ctrl, 2,
+                                  process_enumeration, ENUM_GET_STRING_PRODUCT);
+        break;
+      } else {
+        TU_ATTR_FALLTHROUGH;
       }
+
+    case ENUM_GET_STRING_PRODUCT: {
       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);
+        langid = tu_le16toh(xfer->setup->wIndex); // langid from length's request
+        const uint8_t str_len = xfer->buffer[0];
+        tuh_descriptor_get_string(daddr, dev->i_product, langid, _usbh_epbuf.ctrl, str_len,
+                            process_enumeration, ENUM_GET_STRING_SERIAL_LEN);
         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
+    case ENUM_GET_STRING_SERIAL_LEN:
+      if (dev->i_serial != 0) {
+        if (state == ENUM_GET_STRING_SERIAL_LEN) {
+          langid = tu_le16toh(xfer->setup->wIndex); // get langid from previous setup packet if not fall through
+        }
+        tuh_descriptor_get_string(daddr, dev->i_serial, langid, _usbh_epbuf.ctrl, 2,
+                                  process_enumeration, ENUM_GET_STRING_SERIAL);
+        break;
+      } else {
+        TU_ATTR_FALLTHROUGH;
       }
+
+    case ENUM_GET_STRING_SERIAL: {
       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);
+        langid = tu_le16toh(xfer->setup->wIndex); // langid from length's request
+        const uint8_t str_len = xfer->buffer[0];
+        tuh_descriptor_get_string(daddr, dev->i_serial, langid, _usbh_epbuf.ctrl, str_len,
+                                  process_enumeration, ENUM_GET_9BYTE_CONFIG_DESC);
         break;
       } else {
         TU_ATTR_FALLTHROUGH;
@@ -1627,8 +1649,6 @@ static void process_enumeration(tuh_xfer_t* xfer) {
 
     case ENUM_CONFIG_DRIVER: {
       TU_LOG_USBH("Device configured\r\n");
-      TU_ASSERT(dev,);
-
       dev->configured = 1;
 
       // Parse configuration & set up drivers