spi_flash: fix spi_flash_read into buffer in external RAM, add test

diff --git a/components/spi_flash/flash_ops.c b/components/spi_flash/flash_ops.c
index afb8f407be40e760ba6f68e8e5882b2834200296..991b7deae33a61b4eb6b2ff6a5d37a72f315ede4 100644 (file)
--- a/components/spi_flash/flash_ops.c
+++ b/components/spi_flash/flash_ops.c
@@ -230,9 +230,9 @@ esp_err_t IRAM_ATTR spi_flash_write(size_t dst, const void *srcv, size_t size)
         /* 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 direct_write = ( (uintptr_t) srcc >= 0x3FFAE000
-                              && (uintptr_t) srcc < 0x40000000
-                              && ((uintptr_t) srcc + mid_off) % 4 == 0 );
+        bool direct_write = esp_ptr_internal(srcc)
+                && esp_ptr_byte_accessible(srcc)
+                && ((uintptr_t) srcc + mid_off) % 4 == 0;
 #else
         bool direct_write = true;
 #endif
@@ -395,18 +395,38 @@ esp_err_t IRAM_ATTR spi_flash_read(size_t src, void *dstv, size_t size)
     size_t pad_right_src = (src + pad_left_size + mid_size) & ~3U;
     size_t pad_right_off = (pad_right_src - src);
     size_t pad_right_size = (size - pad_right_off);
+
+#ifdef ESP_PLATFORM
+    bool direct_read = esp_ptr_internal(dstc)
+            && esp_ptr_byte_accessible(dstc)
+            && ((uintptr_t) dstc + dst_mid_off) % 4 == 0;
+#else
+    bool direct_read = true;
+#endif
     if (mid_size > 0) {
         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 = esp_rom_spiflash_read(src + src_mid_off + mid_read, (uint32_t *) (dstc + dst_mid_off + mid_read), read_size);
+            uint32_t read_buf[8];
+            uint8_t *read_dst_final = dstc + dst_mid_off + mid_read;
+            uint8_t *read_dst = read_dst_final;
+            if (!direct_read) {
+                read_size = MIN(read_size, sizeof(read_buf));
+                read_dst = (uint8_t *) read_buf;
+            }
+            rc = esp_rom_spiflash_read(src + src_mid_off + mid_read,
+                    (uint32_t *) read_dst, read_size);
             if (rc != ESP_ROM_SPIFLASH_RESULT_OK) {
                 goto out;
             }
             mid_remaining -= read_size;
             mid_read += read_size;
-            if (mid_remaining > 0) {
+            if (!direct_read) {
+                spi_flash_guard_end();
+                memcpy(read_dst_final, read_buf, read_size);
+                spi_flash_guard_start();
+            } else if (mid_remaining > 0) {
                 /* Drop guard momentarily, allows other tasks to preempt */
                 spi_flash_guard_end();
                 spi_flash_guard_start();
@@ -419,7 +439,13 @@ esp_err_t IRAM_ATTR spi_flash_read(size_t src, void *dstv, size_t size)
          * Note that the shift can be left (src_mid_off < dst_mid_off) or right.
          */
         if (src_mid_off != dst_mid_off) {
+            if (!direct_read) {
+                spi_flash_guard_end();
+            }
             memmove(dstc + src_mid_off, dstc + dst_mid_off, mid_size);
+            if (!direct_read) {
+                spi_flash_guard_start();
+            }
         }
     }
     if (pad_left_size > 0) {
@@ -429,7 +455,13 @@ esp_err_t IRAM_ATTR spi_flash_read(size_t src, void *dstv, size_t size)
             goto out;
         }
         COUNTER_ADD_BYTES(read, 4);
+        if (!direct_read) {
+            spi_flash_guard_end();
+        }
         memcpy(dstc, ((uint8_t *) &t) + (4 - pad_left_size), pad_left_size);
