Merge branch 'feature/usb_host_cleanup' into 'master'

USB: Host stack cleanup and QOL update Closes IDFGH-6120, IDF-2747, IDFGH-4592, and IDFGH-6402 See merge request espressif/esp-idf!16349
espressif · Jan 6, 2022 · d95b15c · d95b15c
2 parents 5d47efb + ada1c9c
commit d95b15c
Show file tree

Hide file tree

Showing 42 changed files with 2,671 additions and 846 deletions.
diff --git a/components/hal/include/hal/usbh_hal.h b/components/hal/include/hal/usbh_hal.h
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -18,7 +18,6 @@ NOTE: Thread safety is the responsibility fo the HAL user. All USB Host HAL
 #include <stdlib.h>
 #include <stddef.h>
 #include "soc/usbh_struct.h"
-#include "soc/usb_wrap_struct.h"
 #include "hal/usbh_ll.h"
 #include "hal/usb_types_private.h"
 #include "hal/assert.h"
@@ -152,7 +151,6 @@ typedef struct {
 typedef struct {
     //Context
     usbh_dev_t *dev;                            /**< Pointer to base address of DWC_OTG registers */
-    usb_wrap_dev_t *wrap_dev;                   /**< Pointer to base address of USB Wrapper registers */
     //Host Port related
     uint32_t *periodic_frame_list;              /**< Pointer to scheduling frame list */
     usb_hal_frame_list_len_t frame_list_len;    /**< Length of the periodic scheduling frame list */
@@ -181,6 +179,7 @@ typedef struct {
  *
  * Entry:
  * - 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)
  * Exit:
@@ -495,7 +494,7 @@ static inline void usbh_hal_disable_debounce_lock(usbh_hal_context_t *hal)
     hal->flags.dbnc_lock_enabled = 0;
     //Clear Conenction and disconenction interrupt in case it triggered again
     usb_ll_intr_clear(hal->dev, USB_LL_INTR_CORE_DISCONNINT);
-    usbh_ll_hprt_intr_clear(hal->dev, USBH_LL_INTR_HPRT_PRTENCHNG);
+    usbh_ll_hprt_intr_clear(hal->dev, USBH_LL_INTR_HPRT_PRTCONNDET);
     //Reenable the hprt (connection) and disconnection interrupts
     usb_ll_en_intrs(hal->dev, USB_LL_INTR_CORE_PRTINT | USB_LL_INTR_CORE_DISCONNINT);
 }

diff --git a/components/hal/include/hal/usbh_ll.h b/components/hal/include/hal/usbh_ll.h
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -13,7 +13,6 @@ extern "C" {
 #include <stdint.h>
 #include <stdbool.h>
 #include "soc/usbh_struct.h"
-#include "soc/usb_wrap_struct.h"
 #include "hal/usb_types_private.h"
 #include "hal/misc.h"
 
@@ -153,25 +152,6 @@ typedef struct {
     uint8_t *buffer;
 } usbh_ll_dma_qtd_t;
 
-/* -----------------------------------------------------------------------------
------------------------------- USB Wrap Registers ------------------------------
------------------------------------------------------------------------------ */
-
-/**
- * @brief Configures the internal PHY to operate as HOST
- *
- * @param hw Start address of the USB Wrap registers
- */
-static inline void usbh_ll_internal_phy_conf(usb_wrap_dev_t *hw)
-{
-    //Enable internal PHY
-    hw->otg_conf.pad_enable = 1;
-    hw->otg_conf.phy_sel = 0;
-    //Set pulldowns on D+ and D-
-    hw->otg_conf.pad_pull_override = 1;
-    hw->otg_conf.dp_pulldown = 1;
-    hw->otg_conf.dm_pulldown = 1;
-}
 
 /* -----------------------------------------------------------------------------
 ------------------------------- Global Registers -------------------------------
@@ -431,7 +411,7 @@ static inline void usbh_ll_hcfg_set_fsls_pclk_sel(usbh_dev_t *hw)
 /**
  * @brief Sets some default values to HCFG to operate in Host mode with scatter/gather DMA
  *
- * @param hw Start address of the USB Wrap registers
+ * @param hw Start address of the DWC_OTG registers
  * @param speed Speed to initialize the host port at
  */
 static inline void usbh_ll_hcfg_set_defaults(usbh_dev_t *hw, usb_priv_speed_t speed)

diff --git a/components/hal/usbh_hal.c b/components/hal/usbh_hal.c
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2020-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -84,7 +84,6 @@
 
 static void set_defaults(usbh_hal_context_t *hal)
 {
-    usbh_ll_internal_phy_conf(hal->wrap_dev);   //Enable and configure internal PHY
     //GAHBCFG register
     usb_ll_en_dma_mode(hal->dev);
 #ifdef CONFIG_IDF_TARGET_ESP32S2
@@ -114,7 +113,6 @@ void usbh_hal_init(usbh_hal_context_t *hal)
     //Initialize HAL context
     memset(hal, 0, sizeof(usbh_hal_context_t));
     hal->dev = dev;
-    hal->wrap_dev = &USB_WRAP;
     set_defaults(hal);
 }
 
@@ -125,7 +123,6 @@ void usbh_hal_deinit(usbh_hal_context_t *hal)
     usb_ll_intr_read_and_clear(hal->dev);   //Clear interrupts
     usb_ll_dis_global_intr(hal->dev);       //Disable interrupt signal
     hal->dev = NULL;
-    hal->wrap_dev = NULL;
 }
 
 void usbh_hal_core_soft_reset(usbh_hal_context_t *hal)

diff --git a/components/usb/hcd.c b/components/usb/hcd.c
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -194,12 +194,13 @@ typedef struct {
             uint32_t reserved28: 28;
         } ctrl;                             //Control transfer related
         struct {
-            uint32_t zero_len_packet: 1;    //Bulk transfer should add a zero length packet at the end regardless
+            uint32_t zero_len_packet: 1;    //Added a zero length packet, so transfer consists of 2 QTDs
             uint32_t reserved31: 31;
         } bulk;                             //Bulk transfer related
         struct {
-            uint32_t num_qtds: 8;           //Number of transfer descriptors filled
-            uint32_t reserved24: 24;
+            uint32_t num_qtds: 8;           //Number of transfer descriptors filled (excluding zero length packet)
+            uint32_t zero_len_packet: 1;    //Added a zero length packet, so true number descriptors is num_qtds + 1
+            uint32_t reserved23: 23;
         } intr;                             //Interrupt transfer related
         struct {
             uint32_t num_qtds: 8;           //Number of transfer descriptors filled (including NULL descriptors)
@@ -1025,20 +1026,6 @@ esp_err_t hcd_install(const hcd_config_t *config)
         goto err;
     }
     s_hcd_obj = p_hcd_obj_dmy;
-    //Set HW prerequisites for each port (there's only one)
-    periph_module_enable(PERIPH_USB_MODULE);
-    periph_module_reset(PERIPH_USB_MODULE);
-    /*
-    Configure GPIOS for Host mode operation using internal PHY
-        - Forces ID to GND for A side
-        - Forces B Valid to GND as we are A side host
-        - Forces VBUS Valid to HIGH
-        - Forces A Valid to HIGH
-    */
-    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ZERO_INPUT, USB_OTG_IDDIG_IN_IDX, false);
-    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ZERO_INPUT, USB_SRP_BVALID_IN_IDX, false);
-    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, USB_OTG_VBUSVALID_IN_IDX, false);
-    esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, USB_OTG_AVALID_IN_IDX, false);
     HCD_EXIT_CRITICAL();
     return ESP_OK;
 
