From debbd254b663815ded3a473d2183a9b59d58c702 Mon Sep 17 00:00:00 2001 From: Graham Sanderson Date: Tue, 21 Dec 2021 13:08:41 -0600 Subject: [PATCH] RP2040 fixes (#424) * RP2040: malloc needs to be thread safe for FreeRTOS whether both cores are used or not * RP2040: CMake file had broken left over test code * RP2040: portIS_FREE_RTOS_CORE() returned an incorrect value prior to scheduler init when the application was compiled without multicore support * RP2040: Bad initialization code was causing IRQs to get disabled before main() was called when using non static allocation --- .../GCC/RP2040/FreeRTOS_Kernel_import.cmake | 3 +-- .../GCC/RP2040/include/freertos_sdk_config.h | 5 +++- portable/ThirdParty/GCC/RP2040/port.c | 23 +++++++++---------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/portable/ThirdParty/GCC/RP2040/FreeRTOS_Kernel_import.cmake b/portable/ThirdParty/GCC/RP2040/FreeRTOS_Kernel_import.cmake index dc68ed038..1f0bf1196 100644 --- a/portable/ThirdParty/GCC/RP2040/FreeRTOS_Kernel_import.cmake +++ b/portable/ThirdParty/GCC/RP2040/FreeRTOS_Kernel_import.cmake @@ -33,8 +33,7 @@ endif () if (NOT FREERTOS_KERNEL_PATH) foreach(POSSIBLE_SUFFIX Source FreeRTOS-Kernel FreeRTOS/Source) # check if FreeRTOS-Kernel exists under directory that included us - set(SEARCH_ROOT ${CMAKE_CURRENT_SOURCE_DIR}}) - set(SEARCH_ROOT ../../../..) + set(SEARCH_ROOT ${CMAKE_CURRENT_SOURCE_DIR}) get_filename_component(_POSSIBLE_PATH ${SEARCH_ROOT}/${POSSIBLE_SUFFIX} REALPATH) if (EXISTS ${_POSSIBLE_PATH}/${FREERTOS_KERNEL_RP2040_RELATIVE_PATH}/CMakeLists.txt) get_filename_component(FREERTOS_KERNEL_PATH ${_POSSIBLE_PATH} REALPATH) diff --git a/portable/ThirdParty/GCC/RP2040/include/freertos_sdk_config.h b/portable/ThirdParty/GCC/RP2040/include/freertos_sdk_config.h index 1a67d28f1..bc9017d58 100644 --- a/portable/ThirdParty/GCC/RP2040/include/freertos_sdk_config.h +++ b/portable/ThirdParty/GCC/RP2040/include/freertos_sdk_config.h @@ -32,7 +32,10 @@ #ifndef __ASSEMBLER__ #include "FreeRTOSConfig.h" #include "rp2040_config.h" - + #ifndef PICO_USE_MALLOC_MUTEX + // malloc needs to be made thread safe + #define PICO_USE_MALLOC_MUTEX 1 + #endif /* PICO_USE_MALLOC_MUTEX */ #if ( configSUPPORT_PICO_SYNC_INTEROP == 1 ) // increase the amount of time it may reasonably take to wake us up #ifndef PICO_TIME_SLEEP_OVERHEAD_ADJUST_US diff --git a/portable/ThirdParty/GCC/RP2040/port.c b/portable/ThirdParty/GCC/RP2040/port.c index 6b54adc38..bf6ff151a 100644 --- a/portable/ThirdParty/GCC/RP2040/port.c +++ b/portable/ThirdParty/GCC/RP2040/port.c @@ -110,8 +110,9 @@ static void prvTaskExitError( void ); /*-----------------------------------------------------------*/ /* Each task maintains its own interrupt status in the critical nesting - * variable. */ -static UBaseType_t uxCriticalNesting = {0xaaaaaaaa}; + * variable. This is initialized to 0 to allow vPortEnter/ExitCritical + * to be called before the scheduler is started */ +static UBaseType_t uxCriticalNesting; /*-----------------------------------------------------------*/ @@ -158,13 +159,9 @@ static UBaseType_t uxCriticalNesting = {0xaaaaaaaa}; /*-----------------------------------------------------------*/ -#if ( LIB_PICO_MULTICORE == 1 ) - #define INVALID_LAUNCH_CORE_NUM 0xffu - static uint8_t ucLaunchCoreNum = INVALID_LAUNCH_CORE_NUM; - #define portIS_FREE_RTOS_CORE() ( ucLaunchCoreNum == get_core_num() ) -#else - #define portIS_FREE_RTOS_CORE() pdTRUE -#endif /* LIB_PICO_MULTICORE */ +#define INVALID_LAUNCH_CORE_NUM 0xffu +static uint8_t ucLaunchCoreNum = INVALID_LAUNCH_CORE_NUM; +#define portIS_FREE_RTOS_CORE() ( ucLaunchCoreNum == get_core_num() ) /* * See header file for description. @@ -266,8 +263,8 @@ BaseType_t xPortStartScheduler( void ) /* Initialise the critical nesting count ready for the first task. */ uxCriticalNesting = 0; + ucLaunchCoreNum = get_core_num(); #if (LIB_PICO_MULTICORE == 1) - ucLaunchCoreNum = get_core_num(); #if ( configSUPPORT_PICO_SYNC_INTEROP == 1) multicore_fifo_clear_irq(); multicore_fifo_drain(); @@ -728,7 +725,6 @@ __attribute__( ( weak ) ) void vPortSetupTimerInterrupt( void ) void vPortLockInternalSpinUnlockWithWait( struct lock_core * pxLock, uint32_t ulSave ) { configASSERT( !portCHECK_IF_IN_ISR() ); - // note no need to check LIB_PICO_MULTICORE, as this is always returns true if that is not defined if( !portIS_FREE_RTOS_CORE() ) { spin_unlock(pxLock->spin_lock, ulSave ); @@ -806,7 +802,6 @@ __attribute__( ( weak ) ) void vPortSetupTimerInterrupt( void ) } else { - configASSERT( portIS_FREE_RTOS_CORE() ); configASSERT( pxYieldSpinLock == NULL ); TickType_t uxTicksToWait = prvGetTicksToWaitBefore( uxUntil ); @@ -858,6 +853,10 @@ __attribute__( ( weak ) ) void vPortSetupTimerInterrupt( void ) #if ( configSUPPORT_STATIC_ALLOCATION == 1 ) xEventGroup = xEventGroupCreateStatic(&xStaticEventGroup); #else + /* Note that it is slightly dubious calling this here before the scheduler is initialized, + * however the only thing it touches is the allocator which then calls vPortEnterCritical + * and vPortExitCritical, and allocating here saves us checking the one time initialized variable in + * some rather critical code paths */ xEventGroup = xEventGroupCreate(); #endif /* configSUPPORT_STATIC_ALLOCATION */ }