Skip to content

Commit

Permalink
CI: Add pendsv build patch from upstream.
Browse files Browse the repository at this point in the history
  • Loading branch information
Gadgetoid committed Feb 18, 2025
1 parent 6805456 commit 7d0c2e0
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 7 deletions.
186 changes: 186 additions & 0 deletions boards/pendsv_build.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
From 71df9d0636c8b0b95ab009f82bde7d5b970835ec Mon Sep 17 00:00:00 2001
From: Angus Gratton <[email protected]>
Date: Tue, 11 Feb 2025 16:42:03 +1100
Subject: [PATCH 1/3] rp2: Fix build failure if threads are disabled.

Regression in 3af006ef meant that pendsv.c no longer compiled if threads
were disabled in the build config. Add an implementation based on the
earlier one (simple counter) for the non-threads case.

It seems like with the current usage patterns there's no need for the
counter to be incremented/decremented atomically on a single core config.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <[email protected]>
---
ports/rp2/pendsv.c | 49 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/ports/rp2/pendsv.c b/ports/rp2/pendsv.c
index 4ba1e81604b0f..2c086f89429be 100644
--- a/ports/rp2/pendsv.c
+++ b/ports/rp2/pendsv.c
@@ -43,10 +43,14 @@

static pendsv_dispatch_t pendsv_dispatch_table[PENDSV_DISPATCH_NUM_SLOTS];

+static inline void pendsv_resume_run_dispatch(void);
+
void PendSV_Handler(void);

-// Using the nowait variant here as softtimer updates PendSV from the loop of mp_wfe_or_timeout(),
-// where we don't want the CPU event bit to be set.
+#if MICROPY_PY_THREAD
+
+// Important to use a 'nowait' mutex here as softtimer updates PendSV from the
+// loop of mp_wfe_or_timeout(), where we don't want the CPU event bit to be set.
static mp_thread_recursive_mutex_t pendsv_mutex;

