From 82dac1f1bdc438da816d82aac64b70ae932d1838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Mon, 25 Nov 2024 17:23:28 +0100 Subject: [PATCH 1/2] fix(fatfs): Mistake in Kconfig for FATFS_USE_DYN_BUFFERS --- components/fatfs/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/fatfs/Kconfig b/components/fatfs/Kconfig index d40d8aa236..1a35dc4ef4 100644 --- a/components/fatfs/Kconfig +++ b/components/fatfs/Kconfig @@ -313,8 +313,7 @@ menu "FAT Filesystem support" config FATFS_USE_DYN_BUFFERS bool "Use dynamic buffers" - depends on CONFIG_WL_SECTOR_SIZE_4096 - default y + default n help If enabled, the buffers used by FATFS will be allocated separately from the rest of the structure. This option is useful when using multiple FATFS instances with different From b1997ebab672d52ec6fdc9da2d7a3ea856d75cda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Rohl=C3=ADnek?= Date: Fri, 24 Jan 2025 13:56:37 +0100 Subject: [PATCH 2/2] feat(storage/fatfs): add dynamic buffer usage test --- .../fatfs/test_apps/.build-test-rules.yml | 8 ++ components/fatfs/test_apps/README.md | 4 + .../test_apps/dyn_buffers/CMakeLists.txt | 7 ++ .../fatfs/test_apps/dyn_buffers/README.md | 8 ++ .../test_apps/dyn_buffers/main/CMakeLists.txt | 3 + .../dyn_buffers/main/test_fatfs_dyn_buffers.c | 101 ++++++++++++++++++ .../test_apps/dyn_buffers/partitions.csv | 5 + .../dyn_buffers/pytest_fatfs_dyn_buffers.py | 22 ++++ .../dyn_buffers/sdkconfig.ci.dyn_buffers | 1 + .../sdkconfig.ci.no_dyn_buffers | 0 .../test_apps/dyn_buffers/sdkconfig.defaults | 14 +++ .../flash_wl/pytest_fatfs_flash_wl.py | 2 +- .../flash_wl/sdkconfig.ci.dyn_buffers | 1 + 13 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 components/fatfs/test_apps/dyn_buffers/CMakeLists.txt create mode 100644 components/fatfs/test_apps/dyn_buffers/README.md create mode 100644 components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt create mode 100644 components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c create mode 100644 components/fatfs/test_apps/dyn_buffers/partitions.csv create mode 100644 components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py create mode 100644 components/fatfs/test_apps/dyn_buffers/sdkconfig.ci.dyn_buffers rename components/fatfs/test_apps/{flash_wl => dyn_buffers}/sdkconfig.ci.no_dyn_buffers (100%) create mode 100644 components/fatfs/test_apps/dyn_buffers/sdkconfig.defaults create mode 100644 components/fatfs/test_apps/flash_wl/sdkconfig.ci.dyn_buffers diff --git a/components/fatfs/test_apps/.build-test-rules.yml b/components/fatfs/test_apps/.build-test-rules.yml index 3b328782d5..86cfac0de4 100644 --- a/components/fatfs/test_apps/.build-test-rules.yml +++ b/components/fatfs/test_apps/.build-test-rules.yml @@ -1,5 +1,13 @@ # Documentation: .gitlab/ci/README.md#manifest-file-to-control-the-buildtest-apps +components/fatfs/test_apps/dyn_buffers: + disable_test: + - if: IDF_TARGET != "esp32" + reason: only one target required + + depends_components: + - fatfs + components/fatfs/test_apps/flash_ro: disable_test: - if: IDF_TARGET not in ["esp32", "esp32c3"] diff --git a/components/fatfs/test_apps/README.md b/components/fatfs/test_apps/README.md index 2140e8c4fe..78a9dcd9de 100644 --- a/components/fatfs/test_apps/README.md +++ b/components/fatfs/test_apps/README.md @@ -1,3 +1,6 @@ +| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C5 | ESP32-C6 | ESP32-C61 | ESP32-H2 | ESP32-P4 | ESP32-S2 | ESP32-S3 | +| ----------------- | ----- | -------- | -------- | -------- | -------- | --------- | -------- | -------- | -------- | -------- | + # fatfs component target tests This directory contains tests for `fatfs` component which are run on chip targets. @@ -9,6 +12,7 @@ Fatfs tests can be executed with different `diskio` backends: `diskio_sdmmc` (SD - [sdcard](sdcard/) — runs fatfs tests with an SD card over SDMMC or SDSPI interface - [flash_wl](flash_wl/) - runs fatfs test in a wear_levelling partition in SPI flash - [flash_ro](flash_ro/) - runs fatfs test in a read-only (no wear levelling) partition in SPI flash +- [dyn_buffers](dyn_buffers/) - check if enabling dynamic buffers in FATFS has an effect These test apps define: - test functions diff --git a/components/fatfs/test_apps/dyn_buffers/CMakeLists.txt b/components/fatfs/test_apps/dyn_buffers/CMakeLists.txt new file mode 100644 index 0000000000..4e6f01d2ec --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/CMakeLists.txt @@ -0,0 +1,7 @@ +# The following five lines of boilerplate have to be in your project's +# CMakeLists in this exact order for cmake to work correctly +cmake_minimum_required(VERSION 3.16) + +include($ENV{IDF_PATH}/tools/cmake/project.cmake) +idf_build_set_property(MINIMAL_BUILD ON) +project(dyn_buffers) diff --git a/components/fatfs/test_apps/dyn_buffers/README.md b/components/fatfs/test_apps/dyn_buffers/README.md new file mode 100644 index 0000000000..261aeee2df --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/README.md @@ -0,0 +1,8 @@ +| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C5 | ESP32-C6 | ESP32-C61 | ESP32-H2 | ESP32-H21 | ESP32-H4 | ESP32-P4 | ESP32-S2 | ESP32-S3 | +| ----------------- | ----- | -------- | -------- | -------- | -------- | --------- | -------- | --------- | -------- | -------- | -------- | -------- | + +This test app checks if `CONFIG_FATFS_USE_DYN_BUFFERS` has any effect. + +These tests should be possible to run on any ESP development board, not extra hardware is necessary. + +See [../README.md](../README.md) for more information about FATFS test apps. diff --git a/components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt b/components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt new file mode 100644 index 0000000000..c3cd1aa78c --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt @@ -0,0 +1,3 @@ +idf_component_register(SRCS "test_fatfs_dyn_buffers.c" + INCLUDE_DIRS "." + REQUIRES wear_levelling fatfs vfs) diff --git a/components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c b/components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c new file mode 100644 index 0000000000..c945d97ba9 --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c @@ -0,0 +1,101 @@ +/* + * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Unlicense OR CC0-1.0 + */ +#include +#include +#include +#include +#include "wear_levelling.h" +#include "esp_partition.h" +#include "esp_vfs.h" +#include "esp_vfs_fat.h" +#include "esp_heap_caps.h" +#include "sdkconfig.h" +#include "ff.h" +#include "esp_debug_helpers.h" + +static const char* TAG = "Test dynamic buffers"; + +static volatile bool g_alloc_count_enable = false; +static volatile int g_buffer_alloc_count = 0; + +static esp_vfs_fat_mount_config_t g_mount_config = { + .format_if_mount_failed = true, + .max_files = 5, +}; + +void esp_heap_trace_alloc_hook(void* ptr, size_t size, uint32_t caps) +{ + (void) ptr; + (void) caps; + + if (!g_alloc_count_enable) { + return; + } + + // This will work only on SPI flash + // Different flash types might break this check + if (size == FF_MAX_SS) { + g_buffer_alloc_count++; + } +} + +void app_main(void) +{ + esp_err_t err = ESP_OK; + + wl_handle_t wl_handle; + + err = esp_vfs_fat_spiflash_format_cfg_rw_wl("/spiflash", NULL, &g_mount_config); + + ESP_LOGI(TAG, "Mounting FATFS"); + + g_mount_config.format_if_mount_failed = false, + + g_alloc_count_enable = true; + + err = esp_vfs_fat_spiflash_mount_rw_wl("/spiflash", NULL, &g_mount_config, &wl_handle); + if (err != ESP_OK) { + ESP_LOGE(TAG, "FATFS mount failed with error: %d", err); + return; + } + + ESP_LOGI(TAG, "Mounted"); + + int fd = open("/spiflash/test.txt", O_RDWR|O_CREAT); + if (fd < 0) { + ESP_LOGE(TAG, "Failed opening file"); + } + + close(fd); + + g_alloc_count_enable = false; + + ESP_LOGI(TAG, "Unmounting FATFS"); + + esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash", wl_handle); + + ESP_LOGI(TAG, "Unmounted"); + + ESP_LOGI(TAG, "Allocs called:\n\tBuffer: %d" + , g_buffer_alloc_count); + +#if CONFIG_FATFS_USE_DYN_BUFFERS + + if (g_buffer_alloc_count != 2) { + ESP_LOGE(TAG, "FATFS buffer should have been allocated once for each context (file and fatfs)"); + return; + } +#else + + if (g_buffer_alloc_count != 0) { + ESP_LOGE(TAG, "FATFS buffer should not have been allocated"); + return; + } + +#endif + + ESP_LOGI(TAG, "Done"); +} diff --git a/components/fatfs/test_apps/dyn_buffers/partitions.csv b/components/fatfs/test_apps/dyn_buffers/partitions.csv new file mode 100644 index 0000000000..d68a9de011 --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/partitions.csv @@ -0,0 +1,5 @@ +# Name, Type, SubType, Offset, Size, Flags +factory, app, factory, 0x10000, 768k, +storage, data, fat, , 528k, +storage2, data, fat, , 528k, +storage1, data, fat, , 32k, diff --git a/components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py b/components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py new file mode 100644 index 0000000000..a60ac87e1f --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py @@ -0,0 +1,22 @@ +# SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: CC0-1.0 +import pytest +from pytest_embedded import Dut +from pytest_embedded_idf.utils import idf_parametrize + + +@pytest.mark.generic +@pytest.mark.parametrize( + 'config', + [ + 'dyn_buffers', + 'no_dyn_buffers', + ], +) +@idf_parametrize('target', ['esp32'], indirect=['target']) +def test_fatfs_flash_dyn_buffers(config: str, dut: Dut) -> None: + dut.expect('Mounting FATFS') + dut.expect('Mounted') + dut.expect('Unmounting FATFS') + dut.expect('Unmounted') + dut.expect('Done') diff --git a/components/fatfs/test_apps/dyn_buffers/sdkconfig.ci.dyn_buffers b/components/fatfs/test_apps/dyn_buffers/sdkconfig.ci.dyn_buffers new file mode 100644 index 0000000000..380dafbff8 --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/sdkconfig.ci.dyn_buffers @@ -0,0 +1 @@ +CONFIG_FATFS_USE_DYN_BUFFERS=y diff --git a/components/fatfs/test_apps/flash_wl/sdkconfig.ci.no_dyn_buffers b/components/fatfs/test_apps/dyn_buffers/sdkconfig.ci.no_dyn_buffers similarity index 100% rename from components/fatfs/test_apps/flash_wl/sdkconfig.ci.no_dyn_buffers rename to components/fatfs/test_apps/dyn_buffers/sdkconfig.ci.no_dyn_buffers diff --git a/components/fatfs/test_apps/dyn_buffers/sdkconfig.defaults b/components/fatfs/test_apps/dyn_buffers/sdkconfig.defaults new file mode 100644 index 0000000000..8b57aacd09 --- /dev/null +++ b/components/fatfs/test_apps/dyn_buffers/sdkconfig.defaults @@ -0,0 +1,14 @@ +# General options for additional checks +CONFIG_HEAP_POISONING_COMPREHENSIVE=y +CONFIG_COMPILER_WARN_WRITE_STRINGS=y +CONFIG_BOOTLOADER_LOG_LEVEL_WARN=y +CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK=y +CONFIG_COMPILER_STACK_CHECK_MODE_STRONG=y +CONFIG_COMPILER_STACK_CHECK=y + +# use custom partition table +CONFIG_PARTITION_TABLE_CUSTOM=y +CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv" + +# to measure allocations +CONFIG_HEAP_USE_HOOKS=y diff --git a/components/fatfs/test_apps/flash_wl/pytest_fatfs_flash_wl.py b/components/fatfs/test_apps/flash_wl/pytest_fatfs_flash_wl.py index add0149f51..a9863635bf 100644 --- a/components/fatfs/test_apps/flash_wl/pytest_fatfs_flash_wl.py +++ b/components/fatfs/test_apps/flash_wl/pytest_fatfs_flash_wl.py @@ -13,7 +13,7 @@ from pytest_embedded_idf.utils import idf_parametrize 'release', 'fastseek', 'auto_fsync', - 'no_dyn_buffers', + 'dyn_buffers', ], ) @idf_parametrize('target', ['esp32', 'esp32c3'], indirect=['target']) diff --git a/components/fatfs/test_apps/flash_wl/sdkconfig.ci.dyn_buffers b/components/fatfs/test_apps/flash_wl/sdkconfig.ci.dyn_buffers new file mode 100644 index 0000000000..380dafbff8 --- /dev/null +++ b/components/fatfs/test_apps/flash_wl/sdkconfig.ci.dyn_buffers @@ -0,0 +1 @@ +CONFIG_FATFS_USE_DYN_BUFFERS=y