spi_flash: support write verify feature on esp_flash_write API

components/spi_flash/Kconfig

@@ -3,6 +3,7 @@ menu "SPI Flash driver"
 
     config SPI_FLASH_VERIFY_WRITE
         bool "Verify SPI flash writes"
+        depends on !SPI_FLASH_ROM_IMPL
         default n
         help
             If this option is enabled, any time SPI flash is written then the data will be read

components/spi_flash/esp_flash_api.c

@@ -23,7 +23,7 @@
 #include "esp_crypto_lock.h" // for locking flash encryption peripheral
 #endif //CONFIG_IDF_TARGET_ESP32S2
 
-static const char TAG[] = "spi_flash";
+DRAM_ATTR static const char TAG[] = "spi_flash";
 
 #ifdef CONFIG_SPI_FLASH_WRITE_CHUNK_SIZE
 #define MAX_WRITE_CHUNK CONFIG_SPI_FLASH_WRITE_CHUNK_SIZE /* write in chunks */
@@ -32,6 +32,7 @@ static const char TAG[] = "spi_flash";
 #endif // CONFIG_SPI_FLASH_WRITE_CHUNK_SIZE
 
 #define MAX_READ_CHUNK 16384
+#define VERIFY_BUF_LEN 64
 
 
 #ifdef CONFIG_SPI_FLASH_DANGEROUS_WRITE_ABORTS
@@ -847,10 +848,89 @@ esp_err_t IRAM_ATTR esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t add
     return err;
 }
 
+#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE
+static esp_err_t IRAM_ATTR s_check_setting_zero_to_one(esp_flash_t *chip, uint32_t verify_address, uint32_t remain_verify_len, const uint32_t *to_write_buf, bool is_encrypted)
+{
+    esp_err_t err = ESP_FAIL;
+    uint8_t verify_buffer[VERIFY_BUF_LEN];
+    uint32_t *val_in_flash = (uint32_t *)verify_buffer;
+
+    while (remain_verify_len) {
+        uint32_t this_len = MIN(remain_verify_len, VERIFY_BUF_LEN);
+
+        err = chip->chip_drv->read(chip, verify_buffer, verify_address, this_len);
+        if (err != ESP_OK) {
+            ESP_DRAM_LOGE(TAG, "failed to read flash to verify if setting zero to one, err: 0x%x", err);
+            return err;
+        }
+
+        for (int r = 0; r < this_len / sizeof(uint32_t); r++) {
+            if (is_encrypted) {
+                (void)to_write_buf;
+                if (val_in_flash[r] != 0xFFFFFFFF) {
+                    ESP_DRAM_LOGW(TAG, "Write at offset 0x%x but not erased (0x%08x)",
+                                  verify_address + r, val_in_flash[r]);
+                }
+            } else {
+                if ((val_in_flash[r] & to_write_buf[r]) != to_write_buf[r]) {
+                    ESP_DRAM_LOGW(TAG, "Write at offset 0x%x requests 0x%08x but will write 0x%08x -> 0x%08x",
+                                  verify_address + r, to_write_buf[r], val_in_flash[r], (val_in_flash[r] & to_write_buf[r]));
+                }
+            }
+        }
+
+        remain_verify_len -= this_len;
+        verify_address += this_len;
+    }
+
+    return ESP_OK;
+}
+#endif  //#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE
+
+#if CONFIG_SPI_FLASH_VERIFY_WRITE
+static esp_err_t IRAM_ATTR s_verify_write(esp_flash_t *chip, uint32_t verify_address, uint32_t remain_verify_len, const uint32_t *expected_buf, bool is_encrypted)
+{
+    esp_err_t err = ESP_FAIL;
+    uint8_t verify_buffer[VERIFY_BUF_LEN];
+    uint32_t *val_in_flash = (uint32_t *)verify_buffer;
+
+    while (remain_verify_len) {
+        uint32_t this_len = MIN(remain_verify_len, VERIFY_BUF_LEN);
+
+        if (is_encrypted) {
+            err = esp_flash_read_encrypted(chip, verify_address, verify_buffer, this_len);
+        } else {
+            err = chip->chip_drv->read(chip, verify_buffer, verify_address, this_len);
+        }
+        if (err != ESP_OK) {
+            ESP_DRAM_LOGE(TAG, "failed to read flash to verify previous write, err: 0x%x", err);
+            return err;
+        }
+
+        for (int r = 0; r < this_len / sizeof(uint32_t); r++) {
+            if (val_in_flash[r] != expected_buf[r]) {
+#if CONFIG_SPI_FLASH_LOG_FAILED_WRITE
+                ESP_DRAM_LOGE(TAG, "Bad write at %d offset: 0x%x, expected: 0x%08x, readback: 0x%08x", r, verify_address + r, expected_buf[r], val_in_flash[r]);
+#endif  //#if CONFIG_SPI_FLASH_LOG_FAILED_WRITE
+                return ESP_FAIL;
+            }
+        }
+
+        expected_buf = (uint32_t *)((void *)expected_buf + this_len);
+        remain_verify_len -= this_len;
+        verify_address += this_len;
+    }
+
+    return ESP_OK;
+}
+#endif //#if CONFIG_SPI_FLASH_VERIFY_WRITE
+
 esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint32_t address, uint32_t length)
 {
+    esp_err_t ret = ESP_FAIL;
 #if CONFIG_SPI_FLASH_VERIFY_WRITE
-    const uint32_t *except_buf = buffer;
+    //used for verify write
+    bool is_encrypted = false;
 #endif //CONFIG_SPI_FLASH_VERIFY_WRITE
 
     esp_err_t err = rom_spiflash_api_funcs->chip_check(&chip);
@@ -902,25 +982,45 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3
 
         err = rom_spiflash_api_funcs->start(chip);
         if (err != ESP_OK) {
-            break;
+            goto restore_cache;
         }
         bus_acquired = true;
 
+#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE
+        err = s_check_setting_zero_to_one(chip, write_addr, write_len, write_buf, is_encrypted);
+        if (err != ESP_OK) {
+            //Error happens, we end flash operation. Re-enable cache and flush it
+            goto restore_cache;
+        }
+#endif  //#if CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE
+
         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) {
+        if (err != ESP_OK) {
-            // On ESP32, the cache re-enable is in the end() function, while flush_cache should
+            //Error happens, we end flash operation. Re-enable cache and flush it
-            // happen when the cache is still disabled on ESP32. Break before the end() function and
-            // do end() later
             assert(bus_acquired);
+            goto restore_cache;
+        }
+
+#if CONFIG_SPI_FLASH_VERIFY_WRITE
+        err = s_verify_write(chip, write_addr, write_len, write_buf, is_encrypted);
+        if (err != ESP_OK) {
+            //Error happens, we end flash operation. Re-enable cache and flush it
+            goto restore_cache;
+        }
+#endif  //#if CONFIG_SPI_FLASH_VERIFY_WRITE
+
+
+        if (len_remain == 0) {
+            //Flash operation done
             break;
         }
 
         err = rom_spiflash_api_funcs->end(chip, err);
         if (err != ESP_OK) {
-            break;
+            goto restore_cache;
         }
         bus_acquired = false;
 
@@ -930,19 +1030,14 @@ esp_err_t IRAM_ATTR esp_flash_write(esp_flash_t *chip, const void *buffer, uint3
 
     err = rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length);
 
-#if CONFIG_SPI_FLASH_VERIFY_WRITE
+    return err;
-    uint32_t *actual_buf = malloc(length);;
-    esp_flash_read(chip, actual_buf, address, length);
 
-    for (int r = 0; r < length / sizeof(uint32_t); r++) {
+restore_cache:
-        if (actual_buf[r] != except_buf[r]) {
-            ESP_LOGE(TAG, "Bad write at %d offset: 0x%x, expected: 0x%08x, readback: 0x%08x",r, address + r, except_buf[r], actual_buf[r]);
-            err = ESP_FAIL;
-        }
-    }
 
-    free(actual_buf);
+    ret = rom_spiflash_api_funcs->flash_end_flush_cache(chip, err, bus_acquired, address, length);
-#endif //CONFIG_SPI_FLASH_VERIFY_WRITE
+    if (ret != ESP_OK) {
+       ESP_DRAM_LOGE(TAG, "restore cache fail\n");
+    }
 
     return err;
 }

