From 64117a0c5932ec8138bfd702fa2f202df732bc71 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Thu, 21 Jul 2022 19:14:10 +0800 Subject: [PATCH] esp_system: Fix esp_cpu_compare_and_set() This commit fixes esp_cpu_compare_and_set() in the following ways - Removed call to esp_ptr_external_ram() as it incurred > 80 CPU cycles (due to multiple nested function calls, and those functions not being in IRAM). We now check manually if the pointer is in external RAM for increased speed. - Fixed infinite wait when attempting to get the external_ram_cas_lock. The function should return immediatley if any part of the compare and set call fails. - The preprocessor conditions of esp_cpu_compare_and_set() to depend on CONFIG_SPIRAM instead of SOC_SPIRAM_SUPPORTED. Even if the target supports SPIRAM, we only need the external RAM compare and set feature if SPIRAM is enabled. Also fixed incorrect inclusion of esp_intr_alloc.h in esp_cpu.h --- components/esp_hw_support/cpu.c | 32 ++++++++++++--------- components/esp_hw_support/include/esp_cpu.h | 1 + 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/components/esp_hw_support/cpu.c b/components/esp_hw_support/cpu.c index 163755b86d..6292cc22d4 100644 --- a/components/esp_hw_support/cpu.c +++ b/components/esp_hw_support/cpu.c @@ -620,7 +620,7 @@ esp_err_t esp_cpu_clear_watchpoint(int wp_num) * * ------------------------------------------------------------------------------------------------------------------ */ -#if __XTENSA__ && XCHAL_HAVE_S32C1I && SOC_SPIRAM_SUPPORTED +#if __XTENSA__ && XCHAL_HAVE_S32C1I && CONFIG_SPIRAM static DRAM_ATTR uint32_t external_ram_cas_lock = 0; #endif @@ -628,35 +628,39 @@ bool esp_cpu_compare_and_set(volatile uint32_t *addr, uint32_t compare_value, ui { #if __XTENSA__ bool ret; -#if XCHAL_HAVE_S32C1I && SOC_SPIRAM_SUPPORTED - if (esp_ptr_external_ram((const void *)addr)) { +#if XCHAL_HAVE_S32C1I && CONFIG_SPIRAM + // Check if the target address is in external RAM + if ((uint32_t)addr >= SOC_EXTRAM_DATA_LOW && (uint32_t)addr < SOC_EXTRAM_DATA_HIGH) { + /* The target address is in external RAM, thus the native CAS instruction cannot be used. Instead, we achieve + atomicity by disabling interrupts and then acquiring an external RAM CAS lock. */ uint32_t intr_level; - // Atomicity is achieved by disabling interrupts then acquiring a an external RAM CAS lock __asm__ __volatile__ ("rsil %0, " XTSTR(XCHAL_EXCM_LEVEL) "\n" : "=r"(intr_level)); - while (!xt_utils_compare_and_set(&external_ram_cas_lock, 0, 1)) { - ; + if (!xt_utils_compare_and_set(&external_ram_cas_lock, 0, 1)) { + // External RAM CAS lock already taken. Exit + ret = false; + goto exit; } // Now we compare and set the target address - uint32_t old_value; - old_value = *addr; - if (old_value == compare_value) { + ret = (*addr == compare_value); + if (ret) { *addr = new_value; } - // Release the external RAM CAS lock and reenable interrupts + // Release the external RAM CAS lock external_ram_cas_lock = 0; +exit: + // Reenable interrupts __asm__ __volatile__ ("memw \n" "wsr %0, ps\n" :: "r"(intr_level)); - - ret = (old_value == compare_value); } else -#endif //XCHAL_HAVE_S32C1I && SOC_SPIRAM_SUPPORTED +#endif // XCHAL_HAVE_S32C1I && CONFIG_SPIRAM { + // The target address is in internal RAM. Use the CPU's native CAS instruction ret = xt_utils_compare_and_set(addr, compare_value, new_value); } return ret; -#else +#else // __XTENSA__ // Single core targets don't have atomic CAS instruction. So access method is the same for internal and external RAM return rv_utils_compare_and_set(addr, compare_value, new_value); #endif diff --git a/components/esp_hw_support/include/esp_cpu.h b/components/esp_hw_support/include/esp_cpu.h index c4d74d93e8..2da71d54cc 100644 --- a/components/esp_hw_support/include/esp_cpu.h +++ b/components/esp_hw_support/include/esp_cpu.h @@ -17,6 +17,7 @@ #elif __riscv #include "riscv/rv_utils.h" #endif +#include "esp_intr_alloc.h" #include "esp_err.h" #ifdef __cplusplus