This is an automated email from the ASF dual-hosted git repository. xiaoxiang781216 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit a5431319bb1eba891690be1f7d34b80b585555e8 Author: Justin Hammond <[email protected]> AuthorDate: Wed Aug 5 23:31:26 2026 +0800 drivers/usbhost: Make xHCI asynchronous transfers deliver their data. Submitting an asynchronous transfer refused any buffer needing a cache line stand-in, and that test also refuses every buffer whose length is not a whole number of cache lines, which an interrupt transfer's rarely is: a HID keyboard reads eight bytes. Every submission returned -EFAULT before a descriptor was written, and a class driver resubmitting from its completion callback never sees a second chance. The refusal existed because the copy out of a stand-in is done by the blocked caller, and an asynchronous transfer has none. The work queue thread handling the completion will do: a buffer given to DRVR_ASYNCH comes from DRVR_ALLOC, so it is kernel memory reachable from any thread. Use the same stand-in machinery as every other transfer and finish the DMA in the completion, just before the callback. A cancelled transfer returns its stand-in on cancellation. The callback also moves outside the spinlock. It is class driver code that queues work and takes its own locks, and it may now free a stand-in. Whether a completion is synchronous is still decided under the lock, since a posted waiter may be carrying a new transfer immediately. The asynchronous setup now records the requested length, as the synchronous setup does. The byte count handed to the callback is worked out from it and the residue, and was previously whatever the endpoint held from an earlier transfer. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond <[email protected]> --- drivers/usbhost/usbhost_xhci.c | 81 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 15 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 5227629d0c5..f332e89b4f9 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -412,9 +412,11 @@ static int xhci_ioc_wait(FAR struct xhci_epinfo_s *epinfo); #ifdef CONFIG_USBHOST_ASYNCH static inline int xhci_ioc_async_setup(FAR struct xhci_rhport_s *rhport, FAR struct xhci_epinfo_s *epinfo, + size_t buflen, usbhost_asynch_t callback, FAR void *arg); -static void xhci_asynch_completion(FAR struct xhci_epinfo_s *epinfo); +static void xhci_asynch_completion(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_epinfo_s *epinfo); #endif static int xhci_control_setup(FAR struct xhci_rhport_s *rhport, FAR struct xhci_epinfo_s *epinfo, @@ -2645,6 +2647,8 @@ static ssize_t xhci_transfer_wait(FAR struct usbhost_xhci_s *priv, * Input Parameters: * epinfo - The IN or OUT endpoint descriptor for the device endpoint on * which the transfer will be performed. + * buflen - The length of the transfer, from which the completion works + * out how much was transferred. * callback - The function to be called when the transfer completes * arg - An arbitrary argument that will be provided with the callback. * @@ -2658,6 +2662,7 @@ static ssize_t xhci_transfer_wait(FAR struct usbhost_xhci_s *priv, static inline int xhci_ioc_async_setup(FAR struct xhci_rhport_s *rhport, FAR struct xhci_epinfo_s *epinfo, + size_t buflen, usbhost_asynch_t callback, FAR void *arg) { @@ -2680,6 +2685,7 @@ static inline int xhci_ioc_async_setup(FAR struct xhci_rhport_s *rhport, epinfo->iocwait = false; /* No synchronous wakeup */ epinfo->status = 0; /* No status yet */ epinfo->xfrd = 0; /* Nothing transferred yet */ + epinfo->buflen = buflen; /* Buffer length */ epinfo->result = -EBUSY; /* Transfer in progress */ epinfo->callback = callback; /* Asynchronous callback */ epinfo->arg = arg; /* Argument that accompanies the callback */ @@ -2694,10 +2700,11 @@ static inline int xhci_ioc_async_setup(FAR struct xhci_rhport_s *rhport, * Name: xhci_asynch_completion * * Description: - * This function is called at the interrupt level when an asynchronous - * transfer completes. It performs the pending callback. + * This function is called from the interrupt work queue when an + * asynchronous transfer completes. It performs the pending callback. * * Input Parameters: + * priv - xHCI private state * epinfo - The IN or OUT endpoint descriptor for the device endpoint on * which the transfer was performed. * @@ -2705,21 +2712,26 @@ static inline int xhci_ioc_async_setup(FAR struct xhci_rhport_s *rhport, * None * * Assumptions: - * - Called from the interrupt level + * - Called from the work queue, without the spinlock held * ****************************************************************************/ -static void xhci_asynch_completion(FAR struct xhci_epinfo_s *epinfo) +static void xhci_asynch_completion(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_epinfo_s *epinfo) { usbhost_asynch_t callback; ssize_t nbytes; FAR void *arg; + irqstate_t flags; int result; - DEBUGASSERT(epinfo != NULL && epinfo->iocwait == false && - epinfo->callback != NULL); + DEBUGASSERT(epinfo != NULL && epinfo->iocwait == false); + + /* Extract and reset the callback info, atomically against a concurrent + * cancellation. + */ - /* Extract and reset the callback info */ + flags = spin_lock_irqsave(&priv->spinlock); callback = epinfo->callback; arg = epinfo->arg; @@ -2731,6 +2743,23 @@ static void xhci_asynch_completion(FAR struct xhci_epinfo_s *epinfo) epinfo->result = OK; epinfo->iocwait = false; + spin_unlock_irqrestore(&priv->spinlock, flags); + + /* A cancellation that got in first has already done the callback */ + + if (callback == NULL) + { + return; + } + + /* Bring back what the controller wrote before anyone reads it. The + * addresses are usable here: a transfer given to DRVR_ASYNCH must use + * memory from DRVR_ALLOC, and that is kernel memory, which this work + * queue thread can reach. + */ + + xhci_dma_finish(epinfo); + /* Then perform the callback. Provide the number of bytes successfully * transferred or the negated errno value in the event of a failure. */ @@ -3166,6 +3195,9 @@ static void xhci_transfer_complete(FAR struct usbhost_xhci_s *priv, uint8_t ep = XHCI_TRB_D2_EP_GET(evt->d2); uint8_t ret = XHCI_TRB_D1_CC_GET(evt->d1); irqstate_t flags; +#ifdef CONFIG_USBHOST_ASYNCH + bool asynch = false; +#endif /* Get EP associated with this transfer */ @@ -3227,17 +3259,32 @@ static void xhci_transfer_complete(FAR struct usbhost_xhci_s *priv, } #ifdef CONFIG_USBHOST_ASYNCH - /* No.. Is there a pending asynchronous transfer? */ + /* No.. Is there a pending asynchronous transfer instead? Decide while + * still holding the lock: the moment the waiter above is posted, the + * endpoint may be given a new transfer, and that one is not complete. + */ - else if (epinfo->callback != NULL) + else { - /* Yes.. perform the callback */ - - xhci_asynch_completion(epinfo); + asynch = epinfo->callback != NULL; } #endif spin_unlock_irqrestore(&priv->spinlock, flags); + +#ifdef CONFIG_USBHOST_ASYNCH + /* The callback runs outside the spinlock: it is class driver code, and + * what it does (queue work, take its own locks) has no business running + * with interrupts masked. + */ + + if (asynch) + { + /* Perform the callback */ + + xhci_asynch_completion(priv, epinfo); + } +#endif } /**************************************************************************** @@ -4674,7 +4721,7 @@ static int xhci_asynch(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep, /* Set the request for the callback well BEFORE initiating the transfer. */ - ret = xhci_ioc_async_setup(rhport, epinfo, callback, arg); + ret = xhci_ioc_async_setup(rhport, epinfo, buflen, callback, arg); if (ret != OK) { goto errout_with_lock; @@ -4814,9 +4861,13 @@ static int xhci_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) else { - /* Yes.. perform the callback */ + /* Yes.. give back any stand-in buffer, then perform the callback. + * The endpoint has been stopped, so the controller is no longer + * writing into it. + */ DEBUGASSERT(callback != NULL); + xhci_dma_finish(epinfo); callback(arg, -ESHUTDOWN); } #endif
