diff --git a/hw/bsp/rp2040/family.cmake b/hw/bsp/rp2040/family.cmake index 5dba9dc39..f83120567 100644 --- a/hw/bsp/rp2040/family.cmake +++ b/hw/bsp/rp2040/family.cmake @@ -116,6 +116,7 @@ if (NOT TARGET _rp2040_family_inclusion_marker) target_compile_definitions(tinyusb_additions INTERFACE PICO_RP2040_USB_DEVICE_ENUMERATION_FIX=1 + PICO_RP2040_USB_DEVICE_UFRAME_FIX=1 ) if(DEFINED LOG) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 4952d29b1..b5fa90c92 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -46,9 +46,6 @@ /* Low level controller *------------------------------------------------------------------*/ -#define usb_hw_set hw_set_alias(usb_hw) -#define usb_hw_clear hw_clear_alias(usb_hw) - // Init these in dcd_init static uint8_t *next_buffer_ptr; @@ -247,104 +244,133 @@ static void __tusb_irq_path_func(reset_non_control_endpoints)(void) static void __tusb_irq_path_func(dcd_rp2040_irq)(void) { - uint32_t const status = usb_hw->ints; - uint32_t handled = 0; + uint32_t const status = usb_hw->ints; + uint32_t handled = 0; - if (status & USB_INTF_DEV_SOF_BITS) + if ( status & USB_INTF_DEV_SOF_BITS ) + { + bool keep_sof_alive = false; + + handled |= USB_INTF_DEV_SOF_BITS; + +#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX + // Errata 15 workaround for Device Bulk-In endpoint + e15_last_sof = time_us_32(); + + for ( uint8_t i = 0; i < USB_MAX_ENDPOINTS; i++ ) { - handled |= USB_INTF_DEV_SOF_BITS; + struct hw_endpoint * ep = hw_endpoint_get_by_num(i, TUSB_DIR_IN); - // disable SOF interrupt if it is used for RESUME in remote wakeup - if (!_sof_enable) usb_hw_clear->inte = USB_INTS_DEV_SOF_BITS; + // Active Bulk IN endpoint requires SOF + if ( (ep->transfer_type == TUSB_XFER_BULK) && ep->active ) + { + keep_sof_alive = true; - dcd_event_sof(0, usb_hw->sof_rd & USB_SOF_RD_BITS, true); - } + hw_endpoint_lock_update(ep, 1); - // xfer events are handled before setup req. So if a transfer completes immediately - // before closing the EP, the events will be delivered in same order. - if (status & USB_INTS_BUFF_STATUS_BITS) - { - handled |= USB_INTS_BUFF_STATUS_BITS; - hw_handle_buff_status(); - } - - if (status & USB_INTS_SETUP_REQ_BITS) - { - handled |= USB_INTS_SETUP_REQ_BITS; - uint8_t const *setup = (uint8_t const *)&usb_dpram->setup_packet; - - // reset pid to both 1 (data and ack) - reset_ep0_pid(); - - // Pass setup packet to tiny usb - dcd_event_setup_received(0, setup, true); - usb_hw_clear->sie_status = USB_SIE_STATUS_SETUP_REC_BITS; - } - -#if FORCE_VBUS_DETECT == 0 - // Since we force VBUS detect On, device will always think it is connected and - // couldn't distinguish between disconnect and suspend - if (status & USB_INTS_DEV_CONN_DIS_BITS) - { - handled |= USB_INTS_DEV_CONN_DIS_BITS; - - if ( usb_hw->sie_status & USB_SIE_STATUS_CONNECTED_BITS ) + // Deferred enable? + if ( ep->pending ) { - // Connected: nothing to do - }else - { - // Disconnected - dcd_event_bus_signal(0, DCD_EVENT_UNPLUGGED, true); + ep->pending = 0; + hw_endpoint_start_next_buffer(ep); } - usb_hw_clear->sie_status = USB_SIE_STATUS_CONNECTED_BITS; + hw_endpoint_lock_update(ep, -1); + } } #endif - // SE0 for 2.5 us or more (will last at least 10ms) - if (status & USB_INTS_BUS_RESET_BITS) + // disable SOF interrupt if it is used for RESUME in remote wakeup + if ( !keep_sof_alive && !_sof_enable ) usb_hw_clear->inte = USB_INTS_DEV_SOF_BITS; + + dcd_event_sof(0, usb_hw->sof_rd & USB_SOF_RD_BITS, true); + } + + // xfer events are handled before setup req. So if a transfer completes immediately + // before closing the EP, the events will be delivered in same order. + if ( status & USB_INTS_BUFF_STATUS_BITS ) + { + handled |= USB_INTS_BUFF_STATUS_BITS; + hw_handle_buff_status(); + } + + if ( status & USB_INTS_SETUP_REQ_BITS ) + { + handled |= USB_INTS_SETUP_REQ_BITS; + uint8_t const * setup = (uint8_t const*) &usb_dpram->setup_packet; + + // reset pid to both 1 (data and ack) + reset_ep0_pid(); + + // Pass setup packet to tiny usb + dcd_event_setup_received(0, setup, true); + usb_hw_clear->sie_status = USB_SIE_STATUS_SETUP_REC_BITS; + } + +#if FORCE_VBUS_DETECT == 0 + // Since we force VBUS detect On, device will always think it is connected and + // couldn't distinguish between disconnect and suspend + if (status & USB_INTS_DEV_CONN_DIS_BITS) + { + handled |= USB_INTS_DEV_CONN_DIS_BITS; + + if ( usb_hw->sie_status & USB_SIE_STATUS_CONNECTED_BITS ) { - pico_trace("BUS RESET\n"); + // Connected: nothing to do + }else + { + // Disconnected + dcd_event_bus_signal(0, DCD_EVENT_UNPLUGGED, true); + } - handled |= USB_INTS_BUS_RESET_BITS; + usb_hw_clear->sie_status = USB_SIE_STATUS_CONNECTED_BITS; + } +#endif - usb_hw->dev_addr_ctrl = 0; - reset_non_control_endpoints(); - dcd_event_bus_reset(0, TUSB_SPEED_FULL, true); - usb_hw_clear->sie_status = USB_SIE_STATUS_BUS_RESET_BITS; + // SE0 for 2.5 us or more (will last at least 10ms) + if ( status & USB_INTS_BUS_RESET_BITS ) + { + pico_trace("BUS RESET\n"); + + handled |= USB_INTS_BUS_RESET_BITS; + + usb_hw->dev_addr_ctrl = 0; + reset_non_control_endpoints(); + dcd_event_bus_reset(0, TUSB_SPEED_FULL, true); + usb_hw_clear->sie_status = USB_SIE_STATUS_BUS_RESET_BITS; #if TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX - // Only run enumeration walk-around if pull up is enabled - if ( usb_hw->sie_ctrl & USB_SIE_CTRL_PULLUP_EN_BITS ) rp2040_usb_device_enumeration_fix(); + // Only run enumeration workaround if pull up is enabled + if ( usb_hw->sie_ctrl & USB_SIE_CTRL_PULLUP_EN_BITS ) rp2040_usb_device_enumeration_fix(); #endif - } + } - /* Note from pico datasheet 4.1.2.6.4 (v1.2) - * If you enable the suspend interrupt, it is likely you will see a suspend interrupt when - * the device is first connected but the bus is idle. The bus can be idle for a few ms before - * the host begins sending start of frame packets. You will also see a suspend interrupt - * when the device is disconnected if you do not have a VBUS detect circuit connected. This is - * because without VBUS detection, it is impossible to tell the difference between - * being disconnected and suspended. - */ - if (status & USB_INTS_DEV_SUSPEND_BITS) - { - handled |= USB_INTS_DEV_SUSPEND_BITS; - dcd_event_bus_signal(0, DCD_EVENT_SUSPEND, true); - usb_hw_clear->sie_status = USB_SIE_STATUS_SUSPENDED_BITS; - } + /* Note from pico datasheet 4.1.2.6.4 (v1.2) + * If you enable the suspend interrupt, it is likely you will see a suspend interrupt when + * the device is first connected but the bus is idle. The bus can be idle for a few ms before + * the host begins sending start of frame packets. You will also see a suspend interrupt + * when the device is disconnected if you do not have a VBUS detect circuit connected. This is + * because without VBUS detection, it is impossible to tell the difference between + * being disconnected and suspended. + */ + if ( status & USB_INTS_DEV_SUSPEND_BITS ) + { + handled |= USB_INTS_DEV_SUSPEND_BITS; + dcd_event_bus_signal(0, DCD_EVENT_SUSPEND, true); + usb_hw_clear->sie_status = USB_SIE_STATUS_SUSPENDED_BITS; + } - if (status & USB_INTS_DEV_RESUME_FROM_HOST_BITS) - { - handled |= USB_INTS_DEV_RESUME_FROM_HOST_BITS; - dcd_event_bus_signal(0, DCD_EVENT_RESUME, true); - usb_hw_clear->sie_status = USB_SIE_STATUS_RESUME_BITS; - } + if ( status & USB_INTS_DEV_RESUME_FROM_HOST_BITS ) + { + handled |= USB_INTS_DEV_RESUME_FROM_HOST_BITS; + dcd_event_bus_signal(0, DCD_EVENT_RESUME, true); + usb_hw_clear->sie_status = USB_SIE_STATUS_RESUME_BITS; + } - if (status ^ handled) - { - panic("Unhandled IRQ 0x%x\n", (uint) (status ^ handled)); - } + if ( status ^ handled ) + { + panic("Unhandled IRQ 0x%x\n", (uint) (status ^ handled)); + } } #define USB_INTS_ERROR_BITS ( \ @@ -452,7 +478,11 @@ void dcd_sof_enable(uint8_t rhport, bool en) usb_hw_set->inte = USB_INTS_DEV_SOF_BITS; }else { + // Don't clear immediately if the SOF workaround is in use. + // The SOF handler will conditionally disable the interrupt. +#if !TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX usb_hw_clear->inte = USB_INTS_DEV_SOF_BITS; +#endif } } diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c index 9e6bdad44..28abd7939 100644 --- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c @@ -56,9 +56,6 @@ static_assert(PICO_USB_HOST_INTERRUPT_ENDPOINTS <= USB_MAX_ENDPOINTS, ""); static struct hw_endpoint ep_pool[1 + PICO_USB_HOST_INTERRUPT_ENDPOINTS]; #define epx (ep_pool[0]) -#define usb_hw_set hw_set_alias(usb_hw) -#define usb_hw_clear hw_clear_alias(usb_hw) - // Flags we set by default in sie_ctrl (we add other bits on top) enum { SIE_CTRL_BASE = USB_SIE_CTRL_SOF_EN_BITS | USB_SIE_CTRL_KEEP_ALIVE_EN_BITS | diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index 5c7645902..df05697fe 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -32,20 +32,25 @@ #include #include "rp2040_usb.h" +//--------------------------------------------------------------------+ +// MACRO CONSTANT TYPEDEF PROTOTYPE +//--------------------------------------------------------------------+ + // Direction strings for debug const char *ep_dir_string[] = { "out", "in", }; -TU_ATTR_ALWAYS_INLINE static inline void _hw_endpoint_lock_update(__unused struct hw_endpoint * ep, __unused int delta) { - // todo add critsec as necessary to prevent issues between worker and IRQ... - // note that this is perhaps as simple as disabling IRQs because it would make - // sense to have worker and IRQ on same core, however I think using critsec is about equivalent. -} - static void _hw_endpoint_xfer_sync(struct hw_endpoint *ep); -static void _hw_endpoint_start_next_buffer(struct hw_endpoint *ep); + +#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX + static bool e15_is_bulkin_ep(struct hw_endpoint *ep); + static bool e15_is_critical_frame_period(struct hw_endpoint *ep); +#else + #define e15_is_bulkin_ep(x) (false) + #define e15_is_critical_frame_period(x) (false) +#endif // if usb hardware is in host mode TU_ATTR_ALWAYS_INLINE static inline bool is_host_mode(void) @@ -54,7 +59,7 @@ TU_ATTR_ALWAYS_INLINE static inline bool is_host_mode(void) } //--------------------------------------------------------------------+ -// +// Implementation //--------------------------------------------------------------------+ void rp2040_usb_init(void) @@ -87,12 +92,15 @@ void __tusb_irq_path_func(hw_endpoint_reset_transfer)(struct hw_endpoint *ep) ep->user_buf = 0; } -void __tusb_irq_path_func(_hw_endpoint_buffer_control_update32)(struct hw_endpoint *ep, uint32_t and_mask, uint32_t or_mask) { +void __tusb_irq_path_func(_hw_endpoint_buffer_control_update32)(struct hw_endpoint *ep, uint32_t and_mask, uint32_t or_mask) +{ uint32_t value = 0; + if ( and_mask ) { value = *ep->buffer_control & and_mask; } + if ( or_mask ) { value |= or_mask; @@ -118,6 +126,7 @@ void __tusb_irq_path_func(_hw_endpoint_buffer_control_update32)(struct hw_endpoi #endif } } + *ep->buffer_control = value; } @@ -157,7 +166,7 @@ static uint32_t __tusb_irq_path_func(prepare_ep_buffer)(struct hw_endpoint *ep, } // Prepare buffer control register value -static void __tusb_irq_path_func(_hw_endpoint_start_next_buffer)(struct hw_endpoint *ep) +void __tusb_irq_path_func(hw_endpoint_start_next_buffer)(struct hw_endpoint *ep) { uint32_t ep_ctrl = *ep->endpoint_control; @@ -201,7 +210,7 @@ static void __tusb_irq_path_func(_hw_endpoint_start_next_buffer)(struct hw_endpo void hw_endpoint_xfer_start(struct hw_endpoint *ep, uint8_t *buffer, uint16_t total_len) { - _hw_endpoint_lock_update(ep, 1); + hw_endpoint_lock_update(ep, 1); if ( ep->active ) { @@ -218,8 +227,20 @@ void hw_endpoint_xfer_start(struct hw_endpoint *ep, uint8_t *buffer, uint16_t to ep->active = true; ep->user_buf = buffer; - _hw_endpoint_start_next_buffer(ep); - _hw_endpoint_lock_update(ep, -1); + if ( e15_is_bulkin_ep(ep) ) + { + usb_hw_set->inte = USB_INTS_DEV_SOF_BITS; + } + + if ( e15_is_critical_frame_period(ep) ) + { + ep->pending = 1; + } else + { + hw_endpoint_start_next_buffer(ep); + } + + hw_endpoint_lock_update(ep, -1); } // sync endpoint buffer and return transferred bytes @@ -312,7 +333,8 @@ static void __tusb_irq_path_func(_hw_endpoint_xfer_sync) (struct hw_endpoint *ep // Returns true if transfer is complete bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint *ep) { - _hw_endpoint_lock_update(ep, 1); + hw_endpoint_lock_update(ep, 1); + // Part way through a transfer if (!ep->active) { @@ -329,17 +351,75 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint *ep) pico_trace("Completed transfer of %d bytes on ep %d %s\n", ep->xferred_len, tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); // Notify caller we are done so it can notify the tinyusb stack - _hw_endpoint_lock_update(ep, -1); + hw_endpoint_lock_update(ep, -1); return true; } else { - _hw_endpoint_start_next_buffer(ep); + if ( e15_is_critical_frame_period(ep) ) + { + ep->pending = 1; + } else + { + hw_endpoint_start_next_buffer(ep); + } } - _hw_endpoint_lock_update(ep, -1); + hw_endpoint_lock_update(ep, -1); // More work to do return false; } +//--------------------------------------------------------------------+ +// Errata 15 +//--------------------------------------------------------------------+ + +#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX + +/* Don't mark IN buffers as available during the last 200us of a full-speed + frame. This avoids a situation seen with the USB2.0 hub on a Raspberry + Pi 4 where a late IN token before the next full-speed SOF can cause port + babble and a corrupt ACK packet. The nature of the data corruption has a + chance to cause device lockup. + + Use the next SOF to mark delayed buffers as available. This reduces + available Bulk IN bandwidth by approximately 20%, and requires that the + SOF interrupt is enabled while these transfers are ongoing. + + Inherit the top-level enable from the corresponding Pico-SDK flag. + Applications that will not use the device in a situation where it could + be plugged into a Pi 4 or Pi 400 (for example, when directly connected + to a commodity hub or other host) can turn off the flag in the SDK. +*/ + +volatile uint32_t e15_last_sof = 0; + +// check if Errata 15 is needed for this endpoint i.e device bulk-in +static bool __tusb_irq_path_func(e15_is_bulkin_ep) (struct hw_endpoint *ep) +{ + return (!is_host_mode() && tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN && + ep->transfer_type == TUSB_XFER_BULK); +} + +// check if we need to apply Errata 15 workaround : i.e +// Endpoint is BULK IN and is currently in critical frame period i.e 20% of last usb frame +static bool __tusb_irq_path_func(e15_is_critical_frame_period) (struct hw_endpoint *ep) +{ + TU_VERIFY(e15_is_bulkin_ep(ep)); + + /* Avoid the last 200us (uframe 6.5-7) of a frame, up to the EOF2 point. + * The device state machine cannot recover from receiving an incorrect PID + * when it is expecting an ACK. + */ + uint32_t delta = time_us_32() - e15_last_sof; + if (delta < 800 || delta > 998) { + return false; + } + TU_LOG(3, "Avoiding sof %u now %lu last %lu\n", (usb_hw->sof_rd + 1) & USB_SOF_RD_BITS, time_us_32(), e15_last_sof); + return true; +} + +#endif + + #endif diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.h b/src/portable/raspberrypi/rp2040/rp2040_usb.h index b65d32fd4..a06407f23 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.h +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.h @@ -11,11 +11,21 @@ #include "hardware/structs/usb.h" #include "hardware/irq.h" #include "hardware/resets.h" +#include "hardware/timer.h" #if defined(PICO_RP2040_USB_DEVICE_ENUMERATION_FIX) && !defined(TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX) #define TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX PICO_RP2040_USB_DEVICE_ENUMERATION_FIX #endif +#if defined(PICO_RP2040_USB_DEVICE_UFRAME_FIX) && !defined(TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX) +#define TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX PICO_RP2040_USB_DEVICE_UFRAME_FIX +#endif + +#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX +#undef PICO_RP2040_USB_FAST_IRQ +#define PICO_RP2040_USB_FAST_IRQ 1 +#endif + #ifndef PICO_RP2040_USB_FAST_IRQ #define PICO_RP2040_USB_FAST_IRQ 0 #endif @@ -26,6 +36,9 @@ #define __tusb_irq_path_func(x) x #endif +#define usb_hw_set hw_set_alias(usb_hw) +#define usb_hw_clear hw_clear_alias(usb_hw) + #define pico_info(...) TU_LOG(2, __VA_ARGS__) #define pico_trace(...) TU_LOG(3, __VA_ARGS__) @@ -38,7 +51,7 @@ typedef struct hw_endpoint // Transfer direction (i.e. IN is rx for host but tx for device) // allows us to common up transfer functions bool rx; - + uint8_t ep_addr; uint8_t next_pid; @@ -51,20 +64,25 @@ typedef struct hw_endpoint // Buffer pointer in usb dpram uint8_t *hw_data_buf; - // Current transfer information - bool active; - uint16_t remaining_len; - uint16_t xferred_len; - // User buffer in main memory uint8_t *user_buf; + // Current transfer information + uint16_t remaining_len; + uint16_t xferred_len; + // Data needed from EP descriptor uint16_t wMaxPacketSize; + // Endpoint is in use + bool active; + // Interrupt, bulk, etc uint8_t transfer_type; - + + // Transfer scheduled but not active + uint8_t pending; + #if CFG_TUH_ENABLED // Only needed for host uint8_t dev_addr; @@ -72,13 +90,25 @@ typedef struct hw_endpoint // If interrupt endpoint uint8_t interrupt_num; #endif + } hw_endpoint_t; +#if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX +extern volatile uint32_t e15_last_sof; +#endif + void rp2040_usb_init(void); void hw_endpoint_xfer_start(struct hw_endpoint *ep, uint8_t *buffer, uint16_t total_len); bool hw_endpoint_xfer_continue(struct hw_endpoint *ep); void hw_endpoint_reset_transfer(struct hw_endpoint *ep); +void hw_endpoint_start_next_buffer(struct hw_endpoint *ep); + +TU_ATTR_ALWAYS_INLINE static inline void hw_endpoint_lock_update(__unused struct hw_endpoint * ep, __unused int delta) { + // todo add critsec as necessary to prevent issues between worker and IRQ... + // note that this is perhaps as simple as disabling IRQs because it would make + // sense to have worker and IRQ on same core, however I think using critsec is about equivalent. +} void _hw_endpoint_buffer_control_update32(struct hw_endpoint *ep, uint32_t and_mask, uint32_t or_mask);