From: Angus Gratton Date: Fri, 20 Oct 2017 03:15:19 +0000 (+0800) Subject: freertos: Update comments in "port" section (portMUX/etc) X-Git-Tag: v3.1-dev~94^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b10e1a92b6c4ad35995622a919f85c268ebf3f4a;p=esp-idf freertos: Update comments in "port" section (portMUX/etc) Some comments had fallen out of date. --- diff --git a/components/freertos/include/freertos/portmacro.h b/components/freertos/include/freertos/portmacro.h index 370551ceaf..11d783e3ac 100644 --- a/components/freertos/include/freertos/portmacro.h +++ b/components/freertos/include/freertos/portmacro.h @@ -126,8 +126,20 @@ typedef unsigned portBASE_TYPE UBaseType_t; #include "sdkconfig.h" #include "esp_attr.h" +/* "mux" data structure (spinlock) */ typedef struct { + /* owner field values: + * 0 - Uninitialized (invalid) + * portMUX_FREE_VAL - Mux is free, can be locked by either CPU + * CORE_ID_PRO / CORE_ID_APP - Mux is locked to the particular core + * + * Any value other than portMUX_FREE_VAL, CORE_ID_PRO, CORE_ID_APP indicates corruption + */ uint32_t owner; + /* count field: + * If mux is unlocked, count should be zero. + * If mux is locked, count is non-zero & represents the number of recursive locks on the mux. + */ uint32_t count; #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG const char *lastLockedFn; @@ -135,22 +147,13 @@ typedef struct { #endif } portMUX_TYPE; - /* - * Kernel mux values can be: - * 0 - Uninitialized - * (0-portNUM_PROCESSORS)|(recCnt<<8)|0xB33F0000 - taken by core (val), recurse count is (recCnt) - * 0xB33FFFFF - free - * - * The magic number in the top 16 bits is there so we can detect uninitialized and corrupted muxes. - */ - #define portMUX_FREE_VAL 0xB33FFFFF /* Special constants for vPortCPUAcquireMutexTimeout() */ #define portMUX_NO_TIMEOUT (-1) /* When passed for 'timeout_cycles', spin forever if necessary */ #define portMUX_TRY_LOCK 0 /* Try to acquire the spinlock a single time only */ -//Keep this in sync with the portMUX_TYPE struct definition please. +// Keep this in sync with the portMUX_TYPE struct definition please. #ifndef CONFIG_FREERTOS_PORTMUX_DEBUG #define portMUX_INITIALIZER_UNLOCKED { \ .owner = portMUX_FREE_VAL, \ @@ -165,10 +168,6 @@ typedef struct { } #endif -/* Critical section management. NW-TODO: replace XTOS_SET_INTLEVEL with more efficient version, if any? */ -// These cannot be nested. They should be used with a lot of care and cannot be called from interrupt level. -#define portDISABLE_INTERRUPTS() do { XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); portbenchmarkINTERRUPT_DISABLE(); } while (0) -#define portENABLE_INTERRUPTS() do { portbenchmarkINTERRUPT_RESTORE(0); XTOS_SET_INTLEVEL(0); } while (0) #define portASSERT_IF_IN_ISR() vPortAssertIfInISR() void vPortAssertIfInISR(); @@ -176,9 +175,11 @@ void vPortAssertIfInISR(); #define portCRITICAL_NESTING_IN_TCB 1 /* -Modifications to portENTER_CRITICAL: +Modifications to portENTER_CRITICAL. + +For an introduction, see "Critical Sections & Disabling Interrupts" in docs/api-guides/freertos-smp.rst -The original portENTER_CRITICAL only disabled the ISRs. This is enough for single-CPU operation: by +The original portENTER_CRITICAL only disabled the ISRs. This is enough for single-CPU operation: by disabling the interrupts, there is no task switch so no other tasks can meddle in the data, and because interrupts are disabled, ISRs can't corrupt data structures either. @@ -192,7 +193,7 @@ CPU from meddling with the data, it does not stop interrupts on the other cores data. For this, we also use a spinlock in the routines called by the ISR, but these spinlocks do not disable the interrupts (because they already are). -This all assumes that interrupts are either entirely disabled or enabled. Interrupr priority levels +This all assumes that interrupts are either entirely disabled or enabled. Interrupt priority levels will break this scheme. Remark: For the ESP32, portENTER_CRITICAL and portENTER_CRITICAL_ISR both alias vTaskEnterCritical, meaning @@ -234,11 +235,21 @@ void vPortCPUReleaseMutex(portMUX_TYPE *mux); #define portEXIT_CRITICAL_ISR(mux) vTaskExitCritical(mux) #endif -// Cleaner and preferred solution allows nested interrupts disabling and restoring via local registers or stack. +// Critical section management. NW-TODO: replace XTOS_SET_INTLEVEL with more efficient version, if any? +// These cannot be nested. They should be used with a lot of care and cannot be called from interrupt level. +// +// Only applies to one CPU. See notes above & below for reasons not to use these. +#define portDISABLE_INTERRUPTS() do { XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); portbenchmarkINTERRUPT_DISABLE(); } while (0) +#define portENABLE_INTERRUPTS() do { portbenchmarkINTERRUPT_RESTORE(0); XTOS_SET_INTLEVEL(0); } while (0) + +// Cleaner solution allows nested interrupts disabling and restoring via local registers or stack. // They can be called from interrupts too. -// WARNING: This ONLY disables interrupt on the current CPU, meaning they cannot be used as a replacement for the vTaskExitCritical spinlock -// on a multicore system. Only use if disabling interrupts on the current CPU only is indeed what you want. -static inline unsigned portENTER_CRITICAL_NESTED() { unsigned state = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); portbenchmarkINTERRUPT_DISABLE(); return state; } +// WARNING: Only applies to current CPU. See notes above. +static inline unsigned portENTER_CRITICAL_NESTED() { + unsigned state = XTOS_SET_INTLEVEL(XCHAL_EXCM_LEVEL); + portbenchmarkINTERRUPT_DISABLE(); + return state; +} #define portEXIT_CRITICAL_NESTED(state) do { portbenchmarkINTERRUPT_RESTORE(state); XTOS_RESTORE_JUST_INTLEVEL(state); } while (0) // These FreeRTOS versions are similar to the nested versions above @@ -260,12 +271,12 @@ static inline unsigned portENTER_CRITICAL_NESTED() { unsigned state = XTOS_SET_I /* * Wrapper for the Xtensa compare-and-set instruction. This subroutine will atomically compare - * *mux to compare, and if it's the same, will set *mux to set. It will return the old value - * of *addr in *set. + * *addr to 'compare'. If *addr == compare, *addr is set to *set. *set is updated with the previous + * value of *addr (either 'compare' or some other value.) * * Warning: From the ISA docs: in some (unspecified) cases, the s32c1i instruction may return the * *bitwise inverse* of the old mem if the mem wasn't written. This doesn't seem to happen on the - * ESP32, though. (Would show up directly if it did because the magic wouldn't match.) + * ESP32 (portMUX assertions would fail). */ static inline void uxPortCompareSet(volatile uint32_t *addr, uint32_t compare, uint32_t *set) { __asm__ __volatile__ ( diff --git a/components/freertos/include/freertos/xtensa_context.h b/components/freertos/include/freertos/xtensa_context.h index 58520f8538..9e6fe558f5 100644 --- a/components/freertos/include/freertos/xtensa_context.h +++ b/components/freertos/include/freertos/xtensa_context.h @@ -315,7 +315,7 @@ STRUCT_END(XtSolFrame) /* Macro to get the current core ID. Only uses the reg given as an argument. Reading PRID on the ESP32 gives us 0xCDCD on the PRO processor (0) - and 0xABAB on the APP CPU (1). We distinguish between the two by simply checking + and 0xABAB on the APP CPU (1). We can distinguish between the two by checking bit 13: it's 1 on the APP and 0 on the PRO processor. */ #ifdef __ASSEMBLER__