fix incorrect data toggle when max packet size < 64

fix host buf_sel panic with "already available"

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 <string.h>
 
 #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