From d4b0f85eb4dc5c2708c2c48d978bb24e9a5978a6 Mon Sep 17 00:00:00 2001 From: Jeroen Domburg Date: Mon, 5 Jun 2017 15:54:50 +0800 Subject: [PATCH] Make portENTER_CRITICAL and portENTER_CRITICAL_ISR the same, plus some changes to fix an issue this causes --- .../freertos/include/freertos/portmacro.h | 15 ++++-- components/freertos/tasks.c | 52 ++++++++++++------- tools/unit-test-app/sdkconfig | 9 ++-- 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/components/freertos/include/freertos/portmacro.h b/components/freertos/include/freertos/portmacro.h index ea149b663..00db60c91 100644 --- a/components/freertos/include/freertos/portmacro.h +++ b/components/freertos/include/freertos/portmacro.h @@ -194,6 +194,10 @@ do not disable the interrupts (because they already are). This all assumes that interrupts are either entirely disabled or enabled. Interrupr priority levels will break this scheme. + +Remark: For the ESP32, portENTER_CRITICAL and portENTER_CRITICAL_ISR both alias vTaskEnterCritical, meaning +that either function can be called both from ISR as well as task context. This is not standard FreeRTOS +behaviour; please keep this in mind if you need any compatibility with other FreeRTOS implementations. */ void vPortCPUInitializeMutex(portMUX_TYPE *mux); #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG @@ -203,8 +207,8 @@ void vTaskEnterCritical( portMUX_TYPE *mux, const char *function, int line ); void vTaskExitCritical( portMUX_TYPE *mux, const char *function, int line ); #define portENTER_CRITICAL(mux) vTaskEnterCritical(mux, __FUNCTION__, __LINE__) #define portEXIT_CRITICAL(mux) vTaskExitCritical(mux, __FUNCTION__, __LINE__) -#define portENTER_CRITICAL_ISR(mux) vPortCPUAcquireMutex(mux, __FUNCTION__, __LINE__) -#define portEXIT_CRITICAL_ISR(mux) vPortCPUReleaseMutex(mux, __FUNCTION__, __LINE__) +#define portENTER_CRITICAL_ISR(mux) vTaskEnterCritical(mux, __FUNCTION__, __LINE__) +#define portEXIT_CRITICAL_ISR(mux) vTaskExitCritical(mux, __FUNCTION__, __LINE__) #else void vTaskExitCritical( portMUX_TYPE *mux ); void vTaskEnterCritical( portMUX_TYPE *mux ); @@ -212,13 +216,14 @@ void vPortCPUAcquireMutex(portMUX_TYPE *mux); portBASE_TYPE vPortCPUReleaseMutex(portMUX_TYPE *mux); #define portENTER_CRITICAL(mux) vTaskEnterCritical(mux) #define portEXIT_CRITICAL(mux) vTaskExitCritical(mux) -#define portENTER_CRITICAL_ISR(mux) vPortCPUAcquireMutex(mux) -#define portEXIT_CRITICAL_ISR(mux) vPortCPUReleaseMutex(mux) +#define portENTER_CRITICAL_ISR(mux) vTaskEnterCritical(mux) +#define portEXIT_CRITICAL_ISR(mux) vTaskExitCritical(mux) #endif // Cleaner and preferred solution allows nested interrupts disabling and restoring via local registers or stack. // They can be called from interrupts too. -//NOT SMP-COMPATIBLE! Use only if all you want is to disable the interrupts locally! +// 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; } #define portEXIT_CRITICAL_NESTED(state) do { portbenchmarkINTERRUPT_RESTORE(state); XTOS_RESTORE_JUST_INTLEVEL(state); } while (0) diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index d70a278b1..6e4bbf614 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -2679,11 +2679,8 @@ BaseType_t xSwitchRequired = pdFALSE; void vTaskSwitchContext( void ) { - //Note: This can be called from interrupt context as well as from non-interrupt context (voluntary yield). The - //taskENTER_CRITICAL/taskEXIT_CRITICAL is modified to work in both scenarios for the ESP32, so we can freely use - //them here. However, in case of a voluntary yield, a nonvoluntary yield can still happen *during* the voluntary - //yield. Disabling interrupts using portENTER_CRITICAL_NESTED puts a stop to this and makes the rest of the code a - //bit neater. + //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(); tskTCB * pxTCB; if( uxSchedulerSuspended[ xPortGetCoreID() ] != ( UBaseType_t ) pdFALSE ) @@ -2706,9 +2703,9 @@ void vTaskSwitchContext( void ) #endif /* Add the amount of time the task has been running to the - accumulated time so far. The time the task started running was + accumulated time so far. The time the task started running was stored in ulTaskSwitchedInTime. Note that there is no overflow - protection here so count values are only valid until the timer + protection here so count values are only valid until the timer overflows. The guard against negative values is to protect against suspect run time stat counter implementations - which are provided by the application, not the kernel. */ @@ -2730,13 +2727,18 @@ void vTaskSwitchContext( void ) taskFIRST_CHECK_FOR_STACK_OVERFLOW(); taskSECOND_CHECK_FOR_STACK_OVERFLOW(); - /* Select a new task to run using either the generic C or port - optimised asm code. */ - /* ToDo: either get rid of port-changable task switching stuff, or put all this inside the - taskSELECT_HIGHEST_PRIORITY_TASK macro, then replace this all with a taskSELECT_HIGHEST_PRIORITY_TASK(); - call */ + /* Select a new task to run */ - taskENTER_CRITICAL_ISR(&xTaskQueueMutex); + /* + We cannot do taskENTER_CRITICAL_ISR(&xTaskQueueMutex); here because it saves the interrupt context to the task tcb, and we're + swapping that out here. Instead, we're going to do the work here ourselves. Because interrupts are already disabled, we only + need to acquire the mutex. + */ +#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG + vPortCPUAcquireMutex( &xTaskQueueMutex, __FUNCTION__, __LINE__ ); +#else + vPortCPUAcquireMutex( &xTaskQueueMutex ); +#endif unsigned portBASE_TYPE foundNonExecutingWaiter = pdFALSE, ableToSchedule = pdFALSE, resetListHead; portBASE_TYPE uxDynamicTopReady = uxTopReadyPriority; @@ -2821,12 +2823,17 @@ void vTaskSwitchContext( void ) } --uxDynamicTopReady; } - taskEXIT_CRITICAL_ISR(&xTaskQueueMutex); - - /* ToDo: taskSELECT_HIGHEST_PRIORITY_TASK replacement code ends here. */ traceTASK_SWITCHED_IN(); + //Exit critical region manually as well: release the mux now, interrupts will be re-enabled when we + //exit the function. +#ifdef CONFIG_FREERTOS_PORTMUX_DEBUG + vPortCPUReleaseMutex( &xTaskQueueMutex, function, line ); +#else + vPortCPUReleaseMutex( &xTaskQueueMutex ); +#endif + #if CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK vPortSetStackWatchpoint(pxCurrentTCB[xPortGetCoreID()]->pxStack); #endif @@ -4060,7 +4067,10 @@ TCB_t *pxTCB; /* Gotcha (which seems to be deliberate in FreeRTOS, according to http://www.freertos.org/FreeRTOS_Support_Forum_Archive/December_2012/freertos_PIC32_Bug_-_vTaskEnterCritical_6400806.html ) is that calling vTaskEnterCritical followed by vTaskExitCritical will leave the interrupts DISABLED when the scheduler -is not running. Re-enabling the scheduler will re-enable the interrupts instead. */ +is not running. Re-enabling the scheduler will re-enable the interrupts instead. + +For ESP32 FreeRTOS, vTaskEnterCritical implements both portENTER_CRITICAL and portENTER_CRITICAL_ISR. +*/ #if ( portCRITICAL_NESTING_IN_TCB == 1 ) @@ -4103,7 +4113,9 @@ is not running. Re-enabling the scheduler will re-enable the interrupts instead protect against recursive calls if the assert function also uses a critical section. */ - /* DISABLED in the esp32 port - because of SMP, vTaskEnterCritical + /* DISABLED in the esp32 port - because of SMP, For ESP32 + FreeRTOS, vTaskEnterCritical implements both + portENTER_CRITICAL and portENTER_CRITICAL_ISR. vTaskEnterCritical has to be used in way more places than before, and some are called both from ISR as well as non-ISR code, thus we re-organized vTaskEnterCritical to also work in ISRs. */ @@ -4124,6 +4136,10 @@ is not running. Re-enabling the scheduler will re-enable the interrupts instead #endif /* portCRITICAL_NESTING_IN_TCB */ /*-----------------------------------------------------------*/ + +/* +For ESP32 FreeRTOS, vTaskExitCritical implements both portEXIT_CRITICAL and portEXIT_CRITICAL_ISR. +*/ #if ( portCRITICAL_NESTING_IN_TCB == 1 ) #ifdef CONFIG_FREERTOS_PORTMUX_DEBUG diff --git a/tools/unit-test-app/sdkconfig b/tools/unit-test-app/sdkconfig index e4b897348..abbb29229 100644 --- a/tools/unit-test-app/sdkconfig +++ b/tools/unit-test-app/sdkconfig @@ -111,12 +111,9 @@ CONFIG_ESP32_ENABLE_COREDUMP_TO_NONE=y # CONFIG_ESP32_APPTRACE_DEST_UART is not set CONFIG_ESP32_APPTRACE_DEST_NONE=y # CONFIG_ESP32_APPTRACE_ENABLE is not set -CONFIG_BASE_MAC_STORED_DEFAULT_EFUSE=y -# CONFIG_BASE_MAC_STORED_CUSTOMER_DEFINED_EFUSE is not set -# CONFIG_BASE_MAC_STORED_OTHER_CUSTOMER_DEFINED_PLACE is not set -# CONFIG_TWO_MAC_ADDRESS_FROM_EFUSE is not set -CONFIG_FOUR_MAC_ADDRESS_FROM_EFUSE=y -CONFIG_NUMBER_OF_MAC_ADDRESS_GENERATED_FROM_EFUSE=4 +# CONFIG_TWO_UNIVERSAL_MAC_ADDRESS is not set +CONFIG_FOUR_UNIVERSAL_MAC_ADDRESS=y +CONFIG_NUMBER_OF_UNIVERSAL_MAC_ADDRESS=4 CONFIG_SYSTEM_EVENT_QUEUE_SIZE=32 CONFIG_SYSTEM_EVENT_TASK_STACK_SIZE=2048 CONFIG_MAIN_TASK_STACK_SIZE=4096