From 7858601a58fed753c5aaa5ff6828cc0492821bef Mon Sep 17 00:00:00 2001 From: Graham Sanderson Date: Mon, 20 Jun 2022 09:51:51 -0500 Subject: [PATCH] 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 --- .../hardware_irq/include/hardware/irq.h | 8 +++ src/rp2_common/hardware_irq/irq.c | 11 ++++ .../pico_stdio_usb/include/tusb_config.h | 3 + .../pico_stdio_usb/reset_interface.c | 3 + src/rp2_common/pico_stdio_usb/stdio_usb.c | 57 +++++++++++++++---- 5 files changed, 72 insertions(+), 10 deletions(-) diff --git a/src/rp2_common/hardware_irq/include/hardware/irq.h b/src/rp2_common/hardware_irq/include/hardware/irq.h index 4f83466..66679fe 100644 --- a/src/rp2_common/hardware_irq/include/hardware/irq.h +++ b/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 diff --git a/src/rp2_common/hardware_irq/irq.c b/src/rp2_common/hardware_irq/irq.c index 170fcf1..251cdf7 100644 --- a/src/rp2_common/hardware_irq/irq.c +++ b/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]; diff --git a/src/rp2_common/pico_stdio_usb/include/tusb_config.h b/src/rp2_common/pico_stdio_usb/include/tusb_config.h index c0a2843..c0d2ed3 100644 --- a/src/rp2_common/pico_stdio_usb/include/tusb_config.h +++ b/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 diff --git a/src/rp2_common/pico_stdio_usb/reset_interface.c b/src/rp2_common/pico_stdio_usb/reset_interface.c index 06dce86..cc25177 100644 --- a/src/rp2_common/pico_stdio_usb/reset_interface.c +++ b/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 \ No newline at end of file diff --git a/src/rp2_common/pico_stdio_usb/stdio_usb.c b/src/rp2_common/pico_stdio_usb/stdio_usb.c index df3920f..cd2e2c9 100644 --- a/src/rp2_common/pico_stdio_usb/stdio_usb.c +++ b/src/rp2_common/pico_stdio_usb/stdio_usb.c @@ -4,15 +4,27 @@ * 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 @@ -20,8 +32,6 @@ static_assert(PICO_STDIO_USB_LOW_PRIORITY_IRQ >= NUM_IRQS - NUM_USER_IRQS, ""); 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 +