drivers/usbhost: Support USB hubs on xHCI. - #19862
Draft
Fishwaldo wants to merge 26 commits into
Draft
Conversation
Fishwaldo
force-pushed
the
upstream-xhci-hubs
branch
from
August 16, 2026 12:15
a0eff23 to
e19ad88
Compare
🔗 Cross-repo PR dependenciesThe read-only Build run reported the following dependent PR(s) and fetched head SHA(s): CI run: https://github.com/apache/nuttx/actions/runs/31946594586 |
Fishwaldo
force-pushed
the
upstream-xhci-hubs
branch
from
August 17, 2026 13:17
e19ad88 to
4036612
Compare
Contributor
Author
|
Rebased after #19745 landed |
Fishwaldo
marked this pull request as draft
August 17, 2026 15:16
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
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 <justin@dynam.ac>
Fishwaldo
force-pushed
the
upstream-xhci-hubs
branch
from
August 18, 2026 13:32
4036612 to
25e387f
Compare
🔗 Cross-repo PR dependenciesThe read-only Build run reported the following dependent PR(s) and fetched head SHA(s): CI run: https://github.com/apache/nuttx/actions/runs/32142998118 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The first twenty-eight commits belong to #19745, #19860 and #19861 and are not
part of this change. This PR adds the last five.
The xHCI driver refused
CONFIG_USBHOST_HUBat compile time. A controllerreaches a device behind a hub by the path to it, and for a low or full speed
device through the hub that translates for it, and none of that was described.
slot, the default control endpoint and the device context lived in the root
port structure, which holds only while every device is plugged straight into
the controller. Two keys replace it: an endpoint records the slot it was
opened on, and a hub port belongs to one device wherever it sits. No
functional change for a directly attached device, whose port slot and
endpoint slot are the same one.
be told about the hubs in a topology, not only about the device at the end of
it: a hub's slot context carries a hub flag, its downstream port count and
its transaction translator think time. The hub class driver already reads
both values from the hub descriptor; this publishes them on the hub's own hub
port, beside the speed and function address already there. Nothing is
required to read them, so a controller that does not need them is unaffected.
the path to it, a nibble per tier with the tier nearest the root lowest,
stopping after five as the field and USB both require. Slot context dword 2
names the transaction translator, reported by slot rather than by USB address
as EHCI does, and naming the nearest high speed ancestor rather than the
immediate parent.
xhci_epalloc()carried a copy ofsam_ehci.c'ssplit-transaction block, writing fields this driver never read; both are
removed.
asks for a port's control endpoint before it reports the connection, so an
endpoint may exist before its slot does; a hub must be described as a hub
before anything behind it can be reached, which is only known once its class
driver has read the descriptor, so
xhci_hub_update()corrects the slotcontext the first time something appears behind it; and a hub reports each
changed port without waiting for the last, so the connect method queues them
rather than holding one pointer.
struct usbhost_roothubport_scarries that number and nothing set it.One commit touches shared code outside the xHCI driver: reporting the hub's
port count and think time adds two fields to
struct usbhost_hubport_s, bothinside
CONFIG_USBHOST_HUB, as the parent pointer already is.Multi-TT is not implemented. It comes from the hub's interface protocol rather
than its descriptor, and driving a multi-TT hub as single-TT costs bandwidth
behind that hub but is correct.
Impact
Enables
USBHOST_HUBwithUSBHOST_XHCI, previously rejected at compile time.No change when hub support is disabled; the two new
usbhost_hubport_sfieldsare compiled out without it.
Testing
EIC7700 EVB, Synopsys DWC3 in host mode, with a real Fresco Logic 6-port USB
2.0 hub:
The device behind the hub mounts as a 59 GB vfat volume, its directory lists,
and
dd if=/dev/sda bs=512 count=256reads at 703 KB/s.QEMU with
-device qemu-xhciplus ausb-hubcarrying both ausb-storageand a
usb-kbd: both enumerate behind the hub, giving/dev/sdaand/dev/kbda, the file reads backqemu-xhci-regression-ok, and no assertionsfire.
Note for anyone reproducing this:
qemu-intel64:jumbohas xHCI, mass storageand HID keyboard but does not set
CONFIG_USBHOST_HUB, so hub coverage needsit enabled.
Depends-On: #19861