Merge branch 'refactor/freertos_deprecate_critical_nested_macros' into 'master'

freertos: Deprecate critical nested macros

Closes IDF-4036

See merge request espressif/esp-idf!15539

components/app_trace/app_trace_util.c

@@ -61,7 +61,7 @@ esp_err_t esp_apptrace_lock_take(esp_apptrace_lock_t *lock, esp_apptrace_tmo_t *
 
     while (1) {
         // do not overwrite lock->int_state before we actually acquired the mux
-        unsigned int_state = portENTER_CRITICAL_NESTED();
+        unsigned int_state = portSET_INTERRUPT_MASK_FROM_ISR();
         // FIXME: if mux is busy it is not good idea to loop during the whole tmo with disabled IRQs.
         // So we check mux state using zero tmo, restore IRQs and let others tasks/IRQs to run on this CPU
         // while we are doing our own tmo check.
@@ -74,8 +74,8 @@ esp_err_t esp_apptrace_lock_take(esp_apptrace_lock_t *lock, esp_apptrace_tmo_t *
             lock->int_state = int_state;
             return ESP_OK;
         }
-        portEXIT_CRITICAL_NESTED(int_state);
-        // we can be preempted from this place till the next call (above) to portENTER_CRITICAL_NESTED()
+        portCLEAR_INTERRUPT_MASK_FROM_ISR(int_state);
+        // we can be preempted from this place till the next call (above) to portSET_INTERRUPT_MASK_FROM_ISR()
         res = esp_apptrace_tmo_check(tmo);
         if (res != ESP_OK) {
             break;
@@ -95,7 +95,7 @@ esp_err_t esp_apptrace_lock_give(esp_apptrace_lock_t *lock)
 #else
     vPortCPUReleaseMutex(&lock->mux);
 #endif
-    portEXIT_CRITICAL_NESTED(int_state);
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(int_state);
     return ESP_OK;
 }
 

components/esp_ipc/src/esp_ipc_isr/esp_ipc_isr.c

@@ -122,7 +122,7 @@ void esp_ipc_isr_waiting_for_finish_cmd(void* finish_cmd);
 void IRAM_ATTR esp_ipc_isr_stall_other_cpu(void)
 {
     if (s_stall_state == STALL_STATE_RUNNING) {
-        BaseType_t intLvl = portENTER_CRITICAL_NESTED();
+        BaseType_t intLvl = portSET_INTERRUPT_MASK_FROM_ISR();
         const uint32_t cpu_id = xPortGetCoreID();
         if (s_count_of_nested_calls[cpu_id]++ == 0) {
             IPC_ISR_ENTER_CRITICAL();
@@ -133,7 +133,7 @@ void IRAM_ATTR esp_ipc_isr_stall_other_cpu(void)
         }
 
         /* Interrupts are already disabled by the parent, we're nested here. */
-        portEXIT_CRITICAL_NESTED(intLvl);
+        portCLEAR_INTERRUPT_MASK_FROM_ISR(intLvl);
     }
 }
 
@@ -144,7 +144,7 @@ void IRAM_ATTR esp_ipc_isr_release_other_cpu(void)
         if (--s_count_of_nested_calls[cpu_id] == 0) {
             esp_ipc_isr_finish_cmd = 1;
             IPC_ISR_EXIT_CRITICAL();
-            portEXIT_CRITICAL_NESTED(s_stored_interrupt_level);
+            portCLEAR_INTERRUPT_MASK_FROM_ISR(s_stored_interrupt_level);
         } else if (s_count_of_nested_calls[cpu_id] < 0) {
             assert(0);
         }

components/esp_pm/pm_impl.c

@@ -1,16 +1,8 @@
-// Copyright 2016-2017 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.
+/*
+ * SPDX-FileCopyrightText: 2016-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #include <stdlib.h>
 #include <stdbool.h>
@@ -786,7 +778,7 @@ void esp_pm_impl_init(void)
 void esp_pm_impl_idle_hook(void)
 {
     int core_id = xPortGetCoreID();
-    uint32_t state = portENTER_CRITICAL_NESTED();
+    uint32_t state = portSET_INTERRUPT_MASK_FROM_ISR();
     if (!s_core_idle[core_id]
 #ifdef CONFIG_FREERTOS_USE_TICKLESS_IDLE
     && !periph_should_skip_light_sleep()
@@ -795,7 +787,7 @@ void esp_pm_impl_idle_hook(void)
         esp_pm_lock_release(s_rtos_lock_handle[core_id]);
         s_core_idle[core_id] = true;
     }
-    portEXIT_CRITICAL_NESTED(state);
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(state);
     ESP_PM_TRACE_ENTER(IDLE, core_id);
 }
 
@@ -806,7 +798,7 @@ void IRAM_ATTR esp_pm_impl_isr_hook(void)
     /* Prevent higher level interrupts (than the one this function was called from)
      * from happening in this section, since they will also call into esp_pm_impl_isr_hook.
      */
-    uint32_t state = portENTER_CRITICAL_NESTED();
+    uint32_t state = portSET_INTERRUPT_MASK_FROM_ISR();
 #if defined(CONFIG_FREERTOS_SYSTICK_USES_CCOUNT) && (portNUM_PROCESSORS == 2)
     if (s_need_update_ccompare[core_id]) {
         update_ccompare();
@@ -817,7 +809,7 @@ void IRAM_ATTR esp_pm_impl_isr_hook(void)
 #else
     leave_idle();
 #endif // CONFIG_FREERTOS_SYSTICK_USES_CCOUNT && portNUM_PROCESSORS == 2
-    portEXIT_CRITICAL_NESTED(state);
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(state);
     ESP_PM_TRACE_EXIT(ISR_HOOK, core_id);
 }
 

components/esp_system/test/test_reset_reason.c

@@ -181,7 +181,8 @@ TEST_CASE_MULTIPLE_STAGES("reset reason ESP_RST_SW after restart from APP CPU",
 static void do_int_wdt(void)
 {
     setup_values();
-    portENTER_CRITICAL_NESTED();
+    BaseType_t prev_level = portSET_INTERRUPT_MASK_FROM_ISR();
+    (void) prev_level;
     while(1);
 }
 

components/freertos/port/riscv/include/freertos/portmacro.h

@@ -139,18 +139,6 @@ BaseType_t xPortInIsrContext(void);
  */
 BaseType_t xPortInterruptedFromISRContext(void);
 
-/**
- * @brief Disable interrupts in a nested manner
- *
- * - Cleaner solution allows nested interrupts disabling and restoring via local registers or stack.
- * - They can be called from interrupts too.
- * - WARNING Only applies to current CPU.
- *
- * @note [refactor-todo] Define this as portSET_INTERRUPT_MASK_FROM_ISR() instead
- * @return unsigned Previous interrupt state
- */
-static inline unsigned portENTER_CRITICAL_NESTED(void);
-
 /* ---------------------- Spinlocks ------------------------
  - Spinlocks added to match API with SMP FreeRTOS. Single core RISC-V does not need spin locks
  - Because single core does not have a primitive spinlock data type, we have to implement one here
@@ -403,11 +391,10 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void)
 
 // --------------------- Interrupts ------------------------
 
-#define portEXIT_CRITICAL_NESTED(state)     do { portCLEAR_INTERRUPT_MASK_FROM_ISR(state);} while(0);
 #define portDISABLE_INTERRUPTS()            portSET_INTERRUPT_MASK_FROM_ISR()
 #define portENABLE_INTERRUPTS()             portCLEAR_INTERRUPT_MASK_FROM_ISR(1)
 #define portSET_INTERRUPT_MASK_FROM_ISR()                       vPortSetInterruptMask()
-#define portCLEAR_INTERRUPT_MASK_FROM_ISR(uxSavedStatusValue) vPortClearInterruptMask(uxSavedStatusValue)
+#define portCLEAR_INTERRUPT_MASK_FROM_ISR(uxSavedStatusValue)   vPortClearInterruptMask(uxSavedStatusValue)
 
 // ------------------ Critical Sections --------------------
 
@@ -472,11 +459,7 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void)
 
 // --------------------- Interrupts ------------------------
 
-static inline unsigned portENTER_CRITICAL_NESTED(void)
-{
-    unsigned state = portSET_INTERRUPT_MASK_FROM_ISR();
-    return state;
-}
+
 
 // ---------------------- Spinlocks ------------------------
 
@@ -538,6 +521,14 @@ bool xPortcheckValidStackMem(const void *ptr);
 #define portVALID_TCB_MEM(ptr) xPortCheckValidTCBMem(ptr)
 #define portVALID_STACK_MEM(ptr) xPortcheckValidStackMem(ptr)
 
+
+
+/* ---------------------------------------------------- Deprecate ------------------------------------------------------
+ * - Pull in header containing deprecated macros here
+ * ------------------------------------------------------------------------------------------------------------------ */
+
+#include "portmacro_deprecated.h"
+
 #ifdef __cplusplus
 }
 #endif

components/freertos/port/riscv/include/freertos/portmacro_deprecated.h

@@ -0,0 +1,34 @@
+/*
+ * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/* ---------------------------------------------------- Deprecate ------------------------------------------------------
+ * - Macros or functions that should be deprecated in v5.0, then removed in the next major release
+ * - Kept as not to cause a breaking change
+ * - Include this header at the end of portmacro.h
+ * ------------------------------------------------------------------------------------------------------------------ */
+
+/**
+ * @brief Disable interrupts in a nested manner
+ *
+ * Does the exact same thing as portSET_INTERRUPT_MASK_FROM_ISR()
+ *
+ * @deprecated This function is deprecated. Call portSET_INTERRUPT_MASK_FROM_ISR() instead
+ */
+static inline __attribute__((deprecated)) UBaseType_t portENTER_CRITICAL_NESTED(void) {
+    return portSET_INTERRUPT_MASK_FROM_ISR();
+}
+
+/**
+ * @brief Reenables interrupts in a nested manner
+ *
+ * Does the exact same thing as portCLEAR_INTERRUPT_MASK_FROM_ISR()
+ *
+ * @deprecated This function is deprecated. Call portCLEAR_INTERRUPT_MASK_FROM_ISR() instead
+ */
+static inline void __attribute__((deprecated)) portEXIT_CRITICAL_NESTED(UBaseType_t prev_level)
+{
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(prev_level);
+}

components/freertos/port/riscv/port.c

@@ -286,7 +286,7 @@ void vPortCPUReleaseMutex(portMUX_TYPE *mux)
 
 void vPortEnterCritical(void)
 {
-    BaseType_t state = portENTER_CRITICAL_NESTED();
+    BaseType_t state = portSET_INTERRUPT_MASK_FROM_ISR();
     uxCriticalNesting++;
 
     if (uxCriticalNesting == 1) {
@@ -299,7 +299,7 @@ void vPortExitCritical(void)
     if (uxCriticalNesting > 0) {
         uxCriticalNesting--;
         if (uxCriticalNesting == 0) {
-            portEXIT_CRITICAL_NESTED(uxSavedInterruptState);
+            portCLEAR_INTERRUPT_MASK_FROM_ISR(uxSavedInterruptState);
         }
     }
 }

components/freertos/port/xtensa/include/freertos/portmacro.h

@@ -155,15 +155,20 @@ void vPortAssertIfInISR(void);
 BaseType_t xPortInterruptedFromISRContext(void);
 
 /**
- * @brief Disable interrupts in a nested manner
+ * @brief Disable interrupts in a nested manner (meant to be called from ISRs)
  *
- * - Cleaner solution allows nested interrupts disabling and restoring via local registers or stack.
- * - They can be called from interrupts too.
- * - WARNING Only applies to current CPU.
- * @note [refactor-todo] Define this as portSET_INTERRUPT_MASK_FROM_ISR() instead
- * @return unsigned Previous interrupt state
+ * @warning Only applies to current CPU.
+ * @return UBaseType_t Previous interrupt level
  */
-static inline unsigned __attribute__((always_inline)) portENTER_CRITICAL_NESTED(void);
+static inline UBaseType_t xPortSetInterruptMaskFromISR(void);
+
+/**
+ * @brief Reenable interrupts in a nested manner (meant to be called from ISRs)
+ *
+ * @warning Only applies to current CPU.
+ * @param prev_level Previous interrupt level
+ */
+static inline void vPortClearInterruptMaskFromISR(UBaseType_t prev_level);
 
 /* ---------------------- Spinlocks ------------------------
  * - Modifications made to critical sections to support SMP
@@ -416,8 +421,6 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void);
 
 // --------------------- Interrupts ------------------------
 
-#define portEXIT_CRITICAL_NESTED(state)     do { portbenchmarkINTERRUPT_RESTORE(state); XTOS_RESTORE_JUST_INTLEVEL(state); } while (0)
-
 /**
  * - Only applies to current core
  * - These cannot be nested. They should be used with a lot of care and cannot be called from interrupt level.
@@ -430,8 +433,8 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void);
 /**
  * ISR versions to enable/disable interrupts
  */
-#define portSET_INTERRUPT_MASK_FROM_ISR()            portENTER_CRITICAL_NESTED()
-#define portCLEAR_INTERRUPT_MASK_FROM_ISR(state)     portEXIT_CRITICAL_NESTED(state)
+#define portSET_INTERRUPT_MASK_FROM_ISR()                   xPortSetInterruptMaskFromISR()
+#define portCLEAR_INTERRUPT_MASK_FROM_ISR(prev_level)       vPortClearInterruptMaskFromISR(prev_level)
 
 #define portASSERT_IF_IN_ISR() vPortAssertIfInISR()
 
@@ -530,11 +533,17 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void);
 
 // --------------------- Interrupts ------------------------
 
-static inline unsigned portENTER_CRITICAL_NESTED(void)
+static inline UBaseType_t xPortSetInterruptMaskFromISR(void)
 {
-    unsigned state = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL);
+    UBaseType_t prev_int_level = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL);
     portbenchmarkINTERRUPT_DISABLE();
-    return state;
+    return prev_int_level;
+}
+
+static inline void vPortClearInterruptMaskFromISR(UBaseType_t prev_level)
+{
+    portbenchmarkINTERRUPT_RESTORE(prev_level);
+    XTOS_RESTORE_JUST_INTLEVEL(prev_level);
 }
 
 // ---------------------- Spinlocks ------------------------
