From 233fa3f948817fa9ab2ff266372b6130dd814602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Tue, 4 Aug 2026 15:38:11 +0200 Subject: [PATCH] fix(sdmmc): release aligned DMA buffer on card deinit --- .../host_sdmmc/main/test_sdio_sdhost.c | 4 +- .../sdmmc_test_cd_wp_common.c | 2 + .../sdmmc_tests/sdmmc_test_begin_end_sd.c | 1 + components/fatfs/vfs/vfs_fat_sdmmc.c | 13 ++- components/sdmmc/include/sdmmc_cmd.h | 24 +++++- components/sdmmc/sdmmc_init.c | 41 +++++++-- .../sdmmc/test_apps/main/test_sdmmc_app.c | 83 +++++++++++++++++++ docs/en/api-reference/storage/sdmmc.rst | 4 +- docs/zh_CN/api-reference/storage/sdmmc.rst | 4 +- .../main/perf_benchmark_example_sd_utils.c | 1 + .../components/cmd_sdmmc/cmd_sdmmc.c | 9 +- 11 files changed, 170 insertions(+), 16 deletions(-) diff --git a/components/esp_driver_sdio/test_apps/sdio/sdio_common_tests/host_sdmmc/main/test_sdio_sdhost.c b/components/esp_driver_sdio/test_apps/sdio/sdio_common_tests/host_sdmmc/main/test_sdio_sdhost.c index 732ac2940f5..70675150735 100644 --- a/components/esp_driver_sdio/test_apps/sdio/sdio_common_tests/host_sdmmc/main/test_sdio_sdhost.c +++ b/components/esp_driver_sdio/test_apps/sdio/sdio_common_tests/host_sdmmc/main/test_sdio_sdhost.c @@ -114,9 +114,7 @@ static void s_master_init(test_sdio_param_t *host_param, essl_handle_t *out_hand static void s_master_deinit(void) { - free(s_card.host.dma_aligned_buffer); - s_card.host.dma_aligned_buffer = 0; - + sdmmc_card_deinit(&s_card); sdmmc_host_deinit(); } diff --git a/components/esp_driver_sdmmc/test_apps/sd_test_utils/components/common_test_flows/sdmmc_test_cd_wp_common.c b/components/esp_driver_sdmmc/test_apps/sd_test_utils/components/common_test_flows/sdmmc_test_cd_wp_common.c index 2c99c52a97c..bc77081b775 100644 --- a/components/esp_driver_sdmmc/test_apps/sd_test_utils/components/common_test_flows/sdmmc_test_cd_wp_common.c +++ b/components/esp_driver_sdmmc/test_apps/sd_test_utils/components/common_test_flows/sdmmc_test_cd_wp_common.c @@ -36,6 +36,7 @@ void sdmmc_test_cd_input(int gpio_cd_num, const sdmmc_host_t* config) usleep(1000); TEST_ESP_OK(sdmmc_card_init(config, card)); + TEST_ESP_OK(sdmmc_card_deinit(card)); free(card); } @@ -72,5 +73,6 @@ void sdmmc_test_wp_input(int gpio_wp_num, const sdmmc_host_t* config) TEST_ESP_OK(sdmmc_read_sectors(card, &data, 0, 1)); free(data); + TEST_ESP_OK(sdmmc_card_deinit(card)); free(card); } diff --git a/components/esp_driver_sdmmc/test_apps/sdmmc/components/sdmmc_tests/sdmmc_test_begin_end_sd.c b/components/esp_driver_sdmmc/test_apps/sdmmc/components/sdmmc_tests/sdmmc_test_begin_end_sd.c index 1beb632b487..1865391a973 100644 --- a/components/esp_driver_sdmmc/test_apps/sdmmc/components/sdmmc_tests/sdmmc_test_begin_end_sd.c +++ b/components/esp_driver_sdmmc/test_apps/sdmmc/components/sdmmc_tests/sdmmc_test_begin_end_sd.c @@ -108,6 +108,7 @@ void sdmmc_test_sd_begin(int slot, int width, int freq_khz, int ddr, sdmmc_card_ void sdmmc_test_sd_end(sdmmc_card_t *card) { + TEST_ESP_OK(sdmmc_card_deinit(card)); TEST_ESP_OK(sdmmc_host_deinit()); // Reset all GPIOs to their default states diff --git a/components/fatfs/vfs/vfs_fat_sdmmc.c b/components/fatfs/vfs/vfs_fat_sdmmc.c index 51b901a3390..fc733e361ba 100644 --- a/components/fatfs/vfs/vfs_fat_sdmmc.c +++ b/components/fatfs/vfs/vfs_fat_sdmmc.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -278,6 +278,7 @@ esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, esp_err_t err; sdmmc_card_t* card = NULL; bool host_inited = false; + bool card_inited = false; // not using ff_memalloc here, as allocation in internal RAM is preferred card = (sdmmc_card_t*) malloc(sizeof(sdmmc_card_t)); @@ -289,6 +290,7 @@ esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, err = esp_vfs_fat_sdmmc_sdcard_init(host_config, slot_config, card, &host_inited); CHECK_EXECUTE_RESULT(err, "esp_vfs_fat_sdmmc_sdcard_init failed"); + card_inited = true; err = esp_vfs_fat_mount_initialized(card, base_path, mount_config); CHECK_EXECUTE_RESULT(err, "esp_vfs_fat_mount_initialized failed"); @@ -299,6 +301,9 @@ cleanup: if (host_inited) { call_host_deinit(host_config); } + if (card_inited) { + sdmmc_card_deinit(card); + } free(card); return err; } @@ -381,6 +386,7 @@ esp_err_t esp_vfs_fat_sdspi_mount(const char* base_path, const sdmmc_host_t* host_config = host_config_input; esp_err_t err; bool host_inited = false; + bool card_inited = false; sdmmc_card_t* card = NULL; // not using ff_memalloc here, as allocation in internal RAM is preferred @@ -393,6 +399,7 @@ esp_err_t esp_vfs_fat_sdspi_mount(const char* base_path, err = esp_vfs_fat_sdspi_sdcard_init(host_config_input, slot_config, card, &host_inited); CHECK_EXECUTE_RESULT(err, "esp_vfs_fat_sdspi_sdcard_init failed"); + card_inited = true; err = esp_vfs_fat_mount_initialized(card, base_path, mount_config); CHECK_EXECUTE_RESULT(err, "esp_vfs_fat_mount_initialized failed"); @@ -404,6 +411,9 @@ cleanup: if (host_inited) { call_host_deinit(host_config); } + if (card_inited) { + sdmmc_card_deinit(card); + } free(card); return err; } @@ -466,6 +476,7 @@ static esp_err_t unmount_card_core(const char *base_path, sdmmc_card_t *card) ff_diskio_unregister(pdrv); if (pdrv_num == 1) { + sdmmc_card_deinit(card); call_host_deinit(&card->host); free(card); } diff --git a/components/sdmmc/include/sdmmc_cmd.h b/components/sdmmc/include/sdmmc_cmd.h index 487edf98bf0..7a730172113 100644 --- a/components/sdmmc/include/sdmmc_cmd.h +++ b/components/sdmmc/include/sdmmc_cmd.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -25,6 +25,7 @@ extern "C" { * @param host pointer to structure defining host controller * @param out_card pointer to structure which will receive information * about the card when the function completes + * @note Call sdmmc_card_deinit before reinitializing the same card structure. * @return * - ESP_OK on success * - One of the error codes from SDMMC host controller @@ -32,6 +33,27 @@ extern "C" { esp_err_t sdmmc_card_init(const sdmmc_host_t* host, sdmmc_card_t* out_card); +/** + * Release resources allocated by sdmmc_card_init + * + * Currently the only persistent heap allocation made by sdmmc_card_init is + * host.dma_aligned_buffer, and only when SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF + * is set. Host DMA, descriptors, and per-transfer bounce buffers are owned + * by the host driver or the I/O path, not by this function. + * + * This function does not deinitialize the host or free the card structure. + * It is safe to call more than once on the same card. + * Do not also free dma_aligned_buffer if the flag was set; if the application + * provided that buffer without the flag, it remains caller-owned. + * + * @param card pointer to card information structure initialized using + * sdmmc_card_init + * @return + * - ESP_OK on success + * - ESP_ERR_INVALID_ARG if card is NULL + */ +esp_err_t sdmmc_card_deinit(sdmmc_card_t* card); + /** * @brief Print information about the card to a stream * @param stream stream obtained using fopen or fdopen diff --git a/components/sdmmc/sdmmc_init.c b/components/sdmmc/sdmmc_init.c index 428e9d29743..f60aabcf83d 100644 --- a/components/sdmmc/sdmmc_init.c +++ b/components/sdmmc/sdmmc_init.c @@ -24,10 +24,10 @@ static const char* TAG = "sdmmc_init"; #define SDMMC_INIT_STEP(condition, function) \ do { \ if ((condition)) { \ - esp_err_t err = (function)(card); \ - if (err != ESP_OK) { \ - ESP_LOGD(TAG, "%s: %s returned 0x%x", __func__, #function, err); \ - return err; \ + ret = (function)(card); \ + if (ret != ESP_OK) { \ + ESP_LOGD(TAG, "%s: %s returned 0x%x", __func__, #function, ret); \ + goto cleanup; \ } \ } \ } while(0); @@ -35,10 +35,10 @@ static const char* TAG = "sdmmc_init"; #define SDMMC_INIT_STEP_PARAM(condition, function, param) \ do { \ if ((condition)) { \ - esp_err_t err = (function)(card, param); \ - if (err != ESP_OK) { \ - ESP_LOGD(TAG, "%s: %s returned 0x%x", __func__, #function, err); \ - return err; \ + ret = (function)(card, param); \ + if (ret != ESP_OK) { \ + ESP_LOGD(TAG, "%s: %s returned 0x%x", __func__, #function, ret); \ + goto cleanup; \ } \ } \ } while(0); @@ -46,6 +46,12 @@ static const char* TAG = "sdmmc_init"; esp_err_t sdmmc_card_init(const sdmmc_host_t* config, sdmmc_card_t* card) { esp_err_t ret = ESP_FAIL; + + if ((config->flags & SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF) && config->dma_aligned_buffer != NULL) { + ESP_LOGE(TAG, "%s: dma_aligned_buffer must be NULL when SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF is set", __func__); + return ESP_ERR_INVALID_STATE; + } + memset(card, 0, sizeof(*card)); memcpy(&card->host, config, sizeof(*config)); @@ -189,5 +195,24 @@ esp_err_t sdmmc_card_init(const sdmmc_host_t* config, sdmmc_card_t* card) SDMMC_INIT_STEP_PARAM(is_sdio, sdmmc_io_init_check_card_cap, &card_cap); #endif + return ESP_OK; + +cleanup: + sdmmc_card_deinit(card); + return ret; +} + +esp_err_t sdmmc_card_deinit(sdmmc_card_t* card) +{ + ESP_RETURN_ON_FALSE(card, ESP_ERR_INVALID_ARG, TAG, "invalid argument: null pointer"); + + // sdmmc_card_init allocates at most this one buffer (see sdmmc_allocate_aligned_buf). + // Flag set => we allocated it and must free it. Flag clear => caller owns the pointer. + // Other DMA/bounce buffers are not card-init resources. + if (card->host.flags & SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF) { + free(card->host.dma_aligned_buffer); + card->host.dma_aligned_buffer = NULL; + } + return ESP_OK; } diff --git a/components/sdmmc/test_apps/main/test_sdmmc_app.c b/components/sdmmc/test_apps/main/test_sdmmc_app.c index 3dee78e0621..6f9676a5137 100644 --- a/components/sdmmc/test_apps/main/test_sdmmc_app.c +++ b/components/sdmmc/test_apps/main/test_sdmmc_app.c @@ -14,11 +14,94 @@ #include "sdmmc_cmd.h" #include "sdmmc_test_begin_end_sd.h" #include "sdmmc_test_rw_common.h" +#include "esp_private/sdmmc_common.h" #include "esp_blockdev.h" #include "esp_private/sdmmc_blockdev.h" #define TEST_MEMORY_LEAK_THRESHOLD (200) +static size_t test_get_bus_width(int slot) +{ + (void)slot; + return 1; +} + +static bool test_check_buffer_alignment(int slot, const void *buf, size_t size) +{ + (void)slot; + (void)buf; + (void)size; + return true; +} + +static esp_err_t test_fail_transaction(int slot, sdmmc_command_t *cmdinfo) +{ + (void)slot; + (void)cmdinfo; + return ESP_ERR_INVALID_STATE; +} + +TEST_CASE("sdmmc card deinit releases allocated aligned buffer", "[sdmmc]") +{ + sdmmc_card_t card = {}; + card.host.flags = SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF; + card.host.dma_aligned_buffer = malloc(SDMMC_IO_BLOCK_SIZE); + TEST_ASSERT_NOT_NULL(card.host.dma_aligned_buffer); + + TEST_ESP_OK(sdmmc_card_deinit(&card)); + TEST_ASSERT_NULL(card.host.dma_aligned_buffer); + TEST_ESP_OK(sdmmc_card_deinit(&card)); + + void *caller_owned_buffer = malloc(SDMMC_IO_BLOCK_SIZE); + TEST_ASSERT_NOT_NULL(caller_owned_buffer); + card.host.flags = 0; + card.host.dma_aligned_buffer = caller_owned_buffer; + TEST_ESP_OK(sdmmc_card_deinit(&card)); + TEST_ASSERT_EQUAL_PTR(caller_owned_buffer, card.host.dma_aligned_buffer); + free(caller_owned_buffer); + + TEST_ESP_ERR(ESP_ERR_INVALID_ARG, sdmmc_card_deinit(NULL)); +} + +TEST_CASE("sdmmc card init releases allocated aligned buffer on failure", "[sdmmc]") +{ + sdmmc_host_t host = { + .flags = SDMMC_HOST_FLAG_1BIT | SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF, + .get_bus_width = test_get_bus_width, + .do_transaction = test_fail_transaction, + .check_buffer_alignment = test_check_buffer_alignment, + }; + sdmmc_card_t card; + + TEST_ESP_ERR(ESP_ERR_INVALID_STATE, sdmmc_card_init(&host, &card)); + TEST_ASSERT_NULL(card.host.dma_aligned_buffer); +} + +TEST_CASE("sdmmc card init rejects preset buffer with ALLOC_ALIGNED_BUF", "[sdmmc]") +{ + void *caller_owned_buffer = malloc(SDMMC_IO_BLOCK_SIZE); + TEST_ASSERT_NOT_NULL(caller_owned_buffer); + + sdmmc_host_t host = { + .flags = SDMMC_HOST_FLAG_1BIT | SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF, + .get_bus_width = test_get_bus_width, + .do_transaction = test_fail_transaction, + .check_buffer_alignment = test_check_buffer_alignment, + .dma_aligned_buffer = caller_owned_buffer, + }; + sdmmc_card_t card; + memset(&card, 0xAA, sizeof(card)); + + TEST_ESP_ERR(ESP_ERR_INVALID_STATE, sdmmc_card_init(&host, &card)); + /* card must be left untouched, and the caller keeps ownership of the buffer */ + TEST_ASSERT_EQUAL_PTR(caller_owned_buffer, host.dma_aligned_buffer); + for (size_t i = 0; i < sizeof(card); i++) { + TEST_ASSERT_EQUAL_UINT8(0xAA, ((uint8_t *)&card)[i]); + } + + free(caller_owned_buffer); +} + TEST_CASE("sdmmc blockdev converts byte ranges to sectors", "[sdmmc]") { size_t start_sector; diff --git a/docs/en/api-reference/storage/sdmmc.rst b/docs/en/api-reference/storage/sdmmc.rst index 0ad171c820a..ba3d21cb272 100644 --- a/docs/en/api-reference/storage/sdmmc.rst +++ b/docs/en/api-reference/storage/sdmmc.rst @@ -53,8 +53,10 @@ Using API with SD Memory Cards :SOC_GPSPI_SUPPORTED: - To initialize the SDSPI host, call the host driver functions, e.g., :cpp:func:`sdspi_host_init`, :cpp:func:`sdspi_host_init_slot`. - To initialize the card, call :cpp:func:`sdmmc_card_init` and pass to it the parameters ``host`` - the host driver information, and ``card`` - a pointer to the structure :cpp:class:`sdmmc_card_t` which will be filled with information about the card when the function completes. - To read and write sectors of the card, use :cpp:func:`sdmmc_read_sectors` and :cpp:func:`sdmmc_write_sectors` respectively and pass to it the parameter ``card`` - a pointer to the card information structure. + - When the card is no longer used, call :cpp:func:`sdmmc_card_deinit` to release resources allocated by :cpp:func:`sdmmc_card_init`. + - Then call the host driver function to disable the host peripheral and free the resources allocated by the host driver (``sdmmc_host_deinit`` for SDMMC or ``sdspi_host_deinit`` for SDSPI). If the application allocated the :cpp:class:`sdmmc_card_t` structure, free it after deinitializing the card and host. - - If the card is not used anymore, call the host driver function to disable the host peripheral and free the resources allocated by the driver (``sdmmc_host_deinit`` for SDMMC or ``sdspi_host_deinit`` for SDSPI). +:cpp:func:`sdmmc_card_deinit` frees :cpp:member:`sdmmc_host_t::dma_aligned_buffer` only when :c:macro:`SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF` is set. If the application provides a pre-allocated buffer without setting this flag, the application retains ownership of the buffer and must free it after calling :cpp:func:`sdmmc_card_deinit`. Unaligned Buffer Performance ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/docs/zh_CN/api-reference/storage/sdmmc.rst b/docs/zh_CN/api-reference/storage/sdmmc.rst index 172204aa6cd..b22eb21aa59 100644 --- a/docs/zh_CN/api-reference/storage/sdmmc.rst +++ b/docs/zh_CN/api-reference/storage/sdmmc.rst @@ -53,8 +53,10 @@ SD/SDIO/MMC 驱动支持 SD 存储器、SDIO 卡和 eMMC 芯片。这是一个 :SOC_GPSPI_SUPPORTED: - 初始化 SDSPI 主机，请调用主机驱动函数，例如 :cpp:func:`sdspi_host_init` 和 :cpp:func:`sdspi_host_init_slot`。 - 初始化卡，请调用 :cpp:func:`sdmmc_card_init`，并将参数 ``host`` （主机驱动信息）和参数 ``card`` （指向 :cpp:class:`sdmmc_card_t` 结构体的指针）传递给此函数。函数运行结束后，将会向 :cpp:class:`sdmmc_card_t` 结构体填充该卡的信息。 - 读取或写入卡的扇区，请分别调用 :cpp:func:`sdmmc_read_sectors` 和 :cpp:func:`sdmmc_write_sectors`，并将参数 ``card`` （指向卡信息结构的指针）传递给函数。 + - 如果不再使用该卡，请调用 :cpp:func:`sdmmc_card_deinit`，释放 :cpp:func:`sdmmc_card_init` 分配的资源。 + - 然后调用主机驱动函数以禁用主机外设并释放主机驱动分配的资源（SDMMC 使用 ``sdmmc_host_deinit``，SDSPI 使用 ``sdspi_host_deinit``）。如果应用程序分配了 :cpp:class:`sdmmc_card_t` 结构体，请在反初始化卡和主机后释放该结构体。 - - 如果不再使用该卡，请调用主机驱动函数，例如 ``sdmmc_host_deinit`` 或 ``sdspi_host_deinit``，以禁用SDMMC 主机外设或 SDSPI 主机外设，并释放驱动程序分配的资源。 +仅当设置了 :c:macro:`SDMMC_HOST_FLAG_ALLOC_ALIGNED_BUF` 时，:cpp:func:`sdmmc_card_deinit` 才会释放 :cpp:member:`sdmmc_host_t::dma_aligned_buffer`。如果应用程序提供了预分配的 buffer 但未设置此标志，则应用程序保留 buffer 所有权，并必须在调用 :cpp:func:`sdmmc_card_deinit` 后将其释放。 未对齐 buffer 性能 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/examples/storage/perf_benchmark/main/perf_benchmark_example_sd_utils.c b/examples/storage/perf_benchmark/main/perf_benchmark_example_sd_utils.c index 378080490e6..8f45d384460 100644 --- a/examples/storage/perf_benchmark/main/perf_benchmark_example_sd_utils.c +++ b/examples/storage/perf_benchmark/main/perf_benchmark_example_sd_utils.c @@ -174,6 +174,7 @@ esp_err_t init_sd_card(sdmmc_card_t **out_card) void deinit_sd_card(sdmmc_card_t **card) { + ESP_ERROR_CHECK(sdmmc_card_deinit(*card)); // Unmount SD card #ifdef CONFIG_EXAMPLE_USE_SDMMC sdmmc_host_deinit(); diff --git a/tools/test_apps/storage/sdmmc_console/components/cmd_sdmmc/cmd_sdmmc.c b/tools/test_apps/storage/sdmmc_console/components/cmd_sdmmc/cmd_sdmmc.c index 4a41fbd7c89..eaa8e1937ef 100644 --- a/tools/test_apps/storage/sdmmc_console/components/cmd_sdmmc/cmd_sdmmc.c +++ b/tools/test_apps/storage/sdmmc_console/components/cmd_sdmmc/cmd_sdmmc.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 * @@ -163,6 +163,8 @@ void register_sdmmc_common(void) static int sdmmc_card_init_handler(int argc, char **argv) { + static bool card_initialized; + /* In case the card was already initialized: reset the host to the default settings. * This could be moved to sdmmc_card_init. */ @@ -173,8 +175,13 @@ static int sdmmc_card_init_handler(int argc, char **argv) s_host.set_bus_ddr_mode(s_host.slot, false); ESP_RETURN_ON_ERROR(err, TAG, "set_bus_ddr_mode: error 0x%x (%s)", err, esp_err_to_name(err)); + if (card_initialized) { + ESP_RETURN_ON_ERROR(sdmmc_card_deinit(&s_card), TAG, "sdmmc_card_deinit failed"); + card_initialized = false; + } err = sdmmc_card_init(&s_host, &s_card); ESP_RETURN_ON_ERROR(err, TAG, "sdmmc_card_init: error 0x%x (%s)", err, esp_err_to_name(err)); + card_initialized = true; return 0; }