spi_flash: Split large SPI flash operations into parts, allowing preemption

* Erase range operations allow preemption after each block or sector.
* Write operations allow preemption every 8KB of data.
* Reado operations allow preemption every 16KB of data.

components/spi_flash/flash_ops.c

@@ -35,6 +35,12 @@
 /* bytes erased by SPIEraseBlock() ROM function */
 #define BLOCK_ERASE_SIZE 65536
 
+/* Limit number of bytes written/read in a single SPI operation,
+   as these operations disable all higher priority tasks from running.
+*/
+#define MAX_WRITE_CHUNK 8192
+#define MAX_READ_CHUNK 16384
+
 #if CONFIG_SPI_FLASH_ENABLE_COUNTERS
 static const char* TAG = "spi_flash";
 static spi_flash_counters_t s_flash_stats;
@@ -94,19 +100,6 @@ size_t IRAM_ATTR spi_flash_get_chip_size()
     return g_rom_flashchip.chip_size;
 }
 
-static SpiFlashOpResult IRAM_ATTR spi_flash_unlock()
-{
-    static bool unlocked = false;
-    if (!unlocked) {
-        SpiFlashOpResult rc = SPIUnlock();
-        if (rc != SPI_FLASH_RESULT_OK) {
-            return rc;
-        }
-        unlocked = true;
-    }
-    return SPI_FLASH_RESULT_OK;
-}
-
 static inline void IRAM_ATTR spi_flash_guard_start()
 {
     if (s_flash_guard_ops && s_flash_guard_ops->start) {
@@ -135,6 +128,21 @@ static inline void IRAM_ATTR spi_flash_guard_op_unlock()
     }
 }
 
+static SpiFlashOpResult IRAM_ATTR spi_flash_unlock()
+{
+    static bool unlocked = false;
+    if (!unlocked) {
+        spi_flash_guard_start();
+        SpiFlashOpResult rc = SPIUnlock();
+        spi_flash_guard_end();
+        if (rc != SPI_FLASH_RESULT_OK) {
+            return rc;
+        }
+        unlocked = true;
+    }
+    return SPI_FLASH_RESULT_OK;
+}
+
 esp_err_t IRAM_ATTR spi_flash_erase_sector(size_t sec)
 {
     return spi_flash_erase_range(sec * SPI_FLASH_SEC_SIZE, SPI_FLASH_SEC_SIZE);
@@ -155,11 +163,10 @@ esp_err_t IRAM_ATTR spi_flash_erase_range(uint32_t start_addr, uint32_t size)
     size_t end = start + size / SPI_FLASH_SEC_SIZE;
     const size_t sectors_per_block = BLOCK_ERASE_SIZE / SPI_FLASH_SEC_SIZE;
     COUNTER_START();
-    spi_flash_guard_start();
-    SpiFlashOpResult rc;
-    rc = spi_flash_unlock();
+    SpiFlashOpResult rc = spi_flash_unlock();
     if (rc == SPI_FLASH_RESULT_OK) {
         for (size_t sector = start; sector != end && rc == SPI_FLASH_RESULT_OK; ) {
+            spi_flash_guard_start();
             if (sector % sectors_per_block == 0 && end - sector > sectors_per_block) {
                 rc = SPIEraseBlock(sector / sectors_per_block);
                 sector += sectors_per_block;
@@ -169,9 +176,9 @@ esp_err_t IRAM_ATTR spi_flash_erase_range(uint32_t start_addr, uint32_t size)
                 ++sector;
                 COUNTER_ADD_BYTES(erase, SPI_FLASH_SEC_SIZE);
             }
+            spi_flash_guard_end();
         }
     }
-    spi_flash_guard_end();
     COUNTER_STOP(erase);
     return spi_flash_translate_rc(rc);
 }
@@ -202,9 +209,7 @@ esp_err_t IRAM_ATTR spi_flash_write(size_t dst, const void *srcv, size_t size)
     size_t mid_size = (size - left_size) & ~3U;
     size_t right_off = left_size + mid_size;
     size_t right_size = size - mid_size - left_size;
-    spi_flash_guard_start();
     rc = spi_flash_unlock();
-    spi_flash_guard_end();
     if (rc != SPI_FLASH_RESULT_OK) {
         goto out;
     }