@@ -1059,7 +1046,6 @@ esp_err_t hcd_uninstall(void)
         HCD_EXIT_CRITICAL();
         return ESP_ERR_INVALID_STATE;
     }
-    periph_module_disable(PERIPH_USB_MODULE);
     hcd_obj_t *p_hcd_obj_dmy = s_hcd_obj;
     s_hcd_obj = NULL;
     HCD_EXIT_CRITICAL();
@@ -2082,8 +2068,8 @@ static inline void _buffer_fill_ctrl(dma_buffer_block_t *buffer, usb_transfer_t
         //Not data stage. Fill with an empty descriptor
         usbh_hal_xfer_desc_clear(buffer->xfer_desc_list, 1);
     } else {
-        //Fill data stage
-        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 1, transfer->data_buffer + sizeof(usb_setup_packet_t), setup_pkt->wLength,
+        //Fill data stage. Note that we still fill with transfer->num_bytes instead of setup_pkt->wLength as it's possible to require more bytes than wLength
+        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 1, transfer->data_buffer + sizeof(usb_setup_packet_t), transfer->num_bytes - sizeof(usb_setup_packet_t),
                                 ((data_stg_in) ? USBH_HAL_XFER_DESC_FLAG_IN : 0) | USBH_HAL_XFER_DESC_FLAG_HOC);
     }
     //Fill status stage (i.e., a zero length packet). If data stage is skipped, the status stage is always IN.
