kopia lustrzana https://github.com/espressif/esp-idf
Merge branch 'bugfix/spi_flash_lock_period' into 'master'
SPI flash operations lock for shorter periods Based on bug report here: https://esp32.com/viewtopic.php?f=13&t=1489&p=6816#p6773 Long SPI flash erase (and possibly write/read) cycles block all tasks on both CPUs for an extended period, and can lead to WiFi dissassociation (and general priority inversion.) This MR inserts preemption breaks into large operations (all erases, writes every 8KB, reads every 16KB). Overhead of a single spi_flash_guart_start()/spi_flash_guard_end() cycle measured at approx 67us (assuming no preemption.) See merge request !600pull/430/merge
Ivan Grokhotkov
commit
710c853adc
|
|
@ -85,6 +85,7 @@ SECTIONS
|
|||
*libfreertos.a:(.literal .text .literal.* .text.*)
|
||||
*libesp32.a:panic.o(.literal .text .literal.* .text.*)
|
||||
*libesp32.a:core_dump.o(.literal .text .literal.* .text.*)
|
||||
*libesp32.a:heap_alloc_caps.o(.literal .text .literal.* .text.*)
|
||||
*libphy.a:(.literal .text .literal.* .text.*)
|
||||
*librtc.a:(.literal .text .literal.* .text.*)
|
||||
*librtc_clk.a:(.literal .text .literal.* .text.*)
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
/*
|
||||
|
|
@ -483,7 +491,7 @@ static esp_err_t IRAM_ATTR spi_flash_translate_rc(SpiFlashOpResult rc)
|
|||
|
||||
static inline void dump_counter(spi_flash_counter_t* counter, const char* name)
|
||||
{
|
||||
ESP_LOGI(TAG, "%s count=%8d time=%8dms bytes=%8d\n", name,
|
||||
ESP_LOGI(TAG, "%s count=%8d time=%8dus bytes=%8d\n", name,
|
||||
counter->count, counter->time, counter->bytes);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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]);
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@
|
|||
#include "freertos/FreeRTOS.h"
|
||||
#include "freertos/task.h"
|
||||
#include "esp_log.h"
|
||||
#include "soc/cpu.h"
|
||||
|
||||
#define unity_printf ets_printf
|
||||
|
||||
|
|
@ -167,10 +168,13 @@ void unity_run_menu()
|
|||
int test_index = strtol(cmdline, NULL, 10);
|
||||
if (test_index >= 1 && test_index <= test_count)
|
||||
{
|
||||
uint32_t start = esp_log_timestamp(); /* hacky way to get ms */
|
||||
uint32_t start;
|
||||
RSR(CCOUNT, start);
|
||||
unity_run_single_test_by_index(test_index - 1);
|
||||
uint32_t end = esp_log_timestamp();
|
||||
printf("Test ran in %dms\n", end - start);
|
||||
uint32_t end;
|
||||
RSR(CCOUNT, end);
|
||||
uint32_t ms = (end - start) / (XT_CLOCK_FREQ / 1000);
|
||||
printf("Test ran in %dms\n", ms);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,8 +11,10 @@ void unityTask(void *pvParameters)
|
|||
while(1);
|
||||
}
|
||||
|
||||
void app_main()
|
||||
void app_main()
|
||||
{
|
||||
// Note: if unpinning this task, change the way run times are calculated in
|
||||
// unity_platform
|
||||
xTaskCreatePinnedToCore(unityTask, "unityTask", 4096, NULL, 5, NULL, 0);
|
||||
}
|
||||
|
||||
|
|
|
|||
Ładowanie…
Reference in New Issue