Merge branch 'fix/usb_cache_sync' into 'master'

fix(usb/host): Sync cache for memory shared by CPU and USB-OTG

Closes IDF-9012

See merge request espressif/esp-idf!28255

components/usb/CMakeLists.txt

@@ -10,7 +10,7 @@ set(priv_includes)
 # As CONFIG_SOC_USB_OTG_SUPPORTED comes from Kconfig, it is not evaluated yet
 # when components are being registered.
 # Thus, always add the (private) requirements, regardless of Kconfig
-set(priv_requires esp_driver_gpio)  # usb_phy driver relies on gpio driver API
+set(priv_requires esp_driver_gpio esp_mm)  # usb_phy driver relies on gpio driver API
 
 if(CONFIG_SOC_USB_OTG_SUPPORTED)
     list(APPEND srcs "hcd_dwc.c"

components/usb/hcd_dwc.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -11,6 +11,7 @@
 #include "freertos/task.h"
 #include "freertos/semphr.h"
 #include "esp_heap_caps.h"
+#include "esp_dma_utils.h"
 #include "esp_intr_alloc.h"
 #include "soc/interrupts.h" // For interrupt index
 #include "esp_err.h"
@@ -21,6 +22,12 @@
 #include "usb_private.h"
 #include "usb/usb_types_ch9.h"
 
+#include "soc/soc_caps.h"
+#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+#include "esp_cache.h"
+#include "esp_private/esp_cache_private.h"
+#endif // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+
 // ----------------------------------------------------- Macros --------------------------------------------------------
 
 // --------------------- Constants -------------------------
@@ -84,6 +91,29 @@ const char *HCD_DWC_TAG = "HCD DWC";
             }                                                               \
 })
 
+// ----------------------- Cache sync ----------------------
+
+/**
+ * @brief Cache sync macros
+ *
+ * This macros are relevant only for SOCs that have L1 cache for internal memory
+ * For other SOCs this is no-operation
+ */
+#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+#define ALIGN_UP_BY(num, align)                     (((num) + ((align) - 1)) & ~((align) - 1))
+#define CACHE_SYNC_FRAME_LIST(frame_list)           cache_sync_frame_list(frame_list)
+#define CACHE_SYNC_XFER_DESCRIPTOR_LIST_M2C(buffer) cache_sync_xfer_descriptor_list(buffer, true)
+#define CACHE_SYNC_XFER_DESCRIPTOR_LIST_C2M(buffer) cache_sync_xfer_descriptor_list(buffer, false)
+#define CACHE_SYNC_DATA_BUFFER_M2C(pipe, urb)       cache_sync_data_buffer(pipe, urb, true)
+#define CACHE_SYNC_DATA_BUFFER_C2M(pipe, urb)       cache_sync_data_buffer(pipe, urb, false)
+#else // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+#define CACHE_SYNC_FRAME_LIST(frame_list)
+#define CACHE_SYNC_XFER_DESCRIPTOR_LIST_M2C(buffer)
+#define CACHE_SYNC_XFER_DESCRIPTOR_LIST_C2M(buffer)
+#define CACHE_SYNC_DATA_BUFFER_M2C(pipe, urb)
+#define CACHE_SYNC_DATA_BUFFER_C2M(pipe, urb)
+#endif // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+
 // ------------------------------------------------------ Types --------------------------------------------------------
 
 typedef struct pipe_obj pipe_t;
