From 8ae09194ace00ef6a0b146eda492c27b1e932fde Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Fri, 11 Sep 2020 18:20:08 +0800 Subject: [PATCH 1/3] esp_flash: refactor to support various type of yield There is a periodically yield in the esp_flash driver, to ensure the cache will not be disabled for too long on ESP32. On ESP32-S2 and later, we need to support more different kind of yield: 1. polling conditions, including timeout, SW read request, etc. 2. wait for events, including HW done/error/auto-suspend, timeout semaphore, etc. The check_yield() and yield() is separated into two parts, because we may need to insert suspend, etc. between them. --- components/spi_flash/esp_flash_api.c | 146 +++++++++++++----- components/spi_flash/include/esp_flash.h | 15 +- .../include/spi_flash_chip_generic.h | 11 +- components/spi_flash/spi_flash_chip_generic.c | 52 ++++--- components/spi_flash/spi_flash_os_func_app.c | 111 +++++++++++-- 5 files changed, 258 insertions(+), 77 deletions(-) diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 9521c9c3cf..2a44c0e73e 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -22,8 +22,8 @@ #include "esp_log.h" #include "sdkconfig.h" #include "esp_flash_internal.h" -#include "esp_private/system_internal.h" +#include "spi_flash_chip_generic.h" //for spi_flash_chip_generic_yield() static const char TAG[] = "spi_flash"; @@ -73,11 +73,13 @@ esp_err_t esp_flash_read_chip_id(esp_flash_t* chip, uint32_t* flash_id); static esp_err_t spiflash_start_default(esp_flash_t *chip); static esp_err_t spiflash_end_default(esp_flash_t *chip, esp_err_t err); static esp_err_t check_chip_pointer_default(esp_flash_t **inout_chip); +static esp_err_t flash_end_flush_cache(esp_flash_t* chip, esp_err_t err, bool bus_acquired, uint32_t address, uint32_t length); typedef struct { esp_err_t (*start)(esp_flash_t *chip); esp_err_t (*end)(esp_flash_t *chip, esp_err_t err); esp_err_t (*chip_check)(esp_flash_t **inout_chip); + esp_err_t (*flash_end_flush_cache)(esp_flash_t* chip, esp_err_t err, bool bus_acquired, uint32_t address, uint32_t length); } rom_spiflash_api_func_t; // These functions can be placed in the ROM. For now we use the code in IDF. @@ -85,6 +87,7 @@ DRAM_ATTR static rom_spiflash_api_func_t default_spiflash_rom_api = { .start = spiflash_start_default, .end = spiflash_end_default, .chip_check = check_chip_pointer_default, + .flash_end_flush_cache = flash_end_flush_cache, }; DRAM_ATTR rom_spiflash_api_func_t *rom_spiflash_api_funcs = &default_spiflash_rom_api; @@ -134,6 +137,26 @@ static esp_err_t check_chip_pointer_default(esp_flash_t **inout_chip) return ESP_OK; } +static IRAM_ATTR esp_err_t flash_end_flush_cache(esp_flash_t* chip, esp_err_t err, bool bus_acquired, uint32_t address, uint32_t length) +{ + if (!bus_acquired) { + // Try to acquire the bus again to flush the cache before exit. + esp_err_t acquire_err = rom_spiflash_api_funcs->start(chip); + if (acquire_err != ESP_OK) { + return (err == ESP_OK)? acquire_err: err; + } + } + + if (chip->host->driver->flush_cache) { + esp_err_t flush_err = chip->host->driver->flush_cache(chip->host, address, length); + if (err == ESP_OK) { + err = flush_err; + } + } + return rom_spiflash_api_funcs->end(chip, err); +} + + /* Return true if regions 'a' and 'b' overlap at all, based on their start offsets and lengths. */ inline static bool regions_overlap(uint32_t a_start, uint32_t a_len,uint32_t b_start, uint32_t b_len); @@ -334,12 +357,24 @@ esp_err_t IRAM_ATTR esp_flash_erase_chip(esp_flash_t *chip) VERIFY_CHIP_OP(erase_chip); CHECK_WRITE_ADDRESS(chip, 0, chip->size); + //check before the operation, in case this is called too close to the last operation + err = spi_flash_chip_generic_yield(chip, false); + if (err != ESP_OK) { + return err; + } + err = rom_spiflash_api_funcs->start(chip); if (err != ESP_OK) { return err; } err = chip->chip_drv->erase_chip(chip); + if (chip->host->driver->flush_cache) { + esp_err_t flush_cache_err = chip->host->driver->flush_cache(chip->host, 0, chip->size); + if (err == ESP_OK) { + err = flush_cache_err; + } + } return rom_spiflash_api_funcs->end(chip, err); } @@ -387,47 +422,58 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui // Don't lock the SPI flash for the entire erase, as this may be very long err = rom_spiflash_api_funcs->end(chip, err); } + if (err != ESP_OK) { + return err; + } + + uint32_t erase_addr = start; + uint32_t len_remain = len; + // Indicate whether the bus is acquired by the driver, needs to be released before return + bool bus_acquired = false; + while (1) { + //check before the operation, in case this is called too close to the last operation + err = spi_flash_chip_generic_yield(chip, false); + if (err != ESP_OK) { + break; + } -#ifdef CONFIG_SPI_FLASH_YIELD_DURING_ERASE - int64_t no_yield_time_us = 0; -#endif - while (err == ESP_OK && len >= sector_size) { -#ifdef CONFIG_SPI_FLASH_YIELD_DURING_ERASE - int64_t start_time_us = esp_system_get_time(); -#endif err = rom_spiflash_api_funcs->start(chip); if (err != ESP_OK) { - return err; + break; } + bus_acquired = true; #ifndef CONFIG_SPI_FLASH_BYPASS_BLOCK_ERASE // If possible erase an entire multi-sector block - if (block_erase_size > 0 && len >= block_erase_size && (start % block_erase_size) == 0) { - err = chip->chip_drv->erase_block(chip, start); - start += block_erase_size; - len -= block_erase_size; + if (block_erase_size > 0 && len_remain >= block_erase_size && (erase_addr % block_erase_size) == 0) { + err = chip->chip_drv->erase_block(chip, erase_addr); + erase_addr += block_erase_size; + len_remain -= block_erase_size; } else #endif { // Otherwise erase individual sector only - err = chip->chip_drv->erase_sector(chip, start); - start += sector_size; - len -= sector_size; + err = chip->chip_drv->erase_sector(chip, erase_addr); + erase_addr += sector_size; + len_remain -= sector_size; } - err = rom_spiflash_api_funcs->end(chip, err); - -#ifdef CONFIG_SPI_FLASH_YIELD_DURING_ERASE - no_yield_time_us += (esp_system_get_time() - start_time_us); - if (no_yield_time_us / 1000 >= CONFIG_SPI_FLASH_ERASE_YIELD_DURATION_MS) { - no_yield_time_us = 0; - if (chip->os_func->yield) { - chip->os_func->yield(chip->os_func_data); - } + if (err != ESP_OK || len_remain == 0) { + // On ESP32, the cache re-enable is in the end() function, while flush_cache should + // happen when the cache is still disabled on ESP32. Break before the end() function and + // do end() later + assert(bus_acquired); + break; } -#endif + + err = rom_spiflash_api_funcs->end(chip, ESP_OK); + if (err != ESP_OK) { + break; + } + bus_acquired = false; } - return err; + + return rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, start, len); } esp_err_t IRAM_ATTR esp_flash_get_chip_write_protect(esp_flash_t *chip, bool *out_write_protected) @@ -638,38 +684,62 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3 //when the cache is disabled, only the DRAM can be read, check whether we need to copy the data first bool direct_write = chip->host->driver->supports_direct_write(chip->host, buffer); + // Indicate whether the bus is acquired by the driver, needs to be released before return + bool bus_acquired = false; err = ESP_OK; /* Write output in chunks, either by buffering on stack or by artificially cutting into MAX_WRITE_CHUNK parts (in an OS environment, this prevents writing from causing interrupt or higher priority task starvation.) */ - do { + uint32_t write_addr = address; + uint32_t len_remain = length; + while (1) { uint32_t write_len; const void *write_buf; uint32_t temp_buf[8]; if (direct_write) { - write_len = MIN(length, MAX_WRITE_CHUNK); + write_len = MIN(len_remain, MAX_WRITE_CHUNK); write_buf = buffer; } else { - write_len = MIN(length, sizeof(temp_buf)); + write_len = MIN(len_remain, sizeof(temp_buf)); memcpy(temp_buf, buffer, write_len); write_buf = temp_buf; } - err = rom_spiflash_api_funcs->start(chip); + //check before the operation, in case this is called too close to the last operation + err = spi_flash_chip_generic_yield(chip, false); if (err != ESP_OK) { - return err; + break; } - err = chip->chip_drv->write(chip, write_buf, address, write_len); + err = rom_spiflash_api_funcs->start(chip); + if (err != ESP_OK) { + break; + } + bus_acquired = true; - address += write_len; - buffer = (void *)((intptr_t)buffer + write_len); - length -= write_len; + err = chip->chip_drv->write(chip, write_buf, write_addr, write_len); + len_remain -= write_len; + + if (err != ESP_OK || len_remain == 0) { + // On ESP32, the cache re-enable is in the end() function, while flush_cache should + // happen when the cache is still disabled on ESP32. Break before the end() function and + // do end() later + assert(bus_acquired); + break; + } err = rom_spiflash_api_funcs->end(chip, err); - } while (err == ESP_OK && length > 0); - return err; + if (err != ESP_OK) { + break; + } + bus_acquired = false; + + write_addr += write_len; + buffer = (void *)((intptr_t)buffer + write_len); + } + + return rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length); } //currently the legacy implementation is used, from flash_ops.c diff --git a/components/spi_flash/include/esp_flash.h b/components/spi_flash/include/esp_flash.h index 1f64347c3c..2df1d53c57 100644 --- a/components/spi_flash/include/esp_flash.h +++ b/components/spi_flash/include/esp_flash.h @@ -65,8 +65,21 @@ typedef struct { /** Called for release temp buffer. */ void (*release_temp_buffer)(void* arg, void *temp_buf); + #define SPI_FLASH_YIELD_REQ_YIELD BIT(0) + #define SPI_FLASH_YIELD_REQ_SUSPEND BIT(1) + + /** Yield to other tasks. Called during erase operations. + * @return ESP_OK means yield needs to be called (got an event to handle), while ESP_ERR_TIMEOUT means skip yield.*/ + esp_err_t (*check_yield)(void *arg, uint32_t chip_status, uint32_t* out_request); + + #define SPI_FLASH_YIELD_STA_RESUME BIT(2) + /** Yield to other tasks. Called during erase operations. */ - esp_err_t (*yield)(void *arg); + esp_err_t (*yield)(void *arg, uint32_t* out_status); + + /** Called for get system time. */ + int64_t (*get_system_time)(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/spi_flash_chip_generic.h b/components/spi_flash/include/spi_flash_chip_generic.h index 1c76ec1e44..43475c3fc6 100644 --- a/components/spi_flash/include/spi_flash_chip_generic.h +++ b/components/spi_flash/include/spi_flash_chip_generic.h @@ -381,5 +381,14 @@ esp_err_t spi_flash_common_set_io_mode(esp_flash_t *chip, esp_flash_wrsr_func_t */ esp_err_t spi_flash_chip_generic_config_host_io_mode(esp_flash_t *chip, bool addr_32bit); +/** + * @brief Handle explicit yield requests + * + * @param chip Pointer to SPI flash chip to use. If NULL, esp_flash_default_chip is substituted. + * @param wip Write (erase) in progress, `true` if this function is called during waiting idle of a erase/write command; else `false`. + * @return ESP_OK if success, otherwise failed. + */ +esp_err_t spi_flash_chip_generic_yield(esp_flash_t* chip, bool wip); + /// Default timeout configuration used by most chips -const flash_chip_op_timeout_t spi_flash_chip_generic_timeout; \ No newline at end of file +const flash_chip_op_timeout_t spi_flash_chip_generic_timeout; diff --git a/components/spi_flash/spi_flash_chip_generic.c b/components/spi_flash/spi_flash_chip_generic.c index 48120573c9..2277826541 100644 --- a/components/spi_flash/spi_flash_chip_generic.c +++ b/components/spi_flash/spi_flash_chip_generic.c @@ -119,13 +119,6 @@ esp_err_t spi_flash_chip_generic_erase_chip(esp_flash_t *chip) } if (err == ESP_OK) { chip->host->driver->erase_chip(chip->host); - //to save time, flush cache here - if (chip->host->driver->flush_cache) { - err = chip->host->driver->flush_cache(chip->host, 0, chip->size); - if (err != ESP_OK) { - return err; - } - } #ifdef CONFIG_SPI_FLASH_CHECK_ERASE_TIMEOUT_DISABLED err = chip->chip_drv->wait_idle(chip, ESP_FLASH_CHIP_GENERIC_NO_TIMEOUT); #else @@ -143,13 +136,6 @@ esp_err_t spi_flash_chip_generic_erase_sector(esp_flash_t *chip, uint32_t start_ } if (err == ESP_OK) { chip->host->driver->erase_sector(chip->host, start_address); - //to save time, flush cache here - if (chip->host->driver->flush_cache) { - err = chip->host->driver->flush_cache(chip->host, start_address, chip->chip_drv->sector_size); - if (err != ESP_OK) { - return err; - } - } #ifdef CONFIG_SPI_FLASH_CHECK_ERASE_TIMEOUT_DISABLED err = chip->chip_drv->wait_idle(chip, ESP_FLASH_CHIP_GENERIC_NO_TIMEOUT); #else @@ -167,13 +153,6 @@ esp_err_t spi_flash_chip_generic_erase_block(esp_flash_t *chip, uint32_t start_a } if (err == ESP_OK) { chip->host->driver->erase_block(chip->host, start_address); - //to save time, flush cache here - if (chip->host->driver->flush_cache) { - err = chip->host->driver->flush_cache(chip->host, start_address, chip->chip_drv->block_erase_size); - if (err != ESP_OK) { - return err; - } - } #ifdef CONFIG_SPI_FLASH_CHECK_ERASE_TIMEOUT_DISABLED err = chip->chip_drv->wait_idle(chip, ESP_FLASH_CHIP_GENERIC_NO_TIMEOUT); #else @@ -253,9 +232,8 @@ esp_err_t spi_flash_chip_generic_write(esp_flash_t *chip, const void *buffer, ui length -= write_len; } } - if (err == ESP_OK && chip->host->driver->flush_cache) { - err = chip->host->driver->flush_cache(chip->host, address, length); - } + // The caller is responsible to do host->driver->flush_cache, because this function may be + // called in small pieces. Frequency call of flush cache will do harm to the performance. return err; } @@ -318,6 +296,30 @@ esp_err_t spi_flash_chip_generic_read_reg(esp_flash_t* chip, spi_flash_register_ return chip->host->driver->read_status(chip->host, (uint8_t*)out_reg); } +esp_err_t spi_flash_chip_generic_yield(esp_flash_t* chip, bool wip) +{ + esp_err_t err = ESP_OK; + uint32_t flags = wip? 1: 0; //check_yield() and yield() impls should not issue suspend/resume if this flag is zero + + if (chip->os_func->check_yield) { + uint32_t request; + //According to the implementation, the check_yield() function may block, poll, delay or do nothing but return + err = chip->os_func->check_yield(chip->os_func_data, flags, &request); + if (err == ESP_OK) { + if (err == ESP_OK && (request & SPI_FLASH_YIELD_REQ_YIELD) != 0) { + uint32_t status; + //According to the implementation, the yield() function may block until something happen + err = chip->os_func->yield(chip->os_func_data, &status); + } + } else if (err == ESP_ERR_TIMEOUT) { + err = ESP_OK; + } else { + abort(); + } + } + return err; +} + esp_err_t spi_flash_chip_generic_wait_idle(esp_flash_t *chip, uint32_t timeout_us) { bool timeout_en = (timeout_us != ESP_FLASH_CHIP_GENERIC_NO_TIMEOUT); @@ -607,4 +609,4 @@ esp_err_t spi_flash_common_set_io_mode(esp_flash_t *chip, esp_flash_wrsr_func_t chip->chip_drv->set_chip_write_protect(chip, true); } return ret; -} \ No newline at end of file +} diff --git a/components/spi_flash/spi_flash_os_func_app.c b/components/spi_flash/spi_flash_os_func_app.c index 833d778067..f9c334ac3f 100644 --- a/components/spi_flash/spi_flash_os_func_app.c +++ b/components/spi_flash/spi_flash_os_func_app.c @@ -15,6 +15,7 @@ #include #include //For max/min #include "esp_attr.h" +#include "esp_private/system_internal.h" #include "esp_spi_flash.h" //for ``g_flash_guard_default_ops`` #include "esp_flash.h" #include "esp_flash_partitions.h" @@ -41,11 +42,29 @@ typedef struct { spi_bus_lock_dev_handle_t dev_lock; } app_func_arg_t; +/* + * Time yield algorithm: + * Every time spi_flash_os_check_yield() is called: + * + * 1. If the time since last end() function is longer than CONFIG_SPI_FLASH_ERASE_YIELD_TICKS (time + * to yield), all counters will be reset, as if the yield has just ends; + * 2. If the time since last yield() is longer than CONFIG_SPI_FLASH_ERASE_YIELD_DURATION_MS, will + * return a yield request. When the yield() is called, all counters will be reset. + * Note: Short intervals between start() and end() after the last yield() will not reset the + * counter mentioned in #2, but still be counted into the time mentioned in #2. + */ typedef struct { app_func_arg_t common_arg; //shared args, must be the first item bool no_protect; //to decide whether to check protected region (for the main chip) or not. + uint32_t acquired_since_us; // Time since last explicit yield() + uint32_t released_since_us; // Time since last end() (implicit yield) } spi1_app_func_arg_t; +static inline IRAM_ATTR void on_spi1_released(spi1_app_func_arg_t* ctx); +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); + IRAM_ATTR static void cache_enable(void* arg) { g_flash_guard_default_ops.end(); @@ -82,18 +101,48 @@ static IRAM_ATTR esp_err_t spi1_start(void *arg) #else //directly disable the cache and interrupts when lock is not used cache_disable(NULL); -#endif + on_spi1_acquired((spi1_app_func_arg_t*)arg); return ESP_OK; +#endif } static IRAM_ATTR esp_err_t spi1_end(void *arg) { + esp_err_t ret = ESP_OK; #if CONFIG_SPI_FLASH_SHARE_SPI1_BUS - return spi_end(arg); + ret = spi_end(arg); #else cache_enable(NULL); - return ESP_OK; #endif + on_spi1_released((spi1_app_func_arg_t*)arg); + return ret; +} + +static IRAM_ATTR esp_err_t spi1_flash_os_check_yield(void *arg, uint32_t chip_status, uint32_t* out_request) +{ + assert (chip_status == 0); //TODO: support suspend + esp_err_t ret = ESP_ERR_TIMEOUT; //Nothing happened + uint32_t request = 0; + + if (on_spi1_check_yield((spi1_app_func_arg_t *)arg)) { + request = SPI_FLASH_YIELD_REQ_YIELD; + ret = ESP_OK; + } + if (out_request) { + *out_request = request; + } + return ret; +} + +static IRAM_ATTR esp_err_t spi1_flash_os_yield(void *arg, uint32_t* out_status) +{ +#ifdef CONFIG_SPI_FLASH_ERASE_YIELD_TICKS + vTaskDelay(CONFIG_SPI_FLASH_ERASE_YIELD_TICKS); +#else + vTaskDelay(1); +#endif + on_spi1_yielded((spi1_app_func_arg_t*)arg); + return ESP_OK; } static IRAM_ATTR esp_err_t delay_us(void *arg, uint32_t us) @@ -101,13 +150,6 @@ static IRAM_ATTR esp_err_t delay_us(void *arg, uint32_t us) esp_rom_delay_us(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 void* get_buffer_malloc(void* arg, size_t reqest_size, size_t* out_size) { @@ -155,7 +197,8 @@ static const DRAM_ATTR esp_flash_os_functions_t esp_flash_spi1_default_os_functi .delay_us = delay_us, .get_temp_buffer = get_buffer_malloc, .release_temp_buffer = release_buffer_malloc, - .yield = spi_flash_os_yield, + .check_yield = spi1_flash_os_check_yield, + .yield = spi1_flash_os_yield, }; static const esp_flash_os_functions_t esp_flash_spi23_default_os_functions = { @@ -164,7 +207,9 @@ static const esp_flash_os_functions_t esp_flash_spi23_default_os_functions = { .delay_us = delay_us, .get_temp_buffer = get_buffer_malloc, .release_temp_buffer = release_buffer_malloc, - .yield = spi_flash_os_yield + .region_protected = NULL, + .check_yield = NULL, + .yield = NULL, }; static spi_bus_lock_dev_handle_t register_dev(int host_id) @@ -269,3 +314,45 @@ esp_err_t esp_flash_app_enable_os_functions(esp_flash_t* chip) chip->os_func_data = &main_flash_arg; return ESP_OK; } + +// The goal of this part is to manually insert one valid task execution interval, if the time since +// last valid interval exceed the limitation (CONFIG_SPI_FLASH_ERASE_YIELD_DURATION_MS). +// +// Valid task execution interval: continuous time with the cache enabled, which is longer than +// CONFIG_SPI_FLASH_ERASE_YIELD_TICKS. Yield time shorter than CONFIG_SPI_FLASH_ERASE_YIELD_TICKS is +// not treated as valid interval. +static inline IRAM_ATTR bool on_spi1_check_yield(spi1_app_func_arg_t* ctx) +{ +#ifdef CONFIG_SPI_FLASH_YIELD_DURING_ERASE + uint32_t time = esp_system_get_time(); + // We handle the reset here instead of in `on_spi1_acquired()`, when acquire() and release() is + // larger than CONFIG_SPI_FLASH_ERASE_YIELD_TICKS, to save one `esp_system_get_time()` call + if ((time - ctx->released_since_us) >= CONFIG_SPI_FLASH_ERASE_YIELD_TICKS * portTICK_PERIOD_MS * 1000) { + // Reset the acquired time as if the yield has just happened. + ctx->acquired_since_us = time; + } else if ((time - ctx->acquired_since_us) >= CONFIG_SPI_FLASH_ERASE_YIELD_DURATION_MS * 1000) { + return true; + } +#endif + return false; +} +static inline IRAM_ATTR void on_spi1_released(spi1_app_func_arg_t* ctx) +{ +#ifdef CONFIG_SPI_FLASH_YIELD_DURING_ERASE + ctx->released_since_us = esp_system_get_time(); +#endif +} + +static inline IRAM_ATTR void on_spi1_acquired(spi1_app_func_arg_t* ctx) +{ + // Ideally, when the time after `on_spi1_released()` before this function is called is larger + // than CONFIG_SPI_FLASH_ERASE_YIELD_TICKS, the acquired time should be reset. We assume the + // time after `on_spi1_check_yield()` before this function is so short that we can do the reset + // in that function instead. +} + +static inline IRAM_ATTR void on_spi1_yielded(spi1_app_func_arg_t* ctx) +{ + uint32_t time = esp_system_get_time(); + ctx->acquired_since_us = time; +} From 7ddfcfb8d2e37d52ee7677cdb027c9f1356a7dfe Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Sun, 13 Sep 2020 00:25:20 +0800 Subject: [PATCH 2/3] esp_flash_test: improve unit test From now on, we have two tags for esp_flash tests: - [esp_flash] for main flash chip only tests - [esp_flash_3] for tests with external flash chips To Run all tests, type `[esp_flash`; to run tests for main flash chip only, type `[esp_flash]. --- components/spi_flash/test/test_esp_flash.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/spi_flash/test/test_esp_flash.c b/components/spi_flash/test/test_esp_flash.c index 4298578247..6c11099749 100644 --- a/components/spi_flash/test/test_esp_flash.c +++ b/components/spi_flash/test/test_esp_flash.c @@ -113,10 +113,10 @@ typedef void (*flash_test_func_t)(const esp_partition_t *part); #define FLASH_TEST_CASE_3_IGNORE(STR, FUNCT_TO_RUN) #else #define FLASH_TEST_CASE_3(STR, FUNC_TO_RUN) \ - TEST_CASE(STR", 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH]") {flash_test_func(FUNC_TO_RUN, TEST_CONFIG_NUM);} + TEST_CASE(STR", 3 chips", "[esp_flash_3][test_env=UT_T1_ESP_FLASH]") {flash_test_func(FUNC_TO_RUN, TEST_CONFIG_NUM);} #define FLASH_TEST_CASE_3_IGNORE(STR, FUNC_TO_RUN) \ - TEST_CASE(STR", 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH][ignore]") {flash_test_func(FUNC_TO_RUN, TEST_CONFIG_NUM);} + TEST_CASE(STR", 3 chips", "[esp_flash_3][test_env=UT_T1_ESP_FLASH][ignore]") {flash_test_func(FUNC_TO_RUN, TEST_CONFIG_NUM);} #endif //currently all the configs are the same with esp_flash_spi_device_config_t, no more information required @@ -801,7 +801,7 @@ TEST_CASE("SPI flash test reading with all speed/mode permutations", "[esp_flash } #ifndef CONFIG_SPIRAM -TEST_CASE("SPI flash test reading with all speed/mode permutations, 3 chips", "[esp_flash][test_env=UT_T1_ESP_FLASH]") +TEST_CASE("SPI flash test reading with all speed/mode permutations, 3 chips", "[esp_flash_3][test_env=UT_T1_ESP_FLASH]") { for (int i = 0; i < TEST_CONFIG_NUM; i++) { test_permutations_chip(&config_list[i]); From 6c2b6c93404abdbd3d8f96481efd84fe8e064a93 Mon Sep 17 00:00:00 2001 From: "Michael (XIAO Xufeng)" Date: Thu, 5 Nov 2020 01:18:44 +0800 Subject: [PATCH 3/3] esp_flash: decrease performance threshld To reflect the influence of yield during write --- .../include/esp32/idf_performance_target.h | 14 ++++++++++++++ .../include/esp32s2/idf_performance_target.h | 13 +++++++++++++ .../include/esp32s3/idf_performance_target.h | 13 +++++++++++++ components/idf_test/include/idf_performance.h | 17 ++++------------- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/components/idf_test/include/esp32/idf_performance_target.h b/components/idf_test/include/esp32/idf_performance_target.h index 3002ece216..c31e26061b 100644 --- a/components/idf_test/include/esp32/idf_performance_target.h +++ b/components/idf_test/include/esp32/idf_performance_target.h @@ -21,9 +21,23 @@ #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_4B 50600 #endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB (695*1000) +#endif + +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B 24300 +#endif #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_4B 50300 #endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE 44300 +#endif + +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B 23100 +#endif #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_EXT_WR_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_EXT_WR_4B 68900 diff --git a/components/idf_test/include/esp32s2/idf_performance_target.h b/components/idf_test/include/esp32s2/idf_performance_target.h index 7f3d6ac27b..f098f17279 100644 --- a/components/idf_test/include/esp32s2/idf_performance_target.h +++ b/components/idf_test/include/esp32s2/idf_performance_target.h @@ -20,10 +20,23 @@ #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_4B 53400 #endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB (701*1000) +#endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B 27400 +#endif #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_4B 53600 #endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE 39900 +#endif + +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B 24400 +#endif #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_EXT_WR_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_EXT_WR_4B 64900 diff --git a/components/idf_test/include/esp32s3/idf_performance_target.h b/components/idf_test/include/esp32s3/idf_performance_target.h index 67a1ae3e79..4a35ce8543 100644 --- a/components/idf_test/include/esp32s3/idf_performance_target.h +++ b/components/idf_test/include/esp32s3/idf_performance_target.h @@ -19,10 +19,23 @@ #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_4B 53400 #endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB (701*1000) +#endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B 27400 +#endif #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_4B 53600 #endif +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE 44300 +#endif + +#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B +#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B 24400 +#endif #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_EXT_WR_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_EXT_WR_4B 68900 diff --git a/components/idf_test/include/idf_performance.h b/components/idf_test/include/idf_performance.h index 15a27a3814..5b0b91e1da 100644 --- a/components/idf_test/include/idf_performance.h +++ b/components/idf_test/include/idf_performance.h @@ -94,9 +94,7 @@ #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_4B 22200 #endif // IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_4B in target file -#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB -#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB (701*1000) -#endif +// IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_WR_2KB in target file #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_2KB #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_RD_2KB (7088*1000) #endif @@ -105,9 +103,7 @@ #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_LEGACY_ERASE 12000 #endif -#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B -#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B 27400 -#endif +// IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_4B in target file // IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_4B in target file #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_2KB #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_WR_2KB (694*1000) @@ -115,13 +111,9 @@ #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_2KB #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_RD_2KB (7797*1000) #endif -#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE -#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE 44300 -#endif +// IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_ERASE in target file -#ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B -#define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B 24400 -#endif +// IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_WR_4B in target file #ifndef IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_RD_4B #define IDF_PERFORMANCE_MIN_FLASH_SPEED_BYTE_PER_SEC_SPI1_RD_4B 50100 #endif @@ -156,4 +148,3 @@ #ifndef IDF_PERFORMANCE_MAX_FREE_DEFAULT_AVERAGE_TIME #define IDF_PERFORMANCE_MAX_FREE_DEFAULT_AVERAGE_TIME 950 #endif -