[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

@@ -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 cpu = cpu_hal_get_core_id();
-    uint32_t intmask=~non_iram_int_mask[cpu];
+    uint32_t non_iram_ints = non_iram_int_mask[cpu];
-    if (non_iram_int_disabled_flag[cpu]) abort();
+    if (non_iram_int_disabled_flag[cpu]) {
+        abort();
+    }
     non_iram_int_disabled_flag[cpu] = true;
-    oldint = interrupt_controller_hal_disable_int_mask(intmask);
+    oldint = interrupt_controller_hal_read_interrupt_mask();
-    //Save which ints we did disable
+    interrupt_controller_hal_disable_interrupts(non_iram_ints);
-    non_iram_int_disabled[cpu]=oldint&non_iram_int_mask[cpu];
+    // 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();
+    portENTER_CRITICAL_SAFE(&spinlock);
-    int intmask=non_iram_int_disabled[cpu];
+    uint32_t cpu = cpu_hal_get_core_id();
-    if (!non_iram_int_disabled_flag[cpu]) abort();
+    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_int_mask(intmask);
+    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

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)

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_on = interrupt_controller_hal_enable_interrupts,
-    ._ints_off = xt_ints_off,
+    ._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,

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_on = interrupt_controller_hal_enable_interrupts,
-    ._ints_off = xt_ints_off,
+    ._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,

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_on = interrupt_controller_hal_enable_interrupts,
-    ._ints_off = xt_ints_off,
+    ._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,

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
  *

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
  *

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
  *

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
  *

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
  *

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__ */