On 9/4/23 18:21, Fabrice Gasnier wrote:
On 9/4/23 15:54, Marek Vasut wrote:
On 9/4/23 14:34, Fabrice Gasnier wrote:
On 9/1/23 18:00, Marek Vasut wrote:
On 9/1/23 14:12, Fabrice Gasnier wrote:
On 9/1/23 12:48, Marek Vasut wrote:
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@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.gasnier@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);
One more thing, shouldn't this do
if (ret) continue;
for maximum compatibility ?
I think it does :-) Or I miss your question here, could you clarify ?
In the original PATCH there is:
ret = usb_probe_companion(bus);if (ret)continue;
Then all is OK, thanks. The missing context confused me.
But since rc4 was just tagged, I'll be putting this into next, we're too close to the release.