[PATCH v2] usb: check for companion controller in uclass

[Marek Vasut]

On 9/1/23 11:52, 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>
> ---
> 
> Changes in v2:
> - move companion probing from generic ehci driver to usb-uclass, after
>    Marek's questions on design choice.
> - rename commit title to follow this change
> 
> ---
>   drivers/usb/host/usb-uclass.c | 36 +++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 02c0138a2065..e238eee8c84d 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -249,6 +249,37 @@ static void remove_inactive_children(struct uclass *uc, struct udevice *bus)
>   	}
>   }
>   
> +static int usb_probe_companion(struct udevice *bus)
> +{
> +	struct udevice *companion_dev;
> +	int ret;
> +
> +	/*
> +	 * Enforce optional companion controller is marked as such in order to
> +	 * 1st scan the primary controller, before the companion controller
> +	 * (ownership is given to companion when low or full speed devices
> +	 * have been detected).
> +	 */
> +	ret = uclass_get_device_by_phandle(UCLASS_USB, bus, "companion", &companion_dev);
> +	if (!ret) {
> +		struct usb_bus_priv *companion_bus_priv;
> +
> +		debug("%s is the companion of %s\n", companion_dev->name, bus->name);
> +		companion_bus_priv = dev_get_uclass_priv(companion_dev);
> +		companion_bus_priv->companion = true;
> +	} else if (ret && ret != -ENOENT && ret != -ENODEV) {
> +		/*
> +		 * Treat everything else than no companion or disabled
> +		 * companion as an error. (It may not be enabled on boards
> +		 * that have a High-Speed HUB to handle FS and LS traffic).
> +		 */
> +		printf("Failed to get companion (ret=%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   int usb_init(void)
>   {
>   	int controllers_initialized = 0;
> @@ -299,6 +330,11 @@ int usb_init(void)
>   			printf("probe failed, error %d\n", ret);
>   			continue;
>   		}
> +
> +		ret = usb_probe_companion(bus);

Shouldn't this ^ be in separate uclass_foreach_dev(bus, uc) {} loop 
before this device_probe() loop , so that when the probe of the 
controller is called, its ->companion is already set ?

> +		if (ret)
> +			continue;
> +
>   		controllers_initialized++;
>   		usb_started = true;
>   	}

[...]