RP2350: fix USB CDC stability (competing tud_task() timers)

cmake/rp2350.cmake

@@ -238,6 +238,11 @@ set(RP2350_DEFINITIONS
     PICO_CONFIG_HEADER=pico_sdk_config.h
     PICO_NO_FPGA_CHECK=1
     PICO_FLASH_SIZE_BYTES=4194304
+    # Tell pico_stdio_usb that TinyUSB device mode is already linked by the application.
+    # Without this, stdio_usb_init() would call tusb_init() a second time AND set up its
+    # own low-priority IRQ + alarm to call tud_task(), racing with INAV's own 1ms timer.
+    # The race corrupts TinyUSB internal state, causing USB CDC to drop after a few seconds.
+    LIB_TINYUSB_DEVICE=1
     ${CORTEX_M33_DEFINITIONS}
 )
 

src/main/drivers/serial_uart_rp2350.c

@@ -205,8 +205,8 @@ uartPort_t *serialUART1(uint32_t baudRate, portMode_t mode, portOptions_t option
     s->port.port.txBufferTail = 0;
 
     uart_init(uart0, baudRate);
-    if (mode & MODE_TX) { gpio_set_function(s->tx_pin, GPIO_FUNC_UART); }
-    if (mode & MODE_RX) { gpio_set_function(s->rx_pin, GPIO_FUNC_UART); }
+    if (mode & MODE_TX) { gpio_set_function(s->tx_pin, UART_FUNCSEL_NUM(uart0, s->tx_pin)); }
+    if (mode & MODE_RX) { gpio_set_function(s->rx_pin, UART_FUNCSEL_NUM(uart0, s->rx_pin)); }
 
     rp2350UartHwConfigure(s);
 
@@ -254,8 +254,8 @@ uartPort_t *serialUART2(uint32_t baudRate, portMode_t mode, portOptions_t option
     s->port.port.txBufferTail = 0;
 
     uart_init(uart1, baudRate);
-    if (mode & MODE_TX) { gpio_set_function(s->tx_pin, GPIO_FUNC_UART); }
-    if (mode & MODE_RX) { gpio_set_function(s->rx_pin, GPIO_FUNC_UART); }
+    if (mode & MODE_TX) { gpio_set_function(s->tx_pin, UART_FUNCSEL_NUM(uart1, s->tx_pin)); }
+    if (mode & MODE_RX) { gpio_set_function(s->rx_pin, UART_FUNCSEL_NUM(uart1, s->rx_pin)); }
 
     rp2350UartHwConfigure(s);
 

src/main/drivers/serial_usb_vcp_rp2350.c

@@ -23,11 +23,19 @@
  */
 
 /*
- * RP2350 USB VCP serial driver for INAV — Milestone 4
+ * RP2350 USB VCP serial driver for INAV
  *
  * Implements INAV's serialPort_t vtable using TinyUSB CDC class.
  * TinyUSB is initialized in systemInit() via tusb_init(); background USB
- * event processing is driven by pico_stdio_usb's 1ms repeating alarm.
+ * event processing is driven by a 1ms repeating hardware alarm in systemInit().
+ *
+ * This file also provides the three mandatory TinyUSB descriptor callbacks
+ * (tud_descriptor_device_cb, tud_descriptor_configuration_cb,
+ * tud_descriptor_string_cb).  They were previously supplied by pico_stdio_usb's
+ * stdio_usb_descriptors.c, but that file is guarded by
+ * "#if !defined(LIB_TINYUSB_DEVICE)" and is therefore inactive now that we
+ * define LIB_TINYUSB_DEVICE=1 to prevent pico_stdio_usb from spawning a
+ * competing tud_task() timer.
  */
 
 #include <stdint.h>
@@ -86,7 +94,7 @@ static uint8_t usbVcpRead(serialPort_t *instance)
 {
     UNUSED(instance);
     // Callers must check serialRxBytesWaiting() > 0 before calling serialRead().
-    // Do not spin here — spinning would block the cooperative scheduler.
+    // Do not spin here — that would block the cooperative scheduler.
     uint8_t ch = 0;
     tud_cdc_read(&ch, 1);
     return ch;
@@ -222,4 +230,124 @@ uint32_t usbVcpGetBaudRate(serialPort_t *instance)
     return coding.bit_rate;
 }
 
+// ── TinyUSB descriptor callbacks ─────────────────────────────────────────────
+//
+// These three callbacks are mandatory for TinyUSB device mode.  They were
+// previously provided by pico_stdio_usb/stdio_usb_descriptors.c, but that
+// file is compiled only when LIB_TINYUSB_DEVICE is NOT defined.  Since we
+// now define LIB_TINYUSB_DEVICE=1 (to stop pico_stdio_usb from spawning a
+// competing tud_task() timer), we must provide the callbacks here instead.
+//
+// USB identifiers:
+//   VID 0x2E8A = Raspberry Pi
+//   PID 0x000B = Pico 2 (RP2350) CDC serial application
+//   Product string from USBD_PRODUCT_STRING defined in target.h
+
+#include "pico/unique_id.h"
+
+#ifndef USBD_VID
+#define USBD_VID 0x2E8A  // Raspberry Pi
+#endif
+#ifndef USBD_PID
+#define USBD_PID 0x000B  // Pico 2 (RP2350) CDC application
+#endif
+#ifndef USBD_MANUFACTURER
+#define USBD_MANUFACTURER "Raspberry Pi"
+#endif
+// USBD_PRODUCT_STRING comes from target.h (e.g. "RP2350_PICO")
+#ifndef USBD_PRODUCT_STRING
+#define USBD_PRODUCT_STRING "INAV"
+#endif
+
+#define USBD_DESC_LEN (TUD_CONFIG_DESC_LEN + TUD_CDC_DESC_LEN)
+
+#define USBD_ITF_CDC        0   // two interfaces: control + data
+#define USBD_ITF_MAX        2
+
+#define USBD_CDC_EP_CMD     0x81
+#define USBD_CDC_EP_OUT     0x02
+#define USBD_CDC_EP_IN      0x82
+#define USBD_CDC_CMD_MAX_SIZE   8
+#define USBD_CDC_IN_OUT_MAX_SIZE 64
+
+#define USBD_STR_LANGID     0x00
+#define USBD_STR_MANUF      0x01
+#define USBD_STR_PRODUCT    0x02
+#define USBD_STR_SERIAL     0x03
+#define USBD_STR_CDC        0x04
+
+static const tusb_desc_device_t usbd_desc_device = {
+    .bLength            = sizeof(tusb_desc_device_t),
+    .bDescriptorType    = TUSB_DESC_DEVICE,
+    .bcdUSB             = 0x0200,
+    .bDeviceClass       = TUSB_CLASS_MISC,
+    .bDeviceSubClass    = MISC_SUBCLASS_COMMON,
+    .bDeviceProtocol    = MISC_PROTOCOL_IAD,
+    .bMaxPacketSize0    = CFG_TUD_ENDPOINT0_SIZE,
+    .idVendor           = USBD_VID,
+    .idProduct          = USBD_PID,
+    .bcdDevice          = 0x0100,
+    .iManufacturer      = USBD_STR_MANUF,
+    .iProduct           = USBD_STR_PRODUCT,
+    .iSerialNumber      = USBD_STR_SERIAL,
+    .bNumConfigurations = 1,
+};
+
+static const uint8_t usbd_desc_cfg[USBD_DESC_LEN] = {
+    TUD_CONFIG_DESCRIPTOR(1, USBD_ITF_MAX, USBD_STR_LANGID, USBD_DESC_LEN,
+        0, 250),
+    TUD_CDC_DESCRIPTOR(USBD_ITF_CDC, USBD_STR_CDC, USBD_CDC_EP_CMD,
+        USBD_CDC_CMD_MAX_SIZE, USBD_CDC_EP_OUT, USBD_CDC_EP_IN,
+        USBD_CDC_IN_OUT_MAX_SIZE),
+};
+
+static char usbd_serial_str[PICO_UNIQUE_BOARD_ID_SIZE_BYTES * 2 + 1];
+
+static const char *const usbd_desc_str[] = {
+    [USBD_STR_MANUF]   = USBD_MANUFACTURER,
+    [USBD_STR_PRODUCT] = USBD_PRODUCT_STRING,
+    [USBD_STR_SERIAL]  = usbd_serial_str,
+    [USBD_STR_CDC]     = "INAV MSP",
+};
+
+const uint8_t *tud_descriptor_device_cb(void)
+{
+    return (const uint8_t *)&usbd_desc_device;
+}
+
+const uint8_t *tud_descriptor_configuration_cb(uint8_t index)
+{
+    (void)index;
+    return usbd_desc_cfg;
+}
+
+#define USBD_DESC_STR_MAX 20
+
+const uint16_t *tud_descriptor_string_cb(uint8_t index, uint16_t langid)
+{
+    (void)langid;
+    static uint16_t desc_str[USBD_DESC_STR_MAX];
+
+    if (!usbd_serial_str[0]) {
+        pico_get_unique_board_id_string(usbd_serial_str, sizeof(usbd_serial_str));
+    }
+
+    uint8_t len;
+    if (index == 0) {
+        desc_str[1] = 0x0409;  // English
+        len = 1;
+    } else {
+        if (index >= ARRAYLEN(usbd_desc_str) || !usbd_desc_str[index]) {
+            return NULL;
+        }
+        const char *str = usbd_desc_str[index];
+        for (len = 0; len < USBD_DESC_STR_MAX - 1 && str[len]; len++) {
+            desc_str[1 + len] = str[len];
+        }
+    }
+
+    desc_str[0] = (uint16_t)((TUSB_DESC_STRING << 8) | (2 * len + 2));
+    return desc_str;
+}
+
 #endif // USE_VCP

src/main/drivers/system_rp2350.c

@@ -108,18 +108,22 @@ void systemInit(void)
     // SDK runtime already initialized clocks, GPIO, etc. via crt0 → runtime_init
     SystemCoreClock = clock_get_hz(clk_sys);
 
-    // tusb_init() must be called before stdio_init_all() because
-    // LIB_TINYUSB_DEVICE=1 (from CFG_TUD_ENABLED in tusb_config.h) causes
-    // PICO_STDIO_USB_ENABLE_TINYUSB_INIT=0, so stdio_usb_init() skips it.
+    // Initialize TinyUSB.  LIB_TINYUSB_DEVICE=1 is defined in cmake/rp2350.cmake so
+    // stdio_usb_init() (called via stdio_init_all() below) will skip its own tusb_init()
+    // and, critically, will not set up a competing tud_task() timer.
     tusb_init();
 
-    // Drive USB regardless of scheduler state: fire tud_task() every 1 ms
-    // from a hardware alarm (same approach pico_stdio_usb uses internally).
+    // Drive USB CDC from a dedicated 1ms hardware alarm so tud_task() runs even
+    // when the cooperative scheduler is busy (e.g. during long tasks or CLI mode).
+    // Only ONE caller of tud_task() exists: this timer.  pico_stdio_usb is disabled
+    // (LIB_PICO_STDIO_USB=0) so stdio_usb_init() registers no competing IRQ/alarm.
     if (!add_repeating_timer_ms(-1, usb_task_timer_cb, NULL, &usb_task_timer)) {
         // Alarm pool exhausted — USB CDC will not receive background servicing.
         // Nothing useful to do here; continue boot without functional USB.
     }
 
+    // stdio_init_all() is a no-op for USB (LIB_PICO_STDIO_USB=0 skips stdio_usb_init).
+    // Kept here in case UART stdio is re-enabled in the future.
     stdio_init_all();
 }
 

src/main/target/RP2350_PICO/pico_sdk_config.h

@@ -18,7 +18,10 @@
 #define LIB_CMSIS_CORE 1
 #define LIB_PICO_PLATFORM_PANIC 1
 #define LIB_PICO_STDIO 1
-#define LIB_PICO_STDIO_USB 1
+// Disable USB stdio: INAV's serial_usb_vcp_rp2350.c owns CDC interface 0 exclusively.
+// If pico_stdio_usb is enabled, printf() writes to the same CDC TX FIFO as MSP responses,
+// mixing debug text into the MSP packet stream and causing the configurator to disconnect.
+#define LIB_PICO_STDIO_USB 0
 #define LIB_PICO_PRINTF 1
 #define LIB_PICO_PRINTF_PICO 1
 #define LIB_PICO_RUNTIME 1

src/main/target/RP2350_PICO/target.c

@@ -51,20 +51,24 @@
  *
  * Port A (gpioid 0) = GPIO 0–15; Port B (gpioid 1) = GPIO 16–29.
  *   GP8  = PA8   GP9  = PA9   GP10 = PA10  GP11 = PA11
- *   GP12 = PA12  GP13 = PA13  GP14 = PA14  GP15 = PA15  (dual-use: UART3/4)
+ *   GP12 = PA12  GP13 = PA13  GP14 = PA14  GP15 = PA15
  *   GP20 = PB4   GP21 = PB5
  * GP16-19 are reserved: Flash CS (GP16), Beeper (GP17), I2C1 SDA/SCL (GP18/19).
+ *
+ * Motor pin assignment matches Betaflight RP2350A reference target:
+ *   MOTOR1=GP10, MOTOR2=GP11, MOTOR3=GP12, MOTOR4=GP13.
+ * GP8/9 are dual-use: UART3 (PIO1) when the serial port is active, else servo.
  */
 timerHardware_t timerHardware[] = {
-    /* slice 4 — motor group (GP8/GP9) */
-    DEF_TIM(TIM4,  CH1, PA8,  TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP8  */
-    DEF_TIM(TIM4,  CH2, PA9,  TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP9  */
-    /* slice 5 — motor group (GP10/GP11) */
+    /* slice 5 — motor group (GP10/GP11) — matches BF MOTOR1/MOTOR2 */
     DEF_TIM(TIM5,  CH1, PA10, TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP10 */
     DEF_TIM(TIM5,  CH2, PA11, TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP11 */
-    /* slice 6 — servo group (GP12/GP13; dual-use with UART3 TX/RX on PIO1) */
+    /* slice 6 — motor group (GP12/GP13) — matches BF MOTOR3/MOTOR4 */
     DEF_TIM(TIM6,  CH1, PA12, TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP12 */
     DEF_TIM(TIM6,  CH2, PA13, TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP13 */
+    /* slice 4 — servo group (GP8/GP9; dual-use with UART3 TX/RX on PIO1) */
+    DEF_TIM(TIM4,  CH1, PA8,  TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP8  */
+    DEF_TIM(TIM4,  CH2, PA9,  TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP9  */
     /* slice 7 — servo group (GP14/GP15; dual-use with UART4 TX/RX on PIO1) */
     DEF_TIM(TIM7,  CH1, PA14, TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP14 */
     DEF_TIM(TIM7,  CH2, PA15, TIM_USE_OUTPUT_AUTO, 0, 0),  /* GP15 */

src/main/target/RP2350_PICO/target.h

@@ -48,7 +48,7 @@
 #define SERIAL_PORT_COUNT 5  // VCP + UART1 + UART2 + UART3 + UART4
 
 // DShot motor output via PIO0 (1 SM per motor, up to 4 motors).
-// GP8–11 default to motors 1–4; GP4–7 are reserved for SPI0 (gyro + flash).
+// GP10–13 default to motors 1–4; GP4–7 are reserved for SPI0 (gyro + flash).
 // Motor/servo GPIO assignments come from timerHardware[] in target.c.
 
 // Servo PWM output via hardware PWM slices (GP16–GP19 by default).
@@ -68,23 +68,27 @@
 /*
  * Hardware UART pin assignments for Raspberry Pi Pico 2 — Option C layout.
  *
- * UART1 (INAV) → RP2350 uart0: GP0/1
- * UART2 (INAV) → RP2350 uart1: GP2/3
- * UART3 (INAV) → PIO1 SM0(TX)+SM1(RX): GP12/13
+ * UART1 (INAV) → RP2350 uart0: GP0/1   (F2 = UART0 TX/RX)
+ * UART2 (INAV) → RP2350 uart1: GP6/7   (F11 = UART1 TX/RX via GPIO_FUNC_UART_AUX)
+ * UART3 (INAV) → PIO1 SM0(TX)+SM1(RX): GP8/9
  * UART4 (INAV) → PIO1 SM2(TX)+SM3(RX): GP14/15
  *
- * GP4–7 are reserved for SPI0 (gyro + flash, future M5/M6).
- * GP8–11 are reserved for DShot motors on PIO0 (future M8).
+ * GP2/3 carry SPI0 SCK/MOSI (F1); GP4 SPI0 MISO, GP5 SPI0 CSn.
+ * GP10–13 are reserved for DShot motors on PIO0.
  * PIO2 SM0 is reserved for WS2812 LED strip; SMs 1–3 spare.
+ *
+ * Note: GP6/7 use GPIO function F11 (GPIO_FUNC_UART_AUX), not F2.
+ * See RP2350 datasheet Table 3: F2 on GP6/7 is UART1 CTS/RTS (flow
+ * control only); F11 is UART1 TX/RX.  The driver uses UART_FUNCSEL_NUM().
  */
 #define UART1_TX_PIN  PA0   /* GPIO0  — uart0 TX  (MSP / configurator) */
 #define UART1_RX_PIN  PA1   /* GPIO1  — uart0 RX */
-#define UART2_TX_PIN  PA2   /* GPIO2  — uart1 TX  (receiver: CRSF/SBUS) */
-#define UART2_RX_PIN  PA3   /* GPIO3  — uart1 RX  (HW inversion, no external inverter) */
+#define UART2_TX_PIN  PA6   /* GPIO6  — uart1 TX  (receiver: CRSF/SBUS, F11=UART_AUX) */
+#define UART2_RX_PIN  PA7   /* GPIO7  — uart1 RX  (HW inversion, no external inverter) */
 /* PIO1: UART3 on SM0(TX)+SM1(RX), UART4 on SM2(TX)+SM3(RX) */
 /* PIO2 is reserved for RGB LED strip (SM0) and future UART5/6 (SMs 1–3) */
-#define UART3_TX_PIN  PA12  /* GPIO12 — PIO1 SM0 TX  (GPS) */
-#define UART3_RX_PIN  PA13  /* GPIO13 — PIO1 SM1 RX */
+#define UART3_TX_PIN  PA8   /* GPIO8  — PIO1 SM0 TX  (GPS) */
+#define UART3_RX_PIN  PA9   /* GPIO9  — PIO1 SM1 RX */
 #define UART4_TX_PIN  PA14  /* GPIO14 — PIO1 SM2 TX  (telemetry / extra) */
 #define UART4_RX_PIN  PA15  /* GPIO15 — PIO1 SM3 RX */
 
@@ -139,12 +143,13 @@
 #undef USE_ADAPTIVE_FILTER
 #undef USE_GYRO_KALMAN
 
-// SPI0 — gyro + flash: GP4 (MISO/PA4), GP6 (SCK/PA6), GP7 (MOSI/PA7)
+// SPI0 — gyro + flash: GP2 (SCK/PA2), GP3 (MOSI/PA3), GP4 (MISO/PA4)
+// GP2/3 freed from UART2 (moved to GP6/7); GP6/7 freed from SPI.
 #define USE_SPI
 #define USE_SPI_DEVICE_1
-#define SPI1_SCK_PIN          PA6   /* GPIO6  — spi0 SCK  */
+#define SPI1_SCK_PIN          PA2   /* GPIO2  — spi0 SCK  */
 #define SPI1_MISO_PIN         PA4   /* GPIO4  — spi0 MISO */
-#define SPI1_MOSI_PIN         PA7   /* GPIO7  — spi0 MOSI */
+#define SPI1_MOSI_PIN         PA3   /* GPIO3  — spi0 MOSI */
 
 // FAST_CODE: place hot functions in SRAM (copied from flash at boot) to avoid
 // XIP cache pressure on the large PID/scheduler/gyro code path.
@@ -169,9 +174,9 @@
 /* RP2350 PWM slice "timers" — one TIM_TypeDef per slice, used as group IDs.
  * Analogous to TIM1/TIM3/… on STM32; pins sharing a slice must run at the
  * same update rate.  Defined in drivers/timer_rp2350.c. */
-extern TIM_TypeDef rp2350Pwm4;   /* slice 4:  GP8/GP9   — motors 1-2              */
-extern TIM_TypeDef rp2350Pwm5;   /* slice 5:  GP10/GP11 — motors 3-4              */
-extern TIM_TypeDef rp2350Pwm6;   /* slice 6:  GP12/GP13 — servos (dual-use UART3) */
+extern TIM_TypeDef rp2350Pwm4;   /* slice 4:  GP8/GP9   — servos (dual-use UART3) */
+extern TIM_TypeDef rp2350Pwm5;   /* slice 5:  GP10/GP11 — motors 1-2              */
+extern TIM_TypeDef rp2350Pwm6;   /* slice 6:  GP12/GP13 — motors 3-4              */
 extern TIM_TypeDef rp2350Pwm7;   /* slice 7:  GP14/GP15 — servos (dual-use UART4) */
 extern TIM_TypeDef rp2350Pwm10;  /* slice 10: GP20/GP21 — servos (dedicated)      */