void pendsv_init(void) {
@@ -62,7 +66,40 @@ void pendsv_suspend(void) {

void pendsv_resume(void) {
mp_thread_recursive_mutex_unlock(&pendsv_mutex);
+ pendsv_resume_run_dispatch();
+}
+
+static inline int pendsv_suspend_count(void) {
+ return pendsv_mutex.mutex.enter_count;
+}
+
+#else
+
+// Without threads we don't include any pico-sdk mutex in the build,
+// but also we don't need to worry about cross-thread contention (or
+// races with interrupts that update this counter).
+static int pendsv_lock;
+
+void pendsv_init(void) {
+}
+
+void pendsv_suspend(void) {
+ pendsv_lock++;
+}
+
+void pendsv_resume(void) {
+ assert(pendsv_lock > 0);
+ pendsv_lock--;
+ pendsv_resume_run_dispatch();
+}

+static inline int pendsv_suspend_count(void) {
+ return pendsv_lock;
+}
+
+#endif
+
+static inline void pendsv_resume_run_dispatch(void) {
// Run pendsv if needed. Find an entry with a dispatch and call pendsv dispatch
// with it. If pendsv runs it will service all slots.
int count = PENDSV_DISPATCH_NUM_SLOTS;
@@ -76,7 +113,7 @@ void pendsv_resume(void) {

void pendsv_schedule_dispatch(size_t slot, pendsv_dispatch_t f) {
pendsv_dispatch_table[slot] = f;
- if (pendsv_mutex.mutex.enter_count == 0) {
+ if (pendsv_suspend_count() == 0) {
#if PICO_ARM
// There is a race here where other core calls pendsv_suspend() before
// ISR can execute, but dispatch will happen later when other core
@@ -97,6 +134,7 @@ void pendsv_schedule_dispatch(size_t slot, pendsv_dispatch_t f) {
// PendSV interrupt handler to perform background processing.
void PendSV_Handler(void) {

+ #if MICROPY_PY_THREAD
if (!mp_thread_recursive_mutex_lock(&pendsv_mutex, 0)) {
// Failure here means core 1 holds pendsv_mutex. ISR will
// run again after core 1 calls pendsv_resume().
@@ -104,6 +142,9 @@ void PendSV_Handler(void) {
}
// Core 0 should not already have locked pendsv_mutex
assert(pendsv_mutex.mutex.enter_count == 1);
+ #else
+ assert(pendsv_suspend_count() == 0);
+ #endif

#if MICROPY_PY_NETWORK_CYW43
CYW43_STAT_INC(PENDSV_RUN_COUNT);
@@ -117,5 +158,7 @@ void PendSV_Handler(void) {
}
}

+ #if MICROPY_PY_THREAD
mp_thread_recursive_mutex_unlock(&pendsv_mutex);
+ #endif
}

From 516709be88571da0195125c840e772a3ec70fa66 Mon Sep 17 00:00:00 2001
From: Angus Gratton <[email protected]>
Date: Tue, 11 Feb 2025 17:44:19 +1100
Subject: [PATCH 2/3] py/mkrules.cmake: Support passing CFLAGS_EXTRA in
environment variable.

This works similarly to the existing support in "bare metal" make ports,
with the caveat that CMake will only set this value on a clean build and
will reuse the previous value otherwise.

This is slightly different to the CMake built-in support for CFLAGS,
as this variable is used when evaluating source files for qstr
generation, etc.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <[email protected]>
---
py/mkrules.cmake | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/py/mkrules.cmake b/py/mkrules.cmake
index 3ee4c4c31aa57..907500dd67534 100644
--- a/py/mkrules.cmake
+++ b/py/mkrules.cmake
@@ -53,6 +53,15 @@ foreach(_arg ${MICROPY_CPP_DEF})
endforeach()
list(APPEND MICROPY_CPP_FLAGS ${MICROPY_CPP_FLAGS_EXTRA})

+# Include anything passed in via CFLAGS_EXTRA
+# in both MICROPY_CPP_FLAGS and CMAKE_C_FLAGS
+if(DEFINED ENV{CFLAGS_EXTRA})
+ set(CFLAGS_EXTRA $ENV{CFLAGS_EXTRA})
+ string(APPEND CMAKE_C_FLAGS " ${CFLAGS_EXTRA}") # ... not a list
+ separate_arguments(CFLAGS_EXTRA)
+ list(APPEND MICROPY_CPP_FLAGS ${CFLAGS_EXTRA}) # ... a list
+endif()
+
find_package(Python3 REQUIRED COMPONENTS Interpreter)

target_sources(${MICROPY_TARGET} PRIVATE

From 17038f806ad6e8ceb497ca60c414af8dc3c8b38e Mon Sep 17 00:00:00 2001
From: Angus Gratton <[email protected]>
Date: Tue, 11 Feb 2025 17:46:33 +1100
Subject: [PATCH 3/3] tools/ci,ports/rp2: Build the W5100S_EVB_PICO board with
no threads.

Serves as a build test for a config we don't otherwise support.

Signed-off-by: Angus Gratton <[email protected]>
---
tools/ci.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/ci.sh b/tools/ci.sh
index ff362efd29e2d..d50af4a0ae2ee 100755
--- a/tools/ci.sh
+++ b/tools/ci.sh
@@ -342,7 +342,8 @@ function ci_rp2_build {
make ${MAKEOPTS} -C ports/rp2 BOARD=RPI_PICO2 submodules
make ${MAKEOPTS} -C ports/rp2 BOARD=RPI_PICO2
make ${MAKEOPTS} -C ports/rp2 BOARD=W5100S_EVB_PICO submodules
- make ${MAKEOPTS} -C ports/rp2 BOARD=W5100S_EVB_PICO
+ # This build doubles as a build test for disabling threads in the config
+ make ${MAKEOPTS} -C ports/rp2 BOARD=W5100S_EVB_PICO CFLAGS_EXTRA=-DMICROPY_PY_THREAD=0

# Test building ninaw10 driver and NIC interface.
make ${MAKEOPTS} -C ports/rp2 BOARD=ARDUINO_NANO_RP2040_CONNECT submodules
8 changes: 1 addition & 7 deletions boards/presto/mpconfigboard.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,4 @@ int mp_hal_is_pin_reserved(int n);
#define MICROPY_HW_PSRAM_CS_PIN PIMORONI_PRESTO_PSRAM_CS_PIN

#define MICROPY_PY_THREAD (0)
#define MICROPY_GC_SPLIT_HEAP (0)

// Hack for pendsv.c using "mp_thread_recursive_mutex_t"
struct _mp_state_thread_t;
#include "mpthreadport.h"

typedef recursive_mutex_nowait_t mp_thread_recursive_mutex_t;
#define MICROPY_GC_SPLIT_HEAP (0)
2 changes: 2 additions & 0 deletions ci/micropython.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ function ci_micropython_clone {
cd "$CI_BUILD_ROOT/micropython/lib/pico-sdk"
log_inform "HACK: cyw43 stability backport for Pico2 W. See: https://github.com/raspberrypi/pico-sdk/pull/2209"
git apply "$CI_PROJECT_ROOT/boards/pico2_w_cyw43.patch"
log_inform "HACK: fix for pendsv build regression. See: https://github.com/micropython/micropython/pull/16733"
git apply "$CI_PROJECT_ROOT/boards/pendsv_build.patch"
cd "$CI_BUILD_ROOT"
}

Expand Down

0 comments on commit 7d0c2e0

Please sign in to comment.