@@ -2095,46 +2081,68 @@ static inline void _buffer_fill_ctrl(dma_buffer_block_t *buffer, usb_transfer_t
     buffer->flags.ctrl.cur_stg = 0;
 }
 
-static inline void _buffer_fill_bulk(dma_buffer_block_t *buffer, usb_transfer_t *transfer, bool is_in)
+static inline void _buffer_fill_bulk(dma_buffer_block_t *buffer, usb_transfer_t *transfer, bool is_in, int mps)
 {
+    //Only add a zero length packet if OUT, flag is set, and transfer length is multiple of EP's MPS
+    //Minor optimization: Do the mod operation last
+    bool zero_len_packet = !is_in && (transfer->flags & USB_TRANSFER_FLAG_ZERO_PACK) && (transfer->num_bytes % mps == 0);
     if (is_in) {
         usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 0, transfer->data_buffer, transfer->num_bytes,
                                 USBH_HAL_XFER_DESC_FLAG_IN | USBH_HAL_XFER_DESC_FLAG_HOC);
-    } else if (transfer->flags & USB_TRANSFER_FLAG_ZERO_PACK) {
-        //We need to add an extra zero length packet, so two descriptors are used
-        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 0, transfer->data_buffer, transfer->num_bytes, 0);
-        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 1, NULL, 0, USBH_HAL_XFER_DESC_FLAG_HOC);
-    } else {
-        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 0, transfer->data_buffer, transfer->num_bytes, USBH_HAL_XFER_DESC_FLAG_HOC);
+    } else { //OUT
+        if (zero_len_packet) {
+            //Adding a zero length packet, so two descriptors are used.
+            usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 0, transfer->data_buffer, transfer->num_bytes, 0);
+            usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 1, NULL, 0, USBH_HAL_XFER_DESC_FLAG_HOC);
+        } else {
+            //Zero length packet not required. One descriptor is enough
+            usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, 0, transfer->data_buffer, transfer->num_bytes, USBH_HAL_XFER_DESC_FLAG_HOC);
+        }
     }
     //Update buffer flags
-    buffer->flags.bulk.zero_len_packet = (is_in && (transfer->flags & USB_TRANSFER_FLAG_ZERO_PACK)) ? 1 : 0;
+    buffer->flags.bulk.zero_len_packet = zero_len_packet;
 }
 
 static inline void _buffer_fill_intr(dma_buffer_block_t *buffer, usb_transfer_t *transfer, bool is_in, int mps)
 {
     int num_qtds;
+    int mod_mps = transfer->num_bytes % mps;
+    //Only add a zero length packet if OUT, flag is set, and transfer length is multiple of EP's MPS
+    bool zero_len_packet = !is_in && (transfer->flags & USB_TRANSFER_FLAG_ZERO_PACK) && (mod_mps == 0);
     if (is_in) {
-        assert(transfer->num_bytes % mps == 0);  //IN transfers MUST be integer multiple of MPS
-        num_qtds = transfer->num_bytes / mps;
+        assert(mod_mps == 0);  //IN transfers MUST be integer multiple of MPS
+        num_qtds = transfer->num_bytes / mps;   //Can just floor divide as it's already multiple of MPS
     } else {
-        num_qtds = transfer->num_bytes / mps;    //Floor division for number of MPS packets
-        if (transfer->num_bytes % transfer->num_bytes > 0) {
-            num_qtds++; //For the last shot packet
+        num_qtds = transfer->num_bytes / mps;   //Floor division to get the number of MPS sized packets
+        if (mod_mps > 0) {
+            num_qtds++; //Add a short packet for the remainder
         }
     }
-    assert(num_qtds <= XFER_LIST_LEN_INTR);
-    //Fill all but last descriptor
+    assert((zero_len_packet) ? num_qtds + 1 : num_qtds <= XFER_LIST_LEN_INTR); //Check that the number of QTDs doesn't exceed the QTD list's length
+
+    uint32_t xfer_desc_flags = (is_in) ? USBH_HAL_XFER_DESC_FLAG_IN : 0;
     int bytes_filled = 0;
+    //Fill all but last QTD
     for (int i = 0; i < num_qtds - 1; i++) {
-        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, i, &transfer->data_buffer[bytes_filled], mps, (is_in) ? USBH_HAL_XFER_DESC_FLAG_IN : 0);
+        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, i, &transfer->data_buffer[bytes_filled], mps, xfer_desc_flags);
         bytes_filled += mps;
     }