+        if (!direct_read) {
+            spi_flash_guard_start();
+        }
     }
     if (pad_right_size > 0) {
         uint32_t t[2];
@@ -439,7 +471,13 @@ esp_err_t IRAM_ATTR spi_flash_read(size_t src, void *dstv, size_t size)
             goto out;
         }
         COUNTER_ADD_BYTES(read, read_size);
+        if (!direct_read) {
+            spi_flash_guard_end();
+        }
         memcpy(dstc + pad_right_off, t, pad_right_size);
+        if (!direct_read) {
+            spi_flash_guard_start();
+        }
     }
 out:
     spi_flash_guard_end();

diff --git a/components/spi_flash/test/test_read_write.c b/components/spi_flash/test/test_read_write.c
index 147e4c3c9d1e83fab61c2f262dd1e8a05c3eb32c..8b1061b96d1f5519b8f4275ecb17020008ff0299 100644 (file)
--- a/components/spi_flash/test/test_read_write.c
+++ b/components/spi_flash/test/test_read_write.c
@@ -27,6 +27,7 @@
 #include "../cache_utils.h"
 #include "soc/timer_group_struct.h"
 #include "soc/timer_group_reg.h"
+#include "esp_heap_caps.h"
 
 /* Base offset in flash for tests. */
 static size_t start;
@@ -217,3 +218,51 @@ TEST_CASE("Test spi_flash_write", "[spi_flash]")
     ESP_ERROR_CHECK(spi_flash_write(start, (char *) 0x40078000, 16));
     ESP_ERROR_CHECK(spi_flash_write(start, (char *) 0x40080000, 16));
 }
+
+#ifdef CONFIG_SPIRAM_SUPPORT
+
+TEST_CASE("spi_flash_read can read into buffer in external RAM", "[spi_flash]")
+{
+    uint8_t* buf_ext = (uint8_t*) heap_caps_malloc(SPI_FLASH_SEC_SIZE, MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT);
+    TEST_ASSERT_NOT_NULL(buf_ext);
+
+    uint8_t* buf_int = (uint8_t*) heap_caps_malloc(SPI_FLASH_SEC_SIZE, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+    TEST_ASSERT_NOT_NULL(buf_int);
+
+    TEST_ESP_OK(spi_flash_read(0x1000, buf_int, SPI_FLASH_SEC_SIZE));
+    TEST_ESP_OK(spi_flash_read(0x1000, buf_ext, SPI_FLASH_SEC_SIZE));
+
+    TEST_ASSERT_EQUAL(0, memcmp(buf_ext, buf_int, SPI_FLASH_SEC_SIZE));
+    free(buf_ext);
+    free(buf_int);
+}
+
+TEST_CASE("spi_flash_write can write from external RAM buffer", "[spi_flash]")
+{
+    uint32_t* buf_ext = (uint32_t*) heap_caps_malloc(SPI_FLASH_SEC_SIZE, MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT);
+    TEST_ASSERT_NOT_NULL(buf_ext);
+
+    srand(0);
+    for (size_t i = 0; i < SPI_FLASH_SEC_SIZE / sizeof(uint32_t); i++)
+    {
+        uint32_t val = rand();
+        buf_ext[i] = val;
+    }
+
+    uint8_t* buf_int = (uint8_t*) heap_caps_malloc(SPI_FLASH_SEC_SIZE, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+    TEST_ASSERT_NOT_NULL(buf_int);
+
+    /* Write to flash from buf_ext */
+    const esp_partition_t *part = get_test_data_partition();
+    TEST_ESP_OK(spi_flash_erase_range(part->address, SPI_FLASH_SEC_SIZE));
+    TEST_ESP_OK(spi_flash_write(part->address, buf_ext, SPI_FLASH_SEC_SIZE));
+
+    /* Read back to buf_int and compare */
+    TEST_ESP_OK(spi_flash_read(part->address, buf_int, SPI_FLASH_SEC_SIZE));
+    TEST_ASSERT_EQUAL(0, memcmp(buf_ext, buf_int, SPI_FLASH_SEC_SIZE));
+
+    free(buf_ext);
+    free(buf_int);
+}
+
+#endif // CONFIG_SPIRAM_SUPPORT