diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 0e9dd84e5..5c564cb1c 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -60,7 +60,7 @@ TU_ATTR_ALWAYS_INLINE static inline struct hw_endpoint* hw_endpoint_get_by_num(u return &hw_endpoints[num][dir]; } -static struct hw_endpoint* hw_endpoint_get_by_addr(uint8_t ep_addr) { +TU_ATTR_ALWAYS_INLINE static inline struct hw_endpoint* hw_endpoint_get_by_addr(uint8_t ep_addr) { uint8_t num = tu_edpt_number(ep_addr); tusb_dir_t dir = tu_edpt_dir(ep_addr); return hw_endpoint_get_by_num(num, dir); @@ -192,47 +192,31 @@ static void __tusb_irq_path_func(hw_handle_buff_status)(void) { } } -TU_ATTR_ALWAYS_INLINE static inline void reset_ep0_pid(void) -{ - // Abort any transactions from a prior control transfer, because - // receiving SETUP doesn't reset buffer control state. This works around - // a possible USB hardware erratum. +TU_ATTR_ALWAYS_INLINE static inline void reset_ep0(void) { + // If we have finished this transfer on EP0 set pid back to 1 for next + // setup transfer. Also clear a stall in case + for (uint8_t dir = 0; dir < 2; dir++) { + struct hw_endpoint* ep = hw_endpoint_get_by_num(0, dir); + if (ep->active) { + // Abort any pending transfer from a prior control transfer per USB specs + // Due to Errata RP2040-E2: ABORT flag is only applicable for B2 and later (unusable for B0, B1). + // Which means we are not guaranteed to safely abort pending transfer on B0 and B1. + uint32_t const abort_mask = (dir ? USB_EP_ABORT_EP0_IN_BITS : USB_EP_ABORT_EP0_OUT_BITS); + if (rp2040_chip_version() >= 2) { + usb_hw_set->abort = abort_mask; + while ((usb_hw->abort_done & abort_mask) != abort_mask) {} + } - // With this workaround a race window still exists, but smaller. - // ABORT flag is unusable prior to hardware B2 (RP2040-E2), so a larger - // race window exists for B1 and earlier. - if (rp2040_chip_version() >= 2) { - usb_hw_set->abort = 0x3; - while ((usb_hw->abort_done & 0x3) != 0x3) - ; - } - // If we have finished this transfer on EP0 set pid back to 1 for next - // setup transfer. Also clear a stall in case - uint8_t addrs[] = {0x0, 0x80}; - for (uint i = 0 ; i < TU_ARRAY_SIZE(addrs); i++) - { - struct hw_endpoint *ep = hw_endpoint_get_by_addr(addrs[i]); - ep->next_pid = 1u; - // Reset the buffer control now to minimize race conditions - _hw_endpoint_buffer_control_set_value32(ep, USB_BUF_CTRL_DATA1_PID | USB_BUF_CTRL_SEL); - // Explicit delay, because the one in - // _hw_endpoint_buffer_control_set_value32 is only to set AVAILABLE - __asm volatile ( - "b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1:\n" - : : : "memory"); - // Make sure local ep state matches peripheral - hw_endpoint_reset_transfer(ep); - } - if (rp2040_chip_version() >= 2) { - usb_hw_clear->abort = 0x3; - usb_hw_clear->abort_done = 0x3; + _hw_endpoint_buffer_control_set_value32(ep, USB_BUF_CTRL_DATA1_PID | USB_BUF_CTRL_SEL); + hw_endpoint_reset_transfer(ep); + + if (rp2040_chip_version() >= 2) { + usb_hw_clear->abort_done = abort_mask; + usb_hw_clear->abort = abort_mask; + } } + ep->next_pid = 1u; + } } static void __tusb_irq_path_func(reset_non_control_endpoints)(void) { @@ -300,7 +284,7 @@ static void __tusb_irq_path_func(dcd_rp2040_irq)(void) { uint8_t const* setup = remove_volatile_cast(uint8_t const*, &usb_dpram->setup_packet); // reset pid to both 1 (data and ack) - reset_ep0_pid(); + reset_ep0(); // Pass setup packet to tiny usb dcd_event_setup_received(0, setup, true); @@ -388,6 +372,8 @@ static void __tusb_irq_path_func(dcd_rp2040_irq)(void) { void dcd_init(uint8_t rhport) { assert(rhport == 0); + TU_LOG(2, "Chip Version B%u\r\n", rp2040_chip_version()); + // Reset hardware to default state rp2040_usb_init(); diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index b5bace972..1ca711c77 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -35,13 +35,6 @@ //--------------------------------------------------------------------+ // MACRO CONSTANT TYPEDEF PROTOTYPE //--------------------------------------------------------------------+ - -// Direction strings for debug -const char* ep_dir_string[] = { - "out", - "in", -}; - static void _hw_endpoint_xfer_sync(struct hw_endpoint* ep); #if TUD_OPT_RP2040_USB_DEVICE_UFRAME_FIX @@ -105,22 +98,14 @@ void __tusb_irq_path_func(_hw_endpoint_buffer_control_update32)(struct hw_endpoi value |= or_mask; if (or_mask & USB_BUF_CTRL_AVAIL) { if (*ep->buffer_control & USB_BUF_CTRL_AVAIL) { - panic("ep %d %s was already available", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); + panic("ep %02X was already available", ep->ep_addr); } *ep->buffer_control = value & ~USB_BUF_CTRL_AVAIL; - // 12 cycle delay.. (should be good for 48*12Mhz = 576Mhz) + // 4.1.2.5.1 Con-current access: 12 cycles (should be good for 48*12Mhz = 576Mhz) after write to buffer control // Don't need delay in host mode as host is in charge -#if !CFG_TUH_ENABLED - __asm volatile ( - "b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1: b 1f\n" - "1:\n" - : : : "memory"); -#endif + if ( !is_host_mode()) { + busy_wait_at_least_cycles(12); + } } } @@ -204,9 +189,7 @@ void hw_endpoint_xfer_start(struct hw_endpoint* ep, uint8_t* buffer, uint16_t to if (ep->active) { // TODO: Is this acceptable for interrupt packets? - TU_LOG(1, "WARN: starting new transfer on already active ep %d %s\r\n", tu_edpt_number(ep->ep_addr), - ep_dir_string[tu_edpt_dir(ep->ep_addr)]); - + TU_LOG(1, "WARN: starting new transfer on already active ep %02X\r\n", ep->ep_addr); hw_endpoint_reset_transfer(ep); } @@ -314,8 +297,7 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint* ep) { // Part way through a transfer if (!ep->active) { - panic("Can't continue xfer on inactive ep %d %s", tu_edpt_number(ep->ep_addr), - ep_dir_string[tu_edpt_dir(ep->ep_addr)]); + panic("Can't continue xfer on inactive ep %02X", ep->ep_addr); } // Update EP struct from hardware state @@ -324,8 +306,7 @@ bool __tusb_irq_path_func(hw_endpoint_xfer_continue)(struct hw_endpoint* ep) { // Now we have synced our state with the hardware. Is there more data to transfer? // If we are done then notify tinyusb if (ep->remaining_len == 0) { - pico_trace("Completed transfer of %d bytes on ep %d %s\r\n", - ep->xferred_len, tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); + pico_trace("Completed transfer of %d bytes on ep %02X\r\n", ep->xferred_len, ep->ep_addr); // Notify caller we are done so it can notify the tinyusb stack hw_endpoint_lock_update(ep, -1); return true;