فهرست منبع

Merge branch 'bugfix/usb_hcd_robustness' into 'master'

USB Host: Fix how disconnections an EP halts are handled

Closes IDFGH-5797 and IDFGH-6108

See merge request espressif/esp-idf!15640
Darian 4 سال پیش
والد
کامیت
ea6a0dde5a

+ 25 - 56
components/hal/include/hal/usbh_hal.h

@@ -1,16 +1,8 @@
-// Copyright 2020 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #pragma once
 
@@ -50,17 +42,6 @@ typedef struct {
     uint32_t ptx_fifo_lines;                /**< Size of the Periodic FIFO in terms the number of FIFO lines */
 } usbh_hal_fifo_config_t;
 
-// --------------------- HAL States ------------------------
-
-/**
- * @brief Channel states
- */
-typedef enum {
-    USBH_HAL_CHAN_STATE_HALTED = 0,         /**< The channel is halted. No transfer descriptor list is being executed */
-    USBH_HAL_CHAN_STATE_ACTIVE,             /**< The channel is active. A transfer descriptor list is being executed */
-    USBH_HAL_CHAN_STATE_ERROR,              /**< The channel is in the error state */
-} usbh_hal_chan_state_t;
-
 // --------------------- HAL Events ------------------------
 
 /**
@@ -153,8 +134,7 @@ typedef struct {
         struct {
             uint32_t active: 1;             /**< Debugging bit to indicate whether channel is enabled */
             uint32_t halt_requested: 1;     /**< A halt has been requested */
-            uint32_t error_pending: 1;      /**< The channel is waiting for the error to be handled */
-            uint32_t reserved: 1;
+            uint32_t reserved: 2;
             uint32_t chan_idx: 4;           /**< The index number of the channel */
             uint32_t reserved24: 24;
         };
@@ -556,23 +536,6 @@ static inline void *usbh_hal_chan_get_context(usbh_hal_chan_t *chan_obj)
     return chan_obj->chan_ctx;
 }
 
-/**
- * @brief Get the current state of a channel
- *
- * @param chan_obj Channel object
- * @return usbh_hal_chan_state_t State of the channel
- */
-static inline usbh_hal_chan_state_t usbh_hal_chan_get_state(usbh_hal_chan_t *chan_obj)
-{
-    if (chan_obj->flags.error_pending) {
-        return USBH_HAL_CHAN_STATE_ERROR;
-    } else if (chan_obj->flags.active) {
-        return USBH_HAL_CHAN_STATE_ACTIVE;
-    } else {
-        return USBH_HAL_CHAN_STATE_HALTED;
-    }
-}
-
 /**
  * @brief Set the endpoint information for a particular channel
  *
@@ -602,7 +565,7 @@ void usbh_hal_chan_set_ep_char(usbh_hal_context_t *hal, usbh_hal_chan_t *chan_ob
 static inline void usbh_hal_chan_set_dir(usbh_hal_chan_t *chan_obj, bool is_in)
 {
     //Cannot change direction whilst channel is still active or in error
-    HAL_ASSERT(!chan_obj->flags.active && !chan_obj->flags.error_pending);
+    HAL_ASSERT(!chan_obj->flags.active);
     usbh_ll_chan_set_dir(chan_obj->regs, is_in);
 }
 
@@ -621,7 +584,7 @@ static inline void usbh_hal_chan_set_dir(usbh_hal_chan_t *chan_obj, bool is_in)
 static inline void usbh_hal_chan_set_pid(usbh_hal_chan_t *chan_obj, int pid)
 {
     //Cannot change pid whilst channel is still active or in error
-    HAL_ASSERT(!chan_obj->flags.active && !chan_obj->flags.error_pending);
+    HAL_ASSERT(!chan_obj->flags.active);
     //Update channel object and set the register
     usbh_ll_chan_set_pid(chan_obj->regs, pid);
 }
@@ -638,7 +601,7 @@ static inline void usbh_hal_chan_set_pid(usbh_hal_chan_t *chan_obj, int pid)
  */
 static inline uint32_t usbh_hal_chan_get_pid(usbh_hal_chan_t *chan_obj)
 {
-    HAL_ASSERT(!chan_obj->flags.active && !chan_obj->flags.error_pending);
+    HAL_ASSERT(!chan_obj->flags.active);
     return usbh_ll_chan_get_pid(chan_obj->regs);
 }
 
@@ -688,27 +651,33 @@ static inline int usbh_hal_chan_get_qtd_idx(usbh_hal_chan_t *chan_obj)
 bool usbh_hal_chan_request_halt(usbh_hal_chan_t *chan_obj);
 
 /**
- * @brief Get a channel's error
+ * @brief Indicate that a channel is halted after a port error
+ *
+ * When a port error occurs (e.g., discconect, overcurrent):
+ * - Any previously active channels will remain active (i.e., they will not receive a channel interrupt)
+ * - Attempting to disable them using usbh_hal_chan_request_halt() will NOT generate an interrupt for ISOC channels
+ *   (probalby something to do with the periodic scheduling)
+ *
+ * However, the channel's enable bit can be left as 1 since after a port error, a soft reset will be done anyways.
+ * This function simply updates the channels internal state variable to indicate it is halted (thus allowing it to be
+ * freed).
  *
  * @param chan_obj Channel object
- * @return usbh_hal_chan_error_t The type of error the channel has encountered
  */
-static inline usbh_hal_chan_error_t usbh_hal_chan_get_error(usbh_hal_chan_t *chan_obj)
+static inline void usbh_hal_chan_mark_halted(usbh_hal_chan_t *chan_obj)
 {
-    HAL_ASSERT(chan_obj->flags.error_pending);
-    return chan_obj->error;
+    chan_obj->flags.active = 0;
 }
 
 /**
- * @brief Clear a channel of it's error
+ * @brief Get a channel's error
  *
  * @param chan_obj Channel object
+ * @return usbh_hal_chan_error_t The type of error the channel has encountered
  */
-static inline void usbh_hal_chan_clear_error(usbh_hal_chan_t *chan_obj)
+static inline usbh_hal_chan_error_t usbh_hal_chan_get_error(usbh_hal_chan_t *chan_obj)
 {
-    //Can only clear error when an error has occurred
-    HAL_ASSERT(chan_obj->flags.error_pending);
-    chan_obj->flags.error_pending = 0;
+    return chan_obj->error;
 }
 
 // -------------------------------------------- Transfer Descriptor List -----------------------------------------------

+ 19 - 49
components/hal/include/hal/usbh_ll.h

@@ -1,16 +1,8 @@
-// Copyright 2020 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #pragma once
 
