From ec2ba71f9788d012ff3f39d038bc4f98bfebfa12 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Tue, 14 Nov 2023 18:45:27 +0800 Subject: [PATCH 1/4] refactor(soc): SOC_USB_PERIPH_NUM option This commit refactors SOC_USB_PERIPH_NUM as follows: - Renamed to SOC_USB_OTG_PERIPH_NUM to avoid confusion with USB Serial JTAG - Updated to unsigned integer "1U" - Updated some build rules to depend on SOC_USB_OTG_SUPPORTED instead --- components/soc/esp32s2/include/soc/Kconfig.soc_caps.in | 6 +++--- components/soc/esp32s2/include/soc/soc_caps.h | 3 +-- components/soc/esp32s3/include/soc/Kconfig.soc_caps.in | 6 +++--- components/soc/esp32s3/include/soc/soc_caps.h | 2 +- examples/peripherals/.build-test-rules.yml | 2 +- examples/system/.build-test-rules.yml | 5 ++++- 6 files changed, 13 insertions(+), 11 deletions(-) diff --git a/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in b/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in index dadaacf3b2..f075ea1f6c 100644 --- a/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in @@ -719,9 +719,9 @@ config SOC_SPIRAM_SUPPORTED bool default y -config SOC_USB_PERIPH_NUM - bool - default y +config SOC_USB_OTG_PERIPH_NUM + int + default 1 config SOC_SHA_DMA_MAX_BUFFER_SIZE int diff --git a/components/soc/esp32s2/include/soc/soc_caps.h b/components/soc/esp32s2/include/soc/soc_caps.h index 6e741f1648..0d2c0b6e2a 100644 --- a/components/soc/esp32s2/include/soc/soc_caps.h +++ b/components/soc/esp32s2/include/soc/soc_caps.h @@ -320,8 +320,7 @@ #define SOC_SPIRAM_SUPPORTED 1 /*-------------------------- USB CAPS ----------------------------------------*/ -#define SOC_USB_PERIPH_NUM 1 - +#define SOC_USB_OTG_PERIPH_NUM (1U) /*--------------------------- SHA CAPS ---------------------------------------*/ /* Max amount of bytes in a single DMA operation is 4095, diff --git a/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in b/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in index ae61e74b38..5725c27131 100644 --- a/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in @@ -815,9 +815,9 @@ config SOC_UART_REQUIRE_CORE_RESET bool default y -config SOC_USB_PERIPH_NUM - bool - default y +config SOC_USB_OTG_PERIPH_NUM + int + default 1 config SOC_SHA_DMA_MAX_BUFFER_SIZE int diff --git a/components/soc/esp32s3/include/soc/soc_caps.h b/components/soc/esp32s3/include/soc/soc_caps.h index 93adb5ad50..33fc88c9d6 100644 --- a/components/soc/esp32s3/include/soc/soc_caps.h +++ b/components/soc/esp32s3/include/soc/soc_caps.h @@ -333,7 +333,7 @@ #define SOC_UART_REQUIRE_CORE_RESET (1) /*-------------------------- USB CAPS ----------------------------------------*/ -#define SOC_USB_PERIPH_NUM 1 +#define SOC_USB_OTG_PERIPH_NUM (1U) /*--------------------------- SHA CAPS ---------------------------------------*/ diff --git a/examples/peripherals/.build-test-rules.yml b/examples/peripherals/.build-test-rules.yml index 1e7fc00b5c..4d881f3284 100644 --- a/examples/peripherals/.build-test-rules.yml +++ b/examples/peripherals/.build-test-rules.yml @@ -191,7 +191,7 @@ examples/peripherals/uart/uart_echo_rs485: examples/peripherals/usb: disable: - - if: SOC_USB_PERIPH_NUM != 1 + - if: SOC_USB_OTG_SUPPORTED != 1 examples/peripherals/wave_gen: enable: diff --git a/examples/system/.build-test-rules.yml b/examples/system/.build-test-rules.yml index 1c5a0a9cd5..215a086185 100644 --- a/examples/system/.build-test-rules.yml +++ b/examples/system/.build-test-rules.yml @@ -22,7 +22,10 @@ examples/system/console/advanced: examples/system/console/advanced_usb_cdc: disable: - - if: SOC_USB_PERIPH_NUM == 0 + - if: SOC_USB_OTG_SUPPORTED != 1 + depends_components: + - console + - vfs examples/system/console/basic: disable: From ce351790a8a4eb86fa82ce847bd4eac328613955 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Tue, 14 Nov 2023 20:42:25 +0800 Subject: [PATCH 2/4] refactor(hal/usb_dwc): Add DWC OTG configuration values This commit adds a subset of the DWC OTG configuration values to the 'usb_dwc_ll.h' file. Only relevant configuration values have been added. Some DWC OTG releated constants have also been moved from 'usb_dwc_hal.h' to 'usb_dwc_ll.h' and renamed. --- components/hal/include/hal/usb_dwc_hal.h | 18 +++++++------- components/hal/include/hal/usb_dwc_ll.h | 30 +++++++++++++++++++++++- components/hal/usb_dwc_hal.c | 10 ++++---- components/usb/hcd_dwc.c | 10 ++++---- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/components/hal/include/hal/usb_dwc_hal.h b/components/hal/include/hal/usb_dwc_hal.h index d52e882cb9..4369ff3ac0 100644 --- a/components/hal/include/hal/usb_dwc_hal.h +++ b/components/hal/include/hal/usb_dwc_hal.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -22,15 +22,11 @@ NOTE: Thread safety is the responsibility fo the HAL user. All USB Host HAL #include "hal/usb_types_private.h" #include "hal/assert.h" +#if SOC_USB_OTG_SUPPORTED + // ------------------------------------------------ Macros and Types --------------------------------------------------- -// ------------------ Constants/Configs -------------------- - -#define USB_DWC_HAL_DMA_MEM_ALIGN 512 -#define USB_DWC_HAL_FRAME_LIST_MEM_ALIGN 512 //The frame list needs to be 512 bytes aligned (contrary to the databook) -#define USB_DWC_HAL_NUM_CHAN 8 -#define USB_DWC_HAL_XFER_DESC_SIZE (sizeof(usb_dwc_ll_dma_qtd_t)) -#define USB_DWC_HAL_FIFO_TOTAL_USABLE_LINES 200 //Although we have a 256 lines, only 200 lines are usuable due to EPINFO_CTL +// ----------------------- Configs ------------------------- /** * @brief FIFO size configuration structure @@ -168,7 +164,7 @@ typedef struct { struct { int num_allocd; /**< Number of channels currently allocated */ uint32_t chan_pend_intrs_msk; /**< Bit mask of channels with pending interrupts */ - usb_dwc_hal_chan_t *hdls[USB_DWC_HAL_NUM_CHAN]; /**< Handles of each channel. Set to NULL if channel has not been allocated */ + usb_dwc_hal_chan_t *hdls[USB_DWC_NUM_HOST_CHAN]; /**< Handles of each channel. Set to NULL if channel has not been allocated */ } channels; } usb_dwc_hal_context_t; @@ -229,7 +225,7 @@ void usb_dwc_hal_core_soft_reset(usb_dwc_hal_context_t *hal); * may be situations where this function may need to be called again to resize the FIFOs. If resizing FIFOs dynamically, * it is the user's responsibility to ensure there are no active channels when this function is called. * - * @note The totol size of all the FIFOs must be less than or equal to USB_DWC_HAL_FIFO_TOTAL_USABLE_LINES + * @note The totol size of all the FIFOs must be less than or equal to USB_DWC_FIFO_TOTAL_USABLE_LINES * @note After a port reset, the FIFO size registers will reset to their default values, so this function must be called * again post reset. * @@ -785,6 +781,8 @@ usb_dwc_hal_chan_t *usb_dwc_hal_get_chan_pending_intr(usb_dwc_hal_context_t *hal */ usb_dwc_hal_chan_event_t usb_dwc_hal_chan_decode_intr(usb_dwc_hal_chan_t *chan_obj); +#endif // SOC_USB_OTG_SUPPORTED + #ifdef __cplusplus } #endif diff --git a/components/hal/include/hal/usb_dwc_ll.h b/components/hal/include/hal/usb_dwc_ll.h index 3bbaeca4c2..385e93f444 100644 --- a/components/hal/include/hal/usb_dwc_ll.h +++ b/components/hal/include/hal/usb_dwc_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -17,6 +17,34 @@ extern "C" { #include "hal/misc.h" +/* ----------------------------------------------------------------------------- +--------------------------------- DWC Constants -------------------------------- +----------------------------------------------------------------------------- */ + +#define USB_DWC_QTD_LIST_MEM_ALIGN 512 +#define USB_DWC_FRAME_LIST_MEM_ALIGN 512 // The frame list needs to be 512 bytes aligned (contrary to the databook) +/* +Although we have a 256 lines, only 200 lines are useable due to EPINFO_CTL. +Todo: Check sizes again and express this macro in terms of DWC config options (IDF-7384) +*/ +#define USB_DWC_FIFO_TOTAL_USABLE_LINES 200 + +/* ----------------------------------------------------------------------------- +------------------------------ DWC Configuration ------------------------------- +----------------------------------------------------------------------------- */ + +/* + * List of relevant DWC configurations. See DWC OTG databook Chapter 3 for more + * details. + */ +#define USB_DWC_FSPHY_INTERFACE 1 +#define USB_DWC_NUM_EPS 6 +#define USB_DWC_NUM_IN_EPS 5 // Todo: Add check for when number of IN channels exceeds limit (IDF-8556) +#define USB_DWC_NUM_HOST_CHAN 8 +#define USB_DWC_DFIFO_DEPTH 256 +#define USB_DWC_RX_DFIFO_DEPTH 256 +#define USB_DWC_TX_DFIFO_DEPTH 256 // Same value applies to HNPERIO, NPERIO, HPERIO, and DINEP + /* ----------------------------------------------------------------------------- ------------------------------- Global Registers ------------------------------- ----------------------------------------------------------------------------- */ diff --git a/components/hal/usb_dwc_hal.c b/components/hal/usb_dwc_hal.c index 58fc66f443..31a2695c5d 100644 --- a/components/hal/usb_dwc_hal.c +++ b/components/hal/usb_dwc_hal.c @@ -159,14 +159,14 @@ void usb_dwc_hal_core_soft_reset(usb_dwc_hal_context_t *hal) hal->flags.val = 0; hal->channels.num_allocd = 0; hal->channels.chan_pend_intrs_msk = 0; - memset(hal->channels.hdls, 0, sizeof(usb_dwc_hal_chan_t *) * USB_DWC_HAL_NUM_CHAN); + memset(hal->channels.hdls, 0, sizeof(usb_dwc_hal_chan_t *) * USB_DWC_NUM_HOST_CHAN); } void usb_dwc_hal_set_fifo_size(usb_dwc_hal_context_t *hal, const usb_dwc_hal_fifo_config_t *fifo_config) { - HAL_ASSERT((fifo_config->rx_fifo_lines + fifo_config->nptx_fifo_lines + fifo_config->ptx_fifo_lines) <= USB_DWC_HAL_FIFO_TOTAL_USABLE_LINES); + HAL_ASSERT((fifo_config->rx_fifo_lines + fifo_config->nptx_fifo_lines + fifo_config->ptx_fifo_lines) <= USB_DWC_FIFO_TOTAL_USABLE_LINES); //Check that none of the channels are active - for (int i = 0; i < USB_DWC_HAL_NUM_CHAN; i++) { + for (int i = 0; i < USB_DWC_NUM_HOST_CHAN; i++) { if (hal->channels.hdls[i] != NULL) { HAL_ASSERT(!hal->channels.hdls[i]->flags.active); } @@ -208,11 +208,11 @@ bool usb_dwc_hal_chan_alloc(usb_dwc_hal_context_t *hal, usb_dwc_hal_chan_t *chan { HAL_ASSERT(hal->flags.fifo_sizes_set); //FIFO sizes should be set befor attempting to allocate a channel //Attempt to allocate channel - if (hal->channels.num_allocd == USB_DWC_HAL_NUM_CHAN) { + if (hal->channels.num_allocd == USB_DWC_NUM_HOST_CHAN) { return false; //Out of free channels } int chan_idx = -1; - for (int i = 0; i < USB_DWC_HAL_NUM_CHAN; i++) { + for (int i = 0; i < USB_DWC_NUM_HOST_CHAN; i++) { if (hal->channels.hdls[i] == NULL) { hal->channels.hdls[i] = chan_obj; chan_idx = i; diff --git a/components/usb/hcd_dwc.c b/components/usb/hcd_dwc.c index d38d775fc4..86e785eb57 100644 --- a/components/usb/hcd_dwc.c +++ b/components/usb/hcd_dwc.c @@ -53,7 +53,7 @@ typedef struct { * * RXFIFO * - Recommended: ((LPS/4) * 2) + 2 - * - Actual: Whatever leftover size: USB_DWC_HAL_FIFO_TOTAL_USABLE_LINES(200) - 48 - 48 = 104 + * - Actual: Whatever leftover size: USB_DWC_FIFO_TOTAL_USABLE_LINES(200) - 48 - 48 = 104 * - Worst case can accommodate two packets of 204 bytes, or one packet of 408 * NPTXFIFO * - Recommended: (LPS/4) * 2 @@ -81,7 +81,7 @@ const fifo_mps_limits_t mps_limits_default = { * * RXFIFO * - Recommended: ((LPS/4) * 2) + 2 - * - Actual: Whatever leftover size: USB_DWC_HAL_FIFO_TOTAL_USABLE_LINES(200) - 32 - 16 = 152 + * - Actual: Whatever leftover size: USB_DWC_FIFO_TOTAL_USABLE_LINES(200) - 32 - 16 = 152 * - Worst case can accommodate two packets of 300 bytes or one packet of 600 bytes * NPTXFIFO * - Recommended: (LPS/4) * 2 @@ -117,7 +117,7 @@ const fifo_mps_limits_t mps_limits_bias_rx = { * - Worst case can accommodate one packet of 64 bytes * PTXFIFO * - Recommended: (LPS/4) * 2 - * - Actual: Whatever leftover size: USB_DWC_HAL_FIFO_TOTAL_USABLE_LINES(200) - 34 - 16 = 150 + * - Actual: Whatever leftover size: USB_DWC_FIFO_TOTAL_USABLE_LINES(200) - 34 - 16 = 150 * - Worst case can accommodate two packets of 300 bytes or one packet of 600 bytes */ const usb_dwc_hal_fifo_config_t fifo_config_bias_ptx = { @@ -963,7 +963,7 @@ static port_t *port_obj_alloc(void) { port_t *port = calloc(1, sizeof(port_t)); usb_dwc_hal_context_t *hal = malloc(sizeof(usb_dwc_hal_context_t)); - void *frame_list = heap_caps_aligned_calloc(USB_DWC_HAL_FRAME_LIST_MEM_ALIGN, FRAME_LIST_LEN, sizeof(uint32_t), MALLOC_CAP_DMA); + void *frame_list = heap_caps_aligned_calloc(USB_DWC_FRAME_LIST_MEM_ALIGN, FRAME_LIST_LEN, sizeof(uint32_t), MALLOC_CAP_DMA); SemaphoreHandle_t port_mux = xSemaphoreCreateMutex(); if (port == NULL || hal == NULL || frame_list == NULL || port_mux == NULL) { free(port); @@ -1594,7 +1594,7 @@ static dma_buffer_block_t *buffer_block_alloc(usb_transfer_type_t type) break; } dma_buffer_block_t *buffer = calloc(1, sizeof(dma_buffer_block_t)); - void *xfer_desc_list = heap_caps_aligned_calloc(USB_DWC_HAL_DMA_MEM_ALIGN, desc_list_len, sizeof(usb_dwc_ll_dma_qtd_t), MALLOC_CAP_DMA); + void *xfer_desc_list = heap_caps_aligned_calloc(USB_DWC_QTD_LIST_MEM_ALIGN, desc_list_len, sizeof(usb_dwc_ll_dma_qtd_t), MALLOC_CAP_DMA); if (buffer == NULL || xfer_desc_list == NULL) { free(buffer); heap_caps_free(xfer_desc_list); From 9cdd6ac5f1f791183920803d494b73cd1bfb3dae Mon Sep 17 00:00:00 2001 From: Tomas Rezucha Date: Tue, 14 Nov 2023 11:50:02 +0100 Subject: [PATCH 3/4] fix(usb/host): Do not abort on string descriptor overflow Some devices return full LANGID table, even if short LANGID table was requested. No memory overflow occurs, because we have allocated enough memory for transfers to the default pipe. So we can ignore the error and continue with string desc fetching. --- components/usb/hub.c | 13 ++++++++++--- .../usb/host/usb_host_lib/main/class_driver.c | 1 - 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/components/usb/hub.c b/components/usb/hub.c index 40e2ae1b04..519890d135 100644 --- a/components/usb/hub.c +++ b/components/usb/hub.c @@ -449,9 +449,16 @@ static bool enum_stage_transfer_check(enum_ctrl_t *enum_ctrl) return false; } // Check IN transfer returned the expected correct number of bytes - if (enum_ctrl->expect_num_bytes != 0 && enum_ctrl->expect_num_bytes != transfer->actual_num_bytes) { - ESP_LOGE(HUB_DRIVER_TAG, "Incorrect number of bytes returned %d: %s", transfer->actual_num_bytes, enum_stage_strings[enum_ctrl->stage]); - return false; + if (enum_ctrl->expect_num_bytes != 0 && transfer->actual_num_bytes != enum_ctrl->expect_num_bytes) { + if (transfer->actual_num_bytes > enum_ctrl->expect_num_bytes) { + // The device returned more bytes than requested. + // This violates the USB specs chapter 9.3.5, but we can continue + ESP_LOGW(HUB_DRIVER_TAG, "Incorrect number of bytes returned %d: %s", transfer->actual_num_bytes, enum_stage_strings[enum_ctrl->stage]); + } else { + // The device returned less bytes than requested. We cannot continue. + ESP_LOGE(HUB_DRIVER_TAG, "Incorrect number of bytes returned %d: %s", transfer->actual_num_bytes, enum_stage_strings[enum_ctrl->stage]); + return false; + } } // Stage specific checks and updates diff --git a/examples/peripherals/usb/host/usb_host_lib/main/class_driver.c b/examples/peripherals/usb/host/usb_host_lib/main/class_driver.c index 15ae3201f8..d717b6ed99 100644 --- a/examples/peripherals/usb/host/usb_host_lib/main/class_driver.c +++ b/examples/peripherals/usb/host/usb_host_lib/main/class_driver.c @@ -70,7 +70,6 @@ static void action_get_info(class_driver_t *driver_obj) ESP_ERROR_CHECK(usb_host_device_info(driver_obj->dev_hdl, &dev_info)); ESP_LOGI(TAG, "\t%s speed", (dev_info.speed == USB_SPEED_LOW) ? "Low" : "Full"); ESP_LOGI(TAG, "\tbConfigurationValue %d", dev_info.bConfigurationValue); - //Todo: Print string descriptors //Get the device descriptor next driver_obj->actions &= ~ACTION_GET_DEV_INFO; From e50593662f999a462fe47ac7dfd6f34d6a2b858a Mon Sep 17 00:00:00 2001 From: Roman Leonov Date: Fri, 10 Nov 2023 13:15:45 +0100 Subject: [PATCH 4/4] fix(usb/host): remove bInterval verification during pipe opening for INTR and ISOC EPs --- components/usb/hcd_dwc.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/components/usb/hcd_dwc.c b/components/usb/hcd_dwc.c index 86e785eb57..19e3165197 100644 --- a/components/usb/hcd_dwc.c +++ b/components/usb/hcd_dwc.c @@ -137,7 +137,7 @@ const fifo_mps_limits_t mps_limits_bias_ptx = { #define XFER_LIST_LEN_CTRL 3 // One descriptor for each stage #define XFER_LIST_LEN_BULK 2 // One descriptor for transfer, one to support an extra zero length packet -#define XFER_LIST_LEN_INTR 32 +#define XFER_LIST_LEN_INTR FRAME_LIST_LEN #define XFER_LIST_LEN_ISOC FRAME_LIST_LEN // Same length as the frame list makes it easier to schedule. Must be power of 2 // ------------------------ Flags -------------------------- @@ -1642,22 +1642,6 @@ static bool pipe_alloc_hcd_support_verification(const usb_ep_desc_t *ep_desc, co return false; } - // Check if the pipe's interval is compatible with the periodic frame list's length - if (type == USB_TRANSFER_TYPE_INTR && - (ep_desc->bInterval > FRAME_LIST_LEN)) { - ESP_LOGE(HCD_DWC_TAG, "bInterval value (%d) of Interrupt pipe exceeds max supported limit", - ep_desc->bInterval); - return false; - } - - if (type == USB_TRANSFER_TYPE_ISOCHRONOUS && - ((1 << (ep_desc->bInterval - 1)) > FRAME_LIST_LEN)) { - // (where 0 < 2^(bInterval - 1) <= FRAME_LIST_LEN) - ESP_LOGE(HCD_DWC_TAG, "bInterval value (%d) of Isochronous pipe exceeds max supported limit", - ep_desc->bInterval); - return false; - } - // Check if pipe MPS exceeds HCD MPS limits (due to DWC FIFO sizing) int limit; if (USB_EP_DESC_GET_EP_DIR(ep_desc)) { // IN @@ -1710,12 +1694,16 @@ static void pipe_set_ep_char(const hcd_pipe_config_t *pipe_config, usb_transfer_ ep_char->dev_addr = pipe_config->dev_addr; ep_char->ls_via_fs_hub = (port_speed == USB_SPEED_FULL && pipe_config->dev_speed == USB_SPEED_LOW); // Calculate the pipe's interval in terms of USB frames + // @see USB-OTG programming guide chapter 6.5 for more information if (type == USB_TRANSFER_TYPE_INTR || type == USB_TRANSFER_TYPE_ISOCHRONOUS) { - int interval_frames; + unsigned int interval_frames; + unsigned int xfer_list_len; if (type == USB_TRANSFER_TYPE_INTR) { interval_frames = pipe_config->ep_desc->bInterval; + xfer_list_len = XFER_LIST_LEN_INTR; } else { interval_frames = (1 << (pipe_config->ep_desc->bInterval - 1)); + xfer_list_len = XFER_LIST_LEN_ISOC; } // Round down interval to nearest power of 2 if (interval_frames >= 32) { @@ -1733,7 +1721,7 @@ static void pipe_set_ep_char(const hcd_pipe_config_t *pipe_config, usb_transfer_ } ep_char->periodic.interval = interval_frames; // We are the Nth pipe to be allocated. Use N as a phase offset - ep_char->periodic.phase_offset_frames = pipe_idx & (XFER_LIST_LEN_ISOC - 1); + ep_char->periodic.phase_offset_frames = pipe_idx & (xfer_list_len - 1); } else { ep_char->periodic.interval = 0; ep_char->periodic.phase_offset_frames = 0;