Merge branch 'master' into rp2040_warning

2 files changed, 89 insertions, 81 deletions

diff --git a/src/portable/raspberrypi/rp2040/hcd_rp2040.c b/src/portable/raspberrypi/rp2040/hcd_rp2040.c
index 67d2de254..e9118c6a3 100644
--- a/src/portable/raspberrypi/rp2040/hcd_rp2040.c
+++ b/src/portable/raspberrypi/rp2040/hcd_rp2040.c
@@ -104,6 +104,8 @@ static void __tusb_irq_path_func(hw_xfer_complete)(struct hw_endpoint *ep, xfer_
 static void __tusb_irq_path_func(_handle_buff_status_bit)(uint bit, struct hw_endpoint *ep)
 {
     usb_hw_clear->buf_status = bit;
+    // EP may have been stalled?
+    assert(ep->active);
     bool done = hw_endpoint_xfer_continue(ep);
     if (done)
     {
@@ -166,6 +168,8 @@ static void __tusb_irq_path_func(hw_trans_complete)(void)
     pico_trace("Sent setup packet\n");
     struct hw_endpoint *ep = &epx;
     assert(ep->active);
+    // Set transferred length to 8 for a setup packet
+    ep->xferred_len = 8;
     hw_xfer_complete(ep, XFER_RESULT_SUCCESS);
   }
   else
@@ -197,6 +201,18 @@ static void __tusb_irq_path_func(hcd_rp2040_irq)(void)
         usb_hw_clear->sie_status = USB_SIE_STATUS_SPEED_BITS;
     }
 
+    if (status & USB_INTS_STALL_BITS)
+    {
+        // We have rx'd a stall from the device
+        // NOTE THIS SHOULD HAVE PRIORITY OVER BUFF_STATUS
+        // AND TRANS_COMPLETE as the stall is an alternative response
+        // to one of those events
+        pico_trace("Stall REC\n");
+        handled |= USB_INTS_STALL_BITS;
+        usb_hw_clear->sie_status = USB_SIE_STATUS_STALL_REC_BITS;
+        hw_xfer_complete(&epx, XFER_RESULT_STALLED);
+    }
+
     if (status & USB_INTS_BUFF_STATUS_BITS)
     {
         handled |= USB_INTS_BUFF_STATUS_BITS;
@@ -212,15 +228,6 @@ static void __tusb_irq_path_func(hcd_rp2040_irq)(void)
         hw_trans_complete();
     }
 
-    if (status & USB_INTS_STALL_BITS)
-    {
-        // We have rx'd a stall from the device
-        pico_trace("Stall REC\n");
-        handled |= USB_INTS_STALL_BITS;
-        usb_hw_clear->sie_status = USB_SIE_STATUS_STALL_REC_BITS;
-        hw_xfer_complete(&epx, XFER_RESULT_STALLED);
-    }
-
     if (status & USB_INTS_ERROR_RX_TIMEOUT_BITS)
     {
         handled |= USB_INTS_ERROR_RX_TIMEOUT_BITS;
@@ -255,7 +262,7 @@ static struct hw_endpoint *_next_free_interrupt_ep(void)
         if (!ep->configured)
         {
             // Will be configured by _hw_endpoint_init / _hw_endpoint_allocate
-            ep->interrupt_num = i - 1;
+            ep->interrupt_num = (uint8_t) (i - 1);
             return ep;
         }
     }
@@ -290,7 +297,7 @@ static struct hw_endpoint *_hw_endpoint_allocate(uint8_t transfer_type)
     return ep;
 }
 
