rp2040 correct ep set/clear stall

- stall will remove pending (not complete) transfer. Correct reset data
toggle when clear stall.
- remove buf ctrl debug code

src/common/tusb_common.h

@@ -110,6 +110,9 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_u32_byte2(uint32_t u32) { return
 TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_u32_byte1(uint32_t u32) { return TU_U32_BYTE1(u32); }
 TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_u32_byte0(uint32_t u32) { return TU_U32_BYTE0(u32); }
 
+TU_ATTR_ALWAYS_INLINE static inline uint16_t tu_u32_high16(uint32_t u32) { return (uint16_t) (u32 >> 16); }
+TU_ATTR_ALWAYS_INLINE static inline uint16_t tu_u32_low16 (uint32_t u32) { return (uint16_t) (u32 & 0x0000ffffu); }
+
 TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_u16_high(uint16_t u16) { return TU_U16_HIGH(u16); }
 TU_ATTR_ALWAYS_INLINE static inline uint8_t tu_u16_low (uint16_t u16) { return TU_U16_LOW(u16); }
 

src/device/dcd.h

@@ -153,7 +153,7 @@ bool dcd_edpt_xfer            (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer
 // This API is optional, may be useful for register-based for transferring data.
 bool dcd_edpt_xfer_fifo       (uint8_t rhport, uint8_t ep_addr, tu_fifo_t * ff, uint16_t total_bytes) TU_ATTR_WEAK;
 
-// Stall endpoint
+// Stall endpoint, any queuing transfer should be removed from endpoint
 void dcd_edpt_stall           (uint8_t rhport, uint8_t ep_addr);
 
 // clear stall, data toggle is also reset to DATA0

src/portable/raspberrypi/rp2040/dcd_rp2040.c

@@ -145,8 +145,7 @@ static void hw_endpoint_init(uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t t
 
   if ( num == 0 )
   {
-    // EP0 has no endpoint control register because
+    // EP0 has no endpoint control register because the buffer offsets are fixed
-    // the buffer offsets are fixed
     ep->endpoint_control = NULL;
 
     // Buffer offset is fixed (also double buffered)
@@ -178,7 +177,7 @@ static void hw_endpoint_xfer(uint8_t ep_addr, uint8_t *buffer, uint16_t total_by
 static void hw_handle_buff_status(void)
 {
     uint32_t remaining_buffers = usb_hw->buf_status;
-    pico_trace("buf_status 0x%08x\n", remaining_buffers);
+    pico_trace("buf_status = 0x%08x\n", remaining_buffers);
     uint bit = 1u;
     for (uint i = 0; remaining_buffers && i < USB_MAX_ENDPOINTS * 2; i++)
     {
@@ -186,8 +185,10 @@ static void hw_handle_buff_status(void)
         {
             // clear this in advance
             usb_hw_clear->buf_status = bit;
+
             // IN transfer for even i, OUT transfer for odd i
             struct hw_endpoint *ep = hw_endpoint_get_by_num(i >> 1u, !(i & 1u));
+
             // Continue xfer
             bool done = hw_endpoint_xfer_continue(ep);
             if (done)
@@ -325,7 +326,6 @@ static void dcd_rp2040_irq(void)
 
 void dcd_init (uint8_t rhport)
 {
-  pico_trace("dcd_init %d\n", rhport);
   assert(rhport == 0);
 
   // Reset hardware to default state
@@ -370,7 +370,6 @@ void dcd_int_disable(uint8_t rhport)
 
 void dcd_set_address (uint8_t rhport, uint8_t dev_addr)
 {
-  pico_trace("dcd_set_address %d %d\n", rhport, dev_addr);
   assert(rhport == 0);
 
   // Can't set device address in hardware until status xfer has complete
@@ -412,7 +411,6 @@ void dcd_edpt0_status_complete(uint8_t rhport, tusb_control_request_t const * re
        request->bmRequestType_bit.type == TUSB_REQ_TYPE_STANDARD &&
        request->bRequest == TUSB_REQ_SET_ADDRESS )
   {
-    pico_trace("Set HW address %d\n", request->wValue);
     usb_hw->dev_addr_ctrl = (uint8_t) request->wValue;
   }
 
@@ -429,7 +427,7 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * desc_edpt)
 void dcd_edpt_close_all (uint8_t rhport)
 {
   (void) rhport;
-  // TODO implement dcd_edpt_close_all()
+  // reset_all_endpoints(); TODO double check endpoint control
 }
 
 bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t total_bytes)
@@ -441,8 +439,7 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t t
 
 void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr)
 {
-  pico_trace("dcd_edpt_stall %02x\n", ep_addr);
+  (void) rhport;
-  assert(rhport == 0);
 
   if ( tu_edpt_number(ep_addr) == 0 )
   {
@@ -452,22 +449,22 @@ void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr)
 
   struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr);
 
-  // TODO check with double buffered
+  // stall and clear current pending buffer
-  _hw_endpoint_buffer_control_set_mask32(ep, USB_BUF_CTRL_STALL);
+  // may need to use EP_ABORT
+  _hw_endpoint_buffer_control_set_value32(ep, USB_BUF_CTRL_STALL);
 }
 
 void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr)
 {
-  pico_trace("dcd_edpt_clear_stall %02x\n", ep_addr);
+  (void) rhport;
-  assert(rhport == 0);
 
   if (tu_edpt_number(ep_addr))
   {
     struct hw_endpoint *ep = hw_endpoint_get_by_addr(ep_addr);
 
-    // clear stall also reset toggle to DATA0
+    // clear stall also reset toggle to DATA0, ready for next transfer
-    // TODO check with double buffered
+    ep->next_pid = 0;
-    _hw_endpoint_buffer_control_clear_mask32(ep, USB_BUF_CTRL_STALL | USB_BUF_CTRL_DATA1_PID);
+    _hw_endpoint_buffer_control_clear_mask32(ep, USB_BUF_CTRL_STALL);
   }
 }
 

src/portable/raspberrypi/rp2040/hcd_rp2040.c

@@ -201,7 +201,6 @@ static void hcd_rp2040_irq(void)
     {
         handled |= USB_INTS_BUFF_STATUS_BITS;
         TU_LOG(2, "Buffer complete\n");
-        // print_bufctrl32(*epx.buffer_control);
         hw_handle_buff_status();
     }
 
@@ -231,7 +230,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);
+        TU_LOG(3, "  Seq Error: [0] = 0x%04u  [1] = 0x%04x\r\n", tu_u32_low16(*epx.buffer_control), tu_u32_high16(*epx.buffer_control));
         panic("Data Seq Error \n");
     }
 

src/portable/raspberrypi/rp2040/rp2040_usb.c

@@ -67,7 +67,6 @@ void rp2040_usb_init(void)
 
 void hw_endpoint_reset_transfer(struct hw_endpoint *ep)
 {
-  ep->stalled = false;
   ep->active = false;
   ep->remaining_len = 0;
   ep->xferred_len = 0;
@@ -171,8 +170,7 @@ static void _hw_endpoint_start_next_buffer(struct hw_endpoint *ep)
 
   *ep->endpoint_control = ep_ctrl;
 
-  TU_LOG(3, "Prepare Buffer Control:\r\n");
+  TU_LOG(3, "  Prepare BufCtrl: [0] = 0x%04u  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
-  print_bufctrl32(buf_ctrl);
 
   // Finally, write to buffer_control which will trigger the transfer
   // the next time the controller polls this dpram address
@@ -231,7 +229,7 @@ static uint16_t sync_ep_buffer(struct hw_endpoint *ep, uint8_t buf_id)
   // Short packet
   if (xferred_bytes < ep->wMaxPacketSize)
   {
-    pico_trace("Short rx transfer on buffer %d with %u bytes\n", buf_id, xferred_bytes);
+    pico_trace("  Short packet on buffer %d with %u bytes\n", buf_id, xferred_bytes);
     // Reduce total length as this is last packet
     ep->remaining_len = 0;
   }
@@ -245,8 +243,7 @@ static void _hw_endpoint_xfer_sync (struct hw_endpoint *ep)
   // after a buff status interrupt
 
   uint32_t buf_ctrl = _hw_endpoint_buffer_control_get_value32(ep);
-  TU_LOG(3, "_hw_endpoint_xfer_sync:\r\n");
+  TU_LOG(3, "  Sync BufCtrl: [0] = 0x%04u  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
-  print_bufctrl32(buf_ctrl);
 
   // always sync buffer 0
   uint16_t buf0_bytes = sync_ep_buffer(ep, 0);
@@ -284,7 +281,7 @@ static void _hw_endpoint_xfer_sync (struct hw_endpoint *ep)
       usb_hw->abort &= ~TU_BIT(ep_id);
 
       TU_LOG(3, "----SHORT PACKET buffer0 on EP %02X:\r\n", ep->ep_addr);
-      print_bufctrl32(buf_ctrl);
+      TU_LOG(3, "  BufCtrl: [0] = 0x%04u  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
 #endif
     }
   }

src/portable/raspberrypi/rp2040/rp2040_usb.h

@@ -42,9 +42,6 @@ struct hw_endpoint
     // Buffer pointer in usb dpram
     uint8_t *hw_data_buf;
 
-    // Have we been stalled TODO remove later
-    bool stalled;
-
     // Current transfer information
     bool active;
     uint16_t remaining_len;
@@ -96,52 +93,4 @@ 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");
-
-#if CFG_TUSB_DEBUG >= 3
-static inline void print_bufctrl16(uint32_t u16)
-{
-  rp2040_buffer_control_t bufctrl = {
-      .u16 = u16
-  };
-
-  TU_LOG(3, "len = %u, available = %u, full = %u, last = %u, stall = %u, reset = %u, toggle = %u\r\n",
-         bufctrl.xfer_len, bufctrl.available, bufctrl.full, bufctrl.last_buf, bufctrl.stall, bufctrl.reset_bufsel, bufctrl.data_toggle);
-}
-
-static inline void print_bufctrl32(uint32_t u32)
-{
-  uint16_t u16;
-
-  u16 = u32 >> 16;
-  TU_LOG(3, "  Buffer Control 1 0x%x: ", u16);
-  print_bufctrl16(u16);
-
-  u16 = u32 & 0x0000ffff;
-  TU_LOG(3, "  Buffer Control 0 0x%x: ", u16);
-  print_bufctrl16(u16);
-}
-
-#else
-
-#define print_bufctrl16(u16)
-#define print_bufctrl32(u32)
-
-#endif
-
 #endif