Re: [PATCH v5 3/4] usb: host: xhci-plat: Create platform device for onboard hubs in probe()

From: Stephen Boyd <swboyd@chromium.org>
To: Matthias Kaehlcke <mka@chromium.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	devicetree@vger.kernel.org, Peter Chen <peter.chen@nxp.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Ravi Chandra Sadineni <ravisadineni@chromium.org>,
	Bastien Nocera <hadess@hadess.net>,
	linux-kernel@vger.kernel.org,
	Douglas Anderson <dianders@chromium.org>,
	linux-usb@vger.kernel.org,
	Mathias Nyman <mathias.nyman@intel.com>
Subject: Re: [PATCH v5 3/4] usb: host: xhci-plat: Create platform device for onboard hubs in probe()
Date: Thu, 11 Feb 2021 12:45:31 -0800	[thread overview]
Message-ID: <161307633188.1254594.14761449293146776857@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <YCWVU7YCE+WzRx9v@google.com>

Quoting Matthias Kaehlcke (2021-02-11 12:36:35)
> Hi Stephen,
> 
> On Thu, Feb 11, 2021 at 11:14:39AM -0800, Stephen Boyd wrote:
> > 
> > Is it possible to use the graph binding to connect the USB controller on
> > the SoC to the port on the hub? Then the hub would be a standalone node
> > at the root of DT connected to the USB controller (or phy) and xhci code
> > could probe the firmware to see if there's a graph connection downstream
> > that is a powered hub like this. I didn't see this idea mentioned in the
> > previous discussions, but maybe I missed it.
> 
> Thanks for bringing this up. I'm not sure I completely understand your
> suggestion, but in general it seems a direction that could be worth
> exploring.

Cool.

> 
> I think something like the following should work even without requiring
> cooperation from the XHCI code:
> 
> onboard-usb-hub {
>     compatible = “realtek,rts5411”, “onboard_usb_hub”;
>     #address-cells = <1>;
>     #size-cells = <0>;
> 
>     vdd-supply = <&pp3300_hub>;
> 
>     port@0 {
>         reg = <0>;
> 
>         rts5411_3_0: endpoint {
>             // should not be needed
>             remote-endpoint = <&usb_1_dwc3_port1>;
>         };
>     };
> 
>     port@1 {
>         reg = <1>;
> 
>         rts5411_2_0: endpoint {
>             // should not be needed
>             remote-endpoint = <&usb_1_dwc3_port2>;
>         };
>     };
> };
> 
> &usb_1_dwc3 {
>     dr_mode = "host";
>     #address-cells = <1>;
>     #size-cells = <0>;
> 
>     port@1 {
>         reg = <1>;
> 
>         usb_1_dwc3_port1: endpoint {
>             remote-endpoint = <&rts5411_3_0>;
>         };
>     };
> 
>     port@2 {
>         reg = <2>;
> 
>         usb_1_dwc3_port2: endpoint {
>             remote-endpoint = <&rts5411_2_0>;
>         };
>     };
> };
> 
> That looks like an actual description of the hardware, without multiple DT
> nodes for the hub.
> 
> The USB part of the onboard_hub driver could determine the platform device
> from the remote endpoint and register the USB device with it.

Yeah you got it, this is what I'm suggesting. I'd like to go even
further and also describe the ports on "the other side" or "downstream
facing side" of the USB hub that go to things like type-c ports or
type-a ports. This would allow us to describe the USB topology and how
it is physically connected to ports on the device that humans see. But
that's mostly a job for the type-c subsystem, so it's a parallel
discussion that's only related because of the graph binding.