Fix memory leak in tone/noTone calls (#125)

Was never deleting the tone object on noTone, so eventually the leaked
objects would fill up heap and cause a crash.

Fixes #121

README.md

@@ -45,7 +45,7 @@ cd ~/Arduino/hardware/pico/rp2040
 git submodule update --init
 cd pico-sdk
 git submodule update --init
-cd pico-extras
+cd ../pico-extras
 git submodule update --init
 cd ../tools
 python3 ./get.py
@@ -98,6 +98,7 @@ The installed tools include a version of OpenOCD (in the pqt-openocd directory)
 
 # Status of Port
 Relatively stable and very functional, but bug reports and PRs always accepted.
+* Multicore support (setup1() and loop1())
 * digitalWrite/Read
 * shiftIn/Out
 * SPI master
@@ -136,7 +137,7 @@ If you want to contribute or have bugfixes, drop me a note at <earlephilhower@ya
 * The [Pico-SDK](https://github.com/raspberrypi/pico-sdk) and [Pico-Extras](https://github.com/raspberrypi/pico-extras) are by Raspberry Pi (Trading) Ltd and licensed under the BSD 3-Clause license.
 * [Arduino-Pico](https://github.com/earlephilhower/arduino-pico) core files are licenses under the LGPL.
 * [LittleFS](https://github.com/ARMmbed/littlefs) library written by ARM Limited and released under the [BSD 3-clause license](https://github.com/ARMmbed/littlefs/blob/master/LICENSE.md).
-* [UF2CONV.PY](https://github.com/microsoft/uf2) is by Microsoft Corporatio and licensed under the MIT license.
+* [UF2CONV.PY](https://github.com/microsoft/uf2) is by Microsoft Corporation and licensed under the MIT license.
 * Some filesystem code taken from the [ESP8266 Arduino Core](https://github.com/esp8266/Arduino) and licensed under the LGPL.
 
 -Earle F. Philhower, III

cores/rp2040/Arduino.h

@@ -68,6 +68,9 @@ float analogReadTemp();  // Returns core temp in Centigrade
 
 // PWM
 void analogWrite(pin_size_t pinNumber, int value);
+void analogWriteFreq(uint32_t freq);
+void analogWriteRange(uint32_t range);
+void analogWriteResolution(int res);
 
 // Timing
 void delay(unsigned long);

cores/rp2040/Bootsel.cpp

@@ -26,7 +26,8 @@ static bool __no_inline_not_in_flash_func(get_bootsel_button)() {
 
     // Must disable interrupts, as interrupt handlers may be in flash, and we
     // are about to temporarily disable flash access!
-    uint32_t flags = save_and_disable_interrupts();
+    noInterrupts();
+    rp2040.idleOtherCore();
 
     // Set chip select to Hi-Z
     hw_write_masked(&ioqspi_hw->io[CS_PIN_INDEX].ctrl,
@@ -46,7 +47,8 @@ static bool __no_inline_not_in_flash_func(get_bootsel_button)() {
                     GPIO_OVERRIDE_NORMAL << IO_QSPI_GPIO_QSPI_SS_CTRL_OEOVER_LSB,
                     IO_QSPI_GPIO_QSPI_SS_CTRL_OEOVER_BITS);
 
-    restore_interrupts(flags);
+    interrupts();
+    rp2040.resumeOtherCore();
 
     return button_state;
 }

cores/rp2040/HardwareSerial.h

@@ -0,0 +1 @@
+#include "api/HardwareSerial.h"

cores/rp2040/RP2040.h

@@ -18,8 +18,97 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
-#include <hardware/pio.h>
 #include <hardware/clocks.h>
+#include <hardware/irq.h>
+#include <hardware/pio.h>
+#include <pico/multicore.h>
+#include <pico/util/queue.h>
+#include <CoreMutex.h>
+
+class _MFIFO {
+public:
+    _MFIFO() { /* noop */ };
+    ~_MFIFO() { /* noop */ };
+
+    void begin(int cores) {
+        constexpr int FIFOCNT = 8;
+        if (cores == 1) {
+            _multicore = false;
+            return;
+        }
+        mutex_init(&_idleMutex);
+        queue_init(&_queue[0], sizeof(uint32_t), FIFOCNT);
+        queue_init(&_queue[1], sizeof(uint32_t), FIFOCNT);
+        _multicore = true;
+    }
+
+    void registerCore() {
+        multicore_fifo_clear_irq();
+        irq_set_exclusive_handler(SIO_IRQ_PROC0 + get_core_num(), _irq);
+        irq_set_enabled(SIO_IRQ_PROC0 + get_core_num(), true);
+    }
+
+    void push(uint32_t val) {
+        while (!push_nb(val)) { /* noop */ }
+    }
+
+    bool push_nb(uint32_t val) {
+        // Push to the other FIFO
+        return queue_try_add(&_queue[get_core_num()^1], &val);
+    }
+
+    uint32_t pop() {
+        uint32_t ret;
+        while (!pop_nb(&ret)) { /* noop */ }
+        return ret;
+    }
+
+    bool pop_nb(uint32_t *val) {
+        // Pop from my own FIFO
+        return queue_try_remove(&_queue[get_core_num()], val);
+    }
+
+    int available() {
+        return queue_get_level(&_queue[get_core_num()]);
+    }
+
+    void idleOtherCore() {
+        if (!_multicore) return;
+        mutex_enter_blocking(&_idleMutex);
+        _otherIdled = false;
+        multicore_fifo_push_blocking(_GOTOSLEEP);
+        while (!_otherIdled) { /* noop */ }
+    }
+
+    void resumeOtherCore() {
+        if (!_multicore) return;
+        mutex_exit(&_idleMutex);
+        _otherIdled = false;
+        // Other core will exit busy-loop and return to operation
+        // once otherIdled == false.
+    }
+
+private:
+    static void __no_inline_not_in_flash_func(_irq)() {
+        multicore_fifo_clear_irq();
+        noInterrupts(); // We need total control, can't run anything
+        while (multicore_fifo_rvalid()) {
+            if (_GOTOSLEEP == multicore_fifo_pop_blocking()) {
+                _otherIdled = true;
+                while (_otherIdled) { /* noop */ }
+                break;
+            }
+        }
+        interrupts();
+    }
+    bool _multicore = false;
+
+    mutex_t _idleMutex;
+    static volatile bool _otherIdled;
+    queue_t _queue[2];
+
+    static constexpr int _GOTOSLEEP = 0x66666666;
+};
 
 class RP2040 {
 public:
@@ -33,10 +122,17 @@ public:
     static int f_cpu() {
         return clock_get_hz(clk_sys);
     }
+
+    void idleOtherCore() { fifo.idleOtherCore(); }
+    void resumeOtherCore() { fifo.resumeOtherCore(); }
+
+    // Multicore comms FIFO
+    _MFIFO fifo;
 };
 
+extern RP2040 rp2040;
+
 // Wrapper class for PIO programs, abstracting common operations out
-// TODO - Make dualcore safe
 // TODO - Add unload/destructor
 class PIOProgram {
 public:
@@ -44,6 +140,8 @@ public:
 
     // Possibly load into a PIO and allocate a SM
     bool prepare(PIO *pio, int *sm, int *offset) {
+        extern mutex_t _pioMutex;
+        CoreMutex m(&_pioMutex);
         // Is there an open slot to run in, first?
         if (!_findFreeSM(pio, sm)) return false;
 	// Is it loaded on that PIO?

cores/rp2040/Tone.cpp

@@ -95,6 +95,7 @@ void noTone(uint8_t pin) {
     if (entry != _toneMap.end()) {
         pio_sm_set_enabled(entry->second->pio, entry->second->sm, false);
 	pio_sm_unclaim(entry->second->pio, entry->second->sm);
+        delete entry->second;
         _toneMap.erase(entry);
         pinMode(pin, OUTPUT);
         digitalWrite(pin, LOW);

cores/rp2040/main.cpp

@@ -20,6 +20,13 @@
 
 #include <Arduino.h>
 #include <pico/stdlib.h>
+#include <pico/multicore.h>
+
+RP2040 rp2040;
+
+volatile bool _MFIFO::_otherIdled = false;
+mutex_t _pioMutex;
+
 
 extern void setup();
 extern void loop();
@@ -29,11 +36,28 @@ extern void loop();
 void initVariant() __attribute__((weak));
 void initVariant() { }
 
+
+// Optional 2nd core setup and loop
+extern void setup1() __attribute__((weak));
+extern void loop1() __attribute__((weak));
+static void main1() {
+    rp2040.fifo.registerCore();
+    if (setup1) {
+        setup1();
+    }
+    while (true) {
+        if (loop1) {
+            loop1();
+        }
+    }
+}
+
 extern "C" int main() {
 #if F_CPU != 125000000
     set_sys_clock_khz(F_CPU / 1000, true);
 #endif
 
+    mutex_init(&_pioMutex);
     initVariant();
 
 #ifndef DISABLE_USB_SERIAL
@@ -45,8 +69,16 @@ extern "C" int main() {
     DEBUG_RP2040_PORT.begin();
 #endif
 
+    if (setup1 || loop1) {
+        rp2040.fifo.begin(2);
+        multicore_launch_core1(main1);
+    } else {
+        rp2040.fifo.begin(1);
+    }
+    rp2040.fifo.registerCore();
+
     setup();
-    while (1) {
+    while (true) {
         loop();
         if (arduino::serialEventRun) {
             arduino::serialEventRun();

cores/rp2040/wiring_private.cpp

@@ -33,8 +33,9 @@ extern "C" void interrupts() {
         // ERROR
         return;
     }
-    restore_interrupts(_irqStack[get_core_num()].top());
+    auto oldIrqs = _irqStack[get_core_num()].top();
     _irqStack[get_core_num()].pop();
+    restore_interrupts(oldIrqs);
 }
 
 extern "C" void noInterrupts() {

docs/index.rst

@@ -34,6 +34,8 @@ For the latest version, always check https://github.com/earlephilhower/arduino-p
 
    Ported/Optimized Libraries <libraries>
 
+   Multicore Processing <multicore>
+
    Using Pico-SDK <sdk>
 
    Licenses <license>

docs/multicore.rst

@@ -0,0 +1,77 @@
+Multicore Processing
+====================
+
+The RP2040 chip has 2 cores that can run independently of each other, sharing
+peripherals and memory with each other.  Arduino code will normally execute
+only on core 0, with the 2nd core sitting idle in a low power state.
+
+By adding a ``setup1()`` and ``loop1()`` function to your sketch you can make
+use of the second core.  Anything called from within the ``setup1()`` or
+``loop1()`` routines will execute on the second core.
+
+``setup()`` and ``setup1()`` will be called at the same time, and the ``loop()``
+or ``loop1()`` will be started as soon as the core's ``setup()`` completes (i.e.
+not necessarily simultaneously!).
+
+See the ``Multicore.ino`` example in the ``rp2040`` example directory for a
+quick introduction.
+
+Pausing Cores
+-------------
+
+Sometimes an application needs to pause the other core on chip (i.e. it is
+writing to flash or needs to stop processing while some other event occurs).
+
+void rp2040.idleOtherCore()
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sends a message to stop the other core (i.e. when called from core 0 it
+pauses core 1, and vice versa).  Waits for the other core to acknowledge
+before returning.
+
+The other core will have its interrupts disabled and be busy-waiting in
+an RAM-based routine, so flash and other peripherals can be accessed.
+
+**NOTE** If you idle core 0 too long, then the USB port can become frozen.
+This is because core 0 manages the USB and needs to service IRQs in a
+timely manner (which it can't do when idled).
+
+void rp2040.resumeOtherCore()
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Resumes processing in the other core, where it left off.
+
+Communicating Between Cores
+---------------------------
+
+The RP2040 provides a hardware FIFO for communicating between cores, but it
+is used exclusively for the idle/resume calls described above.  Instead, please
+use the following functions to access a software-managed, multicore safe
+FIFO.
+
+void rp2040.fifo.push(uint32_t)
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Pushes a value to the other core.  Will block if the FIFO is full.
+
+bool rp2040.fifo.push_nb(uint32_t)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Pushes a value to the other core.  If the FIFO is full, returns ``false``
+immediately and doesn't block.  If the push is successful, returns ``true``.
+
+uint32_t rp2040.fifo.pop()
+~~~~~~~~~~~~~~~~~~~~~
+
+Reads a value from this core's FIFO.  Blocks until one is available.
+
+bool rp2040.fifo.pop_nb(uint32_t *dest)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Reads a value from this core's FIFO and places it in dest.  Will return
+``true`` if successful, or ``false`` if the pop would block.
+
+int rp2040.fifo.available()
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Returns the number of values available in this core's FIFO.

keywords.txt

@@ -0,0 +1,20 @@
+#######################################
+# Syntax Coloring Map
+#######################################
+
+#######################################
+# Datatypes (KEYWORD1)
+#######################################
+
+#######################################
+# Methods and Functions (KEYWORD2)
+#######################################
+setup1  KEYWORD2
+loop2   KEYWORD2
+analogWriteFreq	KEYWORD2
+analogWriteRange	KEYWORD2
+analogWriteResolution KEYWORD2
+
+######################################
+# Constants (LITERAL1)
+#######################################

libraries/EEPROM/EEPROM.cpp

@@ -105,11 +105,12 @@ bool EEPROMClass::commit() {
     if (!_data)
         return false;
 
-    // TODO - this only stops IRQs on 1 core, need to do it on both
-    uint32_t save = save_and_disable_interrupts();
+    noInterrupts();
+    rp2040.idleOtherCore();
     flash_range_erase((intptr_t)_sector - (intptr_t)XIP_BASE, 4096);
     flash_range_program((intptr_t)_sector - (intptr_t)XIP_BASE, _data, _size);
-    restore_interrupts(save);
+    rp2040.resumeOtherCore();
+    interrupts();
 
     return true;
 }

libraries/LittleFS/src/LittleFS.cpp

@@ -173,10 +173,12 @@ int LittleFSImpl::lfs_flash_prog(const struct lfs_config *c,
     lfs_block_t block, lfs_off_t off, const void *buffer, lfs_size_t size) {
     LittleFSImpl *me = reinterpret_cast<LittleFSImpl*>(c->context);
     uint8_t *addr = me->_start + (block * me->_blockSize) + off;
-    uint32_t save = save_and_disable_interrupts();
+    noInterrupts();
+    rp2040.idleOtherCore();
 //    Serial.printf("WRITE: %p, $d\n", (intptr_t)addr - (intptr_t)XIP_BASE, size);
     flash_range_program((intptr_t)addr - (intptr_t)XIP_BASE, (const uint8_t *)buffer, size);
-    restore_interrupts(save);
+    rp2040.resumeOtherCore();
+    interrupts();
     return 0;
 }
 
@@ -184,9 +186,11 @@ int LittleFSImpl::lfs_flash_erase(const struct lfs_config *c, lfs_block_t block)
     LittleFSImpl *me = reinterpret_cast<LittleFSImpl*>(c->context);
     uint8_t *addr = me->_start + (block * me->_blockSize);
 //    Serial.printf("ERASE: %p, %d\n", (intptr_t)addr - (intptr_t)XIP_BASE, me->_blockSize);
-    uint32_t save = save_and_disable_interrupts();
+    noInterrupts();
+    rp2040.idleOtherCore();
     flash_range_erase((intptr_t)addr - (intptr_t)XIP_BASE, me->_blockSize);
-    restore_interrupts(save);
+    rp2040.resumeOtherCore();
+    interrupts();
     return 0;
 }
 

libraries/rp2040/examples/Multicore/Multicore.ino

@@ -0,0 +1,40 @@
+// Demonstrates a simple use of the setup1()/loop1() functions
+// for a multiprocessor run.
+
+// Will output something like, where C0 is running on core 0 and
+// C1 is on core 1, in parallel.
+
+// 11:23:07.507 -> C0: Blue leader standing by...
+// 11:23:07.507 -> C1: Red leader standing by...
+// 11:23:07.507 -> C1: Stay on target...
+// 11:23:08.008 -> C1: Stay on target...
+// 11:23:08.505 -> C0: Blue leader standing by...
+// 11:23:08.505 -> C1: Stay on target...
+// 11:23:09.007 -> C1: Stay on target...
+// 11:23:09.511 -> C0: Blue leader standing by...
+// 11:23:09.511 -> C1: Stay on target...
+// 11:23:10.015 -> C1: Stay on target...
+
+// Released to the public domain
+
+// The normal, core0 setup
+void setup() {
+  Serial.begin();
+  delay(5000);
+}
+
+void loop() {
+  Serial.printf("C0: Blue leader standing by...\n");
+  delay(1000);
+}
+
+// Running on core1
+void setup1() {
+  delay(5000);
+  Serial.printf("C1: Red leader standing by...\n");
+}
+
+void loop1() {
+  Serial.printf("C1: Stay on target...\n");
+  delay(500);
+}

pico_base/pico/version.h

@@ -13,7 +13,7 @@
 
 #define PICO_SDK_VERSION_MAJOR    1
 #define PICO_SDK_VERSION_MINOR    1
-#define PICO_SDK_VERSION_REVISION 0
-#define PICO_SDK_VERSION_STRING   "1.1.0"
+#define PICO_SDK_VERSION_REVISION 2
+#define PICO_SDK_VERSION_STRING   "1.1.2"
 
 #endif

tools/uf2conv.py

@@ -236,6 +236,10 @@ def get_drives():
                 drives.append(words[0])
     else:
         rootpath = "/media"
+        if not os.path.isdir(rootpath):
+            rootpath = "/run/media"
+        if not os.path.isdir(rootpath):
+            rootpath = "/opt/media"
         if sys.platform == "darwin":
             rootpath = "/Volumes"
         elif sys.platform == "linux":