Re: [PATCH v4 1/2] dt-bindings: usb: Add binding for discrete onboard USB hubs
From: Alan Stern
Date: Fri Oct 02 2020 - 14:36:42 EST
On Fri, Oct 02, 2020 at 10:08:17AM -0700, Doug Anderson wrote:
> As a more similar example of single device that is listed in more than
> one location in the device tree, we can also look at embedded SDIO
> BT/WiFi combo cards. This single device often provides WiFi under an
> SDIO bus and BT under a serial / USB bus. I'm not 100% sure there are
> actually cases were the same board provides device tree data to both
> at the same time, but "brcm,bcm43540-bt" is an example of providing
> data to the Bluetooth (connected over serial port) and
> "brcm,bcm4329-fmac" to the WiFi (connected over the SDIO bus). Of
> course WiFi/BT cheat in that the control logic is represented by the
> SDIO power sequencing stuff...
>
>
> Back to our case, though. I guess the issue here is that we're the
> child of more than one bus. Let's first pretend that the i2c lines of
> this hub are actually hooked up and establish how that would look
> first. Then we can think about how it looks if this same device isn't
> hooked up via i2c. In this case, it sounds as if you still don't want
> the device split among two nodes. So I guess you'd prefer something
> like:
>
> i2c {
> usb-hub@xx {
> reg = <xx>;
> compatible = "realtek,rts5411", "onboard-usb-hub";
> vdd-supply = <&pp3300_hub>;
> usb-devices = <&usb_controller 1>;
> };
> };
>
> ...and then you wouldn't have anything under the USB controller
> itself. Is that correct? So even though there are existing bindings
> saying that a USB device should be listed via VID/PID, the desire to
> represent this as a single node overrides that, right? (NOTE: this is
> similar to what Matthias proposed in his response except that I've
> added an index so that we don't need _anything_ under the controller).
>
> Having this primarily listed under the i2c bus makes sense because the
> control logic for the hub is hooked up via i2c. Having the power
> supply associated with it also makes some amount of sense since it's a
> control signal. It's also convenient that i2c devices have their
> probe called _before_ we try to detect if they're there because it's
> common that i2c devices need power applied first.
>
> Now, just because we don't have the i2c bus hooked up doesn't change
> the fact that there is control logic. We also certainly wouldn't want
> two ways of describing this same hub: one way if the i2c is hooked up
> and one way if it's not hooked up. To me this means that the we
> should be describing this hub as a top-level node if i2c isn't hooked
> up, just like we do with "smsc,usb3503a"
>
> Said another way, we have these points:
>
> a) The control logic for this bus could be hooked up to an i2c bus.
>
> b) If the control logic is hooked up to an i2c bus it feels like
> that's where the device's primary node should be placed, not under the
> USB controller.
>
> c) To keep the i2c and non-i2c case as similar as possible, if the i2c
> bus isn't hooked up the hub's primary node should be a top-level node,
> not under the USB controller.
>
>
> NOTE ALSO: the fact that we might want to list this hub under an i2c
> controller also seems like it's a good argument against putting this
> logic in the xhci-platform driver?
More and more we are going to see devices that are attached to multiple
buses. In this case, one for power control and another for
commands/data. If DT doesn't already have a canonical way of handling
such situations, it needs to develop one soon.
One can make a case that there should be multiple device nodes in this
situation, somehow referring to each other so that the system knows they
all describe the same device. Maybe one "primary" node for the device
and the others acting kind of like symbolic links.
Regardless of how the situation is represented in DT, there remains the
issue of where (i.e., in which driver module) the appropriate code
belongs. This goes far beyond USB. In general, what happens when one
sort of device normally isn't hooked up through a power regulator, so
its driver doesn't have any code to enable a regulator, but then some
system does exactly that?
Even worse, what if the device is on a discoverable bus, so the driver
doesn't get invoked at all until the device is discovered, but on the
new system it can't be discovered until the regulator is enabled?
Alan Stern