Merge branch 'bugfix/spi_flash_large_writes' into 'master'

spi_flash: Fix large writes, unaligned writes, and writes from flash

See merge request !843

components/log/include/esp_log.h

@@ -102,7 +102,7 @@ void esp_log_write(esp_log_level_t level, const char* tag, const char* format, .
  *
  * @param  buffer   Pointer to the buffer array
  *
- * @param  buff_len length of buffer
+ * @param  buff_len length of buffer in bytes
  *
  */
 void esp_log_buffer_hex(const char *tag, const void *buffer, uint16_t buff_len);
@@ -114,7 +114,7 @@ void esp_log_buffer_hex(const char *tag, const void *buffer, uint16_t buff_len);
  *
  * @param  buffer   Pointer to the buffer array
  *
- * @param  buff_len length of buffer
+ * @param  buff_len length of buffer in bytes
  *
  */
 void esp_log_buffer_char(const char *tag, const void *buffer, uint16_t buff_len);

components/spi_flash/flash_ops.c

@@ -197,7 +197,7 @@ esp_err_t IRAM_ATTR spi_flash_write(size_t dst, const void *srcv, size_t size)
 
     esp_rom_spiflash_result_t rc = ESP_ROM_SPIFLASH_RESULT_OK;
     COUNTER_START();
-    const char *srcc = (const char *) srcv;
+    const uint8_t *srcc = (const uint8_t *) srcv;
     /*
      * Large operations are split into (up to) 3 parts:
      * - Left padding: 4 bytes up to the first 4-byte aligned destination offset.
@@ -210,6 +210,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;
+
     rc = spi_flash_unlock();
     if (rc != ESP_ROM_SPIFLASH_RESULT_OK) {
         goto out;
@@ -237,17 +238,15 @@ esp_err_t IRAM_ATTR spi_flash_write(size_t dst, const void *srcv, size_t size)
 #endif
         while(mid_size > 0 && rc == ESP_ROM_SPIFLASH_RESULT_OK) {
             uint32_t write_buf[8];
-            uint32_t write_size;
+            uint32_t write_size = MIN(mid_size, MAX_WRITE_CHUNK);
-            const uint32_t *write_src = (const uint32_t *) (srcc + mid_off);
+            const uint8_t *write_src = srcc + mid_off;
-            if (direct_write) {
+            if (!direct_write) {
-                write_size = MIN(mid_size, MAX_WRITE_CHUNK); /* Write in chunks, to avoid starving other CPU/tasks */
+                write_size = MIN(write_size, sizeof(write_buf));
-            } else {
-                write_size = MIN(mid_size, sizeof(write_buf));
                 memcpy(write_buf, write_src, write_size);
-                write_src = write_buf;
+                write_src = (const uint8_t *)write_buf;
             }
             spi_flash_guard_start();
-            rc = esp_rom_spiflash_write(dst + mid_off, (const uint32_t *) (srcc + mid_off), mid_size);
+            rc = esp_rom_spiflash_write(dst + mid_off, (const uint32_t *) write_src, write_size);
             spi_flash_guard_end();
             COUNTER_ADD_BYTES(write, write_size);
             mid_size -= write_size;
@@ -369,10 +368,10 @@ esp_err_t IRAM_ATTR spi_flash_read(size_t src, void *dstv, size_t size)
             goto out;
         }
         COUNTER_ADD_BYTES(read, read_size);
-        memcpy(dstv, ((char *) t) + left_off, size);
+        memcpy(dstv, ((uint8_t *) t) + left_off, size);
         goto out;
     }
-    char *dstc = (char *) dstv;
+    uint8_t *dstc = (uint8_t *) dstv;
     intptr_t dsti = (intptr_t) dstc;
     /*
      * Large operations are split into (up to) 3 parts:

components/spi_flash/test/test_large_flash_writes.c

@@ -0,0 +1,96 @@
+// 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_write() with large buffers (in RAM or on flash)
+
+#include <assert.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/param.h>
+
+#include <unity.h>
+#include <test_utils.h>
+#include <esp_spi_flash.h>
+#include <esp_log.h>
+#include <rom/spi_flash.h>
+#include "../cache_utils.h"
+#include "soc/timer_group_struct.h"
+#include "soc/timer_group_reg.h"
+
+static const uint8_t large_const_buffer[16400] = {
+    203, // first byte
+    1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,
+    21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,
+    [50 ... 99] = 2,
+    [1600 ... 2000] = 3,
+    [8000 ... 9000] = 77,
+    [15000 ... 16398] = 8,
+    43 // last byte
+};
+
+static void test_write_large_buffer(const uint8_t *source, size_t length);
+
+TEST_CASE("Test spi_flash_write large const buffer", "[spi_flash]")
+{
+    // buffer in flash
+    test_write_large_buffer(large_const_buffer, sizeof(large_const_buffer));
+}
+
+TEST_CASE("Test spi_flash_write large RAM buffer", "[spi_flash]")
+{
+    // buffer in RAM
+    uint8_t *source_buf = malloc(sizeof(large_const_buffer));
+    TEST_ASSERT_NOT_NULL(source_buf);
+    memcpy(source_buf, large_const_buffer, sizeof(large_const_buffer));
+    test_write_large_buffer(source_buf, sizeof(large_const_buffer));
+    free(source_buf);
+}
+
+static void test_write_large_buffer(const uint8_t *source, size_t length)
+{
+    const esp_partition_t *part = get_test_data_partition();
+    TEST_ASSERT(part->size > length + 2 + SPI_FLASH_SEC_SIZE);
+
+    printf("Writing %d bytes from source %p\n", length, source);
+
+    uint8_t *buf = malloc(length);
+    TEST_ASSERT_NOT_NULL(buf);
+
+    ESP_ERROR_CHECK( spi_flash_erase_range(part->address, (length + SPI_FLASH_SEC_SIZE) & ~(SPI_FLASH_SEC_SIZE-1)) );
+
+    // note writing to unaligned address
+    ESP_ERROR_CHECK( spi_flash_write(part->address + 1, source, length) );
+
+    ESP_ERROR_CHECK( spi_flash_read(part->address + 1, buf, length) );
+
+    TEST_ASSERT_EQUAL_HEX8_ARRAY(source, buf, length);
+
+    free(buf);
+
+    // check nothing was written at beginning or end
+    uint8_t ends[8];
+
+    ESP_ERROR_CHECK( spi_flash_read(part->address, ends, sizeof(ends)) );
+    TEST_ASSERT_EQUAL_HEX8(0xFF, ends[0]);
+    TEST_ASSERT_EQUAL_HEX8(source[0] , ends[1]);
+
+    ESP_ERROR_CHECK( spi_flash_read(part->address + length, ends, sizeof(ends)) );
+
+    TEST_ASSERT_EQUAL_HEX8(source[length-1], ends[0]);
+    TEST_ASSERT_EQUAL_HEX8(0xFF, ends[1]);
+    TEST_ASSERT_EQUAL_HEX8(0xFF, ends[2]);
+    TEST_ASSERT_EQUAL_HEX8(0xFF, ends[3]);
+}