-    //Fill in the last descriptor with HOC flag
-    usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, num_qtds - 1, &transfer->data_buffer[bytes_filled], transfer->num_bytes - bytes_filled,
-                            ((is_in) ? USBH_HAL_XFER_DESC_FLAG_IN : 0) | USBH_HAL_XFER_DESC_FLAG_HOC);
+    //Fill last QTD and zero length packet
+    if (zero_len_packet) {
+        //Fill in last data packet without HOC flag
+        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, num_qtds - 1, &transfer->data_buffer[bytes_filled], transfer->num_bytes - bytes_filled,
+                                xfer_desc_flags);
+        //HOC flag goes to zero length packet instead
+        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, num_qtds, NULL, 0, USBH_HAL_XFER_DESC_FLAG_HOC);
+    } else {
+        //Zero length packet not required. Fill in last QTD with HOC flag
+        usbh_hal_xfer_desc_fill(buffer->xfer_desc_list, num_qtds - 1, &transfer->data_buffer[bytes_filled], transfer->num_bytes - bytes_filled,
+                                xfer_desc_flags | USBH_HAL_XFER_DESC_FLAG_HOC);
+    }
+
     //Update buffer members and flags
     buffer->flags.intr.num_qtds = num_qtds;
+    buffer->flags.intr.zero_len_packet = zero_len_packet;
 }
 
 static inline void _buffer_fill_isoc(dma_buffer_block_t *buffer, usb_transfer_t *transfer, bool is_in, int mps, int interval, int start_idx)
@@ -2220,7 +2228,7 @@ static void _buffer_fill(pipe_t *pipe)
             break;
         }
         case USB_PRIV_XFER_TYPE_BULK: {
-            _buffer_fill_bulk(buffer_to_fill, transfer, is_in);
+            _buffer_fill_bulk(buffer_to_fill, transfer, is_in, mps);
             break;
         }
         case USB_PRIV_XFER_TYPE_INTR: {
@@ -2269,7 +2277,7 @@ static void _buffer_exec(pipe_t *pipe)
         }
         case USB_PRIV_XFER_TYPE_INTR: {
             start_idx = 0;
-            desc_list_len = buffer_to_exec->flags.intr.num_qtds;
+            desc_list_len = (buffer_to_exec->flags.intr.zero_len_packet) ? buffer_to_exec->flags.intr.num_qtds + 1 : buffer_to_exec->flags.intr.num_qtds;
             break;
         }
         default: {
@@ -2389,7 +2397,7 @@ static inline void _buffer_parse_intr(dma_buffer_block_t *buffer, bool is_in, in
             transfer->actual_num_bytes = transfer->num_bytes - last_packet_rem_len;
         }
     } else {
-        //OUT INTR transfers can only complete successfully if all MPS packets have been transmitted. Double check
+        //OUT INTR transfers can only complete successfully if all packets have been transmitted. Double check
         for (int i = 0 ; i < buffer->flags.intr.num_qtds; i++) {
             int rem_len;
             int desc_status;