diff --git a/components/hal/include/hal/usb_dwc_hal.h b/components/hal/include/hal/usb_dwc_hal.h index 9b2a2c3bc3..33ac57be75 100644 --- a/components/hal/include/hal/usb_dwc_hal.h +++ b/components/hal/include/hal/usb_dwc_hal.h @@ -205,7 +205,7 @@ typedef struct { * - The peripheral must have been reset and clock un-gated * - The USB PHY (internal or external) and associated GPIOs must already be configured * - GPIO pins configured - * - Interrupt allocated but DISABLED (in case of an unknown interupt state) + * - Interrupt allocated but DISABLED (in case of an unknown interrupt state) * Exit: * - Checks to see if DWC_OTG is alive, and if HW version/config is correct * - HAl context initialized @@ -290,7 +290,7 @@ static inline void usb_dwc_hal_port_init(usb_dwc_hal_context_t *hal) /** * @brief Deinitialize the host port * - * - Will disable the host port's interrupts preventing further port aand channel events from ocurring + * - Will disable the host port's interrupts preventing further port aand channel events from occurring * * @param hal Context of the HAL layer */ @@ -333,7 +333,6 @@ static inline void usb_dwc_hal_port_toggle_power(usb_dwc_hal_context_t *hal, boo */ static inline void usb_dwc_hal_port_toggle_reset(usb_dwc_hal_context_t *hal, bool enable) { - HAL_ASSERT(hal->channels.num_allocd == 0); //Cannot reset if there are still allocated channels usb_dwc_ll_hprt_set_port_reset(hal->dev, enable); } @@ -447,7 +446,7 @@ static inline void usb_dwc_hal_port_periodic_enable(usb_dwc_hal_context_t *hal) /** * @brief Disable periodic scheduling * - * Disabling periodic scheduling will save a bit of DMA bandwith (as the controller will no longer fetch the schedule + * Disabling periodic scheduling will save a bit of DMA bandwidth (as the controller will no longer fetch the schedule * from the frame list). * * @note Before disabling periodic scheduling, it is the user's responsibility to ensure that all periodic channels have @@ -505,17 +504,17 @@ static inline usb_dwc_speed_t usb_dwc_hal_port_get_conn_speed(usb_dwc_hal_contex * @brief Disable the debounce lock * * This function must be called after calling usb_dwc_hal_port_check_if_connected() and will allow connection/disconnection - * events to occur again. Any pending connection or disconenction interrupts are cleared. + * events to occur again. Any pending connection or disconnection interrupts are cleared. * * @param hal Context of the HAL layer */ static inline void usb_dwc_hal_disable_debounce_lock(usb_dwc_hal_context_t *hal) { hal->flags.dbnc_lock_enabled = 0; - //Clear Conenction and disconenction interrupt in case it triggered again + //Clear Connection and disconnection interrupt in case it triggered again usb_dwc_ll_gintsts_clear_intrs(hal->dev, USB_DWC_LL_INTR_CORE_DISCONNINT); usb_dwc_ll_hprt_intr_clear(hal->dev, USB_DWC_LL_INTR_HPRT_PRTCONNDET); - //Reenable the hprt (connection) and disconnection interrupts + //Re-enable the hprt (connection) and disconnection interrupts usb_dwc_ll_gintmsk_en_intrs(hal->dev, USB_DWC_LL_INTR_CORE_PRTINT | USB_DWC_LL_INTR_CORE_DISCONNINT); } @@ -672,10 +671,10 @@ bool usb_dwc_hal_chan_request_halt(usb_dwc_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): + * When a port error occurs (e.g., disconnect, overcurrent): * - Any previously active channels will remain active (i.e., they will not receive a channel interrupt) * - Attempting to disable them using usb_dwc_hal_chan_request_halt() will NOT generate an interrupt for ISOC channels - * (probalby something to do with the periodic scheduling) + * (probably 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 diff --git a/components/usb/hcd_dwc.c b/components/usb/hcd_dwc.c index f9834b7ad8..bc0c24ddaf 100644 --- a/components/usb/hcd_dwc.c +++ b/components/usb/hcd_dwc.c @@ -214,9 +214,7 @@ struct pipe_obj { uint32_t waiting_halt: 1; uint32_t pipe_cmd_processing: 1; uint32_t has_urb: 1; // Indicates there is at least one URB either pending, in-flight, or done - uint32_t persist: 1; // indicates that this pipe should persist through a run-time port reset - uint32_t reset_lock: 1; // Indicates that this pipe is undergoing a run-time reset - uint32_t reserved27: 27; + uint32_t reserved29: 29; }; uint32_t val; } cs_flags; @@ -560,28 +558,6 @@ static esp_err_t _pipe_cmd_clear(pipe_t *pipe); // ------------------------ Port --------------------------- -/** - * @brief Prepare persistent pipes for reset - * - * This function checks if all pipes are reset persistent and proceeds to free their underlying HAL channels for the - * persistent pipes. This should be called before a run time reset - * - * @param port Port object - * @return true All pipes are persistent and their channels are freed - * @return false Not all pipes are persistent - */ -static bool _port_persist_all_pipes(port_t *port); - -/** - * @brief Recovers all persistent pipes after a reset - * - * This function will recover all persistent pipes after a reset and reallocate their underlying HAl channels. This - * function should be called after a reset. - * - * @param port Port object - */ -static void _port_recover_all_pipes(port_t *port); - /** * @brief Checks if all pipes are in the halted state * @@ -1162,44 +1138,6 @@ esp_err_t hcd_uninstall(void) // ----------------------- Helpers ------------------------- -static bool _port_persist_all_pipes(port_t *port) -{ - if (port->num_pipes_queued > 0) { - // All pipes must be idle before we run-time reset - return false; - } - bool all_persist = true; - pipe_t *pipe; - // Check that each pipe is persistent - TAILQ_FOREACH(pipe, &port->pipes_idle_tailq, tailq_entry) { - if (!pipe->cs_flags.persist) { - all_persist = false; - break; - } - } - if (!all_persist) { - // At least one pipe is not persistent. All pipes must be freed or made persistent before we can reset - return false; - } - TAILQ_FOREACH(pipe, &port->pipes_idle_tailq, tailq_entry) { - pipe->cs_flags.reset_lock = 1; - usb_dwc_hal_chan_free(port->hal, pipe->chan_obj); - } - return true; -} - -static void _port_recover_all_pipes(port_t *port) -{ - pipe_t *pipe; - TAILQ_FOREACH(pipe, &port->pipes_idle_tailq, tailq_entry) { - pipe->cs_flags.persist = 0; - pipe->cs_flags.reset_lock = 0; - usb_dwc_hal_chan_alloc(port->hal, pipe->chan_obj, (void *)pipe); - usb_dwc_hal_chan_set_ep_char(port->hal, pipe->chan_obj, &pipe->ep_char); - } - CACHE_SYNC_FRAME_LIST(port->frame_list); -} - static bool _port_check_all_pipes_halted(port_t *port) { bool all_halted = true; @@ -1276,20 +1214,26 @@ static esp_err_t _port_cmd_power_off(port_t *port) static esp_err_t _port_cmd_reset(port_t *port) { esp_err_t ret; - // Port can only a reset when it is in the enabled or disabled states (in case of new connection) + + // Port can only a reset when it is in the enabled or disabled (in the case of a new connection)states. if (port->state != HCD_PORT_STATE_ENABLED && port->state != HCD_PORT_STATE_DISABLED) { ret = ESP_ERR_INVALID_STATE; goto exit; } - bool is_runtime_reset = (port->state == HCD_PORT_STATE_ENABLED) ? true : false; - if (is_runtime_reset && !_port_persist_all_pipes(port)) { - // If this is a run time reset, check all pipes that are still allocated can persist the reset + // Port can only be reset if all pipes are idle + if (port->num_pipes_queued > 0) { ret = ESP_ERR_INVALID_STATE; goto exit; } - // All pipes (if any_) are guaranteed to be persistent at this point. Proceed to resetting the bus + /* + Proceed to resetting the bus + - Update the port's state variable + - Hold the bus in the reset state for RESET_HOLD_MS. + - Return the bus to the idle state for RESET_RECOVERY_MS + */ port->state = HCD_PORT_STATE_RESETTING; - // Put and hold the bus in the reset state. If the port was previously enabled, a disabled event will occur after this + + // Place the bus into the reset state. If the port was previously enabled, a disabled event will occur after this usb_dwc_hal_port_toggle_reset(port->hal, true); HCD_EXIT_CRITICAL(); vTaskDelay(pdMS_TO_TICKS(RESET_HOLD_MS)); @@ -1299,7 +1243,8 @@ static esp_err_t _port_cmd_reset(port_t *port) ret = ESP_ERR_INVALID_RESPONSE; goto bailout; } - // Return the bus to the idle state and hold it for the required reset recovery time. Port enabled event should occur + + // Return the bus to the idle state. Port enabled event should occur usb_dwc_hal_port_toggle_reset(port->hal, false); HCD_EXIT_CRITICAL(); vTaskDelay(pdMS_TO_TICKS(RESET_RECOVERY_MS)); @@ -1309,16 +1254,20 @@ static esp_err_t _port_cmd_reset(port_t *port) ret = ESP_ERR_INVALID_RESPONSE; goto bailout; } - // Set FIFO sizes based on the selected biasing - usb_dwc_hal_set_fifo_bias(port->hal, port->fifo_bias); - // We start periodic scheduling only after a RESET command since SOFs only start after a reset - usb_dwc_hal_port_set_frame_list(port->hal, port->frame_list, FRAME_LIST_LEN); - usb_dwc_hal_port_periodic_enable(port->hal); + + // Reinitialize port registers. + usb_dwc_hal_set_fifo_bias(port->hal, port->fifo_bias); // Set FIFO biases + usb_dwc_hal_port_set_frame_list(port->hal, port->frame_list, FRAME_LIST_LEN); // Set periodic frame list + usb_dwc_hal_port_periodic_enable(port->hal); // Enable periodic scheduling + ret = ESP_OK; bailout: - if (is_runtime_reset) { - _port_recover_all_pipes(port); + // Reinitialize channel registers + pipe_t *pipe; + TAILQ_FOREACH(pipe, &port->pipes_idle_tailq, tailq_entry) { + usb_dwc_hal_chan_set_ep_char(port->hal, pipe->chan_obj, &pipe->ep_char); } + CACHE_SYNC_FRAME_LIST(port->frame_list); exit: return ret; } @@ -1987,8 +1936,7 @@ esp_err_t hcd_pipe_free(hcd_pipe_handle_t pipe_hdl) HCD_ENTER_CRITICAL(); // Check that all URBs have been removed and pipe has no pending events HCD_CHECK_FROM_CRIT(!pipe->multi_buffer_control.buffer_is_executing - && !pipe->cs_flags.has_urb - && !pipe->cs_flags.reset_lock, + && !pipe->cs_flags.has_urb, ESP_ERR_INVALID_STATE); // Remove pipe from the list of idle pipes (it must be in the idle list because it should have no queued URBs) TAILQ_REMOVE(&pipe->port->pipes_idle_tailq, pipe, tailq_entry); @@ -2011,8 +1959,7 @@ esp_err_t hcd_pipe_update_mps(hcd_pipe_handle_t pipe_hdl, int mps) HCD_ENTER_CRITICAL(); // Check if pipe is in the correct state to be updated HCD_CHECK_FROM_CRIT(!pipe->cs_flags.pipe_cmd_processing && - !pipe->cs_flags.has_urb && - !pipe->cs_flags.reset_lock, + !pipe->cs_flags.has_urb, ESP_ERR_INVALID_STATE); pipe->ep_char.mps = mps; // Update the underlying channel's registers @@ -2027,8 +1974,7 @@ esp_err_t hcd_pipe_update_dev_addr(hcd_pipe_handle_t pipe_hdl, uint8_t dev_addr) HCD_ENTER_CRITICAL(); // Check if pipe is in the correct state to be updated HCD_CHECK_FROM_CRIT(!pipe->cs_flags.pipe_cmd_processing && - !pipe->cs_flags.has_urb && - !pipe->cs_flags.reset_lock, + !pipe->cs_flags.has_urb, ESP_ERR_INVALID_STATE); pipe->ep_char.dev_addr = dev_addr; // Update the underlying channel's registers @@ -2043,8 +1989,7 @@ esp_err_t hcd_pipe_update_callback(hcd_pipe_handle_t pipe_hdl, hcd_pipe_callback HCD_ENTER_CRITICAL(); // Check if pipe is in the correct state to be updated HCD_CHECK_FROM_CRIT(!pipe->cs_flags.pipe_cmd_processing && - !pipe->cs_flags.has_urb && - !pipe->cs_flags.reset_lock, + !pipe->cs_flags.has_urb, ESP_ERR_INVALID_STATE); pipe->callback = callback; pipe->callback_arg = user_arg; @@ -2052,20 +1997,6 @@ esp_err_t hcd_pipe_update_callback(hcd_pipe_handle_t pipe_hdl, hcd_pipe_callback return ESP_OK; } -esp_err_t hcd_pipe_set_persist_reset(hcd_pipe_handle_t pipe_hdl) -{ - pipe_t *pipe = (pipe_t *)pipe_hdl; - HCD_ENTER_CRITICAL(); - // Check if pipe is in the correct state to be updated - HCD_CHECK_FROM_CRIT(!pipe->cs_flags.pipe_cmd_processing && - !pipe->cs_flags.has_urb && - !pipe->cs_flags.reset_lock, - ESP_ERR_INVALID_STATE); - pipe->cs_flags.persist = 1; - HCD_EXIT_CRITICAL(); - return ESP_OK; -} - void *hcd_pipe_get_context(hcd_pipe_handle_t pipe_hdl) { pipe_t *pipe = (pipe_t *)pipe_hdl; @@ -2102,27 +2033,22 @@ esp_err_t hcd_pipe_command(hcd_pipe_handle_t pipe_hdl, hcd_pipe_cmd_t command) esp_err_t ret = ESP_OK; 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) { - ret = ESP_ERR_INVALID_STATE; - } else { - pipe->cs_flags.pipe_cmd_processing = 1; - switch (command) { - case HCD_PIPE_CMD_HALT: { - ret = _pipe_cmd_halt(pipe); - break; - } - case HCD_PIPE_CMD_FLUSH: { - ret = _pipe_cmd_flush(pipe); - break; - } - case HCD_PIPE_CMD_CLEAR: { - ret = _pipe_cmd_clear(pipe); - break; - } - } - pipe->cs_flags.pipe_cmd_processing = 0; + pipe->cs_flags.pipe_cmd_processing = 1; + switch (command) { + case HCD_PIPE_CMD_HALT: { + ret = _pipe_cmd_halt(pipe); + break; } + case HCD_PIPE_CMD_FLUSH: { + ret = _pipe_cmd_flush(pipe); + break; + } + case HCD_PIPE_CMD_CLEAR: { + ret = _pipe_cmd_clear(pipe); + break; + } + } + pipe->cs_flags.pipe_cmd_processing = 0; HCD_EXIT_CRITICAL(); return ret; } @@ -2658,8 +2584,7 @@ esp_err_t hcd_urb_enqueue(hcd_pipe_handle_t pipe_hdl, urb_t *urb) // Check that pipe and port are in the correct state to receive URBs HCD_CHECK_FROM_CRIT(pipe->port->state == HCD_PORT_STATE_ENABLED // The pipe's port must be in the correct state && pipe->state == HCD_PIPE_STATE_ACTIVE // The pipe must be in the correct state - && !pipe->cs_flags.pipe_cmd_processing // Pipe cannot currently be processing a pipe command - && !pipe->cs_flags.reset_lock, // Pipe cannot be persisting through a port reset + && !pipe->cs_flags.pipe_cmd_processing, // Pipe cannot currently be processing a pipe command ESP_ERR_INVALID_STATE); // Use the URB's reserved_ptr to store the pipe's urb->hcd_ptr = (void *)pipe; diff --git a/components/usb/hub.c b/components/usb/hub.c index 52aa84e00a..6709bea64a 100644 --- a/components/usb/hub.c +++ b/components/usb/hub.c @@ -285,7 +285,6 @@ static bool enum_stage_start(enum_ctrl_t *enum_ctrl) static bool enum_stage_second_reset(enum_ctrl_t *enum_ctrl) { - ESP_ERROR_CHECK(hcd_pipe_set_persist_reset(enum_ctrl->pipe)); // Persist the default pipe through the reset if (hcd_port_command(p_hub_driver_obj->constant.root_port_hdl, HCD_PORT_CMD_RESET) != ESP_OK) { ESP_LOGE(HUB_DRIVER_TAG, "Failed to issue second reset"); return false; diff --git a/components/usb/private_include/hcd.h b/components/usb/private_include/hcd.h index f0c4a364b9..24ec22b085 100644 --- a/components/usb/private_include/hcd.h +++ b/components/usb/private_include/hcd.h @@ -434,20 +434,6 @@ esp_err_t hcd_pipe_update_dev_addr(hcd_pipe_handle_t pipe_hdl, uint8_t dev_addr) */ esp_err_t hcd_pipe_update_callback(hcd_pipe_handle_t pipe_hdl, hcd_pipe_callback_t callback, void *user_arg); -/** - * @brief Make a pipe persist through a run time reset - * - * Normally when a HCD_PORT_CMD_RESET is called, all pipes should already have been freed. However There may be cases - * (such as during enumeration) when a pipe must persist through a reset. This function will mark a pipe as - * persistent allowing it to survive a reset. When HCD_PORT_CMD_RESET is called, the pipe can continue to be used after - * the reset. - * - * @param pipe_hdl Pipe handle - * @retval ESP_OK: Pipe successfully marked as persistent - * @retval ESP_ERR_INVALID_STATE: Pipe is not in a condition to be made persistent - */ -esp_err_t hcd_pipe_set_persist_reset(hcd_pipe_handle_t pipe_hdl); - /** * @brief Get the context variable of a pipe from its handle *