On 9/1/23 11:49, Fabrice Gasnier wrote:
On 8/30/23 17:12, Marek Vasut wrote:
On 8/30/23 10:00, Fabrice Gasnier wrote:
EHCI is usually used with companion controller (like OHCI) as companion
controller. This information on the companion is missing currently in
companion drivers.
So, if the usb-uclass isn't aware, it may scan busses in any order: OHCI
first, then EHCI.
This is seen on STM32MP1 where DT probing makes the probe order to occur
by increasing address (OHCI address < EHCI address).
When a low speed or full-speed device is plugged in, it's not detected as
EHCI should first detect it, and give ownership (handover) to OHCI.
Current situation on STM32MP1 (with a low speed device plugged-in)
STM32MP> usb start
starting USB...
Bus usb@5800c000: USB OHCI 1.0
Bus usb@5800d000: USB EHCI 1.00
scanning bus usb@5800c000 for devices... 1 USB Device(s) found
scanning bus usb@5800d000 for devices... 1 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
The "companion" property in the device tree allow to retrieve companion
controller information, from the EHCI node. This allow marking the
companion driver as such.
With this patch (same low speed device plugged in):
STM32MP> usb start
starting USB...
Bus usb@5800c000: USB OHCI 1.0
Bus usb@5800d000: USB EHCI 1.00
scanning bus usb@5800d000 for devices... 1 USB Device(s) found
scanning bus usb@5800c000 for devices... 2 USB Device(s) found
scanning usb for storage devices... 0 Storage Device(s) found
STM32MP> usb tree
USB device tree:
1 Hub (12 Mb/s, 0mA)
| U-Boot Root Hub
|
+-2 Human Interface (1.5 Mb/s, 100mA)
HP HP USB 1000dpi Laser Mouse
1 Hub (480 Mb/s, 0mA)
u-boot EHCI Host Controller
This also optimize bus scan when a High speed device is plugged in, as
the usb-uclass skips OHCI in this case:
STM32MP> usb reset
resetting USB...
Bus usb@5800c000: USB OHCI 1.0
Bus usb@5800d000: USB EHCI 1.00
scanning bus usb@5800d000 for devices... 2 USB Device(s) found
scanning usb for storage devices... 1 Storage Device(s) found
STM32MP> usb tree
USB device tree:
1 Hub (480 Mb/s, 0mA)
| u-boot EHCI Host Controller
|
+-2 Mass Storage (480 Mb/s, 200mA)
SanDisk Cruzer Blade 03003432021922011407
Signed-off-by: Fabrice Gasnier <fabrice.gasn...@foss.st.com>
---
drivers/usb/host/ehci-generic.c | 24 ++++++++++++++++++++++++
More of a design question -- shouldn't the usb-uclass handle this
property ad manipulate with private data of "remote" devices instead ?
Hi Marek,
That's an option I haven't really explored earlier. I was a bit confused
when I figured out the usb-uclass was able to manage the companion
controller, by using the "companion" flag. But I couldn't find any
driver that was actually using this flag :-O.
So I had a look at the git log, and I found out:
- Companion has been introduced in:
b6de4d1093d3 dm: usb: Add support for companion controllers
- First user for this flag was introduced in same series:
6a72e804a2b2 sunxi: ohci: Add ohci usb host controller support
e.g. drivers/usb/host/ohci-sunxi.c
So, in the original design, the flag was set in OHCI controller driver
side. This is why I implemented this in controller driver, but in EHCI
generic driver, because it has the companion property (not in OHCI node).
- Later this driver has been dropped in, to use the generic OHCI driver
instead: 543049ab5906 usb: host: Drop [e-o]hci-sunxi drivers
It looks like that setting the companion flag has been missed when
moving to OHCI generic driver. I admit the strange thing with current
approach is: implementation is done in EHCI driver, to impact OHCI
private struct.
I'll send a v2, to handle this in usb-uclass as you suggest. This makes
sense.
Thank you ! Esp. for digging in properly and solving this properly.
