From 164778a71682559670cc2549565b4d8d5f021fa9 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 28 May 2021 17:42:13 +0700 Subject: [PATCH 1/5] update limit each transfer not less than 64 --- src/portable/raspberrypi/rp2040/hcd_rp2040.c | 2 ++ src/portable/raspberrypi/rp2040/rp2040_usb.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c index 5201d1d4c..eff950de2 100644 --- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c @@ -452,6 +452,7 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t * if (ep_addr != ep->ep_addr) { // Direction has flipped so re init it but with same properties + // TODO treat IN and OUT as invidual endpoints _hw_endpoint_init(ep, dev_addr, ep_addr, ep->wMaxPacketSize, ep->transfer_type, 0); } @@ -531,6 +532,7 @@ bool hcd_edpt_busy(uint8_t dev_addr, uint8_t ep_addr) bool hcd_edpt_stalled(uint8_t dev_addr, uint8_t ep_addr) { panic("hcd_pipe_stalled"); + return false; } bool hcd_edpt_clear_stall(uint8_t dev_addr, uint8_t ep_addr) diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index 98ffea9a3..88d94aaec 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -161,7 +161,10 @@ void _hw_endpoint_xfer_start(struct hw_endpoint *ep, uint8_t *buffer, uint16_t t // Fill in info now that we're kicking off the hw ep->total_len = total_len; ep->len = 0; - ep->transfer_size = tu_min16(total_len, ep->wMaxPacketSize); + + // Limit by packet size but not less 64 (i.e low speed 8 bytes EP0) + ep->transfer_size = tu_min16(total_len, tu_max16(64, ep->wMaxPacketSize)); + ep->active = true; ep->user_buf = buffer; #if TUSB_OPT_HOST_ENABLED @@ -240,8 +243,9 @@ bool _hw_endpoint_xfer_continue(struct hw_endpoint *ep) _hw_endpoint_xfer_sync(ep); // Now we have synced our state with the hardware. Is there more data to transfer? + // Limit by packet size but not less 64 (i.e low speed 8 bytes EP0) uint16_t remaining_bytes = ep->total_len - ep->len; - ep->transfer_size = tu_min16(remaining_bytes, ep->wMaxPacketSize); + ep->transfer_size = tu_min16(remaining_bytes, tu_max16(64, ep->wMaxPacketSize)); #if TUSB_OPT_HOST_ENABLED _hw_endpoint_update_last_buf(ep); #endif From eb8ca14bf8171b48832410ed5e3f888df6564800 Mon Sep 17 00:00:00 2001 From: hathach Date: Sun, 30 May 2021 22:19:46 +0700 Subject: [PATCH 2/5] add level 3 log for info, add generic TU_LOG() --- src/common/tusb_common.h | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/common/tusb_common.h b/src/common/tusb_common.h index 1ebd8dd61..6f2905697 100644 --- a/src/common/tusb_common.h +++ b/src/common/tusb_common.h @@ -307,25 +307,40 @@ void tu_print_var(uint8_t const* buf, uint32_t bufsize) for(uint32_t i=0; i 1 +// Log Level 2: Warn +#if CFG_TUSB_DEBUG >= 2 #define TU_LOG2 TU_LOG1 #define TU_LOG2_MEM TU_LOG1_MEM #define TU_LOG2_VAR TU_LOG1_VAR #define TU_LOG2_INT TU_LOG1_INT #define TU_LOG2_HEX TU_LOG1_HEX - #define TU_LOG2_LOCATION() TU_LOG1_LOCATION() #endif +// Log Level 3: Info +#if CFG_TUSB_DEBUG >= 3 + #define TU_LOG3 TU_LOG1 + #define TU_LOG3_MEM TU_LOG1_MEM + #define TU_LOG3_VAR TU_LOG1_VAR + #define TU_LOG3_INT TU_LOG1_INT + #define TU_LOG3_HEX TU_LOG1_HEX +#endif typedef struct { @@ -370,6 +385,15 @@ static inline const char* tu_lookup_find(tu_lookup_table_t const* p_table, uint3 #define TU_LOG2_LOCATION() #endif +#ifndef TU_LOG3 + #define TU_LOG3(...) + #define TU_LOG3_MEM(...) + #define TU_LOG3_VAR(...) + #define TU_LOG3_INT(...) + #define TU_LOG3_HEX(...) + #define TU_LOG3_LOCATION() +#endif + #ifdef __cplusplus } #endif From 6498ee19966d394113530c44c91deb860ce37d78 Mon Sep 17 00:00:00 2001 From: hathach Date: Sun, 30 May 2021 23:35:54 +0700 Subject: [PATCH 3/5] fix incorrect data toggle when max packet size < 64 fix host buf_sel panic with "already available" --- src/portable/raspberrypi/rp2040/hcd_rp2040.c | 2 + src/portable/raspberrypi/rp2040/rp2040_usb.c | 28 ++++++++++++- src/portable/raspberrypi/rp2040/rp2040_usb.h | 44 ++++++++++++++++++-- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c index eff950de2..c09469ff5 100644 --- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c @@ -212,6 +212,7 @@ static void hcd_rp2040_irq(void) if (status & USB_INTS_BUFF_STATUS_BITS) { handled |= USB_INTS_BUFF_STATUS_BITS; + // print_bufctrl32(*epx.buffer_control); hw_handle_buff_status(); } @@ -233,6 +234,7 @@ static void hcd_rp2040_irq(void) if (status & USB_INTS_ERROR_DATA_SEQ_BITS) { usb_hw_clear->sie_status = USB_SIE_STATUS_DATA_SEQ_ERROR_BITS; + // print_bufctrl32(*epx.buffer_control); panic("Data Seq Error \n"); } diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index 88d94aaec..60a039556 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -46,7 +46,7 @@ static inline void _hw_endpoint_lock_update(struct hw_endpoint *ep, int delta) { #if TUSB_OPT_HOST_ENABLED static inline void _hw_endpoint_update_last_buf(struct hw_endpoint *ep) { - ep->last_buf = ep->len + ep->transfer_size == ep->total_len; + ep->last_buf = (ep->len + ep->transfer_size == ep->total_len); } #endif @@ -126,8 +126,29 @@ void _hw_endpoint_start_next_buffer(struct hw_endpoint *ep) // PID val |= ep->next_pid ? USB_BUF_CTRL_DATA1_PID : USB_BUF_CTRL_DATA0_PID; + +#if TUSB_OPT_DEVICE_ENABLED ep->next_pid ^= 1u; +#else + // For Host (also device but since we dictate the endpoint size, following scenario does not occur) + // Next PID depends on the number of packet in case wMaxPacketSize < 64 (e.g Interrupt Endpoint 8, or 12) + // Special case with control status stage where PID is always DATA1 + if ( ep->transfer_size == 0 ) + { + ep->next_pid ^= 1u; + }else + { + uint32_t packet_count = 1 + ((ep->transfer_size - 1) / ep->wMaxPacketSize); + + if ( packet_count & 0x01 ) + { + ep->next_pid ^= 1u; + } + } +#endif + + #if TUSB_OPT_HOST_ENABLED // Is this the last buffer? Only really matters for host mode. Will trigger // the trans complete irq but also stop it polling. We only really care about @@ -143,6 +164,7 @@ void _hw_endpoint_start_next_buffer(struct hw_endpoint *ep) // the next time the controller polls this dpram address _hw_endpoint_buffer_control_set_value32(ep, val); pico_trace("buffer control (0x%p) <- 0x%x\n", ep->buffer_control, val); + //print_bufctrl16(val); } @@ -189,10 +211,14 @@ void _hw_endpoint_xfer_sync(struct hw_endpoint *ep) #if TUSB_OPT_HOST_ENABLED // tag::host_buf_sel_fix[] + // TODO need changes to support double buffering if (ep->buf_sel == 1) { // Host can erroneously write status to top half of buf_ctrl register buf_ctrl = buf_ctrl >> 16; + + // update buf1 -> buf0 to prevent panic with "already available" + *ep->buffer_control = buf_ctrl; } // Flip buf sel for host ep->buf_sel ^= 1u; diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.h b/src/portable/raspberrypi/rp2040/rp2040_usb.h index bb88d977f..32d20051f 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.h +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.h @@ -16,8 +16,6 @@ #define TUD_OPT_RP2040_USB_DEVICE_ENUMERATION_FIX PICO_RP2040_USB_DEVICE_ENUMERATION_FIX #endif -// For memset -#include #if false && !defined(NDEBUG) #define pico_trace(format,args...) printf(format, ## args) @@ -78,7 +76,7 @@ struct hw_endpoint #if TUSB_OPT_HOST_ENABLED // Only needed for host mode bool last_buf; - // HOST BUG. Host will incorrect write status to top half of buffer + // RP2040-E4: HOST BUG. Host will incorrect write status to top half of buffer // control register when doing transfers > 1 packet uint8_t buf_sel; // Only needed for host @@ -119,4 +117,44 @@ static inline uintptr_t hw_data_offset(uint8_t *buf) extern const char *ep_dir_string[]; +typedef union TU_ATTR_PACKED +{ + uint16_t u16; + struct TU_ATTR_PACKED + { + uint16_t xfer_len : 10; + uint16_t available : 1; + uint16_t stall : 1; + uint16_t reset_bufsel : 1; + uint16_t data_toggle : 1; + uint16_t last_buf : 1; + uint16_t full : 1; + }; +} rp2040_buffer_control_t; + +TU_VERIFY_STATIC(sizeof(rp2040_buffer_control_t) == 2, "size is not correct"); + +static inline void print_bufctrl16(uint32_t u16) +{ + rp2040_buffer_control_t bufctrl; + + bufctrl.u16 = u16; + + TU_LOG(2, "len = %u, available = %u, stall = %u, reset = %u, toggle = %u, last = %u, full = %u\r\n", + bufctrl.xfer_len, bufctrl.available, bufctrl.stall, bufctrl.reset_bufsel, bufctrl.data_toggle, bufctrl.last_buf, bufctrl.full); +} + +static inline void print_bufctrl32(uint32_t u32) +{ + uint16_t u16; + + u16 = u32 >> 16; + TU_LOG(2, "Buffer Control 1 0x%x: ", u16); + print_bufctrl16(u16); + + u16 = u32 & 0x0000ffff; + TU_LOG(2, "Buffer Control 0 0x%x: ", u16); + print_bufctrl16(u16); +} + #endif From 54c915057484d4d63d0697114f156aef2837b728 Mon Sep 17 00:00:00 2001 From: hathach Date: Sun, 30 May 2021 23:41:59 +0700 Subject: [PATCH 4/5] add errata number --- src/portable/raspberrypi/rp2040/rp2040_usb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index 60a039556..7718293e5 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -210,6 +210,7 @@ void _hw_endpoint_xfer_sync(struct hw_endpoint *ep) uint16_t transferred_bytes = buf_ctrl & USB_BUF_CTRL_LEN_MASK; #if TUSB_OPT_HOST_ENABLED + // RP2040-E4 // tag::host_buf_sel_fix[] // TODO need changes to support double buffering if (ep->buf_sel == 1) From c2a0c1507b0db15a06773e321bd5c782bac54d39 Mon Sep 17 00:00:00 2001 From: hathach Date: Sun, 30 May 2021 23:44:29 +0700 Subject: [PATCH 5/5] add more comment --- src/portable/raspberrypi/rp2040/rp2040_usb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index 7718293e5..a0263612f 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -136,6 +136,7 @@ void _hw_endpoint_start_next_buffer(struct hw_endpoint *ep) // Special case with control status stage where PID is always DATA1 if ( ep->transfer_size == 0 ) { + // ZLP also toggle data ep->next_pid ^= 1u; }else {