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

[Marek Vasut]

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 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.

Thank you ! Esp. for digging in properly and solving this properly.