Hi Jerome,
On Thu, 27 Feb 2025 at 10:30, Jerome Forissier
<jerome.foriss...@linaro.org> wrote:
>
>
>
> On 2/27/25 17:25, Simon Glass wrote:
> > Hi Jerome,
> >
> > On Tue, 25 Feb 2025 at 09:35, Jerome Forissier
> > <jerome.foriss...@linaro.org> wrote:
> >>
> >> Use the uthread framework to initialize and scan USB buses in parallel
> >> for better performance. The console output is slightly modified with a
> >> final per-bus report of the number of devices found, common to UTHREAD
> >> and !UTHREAD. The USB tests are updated accordingly.
> >>
> >> Tested on two platforms:
> >>
> >> 1. arm64 QEMU on a somewhat contrived example (4 USB buses, each with
> >> one audio device, one keyboard, one mouse and one tablet)
> >>
> >> $ make qemu_arm64_defconfig
> >> $ make -j$(nproc) CROSS_COMPILE="ccache aarch64-linux-gnu-"
> >> $ qemu-system-aarch64 -M virt -nographic -cpu max -bios u-boot.bin \
> >> $(for i in {1..4}; do echo -device qemu-xhci,id=xhci$i \
> >> -device\ usb-{audio,kbd,mouse,tablet},bus=xhci$i.0; \
> >> done)
> >>
> >> 2. i.MX93 EVK (imx93_11x11_evk_defconfig) with two USB hubs, each with
> >> one webcam and one ethernet adapter, resulting in the following device
> >> tree:
> >>
> >> USB device tree:
> >> 1 Hub (480 Mb/s, 0mA)
> >> | u-boot EHCI Host Controller
> >> |
> >> +-2 Hub (480 Mb/s, 100mA)
> >> | GenesysLogic USB2.1 Hub
> >> |
> >> +-3 Vendor specific (480 Mb/s, 350mA)
> >> | Realtek USB 10/100/1000 LAN 001000001
> >> |
> >> +-4 (480 Mb/s, 500mA)
> >> HD Pro Webcam C920 8F7CD51F
> >>
> >> 1 Hub (480 Mb/s, 0mA)
> >> | u-boot EHCI Host Controller
> >> |
> >> +-2 Hub (480 Mb/s, 100mA)
> >> | USB 2.0 Hub
> >> |
> >> +-3 Vendor specific (480 Mb/s, 200mA)
> >> | Realtek USB 10/100/1000 LAN 000001
> >> |
> >> +-4 (480 Mb/s, 500mA)
> >> Generic OnLan-CS30 201801010008
> >>
> >> Note that i.MX was tested on top of the downstream repository [1] since
> >> USB doesn't work in the upstream master branch.
> >>
> >> [1] https://github.com/nxp-imx/uboot-imx/tree/lf-6.6.52-2.2.0
> >> commit 6c4545203d12 ("LF-13928 update key for capsule")
> >>
> >> The time spent in usb_init() ("usb start" command) is reported on
> >> the console. Here are the results:
> >>
> >> | CONFIG_UTHREAD=n | CONFIG_UTHREAD=y
> >> --------+------------------+-----------------
> >> QEMU | 5628 ms | 2212 ms
> >> i.MX93 | 4591 ms | 2441 ms
> >>
> >> Signed-off-by: Jerome Forissier <jerome.foriss...@linaro.org>
> >> ---
> >> drivers/usb/host/usb-uclass.c | 92 ++++++++++++++++++++++++++++-------
> >> test/boot/bootdev.c | 14 +++---
> >> test/boot/bootflow.c | 2 +-
> >> 3 files changed, 83 insertions(+), 25 deletions(-)
> >
> > What happens to output produced by a thread? Does it get stored
> > somewhere and written when the thread completes, or do the threads
> > intermingle their output?
>
> The latter. That is why I slightly reworked the USB initialization
> output to print a status once the threads are done, and I also updated
> the related tests as you can see in the patch. For instance the tests
> were expecting:
> "Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found"
> The "scanning bus usb@1 for devices..." part is printed by the threads,
> therefore in any order. I decided to move the text
> "Bus usb@1: 5 USB Device(s) found" to after the threads are complete,
> iterating over the devices in a deterministic order.
OK. I think at some point we would want to collected the output and
only show it when at a prompt or when all processing is done.
>
> >
> > I'm not sure if you saw my email about using a state machine for USB.
> > If so, could you please point me to your reply?
>
> I did, but I did not reply because I did not try. I have a feeling that
> the change would be more intrusive than what I did, but above all I am
> not doing the uthread thing to address only USB but as a general
> technique to make things parallel without too much trouble (at least
> that's my hope). So kind of a different goal.
They are separate approaches. I believe that having the usb-scanning
state in one place as an iterator would benefit U-Boot. It is
currently a bit messy.
Regards,
Simon
