Merge pull request #1779 from P33M/rp2040_device_babble_fix

rp2040: avoid device-mode state machine hang
This commit is contained in:
Ha Thach 2023-01-31 21:38:27 +07:00 committed by GitHub
commit 49628d8c18
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 246 additions and 108 deletions

View File

@ -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)

View File

@ -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
}
}

View File

@ -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 |

View File

@ -32,20 +32,25 @@
#include <stdlib.h>
#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

View File

@ -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);