From 7d0c2e0fb01d2cbfd4a996651e407b805198f5db Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Tue, 18 Feb 2025 11:55:40 +0000 Subject: [PATCH] CI: Add pendsv build patch from upstream. --- boards/pendsv_build.patch | 186 ++++++++++++++++++++++++++++++++++ boards/presto/mpconfigboard.h | 8 +- ci/micropython.sh | 2 + 3 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 boards/pendsv_build.patch diff --git a/boards/pendsv_build.patch b/boards/pendsv_build.patch new file mode 100644 index 0000000..564c22d --- /dev/null +++ b/boards/pendsv_build.patch @@ -0,0 +1,186 @@ +From 71df9d0636c8b0b95ab009f82bde7d5b970835ec Mon Sep 17 00:00:00 2001 +From: Angus Gratton +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 +--- + 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 +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 +--- + 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 +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 +--- + 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 diff --git a/boards/presto/mpconfigboard.h b/boards/presto/mpconfigboard.h index cc149bd..371df28 100644 --- a/boards/presto/mpconfigboard.h +++ b/boards/presto/mpconfigboard.h @@ -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; \ No newline at end of file +#define MICROPY_GC_SPLIT_HEAP (0) \ No newline at end of file diff --git a/ci/micropython.sh b/ci/micropython.sh index 6eb0c5e..6959c62 100644 --- a/ci/micropython.sh +++ b/ci/micropython.sh @@ -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" }