From b23c9142d5a8edc3d75e7acd0c8f7111629237cf Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Wed, 24 Feb 2021 12:07:11 +0800 Subject: [PATCH] [hal]: cleaned up interrupt mask functions * Functions for setting and clearing interrupts as well as function to read interrupt mask should be clearer now. * Using hal layer interrupt set and clear functions in esp_wifi component --- components/esp_system/intr_alloc.c | 39 +++++++++++------- components/esp_wifi/CMakeLists.txt | 2 +- components/esp_wifi/esp32/esp_adapter.c | 5 ++- components/esp_wifi/esp32s2/esp_adapter.c | 5 ++- components/esp_wifi/esp32s3/esp_adapter.c | 5 ++- .../include/hal/interrupt_controller_ll.h | 35 +++++++--------- .../include/hal/interrupt_controller_ll.h | 40 +++++------------- .../include/hal/interrupt_controller_ll.h | 35 +++++++--------- .../include/hal/interrupt_controller_ll.h | 35 +++++++--------- .../include/hal/interrupt_controller_hal.h | 31 +++++--------- components/xtensa/include/xtensa/xtensa_api.h | 41 ------------------- 11 files changed, 96 insertions(+), 177 deletions(-) diff --git a/components/esp_system/intr_alloc.c b/components/esp_system/intr_alloc.c index 7e7de6dbe9..012b1ed21b 100644 --- a/components/esp_system/intr_alloc.c +++ b/components/esp_system/intr_alloc.c @@ -456,7 +456,7 @@ esp_err_t esp_intr_alloc_intrstatus(int source, int flags, uint32_t intrstatusre { intr_handle_data_t *ret=NULL; int force=-1; - ESP_EARLY_LOGV(TAG, "esp_intr_alloc_intrstatus (cpu %d): checking args", cpu_hal_get_core_id()); + ESP_EARLY_LOGV(TAG, "esp_intr_alloc_intrstatus (cpu %u): checking args", cpu_hal_get_core_id()); //Shared interrupts should be level-triggered. if ((flags&ESP_INTR_FLAG_SHARED) && (flags&ESP_INTR_FLAG_EDGE)) return ESP_ERR_INVALID_ARG; //You can't set an handler / arg for a non-C-callable interrupt. @@ -481,7 +481,7 @@ esp_err_t esp_intr_alloc_intrstatus(int source, int flags, uint32_t intrstatusre flags|=ESP_INTR_FLAG_LOWMED; } } - ESP_EARLY_LOGV(TAG, "esp_intr_alloc_intrstatus (cpu %d): Args okay. Resulting flags 0x%X", cpu_hal_get_core_id(), flags); + ESP_EARLY_LOGV(TAG, "esp_intr_alloc_intrstatus (cpu %u): Args okay. Resulting flags 0x%X", cpu_hal_get_core_id(), flags); //Check 'special' interrupt sources. These are tied to one specific interrupt, so we //have to force get_free_int to only look at that. @@ -497,7 +497,7 @@ esp_err_t esp_intr_alloc_intrstatus(int source, int flags, uint32_t intrstatusre if (ret==NULL) return ESP_ERR_NO_MEM; portENTER_CRITICAL(&spinlock); - int cpu=cpu_hal_get_core_id(); + uint32_t cpu = cpu_hal_get_core_id(); //See if we can find an interrupt that matches the flags. int intr=get_available_int(flags, cpu, force, source); if (intr==-1) { @@ -796,23 +796,32 @@ esp_err_t IRAM_ATTR esp_intr_disable(intr_handle_t handle) void IRAM_ATTR esp_intr_noniram_disable(void) { + portENTER_CRITICAL_SAFE(&spinlock); uint32_t oldint; - int cpu=cpu_hal_get_core_id(); - uint32_t intmask=~non_iram_int_mask[cpu]; - if (non_iram_int_disabled_flag[cpu]) abort(); - non_iram_int_disabled_flag[cpu]=true; - oldint = interrupt_controller_hal_disable_int_mask(intmask); - //Save which ints we did disable - non_iram_int_disabled[cpu]=oldint&non_iram_int_mask[cpu]; + uint32_t cpu = cpu_hal_get_core_id(); + uint32_t non_iram_ints = non_iram_int_mask[cpu]; + if (non_iram_int_disabled_flag[cpu]) { + abort(); + } + non_iram_int_disabled_flag[cpu] = true; + oldint = interrupt_controller_hal_read_interrupt_mask(); + interrupt_controller_hal_disable_interrupts(non_iram_ints); + // Save disabled ints + non_iram_int_disabled[cpu] = oldint & non_iram_ints; + portEXIT_CRITICAL_SAFE(&spinlock); } void IRAM_ATTR esp_intr_noniram_enable(void) { - int cpu=cpu_hal_get_core_id(); - int intmask=non_iram_int_disabled[cpu]; - if (!non_iram_int_disabled_flag[cpu]) abort(); - non_iram_int_disabled_flag[cpu]=false; - interrupt_controller_hal_enable_int_mask(intmask); + portENTER_CRITICAL_SAFE(&spinlock); + uint32_t cpu = cpu_hal_get_core_id(); + int non_iram_ints = non_iram_int_disabled[cpu]; + if (!non_iram_int_disabled_flag[cpu]) { + abort(); + } + non_iram_int_disabled_flag[cpu] = false; + interrupt_controller_hal_enable_interrupts(non_iram_ints); + portEXIT_CRITICAL_SAFE(&spinlock); } //These functions are provided in ROM, but the ROM-based functions use non-multicore-capable diff --git a/components/esp_wifi/CMakeLists.txt b/components/esp_wifi/CMakeLists.txt index 2b181538b2..20972c5733 100644 --- a/components/esp_wifi/CMakeLists.txt +++ b/components/esp_wifi/CMakeLists.txt @@ -37,7 +37,7 @@ idf_component_register(SRCS "src/coexist.c" INCLUDE_DIRS "include" "${idf_target}/include" REQUIRES esp_event PRIV_REQUIRES driver esptool_py esp_netif esp_pm esp_timer nvs_flash - wpa_supplicant ${extra_priv_requires} + wpa_supplicant hal ${extra_priv_requires} LDFRAGMENTS "${ldfragments}") idf_build_get_property(build_dir BUILD_DIR) diff --git a/components/esp_wifi/esp32/esp_adapter.c b/components/esp_wifi/esp32/esp_adapter.c index c32067273e..0356d5e94a 100644 --- a/components/esp_wifi/esp32/esp_adapter.c +++ b/components/esp_wifi/esp32/esp_adapter.c @@ -40,6 +40,7 @@ #include "esp_phy_init.h" #include "soc/dport_reg.h" #include "soc/syscon_reg.h" +#include "hal/interrupt_controller_hal.h" #include "phy_init_data.h" #include "driver/periph_ctrl.h" #include "nvs.h" @@ -667,8 +668,8 @@ wifi_osi_funcs_t g_wifi_osi_funcs = { ._set_intr = set_intr_wrapper, ._clear_intr = clear_intr_wrapper, ._set_isr = set_isr_wrapper, - ._ints_on = xt_ints_on, - ._ints_off = xt_ints_off, + ._ints_on = interrupt_controller_hal_enable_interrupts, + ._ints_off = interrupt_controller_hal_disable_interrupts, ._is_from_isr = is_from_isr_wrapper, ._spin_lock_create = spin_lock_create_wrapper, ._spin_lock_delete = free, diff --git a/components/esp_wifi/esp32s2/esp_adapter.c b/components/esp_wifi/esp32s2/esp_adapter.c index 1c73dbfd2a..59f1ffda2d 100644 --- a/components/esp_wifi/esp32s2/esp_adapter.c +++ b/components/esp_wifi/esp32s2/esp_adapter.c @@ -42,6 +42,7 @@ #include "soc/dport_reg.h" #include "soc/rtc.h" #include "soc/syscon_reg.h" +#include "hal/interrupt_controller_hal.h" #include "phy_init_data.h" #include "driver/periph_ctrl.h" #include "nvs.h" @@ -648,8 +649,8 @@ wifi_osi_funcs_t g_wifi_osi_funcs = { ._set_intr = set_intr_wrapper, ._clear_intr = clear_intr_wrapper, ._set_isr = set_isr_wrapper, - ._ints_on = xt_ints_on, - ._ints_off = xt_ints_off, + ._ints_on = interrupt_controller_hal_enable_interrupts, + ._ints_off = interrupt_controller_hal_disable_interrupts, ._is_from_isr = is_from_isr_wrapper, ._spin_lock_create = spin_lock_create_wrapper, ._spin_lock_delete = free, diff --git a/components/esp_wifi/esp32s3/esp_adapter.c b/components/esp_wifi/esp32s3/esp_adapter.c index f55d8c7c91..78c978106b 100644 --- a/components/esp_wifi/esp32s3/esp_adapter.c +++ b/components/esp_wifi/esp32s3/esp_adapter.c @@ -42,6 +42,7 @@ #include "soc/rtc_cntl_reg.h" #include "soc/rtc.h" #include "soc/syscon_reg.h" +#include "hal/interrupt_controller_hal.h" #include "phy_init_data.h" #include "driver/periph_ctrl.h" #include "nvs.h" @@ -686,8 +687,8 @@ wifi_osi_funcs_t g_wifi_osi_funcs = { ._set_intr = set_intr_wrapper, ._clear_intr = clear_intr_wrapper, ._set_isr = set_isr_wrapper, - ._ints_on = xt_ints_on, - ._ints_off = xt_ints_off, + ._ints_on = interrupt_controller_hal_enable_interrupts, + ._ints_off = interrupt_controller_hal_disable_interrupts, ._is_from_isr = is_from_isr_wrapper, ._spin_lock_create = spin_lock_create_wrapper, ._spin_lock_delete = free, diff --git a/components/hal/esp32/include/hal/interrupt_controller_ll.h b/components/hal/esp32/include/hal/interrupt_controller_ll.h index 6634b003c4..f31356b617 100644 --- a/components/hal/esp32/include/hal/interrupt_controller_ll.h +++ b/components/hal/esp32/include/hal/interrupt_controller_ll.h @@ -18,6 +18,8 @@ #include "soc/soc_caps.h" #include "soc/soc.h" #include "xtensa/xtensa_api.h" +#include "xt_instr_macros.h" +#include "xtensa/config/specreg.h" #ifdef __cplusplus extern "C" { @@ -43,6 +45,18 @@ static inline void intr_cntrl_ll_disable_interrupts(uint32_t mask) xt_ints_off(mask); } +/** + * @brief Read the current interrupt mask of the CPU running this code. + * + * @return The current interrupt bitmask. + */ +static inline uint32_t intr_cntrl_ll_read_interrupt_mask(void) +{ + uint32_t int_mask; + RSR(INTENABLE, int_mask); + return int_mask; +} + /** * @brief checks if given interrupt number has a valid handler * @@ -79,27 +93,6 @@ static inline void *intr_cntrl_ll_get_int_handler_arg(uint8_t intr) return xt_get_interrupt_handler_arg(intr); } -/** - * @brief Disables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - * @return oldmask where to store old interrupts state - */ -static inline uint32_t intr_cntrl_ll_disable_int_mask(uint32_t newmask) -{ - return xt_int_disable_mask(newmask); -} - -/** - * @brief Enables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - */ -static inline void intr_cntrl_ll_enable_int_mask(uint32_t newmask) -{ - xt_int_enable_mask(newmask); -} - /** * @brief Acknowledge an edge-trigger interrupt by clearing its pending flag * diff --git a/components/hal/esp32c3/include/hal/interrupt_controller_ll.h b/components/hal/esp32c3/include/hal/interrupt_controller_ll.h index 1957e0384b..70afe314cf 100644 --- a/components/hal/esp32c3/include/hal/interrupt_controller_ll.h +++ b/components/hal/esp32c3/include/hal/interrupt_controller_ll.h @@ -49,6 +49,16 @@ static inline void intr_cntrl_ll_disable_interrupts(uint32_t mask) RV_SET_CSR(mstatus, old_mstatus & MSTATUS_MIE); } +/** + * @brief Read the current interrupt mask of the CPU running this code. + * + * @return The current interrupt bitmask. + */ +static inline uint32_t intr_cntrl_ll_read_interrupt_mask(void) +{ + return REG_READ(INTERRUPT_CORE0_CPU_INT_ENABLE_REG); +} + /** * @brief checks if given interrupt number has a valid handler * @@ -85,36 +95,6 @@ static inline void *intr_cntrl_ll_get_int_handler_arg(uint8_t intr) return intr_handler_get_arg(intr); } -/** - * @brief Disables interrupts that are not located in iram - * - * @param newmask mask of interrupts TO KEEP ENABLED (note: this is probably a bug, see IDF-2308) - * @return oldmask previous interrupt mask value - */ -static inline uint32_t intr_cntrl_ll_disable_int_mask(uint32_t newmask) -{ - // Disable interrupts in order to atomically update the interrupt enable register - unsigned old_mstatus = RV_CLEAR_CSR(mstatus, MSTATUS_MIE); - - uint32_t old_int_enable = REG_READ(INTERRUPT_CORE0_CPU_INT_ENABLE_REG); - REG_WRITE(INTERRUPT_CORE0_CPU_INT_ENABLE_REG, old_int_enable & newmask); - - RV_SET_CSR(mstatus, old_mstatus & MSTATUS_MIE); - return old_int_enable; -} - -/** - * @brief Enables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be enabled - */ -static inline void intr_cntrl_ll_enable_int_mask(uint32_t newmask) -{ - unsigned old_mstatus = RV_CLEAR_CSR(mstatus, MSTATUS_MIE); - esprv_intc_int_enable(newmask); - RV_SET_CSR(mstatus, old_mstatus & MSTATUS_MIE); -} - /** * @brief Acknowledge an edge-trigger interrupt by clearing its pending flag * diff --git a/components/hal/esp32s2/include/hal/interrupt_controller_ll.h b/components/hal/esp32s2/include/hal/interrupt_controller_ll.h index 5c270ec66e..3a33d92a48 100644 --- a/components/hal/esp32s2/include/hal/interrupt_controller_ll.h +++ b/components/hal/esp32s2/include/hal/interrupt_controller_ll.h @@ -18,6 +18,8 @@ #include "soc/soc_caps.h" #include "soc/soc.h" #include "xtensa/xtensa_api.h" +#include "xtensa/config/specreg.h" +#include "xt_instr_macros.h" #ifdef __cplusplus extern "C" { @@ -43,6 +45,18 @@ static inline void intr_cntrl_ll_disable_interrupts(uint32_t mask) xt_ints_off(mask); } +/** + * @brief Read the current interrupt mask of the CPU running this code. + * + * @return The current interrupt bitmask. + */ +static inline uint32_t intr_cntrl_ll_read_interrupt_mask(void) +{ + uint32_t int_mask; + RSR(INTENABLE, int_mask); + return int_mask; +} + /** * @brief checks if given interrupt number has a valid handler * @@ -79,27 +93,6 @@ static inline void *intr_cntrl_ll_get_int_handler_arg(uint8_t intr) return xt_get_interrupt_handler_arg(intr); } -/** - * @brief Disables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - * @return oldmask where to store old interrupts state - */ -static inline uint32_t intr_cntrl_ll_disable_int_mask(uint32_t newmask) -{ - return xt_int_disable_mask(newmask); -} - -/** - * @brief Enables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - */ -static inline void intr_cntrl_ll_enable_int_mask(uint32_t newmask) -{ - xt_int_enable_mask(newmask); -} - /** * @brief Acknowledge an edge-trigger interrupt by clearing its pending flag * diff --git a/components/hal/esp32s3/include/hal/interrupt_controller_ll.h b/components/hal/esp32s3/include/hal/interrupt_controller_ll.h index 5c270ec66e..3a33d92a48 100644 --- a/components/hal/esp32s3/include/hal/interrupt_controller_ll.h +++ b/components/hal/esp32s3/include/hal/interrupt_controller_ll.h @@ -18,6 +18,8 @@ #include "soc/soc_caps.h" #include "soc/soc.h" #include "xtensa/xtensa_api.h" +#include "xtensa/config/specreg.h" +#include "xt_instr_macros.h" #ifdef __cplusplus extern "C" { @@ -43,6 +45,18 @@ static inline void intr_cntrl_ll_disable_interrupts(uint32_t mask) xt_ints_off(mask); } +/** + * @brief Read the current interrupt mask of the CPU running this code. + * + * @return The current interrupt bitmask. + */ +static inline uint32_t intr_cntrl_ll_read_interrupt_mask(void) +{ + uint32_t int_mask; + RSR(INTENABLE, int_mask); + return int_mask; +} + /** * @brief checks if given interrupt number has a valid handler * @@ -79,27 +93,6 @@ static inline void *intr_cntrl_ll_get_int_handler_arg(uint8_t intr) return xt_get_interrupt_handler_arg(intr); } -/** - * @brief Disables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - * @return oldmask where to store old interrupts state - */ -static inline uint32_t intr_cntrl_ll_disable_int_mask(uint32_t newmask) -{ - return xt_int_disable_mask(newmask); -} - -/** - * @brief Enables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - */ -static inline void intr_cntrl_ll_enable_int_mask(uint32_t newmask) -{ - xt_int_enable_mask(newmask); -} - /** * @brief Acknowledge an edge-trigger interrupt by clearing its pending flag * diff --git a/components/hal/include/hal/interrupt_controller_hal.h b/components/hal/include/hal/interrupt_controller_hal.h index a7c47e6a5c..39a5a8e684 100644 --- a/components/hal/include/hal/interrupt_controller_hal.h +++ b/components/hal/include/hal/interrupt_controller_hal.h @@ -135,6 +135,16 @@ static inline void interrupt_controller_hal_disable_interrupts(uint32_t mask) intr_cntrl_ll_disable_interrupts(mask); } +/** + * @brief Read the current interrupt mask. + * + * @return The bitmask of current interrupts + */ +static inline uint32_t interrupt_controller_hal_read_interrupt_mask(void) +{ + return intr_cntrl_ll_read_interrupt_mask(); +} + /** * @brief checks if given interrupt number has a valid handler * @@ -171,27 +181,6 @@ static inline void * interrupt_controller_hal_get_int_handler_arg(uint8_t intr) return intr_cntrl_ll_get_int_handler_arg(intr); } -/** - * @brief Disables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - * @return oldmask where to store old interrupts state - */ -static inline uint32_t interrupt_controller_hal_disable_int_mask(uint32_t newmask) -{ - return intr_cntrl_ll_disable_int_mask(newmask); -} - -/** - * @brief Enables interrupts that are not located in iram - * - * @param newmask mask of interrupts needs to be disabled - */ -static inline void interrupt_controller_hal_enable_int_mask(uint32_t newmask) -{ - intr_cntrl_ll_enable_int_mask(newmask); -} - /** * @brief Acknowledge an edge-trigger interrupt by clearing its pending flag * diff --git a/components/xtensa/include/xtensa/xtensa_api.h b/components/xtensa/include/xtensa/xtensa_api.h index 95501936c1..deef36f7c6 100644 --- a/components/xtensa/include/xtensa/xtensa_api.h +++ b/components/xtensa/include/xtensa/xtensa_api.h @@ -136,45 +136,4 @@ extern void * xt_get_interrupt_handler_arg(int n); */ bool xt_int_has_handler(int intr, int cpu); -/* -------------------------------------------------------------------------------- - Call this function to disable non iram located interrupts. - - newmask - mask containing the interrupts to disable. -------------------------------------------------------------------------------- -*/ -static inline uint32_t xt_int_disable_mask(uint32_t newmask) -{ - uint32_t oldint; - asm volatile ( - "movi %0,0\n" - "xsr %0,INTENABLE\n" //disable all ints first - "rsync\n" - "and a3,%0,%1\n" //mask ints that need disabling - "wsr a3,INTENABLE\n" //write back - "rsync\n" - :"=&r"(oldint):"r"(newmask):"a3"); - - return oldint; -} - -/* -------------------------------------------------------------------------------- - Call this function to enable non iram located interrupts. - - newmask - mask containing the interrupts to enable. -------------------------------------------------------------------------------- -*/ -static inline void xt_int_enable_mask(uint32_t newmask) -{ - asm volatile ( - "movi a3,0\n" - "xsr a3,INTENABLE\n" - "rsync\n" - "or a3,a3,%0\n" - "wsr a3,INTENABLE\n" - "rsync\n" - ::"r"(newmask):"a3"); -} - #endif /* __XTENSA_API_H__ */