⚡️ RP2040 HAL Improvements (#28215)

Marlin/src/HAL/RP2040/HAL.cpp

@@ -28,6 +28,7 @@
 
 #include "../../inc/MarlinConfig.h"
 #include "../shared/Delay.h"
+#include "../../module/temperature.h"  // For OVERSAMPLENR
 
 extern "C" {
   #include "pico/bootrom.h"
@@ -41,50 +42,38 @@ extern "C" {
   #include "msc_sd.h"
 #endif
 
-// Core 1 watchdog configuration
-#define CORE1_MAX_RESETS 5  // Maximum number of Core 1 resets before halting system
-
 // ------------------------
 // Public Variables
 // ------------------------
 
 volatile uint32_t adc_accumulators[5] = {0}; // Accumulators for oversampling (sum of readings)
 volatile uint8_t adc_counts[5] = {0};        // Count of readings accumulated per channel
-volatile uint16_t adc_values[5] = {512, 512, 512, 512, 512}; // Final oversampled ADC values (averages) - initialized to mid-range
+volatile uint16_t adc_values[5] = {4095, 4095, 4095, 4095, 4095}; // Averaged ADC values (single reading equivalent) - initialized to max (open circuit)
 
-// Core 1 watchdog monitoring
+// Core monitoring for watchdog
+volatile uint32_t core0_last_heartbeat = 0; // Timestamp of Core 0's last activity
 volatile uint32_t core1_last_heartbeat = 0; // Timestamp of Core 1's last activity
-volatile bool core1_watchdog_triggered = false; // Flag to indicate Core 1 reset
-volatile uint8_t core1_reset_count = 0; // Count of Core 1 resets - halt system if >= CORE1_MAX_RESETS
+#if ENABLED(MARLIN_DEV_MODE)
+  volatile bool core1_freeze_test = false;  // Flag to freeze Core 1 for watchdog testing
+#endif
 volatile uint8_t current_pin;
 volatile bool MarlinHAL::adc_has_result;
 volatile uint8_t adc_channels_enabled[5] = {false}; // Track which ADC channels are enabled
 
-// Helper function for LED blinking patterns
-void blink_led_pattern(uint8_t blink_count, uint32_t blink_duration_us = 100000) {
-  #if DISABLED(PINS_DEBUGGING) && PIN_EXISTS(LED)
-    for (uint8_t i = 0; i < blink_count; i++) {
-      WRITE(LED_PIN, HIGH);
-      busy_wait_us(blink_duration_us);
-      WRITE(LED_PIN, LOW);
-      if (i < blink_count - 1) { // Don't delay after the last blink
-        busy_wait_us(blink_duration_us);
-      }
-    }
-  #endif
-}
-
 // Core 1 ADC reading task - dynamically reads all enabled channels with oversampling
 void core1_adc_task() {
-  static uint32_t last_led_toggle = 0;
-  const uint8_t OVERSAMPLENR = 16; // Standard Marlin oversampling count
-
-  // Signal successful Core 1 startup/restart
-  SERIAL_ECHO_MSG("Core 1 ADC task started");
+  static uint32_t last_temp_update = 0;
 
   while (true) {
-    // Update heartbeat timestamp at start of each scan cycle
-    core1_last_heartbeat = time_us_32();
+    #if ENABLED(MARLIN_DEV_MODE)
+      // Check if we should freeze for watchdog test
+      if (core1_freeze_test) {
+        // Stop updating heartbeat and spin forever
+        while (core1_freeze_test) {
+          busy_wait_us(100000); // 100ms delay
+        }
+      }
+    #endif
 
     // Scan all enabled ADC channels
     for (uint8_t channel = 0; channel < 5; channel++) {
@@ -114,11 +103,9 @@ void core1_adc_task() {
       adc_accumulators[channel] += reading;
       adc_counts[channel]++;
 
-      // Update the averaged value with current accumulation (provides immediate valid data)
-      adc_values[channel] = adc_accumulators[channel] / adc_counts[channel];
-
-      // When we reach the full oversampling count, reset accumulator for next cycle
+      // When we reach the full oversampling count, calculate averaged value (Marlin ISR does its own oversampling)
       if (adc_counts[channel] >= OVERSAMPLENR) {
+        adc_values[channel] = adc_accumulators[channel] / OVERSAMPLENR; // Return single-reading equivalent
         adc_accumulators[channel] = 0;
         adc_counts[channel] = 0;
       }
@@ -129,17 +116,19 @@ void core1_adc_task() {
       }
     }
 
-    // Core 1 LED indicator: Double blink every 2 seconds to show Core 1 is active
+    // Core 1 just provides ADC readings - don't trigger temperature updates from here
+    // Let Marlin's main temperature ISR on Core 0 handle the timing and updates
     uint32_t now = time_us_32();
-    if (now - last_led_toggle >= 2000000) { // 2 seconds
-      last_led_toggle = now;
-      #if DISABLED(PINS_DEBUGGING) && PIN_EXISTS(LED)
-        // Triple blink pattern if watchdog was triggered (shows Core 1 was reset)
-        if (core1_watchdog_triggered) {
-          core1_watchdog_triggered = false; // Clear flag
-          blink_led_pattern(3); // Triple blink for watchdog reset
-        } else {
-          blink_led_pattern(2); // Normal double blink
+    if (now - last_temp_update >= 100000) { // 100ms = 100000 microseconds
+      last_temp_update = now;
+      #if ENABLED(USE_WATCHDOG)
+        // Refresh watchdog here like AVR ISR does indirectly via temperature updates
+        // Use 2 second delay to allow watchdog_init to be called during boot
+        static uint32_t core1_start_time = 0;
+        if (core1_start_time == 0) core1_start_time = time_us_32();
+
+        if (time_us_32() - core1_start_time > 2000000) {
+          hal.watchdog_refresh(1); // Refresh from Core 1
         }
       #endif
     }
@@ -219,37 +208,42 @@ void MarlinHAL::reboot() { watchdog_reboot(0, 0, 1); }
   void MarlinHAL::watchdog_init() {
     #if DISABLED(DISABLE_WATCHDOG_INIT)
       static_assert(WDT_TIMEOUT_US > 1000, "WDT Timeout is too small, aborting");
+      // Initialize Core 0 heartbeat
+      core0_last_heartbeat = time_us_32();
       watchdog_enable(WDT_TIMEOUT_US/1000, true);
     #endif
   }
 
-  void MarlinHAL::watchdog_refresh() {
-    // If Core 1 has reset CORE1_MAX_RESETS+ times, stop updating watchdog to halt system
-    if (core1_reset_count >= CORE1_MAX_RESETS) {
-      SERIAL_ECHO_MSG("Core 1 reset limit exceeded (", core1_reset_count, " resets) - halting system for safety");
-      return; // Don't update watchdog - system will halt
+  void MarlinHAL::watchdog_refresh(const uint8_t core/*=0*/) {
+    if (core == 0) {
+      // Update Core 0 heartbeat
+      core0_last_heartbeat = time_us_32();
+
+      // Check if Core 1 is alive (2 second timeout)
+      if (time_us_32() - core1_last_heartbeat < 2000000) {
+        watchdog_update(); // Only refresh if Core 1 is responding
+        #if DISABLED(PINS_DEBUGGING) && PIN_EXISTS(LED)
+          TOGGLE(LED_PIN); // Heartbeat indicator
+        #endif
+      }
+      // If Core 1 is stuck, don't refresh - let watchdog reset the system
     }
+    else {
+      // Update Core 1 heartbeat
+      core1_last_heartbeat = time_us_32();
 
-    watchdog_update();
-
-    // Check Core 1 watchdog (15 second timeout)
-    uint32_t now = time_us_32();
-    if (now - core1_last_heartbeat > 15000000) { // 15 seconds
-      // Core 1 appears stuck - reset it
-      multicore_reset_core1();
-      multicore_launch_core1(core1_adc_task);
-      core1_watchdog_triggered = true; // Signal for LED indicator
-      core1_reset_count++; // Increment reset counter
-      SERIAL_ECHO_MSG("Core 1 ADC watchdog triggered - resetting Core 1 (attempt ", core1_reset_count, ")");
+      // Check if Core 0 is alive (2 second timeout)
+      if (time_us_32() - core0_last_heartbeat < 2000000) {
+        watchdog_update(); // Only refresh if Core 0 is responding
+        #if DISABLED(PINS_DEBUGGING) && PIN_EXISTS(LED)
+          TOGGLE(LED_PIN); // Heartbeat indicator
+        #endif
+      }
+      // If Core 0 is stuck, don't refresh - let watchdog reset the system
     }
-
-    #if DISABLED(PINS_DEBUGGING) && PIN_EXISTS(LED)
-      // Core 0 LED indicator: Single toggle every watchdog refresh (shows Core 0 activity)
-      TOGGLE(LED_PIN);
-    #endif
   }
 
-#endif
+#endif // USE_WATCHDOG
 
 // ------------------------
 // ADC
@@ -290,13 +284,15 @@ void flashFirmware(const int16_t) { hal.reboot(); }
 
 extern "C" {
   void * _sbrk(int incr);
-  extern unsigned int __bss_end__; // end of bss section
+  extern unsigned int __StackLimit;    // Lowest address the stack can grow to
 }
 
-// Return free memory between end of heap (or end bss) and whatever is current
+// Return free memory between end of heap and start of stack
 int freeMemory() {
-  int free_memory, heap_end = (int)_sbrk(0);
-  return (int)&free_memory - (heap_end ?: (int)&__bss_end__);
+  void* heap_end = _sbrk(0);
+  // Use the linker-provided stack limit instead of a local variable
+  // __StackLimit is the lowest address the stack can grow to
+  return (char*)&__StackLimit - (char*)heap_end;
 }
 
 #endif // __PLAT_RP2040__

Marlin/src/HAL/RP2040/HAL.h

@@ -51,6 +51,28 @@
 
 #include "MarlinSerial.h"
 
+#if !WITHIN(SERIAL_PORT, -1, 1)
+  #error "SERIAL_PORT must be from -1 to 1."
+#endif
+
+#ifdef SERIAL_PORT_2
+  #if !WITHIN(SERIAL_PORT_2, -1, 1)
+    #error "SERIAL_PORT_2 must be from -1 to 1."
+  #endif
+#endif
+
+#ifdef SERIAL_PORT_3
+  #if !WITHIN(SERIAL_PORT_3, -1, 1)
+    #error "SERIAL_PORT_3 must be from -1 to 1."
+  #endif
+#endif
+
+#ifdef LCD_SERIAL_PORT
+  #if !WITHIN(LCD_SERIAL_PORT, -1, 1)
+    #error "LCD_SERIAL_PORT must be from -1 to 1."
+  #endif
+#endif
+
 // ------------------------
 // Defines
 // ------------------------
@@ -131,7 +153,7 @@ public:
 
   // Watchdog
   static void watchdog_init()    IF_DISABLED(USE_WATCHDOG, {});
-  static void watchdog_refresh() IF_DISABLED(USE_WATCHDOG, {});
+  static void watchdog_refresh(const uint8_t=0) IF_DISABLED(USE_WATCHDOG, {});
 
   static void init();          // Called early in setup()
   static void init_board() {}  // Called less early in setup()

Marlin/src/HAL/RP2040/MarlinSerial.cpp

@@ -30,10 +30,40 @@
   #include "../../feature/e_parser.h"
 #endif
 
-#define _IMPLEMENT_SERIAL(X) DefaultSerial##X MSerial##X(false, Serial##X)
-#define IMPLEMENT_SERIAL(X)  _IMPLEMENT_SERIAL(X)
-#if WITHIN(SERIAL_PORT, 0, 3)
-  IMPLEMENT_SERIAL(SERIAL_PORT);
+#include <HardwareSerial.h>
+
+// Marlin uses: -1=USB, 0=UART0, 1=UART1
+// Arduino uses: Serial=USB, Serial1=UART0, Serial2=UART1
+//
+// To remap Arduino's numbering to Marlin's convention, we create MarlinSerial0/MarlinSerial1
+// as new UART instances with custom pins.
+//
+// We use distinct names (MarlinSerial0/MarlinSerial1) to avoid symbol conflicts with
+// the Arduino framework's pre-defined Serial1/Serial2 objects, which use the same
+// underlying hardware (_UART0_ and _UART1_).
+
+// Create Serial0 as UART0 with custom or default pins
+arduino::UART MarlinSerial0(
+  #if PINS_EXIST(SERIAL0_TX, SERIAL0_RX)
+    SERIAL0_TX_PIN, SERIAL0_RX_PIN  // Custom pins for UART0 (Marlin Serial0)
+  #else
+    0, 1  // Default UART0 pins (GP0/GP1)
+  #endif
+);
+
+// Not using PINS_EXIST(SERIAL1_TX, SERIAL1_RX) because SERIAL1_TX and SERIAL1_RX
+// are defined in framework-arduino-mbed/variants/RASPBERRY_PI_PICO/pins_arduino.h
+
+// Create Serial1 as UART1 with custom or default pins
+#if defined(SERIAL1_TX_PIN) && defined(SERIAL1_RX_PIN)
+  arduino::UART MarlinSerial1(SERIAL1_TX_PIN, SERIAL1_RX_PIN);  // Custom pins for UART1 (Marlin Serial1)
 #endif
 
+// Wrap the serial ports for Marlin
+DefaultSerial0 MSerial0(false, MarlinSerial0);   // Marlin Serial0 = UART0
+#if defined(SERIAL1_TX_PIN) && defined(SERIAL1_RX_PIN)
+  DefaultSerial1 MSerial1(false, MarlinSerial1); // Marlin Serial1 = UART1
+#endif
+DefaultSerial2 MSerial2(false, Serial);          // Marlin Serial2 = USB (-1)
+
 #endif // __PLAT_RP2040__

Marlin/src/HAL/RP2040/MarlinSerial.h

@@ -29,20 +29,50 @@
 
 #include "../../core/serial_hook.h"
 
-typedef ForwardSerial1Class< decltype(Serial) > DefaultSerial1;
-extern DefaultSerial1 MSerial0;
+/**
+ * Serial Port Configuration for RP2040 (Raspberry Pi Pico)
+ *
+ * Arduino-Pico Core Serial Objects:
+ * - Serial: USB Serial (CDC ACM)
+ * - Serial1: Hardware UART0
+ * - Serial2: Hardware UART1
+ * - SerialUSB: Alias for Serial (USB)
+ *
+ * Marlin Serial Wrappers:
+ * - MSerial0: Wrapper for MarlinSerial0 (UART0), used as Serial0
+ * - MSerial1: Wrapper for MarlinSerial1 (UART1), declared dynamically if used
+ * - MSerial2: Wrapper for Serial (USB)
+ * - USBSerial: Wrapper for SerialUSB (USB)
+ *
+ * How it all joins together:
+ * - Configuration defines SERIAL_PORT, SERIAL_PORT_2, etc. (-1 to 1 range)
+ * - shared/serial_ports.h maps these to MYSERIAL1, MYSERIAL2, etc.
+ * - MYSERIAL1 uses MSerialX based on the port index
+ * - USB ports (-1) use USB_SERIAL_PORT (MSerial2)
+ */
+
+// Forward declare our custom Serial objects (defined in MarlinSerial.cpp)
+namespace arduino { class UART; }
+extern arduino::UART MarlinSerial0;  // Always declared
+extern arduino::UART MarlinSerial1;  // Custom Marlin Serial1 to avoid conflict
+
+typedef ForwardSerial1Class< decltype(MarlinSerial0) > DefaultSerial0;
+extern DefaultSerial0 MSerial0;
+typedef ForwardSerial1Class< decltype(MarlinSerial1) > DefaultSerial1;
+extern DefaultSerial1 MSerial1;
+typedef ForwardSerial1Class< decltype(Serial) > DefaultSerial2;
+extern DefaultSerial2 MSerial2;
 typedef ForwardSerial1Class<decltype(SerialUSB)> USBSerialType;
 extern USBSerialType USBSerial;
 
-#define Serial0 Serial
 #define _DECLARE_SERIAL(X) \
-  typedef ForwardSerial1Class<decltype(Serial##X)> DefaultSerial##X; \
+  typedef ForwardSerial1Class<decltype(MarlinSerial##X)> DefaultSerial##X; \
   extern DefaultSerial##X MSerial##X
 #define DECLARE_SERIAL(X) _DECLARE_SERIAL(X)
 
 #define SERIAL_INDEX_MIN 0
-#define SERIAL_INDEX_MAX 6
-#define USB_SERIAL_PORT(...) MSerial0
+#define SERIAL_INDEX_MAX 1
+#define USB_SERIAL_PORT(...) MSerial2
 #include "../shared/serial_ports.h"
 
 #if defined(LCD_SERIAL_PORT) && ANY(HAS_DGUS_LCD, EXTENSIBLE_UI)

Marlin/src/gcode/gcode_d.cpp

@@ -179,17 +179,43 @@ void GcodeSuite::D(const int16_t dcode) {
       break;
 
     case 100: { // D100 Disable heaters and attempt a hard hang (Watchdog Test)
+
+      #ifdef __PLAT_RP2040__
+        const uint8_t core = parser.byteval('C', 0); // C parameter: which core to freeze (0=Core 0, 1=Core 1)
+      #else
+        constexpr uint8_t core = 0;
+      #endif
+
       SERIAL_ECHOLNPGM("Disabling heaters and attempting to trigger Watchdog");
       SERIAL_ECHOLNPGM("(USE_WATCHDOG " TERN(USE_WATCHDOG, "ENABLED", "DISABLED") ")");
+      #ifdef __PLAT_RP2040__
+        SERIAL_ECHOLNPGM("Freezing Core ", core);
+      #endif
+
       thermalManager.disable_all_heaters();
       delay(1000); // Allow time to print
-      hal.isr_off();
-      // Use a low-level delay that does not rely on interrupts to function
-      // Do not spin forever, to avoid thermal risks if heaters are enabled and
-      // watchdog does not work.
-      for (int i = 10000; i--;) DELAY_US(1000UL);
-      hal.isr_on();
+
+      if (core == 1) {
+        #ifdef __PLAT_RP2040__
+          // Freeze Core 1 by setting a flag it will check
+          extern volatile bool core1_freeze_test;
+          core1_freeze_test = true;
+          delay(10000); // Wait 10 seconds for watchdog to trigger
+          core1_freeze_test = false;
+        #endif
+      }
+      else {
+        // Freeze Core 0 (original behavior)
+        hal.isr_off();
+        // Use a low-level delay that does not rely on interrupts to function
+        // Do not spin forever, to avoid thermal risks if heaters are enabled and
+        // watchdog does not work.
+        for (int i = 10000; i--;) DELAY_US(1000UL);
+        hal.isr_on();
+      }
+
       SERIAL_ECHOLNPGM("FAILURE: Watchdog did not trigger board reset.");
+
     } break;
 
     #if HAS_MEDIA

Marlin/src/pins/rp2040/pins_BTT_SKR_Pico.h

@@ -107,51 +107,35 @@
 //
 #define NEOPIXEL_PIN                          24
 
+// Custom serial pins for RP2040 UART remapping
+#define SERIAL1_TX_PIN                         8
+#define SERIAL1_RX_PIN                         9
+
 /**
  * TMC2208/TMC2209 stepper drivers
  */
 #if HAS_TMC_UART
   /**
    * Hardware serial communication ports.
-   * If undefined software serial is used according to the pins below
    */
-  //#define X_HARDWARE_SERIAL  Serial1
-  //#define X2_HARDWARE_SERIAL Serial1
-  //#define Y_HARDWARE_SERIAL  Serial1
-  //#define Y2_HARDWARE_SERIAL Serial1
-  //#define Z_HARDWARE_SERIAL MSerial1
-  //#define Z2_HARDWARE_SERIAL Serial1
-  //#define E0_HARDWARE_SERIAL Serial1
-  //#define E1_HARDWARE_SERIAL Serial1
-  //#define E2_HARDWARE_SERIAL Serial1
-  //#define E3_HARDWARE_SERIAL Serial1
-  //#define E4_HARDWARE_SERIAL Serial1
+  #define X_HARDWARE_SERIAL  MarlinSerial1
+  #define Y_HARDWARE_SERIAL  MarlinSerial1
+  #define Z_HARDWARE_SERIAL  MarlinSerial1
+  #define E0_HARDWARE_SERIAL MarlinSerial1
 
-  /**
-   * Software serial
-   */
-  #ifndef X_SERIAL_TX_PIN
-    #define X_SERIAL_TX_PIN                    8
+  // Default TMC slave addresses
+  #ifndef X_SLAVE_ADDRESS
+    #define X_SLAVE_ADDRESS                     0
   #endif
-  #ifndef X_SERIAL_RX_PIN
-    #define X_SERIAL_RX_PIN                    9
+  #ifndef Y_SLAVE_ADDRESS
+    #define Y_SLAVE_ADDRESS                     2
   #endif
-  #ifndef Y_SERIAL_TX_PIN
-    #define Y_SERIAL_TX_PIN                    8
+  #ifndef Z_SLAVE_ADDRESS
+    #define Z_SLAVE_ADDRESS                     1
   #endif
-  #ifndef Y_SERIAL_RX_PIN
-    #define Y_SERIAL_RX_PIN                    9
-  #endif
-  #ifndef Z_SERIAL_TX_PIN
-    #define Z_SERIAL_TX_PIN                    8
-  #endif
-  #ifndef Z_SERIAL_RX_PIN
-    #define Z_SERIAL_RX_PIN                    9
-  #endif
-  #ifndef E0_SERIAL_TX_PIN
-    #define E0_SERIAL_TX_PIN                   8
-  #endif
-  #ifndef E0_SERIAL_RX_PIN
-    #define E0_SERIAL_RX_PIN                   9
+  #ifndef E0_SLAVE_ADDRESS
+    #define E0_SLAVE_ADDRESS                    3
   #endif
+
+  #define TMC_BAUD_RATE                115200
 #endif

ini/raspberrypi.ini

@@ -20,8 +20,7 @@ lib_deps         = ${common.lib_deps}
                    #lvgl/lvgl@^8.1.0
 lib_ignore       = WiFi
 build_flags      = ${common.build_flags} -D__PLAT_RP2040__ -DPLATFORM_M997_SUPPORT -DNO_SD_HOST_DRIVE -Wno-expansion-to-defined -Wno-vla -Wno-ignored-qualifiers
-#debug_tool      = jlink
-#upload_protocol = jlink
+upload_protocol  = picotool
 #custom_marlin.HAS_SD_HOST_DRIVE = tinyusb
 
 [env:RP2040-alt]
@@ -34,6 +33,5 @@ board_build.core = earlephilhower
 #
 [env:SKR_Pico]
 extends           = env:RP2040
-
-[env:SKR_Pico_UART]
-extends           = env:SKR_Pico
+lib_deps          = ${common.lib_deps}
+                    arduino-libraries/Servo