Fishwaldo opened a new pull request, #19861:
URL: https://github.com/apache/nuttx/pull/19861
## Summary
The first seven commits belong to #19745 and #19860 and are not part of this
change. This PR adds the remaining twenty-one, one fault each.
The driver had only ever run behind QEMU's controller and its message
signalled interrupt, on a host with coherent caches and a flat address space.
These are the faults that reach it once any of that stops being true.
**Interrupts.** The handler returned with the source still asserted, so a
level
triggered line re-fired forever and the worker never ran. The event ring was
acknowledged after being walked, discarding anything that arrived during the
walk. The interrupt was attached before the event ring existed, which races a
controller a boot loader left running. The moderation interval was left at
its
reset value of 1ms, so every completion paid that. A port was disabled by the
act of probing it, since PORTSC change bits are write-one-to-clear.
**Cache and addressing.** Rings were published with `up_flush_dcache_all()`,
which an architecture that can only maintain by address implements as a
barrier
and nothing more, so the controller read stale memory. Data buffers got no
maintenance at all. A buffer that does not own its cache lines now goes
through
an aligned stand-in, since maintaining a partial line disturbs whatever
shares
it. Whether the controller can reach a buffer is asked of the platform
through
a new optional `dmacapable` operation.
**Transfer descriptors.** A Normal TRB describes one run of memory that may
not
cross a 64K boundary, and one TRB was programmed regardless of length. A link
TRB reached part way through a multi-TRB transfer was written without the
chain
bit, which ends the transfer at the link, so nothing is woken and the read
never returns.
**Device description.** Contexts came in one size only and the wider form was
refused with `-EIO`; the EIC7700X reports it on both of its controllers. The
event ring segment count wrapped to zero above 128 segments. The slot context
never carried the device speed, which has no valid zero.
**Endpoints and slots.** The endpoint interval is an exponent and the
descriptor's period was copied across unconverted, so a low speed keyboard
never enumerated. Asynchronous transfers refused every buffer whose length
was
not a whole number of cache lines, which an eight byte HID report never is.
Transfers on one endpoint were not serialised, which a composite device's two
poll threads reach through endpoint 0. A failed enumeration leaked its device
slot and then retried forever.
Two commits cover more than one point and say why in their messages: the
interrupter mask cannot be separated from the unmask that answers it, and the
context stride change touches every context walk, so splitting it further
would
produce commits that do not build.
## Impact
`USBHOST_XHCI` users. Required for any controller that is not QEMU's. No
configuration change; `dmacapable` is optional, so a platform that does not
supply it is unaffected.
## Testing
QEMU with `-device qemu-xhci`, built for `qemu-intel64:nsh` with the driver
compiled and linked:
- a `usb-storage` device enumerates as `/dev/sda`, mounts vfat and reads back
`qemu-xhci-regression-ok`
- behind a `usb-hub`, a `usb-storage` and a `usb-kbd` both enumerate, giving
`/dev/sda` and `/dev/kbda`, with the file read back and no assertions
EIC7700 EVB, Synopsys DWC3 in host mode, with a real 6-port hub:
```
usb 1-1: keyboard, driver attached
usb 0-1: hub, driver attached (USB2.0 Hub, Fresco Logic)
usb 0-1.1: mass storage, driver attached -> /dev/sda
usb 0-1.2: misc, driver attached (SIPEED UARTx4 HS) -> /dev/ttyACM0-3
```
`/dev/sda` mounts as a 59 GB vfat volume and `dd if=/dev/sda bs=512
count=256`
reads at 703 KB/s.
Throughput on that board, doorbell to interrupt 986-1021us before and 13-56us
after the moderation fix:
```
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
```
One note for anyone reproducing the keyboard tests: `CONFIG_HIDKBD_STACKSIZE`
defaults to 1024, which is not enough for the polling thread on a 64-bit
target. It overflows and presents as an assertion at the top of `kbdpoll`,
which reports the damage rather than the cause. 3072 is enough; the EIC7700
EVB configuration already uses that. This is unrelated to the changes here.
Depends-On: https://github.com/apache/nuttx/pull/19860
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