@@ -23,6 +15,8 @@ extern "C" {
 #include "soc/usbh_struct.h"
 #include "soc/usb_wrap_struct.h"
 #include "hal/usb_types_private.h"
+#include "hal/misc.h"
+
 
 /* -----------------------------------------------------------------------------
 ------------------------------- Global Registers -------------------------------
@@ -328,7 +322,7 @@ static inline void usb_ll_dis_intrs(usbh_dev_t *hw, uint32_t intr_mask)
 static inline void usb_ll_set_rx_fifo_size(usbh_dev_t *hw, uint32_t num_lines)
 {
     //Set size in words
-    hw->grxfsiz_reg.rxfdep = num_lines;
+    HAL_FORCE_MODIFY_U32_REG_FIELD(hw->grxfsiz_reg, rxfdep, num_lines);
 }
 
 // -------------------------- GNPTXFSIZ Register -------------------------------
@@ -337,8 +331,8 @@ static inline void usb_ll_set_nptx_fifo_size(usbh_dev_t *hw, uint32_t addr, uint
 {
     usb_gnptxfsiz_reg_t gnptxfsiz;
     gnptxfsiz.val = hw->gnptxfsiz_reg.val;
-    gnptxfsiz.nptxfstaddr = addr;
-    gnptxfsiz.nptxfdep = num_lines;
+    HAL_FORCE_MODIFY_U32_REG_FIELD(gnptxfsiz, nptxfstaddr, addr);
+    HAL_FORCE_MODIFY_U32_REG_FIELD(gnptxfsiz, nptxfdep, num_lines);
     hw->gnptxfsiz_reg.val = gnptxfsiz.val;
 }
 
@@ -373,8 +367,8 @@ static inline void usbh_ll_set_ptx_fifo_size(usbh_dev_t *hw, uint32_t addr, uint
 {
     usb_hptxfsiz_reg_t hptxfsiz;
     hptxfsiz.val = hw->hptxfsiz_reg.val;
-    hptxfsiz.ptxfstaddr = addr;
-    hptxfsiz.ptxfsize = num_lines;
+    HAL_FORCE_MODIFY_U32_REG_FIELD(hptxfsiz, ptxfstaddr, addr);
+    HAL_FORCE_MODIFY_U32_REG_FIELD(hptxfsiz, ptxfsize, num_lines);
     hw->hptxfsiz_reg.val = hptxfsiz.val;
 }
 
@@ -473,7 +467,7 @@ static inline void usbh_ll_hfir_set_defaults(usbh_dev_t *hw, usb_priv_speed_t sp
 
 static inline uint32_t usbh_ll_get_frm_time_rem(usbh_dev_t *hw)
 {
-    return hw->hfnum_reg.frrem;
+    return HAL_FORCE_READ_U32_REG_FIELD(hw->hfnum_reg, frrem);
 }
 
 static inline uint32_t usbh_ll_get_frm_num(usbh_dev_t *hw)
@@ -485,7 +479,7 @@ static inline uint32_t usbh_ll_get_frm_num(usbh_dev_t *hw)
 
 static inline uint32_t usbh_ll_get_p_tx_queue_top(usbh_dev_t *hw)
 {
-    return hw->hptxsts_reg.ptxqtop;
+    return HAL_FORCE_READ_U32_REG_FIELD(hw->hptxsts_reg, ptxqtop);
 }
 
 static inline uint32_t usbh_ll_get_p_tx_queue_space_avail(usbh_dev_t *hw)
@@ -495,20 +489,21 @@ static inline uint32_t usbh_ll_get_p_tx_queue_space_avail(usbh_dev_t *hw)
 
 static inline uint32_t usbh_ll_get_p_tx_fifo_space_avail(usbh_dev_t *hw)
 {
-    return hw->hptxsts_reg.ptxfspcavail;
+    return HAL_FORCE_READ_U32_REG_FIELD(hw->hptxsts_reg, ptxfspcavail);
 }
 
 // ----------------------------- HAINT Register --------------------------------
 
 static inline uint32_t usbh_ll_get_chan_intrs_msk(usbh_dev_t *hw)
 {
-    return hw->haint_reg.haint;
+    return HAL_FORCE_READ_U32_REG_FIELD(hw->haint_reg, haint);
 }
 
 // --------------------------- HAINTMSK Register -------------------------------
 
 static inline void usbh_ll_haintmsk_en_chan_intr(usbh_dev_t *hw, uint32_t mask)
 {
+
     hw->haintmsk_reg.val |= mask;
 }
 
@@ -817,31 +812,6 @@ static inline void usbh_ll_chan_set_dma_addr_non_iso(volatile usb_host_chan_regs
     chan->hcdma_reg.non_iso.ctd = qtd_idx;
 }
 
-static inline void usbh_ll_chan_set_dma_addr_iso(volatile usb_host_chan_regs_t *chan,
-                                                void *dmaaddr,
-                                                uint32_t ntd)
-{
-    int n;
-    if (ntd == 2) {
-        n = 4;
-    } else if (ntd == 4) {
-        n = 5;
-    } else if (ntd == 8) {
-        n = 6;
-    } else if (ntd == 16) {
-        n = 7;
-    } else if (ntd == 32) {
-        n = 8;
-    } else {    //ntd == 64
-        n = 9;
-    }
-    //Set HCTSIZi
-    chan->hctsiz_reg.ntd = ntd -1;
-    chan->hctsiz_reg.sched_info = 0xFF;     //Always set to 0xFF for FS
-    //Set HCDMAi
-    chan->hcdma_reg.iso.dmaaddr_ctd = (((uint32_t)dmaaddr) & 0x1FF) << (n-3);   //ctd is set to 0
-}
-
 static inline int usbh_ll_chan_get_ctd(usb_host_chan_regs_t *chan)
 {
     return chan->hcdma_reg.non_iso.ctd;
@@ -850,12 +820,12 @@ static inline int usbh_ll_chan_get_ctd(usb_host_chan_regs_t *chan)
 static inline void usbh_ll_chan_hctsiz_init(volatile usb_host_chan_regs_t *chan)
 {
     chan->hctsiz_reg.dopng = 0;         //Don't do ping
-    chan->hctsiz_reg.sched_info = 0xFF; //Schedinfo is always 0xFF for fullspeed. Not used in Bulk/Ctrl channels
+    HAL_FORCE_MODIFY_U32_REG_FIELD(chan->hctsiz_reg, sched_info, 0xFF); //Schedinfo is always 0xFF for fullspeed. Not used in Bulk/Ctrl channels
 }
 
 static inline void usbh_ll_chan_set_qtd_list_len(volatile usb_host_chan_regs_t *chan, int qtd_list_len)
 {
-    chan->hctsiz_reg.ntd = qtd_list_len - 1;    //Set the length of the descriptor list
+    HAL_FORCE_MODIFY_U32_REG_FIELD(chan->hctsiz_reg, ntd, qtd_list_len - 1);    //Set the length of the descriptor list
 }
 
 // ---------------------------- HCDMABi Register -------------------------------

+ 12 - 18
components/hal/usbh_hal.c

@@ -1,16 +1,8 @@
-// Copyright 2020 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #include <stddef.h>
 #include <stdint.h>
@@ -238,7 +230,7 @@ void usbh_hal_chan_free(usbh_hal_context_t *hal, usbh_hal_chan_t *chan_obj)
         }
     }
     //Can only free a channel when in the disabled state and descriptor list released
-    HAL_ASSERT(!chan_obj->flags.active && !chan_obj->flags.error_pending);
+    HAL_ASSERT(!chan_obj->flags.active);
     //Disable channel's interrupt
     usbh_ll_haintmsk_dis_chan_intr(hal->dev, 1 << chan_obj->flags.chan_idx);
     //Deallocate channel
@@ -252,7 +244,7 @@ void usbh_hal_chan_free(usbh_hal_context_t *hal, usbh_hal_chan_t *chan_obj)
 void usbh_hal_chan_set_ep_char(usbh_hal_context_t *hal, usbh_hal_chan_t *chan_obj, usbh_hal_ep_char_t *ep_char)
 {
     //Cannot change ep_char whilst channel is still active or in error
-    HAL_ASSERT(!chan_obj->flags.active && !chan_obj->flags.error_pending);
+    HAL_ASSERT(!chan_obj->flags.active);
     //Set the endpoint characteristics of the pipe
     usbh_ll_chan_hcchar_init(chan_obj->regs,
                              ep_char->dev_addr,
@@ -280,7 +272,7 @@ void usbh_hal_chan_set_ep_char(usbh_hal_context_t *hal, usbh_hal_chan_t *chan_ob
 void usbh_hal_chan_activate(usbh_hal_chan_t *chan_obj, void *xfer_desc_list, int desc_list_len, int start_idx)
 {
     //Cannot activate a channel that has already been enabled or is pending error handling
-    HAL_ASSERT(!chan_obj->flags.active && !chan_obj->flags.error_pending);
+    HAL_ASSERT(!chan_obj->flags.active);
     //Set start address of the QTD list and starting QTD index
     usbh_ll_chan_set_dma_addr_non_iso(chan_obj->regs, xfer_desc_list, start_idx);
     usbh_ll_chan_set_qtd_list_len(chan_obj->regs, desc_list_len);
@@ -291,12 +283,14 @@ void usbh_hal_chan_activate(usbh_hal_chan_t *chan_obj, void *xfer_desc_list, int
 bool usbh_hal_chan_request_halt(usbh_hal_chan_t *chan_obj)
 {
     //Cannot request halt on a channel that is pending error handling
-    HAL_ASSERT(chan_obj->flags.active && !chan_obj->flags.error_pending);
     if (usbh_ll_chan_is_active(chan_obj->regs)) {
+        //If the register indicates that the channel is still active, the active flag must have been previously set
+        HAL_ASSERT(chan_obj->flags.active);
         usbh_ll_chan_halt(chan_obj->regs);
         chan_obj->flags.halt_requested = 1;
         return false;
     } else {
+        //We just clear the active flag here as it could still be set (if we have a pending channel interrupt)
         chan_obj->flags.active = 0;
         return true;
     }
@@ -366,6 +360,7 @@ usbh_hal_chan_event_t usbh_hal_chan_decode_intr(usbh_hal_chan_t *chan_obj)
 {
     uint32_t chan_intrs = usbh_ll_chan_intr_read_and_clear(chan_obj->regs);
     usbh_hal_chan_event_t chan_event;
+    //Note: We don't assert on (chan_obj->flags.active) here as it could have been already cleared by usbh_hal_chan_request_halt()
 
     if (chan_intrs & CHAN_INTRS_ERROR_MSK) {    //Note: Errors are uncommon, so we check against the entire interrupt mask to reduce frequency of entering this call path
         HAL_ASSERT(chan_intrs & USBH_LL_INTR_CHAN_CHHLTD);  //An error should have halted the channel
@@ -383,7 +378,6 @@ usbh_hal_chan_event_t usbh_hal_chan_decode_intr(usbh_hal_chan_t *chan_obj)
         //Update flags
         chan_obj->error = error;
         chan_obj->flags.active = 0;
-        chan_obj->flags.error_pending = 1;
         //Save the error to be handled later
         chan_event = USBH_HAL_CHAN_EVENT_ERROR;
     } else if (chan_intrs & USBH_LL_INTR_CHAN_CHHLTD) {

+ 69 - 55
components/usb/hcd.c

@@ -24,8 +24,6 @@
 #include "usb_private.h"
 #include "usb/usb_types_ch9.h"
 
-#include "esp_rom_sys.h"
-
 // ----------------------------------------------------- Macros --------------------------------------------------------
 
 // --------------------- Constants -------------------------
@@ -214,7 +212,8 @@ typedef struct {
     union {
         struct {
             uint32_t executing: 1;          //The buffer is currently executing
-            uint32_t reserved7: 7;
+            uint32_t was_canceled: 1;      //Buffer was done due to a cancellation (i.e., a halt request)
+            uint32_t reserved6: 6;
             uint32_t stop_idx: 8;           //The descriptor index when the channel was halted
             hcd_pipe_event_t pipe_event: 8; //The pipe event when the buffer was done
             uint32_t reserved8: 8;
@@ -257,7 +256,7 @@ struct pipe_obj {
     //Pipe status/state/events related
     hcd_pipe_state_t state;
     hcd_pipe_event_t last_event;
-    TaskHandle_t task_waiting_pipe_notif;   //Task handle used for internal pipe events
+    volatile TaskHandle_t task_waiting_pipe_notif;  //Task handle used for internal pipe events. Set by waiter, cleared by notifier
     union {
         struct {
             uint32_t waiting_halt: 1;
@@ -290,7 +289,7 @@ struct port_obj {
     hcd_port_state_t state;
     usb_speed_t speed;
     hcd_port_event_t last_event;
-    TaskHandle_t task_waiting_port_notif;           //Task handle used for internal port events
+    volatile TaskHandle_t task_waiting_port_notif;  //Task handle used for internal port events. Set by waiter, cleared by notifier
     union {
         struct {
             uint32_t event_pending: 1;              //The port has an event that needs to be handled
@@ -423,13 +422,15 @@ static void _buffer_exec_cont(pipe_t *pipe);
  *
  * @param pipe Pipe object
  * @param stop_idx Descriptor index when the buffer stopped execution
- * @param pipe_event Pipe event that caused the buffer to be complete
+ * @param pipe_event Pipe event that caused the buffer to be complete. Use HCD_PIPE_EVENT_NONE for halt request of disconnections
+ * @param canceled Whether the buffer was done due to a canceled (i.e., halt request). Must set pipe_event to HCD_PIPE_EVENT_NONE
  */
-static inline void _buffer_done(pipe_t *pipe, int stop_idx, hcd_pipe_event_t pipe_event)
+static inline void _buffer_done(pipe_t *pipe, int stop_idx, hcd_pipe_event_t pipe_event, bool canceled)
 {
     //Store the stop_idx and pipe_event for later parsing
     dma_buffer_block_t *buffer_done = pipe->buffers[pipe->multi_buffer_control.rd_idx];
     buffer_done->status_flags.executing = 0;
+    buffer_done->status_flags.was_canceled = canceled;
     buffer_done->status_flags.stop_idx = stop_idx;
     buffer_done->status_flags.pipe_event = pipe_event;
     pipe->multi_buffer_control.rd_idx++;
@@ -474,11 +475,11 @@ static void _buffer_parse(pipe_t *pipe);
  * @note This should only be called on pipes do not have any currently executing buffers.
  *
  * @param pipe Pipe object
- * @param cancelled Whether this flush is due to cancellation
+ * @param canceled Whether this flush is due to cancellation
  * @return true One or more buffers were flushed
  * @return false There were no buffers that needed to be flushed
  */
-static bool _buffer_flush_all(pipe_t *pipe, bool cancelled);
+static bool _buffer_flush_all(pipe_t *pipe, bool canceled);
 
 // ------------------------ Pipe ---------------------------
 
@@ -689,22 +690,28 @@ static void _internal_port_event_wait(port_t *port)
     //There must NOT be another thread/task already waiting for an internal event
     assert(port->task_waiting_port_notif == NULL);
     port->task_waiting_port_notif = xTaskGetCurrentTaskHandle();
-    HCD_EXIT_CRITICAL();
-    //Wait to be notified from ISR
-    ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
-    HCD_ENTER_CRITICAL();
-    port->task_waiting_port_notif = NULL;
+    /* We need to loop as task notifications can come from anywhere. If we this
+    was a port event notification, task_waiting_port_notif will have been cleared
+    by the notifier. */
+    while (port->task_waiting_port_notif != NULL) {
+        HCD_EXIT_CRITICAL();
+        //Wait to be notified from ISR
+        ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+        HCD_ENTER_CRITICAL();
+    }
 }
 
 static bool _internal_port_event_notify_from_isr(port_t *port)
 {
     //There must be a thread/task waiting for an internal event
     assert(port->task_waiting_port_notif != NULL);
-    BaseType_t xTaskWoken = pdFALSE;
+    TaskHandle_t task_to_unblock = port->task_waiting_port_notif;
+    //Clear task_waiting_port_notif to indicate to the waiter that the unblock was indeed an port event notification
+    port->task_waiting_port_notif = NULL;
     //Unblock the thread/task waiting for the notification
-    HCD_EXIT_CRITICAL_ISR();
-    vTaskNotifyGiveFromISR(port->task_waiting_port_notif, &xTaskWoken);
-    HCD_ENTER_CRITICAL_ISR();
+    BaseType_t xTaskWoken = pdFALSE;
+    //Note: We don't exit the critical section to be atomic. vTaskNotifyGiveFromISR() doesn't block anyways
+    vTaskNotifyGiveFromISR(task_to_unblock, &xTaskWoken);
     return (xTaskWoken == pdTRUE);
 }
 
@@ -713,28 +720,34 @@ static void _internal_pipe_event_wait(pipe_t *pipe)
     //There must NOT be another thread/task already waiting for an internal event
     assert(pipe->task_waiting_pipe_notif == NULL);
     pipe->task_waiting_pipe_notif = xTaskGetCurrentTaskHandle();
-    HCD_EXIT_CRITICAL();
-    //Wait to be notified from ISR
-    ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
-    HCD_ENTER_CRITICAL();
-    pipe->task_waiting_pipe_notif = NULL;
+    /* We need to loop as task notifications can come from anywhere. If we this
+    was a pipe event notification, task_waiting_pipe_notif will have been cleared
+    by the notifier. */
+    while (pipe->task_waiting_pipe_notif != NULL) {
+        //Wait to be unblocked by notified
+        HCD_EXIT_CRITICAL();
+        ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+        HCD_ENTER_CRITICAL();
+    }
 }
 
 static bool _internal_pipe_event_notify(pipe_t *pipe, bool from_isr)
 {
     //There must be a thread/task waiting for an internal event
     assert(pipe->task_waiting_pipe_notif != NULL);
+    TaskHandle_t task_to_unblock = pipe->task_waiting_pipe_notif;
+    //Clear task_waiting_pipe_notif to indicate to the waiter that the unblock was indeed an pipe event notification
+    pipe->task_waiting_pipe_notif = NULL;
     bool ret;
     if (from_isr) {
         BaseType_t xTaskWoken = pdFALSE;
-        HCD_EXIT_CRITICAL_ISR();
+        //Note: We don't exit the critical section to be atomic. vTaskNotifyGiveFromISR() doesn't block anyways
         //Unblock the thread/task waiting for the pipe notification
-        vTaskNotifyGiveFromISR(pipe->task_waiting_pipe_notif, &xTaskWoken);
-        HCD_ENTER_CRITICAL_ISR();
+        vTaskNotifyGiveFromISR(task_to_unblock, &xTaskWoken);
         ret = (xTaskWoken == pdTRUE);
     } else {
         HCD_EXIT_CRITICAL();
-        xTaskNotifyGive(pipe->task_waiting_pipe_notif);
+        xTaskNotifyGive(task_to_unblock);
         HCD_ENTER_CRITICAL();
         ret = false;
     }
@@ -836,7 +849,7 @@ static hcd_pipe_event_t _intr_hdlr_chan(pipe_t *pipe, usbh_hal_chan_t *chan_obj,
             event = pipe->last_event;
             //Mark the buffer as done
             int stop_idx = usbh_hal_chan_get_qtd_idx(chan_obj);
-            _buffer_done(pipe, stop_idx, pipe->last_event);
+            _buffer_done(pipe, stop_idx, pipe->last_event, false);
             //First check if there is another buffer we can execute. But we only want to execute if there's still a valid device
             if (_buffer_can_exec(pipe) && pipe->port->flags.conn_dev_ena) {
                 //If the next buffer is filled and ready to execute, execute it
@@ -854,13 +867,12 @@ static hcd_pipe_event_t _intr_hdlr_chan(pipe_t *pipe, usbh_hal_chan_t *chan_obj,
         case USBH_HAL_CHAN_EVENT_ERROR: {
             //Get and store the pipe error event
             usbh_hal_chan_error_t chan_error = usbh_hal_chan_get_error(chan_obj);
-            usbh_hal_chan_clear_error(chan_obj);
             pipe->last_event = pipe_decode_error_event(chan_error);
             event = pipe->last_event;
             pipe->state = HCD_PIPE_STATE_HALTED;
             //Mark the buffer as done with an error
             int stop_idx = usbh_hal_chan_get_qtd_idx(chan_obj);
-            _buffer_done(pipe, stop_idx, pipe->last_event);
+            _buffer_done(pipe, stop_idx, pipe->last_event, false);
             //Parse the buffer
             _buffer_parse(pipe);
             break;
@@ -873,7 +885,7 @@ static hcd_pipe_event_t _intr_hdlr_chan(pipe_t *pipe, usbh_hal_chan_t *chan_obj,
             //Halt request event is triggered when packet is successful completed. But just treat all halted transfers as errors
             pipe->state = HCD_PIPE_STATE_HALTED;
             int stop_idx = usbh_hal_chan_get_qtd_idx(chan_obj);
-            _buffer_done(pipe, stop_idx, HCD_PIPE_EVENT_NONE);
+            _buffer_done(pipe, stop_idx, HCD_PIPE_EVENT_NONE, true);
             //Parse the buffer
             _buffer_parse(pipe);
             //Notify the task waiting for the pipe halt
@@ -1717,17 +1729,23 @@ static void pipe_set_ep_char(const hcd_pipe_config_t *pipe_config, usb_transfer_
 static esp_err_t _pipe_cmd_halt(pipe_t *pipe)
 {
     esp_err_t ret;
-    //Pipe must be in the active state in order to be halted
-    if (pipe->state != HCD_PIPE_STATE_ACTIVE) {
-        ret = ESP_ERR_INVALID_STATE;
+
+    //If pipe is already halted, just return.
+    if (pipe->state == HCD_PIPE_STATE_HALTED) {
+        ret = ESP_OK;
         goto exit;
     }
-    //Request that the channel halts
-    if (!usbh_hal_chan_request_halt(pipe->chan_obj)) {
-        //We need to wait for channel to be halted. State will be updated in the ISR
-        pipe->cs_flags.waiting_halt = 1;
-        _internal_pipe_event_wait(pipe);
+    //If the pipe's port is invalid, we just mark the pipe as halted without needing to halt the underlying channel
+    if (pipe->port->flags.conn_dev_ena //Skip halting the underlying channel if the port is invalid
+        && !usbh_hal_chan_request_halt(pipe->chan_obj)) {   //Check if the channel is already halted
+            //Channel is not halted, we need to request and wait for a haltWe need to wait for channel to be halted.
+            pipe->cs_flags.waiting_halt = 1;
+            _internal_pipe_event_wait(pipe);
+            //State should have been updated in the ISR
+            assert(pipe->state == HCD_PIPE_STATE_HALTED);
     } else {
+        //We are already halted, just need to update the state
+        usbh_hal_chan_mark_halted(pipe->chan_obj);
         pipe->state = HCD_PIPE_STATE_HALTED;
     }
     ret = ESP_OK;
@@ -1743,11 +1761,11 @@ static esp_err_t _pipe_cmd_flush(pipe_t *pipe)
         ret = ESP_ERR_INVALID_STATE;
         goto exit;
     }
-    //Cannot have a currently executing buffer
-    assert(!pipe->multi_buffer_control.buffer_is_executing);
+    //If the port is still valid, we are canceling transfers. Otherwise, we are flushing due to a port error
+    bool canceled = pipe->port->flags.conn_dev_ena;
     bool call_pipe_cb;
     //Flush any filled buffers
-    call_pipe_cb = _buffer_flush_all(pipe, true);
+    call_pipe_cb = _buffer_flush_all(pipe, canceled);
     //Move all URBs from the pending tailq to the done tailq
     if (pipe->num_urb_pending > 0) {
         //Process all remaining pending URBs
@@ -1755,14 +1773,14 @@ static esp_err_t _pipe_cmd_flush(pipe_t *pipe)
         TAILQ_FOREACH(urb, &pipe->pending_urb_tailq, tailq_entry) {
             //Update the URB's current state
             urb->hcd_var = URB_HCD_STATE_DONE;
-            //We are canceling the URB. Update its actual_num_bytes and status
+            //URBs were never executed, Update the actual_num_bytes and status
             urb->transfer.actual_num_bytes = 0;
-            urb->transfer.status = USB_TRANSFER_STATUS_CANCELED;
+            urb->transfer.status = (canceled) ? USB_TRANSFER_STATUS_CANCELED : USB_TRANSFER_STATUS_NO_DEVICE;
             if (pipe->ep_char.type == USB_PRIV_XFER_TYPE_ISOCHRONOUS) {
                 //Update the URB's isoc packet descriptors as well
                 for (int pkt_idx = 0; pkt_idx < urb->transfer.num_isoc_packets; pkt_idx++) {
                     urb->transfer.isoc_packet_desc[pkt_idx].actual_num_bytes = 0;
-                    urb->transfer.isoc_packet_desc[pkt_idx].status = USB_TRANSFER_STATUS_CANCELED;
+                    urb->transfer.isoc_packet_desc[pkt_idx].status = (canceled) ? USB_TRANSFER_STATUS_CANCELED : USB_TRANSFER_STATUS_NO_DEVICE;
                 }
             }
         }
@@ -2011,7 +2029,6 @@ esp_err_t hcd_pipe_command(hcd_pipe_handle_t pipe_hdl, hcd_pipe_cmd_t command)
     pipe_t *pipe = (pipe_t *)pipe_hdl;
     esp_err_t ret = ESP_OK;
 
-    xSemaphoreTake(pipe->port->port_mux, portMAX_DELAY);
     HCD_ENTER_CRITICAL();
     //Cannot execute pipe commands the pipe is already executing a command, or if the pipe or its port are no longer valid
     if (pipe->cs_flags.reset_lock) {
@@ -2035,7 +2052,6 @@ esp_err_t hcd_pipe_command(hcd_pipe_handle_t pipe_hdl, hcd_pipe_cmd_t command)
         pipe->cs_flags.pipe_cmd_processing = 0;
     }
     HCD_EXIT_CRITICAL();
-    xSemaphoreGive(pipe->port->port_mux);
     return ret;
 }
 
@@ -2167,6 +2183,7 @@ static void _buffer_fill(pipe_t *pipe)
     //Select the inactive buffer
     assert(pipe->multi_buffer_control.buffer_num_to_exec <= NUM_BUFFERS);
     dma_buffer_block_t *buffer_to_fill = pipe->buffers[pipe->multi_buffer_control.wr_idx];
+    buffer_to_fill->status_flags.val = 0;   //Clear the buffer's status flags
     assert(buffer_to_fill->urb == NULL);
     bool is_in = pipe->ep_char.bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK;
     int mps = pipe->ep_char.mps;
@@ -2419,16 +2436,13 @@ static inline void _buffer_parse_isoc(dma_buffer_block_t *buffer, bool is_in)
 
 static inline void _buffer_parse_error(dma_buffer_block_t *buffer)
 {
-    //The URB had an error, so we consider that NO bytes were transferred. Set actual_num_bytes to zero
+    //The URB had an error in one of its packet, or a port error), so we the entire URB an error.
     usb_transfer_t *transfer = &buffer->urb->transfer;
     transfer->actual_num_bytes = 0;
-    for (int i = 0; i < transfer->num_isoc_packets; i++) {
-        transfer->isoc_packet_desc[i].actual_num_bytes = 0;
-    }
-    //Update status of URB. Status will depend on the pipe_event
+    //Update the overall status of URB. Status will depend on the pipe_event
     switch (buffer->status_flags.pipe_event) {
         case HCD_PIPE_EVENT_NONE:
-            transfer->status = USB_TRANSFER_STATUS_CANCELED;
+            transfer->status = (buffer->status_flags.was_canceled) ? USB_TRANSFER_STATUS_CANCELED : USB_TRANSFER_STATUS_NO_DEVICE;
             break;
         case HCD_PIPE_EVENT_ERROR_XFER:
             transfer->status = USB_TRANSFER_STATUS_ERROR;
@@ -2496,12 +2510,12 @@ static void _buffer_parse(pipe_t *pipe)
     pipe->multi_buffer_control.buffer_num_to_fill++;
 }
 
-static bool _buffer_flush_all(pipe_t *pipe, bool cancelled)
+static bool _buffer_flush_all(pipe_t *pipe, bool canceled)
 {
     int cur_num_to_mark_done = pipe->multi_buffer_control.buffer_num_to_exec;
     for (int i = 0; i < cur_num_to_mark_done; i++) {
         //Mark any filled buffers as done
-        _buffer_done(pipe, 0, HCD_PIPE_EVENT_NONE);
+        _buffer_done(pipe, 0, HCD_PIPE_EVENT_NONE, canceled);
     }
     int cur_num_to_parse = pipe->multi_buffer_control.buffer_num_to_parse;
     for (int i = 0; i < cur_num_to_parse; i++) {

+ 54 - 16
components/usb/include/usb/usb_host.h

@@ -27,7 +27,14 @@ extern "C" {
 
 // ----------------------- Handles -------------------------
 
-typedef void * usb_host_client_handle_t;           /**< Handle to a client using the USB Host Library */
+/**
+ * @brief Handle to a USB Host Library asynchronous client
+ *
+ * An asynchronous client can be registered using usb_host_client_register()
+ *
+ * @note Asynchronous API
+ */
+typedef struct usb_host_client_handle_s * usb_host_client_handle_t;
 
 // ----------------------- Events --------------------------
 
@@ -93,9 +100,14 @@ typedef struct {
  * Configuration structure for a USB Host Library client. Provided in usb_host_client_register()
  */
 typedef struct {
-    usb_host_client_event_cb_t client_event_callback;   /**< Client's event callback function */
-    void *callback_arg;                                 /**< Event callback function argument */
-    int max_num_event_msg;                              /**< Maximum number of event messages that can be stored (e.g., 3) */
+    bool is_synchronous;        /**< Whether the client is asynchronous or synchronous or not. Set to false for now. */
+    int max_num_event_msg;      /**< Maximum number of event messages that can be stored (e.g., 3) */
+    union {     //Note: Made into union or future expansion
+        struct {
+            usb_host_client_event_cb_t client_event_callback;   /**< Client's event callback function */
+            void *callback_arg;                                 /**< Event callback function argument */
+        } async;
+    };
 } usb_host_client_config_t;
 
 // ------------------------------------------------ Library Functions --------------------------------------------------
@@ -129,12 +141,22 @@ esp_err_t usb_host_uninstall(void);
  * - This function handles all of the USB Host Library's processing and should be called repeatedly in a loop
  * - Check event_flags_ret to see if an flags are set indicating particular USB Host Library events
  *
+ * @note This function can block
  * @param[in] timeout_ticks Timeout in ticks to wait for an event to occur
  * @param[out] event_flags_ret Event flags that indicate what USB Host Library event occurred
  * @return esp_err_t
  */
 esp_err_t usb_host_lib_handle_events(TickType_t timeout_ticks, uint32_t *event_flags_ret);
 
+/**
+ * @brief Unblock the USB Host Library handler
+ *
+ * - This function simply unblocks the USB Host Library event handling function (usb_host_lib_handle_events())
+ *
+ * @return esp_err_t
+ */
+esp_err_t usb_host_lib_unblock(void);
+
 // ------------------------------------------------ Client Functions ---------------------------------------------------
 
 /**
@@ -165,6 +187,7 @@ esp_err_t usb_host_client_deregister(usb_host_client_handle_t client_hdl);
  *
  * - This function handles all of a client's processing and should be called repeatedly in a loop
  *
+ * @note This function can block
  * @param[in] client_hdl Client handle
  * @param[in] timeout_ticks Timeout in ticks to wait for an event to occur
  * @return esp_err_t
@@ -204,6 +227,7 @@ esp_err_t usb_host_device_open(usb_host_client_handle_t client_hdl, uint8_t dev_
  * - A client must close a device after it has finished using the device (claimed interfaces must also be released)
  * - A client must close all devices it has opened before deregistering
  *
+ * @note This function can block
  * @param[in] client_hdl Client handle
  * @param[in] dev_hdl Device handle
  * @return esp_err_t
@@ -220,10 +244,28 @@ esp_err_t usb_host_device_close(usb_host_client_handle_t client_hdl, usb_device_
  *   when all devices have been freed
  * - This function is useful when cleaning up devices before uninstalling the USB Host Library
  *
- * @return esp_err_t
+ * @return
+ *  - ESP_ERR_NOT_FINISHED: There are one or more devices that still need to be freed. Wait for USB_HOST_LIB_EVENT_FLAGS_ALL_FREE event
+ *  - ESP_OK: All devices already freed (i.e., there were no devices)
+ *  - Other: Error
  */
 esp_err_t usb_host_device_free_all(void);
 
+/**
+ * @brief Fill a list of device address
+ *
+ * - This function fills an empty list with the address of connected devices
+ * - The Device addresses can then used in usb_host_device_open()
+ * - If there are more devices than the list_len, this function will only fill
+ *   up to list_len number of devices.
+ *
+ * @param[in] list_len Length of the empty list
+ * @param[inout] dev_addr_list Empty list to be filled
+ * @param[out] num_dev_ret Number of devices
+ * @return esp_err_t
+ */
+esp_err_t usb_host_device_addr_list_fill(int list_len, uint8_t *dev_addr_list, int *num_dev_ret);
+
 // ------------------------------------------------- Device Requests ---------------------------------------------------
 
 // ------------------- Cached Requests ---------------------
@@ -234,6 +276,7 @@ esp_err_t usb_host_device_free_all(void);
  * - This function gets some basic information of a device
  * - The device must be opened first before attempting to get its information
  *
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @param[out] dev_info Device information
  * @return esp_err_t
@@ -265,6 +308,7 @@ esp_err_t usb_host_get_device_descriptor(usb_device_handle_t dev_hdl, const usb_
  * - No control transfer is sent. The device's active configuration descriptor is cached on enumeration
  * - This function simple returns a pointer to the cached descriptor
  *
+ * @note This function can block
  * @note No control transfer is sent. A device's active configuration descriptor is cached on enumeration
  * @param[in] dev_hdl Device handle
  * @param[out] config_desc Configuration descriptor
@@ -280,6 +324,7 @@ esp_err_t usb_host_get_active_config_descriptor(usb_device_handle_t dev_hdl, con
  * - A client must claim a device's interface before attempting to communicate with any of its endpoints
  * - Once an interface is claimed by a client, it cannot be claimed by any other client.
  *
+ * @note This function can block
  * @param[in] client_hdl Client handle
  * @param[in] dev_hdl Device handle
  * @param[in] bInterfaceNumber Interface number
@@ -294,6 +339,7 @@ esp_err_t usb_host_interface_claim(usb_host_client_handle_t client_hdl, usb_devi
  * - A client should release a device's interface after it no longer needs to communicate with the interface
  * - A client must release all of its interfaces of a device it has claimed before being able to close the device
  *
+ * @note This function can block
  * @param[in] client_hdl Client handle
  * @param[in] dev_hdl Device handle
  * @param[in] bInterfaceNumber Interface number
@@ -308,6 +354,7 @@ esp_err_t usb_host_interface_release(usb_host_client_handle_t client_hdl, usb_de
  * - The endpoint must be part of an interface claimed by a client
  * - Once halted, the endpoint must be cleared using usb_host_endpoint_clear() before it can communicate again
  *
+ * @note This function can block
  * @param dev_hdl Device handle
  * @param bEndpointAddress Endpoint address
  * @return esp_err_t
@@ -322,6 +369,7 @@ esp_err_t usb_host_endpoint_halt(usb_device_handle_t dev_hdl, uint8_t bEndpointA
  * - The endpoint must have been halted (either through a transfer error, or usb_host_endpoint_halt())
  * - Flushing an endpoint will caused an queued up transfers to be canceled
  *
+ * @note This function can block
  * @param dev_hdl Device handle
  * @param bEndpointAddress Endpoint address
  * @return esp_err_t
@@ -336,6 +384,7 @@ esp_err_t usb_host_endpoint_flush(usb_device_handle_t dev_hdl, uint8_t bEndpoint
  * - The endpoint must have been halted (either through a transfer error, or usb_host_endpoint_halt())
  * - If the endpoint has any queued up transfers, clearing a halt will resume their execution
  *
+ * @note This function can block
  * @param dev_hdl Device handle
  * @param bEndpointAddress Endpoint address
  * @return esp_err_t
@@ -396,17 +445,6 @@ esp_err_t usb_host_transfer_submit(usb_transfer_t *transfer);
  */
 esp_err_t usb_host_transfer_submit_control(usb_host_client_handle_t client_hdl, usb_transfer_t *transfer);
 
-/**
- * @brief Cancel a submitted transfer
- *
- * - Cancel a previously submitted transfer
- * - In its current implementation, any transfer that is already in-flight will not be canceled
- *
- * @param transfer Transfer object
- * @return esp_err_t
- */
-esp_err_t usb_host_transfer_cancel(usb_transfer_t *transfer);
-
 #ifdef __cplusplus
 }
 #endif

+ 6 - 2
components/usb/include/usb/usb_types_stack.h

@@ -43,7 +43,7 @@ typedef enum {
 /**
  * @brief Handle of a USB Device connected to a USB Host
  */
-typedef void * usb_device_handle_t;
+typedef struct usb_device_handle_s * usb_device_handle_t;
 
 /**
  * @brief Basic information of an enumerated device
@@ -68,6 +68,7 @@ typedef enum {
     USB_TRANSFER_STATUS_STALL,          /**< The transfer was stalled */
     USB_TRANSFER_STATUS_OVERFLOW,       /**< The transfer as more data was sent than was requested */
     USB_TRANSFER_STATUS_SKIPPED,        /**< ISOC packets only. The packet was skipped due to system latency or bus overload */
+    USB_TRANSFER_STATUS_NO_DEVICE,      /**< The transfer failed because the target device is gone */
 } usb_transfer_status_t;
 
 /**
@@ -102,7 +103,10 @@ typedef struct {
  *              split into multiple packets, and each packet is transferred at the endpoint's specified interval.
  * - Isochronous: Represents a stream of bytes that should be transferred to an endpoint at a fixed rate. The transfer
  *                is split into packets according to the each isoc_packet_desc. A packet is transferred at each interval
- *                of the endpoint.
+ *                of the endpoint. If an entire ISOC URB was transferred without error (skipped packets do not count as
+ *                errors), the URB's overall status and the status of each packet descriptor will be updated, and the
+ *                actual_num_bytes reflects the total bytes transferred over all packets. If the ISOC URB encounters an
+ *                error, the entire URB is considered erroneous so only the overall status will updated.
  *
  * @note For Bulk/Control/Interrupt IN transfers, the num_bytes must be a integer multiple of the endpoint's MPS
  * @note This structure should be allocated via usb_host_transfer_alloc()

+ 4 - 2
components/usb/maintainers.md

@@ -24,7 +24,10 @@ stop the remainder of the interrupt transfer.
 
 ## Channel interrupt on port errors
 
-- If there are one or more channels active, and a port error interrupt occurs (such as disconnection, over-current), the active channels will not have an interrupt. Each need to be manually disabled to obtain an interrupt.
+- If there are one or more channels active, and a port error interrupt occurs (such as disconnection, over-current), the active channels will not have an interrupt.
+- The channels will remain active (i.e., `HCCHAR.ChEna` will still be set)
+- Normal methods of disabling the channel (via setting `HCCHAR.ChDis` and waiting for an interrupt) will not work for ISOC channels (the interrupt will never be generated).
+- Therefore, on port errors, just treat all the channels as halted and treat their in-flight transfers as failed. The soft reset that occurs after will reset all the channel's registers.
 
 ## Reset
 
@@ -66,7 +69,6 @@ The HAL layer abstracts the DWC_OTG operating in Host Mode using Internal Scatte
   - If you need to halt the channel early (such as aborting a transfer), call `usbh_hal_chan_request_halt()`
 - In case of a channel error event:
   - Call `usbh_hal_chan_get_error()` to get the specific channel error that occurred
-  - You must call `usbh_hal_chan_clear_error()` after an error to clear the error and allow the channel to continue to be used.
 
 # Host Controller Driver (HCD)
 

+ 27 - 1
components/usb/private_include/usbh.h

@@ -110,6 +110,7 @@ esp_err_t usbh_uninstall(void);
  * - If blocking, the caller can block until a USB_NOTIF_SOURCE_USBH notification is received before running this
  *   function
  *
+ * @note This function can block
  * @return esp_err_t
  */
 esp_err_t usbh_process(void);
@@ -118,6 +119,21 @@ esp_err_t usbh_process(void);
 
 // --------------------- Device Pool -----------------------
 
+/**
+ * @brief Fill list with address of currently connected devices
+ *
+ * - This function fills the provided list with the address of current connected devices
+ * - Device address can then be used in usbh_dev_open()
+ * - If there are more devices than the list_len, this function will only fill
+ *   up to list_len number of devices.
+ *
+ * @param[in] list_len Length of empty list
+ * @param[inout] dev_addr_list Empty list to be filled
+ * @param[out] num_dev_ret Number of devices filled into list
+ * @return esp_err_t
+ */
+esp_err_t usbh_dev_addr_list_fill(int list_len, uint8_t *dev_addr_list, int *num_dev_ret);
+
 /**
  * @brief Open a device by address
  *
@@ -144,7 +160,9 @@ esp_err_t usbh_dev_close(usb_device_handle_t dev_hdl);
  *
  * A device marked as free will not be freed until the last client using the device has called usbh_dev_close()
  *
- * @return esp_err_t
+ * @return
+ *  - ESP_OK: There were no devices to free to begin with. Current state is all free
+ *  - ESP_ERR_NOT_FINISHED: One or more devices still need to be freed (but have been marked "to be freed")
  */
 esp_err_t usbh_dev_mark_all_free(void);
 
@@ -164,6 +182,7 @@ esp_err_t usbh_dev_get_addr(usb_device_handle_t dev_hdl, uint8_t *dev_addr);
 /**
  * @brief Get a device's information
  *
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @param[out] dev_info Device information
  * @return esp_err_t
@@ -186,6 +205,7 @@ esp_err_t usbh_dev_get_desc(usb_device_handle_t dev_hdl, const usb_device_desc_t
  *
  * Simply returns a reference to the internally cached configuration descriptor
  *
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @param config_desc_ret
  * @return esp_err_t
@@ -209,6 +229,7 @@ esp_err_t usbh_dev_submit_ctrl_urb(usb_device_handle_t dev_hdl, urb_t *urb);
  * Clients that have opened a device must call this function to allocate all endpoints in an interface that is claimed.
  * The pipe handle of the endpoint is returned so that clients can use and control the pipe directly.
  *
+ * @note This function can block
  * @note Default pipes are owned by the USBH. For control transfers, use usbh_dev_submit_ctrl_urb() instead
  * @note Device must be opened by the client first
  *
@@ -224,6 +245,7 @@ esp_err_t usbh_ep_alloc(usb_device_handle_t dev_hdl, usbh_ep_config_t *ep_config
  *
  * Free an endpoint previously opened by usbh_ep_alloc()
  *
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @param[in] bEndpointAddress Endpoint's address
  * @return esp_err_t
@@ -235,6 +257,7 @@ esp_err_t usbh_ep_free(usb_device_handle_t dev_hdl, uint8_t bEndpointAddress);
  *
  * Get the context variable assigned to and endpoint on allocation.
  *
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @param[in] bEndpointAddress Endpoint's address
  * @param[out] context_ret Context variable
@@ -336,6 +359,7 @@ esp_err_t usbh_hub_enum_fill_dev_addr(usb_device_handle_t dev_hdl, uint8_t dev_a
  *
  * @note Hub Driver only
  * @note Must call in sequence
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @param config_desc_full
  * @return esp_err_t
@@ -360,6 +384,7 @@ esp_err_t usbh_hub_enum_fill_config_num(usb_device_handle_t dev_hdl, uint8_t bCo
  *
  * @note Hub Driver only
  * @note Must call in sequence
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @return esp_err_t
  */
@@ -373,6 +398,7 @@ esp_err_t usbh_hub_enum_done(usb_device_handle_t dev_hdl);
  *
  * @note Hub Driver only
  * @note Must call in sequence
+ * @note This function can block
  * @param[in] dev_hdl Device handle
  * @return esp_err_t
  */

+ 33 - 0
components/usb/test/common/test_usb_common.c

@@ -0,0 +1,33 @@
+/*
+ * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdbool.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "soc/usb_wrap_struct.h"
+#include "test_usb_common.h"
+
+void test_usb_force_conn_state(bool connected, TickType_t delay_ticks)
+{
+    if (delay_ticks > 0) {
+        //Delay of 0 ticks causes a yield. So skip if delay_ticks is 0.
+        vTaskDelay(delay_ticks);
+    }
+    usb_wrap_dev_t *wrap = &USB_WRAP;
+    if (connected) {
+        //Disable test mode to return to previous internal PHY configuration
+        wrap->test_conf.test_enable = 0;
+    } else {
+        /*
+        Mimic a disconnection by using the internal PHY's test mode.
+        Force Output Enable to 1 (even if the controller isn't outputting). With test_tx_dp and test_tx_dm set to 0,
+        this will look like a disconnection.
+        */
+        wrap->test_conf.val = 0;
+        wrap->test_conf.test_usb_wrap_oe = 1;
+        wrap->test_conf.test_enable = 1;
+    }
+}

+ 16 - 0
components/usb/test/common/test_usb_common.h

@@ -0,0 +1,16 @@
+/*
+ * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdbool.h>
+#include "freertos/FreeRTOS.h"
+
+/**
+ * @brief For the USB PHY into the connected or disconnected state
+ *
+ * @param connected For into connected state if true, disconnected if false
+ * @param delay_ticks Delay in ticks before forcing state
+ */
+void test_usb_force_conn_state(bool connected, TickType_t delay_ticks);

+ 7 - 13
components/usb/test/common/test_usb_mock_classes.c

@@ -1,16 +1,8 @@
-// Copyright 2015-2020 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #include <stdint.h>
 #include <stdbool.h>
@@ -20,6 +12,8 @@
 
 // ---------------------------------------------------- MSC SCSI -------------------------------------------------------
 
+const char *MSC_CLIENT_TAG = "MSC Client";
+
 void mock_msc_scsi_init_cbw(mock_msc_bulk_cbw_t *cbw, bool is_read, int offset, int num_sectors, uint32_t tag)
 {
     cbw->dCBWSignature = 0x43425355;    //Fixed value

+ 7 - 13
components/usb/test/common/test_usb_mock_classes.h

@@ -1,16 +1,8 @@
-// Copyright 2015-2020 Espressif Systems (Shanghai) PTE LTD
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
+/*
+ * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 /*
 This header contains bare-bone mock implementations of some device classes in order to test various layers of the USB
@@ -29,6 +21,8 @@ extern "C" {
 
 // ---------------------------------------------------- MSC SCSI -------------------------------------------------------
 
+const char *MSC_CLIENT_TAG;
+
 /*
 Note: The mock MSC SCSI tests requires that USB flash drive be connected. The flash drive should...
 

+ 5 - 23
components/usb/test/hcd/test_hcd_common.c

@@ -9,17 +9,16 @@
 #include "freertos/FreeRTOS.h"
 #include "freertos/semphr.h"
 #include "test_utils.h"
-#include "soc/gpio_pins.h"
-#include "soc/gpio_sig_map.h"
+#include "soc/usb_wrap_struct.h"
 #include "esp_intr_alloc.h"
 #include "esp_err.h"
 #include "esp_attr.h"
 #include "esp_rom_gpio.h"
-#include "soc/usb_wrap_struct.h"
 #include "hcd.h"
 #include "usb_private.h"
 #include "usb/usb_types_ch9.h"
 #include "test_hcd_common.h"
+#include "test_usb_common.h"
 
 #define PORT_NUM                1
 #define EVENT_QUEUE_LEN         5
@@ -137,23 +136,6 @@ int test_hcd_get_num_pipe_events(hcd_pipe_handle_t pipe_hdl)
 
 // ----------------------------------------------- Driver/Port Related -------------------------------------------------
 
-void test_hcd_force_conn_state(bool connected, TickType_t delay_ticks)
-{
-    vTaskDelay(delay_ticks);
-    usb_wrap_dev_t *wrap = &USB_WRAP;
-    if (connected) {
-        //Swap back to internal PHY that is connected to a device
-        wrap->otg_conf.phy_sel = 0;
-    } else {
-        //Set external PHY input signals to fixed voltage levels mimicking a disconnected state
-        esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ZERO_INPUT, USB_EXTPHY_VP_IDX, false);
-        esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ZERO_INPUT, USB_EXTPHY_VM_IDX, false);
-        esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, USB_EXTPHY_RCV_IDX, false);
-        //Swap to the external PHY
-        wrap->otg_conf.phy_sel = 1;
-    }
-}
-
 hcd_port_handle_t test_hcd_setup(void)
 {
     //Create a queue for port callback to queue up port events
@@ -175,7 +157,7 @@ hcd_port_handle_t test_hcd_setup(void)
     TEST_ASSERT_EQUAL(ESP_OK, hcd_port_init(PORT_NUM, &port_config, &port_hdl));
     TEST_ASSERT_NOT_EQUAL(NULL, port_hdl);
     TEST_ASSERT_EQUAL(HCD_PORT_STATE_NOT_POWERED, hcd_port_get_state(port_hdl));
-    test_hcd_force_conn_state(false, 0);    //Force disconnected state on PHY
+    test_usb_force_conn_state(false, 0);    //Force disconnected state on PHY
     return port_hdl;
 }
 
@@ -198,7 +180,7 @@ usb_speed_t test_hcd_wait_for_conn(hcd_port_handle_t port_hdl)
     TEST_ASSERT_EQUAL(HCD_PORT_STATE_DISCONNECTED, hcd_port_get_state(port_hdl));
     //Wait for connection event
     printf("Waiting for connection\n");
-    test_hcd_force_conn_state(true, pdMS_TO_TICKS(100));     //Allow for connected state on PHY
+    test_usb_force_conn_state(true, pdMS_TO_TICKS(100));     //Allow for connected state on PHY
     test_hcd_expect_port_event(port_hdl, HCD_PORT_EVENT_CONNECTION);
     TEST_ASSERT_EQUAL(HCD_PORT_EVENT_CONNECTION, hcd_port_handle_event(port_hdl));
     TEST_ASSERT_EQUAL(HCD_PORT_STATE_DISABLED, hcd_port_get_state(port_hdl));
@@ -227,7 +209,7 @@ void test_hcd_wait_for_disconn(hcd_port_handle_t port_hdl, bool already_disabled
     }
     //Wait for a safe disconnect
     printf("Waiting for disconnection\n");
-    test_hcd_force_conn_state(false, pdMS_TO_TICKS(100));    //Force disconnected state on PHY
+    test_usb_force_conn_state(false, pdMS_TO_TICKS(100));    //Force disconnected state on PHY
     test_hcd_expect_port_event(port_hdl, HCD_PORT_EVENT_DISCONNECTION);
     TEST_ASSERT_EQUAL(HCD_PORT_EVENT_DISCONNECTION, hcd_port_handle_event(port_hdl));
     TEST_ASSERT_EQUAL(HCD_PORT_STATE_RECOVERY, hcd_port_get_state(port_hdl));

+ 2 - 10
components/usb/test/hcd/test_hcd_common.h

@@ -48,14 +48,6 @@ int test_hcd_get_num_pipe_events(hcd_pipe_handle_t pipe_hdl);
 
 // ----------------------------------------------- Driver/Port Related -------------------------------------------------
 
-/**
- * @brief For the USB PHY into the connected or disconnected state
- *
- * @param connected For into connected state if true, disconnected if false
- * @param delay_ticks Delay in ticks before forcing state
- */
-void test_hcd_force_conn_state(bool connected, TickType_t delay_ticks);
-
 /**
  * @brief Sets up the HCD and initializes an HCD port.
  *
@@ -73,7 +65,7 @@ void test_hcd_teardown(hcd_port_handle_t port_hdl);
 /**
  * @brief Wait for a connection on an HCD port
  *
- * @note This function will internally call test_hcd_force_conn_state() to allow for a connection
+ * @note This function will internally call test_usb_force_conn_state() to allow for a connection
  *
  * @param port_hdl Port handle
  * @return usb_speed_t Speed of the connected device
@@ -83,7 +75,7 @@ usb_speed_t test_hcd_wait_for_conn(hcd_port_handle_t port_hdl);
 /**
  * @brief Wait for a disconnection on an HCD port
  *
- * @note This fucntion will internally call test_hcd_force_conn_state() to force a disconnection
+ * @note This fucntion will internally call test_usb_force_conn_state() to force a disconnection
  *
  * @param port_hdl Port handle
  * @param already_disabled Whether the HCD port is already in the disabled state

+ 100 - 0
components/usb/test/hcd/test_hcd_isoc.c

@@ -11,12 +11,14 @@
 #include "unity.h"
 #include "test_utils.h"
 #include "test_usb_mock_classes.h"
+#include "test_usb_common.h"
 #include "test_hcd_common.h"
 
 #define NUM_URBS                3
 #define NUM_PACKETS_PER_URB     3
 #define ISOC_PACKET_SIZE        MOCK_ISOC_EP_MPS
 #define URB_DATA_BUFF_SIZE      (NUM_PACKETS_PER_URB * ISOC_PACKET_SIZE)
+#define POST_ENQUEUE_DELAY_US   20
 
 /*
 Test HCD ISOC pipe URBs
@@ -78,6 +80,9 @@ TEST_CASE("Test HCD isochronous pipe URBs", "[hcd][ignore]")
         urb_t *urb = hcd_urb_dequeue(isoc_out_pipe);
         TEST_ASSERT_EQUAL(urb_list[urb_idx], urb);
         TEST_ASSERT_EQUAL(URB_CONTEXT_VAL, urb->transfer.context);
+        //Overall URB status and overall number of bytes
+        TEST_ASSERT_EQUAL(URB_DATA_BUFF_SIZE, urb->transfer.actual_num_bytes);
+        TEST_ASSERT_EQUAL(USB_TRANSFER_STATUS_COMPLETED, urb->transfer.status);
         for (int pkt_idx = 0; pkt_idx < NUM_PACKETS_PER_URB; pkt_idx++) {
             TEST_ASSERT_EQUAL(USB_TRANSFER_STATUS_COMPLETED, urb->transfer.isoc_packet_desc[pkt_idx].status);
         }
@@ -92,3 +97,98 @@ TEST_CASE("Test HCD isochronous pipe URBs", "[hcd][ignore]")
     test_hcd_wait_for_disconn(port_hdl, false);
     test_hcd_teardown(port_hdl);
 }
+
+/*
+Test a port sudden disconnect with an active ISOC pipe
+
+Purpose: Test that when sudden disconnection happens on an HCD port, the ISOC pipe will
+    - Remain active after the HCD_PORT_EVENT_SUDDEN_DISCONN port event
+    - ISOC pipe can be halted
+    - ISOC pipe can be flushed (and transfers status are updated accordingly)
+
+Procedure:
+    - Setup HCD and wait for connection
+    - Allocate default pipe and enumerate the device
+    - Allocate an isochronous pipe and multiple URBs. Each URB should contain multiple packets to test HCD's ability to
+      schedule an URB across multiple intervals.
+    - Enqueue those URBs
+    - Trigger a disconnect after a short delay
+    - Check that HCD_PORT_EVENT_SUDDEN_DISCONN event is generated. Handle that port event.
+    - Check that both pipes remain in the HCD_PIPE_STATE_ACTIVE after the port error.
+    - Check that both pipes pipe can be halted.
+    - Check that the default pipe can be flushed. A HCD_PIPE_EVENT_URB_DONE event should be generated for the ISOC pipe
+      because it had enqueued URBs.
+    - Check that all URBs can be dequeued and their status is updated
+    - Free both pipes
+    - Teardown
+*/
+TEST_CASE("Test HCD isochronous pipe sudden disconnect", "[hcd][ignore]")
+{
+    hcd_port_handle_t port_hdl = test_hcd_setup();  //Setup the HCD and port
+    usb_speed_t port_speed = test_hcd_wait_for_conn(port_hdl);  //Trigger a connection
+    //The MPS of the ISOC OUT pipe is quite large, so we need to bias the FIFO sizing
+    TEST_ASSERT_EQUAL(ESP_OK, hcd_port_set_fifo_bias(port_hdl, HCD_PORT_FIFO_BIAS_PTX));
+    vTaskDelay(pdMS_TO_TICKS(100)); //Short delay send of SOF (for FS) or EOPs (for LS)
+
+    //Enumerate and reset device
+    hcd_pipe_handle_t default_pipe = test_hcd_pipe_alloc(port_hdl, NULL, 0, port_speed); //Create a default pipe (using a NULL EP descriptor)
+    uint8_t dev_addr = test_hcd_enum_device(default_pipe);
+
+    //Create ISOC OUT pipe to non-existent device
+    hcd_pipe_handle_t isoc_out_pipe = test_hcd_pipe_alloc(port_hdl, &mock_isoc_out_ep_desc, dev_addr + 1, port_speed);
+    //Create URBs
+    urb_t *urb_list[NUM_URBS];
+    //Initialize URBs
+    for (int urb_idx = 0; urb_idx < NUM_URBS; urb_idx++) {
+        urb_list[urb_idx] = test_hcd_alloc_urb(NUM_PACKETS_PER_URB, URB_DATA_BUFF_SIZE);
+        urb_list[urb_idx]->transfer.num_bytes = URB_DATA_BUFF_SIZE;
+        urb_list[urb_idx]->transfer.context = URB_CONTEXT_VAL;
+        for (int pkt_idx = 0; pkt_idx < NUM_PACKETS_PER_URB; pkt_idx++) {
+            urb_list[urb_idx]->transfer.isoc_packet_desc[pkt_idx].num_bytes = ISOC_PACKET_SIZE;
+            //Each packet will consist of the same byte, but each subsequent packet's byte will increment (i.e., packet 0 transmits all 0x0, packet 1 transmits all 0x1)
+            memset(&urb_list[urb_idx]->transfer.data_buffer[pkt_idx * ISOC_PACKET_SIZE], (urb_idx * NUM_URBS) + pkt_idx, ISOC_PACKET_SIZE);
+        }
+    }
+    //Enqueue URBs
+    for (int i = 0; i < NUM_URBS; i++) {
+        TEST_ASSERT_EQUAL(ESP_OK, hcd_urb_enqueue(isoc_out_pipe, urb_list[i]));
+    }
+    //Add a short delay to let the transfers run for a bit
+    esp_rom_delay_us(POST_ENQUEUE_DELAY_US);
+    test_usb_force_conn_state(false, 0);
+    //Disconnect event should have occurred. Handle the port event
+    test_hcd_expect_port_event(port_hdl, HCD_PORT_EVENT_DISCONNECTION);
+    TEST_ASSERT_EQUAL(HCD_PORT_EVENT_DISCONNECTION, hcd_port_handle_event(port_hdl));
+    TEST_ASSERT_EQUAL(HCD_PORT_STATE_RECOVERY, hcd_port_get_state(port_hdl));
+    printf("Sudden disconnect\n");
+
+    //Both pipes should still be active
+    TEST_ASSERT_EQUAL(HCD_PIPE_STATE_ACTIVE, hcd_pipe_get_state(default_pipe));
+    TEST_ASSERT_EQUAL(HCD_PIPE_STATE_ACTIVE, hcd_pipe_get_state(isoc_out_pipe));
+    //Halt both pipes
+    TEST_ASSERT_EQUAL(ESP_OK, hcd_pipe_command(default_pipe, HCD_PIPE_CMD_HALT));
+    TEST_ASSERT_EQUAL(ESP_OK, hcd_pipe_command(isoc_out_pipe, HCD_PIPE_CMD_HALT));
+    TEST_ASSERT_EQUAL(HCD_PIPE_STATE_HALTED, hcd_pipe_get_state(default_pipe));
+    TEST_ASSERT_EQUAL(HCD_PIPE_STATE_HALTED, hcd_pipe_get_state(isoc_out_pipe));
+    //Flush both pipes. ISOC pipe should return completed URBs
+    TEST_ASSERT_EQUAL(ESP_OK, hcd_pipe_command(default_pipe, HCD_PIPE_CMD_FLUSH));
+    TEST_ASSERT_EQUAL(ESP_OK, hcd_pipe_command(isoc_out_pipe, HCD_PIPE_CMD_FLUSH));
+
+    //Dequeue ISOC URBs
+    for (int urb_idx = 0; urb_idx < NUM_URBS; urb_idx++) {
+        urb_t *urb = hcd_urb_dequeue(isoc_out_pipe);
+        TEST_ASSERT_EQUAL(urb_list[urb_idx], urb);
+        TEST_ASSERT_EQUAL(URB_CONTEXT_VAL, urb->transfer.context);
+        //The URB has either completed entirely or is marked as no_device
+        TEST_ASSERT(urb->transfer.status == USB_TRANSFER_STATUS_COMPLETED || urb->transfer.status == USB_TRANSFER_STATUS_NO_DEVICE);
+    }
+
+    //Free URB list and pipe
+    for (int i = 0; i < NUM_URBS; i++) {
+        test_hcd_free_urb(urb_list[i]);
+    }
+    test_hcd_pipe_free(isoc_out_pipe);
+    test_hcd_pipe_free(default_pipe);
+    //Cleanup
+    test_hcd_teardown(port_hdl);
+}

+ 11 - 8
components/usb/test/hcd/test_hcd_port.c

@@ -10,6 +10,7 @@
 #include "unity.h"
 #include "esp_rom_sys.h"
 #include "test_utils.h"
+#include "test_usb_common.h"
 #include "test_hcd_common.h"
 
 #define TEST_DEV_ADDR               0
@@ -32,7 +33,7 @@ Procedure:
     - Start transfers but trigger a disconnect after a short delay
     - Check that HCD_PORT_EVENT_SUDDEN_DISCONN event is generated. Handle that port event.
     - Check that the default pipe remains in the HCD_PIPE_STATE_ACTIVE after the port error.
-    - Check that the default pipe can be halted, a HCD_PIPE_EVENT_URB_DONE event should be generated
+    - Check that the default pipe can be halted.
     - Check that the default pipe can be flushed, a HCD_PIPE_EVENT_URB_DONE event should be generated
     - Check that all URBs can be dequeued.
     - Free default pipe
@@ -65,8 +66,8 @@ TEST_CASE("Test HCD port sudden disconnect", "[hcd][ignore]")
     }
     //Add a short delay to let the transfers run for a bit
     esp_rom_delay_us(POST_ENQUEUE_DELAY_US);
-    test_hcd_force_conn_state(false, 0);
-    //Disconnect event should have occurred. Handle the event
+    test_usb_force_conn_state(false, 0);
+    //Disconnect event should have occurred. Handle the port event
     test_hcd_expect_port_event(port_hdl, HCD_PORT_EVENT_DISCONNECTION);
     TEST_ASSERT_EQUAL(HCD_PORT_EVENT_DISCONNECTION, hcd_port_handle_event(port_hdl));
     TEST_ASSERT_EQUAL(HCD_PORT_STATE_RECOVERY, hcd_port_get_state(port_hdl));
@@ -75,17 +76,17 @@ TEST_CASE("Test HCD port sudden disconnect", "[hcd][ignore]")
     //We should be able to halt then flush the pipe
     TEST_ASSERT_EQUAL(HCD_PIPE_STATE_ACTIVE, hcd_pipe_get_state(default_pipe));
     TEST_ASSERT_EQUAL(ESP_OK, hcd_pipe_command(default_pipe, HCD_PIPE_CMD_HALT));
-    test_hcd_expect_pipe_event(default_pipe, HCD_PIPE_EVENT_URB_DONE);
+    printf("Pipe halted\n");
     TEST_ASSERT_EQUAL(HCD_PIPE_STATE_HALTED, hcd_pipe_get_state(default_pipe));
     TEST_ASSERT_EQUAL(ESP_OK, hcd_pipe_command(default_pipe, HCD_PIPE_CMD_FLUSH));
     test_hcd_expect_pipe_event(default_pipe, HCD_PIPE_EVENT_URB_DONE);
-    printf("Pipe halted and flushed\n");
+    printf("Pipe flushed\n");
 
     //Dequeue URBs
     for (int i = 0; i < NUM_URBS; i++) {
         urb_t *urb = hcd_urb_dequeue(default_pipe);
         TEST_ASSERT_EQUAL(urb_list[i], urb);
-        TEST_ASSERT(urb->transfer.status == USB_TRANSFER_STATUS_COMPLETED || urb->transfer.status == USB_TRANSFER_STATUS_CANCELED);
+        TEST_ASSERT(urb->transfer.status == USB_TRANSFER_STATUS_COMPLETED || urb->transfer.status == USB_TRANSFER_STATUS_NO_DEVICE);
         if (urb->transfer.status == USB_TRANSFER_STATUS_COMPLETED) {
             //We must have transmitted at least the setup packet, but device may return less than bytes requested
             TEST_ASSERT_GREATER_OR_EQUAL(sizeof(usb_setup_packet_t), urb->transfer.actual_num_bytes);
@@ -259,7 +260,9 @@ TEST_CASE("Test HCD port disable", "[hcd][ignore]")
     for (int i = 0; i < NUM_URBS; i++) {
         urb_t *urb = hcd_urb_dequeue(default_pipe);
         TEST_ASSERT_EQUAL(urb_list[i], urb);
-        TEST_ASSERT(urb->transfer.status == USB_TRANSFER_STATUS_COMPLETED || urb->transfer.status == USB_TRANSFER_STATUS_CANCELED);
+        TEST_ASSERT(urb->transfer.status == USB_TRANSFER_STATUS_COMPLETED ||    //The transfer completed before the pipe halted
+                    urb->transfer.status == USB_TRANSFER_STATUS_CANCELED ||     //The transfer was stopped mid-way by the halt
+                    urb->transfer.status == USB_TRANSFER_STATUS_NO_DEVICE);     //The transfer was never started
         if (urb->transfer.status == USB_TRANSFER_STATUS_COMPLETED) {
             //We must have transmitted at least the setup packet, but device may return less than bytes requested
             TEST_ASSERT_GREATER_OR_EQUAL(sizeof(usb_setup_packet_t), urb->transfer.actual_num_bytes);
@@ -302,7 +305,7 @@ static void concurrent_task(void *arg)
     xSemaphoreTake(sync_sem, portMAX_DELAY);
     vTaskDelay(pdMS_TO_TICKS(10));  //Give a short delay let reset command start in main thread
     //Force a disconnection
-    test_hcd_force_conn_state(false, 0);
+    test_usb_force_conn_state(false, 0);
     vTaskDelay(portMAX_DELAY);  //Block forever and wait to be deleted
 }
 

+ 7 - 4
components/usb/test/usb_host/ctrl_client_async_seq.c

@@ -10,6 +10,7 @@
 #include "freertos/task.h"
 #include "esp_err.h"
 #include "esp_log.h"
+#include "test_usb_common.h"
 #include "ctrl_client.h"
 #include "usb/usb_host.h"
 #include "unity.h"
@@ -32,7 +33,6 @@ Implementation of a control transfer client used for USB Host Tests.
     - Deregister control client
 */
 
-#define MAX(x,y)                        (((x) >= (y)) ? (x) : (y))
 #define CTRL_CLIENT_MAX_EVENT_MSGS      5
 #define NUM_TRANSFER_OBJ                3
 #define MAX_TRANSFER_BYTES              256
@@ -97,9 +97,12 @@ void ctrl_client_async_seq_task(void *arg)
 
     //Register client
     usb_host_client_config_t client_config = {
-        .client_event_callback = ctrl_client_event_cb,
-        .callback_arg = (void *)&ctrl_obj,
+        .is_synchronous = false,
         .max_num_event_msg = CTRL_CLIENT_MAX_EVENT_MSGS,
+        .async = {
+            .client_event_callback = ctrl_client_event_cb,
+            .callback_arg = (void *)&ctrl_obj,
+        },
     };
     TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_register(&client_config, &ctrl_obj.client_hdl));
 
@@ -153,7 +156,7 @@ void ctrl_client_async_seq_task(void *arg)
                 usb_transfer_t *transfer = ctrl_xfer[ctrl_obj.num_xfer_sent % NUM_TRANSFER_OBJ];
                 USB_SETUP_PACKET_INIT_GET_CONFIG_DESC((usb_setup_packet_t *)transfer->data_buffer, 0, MAX_TRANSFER_BYTES);
                 transfer->num_bytes = sizeof(usb_setup_packet_t) + MAX_TRANSFER_BYTES;
-                transfer->bEndpointAddress = 0;
+                transfer->bEndpointAddress = 0x80;
                 TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit_control(ctrl_obj.client_hdl, transfer));
                 ctrl_obj.num_xfer_sent++;
                 ctrl_obj.next_stage = TEST_STAGE_CTRL_XFER_WAIT;

+ 4 - 0
components/usb/test/usb_host/msc_client.h

@@ -6,6 +6,8 @@
 
 #include <stdint.h>
 
+#define MSC_ASYNC_CLIENT_MAX_EVENT_MSGS       5
+
 typedef struct {
     int num_sectors_to_read;
     int num_sectors_per_xfer;
@@ -15,3 +17,5 @@ typedef struct {
 } msc_client_test_param_t;
 
 void msc_client_async_seq_task(void *arg);
+
+void msc_client_async_dconn_task(void *arg);

+ 247 - 0
components/usb/test/usb_host/msc_client_async_dconn.c

@@ -0,0 +1,247 @@
+/*
+ * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdint.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/param.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "esp_err.h"
+#include "esp_log.h"
+#include "test_usb_mock_classes.h"
+#include "test_usb_common.h"
+#include "msc_client.h"
+#include "usb/usb_host.h"
+#include "unity.h"
+#include "test_utils.h"
+
+/*
+Implementation of an asynchronous MSC client used for USB Host disconnection test.
+
+- The MSC client will:
+    - Register itself as a client
+    - Receive USB_HOST_CLIENT_EVENT_NEW_DEV event message, and open the device
+    - Allocate IN and OUT transfer objects for MSC SCSI transfers
+    - Trigger a single MSC SCSI transfer
+        - Split the data stage into multiple transfers (so that the endpoint multiple queued up transfers)
+        - Cause a disconnection mid-way through the data stage
+    - All of the transfers should be automatically deqeueud
+    - Then a USB_HOST_CLIENT_EVENT_DEV_GONE event should occur afterwards
+    - Free transfer objects
+    - Close device
+    - Deregister MSC client
+*/
+
+typedef enum {
+    TEST_STAGE_WAIT_CONN,
+    TEST_STAGE_DEV_OPEN,
+    TEST_STAGE_MSC_RESET,
+    TEST_STAGE_MSC_CBW,
+    TEST_STAGE_MSC_DATA_DCONN,
+    TEST_STAGE_DEV_CLOSE,
+} test_stage_t;
+
+typedef struct {
+    msc_client_test_param_t test_param;
+    test_stage_t cur_stage;
+    test_stage_t next_stage;
+    uint8_t dev_addr_to_open;
+    usb_host_client_handle_t client_hdl;
+    usb_device_handle_t dev_hdl;
+    int num_data_transfers;
+    int event_count;
+} msc_client_obj_t;
+
+static void msc_reset_cbw_transfer_cb(usb_transfer_t *transfer)
+{
+    msc_client_obj_t *msc_obj = (msc_client_obj_t *)transfer->context;
+    //We expect the reset and CBW transfers to complete with no issues
+    TEST_ASSERT_EQUAL(USB_TRANSFER_STATUS_COMPLETED, transfer->status);
+    TEST_ASSERT_EQUAL(transfer->num_bytes, transfer->actual_num_bytes);
+    switch (msc_obj->cur_stage) {
+        case TEST_STAGE_MSC_RESET:
+            msc_obj->next_stage = TEST_STAGE_MSC_CBW;
+            break;
+        case TEST_STAGE_MSC_CBW:
+            msc_obj->next_stage = TEST_STAGE_MSC_DATA_DCONN;
+            break;
+        default:
+            abort();
+            break;
+    }
+}
+
+static void msc_data_transfer_cb(usb_transfer_t *transfer)
+{
+    //The data stage should have either completed, or failed due to the disconnection.
+    TEST_ASSERT(transfer->status == USB_TRANSFER_STATUS_COMPLETED || transfer->status == USB_TRANSFER_STATUS_NO_DEVICE);
+    if (transfer->status == USB_TRANSFER_STATUS_COMPLETED) {
+        TEST_ASSERT_EQUAL(transfer->num_bytes, transfer->actual_num_bytes);
+    } else {
+        TEST_ASSERT_EQUAL(0, transfer->actual_num_bytes);
+    }
+    msc_client_obj_t *msc_obj = (msc_client_obj_t *)transfer->context;
+    msc_obj->event_count++;
+    //If all transfers dequeued and device gone event occurred. Go to next stage
+    if (msc_obj->event_count >= msc_obj->num_data_transfers + 1) {
+        msc_obj->next_stage = TEST_STAGE_DEV_CLOSE;
+    }
+}
+
+static void msc_client_event_cb(const usb_host_client_event_msg_t *event_msg, void *arg)
+{
+    msc_client_obj_t *msc_obj = (msc_client_obj_t *)arg;
+    switch (event_msg->event) {
+        case USB_HOST_CLIENT_EVENT_NEW_DEV:
+            TEST_ASSERT_EQUAL(TEST_STAGE_WAIT_CONN, msc_obj->cur_stage);
+            msc_obj->next_stage = TEST_STAGE_DEV_OPEN;
+            msc_obj->dev_addr_to_open = event_msg->new_dev.address;
+            break;
+        case USB_HOST_CLIENT_EVENT_DEV_GONE:
+            msc_obj->event_count++;
+            //If all transfers dequeued and device gone event occurred. Go to next stage
+            if (msc_obj->event_count >= msc_obj->num_data_transfers + 1) {
+                msc_obj->next_stage = TEST_STAGE_DEV_CLOSE;
+            }
+            break;
+        default:
+            abort();    //Should never occur in this test
+            break;
+    }
+}
+
+void msc_client_async_dconn_task(void *arg)
+{
+    msc_client_obj_t msc_obj;
+    memcpy(&msc_obj.test_param, arg, sizeof(msc_client_test_param_t));
+    msc_obj.cur_stage = TEST_STAGE_WAIT_CONN;
+    msc_obj.next_stage = TEST_STAGE_WAIT_CONN;
+    msc_obj.dev_addr_to_open = 0;
+    msc_obj.client_hdl = NULL;
+    msc_obj.dev_hdl = NULL;
+    msc_obj.num_data_transfers = msc_obj.test_param.num_sectors_per_xfer / MOCK_MSC_SCSI_SECTOR_SIZE;
+    msc_obj.event_count = 0;
+
+    //Register client
+    usb_host_client_config_t client_config = {
+        .is_synchronous = false,
+        .max_num_event_msg = MSC_ASYNC_CLIENT_MAX_EVENT_MSGS,
+        .async = {
+            .client_event_callback = msc_client_event_cb,
+            .callback_arg = (void *)&msc_obj,
+        },
+    };
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_register(&client_config, &msc_obj.client_hdl));
+
+    //Allocate transfers
+    usb_transfer_t *xfer_out;   //Must be large enough to contain CBW and MSC reset control transfer
+    usb_transfer_t *xfer_in[msc_obj.num_data_transfers];   //We manually split the data stage into multiple transfers
+    size_t xfer_out_size = MAX(sizeof(mock_msc_bulk_cbw_t), sizeof(usb_setup_packet_t));
+    size_t xfer_in_size = MOCK_MSC_SCSI_SECTOR_SIZE;
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_alloc(xfer_out_size, 0, &xfer_out));
+    xfer_out->context = (void *)&msc_obj;
+    for (int i = 0; i < msc_obj.num_data_transfers; i++) {
+        TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_alloc(xfer_in_size, 0, &xfer_in[i]));
+        xfer_in[i]->context = (void *)&msc_obj;
+    }
+
+    //Wait to be started by main thread
+    ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
+    ESP_LOGD(MSC_CLIENT_TAG, "Starting");
+
+    bool exit_loop = false;
+    bool skip_event_handling = false;
+    while (!exit_loop) {
+        if (!skip_event_handling) {
+            TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_handle_events(msc_obj.client_hdl, portMAX_DELAY));
+        }
+        skip_event_handling = false;
+        if (msc_obj.cur_stage == msc_obj.next_stage) {
+            continue;
+        }
+        msc_obj.cur_stage = msc_obj.next_stage;
+
+        switch (msc_obj.cur_stage) {
+            case TEST_STAGE_DEV_OPEN: {
+                ESP_LOGD(MSC_CLIENT_TAG, "Open");
+                //Open the device
+                TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_open(msc_obj.client_hdl, msc_obj.dev_addr_to_open, &msc_obj.dev_hdl));
+                //Target our transfers to the device
+                xfer_out->device_handle = msc_obj.dev_hdl;
+                xfer_out->callback = msc_reset_cbw_transfer_cb;
+                for (int i = 0; i < msc_obj.num_data_transfers; i++) {
+                    xfer_in[i]->device_handle = msc_obj.dev_hdl;
+                    xfer_in[i]->callback = msc_data_transfer_cb;
+                }
+                //Check the VID/PID of the opened device
+                const usb_device_desc_t *device_desc;
+                TEST_ASSERT_EQUAL(ESP_OK, usb_host_get_device_descriptor(msc_obj.dev_hdl, &device_desc));
+                TEST_ASSERT_EQUAL(msc_obj.test_param.idVendor, device_desc->idVendor);
+                TEST_ASSERT_EQUAL(msc_obj.test_param.idProduct, device_desc->idProduct);
+                //Claim the MSC interface
+                TEST_ASSERT_EQUAL(ESP_OK, usb_host_interface_claim(msc_obj.client_hdl, msc_obj.dev_hdl, MOCK_MSC_SCSI_INTF_NUMBER, MOCK_MSC_SCSI_INTF_ALT_SETTING));
+                msc_obj.next_stage = TEST_STAGE_MSC_RESET;
+                skip_event_handling = true; //Need to execute TEST_STAGE_MSC_RESET
+                break;
+            }
+            case TEST_STAGE_MSC_RESET: {
+                ESP_LOGD(MSC_CLIENT_TAG, "MSC Reset");
+                //Send an MSC SCSI interface reset
+                MOCK_MSC_SCSI_REQ_INIT_RESET((usb_setup_packet_t *)xfer_out->data_buffer, MOCK_MSC_SCSI_INTF_NUMBER);
+                xfer_out->num_bytes = sizeof(usb_setup_packet_t);
+                xfer_out->bEndpointAddress = 0;
+                TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit_control(msc_obj.client_hdl, xfer_out));
+                //Next stage set from transfer callback
+                break;
+            }
+            case TEST_STAGE_MSC_CBW: {
+                ESP_LOGD(MSC_CLIENT_TAG, "CBW");
+                mock_msc_scsi_init_cbw((mock_msc_bulk_cbw_t *)xfer_out->data_buffer, true, 0, msc_obj.test_param.num_sectors_per_xfer, msc_obj.test_param.msc_scsi_xfer_tag);
+                xfer_out->num_bytes = sizeof(mock_msc_bulk_cbw_t);
+                xfer_out->bEndpointAddress = MOCK_MSC_SCSI_BULK_OUT_EP_ADDR;
+                TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_out));
+                //Next stage set from transfer callback
+                break;
+            }
+            case TEST_STAGE_MSC_DATA_DCONN: {
+                ESP_LOGD(MSC_CLIENT_TAG, "Data and disconnect");
+                //Setup the Data IN transfers
+                for (int i = 0; i < msc_obj.num_data_transfers; i++) {
+                    xfer_in[i]->num_bytes = usb_round_up_to_mps(MOCK_MSC_SCSI_SECTOR_SIZE, MOCK_MSC_SCSI_BULK_EP_MPS);
+                    xfer_in[i]->bEndpointAddress = MOCK_MSC_SCSI_BULK_IN_EP_ADDR;
+                }
+                //Submit those transfers
+                for (int i = 0; i < msc_obj.num_data_transfers; i++) {
+                    TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_in[i]));
+                }
+                //Trigger a disconnect
+                test_usb_force_conn_state(false, 0);
+                //Next stage set from transfer callback
+                break;
+            }
+            case TEST_STAGE_DEV_CLOSE: {
+                ESP_LOGD(MSC_CLIENT_TAG, "Close");
+                TEST_ASSERT_EQUAL(ESP_OK, usb_host_interface_release(msc_obj.client_hdl, msc_obj.dev_hdl, MOCK_MSC_SCSI_INTF_NUMBER));
+                TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_close(msc_obj.client_hdl, msc_obj.dev_hdl));
+                exit_loop = true;
+                break;
+            }
+            default:
+                abort();
+                break;
+        }
+    }
+    //Free transfers
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_free(xfer_out));
+    for (int i = 0; i < msc_obj.num_data_transfers; i++) {
+        TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_free(xfer_in[i]));
+    }
+    //Deregister the client
+    TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_deregister(msc_obj.client_hdl));
+    ESP_LOGD(MSC_CLIENT_TAG, "Done");
+    vTaskDelete(NULL);
+}

+ 9 - 10
components/usb/test/usb_host/msc_client_async_seq.c

@@ -6,12 +6,13 @@
 
 #include <stdint.h>
 #include <string.h>
-#include <assert.h>
+#include <stdlib.h>
+#include <sys/param.h>
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
-#include "freertos/semphr.h"
 #include "esp_err.h"
 #include "esp_log.h"
+#include "test_usb_common.h"
 #include "test_usb_mock_classes.h"
 #include "msc_client.h"
 #include "usb/usb_host.h"
@@ -35,11 +36,6 @@ Implementation of an MSC client used for USB Host Tests
     - Deregister MSC client
 */
 
-#define MAX(x,y)                        (((x) >= (y)) ? (x) : (y))
-#define MSC_CLIENT_MAX_EVENT_MSGS       5
-
-const char *MSC_CLIENT_TAG = "MSC Client";
-
 typedef enum {
     TEST_STAGE_WAIT_CONN,
     TEST_STAGE_DEV_OPEN,
@@ -133,9 +129,12 @@ void msc_client_async_seq_task(void *arg)
 
     //Register client
     usb_host_client_config_t client_config = {
-        .client_event_callback = msc_client_event_cb,
-        .callback_arg = (void *)&msc_obj,
-        .max_num_event_msg = MSC_CLIENT_MAX_EVENT_MSGS,
+        .is_synchronous = false,
+        .max_num_event_msg = MSC_ASYNC_CLIENT_MAX_EVENT_MSGS,
+        .async = {
+            .client_event_callback = msc_client_event_cb,
+            .callback_arg = (void *)&msc_obj,
+        },
     };
     TEST_ASSERT_EQUAL(ESP_OK, usb_host_client_register(&client_config, &msc_obj.client_hdl));
 

+ 3 - 2
components/usb/test/usb_host/test_usb_host_async.c

@@ -8,6 +8,7 @@
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
 #include "freertos/semphr.h"
+#include "esp_err.h"
 #include "esp_intr_alloc.h"
 #include "test_usb_mock_classes.h"
 #include "msc_client.h"
@@ -71,7 +72,7 @@ TEST_CASE("Test USB Host async (single client)", "[usb_host][ignore]")
         usb_host_lib_handle_events(portMAX_DELAY, &event_flags);
         if (event_flags & USB_HOST_LIB_EVENT_FLAGS_NO_CLIENTS) {
             printf("No more clients\n");
-            TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_free_all());
+            TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_device_free_all());
         }
         if (event_flags & USB_HOST_LIB_EVENT_FLAGS_ALL_FREE) {
             break;
@@ -143,7 +144,7 @@ TEST_CASE("Test USB Host async (multi client)", "[usb_host][ignore]")
         usb_host_lib_handle_events(portMAX_DELAY, &event_flags);
         if (event_flags & USB_HOST_LIB_EVENT_FLAGS_NO_CLIENTS) {
             printf("No more clients\n");
-            TEST_ASSERT_EQUAL(ESP_OK, usb_host_device_free_all());
+            TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_device_free_all());
         }
         if (event_flags & USB_HOST_LIB_EVENT_FLAGS_ALL_FREE) {
             break;

+ 111 - 0
components/usb/test/usb_host/test_usb_host_plugging.c

@@ -0,0 +1,111 @@
+/*
+ * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <stdio.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "freertos/timers.h"
+#include "esp_err.h"
+#include "esp_intr_alloc.h"
+#include "test_usb_common.h"
+#include "test_usb_mock_classes.h"
+#include "msc_client.h"
+#include "ctrl_client.h"
+#include "usb/usb_host.h"
+#include "unity.h"
+#include "test_utils.h"
+
+// --------------------------------------------------- Test Cases ------------------------------------------------------
+
+//Safe approximation of time it takes to connect and enumerate the device
+#define TEST_FORCE_DCONN_DELAY_MS           400
+
+static void trigger_dconn_timer_cb(TimerHandle_t xTimer)
+{
+    printf("Forcing Sudden Disconnect\n");
+    test_usb_force_conn_state(false, 0);
+}
+
+TEST_CASE("Test USB Host sudden disconnection (no client)", "[usb_host][ignore]")
+{
+    //Install USB Host Library
+    usb_host_config_t host_config = {
+        .intr_flags = ESP_INTR_FLAG_LEVEL1,
+    };
+    ESP_ERROR_CHECK(usb_host_install(&host_config));
+    printf("Installed\n");
+
+    //Allocate timer to force disconnection after a short delay
+    TimerHandle_t timer_hdl = xTimerCreate("dconn",
+                                           pdMS_TO_TICKS(TEST_FORCE_DCONN_DELAY_MS),
+                                           pdFALSE,
+                                           NULL,
+                                           trigger_dconn_timer_cb);
+    TEST_ASSERT_NOT_EQUAL(NULL, timer_hdl);
+    TEST_ASSERT_EQUAL(pdPASS, xTimerStart(timer_hdl, portMAX_DELAY));
+
+    while (1) {
+        //Start handling system events
+        uint32_t event_flags;
+        usb_host_lib_handle_events(portMAX_DELAY, &event_flags);
+        if (event_flags & USB_HOST_LIB_EVENT_FLAGS_ALL_FREE) {
+            printf("All devices cleaned up\n");
+            break;
+        }
+    }
+
+    //Cleanup timer
+    TEST_ASSERT_EQUAL(pdPASS, xTimerDelete(timer_hdl, portMAX_DELAY));
+    //Clean up USB Host
+    ESP_ERROR_CHECK(usb_host_uninstall());
+}
+
+#define TEST_FORCE_DCONN_NUM_TRANSFERS      3
+#define TEST_MSC_SCSI_TAG                   0xDEADBEEF
+
+TEST_CASE("Test USB Host sudden disconnection (single client)", "[usb_host][ignore]")
+{
+    //Install USB Host
+    usb_host_config_t host_config = {
+        .intr_flags = ESP_INTR_FLAG_LEVEL1,
+    };
+    ESP_ERROR_CHECK(usb_host_install(&host_config));
+    printf("Installed\n");
+
+    //Create task to run client that communicates with MSC SCSI interface
+    msc_client_test_param_t params = {
+        .num_sectors_to_read = 1,   //Unused by disconnect MSC client
+        .num_sectors_per_xfer = TEST_FORCE_DCONN_NUM_TRANSFERS * MOCK_MSC_SCSI_SECTOR_SIZE,
+        .msc_scsi_xfer_tag = TEST_MSC_SCSI_TAG,
+        .idVendor = MOCK_MSC_SCSI_DEV_ID_VENDOR,
+        .idProduct = MOCK_MSC_SCSI_DEV_ID_PRODUCT,
+    };
+    TaskHandle_t task_hdl;
+    xTaskCreatePinnedToCore(msc_client_async_dconn_task, "async", 4096, (void *)&params, 2, &task_hdl, 0);
+    //Start the task
+    xTaskNotifyGive(task_hdl);
+
+    bool all_clients_gone = false;
+    bool all_dev_free = false;
+    while (!all_clients_gone || !all_dev_free) {
+        //Start handling system events
+        uint32_t event_flags;
+        usb_host_lib_handle_events(portMAX_DELAY, &event_flags);
+        if (event_flags & USB_HOST_LIB_EVENT_FLAGS_NO_CLIENTS) {
+            printf("No more clients\n");
+            all_clients_gone = true;
+        }
+        if (event_flags & USB_HOST_LIB_EVENT_FLAGS_ALL_FREE) {
+            printf("All device's freed\n");
+            all_dev_free = true;
+        }
+    }
+
+    //Short delay to allow task to be cleaned up
+    vTaskDelay(10);
+    //Clean up USB Host
+    ESP_ERROR_CHECK(usb_host_uninstall());
+}

+ 26 - 5
components/usb/usb_host.c

@@ -508,6 +508,16 @@ exit:
     return ret;
 }
 
+esp_err_t usb_host_lib_unblock(void)
+{
+    //All devices must have been freed at this point
+    HOST_ENTER_CRITICAL();
+    HOST_CHECK_FROM_CRIT(p_host_lib_obj != NULL, ESP_ERR_INVALID_STATE);
+    _unblock_lib(false);
+    HOST_EXIT_CRITICAL();
+    return ESP_OK;
+}
+
 // ------------------------------------------------ Client Functions ---------------------------------------------------
 
 // ----------------------- Private -------------------------
@@ -562,7 +572,11 @@ static void _handle_pending_ep(client_t *client_obj)
 esp_err_t usb_host_client_register(const usb_host_client_config_t *client_config, usb_host_client_handle_t *client_hdl_ret)
 {
     HOST_CHECK(client_config != NULL && client_hdl_ret != NULL, ESP_ERR_INVALID_ARG);
-    HOST_CHECK(client_config->client_event_callback != NULL && client_config->max_num_event_msg > 0, ESP_ERR_INVALID_ARG);
+    HOST_CHECK(client_config->max_num_event_msg > 0, ESP_ERR_INVALID_ARG);
+    if (!client_config->is_synchronous) {
+        //Asynchronous clients must provide a
+        HOST_CHECK(client_config->async.client_event_callback != NULL, ESP_ERR_INVALID_ARG);
+    }
 
     esp_err_t ret;
     //Create client object
@@ -579,8 +593,8 @@ esp_err_t usb_host_client_register(const usb_host_client_config_t *client_config
     TAILQ_INIT(&client_obj->mux_protected.interface_tailq);
     TAILQ_INIT(&client_obj->dynamic.done_ctrl_xfer_tailq);
     client_obj->constant.event_sem = event_sem;
-    client_obj->constant.event_callback = client_config->client_event_callback;
-    client_obj->constant.callback_arg = client_config->callback_arg;
+    client_obj->constant.event_callback = client_config->async.client_event_callback;
+    client_obj->constant.callback_arg = client_config->async.callback_arg;
     client_obj->constant.event_msg_queue = event_msg_queue;
 
     //Add client to the host library's list of clients
@@ -819,10 +833,16 @@ esp_err_t usb_host_device_free_all(void)
     HOST_EXIT_CRITICAL();
     esp_err_t ret;
     ret = usbh_dev_mark_all_free();
-    //Wait for USB_HOST_LIB_EVENT_FLAGS_ALL_FREE to confirm all devices free
+    //If ESP_ERR_NOT_FINISHED is returned, caller must wait for USB_HOST_LIB_EVENT_FLAGS_ALL_FREE to confirm all devices are free
     return ret;
 }
 
+esp_err_t usb_host_device_addr_list_fill(int list_len, uint8_t *dev_addr_list, int *num_dev_ret)
+{
+    HOST_CHECK(dev_addr_list != NULL && num_dev_ret != NULL, ESP_ERR_INVALID_ARG);
+    return usbh_dev_addr_list_fill(list_len, dev_addr_list, num_dev_ret);
+}
+
 // ------------------------------------------------- Device Requests ---------------------------------------------------
 
 // ------------------- Cached Requests ---------------------
@@ -1266,7 +1286,8 @@ esp_err_t usb_host_transfer_submit_control(usb_host_client_handle_t client_hdl,
     usb_device_info_t dev_info;
     ESP_ERROR_CHECK(usbh_dev_get_info(dev_hdl, &dev_info));
     HOST_CHECK(transfer_check(transfer, USB_TRANSFER_TYPE_CTRL, dev_info.bMaxPacketSize0, xfer_is_in), ESP_ERR_INVALID_ARG);
-    HOST_CHECK(transfer->bEndpointAddress == 0, ESP_ERR_INVALID_ARG);
+    //Control transfers must be targeted at EP 0
+    HOST_CHECK((transfer->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_NUM_MASK) == 0, ESP_ERR_INVALID_ARG);
     //Save client handle into URB
     urb_t *urb_obj = __containerof(transfer, urb_t, transfer);
     urb_obj->usb_host_client = (void *)client_hdl;

+ 215 - 72
components/usb/usbh.c

@@ -12,6 +12,7 @@
 #include "freertos/FreeRTOS.h"
 #include "freertos/portmacro.h"
 #include "freertos/task.h"
+#include "freertos/semphr.h"
 #include "esp_err.h"
 #include "esp_log.h"
 #include "esp_heap_caps.h"
@@ -20,14 +21,15 @@
 #include "usb/usb_helpers.h"
 #include "usb/usb_types_ch9.h"
 
-//Device action flags. Listed in the order they should handled in. Some actions are mutually exclusive
-#define DEV_FLAG_ACTION_SEND_GONE_EVENT             0x01    //Send a USB_HOST_CLIENT_EVENT_DEV_GONE event
+//Device action flags. LISTED IN THE ORDER THEY SHOULD BE HANDLED IN within usbh_process(). Some actions are mutually exclusive
+#define DEV_FLAG_ACTION_PIPE_HALT_AND_FLUSH         0x01    //Halt all non-default pipes then flush them (called after a device gone is gone)
 #define DEV_FLAG_ACTION_DEFAULT_PIPE_FLUSH          0x02    //Retire all URBS in the default pipe
 #define DEV_FLAG_ACTION_DEFAULT_PIPE_DEQUEUE        0x04    //Dequeue all URBs from default pipe
 #define DEV_FLAG_ACTION_DEFAULT_PIPE_CLEAR          0x08    //Move the default pipe to the active state
-#define DEV_FLAG_ACTION_FREE                        0x10    //Free the device object
-#define DEV_FLAG_ACTION_PORT_DISABLE                0x20
-#define DEV_FLAG_ACTION_SEND_NEW                    0x40    //Send a new device event
+#define DEV_FLAG_ACTION_SEND_GONE_EVENT             0x10    //Send a USB_HOST_CLIENT_EVENT_DEV_GONE event
+#define DEV_FLAG_ACTION_FREE                        0x20    //Free the device object
+#define DEV_FLAG_ACTION_PORT_DISABLE                0x40    //Request the hub driver to disable the port of the device
+#define DEV_FLAG_ACTION_SEND_NEW                    0x80    //Send a new device event
 
 #define DEV_ENUM_TODO_FLAG_DEV_ADDR                 0x01
 #define DEV_ENUM_TODO_FLAG_DEV_DESC                 0x02
@@ -56,10 +58,13 @@ struct device_s {
         int num_ctrl_xfers_inflight;
         usb_device_state_t state;
         uint32_t ref_count;
+    } dynamic;
+    //Mux protected members must be protected by the USBH mux_lock when accessed
+    struct {
         usb_config_desc_t *config_desc;
         hcd_pipe_handle_t ep_in[EP_NUM_MAX - 1];    //IN EP owner contexts. -1 to exclude the default endpoint
         hcd_pipe_handle_t ep_out[EP_NUM_MAX - 1];   //OUT EP owner contexts. -1 to exclude the default endpoint
-    } dynamic;
+    } mux_protected;
     //Constant members do no change after device allocation and enumeration thus do not require a critical section
     struct {
         hcd_pipe_handle_t default_pipe;
@@ -76,8 +81,11 @@ typedef struct {
     struct {
         TAILQ_HEAD(tailhead_devs, device_s) devs_idle_tailq;        //Tailq of all enum and configured devices
         TAILQ_HEAD(tailhead_devs_cb, device_s) devs_pending_tailq;  //Tailq of devices that need to have their cb called
-        uint8_t num_device;     //Number of enumerated devices
     } dynamic;
+    //Mux protected members must be protected by the USBH mux_lock when accessed
+    struct {
+        uint8_t num_device;     //Number of enumerated devices
+    } mux_protected;
     //Constant members do no change after installation thus do not require a critical section
     struct {
         usb_notif_cb_t notif_cb;
@@ -88,6 +96,7 @@ typedef struct {
         void *event_cb_arg;
         usbh_ctrl_xfer_cb_t ctrl_xfer_cb;
         void *ctrl_xfer_cb_arg;
+        SemaphoreHandle_t mux_lock;
     } constant;
 } usbh_t;
 
@@ -165,7 +174,7 @@ static void device_free(device_t *dev_obj)
         return;
     }
     //Configuration must be freed
-    assert(dev_obj->dynamic.config_desc == NULL);
+    assert(dev_obj->mux_protected.config_desc == NULL); //Sanity check. No need for mux
     ESP_ERROR_CHECK(hcd_pipe_free(dev_obj->constant.default_pipe));
     heap_caps_free((usb_device_desc_t *)dev_obj->constant.desc);
     heap_caps_free(dev_obj);
@@ -240,8 +249,28 @@ static bool default_pipe_callback(hcd_pipe_handle_t pipe_hdl, hcd_pipe_event_t p
     return yield;
 }
 
+static void handle_pipe_halt_and_flush(device_t *dev_obj)
+{
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    //Halt then flush all non-default IN pipes
+    for (int i = 0; i < (EP_NUM_MAX - 1); i++) {
+        if (dev_obj->mux_protected.ep_in[i] != NULL) {
+            ESP_ERROR_CHECK(hcd_pipe_command(dev_obj->mux_protected.ep_in[i], HCD_PIPE_CMD_HALT));
+            ESP_ERROR_CHECK(hcd_pipe_command(dev_obj->mux_protected.ep_in[i], HCD_PIPE_CMD_FLUSH));
+        }
+        if (dev_obj->mux_protected.ep_out[i] != NULL) {
+            ESP_ERROR_CHECK(hcd_pipe_command(dev_obj->mux_protected.ep_out[i], HCD_PIPE_CMD_HALT));
+            ESP_ERROR_CHECK(hcd_pipe_command(dev_obj->mux_protected.ep_out[i], HCD_PIPE_CMD_FLUSH));
+        }
+    }
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
+}
+
 static bool handle_dev_free(device_t *dev_obj)
 {
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
     USBH_ENTER_CRITICAL();
     //Remove the device object for it's containing list
     if (dev_obj->dynamic.flags.in_pending_list) {
@@ -250,13 +279,13 @@ static bool handle_dev_free(device_t *dev_obj)
     } else {
         TAILQ_REMOVE(&p_usbh_obj->dynamic.devs_idle_tailq, dev_obj, dynamic.tailq_entry);
     }
-    p_usbh_obj->dynamic.num_device--;
-    bool all_free = (p_usbh_obj->dynamic.num_device == 0);
     USBH_EXIT_CRITICAL();
-
-    heap_caps_free(dev_obj->dynamic.config_desc);
-    dev_obj->dynamic.config_desc = NULL;
+    p_usbh_obj->mux_protected.num_device--;
+    bool all_free = (p_usbh_obj->mux_protected.num_device == 0);
+    heap_caps_free(dev_obj->mux_protected.config_desc);
+    dev_obj->mux_protected.config_desc = NULL;
     device_free(dev_obj);
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
     return all_free;
 }
 
@@ -269,11 +298,13 @@ esp_err_t usbh_install(const usbh_config_t *usbh_config)
     USBH_CHECK_FROM_CRIT(p_usbh_obj == NULL, ESP_ERR_INVALID_STATE);
     USBH_EXIT_CRITICAL();
 
+    esp_err_t ret;
     usbh_t *usbh_obj = heap_caps_calloc(1, sizeof(usbh_t), MALLOC_CAP_DEFAULT);
-    if (usbh_obj == NULL) {
-        return ESP_ERR_NO_MEM;
+    SemaphoreHandle_t mux_lock = xSemaphoreCreateMutex();
+    if (usbh_obj == NULL || mux_lock == NULL) {
+        ret = ESP_ERR_NO_MEM;
+        goto alloc_err;
     }
-    esp_err_t ret;
     //Install HCD
     ret = hcd_install(&usbh_config->hcd_config);
     if (ret != ESP_OK) {
@@ -288,6 +319,7 @@ esp_err_t usbh_install(const usbh_config_t *usbh_config)
     usbh_obj->constant.event_cb_arg = usbh_config->event_cb_arg;
     usbh_obj->constant.ctrl_xfer_cb = usbh_config->ctrl_xfer_cb;
     usbh_obj->constant.ctrl_xfer_cb_arg = usbh_config->ctrl_xfer_cb_arg;
+    usbh_obj->constant.mux_lock = mux_lock;
 
     //Assign usbh object pointer
     USBH_ENTER_CRITICAL();
@@ -305,24 +337,47 @@ esp_err_t usbh_install(const usbh_config_t *usbh_config)
 assign_err:
     ESP_ERROR_CHECK(hcd_uninstall());
 hcd_install_err:
+alloc_err:
+    if (mux_lock != NULL) {
+        vSemaphoreDelete(mux_lock);
+    }
     heap_caps_free(usbh_obj);
     return ret;
 }
 
 esp_err_t usbh_uninstall(void)
 {
+    //Check that USBH is in a state to be uninstalled
     USBH_ENTER_CRITICAL();
     USBH_CHECK_FROM_CRIT(p_usbh_obj != NULL, ESP_ERR_INVALID_STATE);
-    //Check that USBH is in a state to be uninstalled
-    USBH_CHECK_FROM_CRIT(p_usbh_obj->dynamic.num_device == 0, ESP_ERR_INVALID_STATE);
     usbh_t *usbh_obj = p_usbh_obj;
+    USBH_EXIT_CRITICAL();
+
+    esp_err_t ret;
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(usbh_obj->constant.mux_lock, portMAX_DELAY);
+    if (p_usbh_obj->mux_protected.num_device > 0) {
+        //There are still devices allocated. Can't uninstall right now.
+        ret = ESP_ERR_INVALID_STATE;
+        goto exit;
+    }
+    //Check again if we can uninstall
+    USBH_ENTER_CRITICAL();
+    assert(p_usbh_obj == usbh_obj);
     p_usbh_obj = NULL;
     USBH_EXIT_CRITICAL();
+    xSemaphoreGive(usbh_obj->constant.mux_lock);
 
-    //Uninstall HCD
+    //Uninstall HCD, free resources
     ESP_ERROR_CHECK(hcd_uninstall());
+    vSemaphoreDelete(usbh_obj->constant.mux_lock);
     heap_caps_free(usbh_obj);
-    return ESP_OK;
+    ret = ESP_OK;
+    return ret;
+
+exit:
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
+    return ret;
 }
 
 esp_err_t usbh_process(void)
@@ -340,14 +395,16 @@ esp_err_t usbh_process(void)
         dev_obj->dynamic.flags.actions = 0;
         dev_obj->dynamic.flags.in_pending_list = 0;
 
+        /* ---------------------------------------------------------------------
+        Exit critical section to handle device action flags in their listed order
+        --------------------------------------------------------------------- */
         USBH_EXIT_CRITICAL();
         ESP_LOGD(USBH_TAG, "Processing actions 0x%x", action_flags);
         //Sanity check. If the device is being freed, there must not be any other action flags set
         assert(!(action_flags & DEV_FLAG_ACTION_FREE) || action_flags == DEV_FLAG_ACTION_FREE);
-        if (action_flags & DEV_FLAG_ACTION_SEND_GONE_EVENT) {
-            //Flush the default pipe. Then do an event gone
-            ESP_LOGE(USBH_TAG, "Device %d gone", dev_obj->constant.address);
-            p_usbh_obj->constant.event_cb((usb_device_handle_t)dev_obj, USBH_EVENT_DEV_GONE, p_usbh_obj->constant.event_cb_arg);
+
+        if (action_flags & DEV_FLAG_ACTION_PIPE_HALT_AND_FLUSH) {
+            handle_pipe_halt_and_flush(dev_obj);
         }
         if (action_flags & DEV_FLAG_ACTION_DEFAULT_PIPE_FLUSH) {
             ESP_ERROR_CHECK(hcd_pipe_command(dev_obj->constant.default_pipe, HCD_PIPE_CMD_HALT));
@@ -371,6 +428,11 @@ esp_err_t usbh_process(void)
             //We allow the pipe command to fail just in case the pipe becomes invalid mid command
             hcd_pipe_command(dev_obj->constant.default_pipe, HCD_PIPE_CMD_CLEAR);
         }
+        if (action_flags & DEV_FLAG_ACTION_SEND_GONE_EVENT) {
+            //Flush the default pipe. Then do an event gone
+            ESP_LOGE(USBH_TAG, "Device %d gone", dev_obj->constant.address);
+            p_usbh_obj->constant.event_cb((usb_device_handle_t)dev_obj, USBH_EVENT_DEV_GONE, p_usbh_obj->constant.event_cb_arg);
+        }
         /*
         Note: We make these action flags mutually exclusive in case they happen in rapid succession. They are handled
         in the order of precedence
@@ -393,6 +455,9 @@ esp_err_t usbh_process(void)
             p_usbh_obj->constant.event_cb((usb_device_handle_t)dev_obj, USBH_EVENT_DEV_NEW, p_usbh_obj->constant.event_cb_arg);
         }
         USBH_ENTER_CRITICAL();
+        /* ---------------------------------------------------------------------
+        Re-enter critical sections. All device action flags should have been handled.
+        --------------------------------------------------------------------- */
 
     }
     USBH_EXIT_CRITICAL();
@@ -403,6 +468,36 @@ esp_err_t usbh_process(void)
 
 // --------------------- Device Pool -----------------------
 
+esp_err_t usbh_dev_addr_list_fill(int list_len, uint8_t *dev_addr_list, int *num_dev_ret)
+{
+    USBH_CHECK(dev_addr_list != NULL && num_dev_ret != NULL, ESP_ERR_INVALID_ARG);
+    USBH_ENTER_CRITICAL();
+    int num_filled = 0;
+    device_t *dev_obj;
+    //Fill list with devices from idle tailq
+    TAILQ_FOREACH(dev_obj, &p_usbh_obj->dynamic.devs_idle_tailq, dynamic.tailq_entry) {
+        if (num_filled < list_len) {
+            dev_addr_list[num_filled] = dev_obj->constant.address;
+            num_filled++;
+        } else {
+            break;
+        }
+    }
+    //Fill list with devices from pending tailq
+    TAILQ_FOREACH(dev_obj, &p_usbh_obj->dynamic.devs_pending_tailq, dynamic.tailq_entry) {
+        if (num_filled < list_len) {
+            dev_addr_list[num_filled] = dev_obj->constant.address;
+            num_filled++;
+        } else {
+            break;
+        }
+    }
+    USBH_EXIT_CRITICAL();
+    //Write back number of devices filled
+    *num_dev_ret = num_filled;
+    return ESP_OK;
+}
+
 esp_err_t usbh_dev_open(uint8_t dev_addr, usb_device_handle_t *dev_hdl)
 {
     USBH_CHECK(dev_hdl != NULL, ESP_ERR_INVALID_ARG);
@@ -482,6 +577,7 @@ esp_err_t usbh_dev_mark_all_free(void)
     Note: We manually traverse the list because we need to add/remove items while traversing
     */
     bool call_notif_cb = false;
+    bool wait_for_free = false;
     for (int i = 0; i < 2; i++) {
         device_t *dev_obj_cur;
         device_t *dev_obj_next;
@@ -503,6 +599,7 @@ esp_err_t usbh_dev_mark_all_free(void)
                 //Device is still opened. Just mark it as waiting to be closed
                 dev_obj_cur->dynamic.flags.waiting_close = 1;
             }
+            wait_for_free = true;   //As long as there is still a device, we need to wait for an event indicating it is freed
             dev_obj_cur = dev_obj_next;
         }
     }
@@ -511,7 +608,7 @@ esp_err_t usbh_dev_mark_all_free(void)
     if (call_notif_cb) {
         p_usbh_obj->constant.notif_cb(USB_NOTIF_SOURCE_USBH, false, p_usbh_obj->constant.notif_cb_arg);
     }
-    return ESP_OK;
+    return (wait_for_free) ? ESP_ERR_NOT_FINISHED : ESP_OK;
 }
 
 // ------------------- Single Device  ----------------------
@@ -534,19 +631,30 @@ esp_err_t usbh_dev_get_info(usb_device_handle_t dev_hdl, usb_device_info_t *dev_
     USBH_CHECK(dev_hdl != NULL && dev_info != NULL, ESP_ERR_INVALID_ARG);
     device_t *dev_obj = (device_t *)dev_hdl;
 
+    esp_err_t ret;
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    //Device must be configured, or not attached (if it suddenly disconnected)
     USBH_ENTER_CRITICAL();
-    USBH_CHECK_FROM_CRIT(dev_obj->dynamic.state == USB_DEVICE_STATE_CONFIGURED || dev_obj->dynamic.state == USB_DEVICE_STATE_NOT_ATTACHED, ESP_ERR_INVALID_STATE);
+    if (!(dev_obj->dynamic.state == USB_DEVICE_STATE_CONFIGURED || dev_obj->dynamic.state == USB_DEVICE_STATE_NOT_ATTACHED)) {
+        USBH_EXIT_CRITICAL();
+        ret = ESP_ERR_INVALID_STATE;
+        goto exit;
+    }
+    //Critical section for the dynamic members
     dev_info->speed = dev_obj->constant.speed;
     dev_info->dev_addr = dev_obj->constant.address;
     dev_info->bMaxPacketSize0 = dev_obj->constant.desc->bMaxPacketSize0;
-    if (dev_obj->dynamic.config_desc == NULL) {
+    USBH_EXIT_CRITICAL();
+    if (dev_obj->mux_protected.config_desc == NULL) {
         dev_info->bConfigurationValue = 0;
     } else {
-        dev_info->bConfigurationValue = dev_obj->dynamic.config_desc->bConfigurationValue;
+        dev_info->bConfigurationValue = dev_obj->mux_protected.config_desc->bConfigurationValue;
     }
-    USBH_EXIT_CRITICAL();
-
-    return ESP_OK;
+    ret = ESP_OK;
+exit:
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
+    return ret;
 }
 
 esp_err_t usbh_dev_get_desc(usb_device_handle_t dev_hdl, const usb_device_desc_t **dev_desc_ret)
@@ -567,12 +675,22 @@ esp_err_t usbh_dev_get_config_desc(usb_device_handle_t dev_hdl, const usb_config
     USBH_CHECK(dev_hdl != NULL && config_desc_ret != NULL, ESP_ERR_INVALID_ARG);
     device_t *dev_obj = (device_t *)dev_hdl;
 
+    esp_err_t ret;
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    //Device must be in the configured state
     USBH_ENTER_CRITICAL();
-    USBH_CHECK_FROM_CRIT(dev_obj->dynamic.state == USB_DEVICE_STATE_CONFIGURED, ESP_ERR_INVALID_STATE);
-    *config_desc_ret = dev_obj->dynamic.config_desc;
+    if (dev_obj->dynamic.state != USB_DEVICE_STATE_CONFIGURED) {
+        USBH_EXIT_CRITICAL();
+        ret = ESP_ERR_INVALID_STATE;
+        goto exit;
+    }
     USBH_EXIT_CRITICAL();
-
-    return ESP_OK;
+    *config_desc_ret = dev_obj->mux_protected.config_desc;
+    ret = ESP_OK;
+exit:
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
+    return ret;
 }
 
 esp_err_t usbh_dev_submit_ctrl_urb(usb_device_handle_t dev_hdl, urb_t *urb)
@@ -633,21 +751,24 @@ esp_err_t usbh_ep_alloc(usb_device_handle_t dev_hdl, usbh_ep_config_t *ep_config
         goto pipe_alloc_err;
     }
 
-    USBH_ENTER_CRITICAL();
-    //Check that endpoint has not be allocated yet
     bool is_in = ep_config->ep_desc->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK;
     uint8_t addr = ep_config->ep_desc->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_NUM_MASK;
-    //Assign the pipe handle
     bool assigned = false;
-    if (is_in && dev_obj->dynamic.ep_in[addr - 1] == NULL) {    //Is an IN EP
-        dev_obj->dynamic.ep_in[addr - 1] = pipe_hdl;
+
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    if (is_in && dev_obj->mux_protected.ep_in[addr - 1] == NULL) {    //Is an IN EP
+        dev_obj->mux_protected.ep_in[addr - 1] = pipe_hdl;
         assigned = true;
     } else {
-        dev_obj->dynamic.ep_out[addr - 1] = pipe_hdl;
+        dev_obj->mux_protected.ep_out[addr - 1] = pipe_hdl;
         assigned = true;
     }
-    dev_obj->dynamic.ref_count--;   //Restore ref_count
+    //Restore ref_count
+    USBH_ENTER_CRITICAL();
+    dev_obj->dynamic.ref_count--;
     USBH_EXIT_CRITICAL();
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
 
     if (!assigned) {
         ret = ESP_ERR_INVALID_STATE;
@@ -669,24 +790,38 @@ esp_err_t usbh_ep_free(usb_device_handle_t dev_hdl, uint8_t bEndpointAddress)
     USBH_CHECK(dev_hdl != NULL, ESP_ERR_INVALID_ARG);
     device_t *dev_obj = (device_t *)dev_hdl;
 
-    USBH_ENTER_CRITICAL();
-    //Un-assign the pipe handle from the endpoint
+    esp_err_t ret;
     bool is_in = bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK;
     uint8_t addr = bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_NUM_MASK;
     hcd_pipe_handle_t pipe_hdl;
+
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    //Check if the EP was previously allocated. If so, set it to NULL
     if (is_in) {
-        USBH_CHECK_FROM_CRIT(dev_obj->dynamic.ep_in[addr - 1] != NULL, ESP_ERR_INVALID_STATE);
-        pipe_hdl = dev_obj->dynamic.ep_in[addr - 1];
-        dev_obj->dynamic.ep_in[addr - 1] = NULL;
+        if (dev_obj->mux_protected.ep_in[addr - 1] != NULL) {
+            pipe_hdl = dev_obj->mux_protected.ep_in[addr - 1];
+            dev_obj->mux_protected.ep_in[addr - 1] = NULL;
+            ret = ESP_OK;
+        } else {
+            ret = ESP_ERR_INVALID_STATE;
+        }
     } else {
-        USBH_CHECK_FROM_CRIT(dev_obj->dynamic.ep_out[addr - 1] != NULL, ESP_ERR_INVALID_STATE);
-        pipe_hdl = dev_obj->dynamic.ep_out[addr - 1];
-        dev_obj->dynamic.ep_out[addr - 1] = NULL;
+        //EP must have been previously allocated
+        if (dev_obj->mux_protected.ep_out[addr - 1] != NULL) {
+            pipe_hdl = dev_obj->mux_protected.ep_out[addr - 1];
+            dev_obj->mux_protected.ep_out[addr - 1] = NULL;
+            ret = ESP_OK;
+        } else {
+            ret = ESP_ERR_INVALID_STATE;
+        }
     }
-    USBH_EXIT_CRITICAL();
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
 
-    ESP_ERROR_CHECK(hcd_pipe_free(pipe_hdl));
-    return ESP_OK;
+    if (ret == ESP_OK) {
+        ESP_ERROR_CHECK(hcd_pipe_free(pipe_hdl));
+    }
+    return ret;
 }
 
 esp_err_t usbh_ep_get_context(usb_device_handle_t dev_hdl, uint8_t bEndpointAddress, void **context_ret)
@@ -698,29 +833,30 @@ esp_err_t usbh_ep_get_context(usb_device_handle_t dev_hdl, uint8_t bEndpointAddr
                addr <= EP_NUM_MAX &&
                context_ret != NULL,
                ESP_ERR_INVALID_ARG);
+
+    esp_err_t ret;
     device_t *dev_obj = (device_t *)dev_hdl;
+    hcd_pipe_handle_t pipe_hdl;
 
-    USBH_ENTER_CRITICAL();
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
     //Get the endpoint's corresponding pipe
-    hcd_pipe_handle_t pipe_hdl;
     if (is_in) {
-        pipe_hdl = dev_obj->dynamic.ep_in[addr - 1];
+        pipe_hdl = dev_obj->mux_protected.ep_in[addr - 1];
     } else {
-        pipe_hdl = dev_obj->dynamic.ep_out[addr - 1];
+        pipe_hdl = dev_obj->mux_protected.ep_out[addr - 1];
     }
-    esp_err_t ret;
+    //Check if the EP was allocated to begin with
     if (pipe_hdl == NULL) {
-        USBH_EXIT_CRITICAL();
         ret = ESP_ERR_NOT_FOUND;
         goto exit;
     }
     //Return the context of the pipe
     void *context = hcd_pipe_get_context(pipe_hdl);
     *context_ret = context;
-    USBH_EXIT_CRITICAL();
-
     ret = ESP_OK;
 exit:
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
     return ret;
 }
 
@@ -772,11 +908,13 @@ esp_err_t usbh_hub_mark_dev_gone(usb_device_handle_t dev_hdl)
     //Check if the device can be freed now
     if (dev_obj->dynamic.ref_count == 0) {
         dev_obj->dynamic.flags.waiting_free = 1;
+        //Device is already waiting free so none of it's pipes will be in use. Can free immediately.
         call_notif_cb = _dev_set_actions(dev_obj, DEV_FLAG_ACTION_FREE);
     } else {
-        call_notif_cb = _dev_set_actions(dev_obj, DEV_FLAG_ACTION_SEND_GONE_EVENT |
+        call_notif_cb = _dev_set_actions(dev_obj, DEV_FLAG_ACTION_PIPE_HALT_AND_FLUSH |
                                                   DEV_FLAG_ACTION_DEFAULT_PIPE_FLUSH |
-                                                  DEV_FLAG_ACTION_DEFAULT_PIPE_DEQUEUE);
+                                                  DEV_FLAG_ACTION_DEFAULT_PIPE_DEQUEUE |
+                                                  DEV_FLAG_ACTION_SEND_GONE_EVENT);
     }
     USBH_EXIT_CRITICAL();
 
@@ -852,10 +990,11 @@ esp_err_t usbh_hub_enum_fill_config_desc(usb_device_handle_t dev_hdl, const usb_
         goto assign_err;
     }
 
-    USBH_ENTER_CRITICAL();
-    assert(dev_obj->dynamic.config_desc == NULL);
-    dev_obj->dynamic.config_desc = config_desc;
-    USBH_EXIT_CRITICAL();
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    assert(dev_obj->mux_protected.config_desc == NULL);
+    dev_obj->mux_protected.config_desc = config_desc;
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
 
     //We can modify the info members outside the critical section
     dev_obj->constant.enum_todo_flags &= ~DEV_ENUM_TODO_FLAG_CONFIG_DESC;
@@ -874,13 +1013,16 @@ esp_err_t usbh_hub_enum_done(usb_device_handle_t dev_hdl)
     device_t *dev_obj = (device_t *)dev_hdl;
     USBH_CHECK(dev_obj->constant.enum_todo_flags == 0, ESP_ERR_INVALID_STATE);  //All enumeration stages to be done
 
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
     USBH_ENTER_CRITICAL();
     dev_obj->dynamic.state = USB_DEVICE_STATE_CONFIGURED;
     //Add the device to list of devices, then trigger a device event
     TAILQ_INSERT_TAIL(&p_usbh_obj->dynamic.devs_idle_tailq, dev_obj, dynamic.tailq_entry);   //Add it to the idle device list first
-    p_usbh_obj->dynamic.num_device++;
     bool call_notif_cb = _dev_set_actions(dev_obj, DEV_FLAG_ACTION_SEND_NEW);
     USBH_EXIT_CRITICAL();
+    p_usbh_obj->mux_protected.num_device++;
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
 
     //Update the default pipe callback
     ESP_ERROR_CHECK(hcd_pipe_update_callback(dev_obj->constant.default_pipe, default_pipe_callback, (void *)dev_obj));
@@ -896,10 +1038,11 @@ esp_err_t usbh_hub_enum_failed(usb_device_handle_t dev_hdl)
     USBH_CHECK(dev_hdl != NULL, ESP_ERR_INVALID_ARG);
     device_t *dev_obj = (device_t *)dev_hdl;
 
-    USBH_ENTER_CRITICAL();
-    usb_config_desc_t *config_desc = dev_obj->dynamic.config_desc;
-    dev_obj->dynamic.config_desc = NULL;
-    USBH_EXIT_CRITICAL();
+    //We need to take the mux_lock to access mux_protected members
+    xSemaphoreTake(p_usbh_obj->constant.mux_lock, portMAX_DELAY);
+    usb_config_desc_t *config_desc = dev_obj->mux_protected.config_desc;
+    dev_obj->mux_protected.config_desc = NULL;
+    xSemaphoreGive(p_usbh_obj->constant.mux_lock);
 
     if (config_desc) {
         heap_caps_free(config_desc);

+ 0 - 5
tools/ci/check_copyright_ignore.txt

@@ -1413,8 +1413,6 @@ components/hal/include/hal/uart_types.h
 components/hal/include/hal/uhci_types.h
 components/hal/include/hal/usb_hal.h
 components/hal/include/hal/usb_types_private.h
-components/hal/include/hal/usbh_hal.h
-components/hal/include/hal/usbh_ll.h
 components/hal/include/hal/wdt_hal.h
 components/hal/include/hal/wdt_types.h
 components/hal/interrupt_controller_hal.c
@@ -1450,7 +1448,6 @@ components/hal/twai_hal_iram.c
 components/hal/uart_hal.c
 components/hal/uart_hal_iram.c
 components/hal/usb_hal.c
-components/hal/usbh_hal.c
 components/hal/wdt_hal_iram.c
 components/heap/heap_caps_init.c
 components/heap/heap_private.h
@@ -2540,8 +2537,6 @@ components/unity/include/unity_fixture_extras.h
 components/unity/include/unity_test_runner.h
 components/unity/unity_port_esp32.c
 components/unity/unity_runner.c
-components/usb/test/common/test_usb_mock_classes.c
-components/usb/test/common/test_usb_mock_classes.h
 components/usb/test/hcd/test_hcd_ctrl.c
 components/vfs/include/esp_vfs_common.h
 components/vfs/include/esp_vfs_eventfd.h