components/spi_flash/include/esp_flash.h

@@ -317,6 +317,7 @@ esp_err_t esp_flash_read(esp_flash_t *chip, void *buffer, uint32_t address, uint
  *
  * @return
  *      - ESP_OK on success,
+ *      - ESP_FAIL, bad write, this will be detected only when CONFIG_SPI_FLASH_VERIFY_WRITE is enabled
  *      - ESP_ERR_NOT_SUPPORTED if the chip is not able to perform the operation. This is indicated by WREN = 1 after the command is sent.
  *      - Other flash error code if operation failed.
  */

components/spi_flash/test_apps/esp_flash/pytest_esp_flash.py

@@ -16,6 +16,7 @@ from pytest_embedded import Dut
     [
         'release',
         'flash_qio',
+        'verify'
     ],
     indirect=True,
 )

components/spi_flash/test_apps/esp_flash/sdkconfig.ci.flash_qio

@@ -1,4 +1,2 @@
 CONFIG_ESP_TASK_WDT_EN=n
-CONFIG_PARTITION_TABLE_CUSTOM=y
-CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv"
 CONFIG_ESPTOOLPY_FLASHMODE_QIO=y

components/spi_flash/test_apps/esp_flash/sdkconfig.ci.release

@@ -3,5 +3,3 @@ CONFIG_FREERTOS_USE_TICKLESS_IDLE=y
 CONFIG_COMPILER_OPTIMIZATION_SIZE=y
 CONFIG_BOOTLOADER_COMPILER_OPTIMIZATION_SIZE=y
 CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y
-CONFIG_PARTITION_TABLE_CUSTOM=y
-CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv"

components/spi_flash/test_apps/esp_flash/sdkconfig.ci.rom_impl

@@ -1,4 +1,2 @@
 CONFIG_ESP_TASK_WDT_EN=n
-CONFIG_PARTITION_TABLE_CUSTOM=y
-CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv"
 CONFIG_SPI_FLASH_ROM_IMPL=y

components/spi_flash/test_apps/esp_flash/sdkconfig.ci.verify

@@ -1,4 +1,4 @@
 CONFIG_ESP_TASK_WDT=n
-CONFIG_PARTITION_TABLE_CUSTOM=y
-CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions.csv"
 CONFIG_SPI_FLASH_VERIFY_WRITE=y
+CONFIG_SPI_FLASH_LOG_FAILED_WRITE=y
+CONFIG_SPI_FLASH_WARN_SETTING_ZERO_TO_ONE=y