@@ -737,6 +746,14 @@ bool xPortcheckValidStackMem(const void *ptr);
 #define portVALID_TCB_MEM(ptr) xPortCheckValidTCBMem(ptr)
 #define portVALID_STACK_MEM(ptr) xPortcheckValidStackMem(ptr)
 
+
+
+/* ---------------------------------------------------- Deprecate ------------------------------------------------------
+ * - Pull in header containing deprecated macros here
+ * ------------------------------------------------------------------------------------------------------------------ */
+
+#include "portmacro_deprecated.h"
+
 #ifdef __cplusplus
 }
 #endif

components/freertos/port/xtensa/include/freertos/portmacro_deprecated.h

@@ -0,0 +1,34 @@
+/*
+ * SPDX-FileCopyrightText: 2017-2021 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/* ---------------------------------------------------- Deprecate ------------------------------------------------------
+ * - Macros or functions that should be deprecated in v5.0, then removed in the next major release
+ * - Kept as not to cause a breaking change
+ * - Include this header at the end of portmacro.h
+ * ------------------------------------------------------------------------------------------------------------------ */
+
+/**
+ * @brief Disable interrupts in a nested manner
+ *
+ * Does the exact same thing as portSET_INTERRUPT_MASK_FROM_ISR()
+ *
+ * @deprecated This function is deprecated. Call portSET_INTERRUPT_MASK_FROM_ISR() instead
+ */
+static inline __attribute__((deprecated)) UBaseType_t portENTER_CRITICAL_NESTED(void) {
+    return portSET_INTERRUPT_MASK_FROM_ISR();
+}
+
+/**
+ * @brief Reenables interrupts in a nested manner
+ *
+ * Does the exact same thing as portCLEAR_INTERRUPT_MASK_FROM_ISR()
+ *
+ * @deprecated This function is deprecated. Call portCLEAR_INTERRUPT_MASK_FROM_ISR() instead
+ */
+static inline void __attribute__((deprecated)) portEXIT_CRITICAL_NESTED(UBaseType_t prev_level)
+{
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(prev_level);
+}

