diff --git a/components/spi_flash/cache_utils.c b/components/spi_flash/cache_utils.c index 605c60115..6da7b47e1 100644 --- a/components/spi_flash/cache_utils.c +++ b/components/spi_flash/cache_utils.c @@ -60,6 +60,12 @@ void spi_flash_op_unlock() { xSemaphoreGive(s_flash_op_mutex); } +/* + If you're going to modify this, keep in mind that while the flash caches of the pro and app + cpu are separate, the psram cache is *not*. If one of the CPUs returns from a flash routine + with its cache enabled but the other CPUs cache is not enabled yet, you will have problems + when accessing psram from the former CPU. +*/ void IRAM_ATTR spi_flash_op_block_func(void* arg) { @@ -68,8 +74,6 @@ void IRAM_ATTR spi_flash_op_block_func(void* arg) // Restore interrupts that aren't located in IRAM esp_intr_noniram_disable(); uint32_t cpuid = (uint32_t) arg; - // Disable cache so that flash operation can start - spi_flash_disable_cache(cpuid, &s_flash_op_cache_state[cpuid]); // s_flash_op_complete flag is cleared on *this* CPU, otherwise the other // CPU may reset the flag back to false before IPC task has a chance to check it // (if it is preempted by an ISR taking non-trivial amount of time) @@ -123,8 +127,12 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu() } // Kill interrupts that aren't located in IRAM esp_intr_noniram_disable(); - // Disable cache on this CPU as well + // This CPU executes this routine, with non-IRAM interrupts and the scheduler + // disabled. The other CPU is spinning in the spi_flash_op_block_func task, also + // with non-iram interrupts and the scheduler disabled. None of these CPUs will + // touch external RAM or flash this way, so we can safely disable caches. spi_flash_disable_cache(cpuid, &s_flash_op_cache_state[cpuid]); + spi_flash_disable_cache(other_cpuid, &s_flash_op_cache_state[other_cpuid]); } void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu() @@ -134,22 +142,20 @@ void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu() #ifndef NDEBUG // Sanity check: flash operation ends on the same CPU as it has started assert(cpuid == s_flash_op_cpu); + // More sanity check: if scheduler isn't started, only CPU0 can call this. + assert(!(xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED && cpuid != 0)); s_flash_op_cpu = -1; #endif - // Re-enable cache on this CPU + // Re-enable cache on both CPUs. After this, cache (flash and external RAM) should work again. spi_flash_restore_cache(cpuid, s_flash_op_cache_state[cpuid]); + spi_flash_restore_cache(other_cpuid, s_flash_op_cache_state[other_cpuid]); - if (xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED) { - // Scheduler is not running yet — this means we are running on PRO CPU. - // other_cpuid is APP CPU, and it is either in reset or is spinning in - // user_start_cpu1, which is in IRAM. So we can simply reenable cache. - assert(other_cpuid == 1); - spi_flash_restore_cache(other_cpuid, s_flash_op_cache_state[other_cpuid]); - } else { + if (xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED) { // Signal to spi_flash_op_block_task that flash operation is complete s_flash_op_complete = true; } + // Re-enable non-iram interrupts esp_intr_noniram_enable();