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

[Neil Armstrong]

On 06/10/2023 10:21, Jerome Brunet wrote:
> On Thu 05 Oct 2023 at 12:04, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
>
> > On 05/10/2023 11:42, Jerome Brunet wrote:
> > > On Tue 03 Oct 2023 at 09:35, Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> > > wrote:
> > >
> > > > 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.
> > > Ok, that is what schema says.
> > > HW wise there is possible problem though.
> > > The phy node has the power supply to the bus.
> > > In that case it is a controllable one.
> > > If fixed USB devices go under the controller instead of the PHY, isn't
> > > it possible that the kernel may attempt to probe them before the bus is
> > > powered ? For this particular board, it would make the reset we are
> > > trying to apply useless.
> >
> > The usb core has a special handling for those usb hubs doing the power
> > up at the right time during the USB setup, including the PHY powering up.
> > So the power sequence should be fine.
> >
> > This has been done on Odroid-C2 and Odroid-N2 already.
>
> Tried it. Unfortunately something is off with the hub under the dwc3 node
> I often get this error (like once in 3 boots):
>
> [    0.419301] usbcore: registered new interface driver usbfs
> [    0.424434] usbcore: registered new interface driver hub
> [    0.429696] usbcore: registered new device driver usb
> [    0.921460] usbcore: registered new interface driver usb-storage
> [    0.968157] usbcore: registered new interface driver usbhid
> [    0.972114] usbhid: USB HID core driver
> [    1.132529] dwc3-meson-g12a ffe09000.usb: USB2 ports: 2
> [    1.134897] dwc3-meson-g12a ffe09000.usb: USB3 ports: 1
> [    1.144451] dwc2 ff400000.usb: supply vusb_d not found, using dummy regulator
> [    1.147231] dwc2 ff400000.usb: supply vusb_a not found, using dummy regulator
> [    1.154464] dwc2 ff400000.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM
> [    1.219515] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
> [    1.469260] usb 1-1: new high-speed USB device number 2 using xhci-hcd
> [    1.745395] usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
> [    9.794777] usbcore: registered new device driver onboard-usb-hub
> [   10.255484] onboard-usb-hub 1-1: Failed to suspend device, error -32
> [   10.261699] onboard-usb-hub 1-1: can't set config #1, error -71
> [   10.287500] onboard-usb-hub 1-1: Failed to suspend device, error -32
> [   10.287844] onboard-usb-hub 1-1: USB disconnect, device number 2
> [   10.573277] usb 1-1: new high-speed USB device number 3 using xhci-hcd
> [   10.921468] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
> [   11.193453] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
>
> While it works reliably when the onboard-usb-hub is under the phy node.
>
> I added the 5v supply as vdd under the hub for good measure.

The .reset_us you used from genesys_gl852g is probably too low, you may need to use a bigger one then.

Neil

> > Neil
> >
> > > > Neil
> > > >
> > > > > > > +};
> > > > > > > +
> > > > > > > +&usb {
> > > > > > > +	status = "okay";
> > > > > > > +};
> > > >
> > > > <snip>