Merge pull request #528 from adafruit/fix-esp32-max3421e

fix compiling hcd max3421e with arduino-esp32 and arduino-pico due to missing atomic_flag function

Author: hathach

.github/workflows/githubci.yml

@@ -38,7 +38,7 @@ jobs:
       run: bash ci/doxy_gen_and_deploy.sh
 
   # ---------------------------------------
-  # Main
+  # build
   # ---------------------------------------
   build:
     runs-on: ubuntu-latest
@@ -83,53 +83,3 @@ jobs:
 
     - name: test platforms
       run: python3 ci/build_platform.py ${{ matrix.arduino-platform }}
-
-  # ---------------------------------------
-  # Build ESP32 v2
-  # ---------------------------------------
-  build-esp32-v2:
-    if: false
-    runs-on: ubuntu-latest
-    needs: pre-commit
-    strategy:
-      fail-fast: false
-      matrix:
-        arduino-platform:
-          - 'feather_esp32s2'
-          - 'feather_esp32s3'
-        esp32-version:
-          - '2.0.17'
-
-    steps:
-      - name: Checkout code
-        uses: actions/checkout@v4
-
-      - name: Checkout adafruit/ci-arduino
-        uses: actions/checkout@v4
-        with:
-          repository: adafruit/ci-arduino
-          path: ci
-
-      - name: pre-install
-        run: bash ci/actions_install.sh
-
-      - name: Install arduino-esp32 v2 and Libraries
-        env:
-          BSP_URLS: https://raw.githubusercontent.com/espressif/arduino-esp32/gh-pages/package_esp32_index.json
-        run: |
-          arduino-cli core install esp32:esp32@${{ matrix.esp32-version }} --additional-urls $BSP_URLS
-          arduino-cli lib install ${{ env.ARDUINO_LIBS }}
-          arduino-cli core list
-          arduino-cli lib list
-
-      - name: Create custom build script
-        working-directory: ${{ github.workspace }}/ci
-        run: |
-          echo 'import build_platform' > build_esp32_v2.py
-          echo 'build_platform.test_examples_in_folder("'${{ matrix.arduino-platform }}'", build_platform.BUILD_DIR)' >> build_esp32_v2.py
-          echo 'exit(build_platform.success)' >> build_esp32_v2.py
-          cat build_esp32_v2.py
-
-      - name: test platforms
-        run: |
-          python3 ci/build_esp32_v2.py

src/class/cdc/cdc_host.c

