From 8e6700c156c7c699308749bad7f8444c6c24b81d Mon Sep 17 00:00:00 2001 From: negativekelvin Date: Fri, 23 Jul 2021 02:35:27 -0700 Subject: [PATCH 1/3] esp_flash_api fixes --- components/spi_flash/esp_flash_api.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 33f69c7af7..3667e43791 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -460,6 +460,9 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui // Can only erase multiples of the sector size, starting at sector boundary return ESP_ERR_INVALID_ARG; } + if (len == 0) { + return ESP_OK; + } err = ESP_OK; // Check for write protected regions overlapping the erase region @@ -522,6 +525,8 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui len_remain -= sector_size; } + assert(len_remain < 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 @@ -668,14 +673,14 @@ esp_err_t IRAM_ATTR esp_flash_set_protected_region(esp_flash_t *chip, const esp_ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t address, uint32_t length) { - if (length == 0) { - return ESP_OK; - } esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip); VERIFY_CHIP_OP(read); if (buffer == NULL || address > chip->size || address+length > chip->size) { return ESP_ERR_INVALID_ARG; } + if (length == 0) { + return ESP_OK; + } //when the cache is disabled, only the DRAM can be read, check whether we need to receive in another buffer in DRAM. bool direct_read = chip->host->driver->supports_direct_read(chip->host, buffer); @@ -735,15 +740,15 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint32_t address, uint32_t length) { - if (length == 0) { - return ESP_OK; - } esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip); VERIFY_CHIP_OP(write); CHECK_WRITE_ADDRESS(chip, address, length); if (buffer == NULL || address > chip->size || address+length > chip->size) { return ESP_ERR_INVALID_ARG; } + if (length == 0) { + return ESP_OK; + } //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); @@ -786,6 +791,7 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3 err = chip->chip_drv->write(chip, write_buf, write_addr, write_len); len_remain -= write_len; + assert(len_remain < length); if (err != ESP_OK || len_remain == 0) { // On ESP32, the cache re-enable is in the end() function, while flush_cache should @@ -810,10 +816,6 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3 esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t address, const void *buffer, uint32_t length) { - if (length == 0) { - return ESP_OK; - } - esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip); // Flash encryption only support on main flash. if (chip != esp_flash_default_chip) { @@ -829,6 +831,10 @@ esp_err_t IRAM_ATTR esp_flash_write_encrypted(esp_flash_t *chip, uint32_t addres return ESP_ERR_INVALID_ARG; } + if (length == 0) { + return ESP_OK; + } + if ((length % 16) != 0) { ESP_EARLY_LOGE(TAG, "flash encrypted write length must be multiple of 16"); return ESP_ERR_INVALID_SIZE; From 118fafef07d974d70bf6b4053314133c87e125f3 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Fri, 23 Jul 2021 14:14:57 +0200 Subject: [PATCH 2/3] spi_flash: add test case for esp_flash_erase_region with 0 size --- components/spi_flash/test/test_esp_flash.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/spi_flash/test/test_esp_flash.c b/components/spi_flash/test/test_esp_flash.c index 141a0d67bb..cde35add71 100644 --- a/components/spi_flash/test/test_esp_flash.c +++ b/components/spi_flash/test/test_esp_flash.c @@ -615,6 +615,13 @@ void test_erase_large_region(const esp_partition_t *part) TEST_ASSERT_EQUAL(ESP_OK, esp_flash_read(chip, &readback, part->address, 4)); TEST_ASSERT_EQUAL_HEX32(0, readback & (~written_data)); + /* Erase zero bytes, check that nothing got erased */ + TEST_ASSERT_EQUAL(ESP_OK, esp_flash_erase_region(chip, part->address, 0)); + TEST_ASSERT_EQUAL(ESP_OK, esp_flash_read(chip, &readback, part->address + part->size - 5, 4)); + TEST_ASSERT_EQUAL_HEX32(0, readback & (~written_data)); + TEST_ASSERT_EQUAL(ESP_OK, esp_flash_read(chip, &readback, part->address, 4)); + TEST_ASSERT_EQUAL_HEX32(0, readback & (~written_data)); + /* Erase whole region */ TEST_ASSERT_EQUAL(ESP_OK, esp_flash_erase_region(chip, part->address, part->size)); From 7534c4467f70a28108f454e0e025f6d44aec4fc3 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Sat, 24 Jul 2021 13:28:25 +0200 Subject: [PATCH 3/3] rom, spi_flash: add a patch for esp_flash_erase_region for C3, S3, H2 --- components/esp_rom/esp32c3/esp_rom_caps.h | 1 + components/esp_rom/esp32c3/ld/esp32c3.rom.ld | 2 +- components/esp_rom/esp32h2/esp_rom_caps.h | 1 + components/esp_rom/esp32h2/ld/esp32h2.rom.ld | 2 +- components/esp_rom/esp32s3/esp_rom_caps.h | 1 + components/esp_rom/esp32s3/ld/esp32s3.rom.ld | 2 +- components/spi_flash/esp_flash_api.c | 23 ++++++++++++++++++++ 7 files changed, 29 insertions(+), 3 deletions(-) diff --git a/components/esp_rom/esp32c3/esp_rom_caps.h b/components/esp_rom/esp32c3/esp_rom_caps.h index de9eda41fb..7a210f1a53 100644 --- a/components/esp_rom/esp32c3/esp_rom_caps.h +++ b/components/esp_rom/esp32c3/esp_rom_caps.h @@ -20,3 +20,4 @@ #define ESP_ROM_UART_CLK_IS_XTAL (1) // UART clock source is selected to XTAL in ROM #define ESP_ROM_USB_SERIAL_DEVICE_NUM (3) // UART uses USB_SERIAL_JTAG port in ROM. #define ESP_ROM_HAS_RETARGETABLE_LOCKING (1) // ROM was built with retargetable locking +#define ESP_ROM_HAS_ERASE_0_REGION_BUG (1) // ROM has esp_flash_erase_region(size=0) bug diff --git a/components/esp_rom/esp32c3/ld/esp32c3.rom.ld b/components/esp_rom/esp32c3/ld/esp32c3.rom.ld index dcafa5edd2..da079eeb14 100644 --- a/components/esp_rom/esp32c3/ld/esp32c3.rom.ld +++ b/components/esp_rom/esp32c3/ld/esp32c3.rom.ld @@ -260,7 +260,7 @@ PROVIDE( esp_flash_chip_driver_initialized = 0x400002fc ); PROVIDE( esp_flash_read_id = 0x40000300 ); PROVIDE( esp_flash_get_size = 0x40000304 ); PROVIDE( esp_flash_erase_chip = 0x40000308 ); -PROVIDE( esp_flash_erase_region = 0x4000030c ); +PROVIDE( rom_esp_flash_erase_region = 0x4000030c ); PROVIDE( esp_flash_get_chip_write_protect = 0x40000310 ); PROVIDE( esp_flash_set_chip_write_protect = 0x40000314 ); PROVIDE( esp_flash_get_protectable_regions = 0x40000318 ); diff --git a/components/esp_rom/esp32h2/esp_rom_caps.h b/components/esp_rom/esp32h2/esp_rom_caps.h index de9eda41fb..7a210f1a53 100644 --- a/components/esp_rom/esp32h2/esp_rom_caps.h +++ b/components/esp_rom/esp32h2/esp_rom_caps.h @@ -20,3 +20,4 @@ #define ESP_ROM_UART_CLK_IS_XTAL (1) // UART clock source is selected to XTAL in ROM #define ESP_ROM_USB_SERIAL_DEVICE_NUM (3) // UART uses USB_SERIAL_JTAG port in ROM. #define ESP_ROM_HAS_RETARGETABLE_LOCKING (1) // ROM was built with retargetable locking +#define ESP_ROM_HAS_ERASE_0_REGION_BUG (1) // ROM has esp_flash_erase_region(size=0) bug diff --git a/components/esp_rom/esp32h2/ld/esp32h2.rom.ld b/components/esp_rom/esp32h2/ld/esp32h2.rom.ld index 356f500f44..acd5821d5a 100644 --- a/components/esp_rom/esp32h2/ld/esp32h2.rom.ld +++ b/components/esp_rom/esp32h2/ld/esp32h2.rom.ld @@ -267,7 +267,7 @@ PROVIDE( esp_flash_chip_driver_initialized = 0x400002f8 ); PROVIDE( esp_flash_read_id = 0x400002fc ); PROVIDE( esp_flash_get_size = 0x40000300 ); PROVIDE( esp_flash_erase_chip = 0x40000304 ); -PROVIDE( esp_flash_erase_region = 0x40000308 ); +PROVIDE( rom_esp_flash_erase_region = 0x40000308 ); PROVIDE( esp_flash_get_chip_write_protect = 0x4000030c ); PROVIDE( esp_flash_set_chip_write_protect = 0x40000310 ); PROVIDE( esp_flash_get_protectable_regions = 0x40000314 ); diff --git a/components/esp_rom/esp32s3/esp_rom_caps.h b/components/esp_rom/esp32s3/esp_rom_caps.h index e883610c59..5955f2c371 100644 --- a/components/esp_rom/esp32s3/esp_rom_caps.h +++ b/components/esp_rom/esp32s3/esp_rom_caps.h @@ -21,3 +21,4 @@ #define ESP_ROM_UART_CLK_IS_XTAL (1) // UART clock source is selected to XTAL in ROM #define ESP_ROM_HAS_RETARGETABLE_LOCKING (1) // ROM was built with retargetable locking #define ESP_ROM_USB_SERIAL_DEVICE_NUM (4) // The serial port ID (UART, USB, ...) of USB_SERIAL_JTAG in the ROM. +#define ESP_ROM_HAS_ERASE_0_REGION_BUG (1) // ROM has esp_flash_erase_region(size=0) bug diff --git a/components/esp_rom/esp32s3/ld/esp32s3.rom.ld b/components/esp_rom/esp32s3/ld/esp32s3.rom.ld index ef277cf17e..ed23ae3f62 100644 --- a/components/esp_rom/esp32s3/ld/esp32s3.rom.ld +++ b/components/esp_rom/esp32s3/ld/esp32s3.rom.ld @@ -334,7 +334,7 @@ PROVIDE( esp_flash_chip_driver_initialized = 0x400010bc ); PROVIDE( esp_flash_read_id = 0x400010c8 ); PROVIDE( esp_flash_get_size = 0x400010d4 ); PROVIDE( esp_flash_erase_chip = 0x400010e0 ); -PROVIDE( esp_flash_erase_region = 0x400010ec ); +PROVIDE( rom_esp_flash_erase_region = 0x400010ec ); PROVIDE( esp_flash_get_chip_write_protect = 0x400010f8 ); PROVIDE( esp_flash_set_chip_write_protect = 0x40001104 ); PROVIDE( esp_flash_get_protectable_regions = 0x40001110 ); diff --git a/components/spi_flash/esp_flash_api.c b/components/spi_flash/esp_flash_api.c index 3667e43791..f077d7eb33 100644 --- a/components/spi_flash/esp_flash_api.c +++ b/components/spi_flash/esp_flash_api.c @@ -23,6 +23,7 @@ #include "sdkconfig.h" #include "esp_flash_internal.h" #include "spi_flash_defs.h" +#include "esp_rom_caps.h" #if CONFIG_IDF_TARGET_ESP32S2 #include "esp_crypto_lock.h" // for locking flash encryption peripheral #endif //CONFIG_IDF_TARGET_ESP32S2 @@ -545,6 +546,28 @@ esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, ui return rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, start, len); } +#endif // !CONFIG_SPI_FLASH_ROM_IMPL + +#if defined(CONFIG_SPI_FLASH_ROM_IMPL) && ESP_ROM_HAS_ERASE_0_REGION_BUG + +/* ROM esp_flash_erase_region implementation doesn't handle 0 erase size correctly. + * Check the size and call ROM function instead of overriding it completely. + * The behavior is slightly different from esp_flash_erase_region above, thought: + * here the check for 0 size is done first, but in esp_flash_erase_region the check is + * done after the other arguments are checked. + */ +extern esp_err_t rom_esp_flash_erase_region(esp_flash_t *chip, uint32_t start, uint32_t len); +esp_err_t IRAM_ATTR esp_flash_erase_region(esp_flash_t *chip, uint32_t start, uint32_t len) +{ + if (len == 0) { + return ESP_OK; + } + return rom_esp_flash_erase_region(chip, start, len); +} +#endif // defined(CONFIG_SPI_FLASH_ROM_IMPL) && ESP_ROM_HAS_ERASE_0_REGION_BUG + +#ifndef CONFIG_SPI_FLASH_ROM_IMPL + esp_err_t IRAM_ATTR esp_flash_get_chip_write_protect(esp_flash_t *chip, bool *out_write_protected) { esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip);