Skip to content
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

drivers:platform:freeRTOS:Added mem functions #2427

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

landas
Copy link
Contributor

@landas landas commented Jan 29, 2025

Add freeRTOS memory functions for no-OS implementation.

Pull Request Description

Added memory allocation functions for use in freeRTOS platform.

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the Coding style guidelines
  • I have performed a self-review of the changes
  • I have commented my code, at least hard-to-understand parts
  • I have build all projects affected by the changes in this PR
  • I have tested in hardware affected projects, at the relevant boards
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe etc), if applies

@buha
Copy link
Contributor

buha commented Jan 29, 2025

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@buha
Copy link
Contributor

buha commented Jan 29, 2025

Looks good to me

As a side note, I think freertos.mk should contain something like, perhaps as a separate commit:

SRCS += $(NO-OS)/drivers/platform/freeRTOS/freertos_mutex.c \
        $(NO-OS)/drivers/platform/freeRTOS/freertos_semaphore.c \
        $(NO-OS)/drivers/platform/freeRTOS/freertos_delay.c \
        $(NO-OS)/drivers/platform/freeRTOS/freertos_alloc.c

This would avoid having to specify these in the project .mk files.

@landas landas force-pushed the freertos branch 3 times, most recently from ae261cc to c390cfe Compare January 29, 2025 10:36
@landas
Copy link
Contributor Author

landas commented Jan 29, 2025

I was a bit hasty with my pull request. The last commit is compiling without error or warnings with STM32CubeIDE.

* @param size - Size of the memory block, in bytes.
* @return Pointer to the allocated memory, or NULL if the request fails.
*/
__attribute__((weak)) void *no_os_malloc(size_t size)
Copy link
Contributor

@buha buha Jan 29, 2025

Choose a reason for hiding this comment

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

is the weak linkage specified needed here ? i would argue that this file is THE implementation of no_os_alloc (.c/.h) for freertos platform so it should replace the 3 alloc API functions there (which are specified as weak) with non-weak versions

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, is the weak linkage what changed between your first and 2nd version ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that the weak linkage should be removed, and I have done that in my last commit now.

Add freeRTOS memory functions for no-OS implementation.

Signed-off-by: Lars Andre Landås <[email protected]>
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