From a9e5dac6ccab1c878cb5806c74399f16e1f784ea Mon Sep 17 00:00:00 2001 From: Armando Date: Sun, 25 Jun 2023 15:20:15 +0800 Subject: [PATCH 1/3] spiflash: fix not calling on_spi_acquired when CONFIG_SPI_FLASH_SHARE_SPI1_BUS issue --- components/spi_flash/spi_flash_os_func_app.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/components/spi_flash/spi_flash_os_func_app.c b/components/spi_flash/spi_flash_os_func_app.c index 6d50eb0860..890059234e 100644 --- a/components/spi_flash/spi_flash_os_func_app.c +++ b/components/spi_flash/spi_flash_os_func_app.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -92,15 +92,16 @@ static IRAM_ATTR esp_err_t spi_end(void *arg) static IRAM_ATTR esp_err_t spi1_start(void *arg) { + esp_err_t ret = ESP_OK; #if CONFIG_SPI_FLASH_SHARE_SPI1_BUS //use the lock to disable the cache and interrupts before using the SPI bus - return spi_start(arg); + ret = spi_start(arg); #else //directly disable the cache and interrupts when lock is not used cache_disable(NULL); - on_spi1_acquired((spi1_app_func_arg_t*)arg); - return ESP_OK; #endif + on_spi1_acquired((spi1_app_func_arg_t*)arg); + return ret; } static IRAM_ATTR esp_err_t spi1_end(void *arg) From 29ff838f5acb5c3c03c2d0e5ae1dfe919b2d3c62 Mon Sep 17 00:00:00 2001 From: Armando Date: Sun, 25 Jun 2023 15:22:42 +0800 Subject: [PATCH 2/3] spi_flash: fix concurrency issue when calling esp_flash apis under xip_psram or auto_suspen --- components/spi_flash/spi_flash_os_func_app.c | 28 ++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/components/spi_flash/spi_flash_os_func_app.c b/components/spi_flash/spi_flash_os_func_app.c index 890059234e..fb0bad6ada 100644 --- a/components/spi_flash/spi_flash_os_func_app.c +++ b/components/spi_flash/spi_flash_os_func_app.c @@ -21,9 +21,13 @@ #include "driver/spi_common_internal.h" -#define SPI_FLASH_CACHE_NO_DISABLE (CONFIG_SPI_FLASH_AUTO_SUSPEND || (CONFIG_SPIRAM_FETCH_INSTRUCTIONS && CONFIG_SPIRAM_RODATA)) +#define SPI_FLASH_CACHE_NO_DISABLE (CONFIG_SPI_FLASH_AUTO_SUSPEND || (CONFIG_SPIRAM_FETCH_INSTRUCTIONS && CONFIG_SPIRAM_RODATA) || CONFIG_APP_BUILD_TYPE_ELF_RAM) static const char TAG[] = "spi_flash"; +#if SPI_FLASH_CACHE_NO_DISABLE +static _lock_t s_spi1_flash_mutex; +#endif // #if SPI_FLASH_CACHE_NO_DISABLE + /* * OS functions providing delay service and arbitration among chips, and with the cache. * @@ -58,19 +62,17 @@ static inline IRAM_ATTR void on_spi1_acquired(spi1_app_func_arg_t* ctx); static inline IRAM_ATTR void on_spi1_yielded(spi1_app_func_arg_t* ctx); static inline IRAM_ATTR bool on_spi1_check_yield(spi1_app_func_arg_t* ctx); +#if !SPI_FLASH_CACHE_NO_DISABLE IRAM_ATTR static void cache_enable(void* arg) { -#if !SPI_FLASH_CACHE_NO_DISABLE g_flash_guard_default_ops.end(); -#endif } IRAM_ATTR static void cache_disable(void* arg) { -#if !SPI_FLASH_CACHE_NO_DISABLE g_flash_guard_default_ops.start(); -#endif } +#endif //#if !SPI_FLASH_CACHE_NO_DISABLE static IRAM_ATTR esp_err_t spi_start(void *arg) { @@ -93,9 +95,19 @@ static IRAM_ATTR esp_err_t spi_end(void *arg) static IRAM_ATTR esp_err_t spi1_start(void *arg) { esp_err_t ret = ESP_OK; + /** + * There are three ways for ESP Flash API lock: + * 1. spi bus lock, this is used when SPI1 is shared with GPSPI Master Driver + * 2. mutex, this is used when the Cache isn't need to be disabled. + * 3. cache lock (from cache_utils.h), this is used when we need to disable Cache to avoid access from SPI0 + * + * From 1 to 3, the lock efficiency decreases. + */ #if CONFIG_SPI_FLASH_SHARE_SPI1_BUS //use the lock to disable the cache and interrupts before using the SPI bus ret = spi_start(arg); +#elif SPI_FLASH_CACHE_NO_DISABLE + _lock_acquire(&s_spi1_flash_mutex); #else //directly disable the cache and interrupts when lock is not used cache_disable(NULL); @@ -107,8 +119,14 @@ static IRAM_ATTR esp_err_t spi1_start(void *arg) static IRAM_ATTR esp_err_t spi1_end(void *arg) { esp_err_t ret = ESP_OK; + + /** + * There are three ways for ESP Flash API lock, see `spi1_start` + */ #if CONFIG_SPI_FLASH_SHARE_SPI1_BUS ret = spi_end(arg); +#elif SPI_FLASH_CACHE_NO_DISABLE + _lock_release(&s_spi1_flash_mutex); #else cache_enable(NULL); #endif From da5acfdca62776ef74220359be9ce31ba1ac44f5 Mon Sep 17 00:00:00 2001 From: Armando Date: Mon, 26 Jun 2023 19:37:06 +0800 Subject: [PATCH 3/3] spi_flash: rename spi_flash_os_func_app: spi_start, spi_end spi_start -> s_acquire_spi_bus_lock spi_end -> s_release_spi_bus_lock --- components/spi_flash/spi_flash_os_func_app.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/spi_flash/spi_flash_os_func_app.c b/components/spi_flash/spi_flash_os_func_app.c index fb0bad6ada..46147f8a95 100644 --- a/components/spi_flash/spi_flash_os_func_app.c +++ b/components/spi_flash/spi_flash_os_func_app.c @@ -74,7 +74,7 @@ IRAM_ATTR static void cache_disable(void* arg) } #endif //#if !SPI_FLASH_CACHE_NO_DISABLE -static IRAM_ATTR esp_err_t spi_start(void *arg) +static IRAM_ATTR esp_err_t acquire_spi_bus_lock(void *arg) { spi_bus_lock_dev_handle_t dev_lock = ((app_func_arg_t *)arg)->dev_lock; @@ -87,7 +87,7 @@ static IRAM_ATTR esp_err_t spi_start(void *arg) return ESP_OK; } -static IRAM_ATTR esp_err_t spi_end(void *arg) +static IRAM_ATTR esp_err_t release_spi_bus_lock(void *arg) { return spi_bus_lock_acquire_end(((app_func_arg_t *)arg)->dev_lock); } @@ -105,7 +105,7 @@ static IRAM_ATTR esp_err_t spi1_start(void *arg) */ #if CONFIG_SPI_FLASH_SHARE_SPI1_BUS //use the lock to disable the cache and interrupts before using the SPI bus - ret = spi_start(arg); + ret = acquire_spi_bus_lock(arg); #elif SPI_FLASH_CACHE_NO_DISABLE _lock_acquire(&s_spi1_flash_mutex); #else @@ -124,7 +124,7 @@ static IRAM_ATTR esp_err_t spi1_end(void *arg) * There are three ways for ESP Flash API lock, see `spi1_start` */ #if CONFIG_SPI_FLASH_SHARE_SPI1_BUS - ret = spi_end(arg); + ret = release_spi_bus_lock(arg); #elif SPI_FLASH_CACHE_NO_DISABLE _lock_release(&s_spi1_flash_mutex); #else @@ -220,8 +220,8 @@ static const DRAM_ATTR esp_flash_os_functions_t esp_flash_spi1_default_os_functi }; static const esp_flash_os_functions_t esp_flash_spi23_default_os_functions = { - .start = spi_start, - .end = spi_end, + .start = acquire_spi_bus_lock, + .end = release_spi_bus_lock, .delay_us = delay_us, .get_temp_buffer = get_buffer_malloc, .release_temp_buffer = release_buffer_malloc,