Re: [PATCH 2/2] arm64: dts: amlogic: add libretech cottonwood support

[Neil Armstrong]

On 02/10/2023 20:57, Jerome Brunet wrote:
> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:

<snip>

> > > +&usb3_pcie_phy {
> > > +	#address-cells = <1>;
> > > +	#size-cells = <0>;
> > > +	phy-supply = <&vcc_5v>;
> > > +
> > > +	hub: hub@1 {
> > > +		compatible = "usb5e3,626";
> > > +		reg = <1>;
> > > +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
> > > +	};
> >
> > Not sure the PHY is the right place to put the USB HUB,
> > and it's probable the HUB is connected to both the USB2 and USB3 lines
>
> It is connected to the USB3.0 only
>
> > so you should have both USB IDs in DT like it'd done for the Odroid-C4:
> >
> > / {
> > ...
> >           /* USB hub supports both USB 2.0 and USB 3.0 root hub */
> >           usb-hub {
> >                   dr_mode = "host";
> >                   #address-cells = <1>;
> >                   #size-cells = <0>;
> >
> >                   /* 2.0 hub on port 1 */
> >                   hub_2_0: hub@1 {
> >                           compatible = "usb2109,2817";
> >                           reg = <1>;
> >                           peer-hub = <&hub_3_0>;
> >                           reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> >                           vdd-supply = <&vcc_5v>;
> >                   };
> >
> >                   /* 3.1 hub on port 4 */
> >                   hub_3_0: hub@2 {
> >                           compatible = "usb2109,817";
> >                           reg = <2>;
> >                           peer-hub = <&hub_2_0>;
> >                           reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> >                           vdd-supply = <&vcc_5v>;
> >                   };
> >           };
> > ...
> > };
> >
> > if it only has a single USB ID, then it should go under the dwc3 node.
>
> The usb controller is connected to the PHY and what's coming out of the PHY
> goes to the hub. It seems logical to hub the hub under it.
>
> Why bypass the PHY ?

The USB bindings the USB devices nodes should be under the controller's node,
not the PHY, see:

Documentation/devicetree/bindings/usb/usb-hcd.yaml
...
patternProperties:
  "^.*@[0-9a-f]{1,2}$":
    description: The hard wired USB devices
    type: object
    $ref: /schemas/usb/usb-device.yaml
...
and the example.

Subnodes aren't allowed in the PHY node.

Neil

> > > +};
> > > +
> > > +&usb {
> > > +	status = "okay";
> > > +};

<snip>