alex/esp-idf
alex/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2024-10-05 20:47:46 -04:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

usb: Fix how the HCD handles sudden disconnection

Browse Source 
This commit fixes how the USB Host HCD handles sudden disconnections.

Bugs:
- HW channels remain active when the port suddenly disconnects, and
previously the channel would be disabled by setting the disabled bit,
then waiting for a disabled interrupt. However, ISOC channels do not
generate the disabled interrupt when the port is invalid, thus leading
to tasks getting indefinitely blocked in hcd_pipe_command().

Fix:
On a sudden disconnection, forcibly treat all channels as halted even
if their HCCHAR.ChEna bit is still set. We do a soft reset after a port
error anyways, so the channels will eventually be reset.

Closes https://github.com/espressif/esp-idf/issues/7505
This commit is contained in:
Darian Leung 2021-10-25 21:42:09 +08:00
parent 0159c37cf2
commit 0c758c8557
9 changed files with 240 additions and 155 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
components/hal/include/hal/usbh_hal.h
View File

@ -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 -----------------------------------------------
/**

30
components/hal/usbh_hal.c
View File

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

124
components/usb/hcd.c
View File

@ -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++) {

6
components/usb/include/usb/usb_types_stack.h
View File

@ -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()

6
components/usb/maintainers.md
View File

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

27
components/usb/test/hcd/test_hcd_common.c
View File

@ -9,13 +9,11 @@
#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"
@ -139,18 +137,23 @@ int test_hcd_get_num_pipe_events(hcd_pipe_handle_t pipe_hdl)
void test_hcd_force_conn_state(bool connected, TickType_t delay_ticks)
{
vTaskDelay(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) {
//Swap back to internal PHY that is connected to a device
wrap->otg_conf.phy_sel = 0;
//Disable test mode to return to previous internal PHY configuration
wrap->test_conf.test_enable = 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;
/*
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;
}
}

99
components/usb/test/hcd/test_hcd_isoc.c
View File

@ -17,6 +17,7 @@
#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 +79,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 +96,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_hcd_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);
}

14
components/usb/test/hcd/test_hcd_port.c
View File

@ -32,7 +32,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
@ -66,7 +66,7 @@ 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
//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 +75,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 +259,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);

2
tools/ci/check_copyright_ignore.txt
View File

@ -1414,7 +1414,6 @@ components/hal/include/hal/uart_types.h
components/hal/include/hal/uhci_types.h
components/hal/include/hal/usb_hal.h
components/hal/include/hal/usb_types_private.h
components/hal/include/hal/usbh_hal.h
components/hal/include/hal/wdt_hal.h
components/hal/include/hal/wdt_types.h
components/hal/interrupt_controller_hal.c
@ -1450,7 +1449,6 @@ components/hal/twai_hal_iram.c
components/hal/uart_hal.c
components/hal/uart_hal_iram.c
components/hal/usb_hal.c
components/hal/usbh_hal.c
components/hal/wdt_hal_iram.c
components/heap/heap_caps_init.c
components/heap/heap_private.h