components/freertos/port/xtensa/port.c

@@ -255,9 +255,9 @@ BaseType_t xPortInIsrContext(void)
 {
     unsigned int irqStatus;
     BaseType_t ret;
-    irqStatus = portENTER_CRITICAL_NESTED();
+    irqStatus = portSET_INTERRUPT_MASK_FROM_ISR();
     ret = (port_interruptNesting[xPortGetCoreID()] != 0);
-    portEXIT_CRITICAL_NESTED(irqStatus);
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(irqStatus);
     return ret;
 }
 
@@ -275,7 +275,7 @@ BaseType_t IRAM_ATTR xPortInterruptedFromISRContext(void)
 
 void __attribute__((optimize("-O3"))) vPortEnterCritical(portMUX_TYPE *mux)
 {
-    BaseType_t oldInterruptLevel = portENTER_CRITICAL_NESTED();
+    BaseType_t oldInterruptLevel = portSET_INTERRUPT_MASK_FROM_ISR();
     /* Interrupts may already be disabled (because we're doing this recursively)
     * but we can't get the interrupt level after
     * vPortCPUAquireMutex, because it also may mess with interrupts.
@@ -304,7 +304,7 @@ void __attribute__((optimize("-O3"))) vPortExitCritical(portMUX_TYPE *mux)
         port_uxCriticalNesting[coreID] = nesting;
 
         if ( nesting == 0 ) {
-            portEXIT_CRITICAL_NESTED(port_uxOldInterruptState[coreID]);
+            portCLEAR_INTERRUPT_MASK_FROM_ISR(port_uxOldInterruptState[coreID]);
         }
     }
 }

components/freertos/tasks.c

@@ -2429,10 +2429,9 @@ void vTaskSuspendAll( void )
      * post in the FreeRTOS support forum before reporting this as a bug! -
      * https://goo.gl/wu4acr */
     unsigned state;
-
-    state = portENTER_CRITICAL_NESTED();
+    state = portSET_INTERRUPT_MASK_FROM_ISR();
     ++uxSchedulerSuspended[ xPortGetCoreID() ];
-    portEXIT_CRITICAL_NESTED(state);
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(state);
 }
 /*----------------------------------------------------------*/
 