@@ -94,6 +124,7 @@ typedef struct port_obj port_t;
  */
 typedef struct {
     void *xfer_desc_list;
+    int xfer_desc_list_len_bytes;           // Only for cache msync
     urb_t *urb;
     union {
         struct {
@@ -236,6 +267,84 @@ static hcd_obj_t *s_hcd_obj = NULL;     // Note: "s_" is for the static pointer
 
 // ------------------------------------------------- Forward Declare ---------------------------------------------------
 
+// --------------------- Cache sync ------------------------
+
+#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+/**
+ * @brief Sync Frame List from cache to memory
+ */
+static inline void cache_sync_frame_list(void *frame_list)
+{
+    esp_err_t ret = esp_cache_msync(frame_list, FRAME_LIST_LEN * sizeof(uint32_t), 0);
+    assert(ret == ESP_OK);
+}
+
+/**
+ * @brief Sync Transfer Descriptor List
+ *
+ * @param[in] buffer       Buffer that holds the Transfer Descriptor List
+ * @param[in] mem_to_cache Direction of cache sync
+ */
+static inline void cache_sync_xfer_descriptor_list(dma_buffer_block_t *buffer, bool mem_to_cache)
+{
+    esp_err_t ret = esp_cache_msync(buffer->xfer_desc_list, buffer->xfer_desc_list_len_bytes, mem_to_cache ? ESP_CACHE_MSYNC_FLAG_DIR_M2C : 0);
+    assert(ret == ESP_OK);
+}
+
+/**
+ * @brief Sync Transfer data buffer
+ *
+ * This function must be called before a URB is enqueued or dequeued.
+ * Based on transfer direction (IN/OUT), this function will msync the data buffer associated with this URB.
+ *
+ * @note Here we also accept UNALIGNED data, for cases where the class drivers force overwrite the allocated data buffers
+ *
+ * @param[in] pipe Pipe belonging to this data buffer
+ * @param[in] urb  URB belonging to this data buffer
+ * @param[in] done Whether data buffer was just processed or is about to be processed
+ */
+static inline void cache_sync_data_buffer(pipe_t *pipe, urb_t *urb, bool done)
+{
+    const bool is_in = pipe->ep_char.bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK;
+    const bool is_ctrl = (pipe->ep_char.type == USB_DWC_XFER_TYPE_CTRL);
+    if ((is_in == done) || is_ctrl) {
+        uint32_t flags = (done) ? ESP_CACHE_MSYNC_FLAG_DIR_M2C : 0;
+        flags |= ESP_CACHE_MSYNC_FLAG_UNALIGNED;
+        esp_err_t ret = esp_cache_msync(urb->transfer.data_buffer, urb->transfer.data_buffer_size, flags);
+        assert(ret == ESP_OK);
+    }
+}
+#endif // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+
+// --------------------- Allocation ------------------------
+
+/**
+ * @brief Allocate Frame List
+ *
+ * - Frame list is allocated in DMA capable memory
+ * - Frame list is aligned to 512 and cache line size
+ *
+ * @note Free the memory with heap_caps_free() call
+ *
+ * @param[in] frame_list_len Length of the Frame List
+ * @return Pointer to allocated frame list
+ */
+static void *frame_list_alloc(size_t frame_list_len);
+
+/**
+ * @brief Allocate Transfer Descriptor List
+ *
+ * - Frame list is allocated in DMA capable memory
+ * - Frame list is aligned to 512 and cache line size
+ *
+ * @note Free the memory with heap_caps_free() call
+ *
+ * @param[in]  list_len           Required length
+ * @param[out] list_len_bytes_out Allocated length in bytes (can be greater than required)
+ * @return Pointer to allocated transfer descriptor list
+ */
+static void *transfer_descriptor_list_alloc(size_t list_len, size_t *list_len_bytes_out);
+
 // ------------------- Buffer Control ----------------------
 
 /**
@@ -891,7 +1000,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_FRAME_LIST_MEM_ALIGN, FRAME_LIST_LEN, sizeof(uint32_t), MALLOC_CAP_DMA);
+    void *frame_list = frame_list_alloc(FRAME_LIST_LEN);
     SemaphoreHandle_t port_mux = xSemaphoreCreateMutex();
     if (port == NULL || hal == NULL || frame_list == NULL || port_mux == NULL) {
         free(port);
@@ -919,6 +1028,45 @@ static void port_obj_free(port_t *port)
     free(port);
 }
 
+void *frame_list_alloc(size_t frame_list_len)
+{
+    void *frame_list = heap_caps_aligned_calloc(USB_DWC_FRAME_LIST_MEM_ALIGN, frame_list_len, sizeof(uint32_t), MALLOC_CAP_DMA);
+
+    // Both Frame List start address and size should be already cache aligned so this is only a sanity check
+    if (frame_list) {
+        if (!esp_dma_is_buffer_aligned(frame_list, frame_list_len * sizeof(uint32_t), ESP_DMA_BUF_LOCATION_AUTO)) {
+            // This should never happen
+            heap_caps_free(frame_list);
+            frame_list = NULL;
+        }
+    }
+    return frame_list;
+}
+
+void *transfer_descriptor_list_alloc(size_t list_len, size_t *list_len_bytes_out)
+{
+#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+    // Required Transfer Descriptor List size (in bytes) might not be aligned to cache line size, align the size up
+    size_t data_cache_line_size = 0;
+    esp_cache_get_alignment(ESP_CACHE_MALLOC_FLAG_DMA, &data_cache_line_size);
+    const size_t required_list_len_bytes = list_len * sizeof(usb_dwc_ll_dma_qtd_t);
+    *list_len_bytes_out = ALIGN_UP_BY(required_list_len_bytes, data_cache_line_size);
+#else
+    *list_len_bytes_out = list_len * sizeof(usb_dwc_ll_dma_qtd_t);
+#endif // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+
+    void *qtd_list =  heap_caps_aligned_calloc(USB_DWC_QTD_LIST_MEM_ALIGN, *list_len_bytes_out, 1, MALLOC_CAP_DMA);
+
+    if (qtd_list) {
+        if (!esp_dma_is_buffer_aligned(qtd_list, *list_len_bytes_out * sizeof(usb_dwc_ll_dma_qtd_t), ESP_DMA_BUF_LOCATION_AUTO)) {
+            // This should never happen
+            heap_caps_free(qtd_list);
+            qtd_list = NULL;
+        }
+    }
+    return qtd_list;
+}
+
 // ----------------------- Public --------------------------
 
 esp_err_t hcd_install(const hcd_config_t *config)
@@ -1026,6 +1174,7 @@ static void _port_recover_all_pipes(port_t *port)
         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)
@@ -1472,13 +1621,15 @@ 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_QTD_LIST_MEM_ALIGN, desc_list_len, sizeof(usb_dwc_ll_dma_qtd_t), MALLOC_CAP_DMA);
+    size_t real_len = 0;
+    void *xfer_desc_list = transfer_descriptor_list_alloc(desc_list_len, &real_len);
     if (buffer == NULL || xfer_desc_list == NULL) {
         free(buffer);
         heap_caps_free(xfer_desc_list);
         return NULL;
     }
     buffer->xfer_desc_list = xfer_desc_list;
+    buffer->xfer_desc_list_len_bytes = real_len;
     return buffer;
 }
 
@@ -1792,6 +1943,7 @@ esp_err_t hcd_pipe_alloc(hcd_port_handle_t port_hdl, const hcd_pipe_config_t *pi
         goto err;
     }
     usb_dwc_hal_chan_set_ep_char(port->hal, pipe->chan_obj, &pipe->ep_char);
+    CACHE_SYNC_FRAME_LIST(port->frame_list);
     // Add the pipe to the list of idle pipes in the port object
     TAILQ_INSERT_TAIL(&port->pipes_idle_tailq, pipe, tailq_entry);
     port->num_pipes_idle++;
@@ -2154,6 +2306,8 @@ static void _buffer_fill(pipe_t *pipe)
         break;
     }
     }
+    // Sync transfer descriptor list to memory
+    CACHE_SYNC_XFER_DESCRIPTOR_LIST_C2M(buffer_to_fill);
     buffer_to_fill->urb = urb;
     urb->hcd_var = URB_HCD_STATE_INFLIGHT;
     // Update multi buffer flags
@@ -2390,6 +2544,9 @@ static void _buffer_parse(pipe_t *pipe)
     bool is_in = pipe->ep_char.bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK;
     int mps = pipe->ep_char.mps;
 
+    // Sync transfer descriptor list to cache
+    CACHE_SYNC_XFER_DESCRIPTOR_LIST_M2C(buffer_to_parse);
+
     // Parsing the buffer will update the buffer's corresponding URB
     if (buffer_to_parse->status_flags.pipe_event == HCD_PIPE_EVENT_URB_DONE) {
         // URB was successful
@@ -2457,6 +2614,9 @@ esp_err_t hcd_urb_enqueue(hcd_pipe_handle_t pipe_hdl, urb_t *urb)
     HCD_CHECK(urb->hcd_ptr == NULL && urb->hcd_var == URB_HCD_STATE_IDLE, ESP_ERR_INVALID_STATE);
     pipe_t *pipe = (pipe_t *)pipe_hdl;
 
+    // Sync user's data from cache to memory. For OUT and CTRL transfers
+    CACHE_SYNC_DATA_BUFFER_C2M(pipe, urb);
+
     HCD_ENTER_CRITICAL();
     // 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
@@ -2513,6 +2673,8 @@ urb_t *hcd_urb_dequeue(hcd_pipe_handle_t pipe_hdl)
             pipe->port->num_pipes_queued--;
             pipe->cs_flags.has_urb = 0;
         }
+        // Sync user's data in memory to cache. For IN and CTRL transfers
+        CACHE_SYNC_DATA_BUFFER_M2C(pipe, urb);
     } else {
         // No more URBs to dequeue from this pipe
         urb = NULL;

components/usb/test_apps/hcd/main/CMakeLists.txt

@@ -2,5 +2,5 @@
 # the component can be registered as WHOLE_ARCHIVE
 idf_component_register(SRC_DIRS "."
                        PRIV_INCLUDE_DIRS "../../../private_include" "."
-                       REQUIRES usb unity common
+                       REQUIRES usb unity common esp_mm
                        WHOLE_ARCHIVE)

components/usb/test_apps/hcd/main/test_hcd_common.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -20,6 +20,7 @@
 #include "test_hcd_common.h"
 #include "test_usb_common.h"
 #include "unity.h"
+#include "esp_dma_utils.h"
 
 #define PORT_NUM                1
 #define EVENT_QUEUE_LEN         5
@@ -201,11 +202,10 @@ usb_speed_t test_hcd_wait_for_conn(hcd_port_handle_t port_hdl)
     //Get speed of connected
     usb_speed_t port_speed;
     TEST_ASSERT_EQUAL(ESP_OK, hcd_port_get_speed(port_hdl, &port_speed));
-    if (port_speed == USB_SPEED_FULL) {
+    TEST_ASSERT_LESS_OR_EQUAL_MESSAGE(USB_SPEED_HIGH, port_speed, "Invalid port speed");
-        printf("Full speed enabled\n");
+    printf("%s speed enabled\n", (char *[]) {
-    } else {
+        "Low", "Full", "High"
-        printf("Low speed enabled\n");
+    }[port_speed]);
-    }
     return port_speed;
 }
 
@@ -263,13 +263,18 @@ void test_hcd_pipe_free(hcd_pipe_handle_t pipe_hdl)
 urb_t *test_hcd_alloc_urb(int num_isoc_packets, size_t data_buffer_size)
 {
     //Allocate a URB and data buffer
-    urb_t *urb = heap_caps_calloc(1, sizeof(urb_t) + (num_isoc_packets * sizeof(usb_isoc_packet_desc_t)), MALLOC_CAP_DEFAULT);
+    urb_t *urb = heap_caps_calloc(1, sizeof(urb_t) + (sizeof(usb_isoc_packet_desc_t) * num_isoc_packets), MALLOC_CAP_DEFAULT);
-    uint8_t *data_buffer = heap_caps_malloc(data_buffer_size, MALLOC_CAP_DMA);
+    void *data_buffer;
-    TEST_ASSERT_NOT_NULL(urb);
+    size_t real_size;
-    TEST_ASSERT_NOT_NULL(data_buffer);
+    esp_dma_malloc(data_buffer_size, 0, &data_buffer, &real_size);
+
+    TEST_ASSERT_NOT_NULL_MESSAGE(urb, "Failed to allocate URB");
+    TEST_ASSERT_NOT_NULL_MESSAGE(data_buffer, "Failed to allocate transfer buffer");
+
     //Initialize URB and underlying transfer structure. Need to cast to dummy due to const fields
     usb_transfer_dummy_t *transfer_dummy = (usb_transfer_dummy_t *)&urb->transfer;
     transfer_dummy->data_buffer = data_buffer;
+    transfer_dummy->data_buffer_size = real_size;
     transfer_dummy->num_isoc_packets = num_isoc_packets;
     return urb;
 }

components/usb/usb_private.c

@@ -1,24 +1,27 @@
 /*
- * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 #include "esp_heap_caps.h"
+#include "esp_dma_utils.h"
 #include "usb_private.h"
 #include "usb/usb_types_ch9.h"
 
 urb_t *urb_alloc(size_t data_buffer_size, int num_isoc_packets)
 {
     urb_t *urb = heap_caps_calloc(1, sizeof(urb_t) + (sizeof(usb_isoc_packet_desc_t) * num_isoc_packets), MALLOC_CAP_DEFAULT);
-    uint8_t *data_buffer = heap_caps_malloc(data_buffer_size, MALLOC_CAP_DMA);
+    void *data_buffer;
+    size_t real_size;
+    esp_dma_malloc(data_buffer_size, 0, &data_buffer, &real_size);
     if (urb == NULL || data_buffer == NULL)     {
         goto err;
     }
     // Cast as dummy transfer so that we can assign to const fields
     usb_transfer_dummy_t *dummy_transfer = (usb_transfer_dummy_t *)&urb->transfer;
     dummy_transfer->data_buffer = data_buffer;
-    dummy_transfer->data_buffer_size = data_buffer_size;
+    dummy_transfer->data_buffer_size = real_size;
     dummy_transfer->num_isoc_packets = num_isoc_packets;
     return urb;
 err: