micropython · dpgeorge · Jan 3, 2024 · Jan 1, 2024 · Jan 2, 2024 · Jan 1, 2024
diff --git a/LICENSE b/LICENSE
@@ -69,6 +69,8 @@ used during the build process and is not part of the compiled source code.
             /FreeRTOS (GPL-2.0 with FreeRTOS exception)
         /esp32
             /ppp_set_auth.* (Apache-2.0)
+        /rp2
+            /mutex_extra.c (BSD-3-clause)
         /stm32
             /usbd*.c (MCD-ST Liberty SW License Agreement V2)
             /stm32_it.* (MIT + BSD-3-clause)

diff --git a/ports/rp2/CMakeLists.txt b/ports/rp2/CMakeLists.txt
@@ -129,6 +129,7 @@ set(MICROPY_SOURCE_PORT
     mphalport.c
     mpnetworkport.c
     mpthreadport.c
+    mutex_extra.c
     pendsv.c
     rp2_flash.c
     rp2_pio.c

diff --git a/ports/rp2/mpthreadport.c b/ports/rp2/mpthreadport.c
@@ -30,6 +30,7 @@
 #include "py/mpthread.h"
 #include "pico/stdlib.h"
 #include "pico/multicore.h"
+#include "mutex_extra.h"
 
 #if MICROPY_PY_THREAD
 
@@ -45,35 +46,31 @@ STATIC uint32_t *core1_stack = NULL;
 STATIC size_t core1_stack_num_words = 0;
 
 // Thread mutex.
-STATIC mp_thread_mutex_t atomic_mutex;
+STATIC mutex_t atomic_mutex;
 
 uint32_t mp_thread_begin_atomic_section(void) {
     if (core1_entry) {
         // When both cores are executing, we also need to provide
         // full mutual exclusion.
-        mp_thread_mutex_lock(&atomic_mutex, 1);
-        // In case this atomic section is for flash access, then
-        // suspend the other core.
-        multicore_lockout_start_blocking();
+        return mutex_enter_blocking_and_disable_interrupts(&atomic_mutex);
+    } else {
+        return save_and_disable_interrupts();
     }
-
-    return save_and_disable_interrupts();
 }
 
 void mp_thread_end_atomic_section(uint32_t state) {
-    restore_interrupts(state);
-
-    if (core1_entry) {
-        multicore_lockout_end_blocking();
-        mp_thread_mutex_unlock(&atomic_mutex);
8000    if (atomic_mutex.owner != LOCK_INVALID_OWNER_ID) {
+        mutex_exit_and_restore_interrupts(&atomic_mutex, state);
+    } else {
+        restore_interrupts(state);
     }
 }
 
 // Initialise threading support.
 void mp_thread_init(void) {
     assert(get_core_num() == 0);
 
-    mp_thread_mutex_init(&atomic_mutex);
+    mutex_init(&atomic_mutex);
 
     // Allow MICROPY_BEGIN_ATOMIC_SECTION to be invoked from core1.
     multicore_lockout_victim_init();

diff --git a/ports/rp2/mutex_extra.c b/ports/rp2/mutex_extra.c
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2020 Raspberry Pi (Trading) Ltd.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include "mutex_extra.h"
+
+// These functions are taken from lib/pico-sdk/src/common/pico_sync/mutex.c and modified
+// so that they atomically obtain the mutex and disable interrupts.
+
+uint32_t __time_critical_func(mutex_enter_blocking_and_disable_interrupts)(mutex_t * mtx) {
+    lock_owner_id_t caller = lock_get_caller_owner_id();
+    do {
+        uint32_t save = spin_lock_blocking(mtx->core.spin_lock);
+        if (!lock_is_owner_id_valid(mtx->owner)) {
+            mtx->owner = caller;
+            spin_unlock_unsafe(mtx->core.spin_lock);
+            return save;
+        }
+        lock_internal_spin_unlock_with_wait(&mtx->core, save);
+    } while (true);
+}
+
+void __time_critical_func(mutex_exit_and_restore_interrupts)(mutex_t * mtx, uint32_t save) {
+    spin_lock_unsafe_blocking(mtx->core.spin_lock);
+    assert(lock_is_owner_id_valid(mtx->owner));
+    mtx->owner = LOCK_INVALID_OWNER_ID;
+    lock_internal_spin_unlock_with_notify(&mtx->core, save);
+}
diff --git a/ports/rp2/mutex_extra.h b/ports/rp2/mutex_extra.h
@@ -0,0 +1,34 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2023 Damien P. George
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef MICROPY_INCLUDED_RP2_MUTEX_EXTRA_H
+#define MICROPY_INCLUDED_RP2_MUTEX_EXTRA_H
+
+#include "pico/mutex.h"
+
+uint32_t mutex_enter_blocking_and_disable_interrupts(mutex_t *mtx);
+void mutex_exit_and_restore_interrupts(mutex_t *mtx, uint32_t save);
+
+#endif // MICROPY_INCLUDED_RP2_MUTEX_EXTRA_H
diff --git a/ports/rp2/rp2_flash.c b/ports/rp2/rp2_flash.c
@@ -70,6 +70,22 @@ bi_decl(bi_block_device(
     BINARY_INFO_BLOCK_DEV_FLAG_WRITE |
     BINARY_INFO_BLOCK_DEV_FLAG_PT_UNKNOWN));
 
+// Flash erase and write must run with interrupts disabled and the other core suspended,
+// because the XIP bit gets disabled.
+static uint32_t begin_critical_flash_section(void) {
+    if (multicore_lockout_victim_is_initialized(1 - get_core_num())) {
+        multicore_lockout_start_blocking();
+    }
+    return save_and_disable_interrupts();
+}
+
+static void end_critical_flash_section(uint32_t state) {
+    restore_interrupts(state);
+    if (multicore_lockout_victim_is_initialized(1 - get_core_num())) {
+        multicore_lockout_end_blocking();
+    }
+}
+
 STATIC mp_obj_t rp2_flash_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
     // Parse arguments
     enum { ARG_start, ARG_len };
@@ -135,19 +151,17 @@ STATIC mp_obj_t rp2_flash_writeblocks(size_t n_args, const mp_obj_t *args) {
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[2], &bufinfo, MP_BUFFER_READ);
     if (n_args == 3) {
-        // Flash erase/program must run in an atomic section because the XIP bit gets disabled.
-        mp_uint_t atomic_state = MICROPY_BEGIN_ATOMIC_SECTION();
+        mp_uint_t atomic_state = begin_critical_flash_section();
         flash_range_erase(self->flash_base + offset, bufinfo.len);
-        MICROPY_END_ATOMIC_SECTION(atomic_state);
+        end_critical_flash_section(atomic_state);
         mp_event_handle_nowait();
         // TODO check return value
     } else {
         offset += mp_obj_get_int(args[3]);
     }
-    // Flash erase/program must run in an atomic section because the XIP bit gets disabled.
-    mp_uint_t atomic_state = MICROPY_BEGIN_ATOMIC_SECTION();
+    mp_uint_t atomic_state = begin_critical_flash_section();
     flash_range_program(self->flash_base + offset, bufinfo.buf, bufinfo.len);
-    MICROPY_END_ATOMIC_SECTION(atomic_state);
+    end_critical_flash_section(atomic_state);
     mp_event_handle_nowait();
     // TODO check return value
     return mp_const_none;
@@ -170,10 +184,9 @@ STATIC mp_obj_t rp2_flash_ioctl(mp_obj_t self_in, mp_obj_t cmd_in, mp_obj_t arg_
             return MP_OBJ_NEW_SMALL_INT(BLOCK_SIZE_BYTES);
         case MP_BLOCKDEV_IOCTL_BLOCK_ERASE: {
             uint32_t offset = mp_obj_get_int(arg_in) * BLOCK_SIZE_BYTES;
-            // Flash erase/program must run in an atomic section because the XIP bit gets disabled.
-            mp_uint_t atomic_state = MICROPY_BEGIN_ATOMIC_SECTION();
+            mp_uint_t atomic_state = begin_critical_flash_section();
             flash_range_erase(self->flash_base + offset, BLOCK_SIZE_BYTES);
-            MICROPY_END_ATOMIC_SECTION(atomic_state);
+            end_critical_flash_section(atomic_state);
             // TODO check return value
             return MP_OBJ_NEW_SMALL_INT(0);
         }