@@ -3363,7 +3362,7 @@ void vTaskSwitchContext( void )
 {
     //Theoretically, this is only called from either the tick interrupt or the crosscore interrupt, so disabling
     //interrupts shouldn't be necessary anymore. Still, for safety we'll leave it in for now.
-    int irqstate=portENTER_CRITICAL_NESTED();
+    int irqstate = portSET_INTERRUPT_MASK_FROM_ISR();
 
     if( uxSchedulerSuspended[ xPortGetCoreID() ] != ( UBaseType_t ) pdFALSE )
     {
@@ -3528,7 +3527,7 @@ void vTaskSwitchContext( void )
         #endif
 
     }
-    portEXIT_CRITICAL_NESTED(irqstate);
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(irqstate);
 }
 /*-----------------------------------------------------------*/
 
@@ -4619,9 +4618,9 @@ static void prvResetNextTaskUnblockTime( void )
         TaskHandle_t xReturn;
         unsigned state;
 
-        state = portENTER_CRITICAL_NESTED();
+        state = portSET_INTERRUPT_MASK_FROM_ISR();
         xReturn = pxCurrentTCB[ xPortGetCoreID() ];
-        portEXIT_CRITICAL_NESTED(state);
+        portCLEAR_INTERRUPT_MASK_FROM_ISR(state);
 
         return xReturn;
     }

