Add RP2040 support #341

kilograham · 2021-06-02T16:47:28Z

Adds support for RP2040 using the Raspberry Pi Pico SDK. FreeRTOS can be run on either core. SDK semaphores, queues, mutexes, and sleep functions can be used freely to/from FreeRTOS tasks and can be used to interact with code running on the other RP2040 core.

The entirety of this change is located within the RP2040 port except for the new portSTACK_LIMIT_PADDING define (which is only used by stack overflow checking). This is added to allow a port to keep extra state for the switched out task below the normal "top" of stack location. This allows extra information to be saved on the stack without disrupting a debuggers ability to parse the stack frame.

codecov · 2021-06-04T16:34:50Z

Codecov Report

Merging #341 (81af7b2) into main (4200226) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #341   +/-   ##
=======================================
  Coverage   92.13%   92.13%           
=======================================
  Files           4        4           
  Lines        1272     1272           
  Branches      342      342           
=======================================
  Hits         1172     1172           
  Misses         53       53           
  Partials       47       47

Flag	Coverage Δ
unittests	`92.13% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4200226...81af7b2. Read the comment docs.

portable/ThirdParty/GCC/RP2040/LICENSE.TXT

portable/ThirdParty/GCC/RP2040/idle_task_static_memory.c

portable/ThirdParty/GCC/RP2040/include/freertos_sdk_config.h

portable/ThirdParty/GCC/RP2040/idle_task_static_memory.c

paulbartell · 2021-06-04T17:24:51Z

portable/ThirdParty/GCC/RP2040/library.cmake

+        FREE_RTOS_KERNEL_SMP=0
+)
+
+add_library(FreeRTOS-Kernel-Static INTERFACE)


Similar to my comments regarding selecting a heap implementation, perhaps you could include idle_task_static_memory.c if configSUPPORT_STATIC_ALLOCATION is set.

configSUPPORT_STATIC_ALLOCATION is a preprocessor define not a build time definition. This library (perhaps poorly named) was intended as a utility library the user could add to get a basic definition of static defines. Basically we prefer adding a library dependency on an INTERFACE library to pull in code rather than explicitly adding source files.

That said, if this code is indeed generic, and useful for all configSUPPORT_STATIC_ALLOCATION=1, we could add the source file, but then guard the code

paulbartell · 2021-06-04T17:27:46Z

portable/ThirdParty/GCC/RP2040/library.cmake

+        pico_base_headers
+        hardware_exception)
+
+target_compile_definitions(FreeRTOS-Kernel INTERFACE


Could you comment on your choice of INTERFACE rather than STATIC for the library definition?

All libraries within the Raspberry Pi Pico SDK build as INTERFACE libraries to gather build options/configuration settings on a per target binary basis. i.e. there may be multiple target executables in a single cmake build that use differently configured FreeRTOS and or SDK settings

paulbartell · 2021-06-04T17:28:45Z

portable/ThirdParty/GCC/RP2040/library.cmake

@@ -0,0 +1,67 @@
+# Called after the Raspberry Pi Pico SDK has been initialized to add our libraries


This file should have a copyright and SPDX identifier.

paulbartell · 2021-06-04T17:32:59Z

portable/ThirdParty/GCC/RP2040/library.cmake

+
+add_library(FreeRTOS-Kernel INTERFACE)
+target_sources(FreeRTOS-Kernel INTERFACE
+        ${CMAKE_CURRENT_LIST_DIR}/port.c


This should probably be named something port specific.

yeah i mean the namespace is what the user build has included. i.e. they chose to include the RP2040 port of FreeRTOS. We would need a more specific name if installed, but I think that could be handled by namespacing during the packaging step (we don't use system installed libraries yet with the SDK)

paulbartell · 2021-06-04T17:34:39Z

portable/ThirdParty/GCC/RP2040/pico_sdk_import.cmake

@@ -0,0 +1,62 @@
+# This is a copy of <PICO_SDK_PATH>/external/pico_sdk_import.cmake


If included, this file should have a copyright statement and SPDX identifier.

Is that so? the build checks said it wasn't a requirement. That said i don't mind adding one; it is just a verbatim copy of an intended-use-is-copying-to-your-project file from the SDK which doesn't have a copyright notice

Co-authored-by: Paul Bartell <[email protected]>

aggarg · 2021-06-30T00:34:58Z

portable/ThirdParty/GCC/RP2040/idle_task_static_memory.c

+
+#include "FreeRTOS.h"
+
+void vApplicationGetIdleTaskMemory( StaticTask_t **ppxIdleTaskTCBBuffer,


This is supposed to be supplied by the application. Any specific reason of providing it as part of port?

We are providing it as an example/convenience.. it is just an easier way for the user to include it than copying/pasting.

this is a stylistic decision for consistency with other Raspberry Pi Pico SDK software use

aggarg · 2021-06-30T00:47:23Z

portable/ThirdParty/GCC/RP2040/include/portmacro.h

+        #define xPortSysTickHandler isr_systick
+    #endif
+
+    #define portCHECK_IF_IN_ISR() ({ \


Other ports do it this way: https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/main/portable/GCC/ARM_CM4F/portmacro.h#L172

You may want to do the same for consistency.

i think it is fine... achieves the same thing

i can change it if you feel strongly.

aggarg · 2021-06-30T01:12:28Z

include/stack_macros.h

 #if ( ( configCHECK_FOR_STACK_OVERFLOW == 1 ) && ( portSTACK_GROWTH < 0 ) )

 /* Only the current stack state is to be checked. */
    #define taskCHECK_FOR_STACK_OVERFLOW()                                                            \
    {                                                                                                 \
        /* Is the currently saved stack pointer within the stack limit? */                            \
-        if( pxCurrentTCB->pxTopOfStack <= pxCurrentTCB->pxStack )                                     \
+        if( pxCurrentTCB->pxTopOfStack <= pxCurrentTCB->pxStack + portSTACK_LIMIT_PADDING)            \


The PendSV handler does not save the SP with extra words 4 in TCB - so this check would not do detect the boundary overflow you are trying to detect? Therefore, I suggest that we remove this common change.

can you clarify why this doesn't detect anything?

i mean the point is that the value saved does not include the extra words, which is why it is checked here... if you save a different pointer in the TCB then it breaks the ability to debug with openocd etc.

Note that this is the only piece of code I found that actually cared whether there was something below pxTopOfStack

* Add RP2040 support * remove spurious tab/spaces comments * add .cmake to ignored kernel checks * Apply suggestions from code review Co-authored-by: Paul Bartell <[email protected]> * license and end of file newline fixes * Rename LICENSE.TXT to LICENSE.md Co-authored-by: Paul Bartell <[email protected]> Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

* Add RP2040 support (#341) * Add RP2040 support * remove spurious tab/spaces comments * add .cmake to ignored kernel checks * Apply suggestions from code review Co-authored-by: Paul Bartell <[email protected]> * license and end of file newline fixes * Rename LICENSE.TXT to LICENSE.md Co-authored-by: Paul Bartell <[email protected]> Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]> * RP2040 updates for SMP * whitepsace fix Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]> * whitespace fix Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]> Co-authored-by: Paul Bartell <[email protected]> Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

* Port shadow demo from product 4 * Fix coding style and formating * Change assert to configASSERT and fix comments. * Delete the demo task when it finish * Relocate the shadow demo code under AWS folder. * Fix uncrustify. * Update FreeRTOS-Plus/Demo/AWS/Device_Shadow_Windows_Simulator/Device_Shadow_Demo/DemoTasks/ShadowDemoMainExample.c Co-authored-by: Oscar Michael Abrina <[email protected]> * Update FreeRTOS-Plus/Demo/AWS/Device_Shadow_Windows_Simulator/Device_Shadow_Demo/DemoTasks/ShadowDemoMainExample.c Co-authored-by: Oscar Michael Abrina <[email protected]> * Fix comments * Update from comments. Co-authored-by: Oscar Michael Abrina <[email protected]>

kilograham requested a review from a team as a code owner June 2, 2021 16:47

kilograham mentioned this pull request Jun 2, 2021

Rp2040 smp #342

Merged

paulbartell reviewed Jun 4, 2021

View reviewed changes

kilograham force-pushed the rp2040 branch from b221e7e to 79b3016 Compare June 4, 2021 21:22

kilograham and others added 6 commits June 9, 2021 14:11

Add RP2040 support

c2d3be8

remove spurious tab/spaces comments

da768f2

add .cmake to ignored kernel checks

5e71554

Apply suggestions from code review

705622c

Co-authored-by: Paul Bartell <[email protected]>

license and end of file newline fixes

489ef8a

Rename LICENSE.TXT to LICENSE.md

07a58ee

kilograham force-pushed the rp2040 branch from 43daef4 to 07a58ee Compare June 9, 2021 19:11

aggarg and others added 2 commits June 22, 2021 11:17

Merge branch 'main' into rp2040

2e533eb

Merge branch 'main' into rp2040

9b3a8c4

aggarg requested changes Jun 30, 2021

View reviewed changes

aggarg approved these changes Jun 30, 2021

View reviewed changes

Merge branch 'main' into rp2040

81af7b2

n9wxu approved these changes Jun 30, 2021

View reviewed changes

n9wxu merged commit 9af72db into FreeRTOS:main Jun 30, 2021

maxregan mentioned this pull request Oct 6, 2021

Add support for FreeRTOS platformio/platform-raspberrypi#18

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RP2040 support #341

Add RP2040 support #341

kilograham commented Jun 2, 2021

codecov bot commented Jun 4, 2021 •

edited

Loading

paulbartell Jun 4, 2021

kilograham Jun 4, 2021

paulbartell Jun 4, 2021

kilograham Jun 4, 2021

paulbartell Jun 4, 2021

paulbartell Jun 4, 2021

kilograham Jun 4, 2021

paulbartell Jun 4, 2021

kilograham Jun 4, 2021

aggarg Jun 30, 2021

kilograham Jun 30, 2021

kilograham Jun 30, 2021

aggarg Jun 30, 2021

kilograham Jun 30, 2021

kilograham Jun 30, 2021

aggarg Jun 30, 2021

kilograham Jun 30, 2021 •

edited

Loading

kilograham Jun 30, 2021

		@@ -0,0 +1,67 @@
		# Called after the Raspberry Pi Pico SDK has been initialized to add our libraries

		@@ -0,0 +1,62 @@
		# This is a copy of <PICO_SDK_PATH>/external/pico_sdk_import.cmake


		#include "FreeRTOS.h"

		void vApplicationGetIdleTaskMemory( StaticTask_t **ppxIdleTaskTCBBuffer,

Add RP2040 support #341

Add RP2040 support #341

Conversation

kilograham commented Jun 2, 2021

codecov bot commented Jun 4, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kilograham Jun 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 4, 2021 •

edited

Loading

kilograham Jun 30, 2021 •

edited

Loading