Merge branch 'bugfix/esp_flash_erase_0_v4.3' into 'release/v4.3'

esp_flash: fix esp_flash_erase_region over-erase with 0 length (Github PR) (v4.3)

See merge request espressif/esp-idf!16805

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_HAS_RETARGETABLE_LOCKING    (1) // ROM was built with retargetable locking
 #define ESP_ROM_USB_SERIAL_DEVICE_NUM (3) // UART uses USB_SERIAL_JTAG port in ROM.
+#define ESP_ROM_HAS_ERASE_0_REGION_BUG      (1) // ROM has esp_flash_erase_region(size=0) bug

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 );

components/esp_rom/esp32s3/esp_rom_caps.h

@@ -20,3 +20,4 @@
 #define ESP_ROM_SUPPORT_MULTIPLE_UART       (1) // ROM has multiple UARTs available for logging
 #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_HAS_ERASE_0_REGION_BUG      (1) // ROM has esp_flash_erase_region(size=0) bug

components/esp_rom/esp32s3/ld/esp32s3.rom.ld

@@ -470,7 +470,7 @@ PROVIDE ( em_base_reg_lut = 0x3ff08258 );
 PROVIDE ( esp_crc8 = 0x40047854 );
 PROVIDE ( esp_flash_chip_driver_initialized = 0x4004e88c );
 PROVIDE ( esp_flash_erase_chip = 0x4004e998 );
-PROVIDE ( esp_flash_erase_region = 0x4004ea00 );
+PROVIDE ( rom_esp_flash_erase_region = 0x4004ea00 );
 PROVIDE ( esp_flash_get_chip_write_protect = 0x4004eb70 );
 PROVIDE ( esp_flash_get_io_mode = 0x4004efa4 );
 PROVIDE ( esp_flash_get_protectable_regions = 0x4004ec00 );

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"
 
 static const char TAG[] = "spi_flash";
 
@@ -422,6 +423,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
@@ -484,6 +488,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
@@ -502,6 +508,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);
@@ -630,14 +658,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);
@@ -697,15 +725,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);
@@ -748,6 +776,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

components/spi_flash/test/test_esp_flash.c

@@ -609,6 +609,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));