Skip to content

pico_flash: Support FreeRTOS Static Allocation #2229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

SConaway
Copy link
Contributor

@SConaway SConaway commented Feb 3, 2025

Fixes #2120

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

@kilograham kilograham added this to the 2.1.1 milestone Feb 3, 2025
@kilograham
Copy link
Contributor

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
Copy link
Contributor Author

SConaway commented Feb 5, 2025

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
Copy link
Contributor Author

SConaway commented Feb 5, 2025

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
Copy link
Contributor

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

Copy link
Contributor

@kilograham kilograham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@kilograham kilograham merged commit 876c09c into raspberrypi:develop Feb 7, 2025
4 checks passed
will-v-pi pushed a commit to will-v-pi/pico-sdk that referenced this pull request Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants