diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b107f0ab91d2f..2dabc559389a7 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -381,6 +381,7 @@ config ARCH_CHIP_RP2040 select ARM_HAVE_WFE_SEV select ARCH_BOARD_COMMON select ARCH_HAVE_CUSTOM_TESTSET + select ARCH_USBDEV_STALLQUEUE ---help--- Raspberry Pi RP2040 architectures (ARM dual Cortex-M0+). @@ -395,6 +396,7 @@ config ARCH_CHIP_RP23XX select ARCH_HAVE_FPU select ARCH_HAVE_CUSTOM_TESTSET select ARCH_BOARD_COMMON + select ARCH_USBDEV_STALLQUEUE ---help--- Raspberry Pi RP23XX architectures (ARM dual Cortex-M33 or RISC-V). diff --git a/arch/arm/src/rp2040/rp2040_usbdev.c b/arch/arm/src/rp2040/rp2040_usbdev.c index ceb7a9e965ca6..8daf3b5fc1556 100644 --- a/arch/arm/src/rp2040/rp2040_usbdev.c +++ b/arch/arm/src/rp2040/rp2040_usbdev.c @@ -318,9 +318,6 @@ static void rp2040_update_buffer_control(struct rp2040_ep_s *privep, static int rp2040_epwrite(struct rp2040_ep_s *privep, uint8_t *buf, uint16_t nbytes); static int rp2040_epread(struct rp2040_ep_s *privep, uint16_t nbytes); -static void rp2040_abortrequest(struct rp2040_ep_s *privep, - struct rp2040_req_s *privreq, - int16_t result); static void rp2040_reqcomplete(struct rp2040_ep_s *privep, int16_t result); static void rp2040_txcomplete(struct rp2040_ep_s *privep); static int rp2040_wrrequest(struct rp2040_ep_s *privep); @@ -586,30 +583,6 @@ static int rp2040_epread(struct rp2040_ep_s *privep, uint16_t nbytes) return OK; } -/**************************************************************************** - * Name: rp2040_abortrequest - * - * Description: - * Discard a request - * - ****************************************************************************/ - -static void rp2040_abortrequest(struct rp2040_ep_s *privep, - struct rp2040_req_s *privreq, - int16_t result) -{ - usbtrace(TRACE_DEVERROR(RP2040_TRACEERR_REQABORTED), - (uint16_t)privep->epphy); - - /* Save the result in the request structure */ - - privreq->req.result = result; - - /* Callback to the request completion handler */ - - privreq->req.callback(&privep->ep, &privreq->req); -} - /**************************************************************************** * Name: rp2040_reqcomplete * @@ -1052,6 +1025,11 @@ static void rp2040_ep0setup(struct rp2040_usbdev_s *priv) } else { + uint8_t response[2]; + + response[0] = 0; + response[1] = 0; + switch (priv->ctrl.type & USB_REQ_RECIPIENT_MASK) { case USB_REQ_RECIPIENT_ENDPOINT: @@ -1068,10 +1046,23 @@ static void rp2040_ep0setup(struct rp2040_usbdev_s *priv) priv->ctrl.type); priv->stalled = true; } + else if (privep->stalled) + { + response[0] = 1; /* Endpoint HALTed */ + } } break; case USB_REQ_RECIPIENT_DEVICE: + usbtrace(TRACE_INTDECODE( + RP2040_TRACEINTID_GETIFDEV), + 0); + if (priv->selfpowered) + { + response[0] = 1 << USB_FEATURE_SELFPOWERED; + } + break; + case USB_REQ_RECIPIENT_INTERFACE: usbtrace(TRACE_INTDECODE( RP2040_TRACEINTID_GETIFDEV), @@ -1087,6 +1078,23 @@ static void rp2040_ep0setup(struct rp2040_usbdev_s *priv) } break; } + + /* Send the two-byte status response. This was previously + * missing entirely: the request was validated but no data + * stage was ever queued, so EP0 NAKed the host's IN + * forever and the control transfer timed out. macOS + * queries GET STATUS (endpoint) on a halted bulk pipe + * before running Bulk-Only reset recovery, so this hole + * broke all mass-storage error recovery on that host. + * (The status stage - the host's zero-length OUT - is + * armed by rp2040_handle_zlp after this handler returns, + * exactly as for class-dispatched IN transfers.) + */ + + if (!priv->stalled) + { + rp2040_epwrite(ep0, response, 2); + } } } break; @@ -1394,7 +1402,16 @@ static bool rp2040_usbintr_buffstat(struct rp2040_usbdev_s *priv) if (privep->in) { - if (!rp2040_rqempty(privep)) + if (privep->epphy != 0 && privep->stalled) + { + /* A completion latched for a transfer that a halt + * just aborted: the request was already canceled by + * rp2040_epstall and any queued requests belong to + * the post-halt phase. Do not attribute the stale + * buffer event to them. + */ + } + else if (!rp2040_rqempty(privep)) { rp2040_txcomplete(privep); } @@ -1694,24 +1711,25 @@ static int rp2040_epsubmit(struct usbdev_ep_s *ep, flags = enter_critical_section(); - if (privep->stalled && privep->in) - { - rp2040_abortrequest(privep, privreq, -EBUSY); - ret = -EBUSY; - } - /* Handle IN (device-to-host) requests */ - else if (privep->in) + if (privep->in) { - /* Add the new request to the request queue for the IN endpoint */ + /* Add the new request to the request queue for the IN endpoint. + * If the endpoint is halted, the request is only queued; it will + * be armed when the halt is cleared (see rp2040_epstall). This + * stall-queuing is required by the mass storage class: the CSW is + * submitted while the bulk IN endpoint is still halted and must + * survive until the host clears the halt and collects it + * (ARCH_USBDEV_STALLQUEUE). + */ bool empty = rp2040_rqempty(privep); rp2040_rqenqueue(privep, privreq); usbtrace(TRACE_INREQQUEUED(privep->epphy), privreq->req.len); - if (empty) + if (empty && !privep->stalled) { rp2040_wrrequest(privep); } @@ -1721,7 +1739,11 @@ static int rp2040_epsubmit(struct usbdev_ep_s *ep, else { - /* Add the new request to the request queue for the OUT endpoint */ + /* Add the new request to the request queue for the OUT endpoint. + * As for IN endpoints, do not arm the hardware while the endpoint + * is halted: arming rewrites the buffer control word and would + * silently clear the STALL bit. + */ bool empty = rp2040_rqempty(privep); @@ -1731,7 +1753,7 @@ static int rp2040_epsubmit(struct usbdev_ep_s *ep, /* This there a incoming data pending the availability of a request? */ - if (empty) + if (empty && !privep->stalled) { ret = rp2040_rdrequest(privep); } @@ -1829,14 +1851,17 @@ static int rp2040_epstall(struct usbdev_ep_s *ep, bool resume) { struct rp2040_ep_s *privep = (struct rp2040_ep_s *)ep; struct rp2040_usbdev_s *priv = privep->dev; + irqstate_t irqflags; irqstate_t flags; + irqflags = enter_critical_section(); flags = spin_lock_irqsave(&priv->lock); if (resume) { usbtrace(TRACE_EPRESUME, privep->epphy); privep->stalled = false; + privep->pending_stall = false; if (privep->epphy == 0) { clrbits_reg32(privep->in ? @@ -1849,8 +1874,29 @@ static int rp2040_epstall(struct usbdev_ep_s *ep, bool resume) ~(RP2040_USBCTRL_DPSRAM_EP_BUFF_CTRL_STALL), 0); + /* Halt clearing resets the data toggle to DATA0 (USB 2.0 9.4.5) */ + privep->next_pid = 0; priv->zlp_stat = RP2040_ZLP_NONE; + + spin_unlock_irqrestore(&priv->lock, flags); + + /* Restart any requests that were queued (but not armed) while the + * endpoint was halted -- e.g. the mass storage CSW that concludes + * a failed command (ARCH_USBDEV_STALLQUEUE). + */ + + if (privep->epphy != 0 && !rp2040_rqempty(privep)) + { + if (privep->in) + { + rp2040_wrrequest(privep); + } + else + { + rp2040_rdrequest(privep); + } + } } else { @@ -1861,18 +1907,32 @@ static int rp2040_epstall(struct usbdev_ep_s *ep, bool resume) /* EP0 IN Transfer ongoing : postpone the stall until the end */ privep->pending_stall = true; + priv->zlp_stat = RP2040_ZLP_NONE; + spin_unlock_irqrestore(&priv->lock, flags); } else { /* Stall immediately */ rp2040_epstall_exec_nolock(ep); - } + priv->zlp_stat = RP2040_ZLP_NONE; + spin_unlock_irqrestore(&priv->lock, flags); + + /* Terminate any IN transfer that the halt just aborted: its + * hardware buffer was disarmed by the STALL write above and + * would otherwise never complete. Anything the class submits + * from the completion callbacks (or later, e.g. the mass + * storage CSW) stays queued until the host clears the halt. + */ - priv->zlp_stat = RP2040_ZLP_NONE; + if (privep->epphy != 0 && privep->in && !rp2040_rqempty(privep)) + { + rp2040_cancelrequests(privep); + } + } } - spin_unlock_irqrestore(&priv->lock, flags); + leave_critical_section(irqflags); return OK; } diff --git a/arch/arm/src/rp23xx/rp23xx_usbdev.c b/arch/arm/src/rp23xx/rp23xx_usbdev.c index cfece8936bf78..e8923dd085481 100644 --- a/arch/arm/src/rp23xx/rp23xx_usbdev.c +++ b/arch/arm/src/rp23xx/rp23xx_usbdev.c @@ -318,9 +318,6 @@ static void rp23xx_update_buffer_control(struct rp23xx_ep_s *privep, static int rp23xx_epwrite(struct rp23xx_ep_s *privep, uint8_t *buf, uint16_t nbytes); static int rp23xx_epread(struct rp23xx_ep_s *privep, uint16_t nbytes); -static void rp23xx_abortrequest(struct rp23xx_ep_s *privep, - struct rp23xx_req_s *privreq, - int16_t result); static void rp23xx_reqcomplete(struct rp23xx_ep_s *privep, int16_t result); static void rp23xx_txcomplete(struct rp23xx_ep_s *privep); static int rp23xx_wrrequest(struct rp23xx_ep_s *privep); @@ -585,30 +582,6 @@ static int rp23xx_epread(struct rp23xx_ep_s *privep, uint16_t nbytes) return OK; } -/**************************************************************************** - * Name: rp23xx_abortrequest - * - * Description: - * Discard a request - * - ****************************************************************************/ - -static void rp23xx_abortrequest(struct rp23xx_ep_s *privep, - struct rp23xx_req_s *privreq, - int16_t result) -{ - usbtrace(TRACE_DEVERROR(RP23XX_TRACEERR_REQABORTED), - (uint16_t)privep->epphy); - - /* Save the result in the request structure */ - - privreq->req.result = result; - - /* Callback to the request completion handler */ - - privreq->req.callback(&privep->ep, &privreq->req); -} - /**************************************************************************** * Name: rp23xx_reqcomplete * @@ -1051,6 +1024,11 @@ static void rp23xx_ep0setup(struct rp23xx_usbdev_s *priv) } else { + uint8_t response[2]; + + response[0] = 0; + response[1] = 0; + switch (priv->ctrl.type & USB_REQ_RECIPIENT_MASK) { case USB_REQ_RECIPIENT_ENDPOINT: @@ -1067,10 +1045,23 @@ static void rp23xx_ep0setup(struct rp23xx_usbdev_s *priv) priv->ctrl.type); priv->stalled = true; } + else if (privep->stalled) + { + response[0] = 1; /* Endpoint HALTed */ + } } break; case USB_REQ_RECIPIENT_DEVICE: + usbtrace(TRACE_INTDECODE( + RP23XX_TRACEINTID_GETIFDEV), + 0); + if (priv->selfpowered) + { + response[0] = 1 << USB_FEATURE_SELFPOWERED; + } + break; + case USB_REQ_RECIPIENT_INTERFACE: usbtrace(TRACE_INTDECODE( RP23XX_TRACEINTID_GETIFDEV), @@ -1086,6 +1077,23 @@ static void rp23xx_ep0setup(struct rp23xx_usbdev_s *priv) } break; } + + /* Send the two-byte status response. This was previously + * missing entirely: the request was validated but no data + * stage was ever queued, so EP0 NAKed the host's IN + * forever and the control transfer timed out. macOS + * queries GET STATUS (endpoint) on a halted bulk pipe + * before running Bulk-Only reset recovery, so this hole + * broke all mass-storage error recovery on that host. + * (The status stage - the host's zero-length OUT - is + * armed by rp23xx_handle_zlp after this handler returns, + * exactly as for class-dispatched IN transfers.) + */ + + if (!priv->stalled) + { + rp23xx_epwrite(ep0, response, 2); + } } } break; @@ -1394,7 +1402,16 @@ static bool rp23xx_usbintr_buffstat(struct rp23xx_usbdev_s *priv) if (privep->in) { - if (!rp23xx_rqempty(privep)) + if (privep->epphy != 0 && privep->stalled) + { + /* A completion latched for a transfer that a halt + * just aborted: the request was already canceled by + * rp23xx_epstall and any queued requests belong to + * the post-halt phase. Do not attribute the stale + * buffer event to them. + */ + } + else if (!rp23xx_rqempty(privep)) { rp23xx_txcomplete(privep); } @@ -1696,24 +1713,25 @@ static int rp23xx_epsubmit(struct usbdev_ep_s *ep, flags = enter_critical_section(); - if (privep->stalled && privep->in) - { - rp23xx_abortrequest(privep, privreq, -EBUSY); - ret = -EBUSY; - } - /* Handle IN (device-to-host) requests */ - else if (privep->in) + if (privep->in) { - /* Add the new request to the request queue for the IN endpoint */ + /* Add the new request to the request queue for the IN endpoint. + * If the endpoint is halted, the request is only queued; it will + * be armed when the halt is cleared (see rp23xx_epstall). This + * stall-queuing is required by the mass storage class: the CSW is + * submitted while the bulk IN endpoint is still halted and must + * survive until the host clears the halt and collects it + * (ARCH_USBDEV_STALLQUEUE). + */ bool empty = rp23xx_rqempty(privep); rp23xx_rqenqueue(privep, privreq); usbtrace(TRACE_INREQQUEUED(privep->epphy), privreq->req.len); - if (empty) + if (empty && !privep->stalled) { rp23xx_wrrequest(privep); } @@ -1723,7 +1741,11 @@ static int rp23xx_epsubmit(struct usbdev_ep_s *ep, else { - /* Add the new request to the request queue for the OUT endpoint */ + /* Add the new request to the request queue for the OUT endpoint. + * As for IN endpoints, do not arm the hardware while the endpoint + * is halted: arming rewrites the buffer control word and would + * silently clear the STALL bit. + */ bool empty = rp23xx_rqempty(privep); @@ -1733,7 +1755,7 @@ static int rp23xx_epsubmit(struct usbdev_ep_s *ep, /* This there a incoming data pending the availability of a request? */ - if (empty) + if (empty && !privep->stalled) { ret = rp23xx_rdrequest(privep); } @@ -1818,14 +1840,17 @@ static int rp23xx_epstall(struct usbdev_ep_s *ep, bool resume) { struct rp23xx_ep_s *privep = (struct rp23xx_ep_s *)ep; struct rp23xx_usbdev_s *priv = privep->dev; + irqstate_t irqflags; irqstate_t flags; + irqflags = enter_critical_section(); flags = spin_lock_irqsave(&g_usbdev.lock); if (resume) { usbtrace(TRACE_EPRESUME, privep->epphy); privep->stalled = false; + privep->pending_stall = false; if (privep->epphy == 0) { clrbits_reg32(privep->in ? @@ -1838,8 +1863,29 @@ static int rp23xx_epstall(struct usbdev_ep_s *ep, bool resume) ~(RP23XX_USBCTRL_DPSRAM_EP_BUFF_CTRL_STALL), 0); + /* Halt clearing resets the data toggle to DATA0 (USB 2.0 9.4.5) */ + privep->next_pid = 0; priv->zlp_stat = RP23XX_ZLP_NONE; + + spin_unlock_irqrestore(&g_usbdev.lock, flags); + + /* Restart any requests that were queued (but not armed) while the + * endpoint was halted -- e.g. the mass storage CSW that concludes + * a failed command (ARCH_USBDEV_STALLQUEUE). + */ + + if (privep->epphy != 0 && !rp23xx_rqempty(privep)) + { + if (privep->in) + { + rp23xx_wrrequest(privep); + } + else + { + rp23xx_rdrequest(privep); + } + } } else { @@ -1850,18 +1896,32 @@ static int rp23xx_epstall(struct usbdev_ep_s *ep, bool resume) /* EP0 IN Transfer ongoing : postpone the stall until the end */ privep->pending_stall = true; + priv->zlp_stat = RP23XX_ZLP_NONE; + spin_unlock_irqrestore(&g_usbdev.lock, flags); } else { /* Stall immediately */ rp23xx_epstall_exec(ep); - } + priv->zlp_stat = RP23XX_ZLP_NONE; + spin_unlock_irqrestore(&g_usbdev.lock, flags); + + /* Terminate any IN transfer that the halt just aborted: its + * hardware buffer was disarmed by the STALL write above and + * would otherwise never complete. Anything the class submits + * from the completion callbacks (or later, e.g. the mass + * storage CSW) stays queued until the host clears the halt. + */ - priv->zlp_stat = RP23XX_ZLP_NONE; + if (privep->epphy != 0 && privep->in && !rp23xx_rqempty(privep)) + { + rp23xx_cancelrequests(privep); + } + } } - spin_unlock_irqrestore(&g_usbdev.lock, flags); + leave_critical_section(irqflags); return OK; }