stdio_usb improvements (#871)

Use shared IRQ if available to avoid 1ms timer. Allow use of stdio_usb with user's tinyusb setup if it has CDC

Author: kilograham

src/rp2_common/hardware_irq/include/hardware/irq.h

@@ -252,6 +252,14 @@ void irq_add_shared_handler(uint num, irq_handler_t handler, uint8_t order_prior
  */
 void irq_remove_handler(uint num, irq_handler_t handler);
 
+/*! \brief Determine if the current handler for the given number is shared
+ *  \ingroup hardware_irq
+ *
+ * \param num Interrupt number \ref interrupt_nums
+ * \param return true if the specified IRQ has a shared handler
+ */
+bool irq_has_shared_handler(uint num);
+
 /*! \brief Get the current IRQ handler for the specified IRQ from the currently installed hardware vector table (VTOR)
  * of the execution core
  *  \ingroup hardware_irq

src/rp2_common/hardware_irq/irq.c

@@ -95,10 +95,21 @@ static int8_t irq_hander_chain_free_slot_head;
 static inline bool is_shared_irq_raw_handler(irq_handler_t raw_handler) {
     return (uintptr_t)raw_handler - (uintptr_t)irq_handler_chain_slots < sizeof(irq_handler_chain_slots);
 }
+
+bool irq_has_shared_handler(uint irq_num) {
+    check_irq_param(irq_num);
+    irq_handler_t handler = irq_get_vtable_handler(irq_num);
+    return handler && is_shared_irq_raw_handler(handler);
+}
+
 #else
 #define is_shared_irq_raw_handler(h) false
+bool irq_has_shared_handler(uint irq_num) {
+    return false;
+}
 #endif
 
+
 irq_handler_t irq_get_vtable_handler(uint num) {
     check_irq_param(num);
     return get_vtable()[16 + num];

src/rp2_common/pico_stdio_usb/include/tusb_config.h

@@ -29,6 +29,7 @@
 
 #include "pico/stdio_usb.h"
 
+#if !defined(LIB_TINYUSB_HOST) && !defined(LIB_TINYUSB_DEVICE)
 #define CFG_TUSB_RHPORT0_MODE   (OPT_MODE_DEVICE)
 
 #define CFG_TUD_CDC             (1)
@@ -38,3 +39,5 @@
 // We use a vendor specific interface but with our own driver
 #define CFG_TUD_VENDOR            (0)
 #endif
+
+#endif

src/rp2_common/pico_stdio_usb/reset_interface.c

@@ -7,6 +7,8 @@
 
 #include "pico/bootrom.h"
 
+#if !defined(LIB_TINYUSB_HOST) && !defined(LIB_TINYUSB_DEVICE)
+
 #if PICO_STDIO_USB_ENABLE_RESET_VIA_VENDOR_INTERFACE && !(PICO_STDIO_USB_RESET_INTERFACE_SUPPORT_RESET_TO_BOOTSEL || PICO_STDIO_USB_RESET_INTERFACE_SUPPORT_RESET_TO_FLASH_BOOT)
 #warning PICO_STDIO_USB_ENABLE_RESET_VIA_VENDOR_INTERFACE has been selected but neither PICO_STDIO_USB_RESET_INTERFACE_SUPPORT_RESET_TO_BOOTSEL nor PICO_STDIO_USB_RESET_INTERFACE_SUPPORT_RESET_TO_FLASH_BOOT have been selected.
 #endif
@@ -110,3 +112,4 @@ void tud_cdc_line_coding_cb(__unused uint8_t itf, cdc_line_coding_t const* p_lin
 }
 #endif
 
+#endif

src/rp2_common/pico_stdio_usb/stdio_usb.c

@@ -4,24 +4,34 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-#if !defined(LIB_TINYUSB_HOST) && !defined(LIB_TINYUSB_DEVICE)
+#ifndef LIB_TINYUSB_HOST
 #include "tusb.h"
+#include "pico/stdio_usb.h"
+
+// these may not be set if the user is providing tud support (i.e. LIB_TINYUSB_DEVICE is 1 because
+// the user linked in tinyusb_device) but they haven't selected CDC
+#if (CFG_TUD_ENABLED | TUSB_OPT_DEVICE_ENABLED) && CFG_TUD_CDC
 
+#include "pico/binary_info.h"
 #include "pico/time.h"
 #include "pico/stdio/driver.h"
-#include "pico/binary_info.h"
 #include "pico/mutex.h"
 #include "hardware/irq.h"
 
+static mutex_t stdio_usb_mutex;
+#ifndef NDEBUG
+static uint8_t stdio_usb_core_num;
+#endif
+
+// when tinyusb_device is explicitly linked we do no background tud processing
+#if !LIB_TINYUSB_DEVICE
 #ifdef PICO_STDIO_USB_LOW_PRIORITY_IRQ
 static_assert(PICO_STDIO_USB_LOW_PRIORITY_IRQ >= NUM_IRQS - NUM_USER_IRQS, "");
 #define low_priority_irq_num PICO_STDIO_USB_LOW_PRIORITY_IRQ
 #else
 static uint8_t low_priority_irq_num;
 #endif
 
-static mutex_t stdio_usb_mutex;
-
 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
@@ -32,10 +42,16 @@ static void low_priority_worker_irq(void) {
     }
 }
 
+static void usb_irq(void) {
+    irq_set_pending(low_priority_irq_num);
+}
+
 static int64_t timer_task(__unused alarm_id_t id, __unused void *user_data) {
+    assert(stdio_usb_core_num == get_core_num()); // if this fails, you have initialized stdio_usb on the wrong core
     irq_set_pending(low_priority_irq_num);
     return PICO_STDIO_USB_TASK_INTERVAL_US;
 }
+#endif
 
 static void stdio_usb_out_chars(const char *buf, int length) {
     static uint64_t last_avail_time;
@@ -95,13 +111,23 @@ stdio_driver_t stdio_usb = {
 };
 
 bool stdio_usb_init(void) {
+#ifndef NDEBUG
+    stdio_usb_core_num = (uint8_t)get_core_num();
+#endif
 #if !PICO_NO_BI_STDIO_USB
     bi_decl_if_func_used(bi_program_feature("USB stdin / stdout"));
 #endif
 
-    // initialize TinyUSB
+#if !defined(LIB_TINYUSB_DEVICE)
+    // initialize TinyUSB, as user hasn't explicitly linked it
     tusb_init();
+#else
+    assert(tud_inited()); // we expect the caller to have initialized if they are using TinyUSB
+#endif
 
+    mutex_init(&stdio_usb_mutex);
+    bool rc = true;
+#if !LIB_TINYUSB_DEVICE
 #ifdef PICO_STDIO_USB_LOW_PRIORITY_IRQ
     user_irq_claim(PICO_STDIO_USB_LOW_PRIORITY_IRQ);
 #else
@@ -110,8 +136,13 @@ bool stdio_usb_init(void) {
     irq_set_exclusive_handler(low_priority_irq_num, low_priority_worker_irq);
     irq_set_enabled(low_priority_irq_num, true);
 
-    mutex_init(&stdio_usb_mutex);
-    bool rc = add_alarm_in_us(PICO_STDIO_USB_TASK_INTERVAL_US, timer_task, NULL, true);
+    if (irq_has_shared_handler(USBCTRL_IRQ)) {
+        // we can use a shared handler to notice when there may be work to do
+        irq_add_shared_handler(USBCTRL_IRQ, usb_irq, PICO_SHARED_IRQ_HANDLER_LOWEST_ORDER_PRIORITY);
+    } else {
+        rc = add_alarm_in_us(PICO_STDIO_USB_TASK_INTERVAL_US, timer_task, NULL, true);
+    }
+#endif
     if (rc) {
         stdio_set_driver_enabled(&stdio_usb, true);
 #if PICO_STDIO_USB_CONNECT_WAIT_TIMEOUT_MS
@@ -138,9 +169,15 @@ bool stdio_usb_connected(void) {
     return tud_cdc_connected();
 }
 #else
-#include "pico/stdio_usb.h"
-#warning stdio USB was configured, but is being disabled as TinyUSB is explicitly linked
+#warning stdio USB was configured along with user use of TinyUSB device mode, but CDC is not enabled
 bool stdio_usb_init(void) {
     return false;
 }
-#endif
+#endif // CFG_TUD_ENABLED && CFG_TUD_CDC
+#else
+#warning stdio USB was configured, but is being disabled as TinyUSB host is explicitly linked
+bool stdio_usb_init(void) {
+    return false;
+}
+#endif // !LIB_TINYUSB_HOST
+