components/freertos/test/test_tasks_snapshot.c

@@ -22,7 +22,7 @@ TEST_CASE("Tasks snapshot", "[freertos]")
 
     // uxTaskGetSnapshotAll is supposed to be called when all tasks on both CPUs are
     // inactive and can not alter FreeRTOS internal tasks lists, e.g. from panic handler
-    unsigned state = portENTER_CRITICAL_NESTED();
+    unsigned state = portSET_INTERRUPT_MASK_FROM_ISR();
 #ifndef CONFIG_FREERTOS_UNICORE
     esp_cpu_stall(other_core_id);
 #endif
@@ -30,7 +30,7 @@ TEST_CASE("Tasks snapshot", "[freertos]")
 #ifndef CONFIG_FREERTOS_UNICORE
     esp_cpu_unstall(other_core_id);
 #endif
-    portEXIT_CRITICAL_NESTED(state);
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(state);
 
     printf("Dumped %d tasks. TCB size %d\n", task_num, tcb_sz);
     TEST_ASSERT_NOT_EQUAL(0, task_num);

components/newlib/stdatomic.c

@@ -36,15 +36,15 @@
 
 #if SOC_CPU_CORES_NUM == 1
 
-// Single core SoC: atomics can be implemented using portENTER_CRITICAL_NESTED
-// and portEXIT_CRITICAL_NESTED, which disable and enable interrupts.
+// Single core SoC: atomics can be implemented using portSET_INTERRUPT_MASK_FROM_ISR
+// and portCLEAR_INTERRUPT_MASK_FROM_ISR, which disables and enables interrupts.
 #define _ATOMIC_ENTER_CRITICAL() ({ \
-    unsigned state = portENTER_CRITICAL_NESTED(); \
+    unsigned state = portSET_INTERRUPT_MASK_FROM_ISR(); \
     state; \
 })
 
 #define _ATOMIC_EXIT_CRITICAL(state)   do { \
-    portEXIT_CRITICAL_NESTED(state); \
+    portCLEAR_INTERRUPT_MASK_FROM_ISR(state); \
     } while (0)
 
 #else // SOC_CPU_CORES_NUM

tools/ci/check_copyright_ignore.txt

@@ -881,7 +881,6 @@ components/esp_pm/include/esp32s3/pm.h
 components/esp_pm/include/esp_pm.h
 components/esp_pm/include/esp_private/pm_impl.h
 components/esp_pm/include/esp_private/pm_trace.h
-components/esp_pm/pm_impl.c
 components/esp_pm/pm_locks.c
 components/esp_pm/pm_trace.c
 components/esp_pm/test/test_pm.c