fix(tusb_msc): Cache sector metrics, enable ping-pong buffering on ES…

…P32S3

Cached `get_sector_size` and `get_sector_count` to improve performance and avoid redundant calls.
Ping-pong buffering mechanism is enabled only for ESP32S3 targets since it
negatively impacts SD card write speed on other platforms.

Closes espressif/esp-idf#11110

device/.gitignore

@@ -0,0 +1,2 @@
+
+.codegpt

device/esp_tinyusb/include/tusb_msc_storage.h

@@ -70,7 +70,7 @@ typedef struct {
  * initializing the SPI Flash media.
  */
 typedef struct {
-    wl_handle_t wl_handle;                          /*!< Pointer to spiflash wera-levelling handle */
+    wl_handle_t wl_handle;                          /*!< Pointer to spiflash wear-levelling handle */
     tusb_msc_callback_t callback_mount_changed;     /*!< Pointer to the function callback that will be delivered AFTER mount/unmount operation is successfully finished */
     tusb_msc_callback_t callback_premount_changed;  /*!< Pointer to the function callback that will be delivered BEFORE mount/unmount operation is started */
     const esp_vfs_fat_mount_config_t mount_config; /*!< FATFS mount config */

device/esp_tinyusb/tusb_msc_storage.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -15,6 +15,7 @@
 #include "esp_partition.h"
 #include "vfs_fat_internal.h"
 #include "tinyusb.h"
+#include "device/usbd_pvt.h"
 #include "class/msc/msc_device.h"
 #include "tusb_msc_storage.h"
 #include "esp_vfs_fat.h"
@@ -24,29 +25,69 @@
 
 static const char *TAG = "tinyusb_msc_storage";
 
+/**
+ * @brief Handle for TinyUSB MSC storage interface.
+ *
+ * This structure holds metadata and function pointers required to
+ * manage the underlying storage medium (SPI flash, SDMMC).
+ */
 typedef struct {
-    bool is_fat_mounted;
-    const char *base_path;
+    bool is_fat_mounted;                  /*!< Indicates if the FAT filesystem is currently mounted. */
+    const char *base_path;                /*!< Base path where the filesystem is mounted. */
     union {
-        wl_handle_t wl_handle;
+        wl_handle_t wl_handle;            /*!< Handle for wear leveling on SPI flash. */
 #if SOC_SDMMC_HOST_SUPPORTED
-        sdmmc_card_t *card;
+        sdmmc_card_t *card;               /*!< Handle for SDMMC card. */
 #endif
     };
-    esp_err_t (*mount)(BYTE pdrv);
-    esp_err_t (*unmount)(void);
-    uint32_t (*sector_count)(void);
-    uint32_t (*sector_size)(void);
-    esp_err_t (*read)(size_t sector_size, uint32_t lba, uint32_t offset, size_t size, void *dest);
-    esp_err_t (*write)(size_t sector_size, size_t addr, uint32_t lba, uint32_t offset, size_t size, const void *src);
-    tusb_msc_callback_t callback_mount_changed;
-    tusb_msc_callback_t callback_premount_changed;
-    int max_files;
-} tinyusb_msc_storage_handle_s; /*!< MSC object */
+    esp_err_t (*mount)(BYTE pdrv);        /*!< Pointer to the mount function. */
+    esp_err_t (*unmount)(void);           /*!< Pointer to the unmount function. */
+    uint32_t sector_count;                /*!< Total number of sectors in the storage medium. */
+    uint32_t sector_size;                 /*!< Size of a single sector in bytes. */
+    esp_err_t (*read)(size_t sector_size, /*!< Function pointer for reading data. */
+                      uint32_t lba, uint32_t offset, size_t size, void *dest);
+    esp_err_t (*write)(size_t sector_size, /*!< Function pointer for writing data. */
+                       size_t addr, uint32_t lba, uint32_t offset, size_t size, const void *src);
+    tusb_msc_callback_t callback_mount_changed; /*!< Callback for mount state change. */
+    tusb_msc_callback_t callback_premount_changed; /*!< Callback for pre-mount state change. */
+    int max_files;                          /*!< Maximum number of files that can be open simultaneously. */
+} tinyusb_msc_storage_handle_s;
 
 /* handle of tinyusb driver connected to application */
 static tinyusb_msc_storage_handle_s *s_storage_handle;
 
+#define MSC_WRITE_BUFFER_SIZE CONFIG_TINYUSB_MSC_BUFSIZE /*!< Size of the buffer, configured via menuconfig (MSC FIFO size) */
+#if ((MSC_WRITE_BUFFER_SIZE) % 4 != 0)
+#error "CONFIG_TINYUSB_MSC_BUFSIZE must be divisible by 4. Adjust your configuration (MSC FIFO size) in menuconfig."
+#endif
+
+#if CONFIG_IDF_TARGET_ESP32S3
+/**
+ * @brief Structure representing a single write buffer for MSC operations.
+ */
+typedef struct {
+    uint8_t data_buffer[MSC_WRITE_BUFFER_SIZE]; /*!< Buffer to store write data. The size is defined by MSC_WRITE_BUFFER_SIZE. */
+    uint32_t lba;                          /*!< Logical Block Address for the current WRITE10 operation. */
+    uint32_t offset;                       /*!< Offset within the specified LBA for the current write operation. */
+    uint32_t bufsize;                      /*!< Number of bytes to be written in this operation. */
+    volatile bool is_data_pending;         /*!< Indicates whether the buffer contains data waiting to be processed. */
+} msc_write_buffer_t;
+
+/**
+ * @brief Structure managing dual write buffers and associated metadata.
+ *
+ * This structure supports a ping-pong buffering mechanism for MSC operations,
+ * allowing concurrent data processing and reception.
+ */
+typedef struct {
+    msc_write_buffer_t buffers[2]; /*!< Array of two write buffers used for ping-pong buffering. */
+    volatile msc_write_buffer_t *p_write_buffer; /*!< Pointer to the active write buffer. */
+    volatile msc_write_buffer_t *p_read_buffer;  /*!< Pointer to the active read buffer. */
+} msc_write_buffer_handle_t;
+
+static msc_write_buffer_handle_t *write_buffer;
+#endif
+
 static esp_err_t _mount_spiflash(BYTE pdrv)
 {
     return ff_diskio_register_wl_partition(pdrv, s_storage_handle->wl_handle);
@@ -170,7 +211,7 @@ static esp_err_t _write_sector_sdmmc(size_t sector_size,
 }
 #endif
 
-static esp_err_t msc_storage_read_sector(uint32_t lba,
+static esp_err_t _msc_storage_read_sector(uint32_t lba,
         uint32_t offset,
         size_t size,
         void *dest)
@@ -180,7 +221,7 @@ static esp_err_t msc_storage_read_sector(uint32_t lba,
     return (s_storage_handle->read)(sector_size, lba, offset, size, dest);
 }
 
-static esp_err_t msc_storage_write_sector(uint32_t lba,
+static esp_err_t _msc_storage_write_sector(uint32_t lba,
         uint32_t offset,
         size_t size,
         const void *src)
@@ -249,6 +290,59 @@ static esp_err_t _mount(char *drv, FATFS *fs)
     return ret;
 }
 
+#if CONFIG_IDF_TARGET_ESP32S3
+static inline void _switch_buffers(void)
+{
+    msc_write_buffer_t *temp = (msc_write_buffer_t *)write_buffer->p_read_buffer;
+    write_buffer->p_read_buffer = write_buffer->p_write_buffer;
+    write_buffer->p_write_buffer = temp;
+}
+
+static void _write_task(void *param)
+{
+    // Process the data in write_buffer
+    if (write_buffer->p_read_buffer->is_data_pending) {
+        esp_err_t err = _msc_storage_write_sector(
+                            write_buffer->p_read_buffer->lba,
+                            write_buffer->p_read_buffer->offset,
+                            write_buffer->p_read_buffer->bufsize,
+                            (const void *)write_buffer->p_read_buffer->data_buffer
+                        );
+        write_buffer->p_read_buffer->is_data_pending = false;
+        if (err != ESP_OK) {
+            ESP_LOGE(TAG, "Write operation failed: 0x%x", err);
+        }
+    }
+}
+
+static esp_err_t _init_write_buffer(void)
+{
+    assert(!write_buffer);
+    write_buffer = heap_caps_malloc(sizeof(msc_write_buffer_handle_t), MALLOC_CAP_DMA);
+    ESP_RETURN_ON_FALSE(write_buffer, ESP_ERR_NO_MEM, TAG, "could not allocate write buffer");
+
+    memset(write_buffer, 0, sizeof(msc_write_buffer_handle_t));
+
+    write_buffer->p_write_buffer = &write_buffer->buffers[0];
+    write_buffer->p_read_buffer = &write_buffer->buffers[1];
+
+    if (!esp_ptr_dma_capable((const void *)write_buffer->p_write_buffer->data_buffer) ||
+            !esp_ptr_dma_capable((const void *)write_buffer->p_read_buffer->data_buffer)) {
+        ESP_LOGW(TAG, "write buffer is not DMA capable");
+    }
+
+    return ESP_OK;
+}
+
+static void _free_write_buffer(void)
+{
+    if (write_buffer) {
+        free(write_buffer);
+        write_buffer = NULL;
+    }
+}
+#endif
+
 esp_err_t tinyusb_msc_storage_mount(const char *base_path)
 {
     esp_err_t ret = ESP_OK;
@@ -364,13 +458,13 @@ esp_err_t tinyusb_msc_storage_unmount(void)
 uint32_t tinyusb_msc_storage_get_sector_count(void)
 {
     assert(s_storage_handle);
-    return (s_storage_handle->sector_count)();
+    return (s_storage_handle->sector_count);
 }
 
 uint32_t tinyusb_msc_storage_get_sector_size(void)
 {
     assert(s_storage_handle);
-    return (s_storage_handle->sector_size)();
+    return (s_storage_handle->sector_size);
 }
 
 esp_err_t tinyusb_msc_storage_init_spiflash(const tinyusb_msc_spiflash_config_t *config)
@@ -380,13 +474,13 @@ esp_err_t tinyusb_msc_storage_init_spiflash(const tinyusb_msc_spiflash_config_t
     ESP_RETURN_ON_FALSE(s_storage_handle, ESP_ERR_NO_MEM, TAG, "could not allocate new handle for storage");
     s_storage_handle->mount = &_mount_spiflash;
     s_storage_handle->unmount = &_unmount_spiflash;
-    s_storage_handle->sector_count = &_get_sector_count_spiflash;
-    s_storage_handle->sector_size = &_get_sector_size_spiflash;
+    s_storage_handle->wl_handle = config->wl_handle;
+    s_storage_handle->sector_count = _get_sector_count_spiflash();
+    s_storage_handle->sector_size = _get_sector_size_spiflash();
     s_storage_handle->read = &_read_sector_spiflash;
     s_storage_handle->write = &_write_sector_spiflash;
     s_storage_handle->is_fat_mounted = false;
     s_storage_handle->base_path = NULL;
-    s_storage_handle->wl_handle = config->wl_handle;
     // In case the user does not set mount_config.max_files
     // and for backward compatibility with versions <1.4.2
     // max_files is set to 2
@@ -405,7 +499,11 @@ esp_err_t tinyusb_msc_storage_init_spiflash(const tinyusb_msc_spiflash_config_t
         tinyusb_msc_unregister_callback(TINYUSB_MSC_EVENT_PREMOUNT_CHANGED);
     }
 
+#if CONFIG_IDF_TARGET_ESP32S3
+    return _init_write_buffer();
+#else
     return ESP_OK;
+#endif
 }
 
 #if SOC_SDMMC_HOST_SUPPORTED
@@ -416,13 +514,13 @@ esp_err_t tinyusb_msc_storage_init_sdmmc(const tinyusb_msc_sdmmc_config_t *confi
     ESP_RETURN_ON_FALSE(s_storage_handle, ESP_ERR_NO_MEM, TAG, "could not allocate new handle for storage");
     s_storage_handle->mount = &_mount_sdmmc;
     s_storage_handle->unmount = &_unmount_sdmmc;
-    s_storage_handle->sector_count = &_get_sector_count_sdmmc;
-    s_storage_handle->sector_size = &_get_sector_size_sdmmc;
+    s_storage_handle->card = config->card;
+    s_storage_handle->sector_count = _get_sector_count_sdmmc();
+    s_storage_handle->sector_size = _get_sector_size_sdmmc();
     s_storage_handle->read = &_read_sector_sdmmc;
     s_storage_handle->write = &_write_sector_sdmmc;
     s_storage_handle->is_fat_mounted = false;
     s_storage_handle->base_path = NULL;
-    s_storage_handle->card = config->card;
     // In case the user does not set mount_config.max_files
     // and for backward compatibility with versions <1.4.2
     // max_files is set to 2
@@ -441,15 +539,23 @@ esp_err_t tinyusb_msc_storage_init_sdmmc(const tinyusb_msc_sdmmc_config_t *confi
         tinyusb_msc_unregister_callback(TINYUSB_MSC_EVENT_PREMOUNT_CHANGED);
     }
 
+#if CONFIG_IDF_TARGET_ESP32S3
+    return _init_write_buffer();
+#else
     return ESP_OK;
+#endif
 }
 #endif
 
 void tinyusb_msc_storage_deinit(void)
 {
-    assert(s_storage_handle);
-    free(s_storage_handle);
-    s_storage_handle = NULL;
+#if CONFIG_IDF_TARGET_ESP32S3
+    _free_write_buffer();
+#endif
+    if (s_storage_handle) {
+        free(s_storage_handle);
+        s_storage_handle = NULL;
+    }
 }
 
 esp_err_t tinyusb_msc_register_callback(tinyusb_msc_event_type_t event_type,
@@ -508,7 +614,7 @@ void tud_msc_inquiry_cb(uint8_t lun, uint8_t vendor_id[8], uint8_t product_id[16
     (void) lun;
     const char vid[] = "TinyUSB";
     const char pid[] = "Flash Storage";
-    const char rev[] = "0.1";
+    const char rev[] = "0.2";
 
     memcpy(vendor_id, vid, strlen(vid));
     memcpy(product_id, pid, strlen(pid));
@@ -567,7 +673,7 @@ bool tud_msc_start_stop_cb(uint8_t lun, uint8_t power_condition, bool start, boo
 // - Application fill the buffer (up to bufsize) with address contents and return number of read byte.
 int32_t tud_msc_read10_cb(uint8_t lun, uint32_t lba, uint32_t offset, void *buffer, uint32_t bufsize)
 {
-    esp_err_t err = msc_storage_read_sector(lba, offset, bufsize, buffer);
+    esp_err_t err = _msc_storage_read_sector(lba, offset, bufsize, buffer);
     if (err != ESP_OK) {
         ESP_LOGE(TAG, "msc_storage_read_sector failed: 0x%x", err);
         return 0;
@@ -580,11 +686,40 @@ int32_t tud_msc_read10_cb(uint8_t lun, uint32_t lba, uint32_t offset, void *buff
 // - Application write data from buffer to address contents (up to bufsize) and return number of written byte.
 int32_t tud_msc_write10_cb(uint8_t lun, uint32_t lba, uint32_t offset, uint8_t *buffer, uint32_t bufsize)
 {
-    esp_err_t err = msc_storage_write_sector(lba, offset, bufsize, buffer);
+#if CONFIG_IDF_TARGET_ESP32S3
+    // Ensure the buffer can accommodate the incoming data
+    if (bufsize > MSC_WRITE_BUFFER_SIZE) {
+        ESP_LOGE(TAG, "Buffer overflow: incoming data size %lu exceeds buffer size %u", bufsize, MSC_WRITE_BUFFER_SIZE);
+        return -1;
+    }
+
+    // check if previous data are still processed
+    if (write_buffer->p_write_buffer->is_data_pending) {
+        ESP_LOGE(TAG,
+                 "Buffer conflict: previous write operation not completed (LBA=%lu, Offset=%lu, Size=%lu). ",
+                 write_buffer->p_write_buffer->lba,
+                 write_buffer->p_write_buffer->offset,
+                 write_buffer->p_write_buffer->bufsize);
+        return -1;
+    }
+    // Copy data to the buffer
+    memcpy((void *)write_buffer->p_write_buffer->data_buffer, buffer, bufsize);
+    write_buffer->p_write_buffer->lba = lba;
+    write_buffer->p_write_buffer->offset = offset;
+    write_buffer->p_write_buffer->bufsize = bufsize;
+    write_buffer->p_write_buffer->is_data_pending = true;
+    _switch_buffers();
+
+    // Defer execution of the write to the TinyUSB task
+    usbd_defer_func(_write_task, NULL, false);
+#else
+    esp_err_t err = _msc_storage_write_sector(lba, offset, bufsize, buffer);
     if (err != ESP_OK) {
         ESP_LOGE(TAG, "msc_storage_write_sector failed: 0x%x", err);
-        return 0;
+        return -1;
     }
+#endif
+    // Return the number of bytes accepted
     return bufsize;
 }