From 03bb2774d9201aa56e408a760ccf865a20d9c965 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Fri, 29 May 2020 21:52:48 +0200 Subject: [PATCH] spi_flash: don't call vTaskDelay in non-os context Fixes regression in core dump, when a crash happens in interrupt context. --- components/spi_flash/esp_flash_api.c | 4 +++- components/spi_flash/flash_ops.c | 22 ++++++++++++++----- components/spi_flash/include/esp_flash.h | 3 +++ components/spi_flash/include/esp_spi_flash.h | 5 +++++ components/spi_flash/spi_flash_os_func_app.c | 14 +++++++++++- components/spi_flash/spi_flash_os_func_noos.c | 1 + 6 files changed, 42 insertions(+), 7 deletions(-) diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 5bcdc20b0..11c5c9b53 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -372,7 +372,9 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui no_yield_time_us += (esp_timer_get_time() - start_time_us); if (no_yield_time_us / 1000 >= CONFIG_SPI_FLASH_ERASE_YIELD_DURATION_MS) { no_yield_time_us = 0; - vTaskDelay(CONFIG_SPI_FLASH_ERASE_YIELD_TICKS); + if (chip->os_func->yield) { + chip->os_func->yield(chip->os_func_data); + } } #endif } diff --git a/components/spi_flash/flash_ops.c b/components/spi_flash/flash_ops.c index a561cb4b5..09b14bed3 100644 --- a/components/spi_flash/flash_ops.c +++ b/components/spi_flash/flash_ops.c @@ -82,6 +82,7 @@ static spi_flash_counters_t s_flash_stats; static esp_err_t spi_flash_translate_rc(esp_rom_spiflash_result_t rc); static bool is_safe_write_address(size_t addr, size_t size); +static void spi_flash_os_yield(void); const DRAM_ATTR spi_flash_guard_funcs_t g_flash_guard_default_ops = { .start = spi_flash_disable_interrupts_caches_and_other_cpu, @@ -89,18 +90,20 @@ const DRAM_ATTR spi_flash_guard_funcs_t g_flash_guard_default_ops = { .op_lock = spi_flash_op_lock, .op_unlock = spi_flash_op_unlock, #if !CONFIG_SPI_FLASH_DANGEROUS_WRITE_ALLOWED - .is_safe_write_address = is_safe_write_address + .is_safe_write_address = is_safe_write_address, #endif + .yield = spi_flash_os_yield, }; const DRAM_ATTR spi_flash_guard_funcs_t g_flash_guard_no_os_ops = { .start = spi_flash_disable_interrupts_caches_and_other_cpu_no_os, .end = spi_flash_enable_interrupts_caches_no_os, - .op_lock = 0, - .op_unlock = 0, + .op_lock = NULL, + .op_unlock = NULL, #if !CONFIG_SPI_FLASH_DANGEROUS_WRITE_ALLOWED - .is_safe_write_address = 0 + .is_safe_write_address = NULL, #endif + .yield = NULL, }; static const spi_flash_guard_funcs_t *s_flash_guard_ops; @@ -185,6 +188,13 @@ static inline void IRAM_ATTR spi_flash_guard_op_unlock(void) } } +static void IRAM_ATTR spi_flash_os_yield(void) +{ +#ifdef CONFIG_SPI_FLASH_YIELD_DURING_ERASE + vTaskDelay(CONFIG_SPI_FLASH_ERASE_YIELD_TICKS); +#endif +} + #ifdef CONFIG_SPI_FLASH_USE_LEGACY_IMPL static esp_rom_spiflash_result_t IRAM_ATTR spi_flash_unlock(void) { @@ -263,7 +273,9 @@ esp_err_t IRAM_ATTR spi_flash_erase_range(size_t start_addr, size_t size) no_yield_time_us += (esp_timer_get_time() - start_time_us); if (no_yield_time_us / 1000 >= CONFIG_SPI_FLASH_ERASE_YIELD_DURATION_MS) { no_yield_time_us = 0; - vTaskDelay(CONFIG_SPI_FLASH_ERASE_YIELD_TICKS); + if (s_flash_guard_ops && s_flash_guard_ops->yield) { + s_flash_guard_ops->yield(); + } } #endif } diff --git a/components/spi_flash/include/esp_flash.h b/components/spi_flash/include/esp_flash.h index a60bfe437..ccea0d072 100644 --- a/components/spi_flash/include/esp_flash.h +++ b/components/spi_flash/include/esp_flash.h @@ -50,6 +50,9 @@ typedef struct { /** Delay for at least 'us' microseconds. Called in between 'start' and 'end'. */ esp_err_t (*delay_us)(void *arg, unsigned us); + + /** Yield to other tasks. Called during erase operations. */ + esp_err_t (*yield)(void *arg); } esp_flash_os_functions_t; /** @brief Structure to describe a SPI flash chip connected to the system. diff --git a/components/spi_flash/include/esp_spi_flash.h b/components/spi_flash/include/esp_spi_flash.h index 9e8b19e86..59d3679da 100644 --- a/components/spi_flash/include/esp_spi_flash.h +++ b/components/spi_flash/include/esp_spi_flash.h @@ -341,6 +341,10 @@ typedef void (*spi_flash_op_unlock_func_t)(void); * @brief Function to protect SPI flash critical regions corruption. */ typedef bool (*spi_flash_is_safe_write_address_t)(size_t addr, size_t size); +/** + * @brief Function to yield to the OS during erase operation. + */ +typedef void (*spi_flash_os_yield_t)(void); /** * Structure holding SPI flash access critical sections management functions. @@ -381,6 +385,7 @@ typedef struct { #if !CONFIG_SPI_FLASH_DANGEROUS_WRITE_ALLOWED spi_flash_is_safe_write_address_t is_safe_write_address; /**< checks flash write addresses.*/ #endif + spi_flash_os_yield_t yield; /**< yield to the OS during flash erase */ } spi_flash_guard_funcs_t; /** diff --git a/components/spi_flash/spi_flash_os_func_app.c b/components/spi_flash/spi_flash_os_func_app.c index af1ee8aa1..ceb046ec6 100644 --- a/components/spi_flash/spi_flash_os_func_app.c +++ b/components/spi_flash/spi_flash_os_func_app.c @@ -17,6 +17,8 @@ #include "esp_spi_flash.h" //for ``g_flash_guard_default_ops`` #include "esp_flash.h" #include "esp_flash_partitions.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" #include "hal/spi_types.h" #include "sdkconfig.h" @@ -101,6 +103,14 @@ static IRAM_ATTR esp_err_t delay_us(void *arg, unsigned us) return ESP_OK; } +static IRAM_ATTR esp_err_t spi_flash_os_yield(void *arg) +{ +#ifdef CONFIG_SPI_FLASH_YIELD_DURING_ERASE + vTaskDelay(CONFIG_SPI_FLASH_ERASE_YIELD_TICKS); +#endif + return ESP_OK; +} + static IRAM_ATTR esp_err_t main_flash_region_protected(void* arg, size_t start_addr, size_t size) { if (((spi1_app_func_arg_t*)arg)->no_protect || esp_partition_main_flash_region_safe(start_addr, size)) { @@ -117,14 +127,16 @@ static DRAM_ATTR spi1_app_func_arg_t main_flash_arg = {}; static const DRAM_ATTR esp_flash_os_functions_t esp_flash_spi1_default_os_functions = { .start = spi1_start, .end = spi1_end, - .delay_us = delay_us, .region_protected = main_flash_region_protected, + .delay_us = delay_us, + .yield = spi_flash_os_yield, }; static const esp_flash_os_functions_t esp_flash_spi23_default_os_functions = { .start = spi_start, .end = spi_end, .delay_us = delay_us, + .yield = spi_flash_os_yield }; static spi_bus_lock_dev_handle_t register_dev(int host_id) diff --git a/components/spi_flash/spi_flash_os_func_noos.c b/components/spi_flash/spi_flash_os_func_noos.c index 851a2d6ab..d565395e0 100644 --- a/components/spi_flash/spi_flash_os_func_noos.c +++ b/components/spi_flash/spi_flash_os_func_noos.c @@ -76,6 +76,7 @@ const DRAM_ATTR esp_flash_os_functions_t esp_flash_noos_functions = { .end = end, .delay_us = delay_us, .region_protected = NULL, + .yield = NULL, }; esp_err_t IRAM_ATTR esp_flash_app_disable_os_functions(esp_flash_t* chip)