From 08e971f181f90a7c5c3cc0f5eaccf598bd49af0e Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:09:58 +0800 Subject: [PATCH 01/26] drivers/usbhost: Attach the xHCI interrupt after the controller starts. The handler defers to a worker that walks the event ring, and the ring is not allocated until the controller is started, several steps later. A controller left running by a boot loader has an interrupt pending as soon as the line is enabled, so attaching earlier is a race with nothing able to answer it. Attach after the start, and clear USBSTS and the interrupter pending flag once the handler is in place: a message signalled interrupt is sent on the flag's clear to set transition, so a flag raised before the handler existed would never produce another. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 37 ++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index ff628cf31fd10..efea0fb89ddcc 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -4545,14 +4545,6 @@ static int xhci_hw_initialize(FAR struct usbhost_xhci_s *priv) goto errout; } - /* Configure interrupts */ - - ret = xhci_irq_initialize(priv); - if (ret < 0) - { - goto errout; - } - /* Halt controller */ ret = xhci_ctrl_halt(priv); @@ -4697,6 +4689,7 @@ xhci_initialize(FAR const char *name, uintptr_t base, { FAR struct usbhost_conn_xhci_s *conn = NULL; FAR struct usbhost_xhci_s *priv = NULL; + uint32_t regval; int ret; DEBUGASSERT(name != NULL && base != 0 && ops != NULL && @@ -4756,6 +4749,34 @@ xhci_initialize(FAR const char *name, uintptr_t base, goto errout; } + /* Take the interrupt only now. + * + * The handler defers to a worker that walks the event ring, and the ring + * does not exist until the controller has been started. A controller + * left running by a boot loader can have an interrupt pending the moment + * the line is enabled, so attaching any earlier is a race with nothing + * to answer it. + */ + + ret = xhci_irq_initialize(priv); + if (ret < 0) + { + uerr("failed to attach interrupt: %d\n", ret); + goto errout; + } + + /* Acknowledge anything the controller raised before the handler was + * attached. A message is sent once, on the transition, so a bit set in + * that window would never produce another. Clear them, so the next + * event is a fresh assertion. + */ + + regval = xhci_oper_getreg(priv, XHCI_USBSTS); + xhci_oper_putreg(priv, XHCI_USBSTS, regval); + + regval = xhci_runt_getreg(priv, XHCI_IMAN(0)); + xhci_runt_putreg(priv, XHCI_IMAN(0), regval | XHCI_IMAN_IP); + #ifdef CONFIG_DEBUG_USB_INFO xhci_dump_mem(priv, "after init"); #endif From ca6502a108ba8cb07a8e6827f5d4a6254f7e6668 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:09:58 +0800 Subject: [PATCH 02/26] drivers/usbhost: Do not disable an xHCI port while probing it. xhci_probe_ports() wrote PORTSC back to clear the change bits, including PED, which is write-one-to-clear. A port that came up enabled, which is what a device attached at power up produces, was switched off by the act of reading it. Mask PED out of the value written back. The port status worker already does this. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index efea0fb89ddcc..50ee8f0f76077 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -1291,8 +1291,13 @@ static void xhci_probe_ports(FAR struct usbhost_xhci_s *priv) portsc = xhci_oper_getreg(priv, XHCI_PORTSC(i)); priv->rhport[i].connected = ((portsc & XHCI_PORTSC_CCS) != 0); - /* Clear status change */ + /* Clear status change, but not PED. Port Enabled/Disabled is + * write-one-to-clear, so writing back what was read disables any + * port that came up enabled, which is what a device attached at + * power up does. + */ + portsc &= ~XHCI_PORTSC_PED; xhci_oper_putreg(priv, XHCI_PORTSC(i), portsc); } } From 7c27394136e43ec70a30f21785161ba79eeb8d3d Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:10:27 +0800 Subject: [PATCH 03/26] drivers/usbhost: Acknowledge xHCI events before walking the ring. The event ring was acknowledged after being walked. An event arriving during the walk sets the pending bit again, and clearing the bit afterwards discards it. Transfers have no timeout, so the transfer that event belonged to waits forever. Acknowledge first. A spurious second pass over an empty ring costs nothing. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 50ee8f0f76077..423090e840dcf 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -2907,6 +2907,20 @@ static void xhci_interrupt_work(FAR void *arg) FAR struct usbhost_xhci_s *priv = arg; uint32_t iman; + /* Acknowledge before walking the ring, not after. An event arriving + * during the walk sets the pending bit again, and clearing after the + * walk discards it. Transfers have no timeout, so the one it belonged + * to would wait forever. + */ + + xhci_oper_putreg(priv, XHCI_USBSTS, priv->pending); + + iman = xhci_runt_getreg(priv, XHCI_IMAN(0)); + if (iman & XHCI_IMAN_IP) + { + xhci_runt_putreg(priv, XHCI_IMAN(0), iman); + } + xhci_events_poll(priv); /* Port Change Detect */ @@ -2939,18 +2953,6 @@ static void xhci_interrupt_work(FAR void *arg) uinfo("Host Controller Error\n"); } - /* ACK interrupts */ - - xhci_oper_putreg(priv, XHCI_USBSTS, priv->pending); - - /* Clear interrupter pending bit */ - - iman = xhci_runt_getreg(priv, XHCI_IMAN(0)); - if (iman & XHCI_IMAN_IP) - { - xhci_runt_putreg(priv, XHCI_IMAN(0), iman); - } - /* Clear pending bits */ priv->pending = 0; From 1d1d21c0b5970d04d3fd21d5c3e7cc2bfd4aa6cb Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:10:44 +0800 Subject: [PATCH 04/26] drivers/usbhost: Silence the xHCI interrupter until its worker has run. The handler read the status, queued the work that would answer it, and returned with the source still asserted. On a level triggered line the interrupt controller sees the condition still true and raises it again at once, so the work that would have cleared it never runs. Mask the interrupter in the handler and let the worker unmask when it is done. The unmask clears the pending flag in the same write, because a message is sent on that flag's clear to set transition and events that arrived while the interrupter was masked have already set it. Clearing opens its own window, so the worker drains the ring again after unmasking and repeats while a drain finds anything; xhci_events_poll() returns how many events it handled for that purpose. A drain that finds nothing is the only state in which no event can have been lost. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 39 +++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 423090e840dcf..ed417a92a528f 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -2804,6 +2804,7 @@ static int xhci_events_poll(FAR struct usbhost_xhci_s *priv) uintptr_t addr; uint8_t type; uint32_t d2; + int count = 0; /* Invalidate event ring */ @@ -2875,6 +2876,7 @@ static int xhci_events_poll(FAR struct usbhost_xhci_s *priv) /* Next event */ + count++; priv->evnt.i++; /* Handle ring wrap */ @@ -2891,7 +2893,7 @@ static int xhci_events_poll(FAR struct usbhost_xhci_s *priv) addr |= XHCI_ERDP_EHB; xhci_runt_putreg_8b(priv, XHCI_ERDP(0), addr); - return OK; + return count; } /**************************************************************************** @@ -2956,6 +2958,29 @@ static void xhci_interrupt_work(FAR void *arg) /* Clear pending bits */ priv->pending = 0; + + /* Let interrupts back in, which the handler masked on its way out, and + * clear the pending flag in the same write. + * + * A message signalled interrupt is sent on the flag's clear to set + * transition; a wire stays asserted while it is set. Events that + * arrived while this interrupter was masked have already set the flag, + * so enabling without clearing leaves a message with nothing to + * transition on, and transfers have no timeout. + * + * Clearing opens its own window: an event delivered between the ring + * going empty and this write is discarded. So drain again, and repeat + * if that drain found anything. A drain that finds nothing is the only + * state in which no event can have been lost. + */ + + do + { + iman = xhci_runt_getreg(priv, XHCI_IMAN(0)); + xhci_runt_putreg(priv, XHCI_IMAN(0), + iman | XHCI_IMAN_IE | XHCI_IMAN_IP); + } + while (xhci_events_poll(priv) > 0); } /**************************************************************************** @@ -2969,11 +2994,23 @@ static void xhci_interrupt_work(FAR void *arg) static int xhci_interrupt(int irq, FAR void *context, FAR void *arg) { FAR struct usbhost_xhci_s *priv = arg; + uint32_t iman; /* Get pending interrupts */ priv->pending = xhci_oper_getreg(priv, XHCI_USBSTS); + /* Silence the interrupter before returning. + * + * Nothing here clears the condition that raised the interrupt; the work + * runs later on a work queue. On a level triggered line the source is + * still asserted on return, so the interrupt re-raises immediately and + * the worker never runs. The worker clears the status and unmasks. + */ + + iman = xhci_runt_getreg(priv, XHCI_IMAN(0)); + xhci_runt_putreg(priv, XHCI_IMAN(0), iman & ~XHCI_IMAN_IE); + /* Handle interrupts in worker */ if (work_available(&priv->work)) From e6d1f6c3508c97c8143942b1e8c1aba1013008b8 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:10:58 +0800 Subject: [PATCH 05/26] drivers/usbhost: Set the xHCI interrupter moderation interval. The interval was left at its reset value of 4000, a millisecond, which is how long the controller waits after an event before reporting it. Every completion paid that, and mass storage spends three transfers on a request. Set it to 160, which is 40us, as Linux does. Zero puts no bound on how often a controller may interrupt: a keyboard on an interrupt endpoint then takes them continuously and occupies a processor. Measured on a DWC3 with a USB 2.0 drive, doorbell to interrupt 986-1021us before and 13-56us after: reading 1MiB before after 512 byte blocks 166 KB/s 775 KB/s 32 KiB blocks 10666 KB/s 18618 KB/s mounting a FAT32 volume: 92.7s before, 21.1s after Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 4 ++++ drivers/usbhost/usbhost_xhci.h | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index ed417a92a528f..2c3d3ad559ae0 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -1141,6 +1141,10 @@ static int xhci_ctrl_start(FAR struct usbhost_xhci_s *priv) xhci_oper_putreg_8b(priv, XHCI_CRCR, up_addrenv_va_to_pa(priv->cmd.ring) | XHCI_CRCR_RCS); + /* Do not sit on completions; see XHCI_IMOD_INTERVAL */ + + xhci_runt_putreg(priv, XHCI_IMOD(0), XHCI_IMOD_DEFAULT); + /* Enable interrupts */ regval = xhci_runt_getreg(priv, XHCI_IMAN(0)); diff --git a/drivers/usbhost/usbhost_xhci.h b/drivers/usbhost/usbhost_xhci.h index 8f940f8a82c42..2fc2aa207aded 100644 --- a/drivers/usbhost/usbhost_xhci.h +++ b/drivers/usbhost/usbhost_xhci.h @@ -315,6 +315,17 @@ #define XHCI_IMOD_IMODI_SHIFT (0) /* Bits 0-15: Interrupt Moderation Interval */ #define XHCI_IMOD_IMODC_SHIFT (16) /* Bits 16-31: Interrupt Moderation Counter */ +/* What to set the moderation interval to, in 250ns units. + * + * The reset default is 4000, a millisecond, which is far too long to wait + * to be told a transfer finished. Zero is too short: it puts no bound on + * how often a controller may interrupt, and a polled device such as a + * keyboard on an interrupt endpoint will then occupy a processor. 160 is + * 40us, which is what Linux uses. + */ + +#define XHCI_IMOD_DEFAULT (160) + /* Event Ring Segment Table Size */ #define XHCI_ERSTS_MASK (0xffff) /* Bit 0-15: Event Ring Segment Table Size */ From 8c427cba8429fb55c2156c9928911583e5e89ec4 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:12:25 +0800 Subject: [PATCH 06/26] drivers/usbhost: Flush the xHCI rings and structures by address. xhci_ctrl_start() published the event ring segment table, the device context base address array and the scratchpad pointers with up_flush_dcache_all(), which an architecture whose cache can only be maintained by address implements as a barrier and nothing more, so none of them reached memory. The controller then reads whatever those addresses held before, which presents as every command timing out with no events arriving. Flush each structure by address. xhci_ring_init() has the same fault from the other direction: it clears a whole ring and flushes only the link entry it writes afterwards, leaving the rest of the clearing in the cache. The controller writes into that memory itself, so a line written back later lands on top of an event somebody is waiting for. Flush the whole ring. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 2c3d3ad559ae0..7b076098a971c 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -808,9 +808,16 @@ static int xhci_ring_init(FAR struct xhci_ring_s *ring, size_t len) ring->len = len; } - /* Reset data in ring */ + /* Reset data in ring. + * + * Clearing dirties every line, and the controller writes into this + * memory itself. Flush now, or a later writeback lands on top of an + * event somebody is waiting for. + */ memset(ring->ring, 0, ring->len * sizeof(struct xhci_trb_s)); + up_flush_dcache((uintptr_t)ring->ring, + (uintptr_t)(ring->ring + ring->len)); /* Fill Link TRB */ @@ -1112,9 +1119,22 @@ static int xhci_ctrl_start(FAR struct usbhost_xhci_s *priv) evnt->size = XHCI_EVENT_MAX; evnt->res = 0; - /* Flush all memory before write to ERDP so xhci sees correct data */ + /* Push the structures the controller is about to be pointed at. + * + * Flush by address: up_flush_dcache_all() is a no-op on architectures + * whose cache can only be maintained by address. + */ - up_flush_dcache_all(); + up_flush_dcache((uintptr_t)priv->pg_erst, + (uintptr_t)priv->pg_erst + + sizeof(struct xhci_event_ring_s) * priv->no_erst); + up_flush_dcache((uintptr_t)priv->pg_ctx, + (uintptr_t)(priv->pg_ctx + priv->no_slots + 1)); + if (priv->pg_sb != NULL) + { + up_flush_dcache((uintptr_t)priv->pg_sb, + (uintptr_t)(priv->pg_sb + priv->no_scratch)); + } xhci_runt_putreg_8b(priv, XHCI_ERDP(0), up_addrenv_va_to_pa(priv->evnt.ring)); @@ -1151,9 +1171,12 @@ static int xhci_ctrl_start(FAR struct usbhost_xhci_s *priv) regval |= XHCI_IMAN_IE; xhci_runt_putreg(priv, XHCI_IMAN(0), regval); - /* Flush all memory once again */ + /* And the command ring, whose last entry was just made to point back at + * its own beginning. + */ - up_flush_dcache_all(); + up_flush_dcache((uintptr_t)priv->cmd.ring, + (uintptr_t)(priv->cmd.ring + XHCI_CMD_MAX)); /* Turn the host controller ON, enable interrupts and system errors */ From 3827a0cacf75d02118954b2da1fda7655ca05711 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:12:59 +0800 Subject: [PATCH 07/26] drivers/usbhost: Chain xHCI TRBs across a 64K boundary. A Normal TRB describes one run of memory that may not cross a 64K boundary, and the block layer hands down whole multi-sector reads whose length is bounded by nothing here. One TRB was programmed regardless, so a long enough transfer, or merely one starting near the wrong side of a boundary, produced a descriptor the controller is entitled to reject or to satisfy in part. Program as many as the run needs, chained, asking for the completion interrupt only on the last so one event still arrives for the transfer. A transfer needing more TRBs than the ring holds is refused. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 53 ++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 7b076098a971c..0c649ce8fc592 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -2365,15 +2365,56 @@ static int xhci_normal_setup(FAR struct xhci_rhport_s *rhport, FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_RHPORT(rhport); struct xhci_trb_s trb; - /* Prepare TRB */ + size_t left; + size_t chunk; + uintptr_t pa; + int n = 0; + + /* One TRB describes one run of memory, and that run may not cross a 64K + * boundary. A longer transfer, or one starting near the wrong side of a + * boundary, becomes several TRBs chained into a single transfer, with + * the interrupt asked for only on the last so that one completion + * arrives for the whole of it. + */ - trb.d0 = up_addrenv_va_to_pa(buffer); - trb.d1 = XHCI_TRB_D1_IRQ_SET(0) | XHCI_TRB_D1_TXLEN_SET(buflen); - trb.d2 = XHCI_TRB_D2_IOC | XHCI_TRB_D2_TYPE_SET(XHCI_TRB_TYPE_NORMAL); + pa = up_addrenv_va_to_pa(buffer); + left = buflen; - /* Add TRBs to ring */ + while (left > 0) + { + chunk = XHCI_TD_LEN_MAX - (pa & (XHCI_TD_LEN_MAX - 1)); + if (chunk > left) + { + chunk = left; + } - xhci_add_trb(priv, &epinfo->td, &trb, 1); + if (++n >= XHCI_TD_MAX) + { + uerr("transfer of %zu needs more TRBs than the ring holds\n", + buflen); + return -EINVAL; + } + + trb.d0 = pa; + trb.d1 = XHCI_TRB_D1_IRQ_SET(0) | XHCI_TRB_D1_TXLEN_SET(chunk); + trb.d2 = XHCI_TRB_D2_TYPE_SET(XHCI_TRB_TYPE_NORMAL); + + left -= chunk; + pa += chunk; + + /* Chain everything but the last, and interrupt only on the last */ + + if (left > 0) + { + trb.d2 |= XHCI_TRB_D2_CH; + } + else + { + trb.d2 |= XHCI_TRB_D2_IOC; + } + + xhci_add_trb(priv, &epinfo->td, &trb, 1); + } /* Trigger transfer */ From 2c52f90980114271b92bbe851873904ce36b0fe3 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:13:14 +0800 Subject: [PATCH 08/26] drivers/usbhost: Maintain the cache over xHCI data buffers. The controller moves every byte itself, so on a machine whose caches are not coherent with it the driver must flush before the controller reads and invalidate before the processor does. Data buffers got no maintenance at all: nothing pushed before an OUT, nothing dropped after an IN. Cache operations act a whole line at a time, which is unsafe for a buffer that does not own its lines: invalidating drops whatever else shares the line, and a writeback lands on top of what the controller has just put there. Mass storage passes a 31 byte command block and a 13 byte status out of its instance structure. Such a buffer is copied through an aligned stand-in; anything large comes from a filesystem or from xhci_ioalloc(), which now rounds its length up as well as aligning its start, so what it returns owns its last line. Whether the controller can reach a buffer at all is asked of the platform through a new dmacapable operation, since it is a property of the system the controller was fitted into rather than of the controller. A platform that does not supply it is taken to accept every address, which is what existing users have. A refused buffer gives -EFAULT, which the FAT filesystem answers by retrying through its own DMA-safe sector buffer. The device output context is also invalidated before the assigned address is read out of it; the controller wrote that address, and reading without invalidating returns whatever the processor had cached. Compiles to nothing where there is no cache to maintain, and dmacapable is NULL on PCI, so the existing user is unaffected. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 226 ++++++++++++++++++++++++++++++++- include/nuttx/usb/xhci.h | 7 + 2 files changed, 231 insertions(+), 2 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 0c649ce8fc592..738c1563e1887 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -27,6 +27,7 @@ #include #include #include +#include #include @@ -34,6 +35,7 @@ #include #include #include +#include #include #include @@ -142,6 +144,9 @@ struct xhci_epinfo_s int result; /* The result of the transfer */ size_t xfrd; /* On completion, will hold the number of bytes transferred */ size_t buflen; /* Buffer length used for transfer */ + FAR uint8_t *buffer; /* The caller's buffer, for cache maintenance */ + FAR uint8_t *bounce; /* Aligned stand-in for it, or NULL */ + bool dmain; /* Direction this buffer was prepared for */ sem_t iocsem; /* Semaphore used to wait for transfer completion */ #ifdef CONFIG_USBHOST_ASYNCH usbhost_asynch_t callback; /* Transfer complete callback */ @@ -411,6 +416,12 @@ static int xhci_isoc_setup(FAR struct xhci_rhport_s *rhport, #endif static ssize_t xhci_transfer_wait(FAR struct usbhost_xhci_s *priv, FAR struct xhci_epinfo_s *epinfo); +static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, + FAR uint8_t *buffer, size_t buflen); +static FAR uint8_t *xhci_dma_prepare(FAR struct xhci_epinfo_s *epinfo, + FAR uint8_t *buffer, size_t buflen, + bool dirin); +static void xhci_dma_finish(FAR struct xhci_epinfo_s *epinfo); /* Interrupt handling *******************************************************/ @@ -2298,6 +2309,13 @@ static int xhci_control_setup(FAR struct xhci_rhport_s *rhport, if (buffer) { + buffer = xhci_dma_prepare(epinfo, buffer, buflen, + (req->type & USB_REQ_DIR_IN) != 0); + if (buffer == NULL) + { + return -ENOMEM; + } + trb[i].d0 = up_addrenv_va_to_pa(buffer); trb[i].d1 = XHCI_TRB_D1_TXLEN_SET(buflen); trb[i].d2 = XHCI_TRB_D2_TYPE_SET(XHCI_TRB_TYPE_DATA_STAGE); @@ -2370,6 +2388,14 @@ static int xhci_normal_setup(FAR struct xhci_rhport_s *rhport, uintptr_t pa; int n = 0; + /* Make the buffer safe for the controller to reach */ + + buffer = xhci_dma_prepare(epinfo, buffer, buflen, epinfo->dirin != 0); + if (buffer == NULL) + { + return -ENOMEM; + } + /* One TRB describes one run of memory, and that run may not cross a 64K * boundary. A longer transfer, or one starting near the wrong side of a * boundary, becomes several TRBs chained into a single transfer, with @@ -2446,6 +2472,14 @@ static int xhci_isoc_setup(FAR struct xhci_rhport_s *rhport, FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_RHPORT(rhport); struct xhci_trb_s trb; + /* Make the buffer safe for the controller to reach */ + + buffer = xhci_dma_prepare(epinfo, buffer, buflen, epinfo->dirin != 0); + if (buffer == NULL) + { + return -ENOMEM; + } + /* Prepare TRB */ trb.d0 = up_addrenv_va_to_pa(buffer); @@ -2739,6 +2773,152 @@ static void xhci_portsc_work(FAR void *arg) } } +/**************************************************************************** + * Name: xhci_dmacapable + * + * Description: + * Whether the controller may be pointed at this buffer. + * + * The driver has no way to know this on its own. Whether an address can + * be turned into one the device will reach, and whether what lies behind + * it is contiguous, is a property of the system the controller was fitted + * into, so the answer comes from there. A platform that says nothing is + * taken to mean every address works, which is what a flat address space + * gives. + * + ****************************************************************************/ + +static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, + FAR uint8_t *buffer, size_t buflen) +{ + if (priv->ops->dmacapable == NULL) + { + return true; + } + + return priv->ops->dmacapable(priv->arg, buffer, buflen); +} + +/**************************************************************************** + * Name: xhci_dma_prepare + * + * Description: + * Make a caller's buffer safe for the controller to reach, and say which + * address to hand it. + * + * The controller writes memory behind the processor's back, so on a + * machine whose caches are not coherent every buffer it touches must be + * flushed before the controller reads and invalidated before the + * processor does. + * + * Both act a whole cache line at a time, which is unsafe for a buffer + * that does not own its lines: invalidating drops whatever shares the + * line, and a writeback lands on top of what the controller just put + * there. Class drivers pass their own structure members, a 31 byte + * command block or a 13 byte status, which share lines. + * + * Such a buffer gets an aligned stand-in and is copied at the ends. + * Anything large enough to matter comes from a filesystem or from + * xhci_ioalloc() and is already aligned. + * + * Returned Value: + * The address to give the controller, or NULL if a stand-in was needed + * and could not be allocated. + * + ****************************************************************************/ + +static FAR uint8_t *xhci_dma_prepare(FAR struct xhci_epinfo_s *epinfo, + FAR uint8_t *buffer, size_t buflen, + bool dirin) +{ + size_t line = up_get_dcache_linesize(); + + epinfo->buffer = buffer; + epinfo->bounce = NULL; + epinfo->dmain = dirin; + + /* No cache to maintain, so nothing to arrange */ + + if (line == 0) + { + return buffer; + } + + if (((uintptr_t)buffer & (line - 1)) != 0 || (buflen & (line - 1)) != 0) + { + /* The buffer shares a line with something else. Work in a stand-in + * that does not. + */ + + epinfo->bounce = kmm_memalign(line, (buflen + line - 1) & ~(line - 1)); + if (epinfo->bounce == NULL) + { + return NULL; + } + + if (!dirin) + { + memcpy(epinfo->bounce, buffer, buflen); + } + + buffer = epinfo->bounce; + } + + /* Push what we are sending; drop what we are about to be sent, so that + * nothing the processor is still holding can be written back over it + * while the transfer is in flight. + */ + + if (dirin) + { + up_invalidate_dcache((uintptr_t)buffer, (uintptr_t)buffer + buflen); + } + else + { + up_clean_dcache((uintptr_t)buffer, (uintptr_t)buffer + buflen); + } + + return buffer; +} + +/**************************************************************************** + * Name: xhci_dma_finish + * + * Description: + * Read back what the controller wrote, and give up any stand-in buffer. + * Called on completion, before whoever is waiting is woken. + * + ****************************************************************************/ + +static void xhci_dma_finish(FAR struct xhci_epinfo_s *epinfo) +{ + FAR uint8_t *dma = epinfo->bounce ? epinfo->bounce : epinfo->buffer; + bool dirin = epinfo->dmain; + + if (dma == NULL) + { + return; + } + + if (dirin) + { + up_invalidate_dcache((uintptr_t)dma, (uintptr_t)dma + epinfo->buflen); + + if (epinfo->bounce != NULL && epinfo->buffer != NULL) + { + memcpy(epinfo->buffer, epinfo->bounce, epinfo->buflen); + } + } + + if (epinfo->bounce != NULL) + { + kmm_free(epinfo->bounce); + epinfo->bounce = NULL; + } + + epinfo->buffer = NULL; +} + /**************************************************************************** * Name: xhci_transfer_complete * @@ -2762,6 +2942,10 @@ static void xhci_transfer_complete(FAR struct usbhost_xhci_s *priv, epinfo = priv->devs[slot - 1].epinfo[ep - 1]; DEBUGASSERT(epinfo != NULL); + /* Read back what the controller wrote before anyone looks at it */ + + xhci_dma_finish(epinfo); + flags = spin_lock_irqsave(&priv->spinlock); /* Get transferred length */ @@ -3737,7 +3921,8 @@ static int xhci_free(FAR struct usbhost_driver_s *drvr, FAR uint8_t *buffer) static int xhci_ioalloc(FAR struct usbhost_driver_s *drvr, FAR uint8_t **buffer, size_t buflen) { - int ret = -ENOMEM; + size_t line; + int ret = -ENOMEM; DEBUGASSERT(drvr && buffer && buflen > 0); @@ -3748,7 +3933,18 @@ static int xhci_ioalloc(FAR struct usbhost_driver_s *drvr, return -ENOMEM; } - /* Allocated buffer must not cross page boundaries */ + /* Allocated buffer must not cross page boundaries. + * + * Round to whole cache lines as well as aligning the start, so that the + * buffer owns every line it touches and can be invalidated without + * disturbing whatever would otherwise share the last one. + */ + + line = up_get_dcache_linesize(); + if (line > 1) + { + buflen = (buflen + line - 1) & ~(line - 1); + } *buffer = (FAR uint8_t *)kmm_memalign((XHCI_PAGE_SIZE / 2) , buflen); if (*buffer) @@ -3847,6 +4043,13 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, len = xhci_getle16(req->len); + /* Refuse a buffer the controller cannot reach, as for bulk transfers */ + + if (buffer != NULL && len > 0 && !xhci_dmacapable(priv, buffer, len)) + { + return -EFAULT; + } + /* Terse output only if we are tracing */ #ifdef CONFIG_USBHOST_TRACE @@ -3870,6 +4073,15 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, ret = xhci_address_set(priv, rhport, true); if (ret == OK) { + /* The controller chose this address and wrote it into the + * output context. Invalidate before reading, or the stale + * copy is used. + */ + + up_invalidate_dcache((uintptr_t)rhport->dev->ctx, + (uintptr_t)rhport->dev->ctx + + sizeof(struct xhci_dev_ctx_s)); + /* Store USB Device Address assigned by xHCI */ ep0info->devaddr = @@ -4010,6 +4222,16 @@ static ssize_t xhci_transfer(FAR struct usbhost_driver_s *drvr, DEBUGASSERT(priv && rhport && epinfo && buffer && buflen > 0); + /* Refuse a buffer the controller cannot reach rather than pointing it at + * the wrong memory. A caller that has somewhere better to put the data + * will try again with it; the FAT filesystem does exactly that. + */ + + if (!xhci_dmacapable(priv, buffer, buflen)) + { + return -EFAULT; + } + /* We must have exclusive access to the xHCI hardware and data * structures. */ diff --git a/include/nuttx/usb/xhci.h b/include/nuttx/usb/xhci.h index 2368a5a9efe23..1010811244ba6 100644 --- a/include/nuttx/usb/xhci.h +++ b/include/nuttx/usb/xhci.h @@ -62,6 +62,13 @@ struct xhci_bus_ops_s /* Undo it, and release anything the bus allocated to make it work */ CODE void (*irq_detach)(FAR void *arg); + + /* Whether the controller may be pointed at a given buffer, which is a + * property of the platform. Leave NULL where every address a caller can + * produce is reachable, as a flat address space gives. + */ + + CODE bool (*dmacapable)(FAR void *arg, FAR uint8_t *buffer, size_t buflen); }; /**************************************************************************** From 042f20ce4bf0cba2625e0748c9f04f6709e84c5f Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:14:53 +0800 Subject: [PATCH 09/26] drivers/usbhost: Read xHCI HCIVERSION with an aligned access. The register dump read HCIVERSION with a 32-bit access at offset two. It is a 16-bit register sharing a word with CAPLENGTH, so that is an unaligned read of a device register: harmless where the bus permits it and a fault where it does not. Read the word once and take both fields from it. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 738c1563e1887..c92fec198bed2 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -729,8 +729,17 @@ static void xhci_dump_mem(FAR struct usbhost_xhci_s *priv, uinfo("Dump xHCI registers: %s\n", msg); uinfo("=== Host Controller Capability Registers ===\n"); - xhci_dump_capa_reg(priv, "CAPLENGTH ", XHCI_CAPLENGTH); - xhci_dump_capa_reg(priv, "HCIVERSION ", XHCI_HCIVERSION); + + /* CAPLENGTH and HCIVERSION share one word, and a register block reached + * over a bus that only answers aligned accesses cannot be read at the + * odd offset the second one has. Read the word once and take both from + * it. + */ + + uinfo("\tCAPLENGTH :\t\t0x%" PRIx32 "\n", + xhci_capa_getreg(priv, XHCI_CAPLENGTH) & 0xff); + uinfo("\tHCIVERSION :\t\t0x%" PRIx32 "\n", + xhci_capa_getreg(priv, XHCI_CAPLENGTH) >> 16); xhci_dump_capa_reg(priv, "HCSPARAMS1 ", XHCI_HCSPARAMS1); xhci_dump_capa_reg(priv, "HCSPARAMS2 ", XHCI_HCSPARAMS2); xhci_dump_capa_reg(priv, "HCSPARAMS3 ", XHCI_HCSPARAMS3); From 84653a3460925830f0ef8f9e447a358f86fe1299 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:14:53 +0800 Subject: [PATCH 10/26] drivers/usbhost: Report which xHCI command was rejected. A failed command logged only its completion code. The difference between a refused Address Device and a refused Evaluate Context is most of the diagnosis, and the completion code does not give it. Keep the command type before the result overwrites the TRB, and name it in the message. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index c92fec198bed2..b1f05a3385ba5 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -1915,7 +1915,8 @@ static void xhci_context_ctrl(FAR struct usbhost_xhci_s *priv, static int xhci_command(FAR struct usbhost_xhci_s *priv, FAR struct xhci_trb_s *trb, uint16_t timeout_ms) { - int ret; + uint32_t cmdtype; + int ret; /* Lock bus */ @@ -1925,6 +1926,10 @@ static int xhci_command(FAR struct usbhost_xhci_s *priv, return ret; } + /* Remember what this was before the result overwrites it */ + + cmdtype = XHCI_TRB_D2_TYPE_GET(trb->d2); + /* Add command to ring */ xhci_add_trb(priv, &priv->cmd, trb, 1); @@ -1960,7 +1965,8 @@ static int xhci_command(FAR struct usbhost_xhci_s *priv, } else { - uerr("event CC = %d\n", XHCI_TRB_D1_CC_GET(trb->d1)); + uerr("command type %d failed, CC = %d\n", cmdtype, + XHCI_TRB_D1_CC_GET(trb->d1)); ret = -EIO; } From 66dfbf49acbdcc933419324939046dbff55c4add Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:15:08 +0800 Subject: [PATCH 11/26] drivers/usbhost: Compute the event ring segment count at full width. The number of event ring segments a controller allows is a power of two reported as its exponent, and the exponent can reach 15. Computing 1 << exponent into the uint8_t that holds it wraps to zero on any controller offering more than 128 segments, and a controller told its event ring table holds no entries has nowhere to report anything: every command times out. Work it out at full width and narrow afterwards. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index b1f05a3385ba5..144a4b66faaca 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -4611,6 +4611,7 @@ static void xhci_disconnect(FAR struct usbhost_driver_s *drvr, static int xhci_hw_getparams(FAR struct usbhost_xhci_s *priv) { uint32_t regval; + uint32_t erst; /* Get data form Host Controller Capability 1 Parameters */ @@ -4651,16 +4652,21 @@ static int xhci_hw_getparams(FAR struct usbhost_xhci_s *priv) uinfo("no scratch = %d\n", priv->no_scratch); - priv->no_erst = 1 << XHCI_HCSPARAMS2_ERST(regval); + /* How many event ring segments the controller will allow, which is a + * power of two and can reach 32768, so it is worked out at full width + * and only then narrowed to what this driver actually uses. Computed + * into the field directly it would wrap to zero on any controller + * offering more than 128 segments, and a table declared to hold no + * entries gives a controller with nowhere to report anything. + */ + + erst = 1ul << XHCI_HCSPARAMS2_ERST(regval); - uinfo("no_erst = %d\n", priv->no_erst); + uinfo("erst max = %" PRIu32 "\n", erst); /* Limit event ring segment table to 1 */ - if (priv->no_erst > XHCI_MAX_ERST) - { - priv->no_erst = XHCI_MAX_ERST; - } + priv->no_erst = (erst > XHCI_MAX_ERST) ? XHCI_MAX_ERST : erst; uinfo("no erst = %d\n", priv->no_erst); From 4246bb3ec51c7aac772407fe9a80e082efa5be1c Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:15:27 +0800 Subject: [PATCH 12/26] drivers/usbhost: Describe devices to an xHCI controller correctly. What a controller is told about a device before it will accept it. A DWC3 core validates these where QEMU's controller does not. - HCCPARAMS1 says whether context structures are 32 or 64 bytes, and the wider form was refused outright with -EIO; the EIC7700X reports 0x0220fe45 on both of its controllers, so this driver could not have driven either. A wide context is the same fields with reserved space after them, so only the stride changes. Read it at start up and use it wherever a context array is walked. - Contexts must be 64 byte aligned, since every device context base address array entry points at one, and the output context came from kmm_zalloc(). - The slot context never carried the device speed, which has no valid zero, so a validating controller answers Address Device with a parameter error. The speed was already implied by the endpoint context's maximum packet size. The numbering is xHCI's own, hence the mapping. - The output device context was cleared and never flushed. That context is the controller's to write, so what stays behind is a dirty line of zeros written back over the slot state, and the next command against the slot is refused with a context state error. Enumeration reached SET_ADDRESS and stopped. - A buffer copied through an aligned stand-in was copied back using buflen, which control transfers deliberately leave zero, so a descriptor read copied nothing back and the caller was handed whatever its buffer held before. Keep the requested length separately, and maintain the cache over the whole stand-in rather than the part in use. - A buffer the controller cannot reach is now copied through a stand-in rather than refused. -EFAULT works for a caller with somewhere better to put the data, and fails outright for one without: reading a block device directly from a user program returned an error where the transfer could have gone through a stand-in. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 281 +++++++++++++++++++++++++-------- drivers/usbhost/usbhost_xhci.h | 16 ++ 2 files changed, 233 insertions(+), 64 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 144a4b66faaca..377791bce5cea 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -81,6 +82,14 @@ */ #define XHCI_PORT_RESET_MS (500) + +/* How much memory a context occupies, which depends on the stride the + * controller asked for. One entry for the slot and one per endpoint, and + * the input context carries its control entry in front of both. + */ + +#define XHCI_DEVCTX_SIZE(priv) ((1 + XHCI_MAX_ENDPOINTS) * (priv)->ctxsize) +#define XHCI_INCTX_SIZE(priv) ((2 + XHCI_MAX_ENDPOINTS) * (priv)->ctxsize) #define XHCI_BUFSIZE (512) /* Port numbers macros */ @@ -146,6 +155,8 @@ struct xhci_epinfo_s size_t buflen; /* Buffer length used for transfer */ FAR uint8_t *buffer; /* The caller's buffer, for cache maintenance */ FAR uint8_t *bounce; /* Aligned stand-in for it, or NULL */ + size_t dmalen; /* Length the cache is maintained over */ + size_t dmacopy; /* Length to copy back out of a stand-in */ bool dmain; /* Direction this buffer was prepared for */ sem_t iocsem; /* Semaphore used to wait for transfer completion */ #ifdef CONFIG_USBHOST_ASYNCH @@ -251,6 +262,7 @@ struct usbhost_xhci_s FAR const struct xhci_bus_ops_s *ops; /* Bus operations */ FAR void *arg; /* Bus private data */ FAR const char *name; /* What to call this controller */ + uint8_t ctxsize; /* Context stride, 32 or 64 bytes */ uint32_t pending; /* IRQ pending status */ struct work_s work; /* IRQ work */ struct work_s pscwork; /* Port status change work */ @@ -418,7 +430,17 @@ static ssize_t xhci_transfer_wait(FAR struct usbhost_xhci_s *priv, FAR struct xhci_epinfo_s *epinfo); static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, FAR uint8_t *buffer, size_t buflen); -static FAR uint8_t *xhci_dma_prepare(FAR struct xhci_epinfo_s *epinfo, +static uint32_t xhci_speed_id(uint8_t speed); +static inline FAR struct xhci_slot_ctx_s * +xhci_in_slot(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_input_dev_ctx_s *input); +static inline FAR struct xhci_ep_ctx_s * +xhci_in_ep(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_input_dev_ctx_s *input, int epidx); +static inline FAR struct xhci_slot_ctx_s * +xhci_out_slot(FAR struct xhci_dev_ctx_s *ctx); +static FAR uint8_t *xhci_dma_prepare(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_epinfo_s *epinfo, FAR uint8_t *buffer, size_t buflen, bool dirin); static void xhci_dma_finish(FAR struct xhci_epinfo_s *epinfo); @@ -1614,7 +1636,7 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, * Initialize all fields to 0. */ - memset(dev->input, 0, sizeof(struct xhci_input_dev_ctx_s)); + memset(dev->input, 0, XHCI_INCTX_SIZE(priv)); /* Step 2. Initialize the Input Control Context by setting the A0 and * A1 flags to 1 (Slot flag and EP0 flag). @@ -1624,9 +1646,16 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, XHCI_IN_CTX1_A(XHCI_EP0_FLAG); xhci_context_ctrl(priv, dev, 0, regval); - /* Step 3. Initialize the Input Slot Context */ + /* Step 3. Initialize the Input Slot Context. + * + * The speed field has no valid zero. This is the only place the + * controller learns the device's speed, and one that checks refuses + * Address Device with a parameter error without it. + */ - regval = XHCI_ST_CTX0_CTXENT_SET(1); + regval = XHCI_ST_CTX0_CTXENT_SET(1) | + XHCI_ST_CTX0_SPEED_SET( + xhci_speed_id(dev->rhport->hport.hport.speed)); #ifdef CONFIG_USBHOST_HUB /* TODO: @@ -1638,7 +1667,7 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, # warning missing logic #endif - dev->input->slot.ctx[0] = htole32(regval); + xhci_in_slot(priv, dev->input)->ctx[0] = htole32(regval); /* Configure Root Hub Port Number (starts from 1) */ @@ -1647,7 +1676,7 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, /* TODO: configure number of ports */ regval |= XHCI_ST_CTX1_PORTS_SET(0); - dev->input->slot.ctx[1] = htole32(regval); + xhci_in_slot(priv, dev->input)->ctx[1] = htole32(regval); /* Step 4. the Transfer Ring for the Default Control Endpoint is already * allocated. @@ -1673,7 +1702,7 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, DEBUGASSERT(drdp != 0); xhci_ep_configure(priv, - &dev->input->ep[0], + xhci_in_ep(priv, dev->input, 0), XHCI_EPTYPE_CTRL, maxpkt, 0, drdp, 0, 0); @@ -1682,13 +1711,22 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, * Initialize all fields to 0. */ - memset(dev->ctx, 0, sizeof(struct xhci_dev_ctx_s)); + memset(dev->ctx, 0, XHCI_DEVCTX_SIZE(priv)); - /* Flush Device input context */ + /* Flush both contexts. + * + * The output context is the controller's to write, so clearing it must + * reach memory: the dirty zeros left in cache are written back later, on + * top of what the controller has put there. The slot state lives in + * that context, and losing it fails the next command against the slot. + */ + + up_flush_dcache((uintptr_t)dev->ctx, + (uintptr_t)dev->ctx + XHCI_DEVCTX_SIZE(priv)); up_flush_dcache((uintptr_t)dev->input, (uintptr_t)dev->input + - sizeof(struct xhci_input_dev_ctx_s)); + XHCI_INCTX_SIZE(priv)); /* Step 7. Load the appropriate (Device Slot ID) entry in the Device * Context Base Address Array with a pointer to the Output Device @@ -1826,8 +1864,15 @@ static int xhci_device_deinit(FAR struct usbhost_xhci_s *priv, rhport->dev->state = XHCI_SLOT_DISABLED; - memset(rhport->dev->ctx, 0, sizeof(struct xhci_dev_ctx_s)); - memset(rhport->dev->input, 0, sizeof(struct xhci_input_dev_ctx_s)); + memset(rhport->dev->ctx, 0, XHCI_DEVCTX_SIZE(priv)); + memset(rhport->dev->input, 0, XHCI_INCTX_SIZE(priv)); + + /* And push both, so nothing is left to be written back later */ + + up_flush_dcache((uintptr_t)rhport->dev->ctx, + (uintptr_t)rhport->dev->ctx + XHCI_DEVCTX_SIZE(priv)); + up_flush_dcache((uintptr_t)rhport->dev->input, + (uintptr_t)rhport->dev->input + XHCI_INCTX_SIZE(priv)); /* Remove reference to a device slot */ @@ -1896,8 +1941,8 @@ static void xhci_context_ctrl(FAR struct usbhost_xhci_s *priv, } } - dev->input->slot.ctx[0] &= ~XHCI_ST_CTX0_CTXENT_MASK; - dev->input->slot.ctx[0] |= XHCI_ST_CTX0_CTXENT_SET(i); + xhci_in_slot(priv, dev->input)->ctx[0] &= ~XHCI_ST_CTX0_CTXENT_MASK; + xhci_in_slot(priv, dev->input)->ctx[0] |= XHCI_ST_CTX0_CTXENT_SET(i); } /**************************************************************************** @@ -2324,7 +2369,7 @@ static int xhci_control_setup(FAR struct xhci_rhport_s *rhport, if (buffer) { - buffer = xhci_dma_prepare(epinfo, buffer, buflen, + buffer = xhci_dma_prepare(priv, epinfo, buffer, buflen, (req->type & USB_REQ_DIR_IN) != 0); if (buffer == NULL) { @@ -2405,7 +2450,8 @@ static int xhci_normal_setup(FAR struct xhci_rhport_s *rhport, /* Make the buffer safe for the controller to reach */ - buffer = xhci_dma_prepare(epinfo, buffer, buflen, epinfo->dirin != 0); + buffer = xhci_dma_prepare(priv, epinfo, buffer, buflen, + epinfo->dirin != 0); if (buffer == NULL) { return -ENOMEM; @@ -2431,8 +2477,8 @@ static int xhci_normal_setup(FAR struct xhci_rhport_s *rhport, if (++n >= XHCI_TD_MAX) { - uerr("transfer of %zu needs more TRBs than the ring holds\n", - buflen); + uerr("transfer of %zu from pa %" PRIxPTR " needs more than %d " + "TRBs\n", buflen, pa, XHCI_TD_MAX); return -EINVAL; } @@ -2489,7 +2535,8 @@ static int xhci_isoc_setup(FAR struct xhci_rhport_s *rhport, /* Make the buffer safe for the controller to reach */ - buffer = xhci_dma_prepare(epinfo, buffer, buflen, epinfo->dirin != 0); + buffer = xhci_dma_prepare(priv, epinfo, buffer, buflen, + epinfo->dirin != 0); if (buffer == NULL) { return -ENOMEM; @@ -2788,6 +2835,82 @@ static void xhci_portsc_work(FAR void *arg) } } +/**************************************************************************** + * Name: xhci_in_slot / xhci_in_ep / xhci_out_slot + * + * Description: + * Reach into a device context. + * + * A context is an array of equally sized entries, and how big they are is + * a property of the controller rather than of the specification: it + * reports either thirty-two or sixty-four bytes, and the wider form is + * the same fields with reserved space after them. So these are the same + * structures at a different stride, and only the arithmetic to find the + * n'th one has to know which. + * + * Output context: slot, then endpoints 1 upward. + * Input context: input control, then slot, then endpoints. + * + * The first entry of either is at offset zero, so only the ones after it + * need this. + * + ****************************************************************************/ + +static inline FAR struct xhci_slot_ctx_s * +xhci_in_slot(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_input_dev_ctx_s *input) +{ + return (FAR struct xhci_slot_ctx_s *)((uintptr_t)input + priv->ctxsize); +} + +static inline FAR struct xhci_ep_ctx_s * +xhci_in_ep(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_input_dev_ctx_s *input, int epidx) +{ + return (FAR struct xhci_ep_ctx_s *)((uintptr_t)input + + (epidx + 2) * priv->ctxsize); +} + +static inline FAR struct xhci_slot_ctx_s * +xhci_out_slot(FAR struct xhci_dev_ctx_s *ctx) +{ + return (FAR struct xhci_slot_ctx_s *)ctx; +} + +/**************************************************************************** + * Name: xhci_speed_id + * + * Description: + * Turn the speed the USB host stack uses into the one a slot context + * wants, which is a different numbering with no relation to it. + * + ****************************************************************************/ + +static uint32_t xhci_speed_id(uint8_t speed) +{ + switch (speed) + { + case USB_SPEED_LOW: + return XHCI_SPEED_LOW; + case USB_SPEED_FULL: + return XHCI_SPEED_FULL; + case USB_SPEED_HIGH: + return XHCI_SPEED_HIGH; + case USB_SPEED_SUPER: + return XHCI_SPEED_SUPER; + case USB_SPEED_SUPER_PLUS: + return XHCI_SPEED_SUPER_PLUS; + default: + + /* Nothing else can be described to a controller, and full speed + * is the safe answer. + */ + + uwarn("no speed ID for USB speed %d\n", speed); + return XHCI_SPEED_FULL; + } +} + /**************************************************************************** * Name: xhci_dmacapable * @@ -2842,30 +2965,53 @@ static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, * ****************************************************************************/ -static FAR uint8_t *xhci_dma_prepare(FAR struct xhci_epinfo_s *epinfo, +static FAR uint8_t *xhci_dma_prepare(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_epinfo_s *epinfo, FAR uint8_t *buffer, size_t buflen, bool dirin) { - size_t line = up_get_dcache_linesize(); + size_t line = up_get_dcache_linesize(); + bool reachable = xhci_dmacapable(priv, buffer, buflen); + + epinfo->buffer = buffer; + epinfo->bounce = NULL; + epinfo->dmalen = buflen; + + /* How much to bring back afterwards. This cannot be taken from buflen + * at completion time: that field means the length of a data transfer and + * control transfers deliberately leave it zero, so a descriptor read + * would copy nothing back and the caller would see whatever its buffer + * held before. + */ - epinfo->buffer = buffer; - epinfo->bounce = NULL; - epinfo->dmain = dirin; + epinfo->dmacopy = buflen; + epinfo->dmain = dirin; - /* No cache to maintain, so nothing to arrange */ + /* Nothing to arrange: no cache to maintain, and an address the + * controller can be pointed at as it stands. + */ - if (line == 0) + if (line == 0 && reachable) { return buffer; } - if (((uintptr_t)buffer & (line - 1)) != 0 || (buflen & (line - 1)) != 0) + if (!reachable || + ((uintptr_t)buffer & (line - 1)) != 0 || (buflen & (line - 1)) != 0) { /* The buffer shares a line with something else. Work in a stand-in * that does not. */ - epinfo->bounce = kmm_memalign(line, (buflen + line - 1) & ~(line - 1)); + /* Maintain the whole stand-in, not just the part in use: cache + * operations work a line at a time and this chip rejects a partial + * range. + */ + + epinfo->dmalen = line ? ((buflen + line - 1) & ~(line - 1)) : buflen; + + epinfo->bounce = kmm_memalign(line ? line : sizeof(uintptr_t), + epinfo->dmalen); if (epinfo->bounce == NULL) { return NULL; @@ -2886,11 +3032,13 @@ static FAR uint8_t *xhci_dma_prepare(FAR struct xhci_epinfo_s *epinfo, if (dirin) { - up_invalidate_dcache((uintptr_t)buffer, (uintptr_t)buffer + buflen); + up_invalidate_dcache((uintptr_t)buffer, + (uintptr_t)buffer + epinfo->dmalen); } else { - up_clean_dcache((uintptr_t)buffer, (uintptr_t)buffer + buflen); + up_clean_dcache((uintptr_t)buffer, + (uintptr_t)buffer + epinfo->dmalen); } return buffer; @@ -2917,11 +3065,12 @@ static void xhci_dma_finish(FAR struct xhci_epinfo_s *epinfo) if (dirin) { - up_invalidate_dcache((uintptr_t)dma, (uintptr_t)dma + epinfo->buflen); + up_invalidate_dcache((uintptr_t)dma, + (uintptr_t)dma + epinfo->dmalen); if (epinfo->bounce != NULL && epinfo->buffer != NULL) { - memcpy(epinfo->buffer, epinfo->bounce, epinfo->buflen); + memcpy(epinfo->buffer, epinfo->bounce, epinfo->dmacopy); } } @@ -3557,8 +3706,11 @@ static int xhci_ep0configure(FAR struct usbhost_driver_s *drvr, { /* Update max packet size */ - rhport->dev->input->ep[0].ctx1 &= ~XHCI_EP_CTX1_MAXPKT_MASK; - rhport->dev->input->ep[0].ctx1 |= XHCI_EP_CTX1_MAXPKT(maxpacketsize); + FAR struct xhci_ep_ctx_s *ep0ctx = + xhci_in_ep(priv, rhport->dev->input, 0); + + ep0ctx->ctx1 &= ~XHCI_EP_CTX1_MAXPKT_MASK; + ep0ctx->ctx1 |= XHCI_EP_CTX1_MAXPKT(maxpacketsize); /* Add Slot Context and EP0 Context */ @@ -3570,13 +3722,17 @@ static int xhci_ep0configure(FAR struct usbhost_driver_s *drvr, up_flush_dcache((uintptr_t)rhport->dev->input, (uintptr_t)rhport->dev->input + - sizeof(struct xhci_input_dev_ctx_s)); + XHCI_INCTX_SIZE(priv)); /* Free mutex before command execution */ nxmutex_unlock(&priv->lock); ctx = up_addrenv_va_to_pa(rhport->dev->input); + + uinfo("slot %d funcaddr %d speed %d maxpacket %d\n", + epinfo->slot, funcaddr, speed, maxpacketsize); + ret = xhci_cmd_evalctx(priv, epinfo->slot, ctx); } @@ -3627,6 +3783,12 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, && ep != NULL); hport = epdesc->hport; + /* Only the tracing alternative below and the hub logic further down use + * this, and a configuration may have neither. + */ + + UNUSED(hport); + /* Terse output only if we are tracing */ #ifdef CONFIG_USBHOST_TRACE @@ -3747,7 +3909,7 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, * Max Burst Size set for 0 for now (USB3.0 specific) */ - xhci_ep_configure(priv, &dev->input->ep[idx - 1], + xhci_ep_configure(priv, xhci_in_ep(priv, dev->input, idx - 1), eptype, epdesc->mxpacketsize, 0, up_addrenv_va_to_pa(epinfo->td.ring), 0, epinfo->interval); @@ -3758,7 +3920,7 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, up_flush_dcache((uintptr_t)dev->input, (uintptr_t)dev->input + - sizeof(struct xhci_input_dev_ctx_s)); + XHCI_INCTX_SIZE(priv)); /* Configure EP */ @@ -4058,13 +4220,6 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, len = xhci_getle16(req->len); - /* Refuse a buffer the controller cannot reach, as for bulk transfers */ - - if (buffer != NULL && len > 0 && !xhci_dmacapable(priv, buffer, len)) - { - return -EFAULT; - } - /* Terse output only if we are tracing */ #ifdef CONFIG_USBHOST_TRACE @@ -4095,13 +4250,14 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, up_invalidate_dcache((uintptr_t)rhport->dev->ctx, (uintptr_t)rhport->dev->ctx + - sizeof(struct xhci_dev_ctx_s)); + XHCI_DEVCTX_SIZE(priv)); /* Store USB Device Address assigned by xHCI */ ep0info->devaddr = - XHCI_ST_CTX3_ADDR_GET(rhport->dev->ctx->slot.ctx[3]); - rhport->dev->input->slot.ctx[3] = rhport->dev->ctx->slot.ctx[3]; + XHCI_ST_CTX3_ADDR_GET(xhci_out_slot(rhport->dev->ctx)->ctx[3]); + xhci_in_slot(priv, rhport->dev->input)->ctx[3] = + xhci_out_slot(rhport->dev->ctx)->ctx[3]; } return OK; @@ -4237,16 +4393,6 @@ static ssize_t xhci_transfer(FAR struct usbhost_driver_s *drvr, DEBUGASSERT(priv && rhport && epinfo && buffer && buflen > 0); - /* Refuse a buffer the controller cannot reach rather than pointing it at - * the wrong memory. A caller that has somewhere better to put the data - * will try again with it; the FAT filesystem does exactly that. - */ - - if (!xhci_dmacapable(priv, buffer, buflen)) - { - return -EFAULT; - } - /* We must have exclusive access to the xHCI hardware and data * structures. */ @@ -4615,12 +4761,14 @@ static int xhci_hw_getparams(FAR struct usbhost_xhci_s *priv) /* Get data form Host Controller Capability 1 Parameters */ + /* Context entry stride, 32 or 64 bytes as the controller reports. The + * wider form is the same fields with padding. + */ + regval = xhci_capa_getreg(priv, XHCI_HCCPARAMS1); - if (regval & XHCI_HCCPARAMS1_CSZ) - { - uerr("Only 32 byte Context data structures supported!\n"); - return -EIO; - } + priv->ctxsize = (regval & XHCI_HCCPARAMS1_CSZ) ? 64 : 32; + + uinfo("context size = %d\n", priv->ctxsize); /* Get data from Structural Parameters 1 register */ @@ -4787,7 +4935,12 @@ static int xhci_mem_alloc(FAR struct usbhost_xhci_s *priv) { /* Allocate Device Context */ - priv->devs[i].ctx = kmm_zalloc(sizeof(struct xhci_dev_ctx_s)); + /* The base address array holds these, and every entry in it must be + * 64 byte aligned, so the allocation has to be too. + */ + + priv->devs[i].ctx = kmm_memalign(XHCI_CTX_ALIGN, + XHCI_DEVCTX_SIZE(priv)); if (!priv->devs[i].ctx) { uerr("dev ctx zalloc failed!\n"); @@ -4799,7 +4952,7 @@ static int xhci_mem_alloc(FAR struct usbhost_xhci_s *priv) */ priv->devs[i].input = kmm_memalign((XHCI_PAGE_SIZE / 2), - sizeof(struct xhci_input_dev_ctx_s)); + XHCI_INCTX_SIZE(priv)); if (!priv->devs[i].input) { uerr("dev input zalloc failed!\n"); diff --git a/drivers/usbhost/usbhost_xhci.h b/drivers/usbhost/usbhost_xhci.h index 2fc2aa207aded..3339721895d95 100644 --- a/drivers/usbhost/usbhost_xhci.h +++ b/drivers/usbhost/usbhost_xhci.h @@ -506,6 +506,22 @@ #define XHCI_ST_CTX0_RTSTR_MASK (0xfffff << XHCI_ST_CTX0_RTSTR_SHIFT) #define XHCI_ST_CTX0_SPEED_SHIFT (20) /* Bits 20:23: Speed */ #define XHCI_ST_CTX0_SPEED_MASK (0xf << XHCI_ST_CTX0_SPEED_SHIFT) +#define XHCI_ST_CTX0_SPEED_SET(x) (((x) << XHCI_ST_CTX0_SPEED_SHIFT) & \ + XHCI_ST_CTX0_SPEED_MASK) + +/* Port Speed IDs, which xHCI numbers its own way rather than USB's. These + * are the values every controller reports in PORTSC and expects back in a + * slot context; a device is described to the controller with one of them + * and with nothing else, so zero is not a default but an invalid context. + * + * Reference: Table 7-13: Default USB Speed ID Mapping + */ + +#define XHCI_SPEED_FULL (1) +#define XHCI_SPEED_LOW (2) +#define XHCI_SPEED_HIGH (3) +#define XHCI_SPEED_SUPER (4) +#define XHCI_SPEED_SUPER_PLUS (5) #define XHCI_ST_CTX0_MTT (1 << 25) /* Bit 25: Multi-TT */ /* Bit 24: Reserved */ #define XHCI_ST_CTX0_HUB (1 << 26) /* Bit 26: Hub */ From 28858db5a07d0d8fb257b99313304958a129e7f4 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:16:12 +0800 Subject: [PATCH 13/26] drivers/usbhost: Carry the xHCI transfer chain across the ring join. A transfer described by more than one TRB can reach the end of the ring part way through, so the link that sends the controller back to the beginning falls inside the transfer rather than between two of them. Written without the chain bit, that link ends the transfer where it stands: the controller follows it, considers the work finished, and reports nothing, because the TRB that asked for the completion interrupt is on the far side of the join. Nothing waiting is woken, and transfers have no timeout, so the symptom is a read that never returns. Carry the chain bit onto the link when the TRB it follows has it. Reading 1MiB from a USB drive, where the last two sizes did not complete at all before: 512 byte blocks 166 KB/s 4 KiB blocks 1333 KB/s 32 KiB blocks 10666 KB/s 64 KiB blocks 15515 KB/s Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 377791bce5cea..e1461ccfbd23c 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -990,6 +990,19 @@ static void xhci_add_trb(FAR struct usbhost_xhci_s *priv, XHCI_TRB_D2_TYPE_SET(XHCI_TRB_TYPE_LINK); } + /* Carry the chain forward across the join. + * + * A multi-TRB transfer can reach the end of the ring part way + * through, putting the link inside it. A link without the + * chain bit ends the transfer where it stands, and the TRB that + * asked for the completion interrupt is never reached. + */ + + if ((trb[i].d2 & XHCI_TRB_D2_CH) != 0) + { + d2 |= XHCI_TRB_D2_CH; + } + /* Other parameters are already correct for this TRB */ ring->ring[ring->i].d2 = htole32(d2); From ddabfb1e6dc380ec1948356b0e0b9c1dbebbc54b Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:16:41 +0800 Subject: [PATCH 14/26] drivers/usbhost: Announce what an xHCI port has attached. Report each device as it comes up, and report it going away. The announcement is made at the end of the port enable rather than at connect, because the PORTSC speed field means nothing until the port has been reset: a USB2 port reports its reset default, full speed, until then, so every device would be announced at 12Mbps regardless of what it negotiates a moment later. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index e1461ccfbd23c..b8068948d68f3 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -358,6 +359,7 @@ static int xhci_ctrl_reset(FAR struct usbhost_xhci_s *priv); /* Port management **********************************************************/ static void xhci_probe_ports(FAR struct usbhost_xhci_s *priv); +static FAR const char *xhci_speed_str(uint32_t portsc); static int xhci_port_enable(FAR struct usbhost_xhci_s *priv, FAR struct usbhost_hubport_s *hport); @@ -1496,6 +1498,16 @@ static int xhci_port_enable(FAR struct usbhost_xhci_s *priv, } } + /* Say what turned up, now that the port can answer. + * + * The speed field only means anything once the port has been reset and + * enabled. A USB2 port reports the reset default, full speed, until + * then. + */ + + syslog(LOG_INFO, "%s: port %d: device attached at %s\n", + priv->name, rhpndx + 1, xhci_speed_str(regval)); + return OK; } @@ -2732,6 +2744,36 @@ static void xhci_asynch_completion(FAR struct xhci_epinfo_s *epinfo) } #endif +/**************************************************************************** + * Name: xhci_speed_str + * + * Description: + * What a port negotiated, in words. PORTSC reports a speed ID, not a + * speed. + * + ****************************************************************************/ + +static FAR const char *xhci_speed_str(uint32_t portsc) +{ + switch (XHCI_PORTSC_PS(portsc)) + { + case XHCI_PORTSC_PS_FULL: + return "full speed, 12Mbps"; + case XHCI_PORTSC_PS_LOW: + return "low speed, 1.5Mbps"; + case XHCI_PORTSC_PS_HIGH: + return "high speed, 480Mbps"; + case XHCI_PORTSC_PS_SUPPER11: + return "SuperSpeed, 5Gbps"; + case XHCI_PORTSC_PS_SUPPER21: + case XHCI_PORTSC_PS_SUPPER12: + case XHCI_PORTSC_PS_SUPPER22: + return "SuperSpeed+, 10Gbps"; + default: + return "an unknown speed"; + } +} + /**************************************************************************** * Name: xhci_portsc_work * @@ -2811,6 +2853,9 @@ static void xhci_portsc_work(FAR void *arg) usbhost_vtrace2(XHCI_VTRACE2_PORTSC_DISCONND, rhpndx + 1, priv->pscwait); + syslog(LOG_INFO, "%s: port %d: device removed\n", + priv->name, rhpndx + 1); + rhport->connected = false; /* Are we bound to a class instance? */ From 843d5acc708fe245a3a4e6a59eef43af90a2dc6e Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:16:51 +0800 Subject: [PATCH 15/26] drivers/usbhost: Copy an xHCI stand-in buffer in the caller's context. The copy out of a stand-in was done in the completion handler, which runs on a work queue, while the buffer it copies into may belong to a user process whose addresses mean nothing there. Reading a block device directly from a user program faulted. The caller is blocked until the transfer finishes, so the copy belongs there. An asynchronous transfer has no blocked caller to come back to, so a buffer that would need a stand-in is refused for that path. Its callers are class drivers using kernel memory, which do not need one. The refusal is lifted once the completion path can do the copy itself. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 69 +++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index b8068948d68f3..64886b5454859 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -433,6 +433,10 @@ static ssize_t xhci_transfer_wait(FAR struct usbhost_xhci_s *priv, static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, FAR uint8_t *buffer, size_t buflen); static uint32_t xhci_speed_id(uint8_t speed); +#ifdef CONFIG_USBHOST_ASYNCH +static bool xhci_dma_direct(FAR struct usbhost_xhci_s *priv, + FAR uint8_t *buffer, size_t buflen); +#endif static inline FAR struct xhci_slot_ctx_s * xhci_in_slot(FAR struct usbhost_xhci_s *priv, FAR struct xhci_input_dev_ctx_s *input); @@ -2995,6 +2999,32 @@ static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, return priv->ops->dmacapable(priv->arg, buffer, buflen); } +#ifdef CONFIG_USBHOST_ASYNCH +/**************************************************************************** + * Name: xhci_dma_direct + * + * Description: + * Whether the controller can be pointed straight at this buffer, with no + * stand-in needed: an address it can reach, owning whole cache lines. + * + ****************************************************************************/ + +static bool xhci_dma_direct(FAR struct usbhost_xhci_s *priv, + FAR uint8_t *buffer, size_t buflen) +{ + size_t line = up_get_dcache_linesize(); + + if (!xhci_dmacapable(priv, buffer, buflen)) + { + return false; + } + + return line == 0 || + (((uintptr_t)buffer & (line - 1)) == 0 && + (buflen & (line - 1)) == 0); +} +#endif + /**************************************************************************** * Name: xhci_dma_prepare * @@ -3057,6 +3087,8 @@ static FAR uint8_t *xhci_dma_prepare(FAR struct usbhost_xhci_s *priv, if (!reachable || ((uintptr_t)buffer & (line - 1)) != 0 || (buflen & (line - 1)) != 0) { + /* A stand-in is needed; see xhci_dma_direct() for the same test */ + /* The buffer shares a line with something else. Work in a stand-in * that does not. */ @@ -3107,7 +3139,13 @@ static FAR uint8_t *xhci_dma_prepare(FAR struct usbhost_xhci_s *priv, * * Description: * Read back what the controller wrote, and give up any stand-in buffer. - * Called on completion, before whoever is waiting is woken. + * + * This must run in the context of whoever asked for the transfer, not in + * the completion handler. The buffer being copied back into may belong + * to a user process, and its address means nothing in the work queue + * thread that handles the completion event, where the write would fault + * or corrupt another process. The caller is blocked until the transfer + * finishes anyway. * ****************************************************************************/ @@ -3164,10 +3202,6 @@ static void xhci_transfer_complete(FAR struct usbhost_xhci_s *priv, epinfo = priv->devs[slot - 1].epinfo[ep - 1]; DEBUGASSERT(epinfo != NULL); - /* Read back what the controller wrote before anyone looks at it */ - - xhci_dma_finish(epinfo); - flags = spin_lock_irqsave(&priv->spinlock); /* Get transferred length */ @@ -4353,6 +4387,11 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, /* And wait for the transfer to complete */ nbytes = xhci_transfer_wait(priv, ep0info); + + /* As for bulk: the copy back belongs in the caller's context */ + + xhci_dma_finish(ep0info); + return nbytes >= 0 ? OK : (int)nbytes; errout_with_iocwait: @@ -4512,6 +4551,13 @@ static ssize_t xhci_transfer(FAR struct usbhost_driver_s *drvr, /* Then wait for the transfer to complete */ nbytes = xhci_transfer_wait(priv, epinfo); + + /* And bring back what it produced, here rather than in the completion, + * because this is the context the caller's buffer belongs to. + */ + + xhci_dma_finish(epinfo); + return nbytes; errout_with_iocwait: @@ -4569,6 +4615,19 @@ static int xhci_asynch(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep, DEBUGASSERT(priv && rhport && epinfo && buffer && buflen > 0); + /* An asynchronous transfer has no caller to come back to, so a buffer + * needing a stand-in cannot be used: the copy back out of it would have + * to happen in the completion handler, which runs in a work queue thread + * where a caller's address means nothing. The callers of this are class + * drivers using kernel memory, which needs no stand-in. + */ + + if (!xhci_dma_direct(priv, buffer, buflen)) + { + uerr("ERROR: asynchronous transfer needs a directly usable buffer\n"); + return -EFAULT; + } + /* We must have exclusive access to the xHCI hardware and data * structures. */ From 67d796deb4e9533fde5e6ac5f6d7891a7b2b4448 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:17:15 +0800 Subject: [PATCH 16/26] drivers/usbhost: Convert the xHCI endpoint interval from the descriptor. The Interval field of an endpoint context is an exponent: the controller services the endpoint every 2^Interval microframes. An endpoint descriptor states its period differently depending on device speed, so the number cannot be copied across, which is what this did. A low speed keyboard asking to be polled every 10ms was programmed as 2^10 microframes, which the controller would not accept: Configure Endpoint went unanswered and allocation failed with -EIO. Low and full speed interrupt endpoints state a period in frames, so the exponent is the highest bit of that period in microframes, clamped to the range the specification allows. Other periodic endpoints already state an exponent, one greater than the one wanted here. Control and bulk endpoints are not periodic and the field means nothing to them. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 62 ++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 64886b5454859..b8b29784ac5ca 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -3831,6 +3831,68 @@ static int xhci_ep0configure(FAR struct usbhost_driver_s *drvr, return ret; } +/**************************************************************************** + * Name: xhci_interval + * + * Description: + * Work out the Interval an endpoint context wants. + * + * The field is an exponent: the controller services the endpoint every + * 2^Interval microframes. An endpoint descriptor does not say it that + * way, and what it does say depends on how fast the device is, so the + * number cannot simply be copied across. + * + * A low or full speed interrupt endpoint counts in frames, so its period + * is bInterval milliseconds, or bInterval * 8 microframes, and the + * exponent is the position of the highest bit of that. Everything else + * that is periodic already states an exponent, one greater than the one + * wanted here. Control and bulk endpoints are not periodic and the field + * means nothing to them. + * + ****************************************************************************/ + +static uint8_t xhci_interval(uint8_t speed, uint8_t xfrtype, + uint8_t interval) +{ + unsigned int exp; + + if (xfrtype != USB_EP_ATTR_XFER_INT && xfrtype != USB_EP_ATTR_XFER_ISOC) + { + return 0; + } + + if ((speed == USB_SPEED_LOW || speed == USB_SPEED_FULL) && + xfrtype == USB_EP_ATTR_XFER_INT) + { + /* Frames. Round down to a power of two, and keep it inside what the + * specification allows for this kind of endpoint: 2^3 microframes is + * one frame, 2^10 is 128 of them. + */ + + if (interval == 0) + { + interval = 1; + } + + for (exp = 0; (1u << (exp + 1)) <= interval * 8u; exp++); + + if (exp < 3) + { + exp = 3; + } + else if (exp > 10) + { + exp = 10; + } + + return exp; + } + + /* Already an exponent, counted from one */ + + return interval > 0 ? interval - 1 : 0; +} + /**************************************************************************** * Name: xhci_epalloc * From 6682b21960c24e0442d0a5fbf71bda6b92ca12e4 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sun, 16 Aug 2026 17:17:15 +0800 Subject: [PATCH 17/26] drivers/usbhost: Check for the device before allocating an endpoint. A root hub port whose enumeration failed is enumerated again, and the slot the failed attempt used has been given back by then, so the port has no device context behind it. xhci_epalloc() took that pointer and wrote the new endpoint through it without looking, so the retry stored through NULL and took the system down in answer to a device that had merely failed to come up. Check for the device, and free the endpoint that has no home rather than leaking it. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index b8b29784ac5ca..b81bd0e35fe35 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -3971,16 +3971,31 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, epinfo->epno = epdesc->addr; #ifndef CONFIG_USBHOST_INT_DISABLE - epinfo->interval = epdesc->interval; + epinfo->interval = xhci_interval(hport->speed, epdesc->xfrtype, + epdesc->interval); #endif epinfo->xfrtype = epdesc->xfrtype; nxsem_init(&epinfo->iocsem, 0, 0); /* xhci_epno_get() returns Device Context Index (DCI) */ - idx = xhci_epno_get(epinfo); - mask = XHCI_IN_CTX1_A(XHCI_EP_FLAG(idx)); - dev = rhport->dev; + idx = xhci_epno_get(epinfo); + mask = XHCI_IN_CTX1_A(XHCI_EP_FLAG(idx)); + dev = rhport->dev; + + /* There has to be a device to hang the endpoint off. A port whose + * enumeration failed is retried after its slot has been given back, so + * this can run for a root hub port with nothing behind it. + */ + + if (dev == NULL) + { + uerr("no device on port %d\n", RHPNDX(rhport)); + nxsem_destroy(&epinfo->iocsem); + kmm_free(epinfo); + return -ENODEV; + } + dev->epinfo[idx - 1] = epinfo; /* TD rings already allocated but not connected yet. */ From b0d9bb9938aa41323e88ea41edf4b7f5fa2e0fc3 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Wed, 5 Aug 2026 23:31:26 +0800 Subject: [PATCH 18/26] 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 --- drivers/usbhost/usbhost_xhci.c | 124 ++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 58 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index b81bd0e35fe35..c8b4d542eab38 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, @@ -433,10 +435,6 @@ static ssize_t xhci_transfer_wait(FAR struct usbhost_xhci_s *priv, static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, FAR uint8_t *buffer, size_t buflen); static uint32_t xhci_speed_id(uint8_t speed); -#ifdef CONFIG_USBHOST_ASYNCH -static bool xhci_dma_direct(FAR struct usbhost_xhci_s *priv, - FAR uint8_t *buffer, size_t buflen); -#endif static inline FAR struct xhci_slot_ctx_s * xhci_in_slot(FAR struct usbhost_xhci_s *priv, FAR struct xhci_input_dev_ctx_s *input); @@ -2649,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. * @@ -2662,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) { @@ -2684,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 */ @@ -2698,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. * @@ -2709,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; @@ -2735,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. */ @@ -2999,32 +3024,6 @@ static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv, return priv->ops->dmacapable(priv->arg, buffer, buflen); } -#ifdef CONFIG_USBHOST_ASYNCH -/**************************************************************************** - * Name: xhci_dma_direct - * - * Description: - * Whether the controller can be pointed straight at this buffer, with no - * stand-in needed: an address it can reach, owning whole cache lines. - * - ****************************************************************************/ - -static bool xhci_dma_direct(FAR struct usbhost_xhci_s *priv, - FAR uint8_t *buffer, size_t buflen) -{ - size_t line = up_get_dcache_linesize(); - - if (!xhci_dmacapable(priv, buffer, buflen)) - { - return false; - } - - return line == 0 || - (((uintptr_t)buffer & (line - 1)) == 0 && - (buflen & (line - 1)) == 0); -} -#endif - /**************************************************************************** * Name: xhci_dma_prepare * @@ -3196,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 */ @@ -3257,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 } /**************************************************************************** @@ -4692,19 +4709,6 @@ static int xhci_asynch(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep, DEBUGASSERT(priv && rhport && epinfo && buffer && buflen > 0); - /* An asynchronous transfer has no caller to come back to, so a buffer - * needing a stand-in cannot be used: the copy back out of it would have - * to happen in the completion handler, which runs in a work queue thread - * where a caller's address means nothing. The callers of this are class - * drivers using kernel memory, which needs no stand-in. - */ - - if (!xhci_dma_direct(priv, buffer, buflen)) - { - uerr("ERROR: asynchronous transfer needs a directly usable buffer\n"); - return -EFAULT; - } - /* We must have exclusive access to the xHCI hardware and data * structures. */ @@ -4717,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; @@ -4857,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 From 1e65485562be0df7815821c5d361de51195e6a24 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 11:55:27 +0800 Subject: [PATCH 19/26] drivers/usbhost: Serialise xHCI transfers per endpoint. xhci_ctrl_xfer() and xhci_transfer() release the controller lock before xhci_transfer_wait(), so the lock does not cover the interval in which a transfer is outstanding. Two threads issuing requests on the same endpoint both reach xhci_ioc_setup(), and the second trips the DEBUGASSERT(!epinfo->iocwait) that guards it, or overwrites the first thread's completion state where assertions are compiled out. A default control endpoint reaches this readily: every interface driver on a composite device speaks through endpoint 0, so a two interface HID keyboard runs two poll threads both issuing GET_REPORT. Other host controller drivers hold the controller lock across the wait, which here would serialise the whole controller and give up the per endpoint rings xHCI provides. Add a mutex to struct xhci_epinfo_s and hold that instead. It is taken before the controller lock on both paths, so the order is endpoint then controller. xhci_epfree() also freed the endpoint container without destroying iocsem. Destroy both. Reachable on any xHCI controller, independently of the preceding commits. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index c8b4d542eab38..23e19e7a7301a 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -160,6 +160,16 @@ struct xhci_epinfo_s size_t dmacopy; /* Length to copy back out of a stand-in */ bool dmain; /* Direction this buffer was prepared for */ sem_t iocsem; /* Semaphore used to wait for transfer completion */ + + /* One transfer at a time on an endpoint. The controller lock below is + * released while a transfer is in flight, so it cannot serve this: two + * threads would each set up a transfer on the same endpoint and the + * second would find iocwait already set. A device's default control + * endpoint is the one that meets this, since every interface driver on + * a composite device speaks through it. + */ + + mutex_t exclsem; /* Serialises transfers on this endpoint */ #ifdef CONFIG_USBHOST_ASYNCH usbhost_asynch_t callback; /* Transfer complete callback */ FAR void *arg; /* Argument that accompanies the callback */ @@ -3993,6 +4003,7 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, #endif epinfo->xfrtype = epdesc->xfrtype; nxsem_init(&epinfo->iocsem, 0, 0); + nxmutex_init(&epinfo->exclsem); /* xhci_epno_get() returns Device Context Index (DCI) */ @@ -4008,6 +4019,7 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, if (dev == NULL) { uerr("no device on port %d\n", RHPNDX(rhport)); + nxmutex_destroy(&epinfo->exclsem); nxsem_destroy(&epinfo->iocsem); kmm_free(epinfo); return -ENODEV; @@ -4160,6 +4172,8 @@ static int xhci_epfree(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) /* Free the container */ + nxmutex_destroy(&epinfo->exclsem); + nxsem_destroy(&epinfo->iocsem); kmm_free(epinfo); return OK; } @@ -4404,6 +4418,17 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, DEBUGASSERT(rhport != NULL && ep0info != NULL && req != NULL); + /* One request at a time on this endpoint. Taken before the controller + * lock and held across the wait, so the ordering is always endpoint then + * controller and never the reverse. + */ + + ret = nxmutex_lock(&ep0info->exclsem); + if (ret < 0) + { + return ret; + } + len = xhci_getle16(req->len); /* Terse output only if we are tracing */ @@ -4446,6 +4471,7 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, xhci_out_slot(rhport->dev->ctx)->ctx[3]; } + nxmutex_unlock(&ep0info->exclsem); return OK; } @@ -4456,6 +4482,7 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, ret = nxmutex_lock(&priv->lock); if (ret < 0) { + nxmutex_unlock(&ep0info->exclsem); return ret; } @@ -4486,12 +4513,14 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, xhci_dma_finish(ep0info); + nxmutex_unlock(&ep0info->exclsem); return nbytes >= 0 ? OK : (int)nbytes; errout_with_iocwait: ep0info->iocwait = false; errout_with_lock: nxmutex_unlock(&priv->lock); + nxmutex_unlock(&ep0info->exclsem); return ret; } @@ -4584,6 +4613,16 @@ static ssize_t xhci_transfer(FAR struct usbhost_driver_s *drvr, DEBUGASSERT(priv && rhport && epinfo && buffer && buflen > 0); + /* One transfer at a time on this endpoint, taken before the controller + * lock and held across the wait. See the note beside exclsem. + */ + + ret = nxmutex_lock(&epinfo->exclsem); + if (ret < 0) + { + return (ssize_t)ret; + } + /* We must have exclusive access to the xHCI hardware and data * structures. */ @@ -4591,6 +4630,7 @@ static ssize_t xhci_transfer(FAR struct usbhost_driver_s *drvr, ret = nxmutex_lock(&priv->lock); if (ret < 0) { + nxmutex_unlock(&epinfo->exclsem); return (ssize_t)ret; } @@ -4652,12 +4692,14 @@ static ssize_t xhci_transfer(FAR struct usbhost_driver_s *drvr, xhci_dma_finish(epinfo); + nxmutex_unlock(&epinfo->exclsem); return nbytes; errout_with_iocwait: epinfo->iocwait = false; errout_with_lock: nxmutex_unlock(&priv->lock); + nxmutex_unlock(&epinfo->exclsem); return (ssize_t)ret; } @@ -5351,6 +5393,7 @@ static inline int xhci_sw_initialize(FAR struct usbhost_xhci_s *priv) rhport->ep0.epno = 0; rhport->ep0.devaddr = 0; nxsem_init(&rhport->ep0.iocsem, 0, 0); + nxmutex_init(&rhport->ep0.exclsem); /* Initialize the public port representation */ From 471a04c1a3b3ab6278995add5d8061f500254389 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 12:37:50 +0800 Subject: [PATCH 20/26] drivers/usbhost: Release the xHCI slot when enumeration fails. A device slot is a finite controller resource: HCSPARAMS1 reports how many exist and Enable Slot fails with No Slots Available once they are gone. Two paths took one and returned without giving it back. xhci_device_init() enables a slot before initialising the transfer ring, the slot context and the device address, and each of those returned directly on failure. It also treated a slot number larger than the controller supports as success, since Enable Slot itself had succeeded. xhci_enumerate() is the larger leak: the device is addressed by the time usbhost_enumerate() runs, so a device whose descriptor cannot be read, or that no class driver claims, leaves the slot held. That path clears hport->connected so the port is retried, taking another slot each time. Release the slot on both paths with xhci_device_deinit(), which issues Disable Slot, clears the DCBAA entry and resets the context. The endpoint ring is left allocated; xhci_ring_init() reuses an existing one. Tested on an EIC7700X board with a device no class driver claims, so the port retries indefinitely: previously the eighth attempt failed with completion code 9 and the controller enumerated nothing further on either port; now 1104 consecutive attempts produced no slot failure. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 42 ++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 23e19e7a7301a..25603cae07dee 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -1810,7 +1810,15 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, ret = xhci_cmd_sloten(priv, &slot); if (ret < 0 || slot > priv->no_slots) { - /* Something goes wrong ! */ + /* A slot the controller cannot address is no more usable than no + * slot at all, and the command itself succeeds in that case, so the + * caller needs an error either way. + */ + + if (ret >= 0) + { + ret = -EINVAL; + } usbhost_vtrace1(XHCI_TRACE1_SLOTEN_FAILED, ret); return ret; @@ -1834,7 +1842,7 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, if (ret < 0) { uerr("ep0 ring init failed\n"); - return ret; + goto errout_with_slot; } rhport->ep0.slot = slot; @@ -1845,7 +1853,7 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, ret = xhci_slot_init(priv, dev); if (ret < 0) { - return ret; + goto errout_with_slot; } /* Step 6: Assign and address to the device and enable its Default @@ -1860,12 +1868,21 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, if (ret < 0) { uerr("failed to set address %d\n", ret); - return ret; + goto errout_with_slot; } /* Steps 7-12 don't belong here! */ return OK; + +errout_with_slot: + + /* Nothing else gives the slot back, and the controller has a fixed + * number of them. + */ + + xhci_device_deinit(priv, rhport); + return ret; } /**************************************************************************** @@ -3770,6 +3787,23 @@ static int xhci_enumerate(FAR struct usbhost_connection_s *conn, { /* Failed to enumerate */ + /* The device is addressed by now, so it holds a slot, and the retry + * below asks for another. + */ + +#ifdef CONFIG_USBHOST_HUB + if (ROOTHUB(hport)) +#endif + { + FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_CONN(conn); + FAR struct xhci_rhport_s *rhport = &priv->rhport[hport->port]; + + if (rhport->dev != NULL) + { + xhci_device_deinit(priv, rhport); + } + } + /* If this is a root hub port, then marking the hub port not connected * will cause xhci_wait() to return and we will try the connection * again. From 5e77cd2efa572882914a173af45df53c9174f134 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 12:55:35 +0800 Subject: [PATCH 21/26] drivers/usbhost: Stop retrying an xHCI port that will not enumerate. xhci_enumerate() reports failure by marking the hub port disconnected, which is what makes xhci_wait() return and the attempt repeat. The root port is still connected, so the two disagree again immediately and the attempt repeats for as long as the device stays plugged in. A device that fails every time is retried forever: 1055 attempts in 90 seconds on an EIC7700X board, enough console traffic to make the board unusable. Count consecutive failures per root port and stop at CONFIG_USBHOST_XHCI_ENUM_RETRIES, leaving the port as it is so xhci_wait() blocks until something physically changes. A new connection clears the count, as does a successful enumeration, so a device needing a second attempt still gets one. The default of three rides out a slow device or a marginal reset. The same board now makes three attempts, reports that it has given up and falls silent, while a keyboard on the other port enumerates throughout. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/Kconfig | 16 ++++++++++++++++ drivers/usbhost/usbhost_xhci.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/drivers/usbhost/Kconfig b/drivers/usbhost/Kconfig index ccc6e63022d33..fbde6e51f59e6 100644 --- a/drivers/usbhost/Kconfig +++ b/drivers/usbhost/Kconfig @@ -832,6 +832,22 @@ config USBHOST_XHCI_MAX_DEVS ---help--- How many USB devices will be supported by xHCI driver. +config USBHOST_XHCI_ENUM_RETRIES + int "xHCI enumeration attempts per port" + default 3 + range 1 255 + ---help--- + How many times to attempt enumeration of a newly connected device + before leaving the port alone until the device is unplugged. + + A device whose descriptors cannot be read, or that no class driver + claims, fails enumeration every time. Each failure marks the port + disconnected so the attempt repeats, so without a limit such a + device is retried for as long as it stays plugged in, logging and + taking a device slot on every pass. + + The count is per root hub port and is cleared by a new connection. + endif # USBHOST_XHCI menuconfig USBHOST_XHCI_PCI diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 25603cae07dee..261ece9ce047c 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -199,6 +199,7 @@ struct xhci_rhport_s /* Root hub port status */ bool connected; /* Connected to device */ + uint8_t enumfail; /* Consecutive failed enumerations */ int8_t slot; /* Slot ID associated with this port */ struct xhci_epinfo_s ep0; /* EP0 endpoint info */ struct usbhost_roothubport_s hport; /* This is the hub port description understood @@ -2882,6 +2883,12 @@ static void xhci_portsc_work(FAR void *arg) rhport->connected = true; + /* A new device gets the full allowance of attempts, + * whatever the last one that sat here managed. + */ + + rhport->enumfail = 0; + usbhost_vtrace2(XHCI_VTRACE2_PORTSC_CONNECTED, rhpndx + 1, priv->pscwait); @@ -3802,6 +3809,18 @@ static int xhci_enumerate(FAR struct usbhost_connection_s *conn, { xhci_device_deinit(priv, rhport); } + + /* Clearing connected below is what makes xhci_wait() return, + * so it is also what repeats the attempt. Leave the port alone + * past the limit; a new connection clears the count. + */ + + if (++rhport->enumfail >= CONFIG_USBHOST_XHCI_ENUM_RETRIES) + { + syslog(LOG_ERR, "%s: port %d: giving up after %d attempts\n", + priv->name, hport->port + 1, rhport->enumfail); + return ret; + } } /* If this is a root hub port, then marking the hub port not connected @@ -3811,6 +3830,17 @@ static int xhci_enumerate(FAR struct usbhost_connection_s *conn, hport->connected = false; } + else + { +#ifdef CONFIG_USBHOST_HUB + if (ROOTHUB(hport)) +#endif + { + FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_CONN(conn); + + priv->rhport[hport->port].enumfail = 0; + } + } return ret; } From 407881eaec946866e984857772833b79bf413c62 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 14:42:19 +0800 Subject: [PATCH 22/26] drivers/usbhost: Key an xHCI device by its port, not by the root port. The slot, the default control endpoint and the device context lived in struct xhci_rhport_s, and anything needing a device reached it as rhport->dev. That holds only while every device is plugged straight into the controller; a hub puts several behind one root port, each with its own slot and context. Two keys replace it. An endpoint records the slot it was opened on, so xhci_dev_from_ep() answers which device a transfer belongs to. A hub port belongs to one device wherever it sits, so xhci_dev_from_hport() answers which device is on a port when there is no endpoint to ask yet. The functions converted here used both at once: xhci_ep0configure() issued Evaluate Context for epinfo->slot while filling in rhport->dev's context, and xhci_ctrl_xfer() reached the endpoint ring through the port and back. xhci_slot_init() read the speed and control ring through the port, which would fail quietly, since the slot context speed field has no valid zero and a low speed device behind a high speed hub does not share its speed. No functional change for a directly attached device: its port's slot and its endpoint's slot are the same, and its hub port is the root port's own. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 112 ++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 28 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 261ece9ce047c..83c220804a65c 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -219,6 +219,13 @@ struct xhci_dev_s FAR struct xhci_input_dev_ctx_s *input; /* Input Device Context. Input to xHC */ FAR struct xhci_rhport_s *rhport; /* Root Hub Port associated with this device */ + /* The port this device is attached to. Several devices can share a root + * hub port once a hub is in between, so this, and not the port above, is + * what identifies a device to the class drivers. + */ + + FAR struct usbhost_hubport_s *hport; + /* Reference to allocated endpoints */ FAR struct xhci_epinfo_s *epinfo[XHCI_MAX_ENDPOINTS]; @@ -1692,8 +1699,7 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, */ regval = XHCI_ST_CTX0_CTXENT_SET(1) | - XHCI_ST_CTX0_SPEED_SET( - xhci_speed_id(dev->rhport->hport.hport.speed)); + XHCI_ST_CTX0_SPEED_SET(xhci_speed_id(dev->hport->speed)); #ifdef CONFIG_USBHOST_HUB /* TODO: @@ -1720,12 +1726,13 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, * allocated. */ - drdp = up_addrenv_va_to_pa(dev->rhport->ep0.td.ring); + DEBUGASSERT(dev->epinfo[0] != NULL); + drdp = up_addrenv_va_to_pa(dev->epinfo[0]->td.ring); /* Step 5. Initialize the Input default control Endpoint 0 Context */ - DEBUGASSERT(dev->rhport != NULL); - if (dev->rhport->hport.hport.speed == USB_SPEED_HIGH) + DEBUGASSERT(dev->hport != NULL); + if (dev->hport->speed == USB_SPEED_HIGH) { /* For high-speed, we must use 64 bytes */ @@ -1848,6 +1855,7 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, rhport->ep0.slot = slot; dev->rhport = rhport; + dev->hport = &rhport->hport.hport; dev->slot = slot; dev->epinfo[0] = &rhport->ep0; @@ -1931,6 +1939,7 @@ static int xhci_device_deinit(FAR struct usbhost_xhci_s *priv, /* Remove reference to a device slot */ + rhport->dev->hport = NULL; rhport->dev = NULL; return OK; @@ -2998,6 +3007,56 @@ xhci_out_slot(FAR struct xhci_dev_ctx_s *ctx) return (FAR struct xhci_slot_ctx_s *)ctx; } +/**************************************************************************** + * Name: xhci_dev_from_ep + * + * Description: + * The device an endpoint belongs to. + * + * An endpoint records the slot it was opened on, and the slot indexes the + * device table, so this holds wherever the device sits. The root hub port + * does not: a class driver reaches the controller through the port it + * descends from, and a hub puts several devices behind one such port. + * + ****************************************************************************/ + +static inline FAR struct xhci_dev_s * +xhci_dev_from_ep(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_epinfo_s *epinfo) +{ + DEBUGASSERT(epinfo->slot > 0 && epinfo->slot <= priv->no_slots); + return &priv->devs[epinfo->slot - 1]; +} + +/**************************************************************************** + * Name: xhci_dev_from_hport + * + * Description: + * The device attached to a hub port, or NULL if there is none. + * + * Used where there is no endpoint to ask yet, which is the case when the + * first one is being allocated. + * + ****************************************************************************/ + +static FAR struct xhci_dev_s * +xhci_dev_from_hport(FAR struct usbhost_xhci_s *priv, + FAR struct usbhost_hubport_s *hport) +{ + uint8_t i; + + for (i = 0; i < priv->no_slots; i++) + { + if (priv->devs[i].state != XHCI_SLOT_DISABLED && + priv->devs[i].hport == hport) + { + return &priv->devs[i]; + } + } + + return NULL; +} + /**************************************************************************** * Name: xhci_speed_id * @@ -3876,42 +3935,44 @@ static int xhci_ep0configure(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep0, uint8_t funcaddr, uint8_t speed, uint16_t maxpacketsize) { - FAR struct xhci_rhport_s *rhport = (FAR struct xhci_rhport_s *)drvr; FAR struct xhci_epinfo_s *epinfo = (FAR struct xhci_epinfo_s *)ep0; FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_DRVR(drvr); + FAR struct xhci_dev_s *dev; uint64_t ctx; int ret; DEBUGASSERT(drvr != NULL && epinfo != NULL && maxpacketsize < 2048); + dev = xhci_dev_from_ep(priv, epinfo); + ret = nxmutex_lock(&priv->lock); if (ret >= 0) { /* Update max packet size */ FAR struct xhci_ep_ctx_s *ep0ctx = - xhci_in_ep(priv, rhport->dev->input, 0); + xhci_in_ep(priv, dev->input, 0); ep0ctx->ctx1 &= ~XHCI_EP_CTX1_MAXPKT_MASK; ep0ctx->ctx1 |= XHCI_EP_CTX1_MAXPKT(maxpacketsize); /* Add Slot Context and EP0 Context */ - xhci_context_ctrl(priv, rhport->dev, 0, + xhci_context_ctrl(priv, dev, 0, XHCI_IN_CTX1_A(XHCI_SLOT_FLAG) | XHCI_IN_CTX1_A(XHCI_EP0_FLAG)); /* Flush Device input context */ - up_flush_dcache((uintptr_t)rhport->dev->input, - (uintptr_t)rhport->dev->input + + up_flush_dcache((uintptr_t)dev->input, + (uintptr_t)dev->input + XHCI_INCTX_SIZE(priv)); /* Free mutex before command execution */ nxmutex_unlock(&priv->lock); - ctx = up_addrenv_va_to_pa(rhport->dev->input); + ctx = up_addrenv_va_to_pa(dev->input); uinfo("slot %d funcaddr %d speed %d maxpacket %d\n", epinfo->slot, funcaddr, speed, maxpacketsize); @@ -4011,7 +4072,6 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, FAR usbhost_ep_t *ep) { FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_DRVR(drvr); - FAR struct xhci_rhport_s *rhport = (FAR struct xhci_rhport_s *)drvr; FAR struct usbhost_hubport_s *hport; FAR struct xhci_epinfo_s *epinfo; FAR struct xhci_dev_s *dev; @@ -4028,12 +4088,6 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, && ep != NULL); hport = epdesc->hport; - /* Only the tracing alternative below and the hub logic further down use - * this, and a configuration may have neither. - */ - - UNUSED(hport); - /* Terse output only if we are tracing */ #ifdef CONFIG_USBHOST_TRACE @@ -4073,16 +4127,16 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, idx = xhci_epno_get(epinfo); mask = XHCI_IN_CTX1_A(XHCI_EP_FLAG(idx)); - dev = rhport->dev; + dev = xhci_dev_from_hport(priv, hport); /* There has to be a device to hang the endpoint off. A port whose * enumeration failed is retried after its slot has been given back, so - * this can run for a root hub port with nothing behind it. + * this can run for a port with nothing behind it. */ if (dev == NULL) { - uerr("no device on port %d\n", RHPNDX(rhport)); + uerr("no device on port %d\n", hport->port); nxmutex_destroy(&epinfo->exclsem); nxsem_destroy(&epinfo->iocsem); kmm_free(epinfo); @@ -4102,7 +4156,7 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, /* Store slot ID for later */ - epinfo->slot = rhport->slot; + epinfo->slot = dev->slot; #ifdef CONFIG_USBHOST_HUB if (hport->speed != USB_SPEED_HIGH) @@ -4511,28 +4565,30 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, * on control EP. */ - xhci_ring_init(&rhport->dev->rhport->ep0.td, 0); + xhci_ring_init(&ep0info->td, 0); /* Issue SET_ADDRESS request */ ret = xhci_address_set(priv, rhport, true); if (ret == OK) { + FAR struct xhci_dev_s *dev = xhci_dev_from_ep(priv, ep0info); + /* The controller chose this address and wrote it into the * output context. Invalidate before reading, or the stale * copy is used. */ - up_invalidate_dcache((uintptr_t)rhport->dev->ctx, - (uintptr_t)rhport->dev->ctx + + up_invalidate_dcache((uintptr_t)dev->ctx, + (uintptr_t)dev->ctx + XHCI_DEVCTX_SIZE(priv)); /* Store USB Device Address assigned by xHCI */ ep0info->devaddr = - XHCI_ST_CTX3_ADDR_GET(xhci_out_slot(rhport->dev->ctx)->ctx[3]); - xhci_in_slot(priv, rhport->dev->input)->ctx[3] = - xhci_out_slot(rhport->dev->ctx)->ctx[3]; + XHCI_ST_CTX3_ADDR_GET(xhci_out_slot(dev->ctx)->ctx[3]); + xhci_in_slot(priv, dev->input)->ctx[3] = + xhci_out_slot(dev->ctx)->ctx[3]; } nxmutex_unlock(&ep0info->exclsem); From 047a5a0cce6182d668bc58fefb7138b47b66b1dc Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 13:39:53 +0800 Subject: [PATCH 23/26] usbhost: Report what a hub is on the port it occupies. Some host controllers must be told about the hubs in a topology, not only about the device at the end of it. xHCI is one: a hub's slot context carries a hub flag, its downstream port count and the think time of its transaction translator, and the controller routes to anything behind that hub using them. The hub class driver already reads both values from the hub descriptor and keeps them privately. Publish them on the hub's own hub port, beside the speed and function address that already describe the device attached there. A driver setting up a device behind a hub finds them on that device's parent. They are written before the hub activates any downstream port, so they are in place before there is anything behind it, and a port with no hub reports zero ports because the hub class clears each child before use. Nothing is required to read them. Fields rather than a driver method: a method would need a null check at the call site and would define an order it must be called in. Both are inside CONFIG_USBHOST_HUB, as struct usbhost_hubport_s's parent pointer already is. Multi-TT is not included; it comes from the hub's interface protocol rather than its descriptor, and driving a multi-TT hub as single-TT costs bandwidth behind it but is correct. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_hub.c | 11 +++++++++++ include/nuttx/usb/usbhost.h | 15 +++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/drivers/usbhost/usbhost_hub.c b/drivers/usbhost/usbhost_hub.c index 52032ee87dd74..a2ea159de000d 100644 --- a/drivers/usbhost/usbhost_hub.c +++ b/drivers/usbhost/usbhost_hub.c @@ -578,6 +578,17 @@ static inline int usbhost_hubdesc(FAR struct usbhost_class_s *hubclass) priv->pwrondelay = (2 * hubdesc->pwrondelay); priv->ctrlcurrent = hubdesc->ctrlcurrent; + /* Publish what describes this hub as a hub, rather than as a device, on + * the port it occupies. A host controller that has to be told about the + * hubs in a topology reads it from there when it sets up a device behind + * this one. This runs before any downstream port is activated, so it is + * in place before there is anything behind it to set up. + */ + + hport->nports = hubdesc->nports; + hport->ttt = (hubchar & USBHUB_CHAR_TTTT_MASK) >> + USBHUB_CHAR_TTTT_SHIFT; + uinfo("Hub Descriptor:\n"); uinfo(" bDescLength: %d\n", hubdesc->len); uinfo(" bDescriptorType: 0x%02x\n", hubdesc->type); diff --git a/include/nuttx/usb/usbhost.h b/include/nuttx/usb/usbhost.h index 3a43bb3a4aeb5..27521f2055fd1 100644 --- a/include/nuttx/usb/usbhost.h +++ b/include/nuttx/usb/usbhost.h @@ -721,6 +721,21 @@ struct usbhost_hubport_s uint8_t port; /* Hub port index */ uint8_t funcaddr; /* Device function address */ uint8_t speed; /* Device speed */ +#ifdef CONFIG_USBHOST_HUB + /* Set by the hub class driver when the device attached here is itself a + * hub, describing that hub rather than this port. Both are zero + * otherwise, and zero ports is not a hub. + * + * A host controller that has to be told about the hubs in a topology, + * rather than only about the device at the end of it, reads these from + * the parent of the port it is working on. They are set before the hub + * activates any downstream port, so they are in place before anything + * behind that hub can be enumerated. + */ + + uint8_t nports; /* Downstream ports on the attached hub */ + uint8_t ttt; /* Its transaction translator think time */ +#endif }; /* The root hub port differs in that it includes a data set that is used to From baef606103bfb15df942f403c28df3136a4e67b6 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 14:42:38 +0800 Subject: [PATCH 24/26] drivers/usbhost: Describe a device behind a hub to the xHCI controller. A controller reaches a device by the path to it and, for a slow device, through the hub that translates for it. Neither was described, so a device behind a hub was addressed as though it were on the root port. The route string is that path: each hub between the device and the root contributes a nibble holding the port the next thing down occupies, tier nearest the root in the lowest nibble. Walking up from the device reaches the deepest tier first, so shifting left by a nibble each time leaves them in the order the field wants. The walk stops after five, which is what the field holds and what USB allows, and a port above fifteen is clamped. Slot context dword 2 names the transaction translator carrying a low or full speed device behind a high speed hub. It reports the hub by slot, where EHCI reports it by USB address, and it names the nearest high speed ancestor rather than the immediate parent, since a full speed hub below a high speed one is itself carried by the translator above it. The think time comes from the hub descriptor by way of the hub class driver, in the same units. xhci_epalloc() carried a copy of sam_ehci.c's block, writing epinfo->hubaddr and epinfo->hubport, which is how EHCI describes a split transaction in its queue head. This driver never read either field, and xHCI wants the information in the slot context. Both fields and the code setting them are removed. Multi-TT is not set, for the reason given in the previous commit. No functional change: hubs cannot be enabled yet, and a device on a root port has neither hubs above it nor a translator. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 162 ++++++++++++++++++++++++++------- drivers/usbhost/usbhost_xhci.h | 21 +++++ 2 files changed, 149 insertions(+), 34 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 83c220804a65c..18a03b61a5bd6 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -176,13 +176,6 @@ struct xhci_epinfo_s #endif struct xhci_ring_s td; /* TD ring for this endpoint */ uint8_t slot; /* Slot where this EP resides */ - - /* These fields are used in the split-transaction protocol. */ - - uint8_t hubaddr; /* USB device address of the high-speed hub below - * which a full/low-speed device is attached. - */ - uint8_t hubport; /* The port on the above high-speed hub. */ }; /* This structure retains the state of one root hub port */ @@ -394,6 +387,11 @@ static int xhci_address_set(FAR struct usbhost_xhci_s *priv, FAR struct xhci_rhport_s *rhport, bool setaddr); static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, FAR struct xhci_dev_s *dev); +#ifdef CONFIG_USBHOST_HUB +static uint32_t xhci_route_string(FAR struct usbhost_hubport_s *hport); +static uint32_t xhci_slot_tt(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_dev_s *dev); +#endif static int xhci_device_init(FAR struct usbhost_xhci_s *priv, FAR struct xhci_rhport_s *rhport); static int xhci_device_deinit(FAR struct usbhost_xhci_s *priv, @@ -1702,10 +1700,11 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, XHCI_ST_CTX0_SPEED_SET(xhci_speed_id(dev->hport->speed)); #ifdef CONFIG_USBHOST_HUB + regval |= XHCI_ST_CTX0_RTSTR_SET(xhci_route_string(dev->hport)); + /* TODO: * 1. Activate the transaction translator if required * 2. Configure hub bit in slot context if hub - * 3. configure route string */ # warning missing logic @@ -1722,6 +1721,10 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, regval |= XHCI_ST_CTX1_PORTS_SET(0); xhci_in_slot(priv, dev->input)->ctx[1] = htole32(regval); +#ifdef CONFIG_USBHOST_HUB + xhci_in_slot(priv, dev->input)->ctx[2] = htole32(xhci_slot_tt(priv, dev)); +#endif + /* Step 4. the Transfer Ring for the Default Control Endpoint is already * allocated. */ @@ -3039,6 +3042,58 @@ xhci_dev_from_ep(FAR struct usbhost_xhci_s *priv, * ****************************************************************************/ +#ifdef CONFIG_USBHOST_HUB +/**************************************************************************** + * Name: xhci_route_string + * + * Description: + * The route string for a device, which is how the controller finds it. + * + * Each hub between the root and the device contributes one nibble holding + * the number of the port the next thing down is plugged into, with the + * tier nearest the root in the lowest nibble. A device on a root hub port + * routes to zero, which is what the field means for "no hubs in between". + * + * Reference: + * - 8.9: Route String Field + * + ****************************************************************************/ + +static uint32_t xhci_route_string(FAR struct usbhost_hubport_s *hport) +{ + uint32_t route = 0; + int tier = 0; + + /* Walking up reaches the deepest tier first, and shifting what is already + * there left by a nibble each time leaves the tier nearest the root in the + * lowest one. USB allows five tiers of hubs and the field holds exactly + * that many, so a chain longer than the bus permits stops here rather than + * writing over the speed field above it. + */ + + while (hport->parent != NULL && tier < 5) + { + uint8_t portno = hport->port + 1; + + /* The nibble cannot express a port above fifteen. A hub that large + * is legal, so clamp rather than let the number wrap into the tier + * below it. + */ + + if (portno > 15) + { + portno = 15; + } + + route = (route << 4) | portno; + hport = hport->parent; + tier++; + } + + return route; +} +#endif + static FAR struct xhci_dev_s * xhci_dev_from_hport(FAR struct usbhost_xhci_s *priv, FAR struct usbhost_hubport_s *hport) @@ -3057,6 +3112,71 @@ xhci_dev_from_hport(FAR struct usbhost_xhci_s *priv, return NULL; } +#ifdef CONFIG_USBHOST_HUB +/**************************************************************************** + * Name: xhci_slot_tt + * + * Description: + * Slot context dword 2, naming the transaction translator that carries a + * low or full speed device behind a high speed hub. Zero when no + * translator is involved, which is what the field means. + * + * Reference: + * - 6.2.2: Slot Context + * + ****************************************************************************/ + +static uint32_t xhci_slot_tt(FAR struct usbhost_xhci_s *priv, + FAR struct xhci_dev_s *dev) +{ + FAR struct usbhost_hubport_s *hport = dev->hport; + FAR struct xhci_dev_s *tthub; + + /* Only a low or full speed device is translated for. */ + + if (hport->speed == USB_SPEED_HIGH) + { + return 0; + } + + /* The translator lives in the nearest high speed ancestor, which need not + * be the hub the device is plugged into: a full speed hub below a high + * speed one is itself carried by the translator above it. + */ + + while (hport->parent != NULL && hport->parent->speed != USB_SPEED_HIGH) + { + hport = hport->parent; + } + + if (hport->parent == NULL) + { + /* Nothing high speed above, so the device is on a root hub port or + * the whole chain runs at its speed. Either way there is no + * translator to name. + */ + + return 0; + } + + tthub = xhci_dev_from_hport(priv, hport->parent); + if (tthub == NULL) + { + uerr("no device for the hub carrying port %d\n", hport->port); + return 0; + } + + /* Think time is the hub's, reported by the hub class driver from the hub + * descriptor. Both fields count in the same units, so the value carries + * across unchanged. + */ + + return XHCI_ST_CTX2_TTHSID_SET(tthub->slot) | + XHCI_ST_CTX2_TTPORT_SET(hport->port + 1) | + XHCI_ST_CTX2_TTT_SET(hport->parent->ttt); +} +#endif + /**************************************************************************** * Name: xhci_speed_id * @@ -4158,32 +4278,6 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, epinfo->slot = dev->slot; -#ifdef CONFIG_USBHOST_HUB - if (hport->speed != USB_SPEED_HIGH) - { - /* A high speed hub exists between this device and the root hub - * otherwise we would not get here. - */ - - FAR struct usbhost_hubport_s *parent = hport->parent; - - for (; parent->speed != USB_SPEED_HIGH; parent = hport->parent) - { - hport = parent; - } - - if (parent->speed == USB_SPEED_HIGH) - { - epinfo->hubport = HPORT(hport); - epinfo->hubaddr = hport->parent->funcaddr; - } - else - { - return -EINVAL; - } - } -#endif - /* Get EP type */ switch (epinfo->xfrtype) diff --git a/drivers/usbhost/usbhost_xhci.h b/drivers/usbhost/usbhost_xhci.h index 3339721895d95..157e07a9db43b 100644 --- a/drivers/usbhost/usbhost_xhci.h +++ b/drivers/usbhost/usbhost_xhci.h @@ -504,6 +504,8 @@ #define XHCI_ST_CTX0_RTSTR_SHIFT (0) /* Bits 0:19: Route String */ #define XHCI_ST_CTX0_RTSTR_MASK (0xfffff << XHCI_ST_CTX0_RTSTR_SHIFT) +#define XHCI_ST_CTX0_RTSTR_SET(x) (((x) << XHCI_ST_CTX0_RTSTR_SHIFT) & \ + XHCI_ST_CTX0_RTSTR_MASK) #define XHCI_ST_CTX0_SPEED_SHIFT (20) /* Bits 20:23: Speed */ #define XHCI_ST_CTX0_SPEED_MASK (0xf << XHCI_ST_CTX0_SPEED_SHIFT) #define XHCI_ST_CTX0_SPEED_SET(x) (((x) << XHCI_ST_CTX0_SPEED_SHIFT) & \ @@ -535,6 +537,25 @@ #define XHCI_ST_CTX1_PORTS_MASK (0xff << XHCI_ST_CTX1_PORTS_SHIFT) #define XHCI_ST_CTX1_PORTS_SET(x) (((x) << XHCI_ST_CTX1_PORTS_SHIFT) & XHCI_ST_CTX1_PORTS_MASK) +/* Slot Context dword 2 describes the transaction translator that carries a + * low or full speed device sitting behind a high speed hub. It names the + * nearest high speed ancestor, which is the hub whose TT does the work, and + * not the hub the device is plugged into if those differ. + */ + +#define XHCI_ST_CTX2_TTHSID_SHIFT (0) /* Bit 0-7: TT Hub Slot ID */ +#define XHCI_ST_CTX2_TTHSID_MASK (0xff << XHCI_ST_CTX2_TTHSID_SHIFT) +#define XHCI_ST_CTX2_TTHSID_SET(x) (((x) << XHCI_ST_CTX2_TTHSID_SHIFT) & \ + XHCI_ST_CTX2_TTHSID_MASK) +#define XHCI_ST_CTX2_TTPORT_SHIFT (8) /* Bit 8-15: TT Port Number */ +#define XHCI_ST_CTX2_TTPORT_MASK (0xff << XHCI_ST_CTX2_TTPORT_SHIFT) +#define XHCI_ST_CTX2_TTPORT_SET(x) (((x) << XHCI_ST_CTX2_TTPORT_SHIFT) & \ + XHCI_ST_CTX2_TTPORT_MASK) +#define XHCI_ST_CTX2_TTT_SHIFT (16) /* Bit 16-17: TT Think Time */ +#define XHCI_ST_CTX2_TTT_MASK (0x3 << XHCI_ST_CTX2_TTT_SHIFT) +#define XHCI_ST_CTX2_TTT_SET(x) (((x) << XHCI_ST_CTX2_TTT_SHIFT) & \ + XHCI_ST_CTX2_TTT_MASK) + #define XHCI_ST_CTX3_ADDR_SHIFT (0) /* Bit 0-7: USB Device Address */ #define XHCI_ST_CTX3_ADDR_MASK (0xff << XHCI_ST_CTX3_ADDR_SHIFT) #define XHCI_ST_CTX3_ADDR_SET(x) (((x) << XHCI_ST_CTX3_ADDR_SHIFT) & XHCI_ST_CTX3_ADDR_MASK) From 0f90af5bb521c0869d28c3bdf0c188e2d32e46d3 Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 14:42:59 +0800 Subject: [PATCH 25/26] drivers/usbhost: Support USB hubs on xHCI. The driver refused CONFIG_USBHOST_HUB outright. Everything needed to describe a device behind a hub is now in place, so implement the rest. - xhci_device_init() took a root hub port and read the slot, the control endpoint and the device out of it, all of which belong to the device. It now takes the hub port and the control endpoint, and records the device on the root port only when that is where it sits: once a hub is plugged in, the device a root port names is the hub. xhci_address_set() and xhci_device_deinit() likewise work on a device, and xhci_disconnect() finds the device by the port going away. - The hub asks for a port's control endpoint before it reports the connection, so xhci_epalloc() has nothing to attach one to. It returns an endpoint with no slot, and xhci_connect() gives it one when it creates the device. - A hub must be described to the controller as a hub before anything behind it can be reached, and nothing knows it is one when its slot is created. xhci_hub_update() corrects the slot context with a Configure Endpoint command the first time something appears behind it. - A hub reports each changed port without waiting for the last to be dealt with, so the connect method queues them; holding one pointer meant the second report overwrote the first. No more can be outstanding than the controller has slots. - Report the root port and slot counts from HCSPARAMS1, and the port count from a hub's descriptor. Tested on an EIC7700X board with a hub on one controller and a keyboard on the other. Behind the hub, a 59 GB mass storage device mounts and reads a file back, and a composite CDC device gives four ttyACM nodes. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 343 +++++++++++++++++++++++++++------ 1 file changed, 283 insertions(+), 60 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 18a03b61a5bd6..09e4015dcd804 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -58,12 +58,6 @@ # error Invalid value for CONFIG_USBHOST_XHCI_MAX_DEVS #endif -/* USB HUB support is not yet implemented */ - -#ifdef CONFIG_USBHOST_HUB -# error XHCI USB HUB support is not yet implemented -#endif - /* Some constants for this implementation */ #define XHCI_MAX_ERST (1) @@ -219,6 +213,13 @@ struct xhci_dev_s FAR struct usbhost_hubport_s *hport; + /* True once the controller has been told this device is a hub. It is not + * known when the slot is created: the hub descriptor is read later, and + * only then does anything know how many ports it has. + */ + + bool ishub; + /* Reference to allocated endpoints */ FAR struct xhci_epinfo_s *epinfo[XHCI_MAX_ENDPOINTS]; @@ -229,7 +230,14 @@ struct xhci_dev_s struct usbhost_xhci_s { #ifdef CONFIG_USBHOST_HUB - FAR struct usbhost_hubport_s *hport; /* Used to pass external hub port events */ + /* Ports a hub has reported and the waiter has not collected. A hub + * reports each changed port without waiting for the last, so several can + * be outstanding, but never more than there are slots. + */ + + FAR struct usbhost_hubport_s *hports[CONFIG_USBHOST_XHCI_MAX_DEVS]; + uint8_t hhead; /* Next free entry */ + uint8_t htail; /* Next entry to collect */ #endif struct usbhost_devaddr_s devgen; /* Address generation data */ bool pscwait; /* TRUE: Thread is waiting for port status change event */ @@ -384,7 +392,7 @@ static void xhci_ep_configure(FAR struct usbhost_xhci_s *priv, uint8_t maxburst, uint64_t tr_dp, uint8_t mult, uint8_t interval); static int xhci_address_set(FAR struct usbhost_xhci_s *priv, - FAR struct xhci_rhport_s *rhport, bool setaddr); + FAR struct xhci_dev_s *dev, bool setaddr); static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, FAR struct xhci_dev_s *dev); #ifdef CONFIG_USBHOST_HUB @@ -393,9 +401,11 @@ static uint32_t xhci_slot_tt(FAR struct usbhost_xhci_s *priv, FAR struct xhci_dev_s *dev); #endif static int xhci_device_init(FAR struct usbhost_xhci_s *priv, - FAR struct xhci_rhport_s *rhport); + FAR struct xhci_rhport_s *rhport, + FAR struct usbhost_hubport_s *hport, + FAR struct xhci_epinfo_s *ep0info); static int xhci_device_deinit(FAR struct usbhost_xhci_s *priv, - FAR struct xhci_rhport_s *rhport); + FAR struct xhci_dev_s *dev); static inline uint8_t xhci_epno_get(FAR struct xhci_epinfo_s *epinfo); static void xhci_context_ctrl(FAR struct usbhost_xhci_s *priv, FAR struct xhci_dev_s *dev, @@ -1642,15 +1652,11 @@ static void xhci_ep_configure(FAR struct usbhost_xhci_s *priv, ****************************************************************************/ static int xhci_address_set(FAR struct usbhost_xhci_s *priv, - FAR struct xhci_rhport_s *rhport, bool setaddr) + FAR struct xhci_dev_s *dev, bool setaddr) { - FAR struct xhci_dev_s *dev; - uint64_t ctx; + uint64_t ctx = up_addrenv_va_to_pa(dev->input); - dev = rhport->dev; - ctx = up_addrenv_va_to_pa(dev->input); - - return xhci_cmd_setaddr(priv, rhport->slot, ctx, !setaddr); + return xhci_cmd_setaddr(priv, dev->slot, ctx, !setaddr); } /**************************************************************************** @@ -1701,13 +1707,6 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, #ifdef CONFIG_USBHOST_HUB regval |= XHCI_ST_CTX0_RTSTR_SET(xhci_route_string(dev->hport)); - - /* TODO: - * 1. Activate the transaction translator if required - * 2. Configure hub bit in slot context if hub - */ - -# warning missing logic #endif xhci_in_slot(priv, dev->input)->ctx[0] = htole32(regval); @@ -1802,7 +1801,9 @@ static int xhci_slot_init(FAR struct usbhost_xhci_s *priv, ****************************************************************************/ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, - FAR struct xhci_rhport_s *rhport) + FAR struct xhci_rhport_s *rhport, + FAR struct usbhost_hubport_s *hport, + FAR struct xhci_epinfo_s *ep0info) { FAR struct xhci_dev_s *dev; uint8_t slot; @@ -1837,9 +1838,17 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, /* Slot ID is an index to the identify Device data */ - rhport->dev = &priv->devs[slot - 1]; - rhport->slot = slot; - dev = rhport->dev; + dev = &priv->devs[slot - 1]; + + /* A root hub port names the device on it, which a hub port must not + * disturb: the device its root port names is the hub itself. + */ + + if (hport == &rhport->hport.hport) + { + rhport->dev = dev; + rhport->slot = slot; + } /* Slot has been allocated to software and is now in Enabled state */ @@ -1849,18 +1858,18 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, * All data structured are already allocated. */ - ret = xhci_ring_init(&rhport->ep0.td, XHCI_TD_MAX); + ret = xhci_ring_init(&ep0info->td, XHCI_TD_MAX); if (ret < 0) { uerr("ep0 ring init failed\n"); goto errout_with_slot; } - rhport->ep0.slot = slot; - dev->rhport = rhport; - dev->hport = &rhport->hport.hport; - dev->slot = slot; - dev->epinfo[0] = &rhport->ep0; + ep0info->slot = slot; + dev->rhport = rhport; + dev->hport = hport; + dev->slot = slot; + dev->epinfo[0] = ep0info; ret = xhci_slot_init(priv, dev); if (ret < 0) @@ -1876,7 +1885,7 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, * stack. */ - ret = xhci_address_set(priv, rhport, false); + ret = xhci_address_set(priv, dev, false); if (ret < 0) { uerr("failed to set address %d\n", ret); @@ -1893,7 +1902,7 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, * number of them. */ - xhci_device_deinit(priv, rhport); + xhci_device_deinit(priv, dev); return ret; } @@ -1909,9 +1918,9 @@ static int xhci_device_init(FAR struct usbhost_xhci_s *priv, ****************************************************************************/ static int xhci_device_deinit(FAR struct usbhost_xhci_s *priv, - FAR struct xhci_rhport_s *rhport) + FAR struct xhci_dev_s *dev) { - uint8_t slot = rhport->slot; + uint8_t slot = dev->slot; int ret; /* Disable Slot */ @@ -1928,22 +1937,30 @@ static int xhci_device_deinit(FAR struct usbhost_xhci_s *priv, /* Clean up device data, but don't touch allocated memory! */ - rhport->dev->state = XHCI_SLOT_DISABLED; + dev->state = XHCI_SLOT_DISABLED; - memset(rhport->dev->ctx, 0, XHCI_DEVCTX_SIZE(priv)); - memset(rhport->dev->input, 0, XHCI_INCTX_SIZE(priv)); + memset(dev->ctx, 0, XHCI_DEVCTX_SIZE(priv)); + memset(dev->input, 0, XHCI_INCTX_SIZE(priv)); /* And push both, so nothing is left to be written back later */ - up_flush_dcache((uintptr_t)rhport->dev->ctx, - (uintptr_t)rhport->dev->ctx + XHCI_DEVCTX_SIZE(priv)); - up_flush_dcache((uintptr_t)rhport->dev->input, - (uintptr_t)rhport->dev->input + XHCI_INCTX_SIZE(priv)); + up_flush_dcache((uintptr_t)dev->ctx, + (uintptr_t)dev->ctx + XHCI_DEVCTX_SIZE(priv)); + up_flush_dcache((uintptr_t)dev->input, + (uintptr_t)dev->input + XHCI_INCTX_SIZE(priv)); + + /* Remove reference to a device slot. Only the device sitting directly + * on the root port is the one that port points at; a device behind a hub + * must leave that pointing at the hub. + */ - /* Remove reference to a device slot */ + if (dev->rhport != NULL && dev->rhport->dev == dev) + { + dev->rhport->dev = NULL; + } - rhport->dev->hport = NULL; - rhport->dev = NULL; + dev->hport = NULL; + dev->rhport = NULL; return OK; } @@ -3829,12 +3846,12 @@ static int xhci_wait(FAR struct usbhost_connection_s *conn, #ifdef CONFIG_USBHOST_HUB /* Is a device connected to an external hub? */ - if (priv->hport) + if (priv->hhead != priv->htail) { /* Yes.. return the external hub port */ - connport = priv->hport; - priv->hport = NULL; + connport = priv->hports[priv->htail]; + priv->htail = (priv->htail + 1) % CONFIG_USBHOST_XHCI_MAX_DEVS; *hport = (FAR struct usbhost_hubport_s *)connport; spin_unlock_irqrestore(&priv->spinlock, flags); @@ -3926,7 +3943,8 @@ static int xhci_rh_enumerate(FAR struct usbhost_connection_s *conn, /* Initialize device data */ - ret = xhci_device_init(priv, rhport); + ret = xhci_device_init(priv, rhport, &rhport->hport.hport, + &rhport->ep0); if (ret < 0) { uerr("Failed to initialize device %d\n", ret); @@ -3986,7 +4004,7 @@ static int xhci_enumerate(FAR struct usbhost_connection_s *conn, if (rhport->dev != NULL) { - xhci_device_deinit(priv, rhport); + xhci_device_deinit(priv, rhport->dev); } /* Clearing connected below is what makes xhci_wait() return, @@ -4249,6 +4267,31 @@ static int xhci_epalloc(FAR struct usbhost_driver_s *drvr, mask = XHCI_IN_CTX1_A(XHCI_EP_FLAG(idx)); dev = xhci_dev_from_hport(priv, hport); +#ifdef CONFIG_USBHOST_HUB + /* A hub asks for the control endpoint of a port before it reports the + * connection, so there is no device to attach it to yet. Hand back an + * endpoint with no slot; xhci_connect() gives it one when it creates the + * device, which is the next thing the hub does. + */ + + if (dev == NULL && !ROOTHUB(hport) && + epdesc->xfrtype == USB_EP_ATTR_XFER_CONTROL) + { + ret = xhci_ring_init(&epinfo->td, XHCI_TD_MAX); + if (ret < 0) + { + uerr("ep0 ring init failed\n"); + nxmutex_destroy(&epinfo->exclsem); + nxsem_destroy(&epinfo->iocsem); + kmm_free(epinfo); + return ret; + } + + *ep = (usbhost_ep_t)epinfo; + return OK; + } +#endif + /* There has to be a device to hang the endpoint off. A port whose * enumeration failed is retried after its slot has been given back, so * this can run for a port with nothing behind it. @@ -4663,7 +4706,8 @@ static int xhci_ctrl_xfer(FAR struct usbhost_driver_s *drvr, /* Issue SET_ADDRESS request */ - ret = xhci_address_set(priv, rhport, true); + ret = xhci_address_set(priv, xhci_dev_from_ep(priv, ep0info), + true); if (ret == OK) { FAR struct xhci_dev_s *dev = xhci_dev_from_ep(priv, ep0info); @@ -5153,11 +5197,186 @@ static int xhci_cancel(FAR struct usbhost_driver_s *drvr, usbhost_ep_t ep) ****************************************************************************/ #ifdef CONFIG_USBHOST_HUB +/**************************************************************************** + * Name: xhci_rhport_from_hport + * + * Description: + * The root hub port a device descends from, however many hubs are in the + * way. The slot context names it, because that is the port the traffic + * physically leaves by. + * + ****************************************************************************/ + +static FAR struct xhci_rhport_s * +xhci_rhport_from_hport(FAR struct usbhost_xhci_s *priv, + FAR struct usbhost_hubport_s *hport) +{ + while (hport->parent != NULL) + { + hport = hport->parent; + } + + return &priv->rhport[hport->port]; +} + +/**************************************************************************** + * Name: xhci_hub_update + * + * Description: + * Tell the controller that a device is a hub, so that it will route to + * what is behind it. + * + * The slot was created before anyone knew: a hub is addressed and + * configured like any other device, and only then does its class driver + * read the descriptor saying how many ports it has. So the slot context + * is corrected here, the first time something appears behind it. + * + ****************************************************************************/ + +static int xhci_hub_update(FAR struct usbhost_xhci_s *priv, + FAR struct usbhost_hubport_s *hubport) +{ + FAR struct xhci_slot_ctx_s *in; + FAR struct xhci_dev_s *dev; + uint64_t ctx; + int ret; + + dev = xhci_dev_from_hport(priv, hubport); + if (dev == NULL || dev->ishub || hubport->nports == 0) + { + /* Nothing to correct: no slot for it, already done, or the hub class + * driver has not reported the descriptor. + */ + + return OK; + } + + ret = nxmutex_lock(&priv->lock); + if (ret < 0) + { + return ret; + } + + /* Only the slot context changes, and it must go in carrying everything + * the controller already holds, so start from the output context it has + * been maintaining. + */ + + up_invalidate_dcache((uintptr_t)dev->ctx, + (uintptr_t)dev->ctx + XHCI_DEVCTX_SIZE(priv)); + + xhci_context_ctrl(priv, dev, 0, XHCI_IN_CTX1_A(XHCI_SLOT_FLAG)); + + in = xhci_in_slot(priv, dev->input); + in->ctx[0] = xhci_out_slot(dev->ctx)->ctx[0] | htole32(XHCI_ST_CTX0_HUB); + in->ctx[1] = (xhci_out_slot(dev->ctx)->ctx[1] & + ~htole32(XHCI_ST_CTX1_PORTS_MASK)) | + htole32(XHCI_ST_CTX1_PORTS_SET(hubport->nports)); + in->ctx[2] = (xhci_out_slot(dev->ctx)->ctx[2] & + ~htole32(XHCI_ST_CTX2_TTT_MASK)) | + htole32(XHCI_ST_CTX2_TTT_SET(hubport->ttt)); + in->ctx[3] = xhci_out_slot(dev->ctx)->ctx[3]; + + up_flush_dcache((uintptr_t)dev->input, + (uintptr_t)dev->input + XHCI_INCTX_SIZE(priv)); + + ctx = up_addrenv_va_to_pa(dev->input); + + nxmutex_unlock(&priv->lock); + + ret = xhci_cmd_cfgep(priv, dev->slot, ctx, false); + if (ret < 0) + { + uerr("failed to describe the hub on slot %d: %d\n", dev->slot, ret); + return ret; + } + + dev->ishub = true; + + uinfo("%s: port %d: hub with %d port%s\n", + priv->name, xhci_rhport_from_hport(priv, hubport)->hport.hport.port + + 1, hubport->nports, hubport->nports == 1 ? "" : "s"); + + return OK; +} + static int xhci_connect(FAR struct usbhost_driver_s *drvr, FAR struct usbhost_hubport_s *hport, bool connected) { -#error missing logic + FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_DRVR(drvr); + FAR struct xhci_dev_s *dev; + irqstate_t flags; + int ret; + + DEBUGASSERT(priv != NULL && hport != NULL && !ROOTHUB(hport)); + + /* The hub owns this port, so there is no port register here to consult + * and no reset to drive: what the hub reports is the whole of what the + * controller can know about it. + */ + + hport->connected = connected; + + if (connected) + { + /* The controller has to know the port belongs to a hub before it will + * carry anything to it. + */ + + ret = xhci_hub_update(priv, hport->parent); + if (ret < 0) + { + return ret; + } + + /* Give the device a slot. The hub allocated its control endpoint + * before saying anything, so that endpoint is what the slot gets. + */ + + ret = xhci_device_init(priv, xhci_rhport_from_hport(priv, hport), + hport, (FAR struct xhci_epinfo_s *)hport->ep0); + if (ret < 0) + { + uerr("port %d: no slot for the device: %d\n", hport->port, ret); + return ret; + } + } + else + { + dev = xhci_dev_from_hport(priv, hport); + if (dev != NULL) + { + xhci_device_deinit(priv, dev); + } + } + + flags = spin_lock_irqsave(&priv->spinlock); + + /* Queue it for the waiter. Dropping one when the queue is full would + * lose a device silently, and the queue is as long as the controller has + * slots, so a full one means every slot is already spoken for. + */ + + if ((uint8_t)(priv->hhead + 1) % CONFIG_USBHOST_XHCI_MAX_DEVS != + priv->htail) + { + priv->hports[priv->hhead] = hport; + priv->hhead = (priv->hhead + 1) % CONFIG_USBHOST_XHCI_MAX_DEVS; + } + else + { + uerr("no room to report port %d\n", hport->port + 1); + } + + if (priv->pscwait) + { + priv->pscwait = false; + nxsem_post(&priv->pscsem); + } + + spin_unlock_irqrestore(&priv->spinlock, flags); + return OK; } #endif @@ -5190,17 +5409,21 @@ static int xhci_connect(FAR struct usbhost_driver_s *drvr, static void xhci_disconnect(FAR struct usbhost_driver_s *drvr, FAR struct usbhost_hubport_s *hport) { - FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_DRVR(drvr); - FAR struct xhci_rhport_s *rhport = (FAR struct xhci_rhport_s *)drvr; + FAR struct usbhost_xhci_s *priv = XHCI_PRIV_FROM_DRVR(drvr); + FAR struct xhci_dev_s *dev; DEBUGASSERT(hport != NULL); hport->devclass = NULL; - /* Deinit device slot */ + /* Deinit the device that was on this port. Taking it from the port and + * not from the root port matters once a hub is in the way, where the root + * port names the hub rather than the device going away. + */ - if (rhport->dev) + dev = xhci_dev_from_hport(priv, hport); + if (dev != NULL) { - xhci_device_deinit(priv, rhport); + xhci_device_deinit(priv, dev); } } From 25e387ffedc9f2b746df975f731ee4fdbb464a7a Mon Sep 17 00:00:00 2001 From: Justin Hammond Date: Sat, 8 Aug 2026 14:39:09 +0800 Subject: [PATCH 26/26] drivers/usbhost: Tell the host stack which controller a port belongs to. struct usbhost_roothubport_s carries the number of the controller its port belongs to, so a port can be named on a system with more than one. Nothing set it. Take the number from whoever brings the controller up rather than counting registrations, which would agree with the name the driver reports only while controllers are registered in the order they are named. Assisted-by: Claude:claude-opus-5 Signed-off-by: Justin Hammond --- drivers/usbhost/usbhost_xhci.c | 11 ++++++++++- drivers/usbhost/usbhost_xhci_pci.c | 2 +- include/nuttx/usb/xhci.h | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/usbhost/usbhost_xhci.c b/drivers/usbhost/usbhost_xhci.c index 09e4015dcd804..8de8d7ebd4a3c 100644 --- a/drivers/usbhost/usbhost_xhci.c +++ b/drivers/usbhost/usbhost_xhci.c @@ -248,6 +248,7 @@ struct usbhost_xhci_s /* xHCI parameters */ uint8_t no_ports; /* Number of USB Ports */ + uint8_t bus; /* Which controller this is */ uint8_t no_slots; /* Maximum number of Device Slots (one per USB device) */ uint8_t no_scratch; /* Number of scratch buffers */ uint8_t no_erst; /* Event Ring Segment Table size */ @@ -5830,6 +5831,7 @@ static inline int xhci_sw_initialize(FAR struct usbhost_xhci_s *priv) rhport->ep0.epno = 0; rhport->ep0.devaddr = 0; nxsem_init(&rhport->ep0.iocsem, 0, 0); + rhport->hport.bus = priv->bus; nxmutex_init(&rhport->ep0.exclsem); /* Initialize the public port representation */ @@ -5874,7 +5876,7 @@ static inline int xhci_sw_initialize(FAR struct usbhost_xhci_s *priv) ****************************************************************************/ FAR struct usbhost_connection_s * -xhci_initialize(FAR const char *name, uintptr_t base, +xhci_initialize(FAR const char *name, uint8_t bus, uintptr_t base, FAR const struct xhci_bus_ops_s *ops, FAR void *arg) { FAR struct usbhost_conn_xhci_s *conn = NULL; @@ -5903,6 +5905,13 @@ xhci_initialize(FAR const char *name, uintptr_t base, conn->priv = priv; priv->name = name; + + /* The bus is what the controller calls itself, so that a port reported + * through the generic host stack and a message from this driver name the + * same thing. Numbering them here instead would agree only by accident. + */ + + priv->bus = bus; priv->ops = ops; priv->arg = arg; priv->base = base; diff --git a/drivers/usbhost/usbhost_xhci_pci.c b/drivers/usbhost/usbhost_xhci_pci.c index 95b794c4c261c..8c7f0819b44b7 100644 --- a/drivers/usbhost/usbhost_xhci_pci.c +++ b/drivers/usbhost/usbhost_xhci_pci.c @@ -213,7 +213,7 @@ static int pci_xhci_probe(FAR struct pci_device_s *dev) goto errout; } - pcix->conn = xhci_initialize("usb", base, &g_pci_xhci_ops, pcix); + pcix->conn = xhci_initialize("usb", 0, base, &g_pci_xhci_ops, pcix); if (pcix->conn == NULL) { pcierr("xhci_initialize failed\n"); diff --git a/include/nuttx/usb/xhci.h b/include/nuttx/usb/xhci.h index 1010811244ba6..32a6638a940f5 100644 --- a/include/nuttx/usb/xhci.h +++ b/include/nuttx/usb/xhci.h @@ -95,7 +95,7 @@ struct xhci_bus_ops_s ****************************************************************************/ FAR struct usbhost_connection_s * -xhci_initialize(FAR const char *name, uintptr_t base, +xhci_initialize(FAR const char *name, uint8_t bus, uintptr_t base, FAR const struct xhci_bus_ops_s *ops, FAR void *arg); /****************************************************************************