[PATCH] usb: host: ehci-generic: check for companion controller

[Fabrice Gasnier]

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 at 5800c000: USB OHCI 1.0
>> Bus usb at 5800d000: USB EHCI 1.00
>> scanning bus usb at 5800c000 for devices... 1 USB Device(s) found
>> scanning bus usb at 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 at 5800c000: USB OHCI 1.0
>> Bus usb at 5800d000: USB EHCI 1.00
>> scanning bus usb at 5800d000 for devices... 1 USB Device(s) found
>> scanning bus usb at 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 at 5800c000: USB OHCI 1.0
>> Bus usb at 5800d000: USB EHCI 1.00
>> scanning bus usb at 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.gasnier at 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.

Thanks for reviewing,
Best Regards,
Fabrice