Re: [PATCH v4 1/2] dt-bindings: usb: Add binding for discrete onboard USB hubs
From: Alan Stern
Date: Wed Sep 30 2020 - 15:19:20 EST
On Wed, Sep 30, 2020 at 08:28:17AM -0700, Doug Anderson wrote:
> > The 2nd issue is where do extra properties for a device go. That's
> > nothing new nor special to USB. They go with the device node. We
> > already went thru that with the last attempt.
> >
> > So for this case, we'd have something like this:
> >
> > usb_controller {
> > dr_mode = "host";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > hub@1 {
> > compatible = "usbbda,5411";
> > reg = <1>;
> > vdd-supply = <&pp3300_hub>;
> > };
> > };
> >
> > This is no different than needing a reset line deasserted as the prior
> > attempt did.
>
> I'd believe that the above could be made to work with enough software
> change in the USB stack. Presumably we wouldn't want to actually do a
> full probe of the device until USB actually enumerated it, but I guess
> you could add some type of optional "pre-probe" step where a driver is
> called? So you'd call a pre-probe on whatever driver implements
> "usbbda,5411" and it would turn on the power supply. ...then, if the
> device is actually there, the normal probe would be called? I guess
> that'd work...
Would a better approach be to move the code into the xhci-platform
driver, rather than adding a new artificial platform device as
Matthias's patch does? That's the way other platform-specific DT
issues have generally been handled in the USB stack.
> One thing that strikes me as a possible problem, though, is that I
> totally envision HW guys coming back and saying: "oh, we want to
> second source that USB hub and randomly stuff a different hub on some
> boards". In theory that's a reasonable suggestion, right? USB is a
> probable bus. We turn on power to the USB hub (and the regulator to
> turn on power is the same no matter which hub is stuffed) and then we
> can just check which device got enumerated. It's likely that both
> hubs would behave the same from a software point of view, but they
> would have different VID/PID.
>
> As far as I understand the current USB bindings account for the fact
> that the device(s) specified in the device tree might or might not be
> there. Adding a node under the controller like you show above means:
> "if something is plugged into port 1 of this USB hub and if that thing
> matches 0x0bda/0x5411 then here are the extra properties (vdd-supply)
> for it". With your proposal I believe we're changing it to mean
> "there will definitely be a device plugged into port 1 of this USB hub
> and it will match 0x0bda/0x5411." Unless I'm mistaken, that will have
> potential impacts on the ability to second source things. I guess
> both pre-probe functions could be called and (since there can be
> multiple users of a regulator) it'd be OK, but if we get into reset
> lines it's not much fun. However, describing the device more like
> Matthias has done it will be nicely compatible with second sourcing.
Can the matching be done purely by port number under the controller's root
hub without regard to the VID/PID?
Alan Stern