pico_flash: Support FreeRTOS Static Allocation #2229

2025-02-03T12:26:08-06:00

SConaway commented

2025-02-03 12:26:08 -06:00

(Migrated from github.com)

Allows pico_flash to use FreeRTOS static allocation as currently static-only builds fail without this change.

Fixes #2120 Allows `pico_flash` to use FreeRTOS static allocation as currently static-only builds fail without this change.

kilograham (Migrated from github.com) reviewed

2025-02-03 13:26:52 -06:00

src/rp2_common/pico_flash/flash.c

					
				@ -145,7 +145,16 @@ static int default_enter_safe_zone_timeout_ms(__unused uint32_t timeout_ms) {

				        uint core_num = get_core_num();

should be if !configSUPPORT_DYNAMIC_ALLOCATION

should be `if !configSUPPORT_DYNAMIC_ALLOCATION`

kilograham (Migrated from github.com) reviewed

2025-02-04 16:17:16 -06:00

src/rp2_common/pico_flash/flash.c

					
				@ -145,7 +145,16 @@ static int default_enter_safe_zone_timeout_ms(__unused uint32_t timeout_ms) {

				        uint core_num = get_core_num();

it was also backwards

kilograham commented

2025-02-05 11:07:17 -06:00

(Migrated from github.com)

Seems like this might be a nicer alternative

    TaskHandle_t xTaskCreateStaticAffinitySet( TaskFunction_t pxTaskCode,
                                               const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
                                               const uint32_t ulStackDepth,
                                               void * const pvParameters,
                                               UBaseType_t uxPriority,
                                               StackType_t * const puxStackBuffer,
                                               StaticTask_t * const pxTaskBuffer,
                                               UBaseType_t uxCoreAffinityMask ) PRIVILEGED_FUNCTION;

Seems like this might be a nicer alternative ``` TaskHandle_t xTaskCreateStaticAffinitySet( TaskFunction_t pxTaskCode, const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */ const uint32_t ulStackDepth, void * const pvParameters, UBaseType_t uxPriority, StackType_t * const puxStackBuffer, StaticTask_t * const pxTaskBuffer, UBaseType_t uxCoreAffinityMask ) PRIVILEGED_FUNCTION; ```

SConaway (Migrated from github.com) reviewed

2025-02-05 11:12:33 -06:00

src/rp2_common/pico_flash/flash.c

					
				@ -145,7 +145,16 @@ static int default_enter_safe_zone_timeout_ms(__unused uint32_t timeout_ms) {

				        uint core_num = get_core_num();

How did I miss that? good catch!

SConaway commented

2025-02-05 11:13:48 -06:00

(Migrated from github.com)

One thing I want to get your thoughts on (and sorry for the couple days of delay here): would it be better to default to static allocation and only switch to dynamic if static allocation isn't enabled?

SConaway commented

2025-02-05 11:14:47 -06:00

(Migrated from github.com)

Seems like this might be a nicer alternative

    TaskHandle_t xTaskCreateStaticAffinitySet( TaskFunction_t pxTaskCode,
                                               const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
                                               const uint32_t ulStackDepth,
                                               void * const pvParameters,
                                               UBaseType_t uxPriority,
                                               StackType_t * const puxStackBuffer,
                                               StaticTask_t * const pxTaskBuffer,
                                               UBaseType_t uxCoreAffinityMask ) PRIVILEGED_FUNCTION;

For some reason I skipped that — I can switch to it and let you know if it compiles. Unfortunately I don't really have any way to make sure that things still work besides compiling.

> Seems like this might be a nicer alternative > > ``` > TaskHandle_t xTaskCreateStaticAffinitySet( TaskFunction_t pxTaskCode, > const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */ > const uint32_t ulStackDepth, > void * const pvParameters, > UBaseType_t uxPriority, > StackType_t * const puxStackBuffer, > StaticTask_t * const pxTaskBuffer, > UBaseType_t uxCoreAffinityMask ) PRIVILEGED_FUNCTION; > ``` For some reason I skipped that — I can switch to it and let you know if it compiles. Unfortunately I don't really have any way to make sure that things still work besides compiling.

kilograham commented

2025-02-05 12:46:04 -06:00

(Migrated from github.com)

compiling is better than me as I don't have a config with this setup, so would have to make one

kilograham (Migrated from github.com) approved these changes

2025-02-06 17:55:27 -06:00

kilograham (Migrated from github.com) left a comment

OK, so this isn't ideal as it will likely crash if flash_safe_execute() is called from both cores concurrently in this case.. I am approving anyway, because

a) This does compile and likely works correctly when not doing that
b) That use case is probably unlikely
c) Arguably the existing implementation is not very robust in that regard, so I will open a separate issue

OK, so this isn't ideal as it will likely crash if `flash_safe_execute()` is called from both cores concurrently in this case.. I am approving anyway, because a) This does compile and likely works correctly when not doing that b) That use case is probably unlikely c) Arguably the existing implementation is not very robust in that regard, so I will open a separate issue

Sign in to join this conversation.

No reviewers

kilograham