@@ -672,7 +672,7 @@ void cdch_close(uint8_t daddr) {
 
 bool cdch_xfer_cb(uint8_t daddr, uint8_t ep_addr, xfer_result_t event, uint32_t xferred_bytes) {
   // TODO handle stall response, retry failed transfer ...
-  TU_ASSERT(event == XFER_RESULT_SUCCESS);
+  TU_VERIFY(event == XFER_RESULT_SUCCESS);
 
   uint8_t const idx = get_idx_by_ep_addr(daddr, ep_addr);
   cdch_interface_t * p_cdc = get_itf(idx);

src/class/hid/hid_host.c

@@ -444,7 +444,7 @@ bool hidh_xfer_cb(uint8_t daddr, uint8_t ep_addr, xfer_result_t result, uint32_t
   hidh_epbuf_t* epbuf = get_hid_epbuf(idx);
 
   if (dir == TUSB_DIR_IN) {
-    TU_LOG_DRV("  Get Report callback (%u, %u)\r\n", daddr, idx);
+    TU_LOG_DRV("  [idx=%u] Get Report callback\r\n", idx);
     TU_LOG3_MEM(epbuf->epin, xferred_bytes, 2);
     tuh_hid_report_received_cb(daddr, idx, epbuf->epin, (uint16_t) xferred_bytes);
   } else {
@@ -461,7 +461,9 @@ void hidh_close(uint8_t daddr) {
     hidh_interface_t* p_hid = &_hidh_itf[i];
     if (p_hid->daddr == daddr) {
       TU_LOG_DRV("  HIDh close addr = %u index = %u\r\n", daddr, i);
-      if (tuh_hid_umount_cb) tuh_hid_umount_cb(daddr, i);
+      if (tuh_hid_umount_cb) {
+        tuh_hid_umount_cb(daddr, i);
+      }
       tu_memclr(p_hid, sizeof(hidh_interface_t));
     }
   }

src/class/vendor/vendor_device.c

@@ -196,8 +196,8 @@ void vendord_reset(uint8_t rhport) {
 
 uint16_t vendord_open(uint8_t rhport, const tusb_desc_interface_t* desc_itf, uint16_t max_len) {
   TU_VERIFY(TUSB_CLASS_VENDOR_SPECIFIC == desc_itf->bInterfaceClass, 0);
+  const uint8_t* desc_end = (const uint8_t*)desc_itf + max_len;
   const uint8_t* p_desc = tu_desc_next(desc_itf);
-  const uint8_t* desc_end = (uint8_t const*)desc_itf + max_len;
 
   // Find available interface
   vendord_interface_t* p_vendor = NULL;
@@ -210,26 +210,26 @@ uint16_t vendord_open(uint8_t rhport, const tusb_desc_interface_t* desc_itf, uin
   TU_VERIFY(p_vendor, 0);
 
   p_vendor->itf_num = desc_itf->bInterfaceNumber;
-  uint8_t found_ep = 0;
-  while (found_ep < desc_itf->bNumEndpoints) {
-    // skip non-endpoint descriptors
-    while ( (TUSB_DESC_ENDPOINT != tu_desc_type(p_desc)) && (p_desc < desc_end) ) {
-      p_desc = tu_desc_next(p_desc);
-    }
-    if (p_desc >= desc_end) {
-      break;
-    }
-
-    const tusb_desc_endpoint_t* desc_ep = (const tusb_desc_endpoint_t*) p_desc;
-    TU_ASSERT(usbd_edpt_open(rhport, desc_ep));
-    found_ep++;
-
-    if (tu_edpt_dir(desc_ep->bEndpointAddress) == TUSB_DIR_IN) {
-      tu_edpt_stream_open(&p_vendor->tx.stream, desc_ep);
-      tud_vendor_n_write_flush((uint8_t)(p_vendor - _vendord_itf));
-    } else {
-      tu_edpt_stream_open(&p_vendor->rx.stream, desc_ep);
-      TU_ASSERT(tu_edpt_stream_read_xfer(rhport, &p_vendor->rx.stream) > 0, 0); // prepare for incoming data
+  while (tu_desc_is_valid(p_desc, desc_end)) {
+    const uint8_t desc_type = tu_desc_type(p_desc);
+    if (desc_type == TUSB_DESC_INTERFACE || desc_type == TUSB_DESC_INTERFACE_ASSOCIATION) {
+      break; // end of this interface
+    } else if (desc_type == TUSB_DESC_ENDPOINT) {
+      const tusb_desc_endpoint_t* desc_ep = (const tusb_desc_endpoint_t*) p_desc;
+      TU_ASSERT(usbd_edpt_open(rhport, desc_ep));
+
+      // open endpoint stream, skip if already opened
+      if (tu_edpt_dir(desc_ep->bEndpointAddress) == TUSB_DIR_IN) {
+        if (p_vendor->tx.stream.ep_addr == 0) {
+          tu_edpt_stream_open(&p_vendor->tx.stream, desc_ep);
+          tud_vendor_n_write_flush((uint8_t)(p_vendor - _vendord_itf));
+        }
+      } else {
+        if (p_vendor->rx.stream.ep_addr == 0) {
+          tu_edpt_stream_open(&p_vendor->rx.stream, desc_ep);
+          TU_ASSERT(tu_edpt_stream_read_xfer(rhport, &p_vendor->rx.stream) > 0, 0); // prepare for incoming data
+        }
+      }
     }
 
     p_desc = tu_desc_next(p_desc);

src/common/tusb_mcu.h

@@ -369,6 +369,10 @@
   #define TUP_DCD_ENDPOINT_MAX    7 // only 5 TX FIFO for endpoint IN
   #define CFG_TUSB_OS_INC_PATH_DEFAULT   freertos/
 
+  #if CFG_TUSB_MCU == OPT_MCU_ESP32S3
+    #define TUP_MCU_MULTIPLE_CORE 1
+  #endif
+
   // Disable slave if DMA is enabled
   #define CFG_TUD_DWC2_SLAVE_ENABLE_DEFAULT  !CFG_TUD_DWC2_DMA_ENABLE
   #define CFG_TUH_DWC2_SLAVE_ENABLE_DEFAULT  !CFG_TUH_DWC2_DMA_ENABLE
@@ -381,6 +385,8 @@
 
   #define CFG_TUSB_OS_INC_PATH_DEFAULT   freertos/
 
+  #define TUP_MCU_MULTIPLE_CORE   1
+
   // Disable slave if DMA is enabled
   #define CFG_TUD_DWC2_SLAVE_ENABLE_DEFAULT  !CFG_TUD_DWC2_DMA_ENABLE
   #define CFG_TUH_DWC2_SLAVE_ENABLE_DEFAULT  !CFG_TUH_DWC2_DMA_ENABLE
@@ -410,6 +416,7 @@
 #elif TU_CHECK_MCU(OPT_MCU_RP2040)
   #define TUP_DCD_EDPT_ISO_ALLOC
   #define TUP_DCD_ENDPOINT_MAX    16
+  #define TUP_MCU_MULTIPLE_CORE   1
 
   #define TU_ATTR_FAST_FUNC       __attribute__((section(".time_critical.tinyusb")))
 

src/common/tusb_types.h

@@ -586,6 +586,12 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_subtype(void const* desc) {
   return ((uint8_t const*) desc)[DESC_OFFSET_SUBTYPE];
 }
 
+TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_desc_is_valid(void const* desc, uint8_t const* desc_end) {
+  const uint8_t* desc8 = (uint8_t const*) desc;
+  return (desc8 < desc_end) && (tu_desc_next(desc) <= desc_end);
+}
+
+
 // find descriptor that match byte1 (type)
 uint8_t const * tu_desc_find(uint8_t const* desc, uint8_t const* end, uint8_t byte1);
 

src/device/usbd.c

@@ -342,15 +342,16 @@ TU_ATTR_ALWAYS_INLINE static inline usbd_class_driver_t const * get_driver(uint8
 enum { RHPORT_INVALID = 0xFFu };
 tu_static uint8_t _usbd_rhport = RHPORT_INVALID;
 
-// Event queue
-// usbd_int_set() is used as mutex in OS NONE config
+static OSAL_SPINLOCK_DEF(_usbd_spin, usbd_int_set);
+
+// Event queue: usbd_int_set() is used as mutex in OS NONE config
 OSAL_QUEUE_DEF(usbd_int_set, _usbd_qdef, CFG_TUD_TASK_QUEUE_SZ, dcd_event_t);
-tu_static osal_queue_t _usbd_q;
+static osal_queue_t _usbd_q;
 
 // Mutex for claiming endpoint
 #if OSAL_MUTEX_REQUIRED
-  tu_static osal_mutex_def_t _ubsd_mutexdef;
-  tu_static osal_mutex_t _usbd_mutex;
+  static osal_mutex_def_t _ubsd_mutexdef;
+  static osal_mutex_t _usbd_mutex;
 #else
   #define _usbd_mutex   NULL
 #endif
@@ -468,7 +469,7 @@ bool tud_rhport_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
   TU_ASSERT(rh_init);
 
   TU_LOG_USBD("USBD init on controller %u, speed = %s\r\n", rhport,
-    rh_init->speed == TUSB_SPEED_HIGH ? "High" : "Full");
+              rh_init->speed == TUSB_SPEED_HIGH ? "High" : "Full");
   TU_LOG_INT(CFG_TUD_LOG_LEVEL, sizeof(usbd_device_t));
   TU_LOG_INT(CFG_TUD_LOG_LEVEL, sizeof(dcd_event_t));
   TU_LOG_INT(CFG_TUD_LOG_LEVEL, sizeof(tu_fifo_t));
@@ -477,6 +478,8 @@ bool tud_rhport_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
   tu_varclr(&_usbd_dev);
   _usbd_queued_setup = 0;
 
+  osal_spin_init(&_usbd_spin);
+
 #if OSAL_MUTEX_REQUIRED
   // Init device mutex
   _usbd_mutex = osal_mutex_create(&_ubsd_mutexdef);
@@ -1248,17 +1251,21 @@ TU_ATTR_FAST_FUNC void dcd_event_handler(dcd_event_t const* event, bool in_isr)
 // USBD API For Class Driver
 //--------------------------------------------------------------------+
 
-void usbd_int_set(bool enabled)
-{
-  if (enabled)
-  {
+void usbd_int_set(bool enabled) {
+  if (enabled) {
     dcd_int_enable(_usbd_rhport);
-  }else
-  {
+  } else {
     dcd_int_disable(_usbd_rhport);
   }
 }
 
+void usbd_spin_lock(bool in_isr) {
+  osal_spin_lock(&_usbd_spin, in_isr);
+}
+void usbd_spin_unlock(bool in_isr) {
+  osal_spin_unlock(&_usbd_spin, in_isr);
+}
+
 // Parse consecutive endpoint descriptors (IN & OUT)
 bool usbd_open_edpt_pair(uint8_t rhport, uint8_t const* p_desc, uint8_t ep_count, uint8_t xfer_type, uint8_t* ep_out, uint8_t* ep_in)
 {

src/device/usbd_pvt.h

@@ -68,6 +68,8 @@ usbd_class_driver_t const* usbd_app_driver_get_cb(uint8_t* driver_count) TU_ATTR
 typedef bool (*usbd_control_xfer_cb_t)(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request);
 
 void usbd_int_set(bool enabled);
+void usbd_spin_lock(bool in_isr);
+void usbd_spin_unlock(bool in_isr);
 
 //--------------------------------------------------------------------+
 // USBD Endpoint API

src/host/hub.c

@@ -201,7 +201,6 @@ bool hub_port_get_status(uint8_t hub_addr, uint8_t hub_port, void* resp,
 
 bool hub_port_get_status_local(uint8_t hub_addr, uint8_t hub_port, hub_port_status_response_t* resp) {
   (void) hub_port;
-  TU_VERIFY(hub_addr > CFG_TUH_DEVICE_MAX);
   hub_interface_t* p_hub = get_hub_itf(hub_addr);
   *resp = p_hub->port_status;
   return true;

src/host/usbh.c

@@ -147,6 +147,9 @@ static osal_mutex_t _usbh_mutex;
 #define _usbh_mutex   NULL
 #endif
 
+// Spinlock for interrupt handler
+static OSAL_SPINLOCK_DEF(_usbh_spin, usbh_int_set);
+
 // Event queue: usbh_int_set() is used as mutex in OS NONE config
 OSAL_QUEUE_DEF(usbh_int_set, _usbh_qdef, CFG_TUH_TASK_QUEUE_SZ, hcd_event_t);
 static osal_queue_t _usbh_q;
@@ -424,6 +427,8 @@ bool tuh_rhport_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) {
     TU_LOG_INT_USBH(sizeof(tu_fifo_t));
     TU_LOG_INT_USBH(sizeof(tu_edpt_stream_t));
 
+    osal_spin_init(&_usbh_spin);
+
     // Event queue
     _usbh_q = osal_queue_create(&_usbh_qdef);
     TU_ASSERT(_usbh_q != NULL);
@@ -547,7 +552,7 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
         // TODO better to have an separated queue for newly attached devices
         if (_usbh_data.enumerating_daddr == TUSB_INDEX_INVALID_8) {
           // New device attached and we are ready
-          TU_LOG1("[%u:] USBH Device Attach\r\n", event.rhport);
+          TU_LOG_USBH("[%u:] USBH Device Attach\r\n", event.rhport);
           _usbh_data.enumerating_daddr = 0; // enumerate new device with address 0
           enum_new_device(&event);
         } else {
@@ -562,7 +567,7 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
         break;
 
       case HCD_EVENT_DEVICE_REMOVE:
-        TU_LOG1("[%u:%u:%u] USBH DEVICE REMOVED\r\n", event.rhport, event.connection.hub_addr, event.connection.hub_port);
+        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 &&
@@ -579,7 +584,8 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
         uint8_t const epnum = tu_edpt_number(ep_addr);
         uint8_t const ep_dir = (uint8_t) tu_edpt_dir(ep_addr);
 
-        TU_LOG_USBH("on EP %02X with %u bytes: %s\r\n", ep_addr, (unsigned int) event.xfer_complete.len, tu_str_xfer_result[event.xfer_complete.result]);
+        TU_LOG_USBH("[:%u] on EP %02X with %u bytes: %s\r\n",
+                    event.dev_addr, ep_addr, (unsigned int) event.xfer_complete.len, tu_str_xfer_result[event.xfer_complete.result]);
 
         if (event.dev_addr == 0) {
           // device 0 only has control endpoint
@@ -618,7 +624,7 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) {
               uint8_t drv_id = dev->ep2drv[epnum][ep_dir];
               usbh_class_driver_t const* driver = get_driver(drv_id);
               if (driver) {
-                TU_LOG_USBH("%s xfer callback\r\n", driver->name);
+                TU_LOG_USBH("  %s xfer callback\r\n", driver->name);
                 driver->xfer_cb(event.dev_addr, ep_addr, (xfer_result_t) event.xfer_complete.result,
                                 event.xfer_complete.len);
               } else {
@@ -894,6 +900,14 @@ void usbh_int_set(bool enabled) {
   }
 }
 
+void usbh_spin_lock(bool in_isr) {
+  osal_spin_lock(&_usbh_spin, in_isr);
+}
+
+void usbh_spin_unlock(bool in_isr) {
+  osal_spin_unlock(&_usbh_spin, in_isr);
+}
+
 void usbh_defer_func(osal_task_func_t func, void *param, bool in_isr) {
   hcd_event_t event = { 0 };
   event.event_id = USBH_EVENT_FUNC_CALL;
@@ -1463,7 +1477,7 @@ static void process_enumeration(tuh_xfer_t* xfer) {
     bool retry = (_usbh_data.enumerating_daddr != TUSB_INDEX_INVALID_8) && (failed_count < ATTEMPT_COUNT_MAX);
     if (retry) {
       tusb_time_delay_ms_api(ATTEMPT_DELAY_MS); // delay a bit
-      TU_LOG1("Enumeration attempt %u/%u\r\n", failed_count+1, ATTEMPT_COUNT_MAX);
+      TU_LOG_USBH("Enumeration attempt %u/%u\r\n", failed_count+1, ATTEMPT_COUNT_MAX);
       retry = tuh_control_xfer(xfer);
     }
 

src/host/usbh_pvt.h

@@ -71,6 +71,9 @@ void usbh_int_set(bool enabled);
 
 void usbh_defer_func(osal_task_func_t func, void *param, bool in_isr);
 
+void usbh_spin_lock(bool in_isr);
+void usbh_spin_unlock(bool in_isr);
+
 //--------------------------------------------------------------------+
 // USBH Endpoint API
 //--------------------------------------------------------------------+

src/osal/osal.h

@@ -75,6 +75,10 @@ typedef void (*osal_task_func_t)( void * );
 // OSAL Porting API
 // Should be implemented as static inline function in osal_port.h header
 /*
+   void osal_spin_init(osal_spinlock_t *ctx);
+   void osal_spin_lock(osal_spinlock_t *ctx, bool in_isr)
+   void osal_spin_unlock(osal_spinlock_t *ctx, bool in_isr);
+
    osal_semaphore_t osal_semaphore_create(osal_semaphore_def_t* semdef);
    bool osal_semaphore_delete(osal_semaphore_t semd_hdl);
    bool osal_semaphore_post(osal_semaphore_t sem_hdl, bool in_isr);