diff --git a/components/hal/include/hal/usbh_hal.h b/components/hal/include/hal/usbh_hal.h index b315f6c0b1..6360f7b3f3 100644 --- a/components/hal/include/hal/usbh_hal.h +++ b/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); } @@ -687,6 +650,25 @@ 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 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 + */ +static inline void usbh_hal_chan_mark_halted(usbh_hal_chan_t *chan_obj) +{ + chan_obj->flags.active = 0; +} + /** * @brief Get a channel's error * @@ -695,22 +677,9 @@ bool usbh_hal_chan_request_halt(usbh_hal_chan_t *chan_obj); */ static inline usbh_hal_chan_error_t usbh_hal_chan_get_error(usbh_hal_chan_t *chan_obj) { - HAL_ASSERT(chan_obj->flags.error_pending); return chan_obj->error; } -/** - * @brief Clear a channel of it's error - * - * @param chan_obj Channel object - */ -static inline void usbh_hal_chan_clear_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; -} - // -------------------------------------------- Transfer Descriptor List ----------------------------------------------- /** diff --git a/components/hal/include/hal/usbh_ll.h b/components/hal/include/hal/usbh_ll.h index a6c03e302a..63f7b8219d 100644 --- a/components/hal/include/hal/usbh_ll.h +++ b/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 ------------------------------- diff --git a/components/hal/usbh_hal.c b/components/hal/usbh_hal.c index 5091952de8..a0e038b0e5 100644 --- a/components/hal/usbh_hal.c +++ b/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 #include @@ -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) { diff --git a/components/usb/hcd.c b/components/usb/hcd.c index 87be3a350a..d28b943e42 100644 --- a/components/usb/hcd.c +++ b/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++) { diff --git a/components/usb/include/usb/usb_host.h b/components/usb/include/usb/usb_host.h index 6c104af6cf..9a91e7ed18 100644 --- a/components/usb/include/usb/usb_host.h +++ b/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 diff --git a/components/usb/include/usb/usb_types_stack.h b/components/usb/include/usb/usb_types_stack.h index 438c22d9ef..974d01def5 100644 --- a/components/usb/include/usb/usb_types_stack.h +++ b/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() diff --git a/components/usb/maintainers.md b/components/usb/maintainers.md index 81e2d00dec..8a0da5d23f 100644 --- a/components/usb/maintainers.md +++ b/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) diff --git a/components/usb/private_include/usbh.h b/components/usb/private_include/usbh.h index 924646fc48..6d530f7f49 100644 --- a/components/usb/private_include/usbh.h +++ b/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 */ diff --git a/components/usb/test/common/test_usb_common.c b/components/usb/test/common/test_usb_common.c new file mode 100644 index 0000000000..61f997a98a --- /dev/null +++ b/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 +#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; + } +} diff --git a/components/usb/test/common/test_usb_common.h b/components/usb/test/common/test_usb_common.h new file mode 100644 index 0000000000..fc8a8379d3 --- /dev/null +++ b/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 +#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); diff --git a/components/usb/test/common/test_usb_mock_classes.c b/components/usb/test/common/test_usb_mock_classes.c index f3d7d42d8e..0e819281de 100644 --- a/components/usb/test/common/test_usb_mock_classes.c +++ b/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 #include @@ -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 diff --git a/components/usb/test/common/test_usb_mock_classes.h b/components/usb/test/common/test_usb_mock_classes.h index 6c80b076c1..ef39aff127 100644 --- a/components/usb/test/common/test_usb_mock_classes.h +++ b/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... diff --git a/components/usb/test/hcd/test_hcd_common.c b/components/usb/test/hcd/test_hcd_common.c index 93a22d8fd9..9116ffa72d 100644 --- a/components/usb/test/hcd/test_hcd_common.c +++ b/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)); diff --git a/components/usb/test/hcd/test_hcd_common.h b/components/usb/test/hcd/test_hcd_common.h index 493b95dc24..a41c9fe451 100644 --- a/components/usb/test/hcd/test_hcd_common.h +++ b/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 diff --git a/components/usb/test/hcd/test_hcd_isoc.c b/components/usb/test/hcd/test_hcd_isoc.c index cb9dbec971..7358ff979e 100644 --- a/components/usb/test/hcd/test_hcd_isoc.c +++ b/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); +} diff --git a/components/usb/test/hcd/test_hcd_port.c b/components/usb/test/hcd/test_hcd_port.c index e7267de5f1..d3e2678160 100644 --- a/components/usb/test/hcd/test_hcd_port.c +++ b/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 } diff --git a/components/usb/test/usb_host/ctrl_client_async_seq.c b/components/usb/test/usb_host/ctrl_client_async_seq.c index 85751adeb6..18cd4ca646 100644 --- a/components/usb/test/usb_host/ctrl_client_async_seq.c +++ b/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; diff --git a/components/usb/test/usb_host/msc_client.h b/components/usb/test/usb_host/msc_client.h index b1809b949c..ab2f0f4dd3 100644 --- a/components/usb/test/usb_host/msc_client.h +++ b/components/usb/test/usb_host/msc_client.h @@ -6,6 +6,8 @@ #include +#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); diff --git a/components/usb/test/usb_host/msc_client_async_dconn.c b/components/usb/test/usb_host/msc_client_async_dconn.c new file mode 100644 index 0000000000..bbbd9170c2 --- /dev/null +++ b/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 +#include +#include +#include +#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); +} diff --git a/components/usb/test/usb_host/msc_client_async_seq.c b/components/usb/test/usb_host/msc_client_async_seq.c index 7189f4cd61..d4be014b1c 100644 --- a/components/usb/test/usb_host/msc_client_async_seq.c +++ b/components/usb/test/usb_host/msc_client_async_seq.c @@ -6,12 +6,13 @@ #include #include -#include +#include +#include #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)); diff --git a/components/usb/test/usb_host/test_usb_host_async.c b/components/usb/test/usb_host/test_usb_host_async.c index 4c7f792e7c..aa3a394e96 100644 --- a/components/usb/test/usb_host/test_usb_host_async.c +++ b/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; diff --git a/components/usb/test/usb_host/test_usb_host_plugging.c b/components/usb/test/usb_host/test_usb_host_plugging.c new file mode 100644 index 0000000000..4136529abf --- /dev/null +++ b/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 +#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 *)¶ms, 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()); +} diff --git a/components/usb/usb_host.c b/components/usb/usb_host.c index 5759fd6a41..1aec4b7ee0 100644 --- a/components/usb/usb_host.c +++ b/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; diff --git a/components/usb/usbh.c b/components/usb/usbh.c index b8e8cfd784..ab72e97fb4 100644 --- a/components/usb/usbh.c +++ b/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(); - usbh_t *usbh_obj = heap_caps_calloc(1, sizeof(usbh_t), MALLOC_CAP_DEFAULT); - if (usbh_obj == NULL) { - return ESP_ERR_NO_MEM; - } esp_err_t ret; + usbh_t *usbh_obj = heap_caps_calloc(1, sizeof(usbh_t), MALLOC_CAP_DEFAULT); + SemaphoreHandle_t mux_lock = xSemaphoreCreateMutex(); + if (usbh_obj == NULL || mux_lock == NULL) { + ret = ESP_ERR_NO_MEM; + goto alloc_err; + } //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; - p_usbh_obj = NULL; USBH_EXIT_CRITICAL(); - //Uninstall HCD + 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, 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; - 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; - } 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; - } - USBH_EXIT_CRITICAL(); - ESP_ERROR_CHECK(hcd_pipe_free(pipe_hdl)); - return ESP_OK; + //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) { + 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 { + //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; + } + } + xSemaphoreGive(p_usbh_obj->constant.mux_lock); + + 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); - device_t *dev_obj = (device_t *)dev_hdl; - USBH_ENTER_CRITICAL(); - //Get the endpoint's corresponding pipe - hcd_pipe_handle_t pipe_hdl; - if (is_in) { - pipe_hdl = dev_obj->dynamic.ep_in[addr - 1]; - } else { - pipe_hdl = dev_obj->dynamic.ep_out[addr - 1]; - } esp_err_t ret; + device_t *dev_obj = (device_t *)dev_hdl; + 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); + //Get the endpoint's corresponding pipe + if (is_in) { + pipe_hdl = dev_obj->mux_protected.ep_in[addr - 1]; + } else { + pipe_hdl = dev_obj->mux_protected.ep_out[addr - 1]; + } + //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);