@@ -220,43 +225,38 @@ esp_err_t IRAM_ATTR spi_flash_write(size_t dst, const void *srcv, size_t size)
         COUNTER_ADD_BYTES(write, 4);
     }
     if (mid_size > 0) {
-        /* If src buffer is 4-byte aligned as well and is not in a region that
-         * requires cache access to be enabled, we can write it all at once. */
+        /* If src buffer is 4-byte aligned as well and is not in a region that requires cache access to be enabled, we
+         * can write directly without buffering in RAM. */
 #ifdef ESP_PLATFORM
-        bool in_dram = ((uintptr_t) srcc >= 0x3FFAE000 &&
-                        (uintptr_t) srcc < 0x40000000);
+        bool direct_write = ( (uintptr_t) srcc >= 0x3FFAE000
+                              && (uintptr_t) srcc < 0x40000000
+                              && ((uintptr_t) srcc + mid_off) % 4 == 0 );
 #else
-        bool in_dram = true;
+        bool direct_write = true;
 #endif
-        if (in_dram && (((uintptr_t) srcc) + mid_off) % 4 == 0) {
+        while(mid_size > 0 && rc == SPI_FLASH_RESULT_OK) {
+            uint32_t write_buf[8];
+            uint32_t write_size;
+            const uint32_t *write_src = (const uint32_t *) (srcc + mid_off);
+            if (direct_write) {
+                write_size = MIN(mid_size, MAX_WRITE_CHUNK); /* Write in chunks, to avoid starving other CPU/tasks */
+            } else {
+                write_size = MIN(mid_size, sizeof(write_buf));
+                memcpy(write_buf, write_src, write_size);
+                write_src = write_buf;
+            }
             spi_flash_guard_start();
-            rc = SPIWrite(dst + mid_off, (const uint32_t *) (srcc + mid_off), mid_size);
+            rc = SPIWrite(dst + mid_off, write_src, write_size);
             spi_flash_guard_end();
-            if (rc != SPI_FLASH_RESULT_OK) {
-                goto out;
-            }
-            COUNTER_ADD_BYTES(write, mid_size);
-        } else {
-            /*
-             * Otherwise, unlike for read, we cannot manipulate data in the
-             * user-provided buffer, so we write in 32 byte blocks.
-             */
-            while (mid_size > 0) {
-                uint32_t t[8];
-                uint32_t write_size = MIN(mid_size, sizeof(t));
-                memcpy(t, srcc + mid_off, write_size);
-                spi_flash_guard_start();
-                rc = SPIWrite(dst + mid_off, t, write_size);
-                spi_flash_guard_end();
-                if (rc != SPI_FLASH_RESULT_OK) {
-                    goto out;
-                }
-                COUNTER_ADD_BYTES(write, write_size);
-                mid_size -= write_size;
-                mid_off += write_size;
-            }
+            COUNTER_ADD_BYTES(write, write_size);
+            mid_size -= write_size;
+            mid_off += write_size;
+        }
+        if (rc != SPI_FLASH_RESULT_OK) {
+            goto out;
         }
     }
+
     if (right_size > 0) {
         uint32_t t = 0xffffffff;
         memcpy(&t, srcc + right_off, right_size);
@@ -289,12 +289,7 @@ esp_err_t IRAM_ATTR spi_flash_write_encrypted(size_t dest_addr, const void *src,
     }
 
     COUNTER_START();
-    spi_flash_disable_interrupts_caches_and_other_cpu();
-    SpiFlashOpResult rc;
-    spi_flash_guard_start();
-    rc = spi_flash_unlock();
-    spi_flash_guard_end();
-    spi_flash_enable_interrupts_caches_and_other_cpu();
+    SpiFlashOpResult rc = spi_flash_unlock();
 
     if (rc == SPI_FLASH_RESULT_OK) {
         /* SPI_Encrypt_Write encrypts data in RAM as it writes,
@@ -331,9 +326,9 @@ esp_err_t IRAM_ATTR spi_flash_write_encrypted(size_t dest_addr, const void *src,
                 memcpy(encrypt_buf, ssrc + i, 32);
             }
 
-            spi_flash_disable_interrupts_caches_and_other_cpu();
+            spi_flash_guard_start();
             rc = SPI_Encrypt_Write(row_addr, (uint32_t *)encrypt_buf, 32);
-            spi_flash_enable_interrupts_caches_and_other_cpu();
+            spi_flash_guard_end();
             if (rc != SPI_FLASH_RESULT_OK) {
                 break;
             }
@@ -341,6 +336,7 @@ esp_err_t IRAM_ATTR spi_flash_write_encrypted(size_t dest_addr, const void *src,
         bzero(encrypt_buf, sizeof(encrypt_buf));
     }
     COUNTER_ADD_BYTES(write, size);
+    COUNTER_STOP(write);
 
     spi_flash_guard_op_lock();
     spi_flash_mark_modified_region(dest_addr, size);
@@ -402,9 +398,21 @@ esp_err_t IRAM_ATTR spi_flash_read(size_t src, void *dstv, size_t size)
     size_t pad_right_off = (pad_right_src - src);
     size_t pad_right_size = (size - pad_right_off);
     if (mid_size > 0) {
-        rc = SPIRead(src + src_mid_off, (uint32_t *) (dstc + dst_mid_off), mid_size);
-        if (rc != SPI_FLASH_RESULT_OK) {
-            goto out;
+        uint32_t mid_remaining = mid_size;
+        uint32_t mid_read = 0;
+        while (mid_remaining > 0) {
+            uint32_t read_size = MIN(mid_remaining, MAX_READ_CHUNK);
+            rc = SPIRead(src + src_mid_off + mid_read, (uint32_t *) (dstc + dst_mid_off + mid_read), read_size);
+            if (rc != SPI_FLASH_RESULT_OK) {
+                goto out;
+            }
+            mid_remaining -= read_size;
+            mid_read += read_size;
+            if (mid_remaining > 0) {
+                /* Drop guard momentarily, allows other tasks to preempt */
+                spi_flash_guard_end();
+                spi_flash_guard_start();
+            }
         }
         COUNTER_ADD_BYTES(read, mid_size);
         /*

components/spi_flash/test/test_partitions.c

@@ -0,0 +1,68 @@
+// Copyright 2010-2016 Espressif Systems (Shanghai) PTE LTD
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Test for spi_flash_{read,write}.
+
+#include <assert.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/param.h>
+
+#include <unity.h>
+#include <test_utils.h>
+#include <esp_partition.h>
+
+TEST_CASE("Test erase partition", "[spi_flash]")
+{
+    const esp_partition_t *part = get_test_data_partition();
+
+#if CONFIG_SPI_FLASH_ENABLE_COUNTERS
+    spi_flash_reset_counters();
+#endif
+
+    // erase whole partition
+    ESP_ERROR_CHECK( esp_partition_erase_range(part, 0, part->size) );
+
+#if CONFIG_SPI_FLASH_ENABLE_COUNTERS
+    spi_flash_dump_counters();
+#endif
+
+    // put some dummy data on sector boundaries
+    const char *some_data = "abcdefghijklmn";
+    for (int i = 0; i < part->size; i+= 4096) {
+        ESP_ERROR_CHECK( esp_partition_write(part, i, some_data, strlen(some_data)) );
+    }
+
+    // check it's there!
+    char buf[strlen(some_data)];
+    for (int i = 0; i < part->size; i+= 4096) {
+        memset(buf, 0x00, sizeof(buf));
+        ESP_ERROR_CHECK( esp_partition_read(part, i, buf, sizeof(buf)) );
+        TEST_ASSERT_EQUAL_INT(0, strncmp(buf, some_data, sizeof(buf)));
+    }
+
+    // erase the whole thing again
+    ESP_ERROR_CHECK( esp_partition_erase_range(part, 0, part->size) );
+
+    // check it's gone
+    for (int i = 0; i < part->size; i+= 4096) {
+        memset(buf, 0x00, sizeof(buf));
+        ESP_ERROR_CHECK( esp_partition_read(part, i, buf, sizeof(buf)) );
+        for (int i = 0; i < sizeof(buf); i++) {
+            TEST_ASSERT_EQUAL_HEX8(0xFF, buf[i]);
+        }
+    }
+
+}

components/spi_flash/test/test_read_write.c

@@ -87,7 +87,7 @@ static void IRAM_ATTR test_read(int src_off, int dst_off, int len)
     TEST_ASSERT_EQUAL_INT(cmp_or_dump(dst_buf, dst_gold, sizeof(dst_buf)), 0);
 }
 
-TEST_CASE("Test spi_flash_read", "[spi_flash_read]")
+TEST_CASE("Test spi_flash_read", "[spi_flash]")
 {
     setup_tests();
 #if CONFIG_SPI_FLASH_MINIMAL_TEST
@@ -165,7 +165,7 @@ static void IRAM_ATTR test_write(int dst_off, int src_off, int len)
     TEST_ASSERT_EQUAL_INT(cmp_or_dump(dst_buf, dst_gold, sizeof(dst_buf)), 0);
 }
 
-TEST_CASE("Test spi_flash_write", "[spi_flash_write]")
+TEST_CASE("Test spi_flash_write", "[spi_flash]")
 {
     setup_tests();
 #if CONFIG_SPI_FLASH_MINIMAL_TEST