-static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t ep_addr, uint wMaxPacketSize, uint8_t transfer_type, uint8_t bmInterval)
+static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t transfer_type, uint8_t bmInterval)
 {
     // Already has data buffer, endpoint control, and buffer control allocated at this point
     assert(ep->endpoint_control);
@@ -322,7 +329,10 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t
                   | EP_CTRL_INTERRUPT_PER_BUFFER
                   | (ep->transfer_type << EP_CTRL_BUFFER_TYPE_LSB)
                   | dpram_offset;
-    ep_reg |= bmInterval ? (bmInterval - 1) << EP_CTRL_HOST_INTERRUPT_INTERVAL_LSB : 0;
+    if (bmInterval)
+    {
+      ep_reg |= (uint32_t) ((bmInterval - 1) << EP_CTRL_HOST_INTERRUPT_INTERVAL_LSB);
+    }
     *ep->endpoint_control = ep_reg;
     pico_trace("endpoint control (0x%p) <- 0x%x\n", ep->endpoint_control, ep_reg);
     ep->configured = true;
@@ -334,7 +344,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t
         // device address
         // endpoint number / direction
         // preamble
-        uint32_t reg = dev_addr | (num << USB_ADDR_ENDP1_ENDPOINT_LSB);
+        uint32_t reg = (uint32_t) (dev_addr | (num << USB_ADDR_ENDP1_ENDPOINT_LSB));
 
         if (dir == TUSB_DIR_OUT)
         {
@@ -360,39 +370,41 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t dev_addr, uint8_t
 //--------------------------------------------------------------------+
 bool hcd_init(uint8_t rhport)
 {
-    pico_trace("hcd_init %d\n", rhport);
-    assert(rhport == 0);
+  (void) rhport;
+  pico_trace("hcd_init %d\n", rhport);
+  assert(rhport == 0);
 
-    // Reset any previous state
-    rp2040_usb_init();
+  // Reset any previous state
+  rp2040_usb_init();
 
-    // Force VBUS detect to always present, for now we assume vbus is always provided (without using VBUS En)
-    usb_hw->pwr = USB_USB_PWR_VBUS_DETECT_BITS | USB_USB_PWR_VBUS_DETECT_OVERRIDE_EN_BITS;
+  // Force VBUS detect to always present, for now we assume vbus is always provided (without using VBUS En)
+  usb_hw->pwr = USB_USB_PWR_VBUS_DETECT_BITS | USB_USB_PWR_VBUS_DETECT_OVERRIDE_EN_BITS;
 
-    irq_add_shared_handler(USBCTRL_IRQ, hcd_rp2040_irq, PICO_SHARED_IRQ_HANDLER_HIGHEST_ORDER_PRIORITY);
+  irq_add_shared_handler(USBCTRL_IRQ, hcd_rp2040_irq, PICO_SHARED_IRQ_HANDLER_HIGHEST_ORDER_PRIORITY);
 
-    // clear epx and interrupt eps
-    memset(&ep_pool, 0, sizeof(ep_pool));
+  // clear epx and interrupt eps
+  memset(&ep_pool, 0, sizeof(ep_pool));
 
-    // Enable in host mode with SOF / Keep alive on
-    usb_hw->main_ctrl = USB_MAIN_CTRL_CONTROLLER_EN_BITS | USB_MAIN_CTRL_HOST_NDEVICE_BITS;
-    usb_hw->sie_ctrl = SIE_CTRL_BASE;
-    usb_hw->inte = USB_INTE_BUFF_STATUS_BITS      | 
-                   USB_INTE_HOST_CONN_DIS_BITS    | 
-                   USB_INTE_HOST_RESUME_BITS      | 
-                   USB_INTE_STALL_BITS            | 
-                   USB_INTE_TRANS_COMPLETE_BITS   |
-                   USB_INTE_ERROR_RX_TIMEOUT_BITS |
-                   USB_INTE_ERROR_DATA_SEQ_BITS   ;
+  // Enable in host mode with SOF / Keep alive on
+  usb_hw->main_ctrl = USB_MAIN_CTRL_CONTROLLER_EN_BITS | USB_MAIN_CTRL_HOST_NDEVICE_BITS;
+  usb_hw->sie_ctrl = SIE_CTRL_BASE;
+  usb_hw->inte = USB_INTE_BUFF_STATUS_BITS      |
+                 USB_INTE_HOST_CONN_DIS_BITS    |
+                 USB_INTE_HOST_RESUME_BITS      |
+                 USB_INTE_STALL_BITS            |
+                 USB_INTE_TRANS_COMPLETE_BITS   |
+                 USB_INTE_ERROR_RX_TIMEOUT_BITS |
+                 USB_INTE_ERROR_DATA_SEQ_BITS   ;
 
-    return true;
+  return true;
 }
 
 void hcd_port_reset(uint8_t rhport)
 {
-    pico_trace("hcd_port_reset\n");
-    assert(rhport == 0);
-    // TODO: Nothing to do here yet. Perhaps need to reset some state?
+  (void) rhport;
+  pico_trace("hcd_port_reset\n");
+  assert(rhport == 0);
+  // TODO: Nothing to do here yet. Perhaps need to reset some state?
 }
 
 void hcd_port_reset_end(uint8_t rhport)
@@ -402,25 +414,27 @@ void hcd_port_reset_end(uint8_t rhport)
 
 bool hcd_port_connect_status(uint8_t rhport)
 {
-    pico_trace("hcd_port_connect_status\n");
-    assert(rhport == 0);
-    return usb_hw->sie_status & USB_SIE_STATUS_SPEED_BITS;
+  (void) rhport;
+  pico_trace("hcd_port_connect_status\n");
+  assert(rhport == 0);
+  return usb_hw->sie_status & USB_SIE_STATUS_SPEED_BITS;
 }
 
 tusb_speed_t hcd_port_speed_get(uint8_t rhport)
 {
-    assert(rhport == 0);
-    // TODO: Should enumval this register
-    switch (dev_speed())
-    {
-        case 1:
-            return TUSB_SPEED_LOW;
-        case 2:
-            return TUSB_SPEED_FULL;
-        default:
-            panic("Invalid speed\n");
-            return TUSB_SPEED_INVALID;
-    }
+  (void) rhport;
+  assert(rhport == 0);
+  // TODO: Should enumval this register
+  switch (dev_speed())
+  {
+      case 1:
+          return TUSB_SPEED_LOW;
+      case 2:
+          return TUSB_SPEED_FULL;
+      default:
+          panic("Invalid speed\n");
+          return TUSB_SPEED_INVALID;
+  }
 }
 
 // Close all opened endpoint belong to this device
@@ -458,15 +472,17 @@ uint32_t hcd_frame_number(uint8_t rhport)
 
 void hcd_int_enable(uint8_t rhport)
 {
-    assert(rhport == 0);
-    irq_set_enabled(USBCTRL_IRQ, true);
+  (void) rhport;
+  assert(rhport == 0);
+  irq_set_enabled(USBCTRL_IRQ, true);
 }
 
 void hcd_int_disable(uint8_t rhport)
 {
-    // todo we should check this is disabling from the correct core; note currently this is never called
-    assert(rhport == 0);
-    irq_set_enabled(USBCTRL_IRQ, false);
+  (void) rhport;
+  // todo we should check this is disabling from the correct core; note currently this is never called
+  assert(rhport == 0);
+  irq_set_enabled(USBCTRL_IRQ, false);
 }
 
 //--------------------------------------------------------------------+
@@ -481,6 +497,7 @@ bool hcd_edpt_open(uint8_t rhport, uint8_t dev_addr, tusb_desc_endpoint_t const
 
     // Allocated differently based on if it's an interrupt endpoint or not
     struct hw_endpoint *ep = _hw_endpoint_allocate(ep_desc->bmAttributes.xfer);
+    TU_ASSERT(ep);
 
     _hw_endpoint_init(ep,
         dev_addr,
@@ -503,7 +520,10 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t *
 
     // Get appropriate ep. Either EPX or interrupt endpoint
     struct hw_endpoint *ep = get_dev_ep(dev_addr, ep_addr);
-    assert(ep);
+    TU_ASSERT(ep);
+
+    // EP should be inactive
+    assert(!ep->active);
 
     // Control endpoint can change direction 0x00 <-> 0x80
     if ( ep_addr != ep->ep_addr )
@@ -522,7 +542,7 @@ bool hcd_edpt_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr, uint8_t *
 
         // That has set up buffer control, endpoint control etc
         // for host we have to initiate the transfer
-        usb_hw->dev_addr_ctrl = dev_addr | (ep_num << USB_ADDR_ENDP_ENDPOINT_LSB);
+        usb_hw->dev_addr_ctrl = (uint32_t) (dev_addr | (ep_num << USB_ADDR_ENDP_ENDPOINT_LSB));
 
         uint32_t flags = USB_SIE_CTRL_START_TRANS_BITS | SIE_CTRL_BASE |
                          (ep_dir ? USB_SIE_CTRL_RECEIVE_DATA_BITS : USB_SIE_CTRL_SEND_DATA_BITS);
@@ -543,14 +563,17 @@ bool hcd_setup_send(uint8_t rhport, uint8_t dev_addr, uint8_t const setup_packet
     (void) rhport;
 
     // Copy data into setup packet buffer
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Warray-bounds"
-#pragma GCC diagnostic ignored "-Wstringop-overflow"
-    memcpy((void*)&usbh_dpram->setup_packet[0], setup_packet, 8);
-#pragma GCC diagnostic pop
+    for(uint8_t i=0; i<8; i++)
+    {
+      usbh_dpram->setup_packet[i] = setup_packet[i];
+    }
 
     // Configure EP0 struct with setup info for the trans complete
     struct hw_endpoint *ep = _hw_endpoint_allocate(0);
+    TU_ASSERT(ep);
+
+    // EPX should be inactive
+    assert(!ep->active);
 
     // EP0 out
     _hw_endpoint_init(ep, dev_addr, 0x00, ep->wMaxPacketSize, 0, 0);
@@ -571,21 +594,6 @@ bool hcd_setup_send(uint8_t rhport, uint8_t dev_addr, uint8_t const setup_packet
     return true;
 }
 
-
-//bool hcd_edpt_busy(uint8_t dev_addr, uint8_t ep_addr)
-//{
-//    // EPX is shared, so multiple device addresses and endpoint addresses share that
-//    // so if any transfer is active on epx, we are busy. Interrupt endpoints have their own
-//    // EPX so ep->active will only be busy if there is a pending transfer on that interrupt endpoint
-//    // on that device
-//    pico_trace("hcd_edpt_busy dev addr %d ep_addr 0x%x\n", dev_addr, ep_addr);
-//    struct hw_endpoint *ep = get_dev_ep(dev_addr, ep_addr);
-//    assert(ep);
-//    bool busy = ep->active;
-//    pico_trace("busy == %d\n", busy);
-//    return busy;
-//}
-
 bool hcd_edpt_clear_stall(uint8_t dev_addr, uint8_t ep_addr)
 {
     (void) dev_addr;
diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c
index f93568f0c..49be90167 100644
--- a/src/portable/raspberrypi/rp2040/rp2040_usb.c
+++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c
@@ -174,7 +174,7 @@ static void __tusb_irq_path_func(_hw_endpoint_start_next_buffer)(struct hw_endpo
 
   *ep->endpoint_control = ep_ctrl;
 
-  TU_LOG(3, "  Prepare BufCtrl: [0] = 0x%04u  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
+  TU_LOG(3, "  Prepare BufCtrl: [0] = 0x%04x  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
 
   // Finally, write to buffer_control which will trigger the transfer
   // the next time the controller polls this dpram address
@@ -247,7 +247,7 @@ static void __tusb_irq_path_func(_hw_endpoint_xfer_sync) (struct hw_endpoint *ep
   // after a buff status interrupt
 
   uint32_t __unused buf_ctrl = _hw_endpoint_buffer_control_get_value32(ep);
-  TU_LOG(3, "  Sync BufCtrl: [0] = 0x%04u  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
+  TU_LOG(3, "  Sync BufCtrl: [0] = 0x%04x  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
 
   // always sync buffer 0
   uint16_t buf0_bytes = sync_ep_buffer(ep, 0);
@@ -285,7 +285,7 @@ static void __tusb_irq_path_func(_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);
-      TU_LOG(3, "  BufCtrl: [0] = 0x%04u  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
+      TU_LOG(3, "  BufCtrl: [0] = 0x%04x  [1] = 0x%04x\r\n", tu_u32_low16(buf_ctrl), tu_u32_high16(buf_ctrl));
 #endif
     }
   }