Fix loading of PICO_TOOLCHAIN_PATH (#262)

- Add double quotes because build option it's a string.
- Remove comment as requested by @kilograham.
- Resolves #258.

cmake/pico_pre_load_toolchain.cmake

@@ -1,6 +1,5 @@
 # PICO_CMAKE_CONFIG: PICO_TOOLCHAIN_PATH, Path to search for compiler, default=none (i.e. search system paths), group=build
-# Set your compiler path here if it's not in the PATH environment variable.
+set(PICO_TOOLCHAIN_PATH "${PICO_TOOLCHAIN_PATH}" CACHE INTERNAL "")
-set(PICO_TOOLCHAIN_PATH "" CACHE INTERNAL "")
 
 # Set a default build type if none was specified
 set(default_build_type "Release")

cmake/preload/toolchains/pico_arm_gcc.cmake

@@ -52,8 +52,9 @@ set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
 option(PICO_DEOPTIMIZED_DEBUG "Build debug builds with -O0" 0)
 
 # todo move to platform/Generix-xxx
-set(ARM_GCC_COMMON_FLAGS " -march=armv6-m -mcpu=cortex-m0plus -mthumb")
+
-#set(ARM_GCC_COMMON_FLAGS " -mcpu=cortex-m0plus -mthumb")
+# on ARM -mcpu should not be mixed with -march
+set(ARM_GCC_COMMON_FLAGS " -mcpu=cortex-m0plus -mthumb")
 foreach(LANG IN ITEMS C CXX ASM)
     set(CMAKE_${LANG}_FLAGS_INIT "${ARM_GCC_COMMON_FLAGS}")
     if (PICO_DEOPTIMIZED_DEBUG)

pico_sdk_version.cmake

@@ -6,7 +6,15 @@ set(PICO_SDK_VERSION_MAJOR 1)
 set(PICO_SDK_VERSION_MINOR 1)
 # PICO_BUILD_DEFINE: PICO_SDK_VERSION_REVISION, SDK version revision, type=int, pico_base
 # PICO_CONFIG: PICO_SDK_VERSION_REVISION, SDK version revision, type=int, pico_base
-set(PICO_SDK_VERSION_REVISION 0)
+set(PICO_SDK_VERSION_REVISION 1)
+# PICO_BUILD_DEFINE: PICO_SDK_VERSION_PRE_RELEASE_ID, optional SDK pre-release version identifier, type=string, pico_base
+# PICO_CONFIG: PICO_SDK_VERSION_PRE_RELEASE_ID, optional SDK pre-release version identifier, type=string, pico_base
+set(PICO_SDK_VERSION_PRE_RELEASE_ID develop)
+
 # PICO_BUILD_DEFINE: PICO_SDK_VERSION_STRING, SDK version, type=string, group=pico_base
 # PICO_CONFIG: PICO_SDK_VERSION_STRING, SDK version, type=string, group=pico_base
 set(PICO_SDK_VERSION_STRING "${PICO_SDK_VERSION_MAJOR}.${PICO_SDK_VERSION_MINOR}.${PICO_SDK_VERSION_REVISION}")
+
+if (PICO_SDK_VERSION_PRE_RELEASE_ID)
+    set(PICO_SDK_VERSION_STRING "${PICO_SDK_VERSION_STRING}-${PICO_SDK_VERSION_PRE_RELEASE_ID}")
+endif()

src/boards/include/boards/sparkfun_micromod.h

@@ -19,7 +19,7 @@
 
 #ifndef PICO_DEFAULT_UART
 #define PICO_DEFAULT_UART 0
-#define
+#endif
 
 #ifndef PICO_DEFAULT_UART_TX_PIN
 #define PICO_DEFAULT_UART_TX_PIN 0

src/common/pico_base/include/pico/types.h

@@ -25,7 +25,7 @@ typedef unsigned int uint;
     \see update_us_since_boot()
     \ingroup timestamp
 */
-#ifndef NDEBUG
+#ifdef NDEBUG
 typedef uint64_t absolute_time_t;
 #else
 typedef struct {
@@ -40,7 +40,7 @@ typedef struct {
  * \ingroup timestamp
  */
 static inline uint64_t to_us_since_boot(absolute_time_t t) {
-#ifndef NDEBUG
+#ifdef NDEBUG
     return t;
 #else
     return t._private_us_since_boot;
@@ -55,7 +55,7 @@ static inline uint64_t to_us_since_boot(absolute_time_t t) {
  * \ingroup timestamp
  */
 static inline void update_us_since_boot(absolute_time_t *t, uint64_t us_since_boot) {
-#ifndef NDEBUG
+#ifdef NDEBUG
     *t = us_since_boot;
 #else
     assert(us_since_boot <= INT64_MAX);
@@ -63,7 +63,7 @@ static inline void update_us_since_boot(absolute_time_t *t, uint64_t us_since_bo
 #endif
 }
 
-#ifndef NDEBUG
+#ifdef NDEBUG
 #define ABSOLUTE_TIME_INITIALIZED_VAR(name, value) name = value
 #else
 #define ABSOLUTE_TIME_INITIALIZED_VAR(name, value) name = {value}

src/common/pico_util/include/pico/util/queue.h

@@ -18,6 +18,10 @@
  * \ingroup pico_util
  */
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 typedef struct {
     spin_lock_t *lock;
     uint8_t *data;
@@ -69,7 +73,7 @@ void queue_free(queue_t *q);
 static inline uint queue_get_level_unsafe(queue_t *q) {
     int32_t rc = (int32_t)q->wptr - (int32_t)q->rptr;
     if (rc < 0) {
-        rc += + q->element_count + 1;
+        rc += q->element_count + 1;
     }
     return (uint)rc;
 }
@@ -181,4 +185,7 @@ void queue_remove_blocking(queue_t *q, void *data);
  */
 void queue_peek_blocking(queue_t *q, void *data);
 
+#ifdef __cplusplus
+}
+#endif
 #endif

src/rp2_common/hardware_timer/timer.c

@@ -166,8 +166,9 @@ bool hardware_alarm_set_target(uint alarm_num, absolute_time_t target) {
         // 1) actually set the hardware timer
         spin_lock_t *lock = spin_lock_instance(PICO_SPINLOCK_ID_TIMER);
         uint32_t save = spin_lock_blocking(lock);
-        timer_hw->intr = 1u << alarm_num;
+        uint8_t old_timer_callbacks_pending = timer_callbacks_pending;
         timer_callbacks_pending |= (uint8_t)(1u << alarm_num);
+        timer_hw->intr = 1u << alarm_num; // clear any IRQ
         timer_hw->alarm[alarm_num] = (uint32_t) t;
         // Set the alarm. Writing time should arm it
         target_hi[alarm_num] = (uint32_t)(t >> 32u);
@@ -178,18 +179,26 @@ bool hardware_alarm_set_target(uint alarm_num, absolute_time_t target) {
             assert(timer_hw->ints & 1u << alarm_num);
         } else {
             if (time_us_64() >= t) {
-                // ok well it is time now; the irq isn't being handled yet because of the spin lock
+                // we are already at or past the right time; there is no point in us racing against the IRQ
-                // however the other core might be in the IRQ handler itself about to do a callback
+                // we are about to generate. note however that, if there was already a timer pending before,
-                // we do the firing ourselves (and indicate to the IRQ handler if any that it shouldn't
+                // then we still let the IRQ fire, as whatever it was, is not handled by our setting missed=true here
                 missed = true;
-                // disarm the timer
+                if (timer_callbacks_pending != old_timer_callbacks_pending) {
-                timer_hw->armed = 1u << alarm_num;
+                    // disarm the timer
-                timer_hw->intr = 1u << alarm_num; // clear the IRQ too
+                    timer_hw->armed = 1u << alarm_num;
-                // and set flag in case we're already in the IRQ handler waiting on the spinlock (on the other core)
+                    // clear the IRQ...
-                timer_callbacks_pending &= (uint8_t)~(1u << alarm_num);
+                    timer_hw->intr = 1u << alarm_num;
+                    // ... including anything pending on the processor - perhaps unnecessary, but
+                    // our timer flag says we aren't expecting anything.
+                    irq_clear(harware_alarm_irq_number(alarm_num));
+                    // and clear our flag so that if the IRQ handler is already active (because it is on
+                    // the other core) it will also skip doing anything
+                    timer_callbacks_pending = old_timer_callbacks_pending;
+                }
             }
         }
         spin_unlock(lock, save);
+        // note at this point any pending timer IRQ can likely run
     }
     return missed;
 }

src/rp2_common/pico_stdio_usb/reset_interface.c

@@ -60,7 +60,7 @@ static bool resetd_control_request_cb(uint8_t __unused rhport, tusb_control_requ
 
 #if PICO_STDIO_USB_RESET_INTERFACE_SUPPORT_RESET_TO_FLASH_BOOT
         if (request->bRequest == RESET_REQUEST_FLASH) {
-            watchdog_reboot(0, SRAM_END, PICO_STDIO_USB_RESET_RESET_TO_FLASH_DELAY_MS);
+            watchdog_reboot(0, 0, PICO_STDIO_USB_RESET_RESET_TO_FLASH_DELAY_MS);
             return true;
         }
 #endif

src/rp2_common/pico_stdio_usb/stdio_usb.c

@@ -10,12 +10,13 @@
 #include "pico/time.h"
 #include "pico/stdio/driver.h"
 #include "pico/binary_info.h"
+#include "pico/mutex.h"
 #include "hardware/irq.h"
 
 static_assert(PICO_STDIO_USB_LOW_PRIORITY_IRQ > RTC_IRQ, ""); // note RTC_IRQ is currently the last one
 static mutex_t stdio_usb_mutex;
 
-static void low_priority_worker_irq() {
+static void low_priority_worker_irq(void) {
     // if the mutex is already owned, then we are in user code
     // in this file which will do a tud_task itself, so we'll just do nothing
     // until the next tick; we won't starve

test/kitchen_sink/kitchen_sink.c

@@ -83,6 +83,7 @@ int main(void) {
     dma_channel_configure(0, &config, &dma_to, &dma_from, 1, true);
     dma_channel_set_config(0, &config, false);
 
+    // note this loop expects to cause a breakpoint!!
     for (int i = 0; i < 20; i++) {
         puts("sleepy");
         sleep_ms(1000);
@@ -94,4 +95,6 @@ int main(void) {
             irq_remove_handler(DMA_IRQ_1, dma_handler_b);
         }
     }
+    // this should compile as we are Cortex M0+
+    __asm volatile("SVC #3");
 }

test/pico_time_test/pico_time_test.c

@@ -64,12 +64,13 @@ static bool repeating_timer_callback(struct repeating_timer *t) {
 #define RESOLUTION_ALLOWANCE PICO_HARDWARE_TIMER_RESOLUTION_US
 #endif
 
+int issue_195_test(void);
+
 int main() {
     setup_default_uart();
     alarm_pool_init_default();
 
     PICOTEST_START();
-
     struct alarm_pool *pools[NUM_TIMERS];
     for(uint i=0; i<NUM_TIMERS; i++) {
         if (i == alarm_pool_hardware_alarm_num(alarm_pool_get_default())) {
@@ -215,6 +216,35 @@ int main() {
     PICOTEST_CHECK(absolute_time_diff_us(near_the_end_of_time, at_the_end_of_time) > 0, "near the end of time should be before the end of time")
     PICOTEST_END_SECTION();
 
+    if (issue_195_test()) {
+        return -1;
+    }
+
     PICOTEST_END_TEST();
 }
 
+#define ISSUE_195_TIMER_DELAY 50
+volatile int issue_195_counter;
+int64_t issue_195_callback(alarm_id_t id, void *user_data) {
+    issue_195_counter++;
+    return -ISSUE_195_TIMER_DELAY;
+}
+
+int issue_195_test(void) {
+    PICOTEST_START_SECTION("Issue #195 race condition - without fix may hang on gcc 10.2.1 release builds");
+    absolute_time_t t1 = get_absolute_time();
+    int id = add_alarm_in_us(ISSUE_195_TIMER_DELAY, issue_195_callback, NULL, true);
+    for(uint i=0;i<5000;i++) {
+        sleep_us(100);
+        sleep_us(100);
+        uint delay = 9; // 9 seems to be the magic number (at least for reproducing on 10.2.1)
+        sleep_us(delay);
+    }
+    absolute_time_t t2 = get_absolute_time();
+    cancel_alarm(id);
+    int expected_count = absolute_time_diff_us(t1, t2) / ISSUE_195_TIMER_DELAY;
+    printf("Timer fires approx_expected=%d actual=%d\n", expected_count, issue_195_counter);
+    PICOTEST_END_SECTION();
+    return 0;
+}
+