Re: [PATCH v2 7/9] arm64: dts: amlogic: Used onboard usb hub reset on odroid c4

[Anand Moon]

Hi Neil,

On Wed, 18 Jan 2023 at 18:54, <neil.armstrong@xxxxxxxxxx> wrote:
>
> On 18/01/2023 12:55, Anand Moon wrote:
> > Hi Neil,
> >
> > Thanks for your review comments.
> >
> > On Wed, 18 Jan 2023 at 13:59, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
> >>
> >> On 18/01/2023 05:44, Anand Moon wrote:
> >>> On Odroid c4 previously use gpio-hog to reset the usb hub,
> >>> switch to used on-board usb hub reset to enable the usb hub
> >>> and enable power to hub.
> >>>
> >>> USB hub is combination of USB 2.0 and USB 3.0 root hub so
> >>> use peer-hub node to link then.
> >>>
> >>> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx>
> >>> ---
> >>> v2: - fix the compatible string.
> >>>       - Fix the hub node to use peer-hub to link the usb 2.0 and usb 3.0.
> >>> ---
> >>>    .../boot/dts/amlogic/meson-sm1-odroid-c4.dts  | 36 ++++++++++++-------
> >>>    1 file changed, 23 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-c4.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-c4.dts
> >>> index 8c30ce63686e..d04768a66bfe 100644
> >>> --- a/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-c4.dts
> >>> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-odroid-c4.dts
> >>> @@ -26,20 +26,30 @@ led-blue {
> >>>        sound {
> >>>                model = "ODROID-C4";
> >>>        };
> >>> -};
> >>>
> >>> -&gpio {
> >>> -     /*
> >>> -      * WARNING: The USB Hub on the Odroid-C4 needs a reset signal
> >>> -      * to be turned high in order to be detected by the USB Controller
> >>> -      * This signal should be handled by a USB specific power sequence
> >>> -      * in order to reset the Hub when USB bus is powered down.
> >>> -      */
> >>> -     hog-0 {
> >>> -             gpio-hog;
> >>> -             gpios = <GPIOH_4 GPIO_ACTIVE_HIGH>;
> >>> -             output-high;
> >>> -             line-name = "usb-hub-reset";
> >>> +     /* USB hub supports both USB 2.0 and USB 3.0 root hub */
> >>> +     usb-hub {
> >>> +             dr_mode = "host";
> >>
> >> Is this really needed ?
> >>
> > I got carried forward from the other device tree binding,
> > If not needed I will drop this.
> >
> >>> +             #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>;
> >>> +             };
> >>
> >> The final discussion in v1 was to drop this /usb-hub node and move the
> >> hub_2_0 & hub_3_0 node under the dwc3 node.
> >>
> >
> > Yes, but It did not work back then, since these are two different events
> > USB node will try to bring the PHY and dwc2 and dwc2 nodes up.
> > USB hub supports the reset of the USB hub and links the power supply
> > to the ports.
> > This works on this board.
>
> Forget the dwc2 node, the dwc2 since GXL is device mode only, so you need to put both
> nodes in the dwc3 node which is host-only.
>
> Neil
>
Ok, I will move this node under dwc3 node, in the next version.

> >
> >> Neil

Thanks
-Anand