[PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[Anand Moon]

Add the binding example for the USB3.1 Genesys Logic GL3523
integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
hub.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
New patch.
---
 .../bindings/usb/genesys,gl850g.yaml          | 28 +++++++++++++++++--
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
index d0927f6768a4..2f6e0c870e1d 100644
--- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
+++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
@@ -22,29 +22,51 @@ properties:
   reg: true
 
   reset-gpios:
+    maxItems: 1
     description: GPIO controlling the RESET# pin.
 
   vdd-supply:
     description:
       the regulator that provides 3.3V core power to the hub.
 
+  peer-hub:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to the peer hub on the controller.
+
 required:
   - compatible
   - reg
+  - reset-gpios
+  - vdd-supply
+  - peer-hub
 
 additionalProperties: false
 
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
+
     usb {
         dr_mode = "host";
         #address-cells = <1>;
         #size-cells = <0>;
 
-        hub: hub@1 {
-            compatible = "usb5e3,608";
+        /* 2.0 hub on port 1 */
+        hub_2_0: hub@1 {
+            compatible = "usb5e3,610";
             reg = <1>;
-            reset-gpios = <&pio 7 2 GPIO_ACTIVE_LOW>;
+            vdd-supply = <&vcc_5v>;
+            peer-hub = <&hub_3_0>;
+            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
+        };
+
+        /* 3.1 hub on port 4 */
+        hub_3_0: hub@2 {
+            compatible = "usb5e3,620";
+            reg = <2>;
+            vdd-supply = <&vcc_5v>;
+            peer-hub = <&hub_2_0>;
+            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
         };
     };
-- 
2.42.0

[PATCH v2 2/2] arm64: dts: amlogic: Used onboard usb hub reset on odroid n2

[Anand Moon]

On Odroid n2/n2+ 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.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2 no change.
---
 .../dts/amlogic/meson-g12b-odroid-n2.dtsi     | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
index 91c9769fda20..9e671444eca6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
@@ -31,6 +31,30 @@ hub_5v: regulator-hub_5v {
 		enable-active-high;
 	};
 
+	/* 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 = "usb5e3,610";
+			reg = <1>;
+			peer-hub = <&hub_3_0>;
+			vdd-supply = <&usb_pwr_en>;
+		};
+
+		/* 3.0 hub on port 4 */
+		hub_3_0: hub@2 {
+			compatible = "usb5e3,620";
+			reg = <2>;
+			peer-hub = <&hub_2_0>;
+			reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
+			vdd-supply = <&vcc_5v>;
+		};
+	};
+
 	sound {
 		compatible = "amlogic,axg-sound-card";
 		model = "ODROID-N2";
@@ -234,18 +258,6 @@ &gpio {
 		"PIN_3",  /* GPIOX_17 */
 		"PIN_5",  /* GPIOX_18 */
 		"PIN_36"; /* GPIOX_19 */
-	/*
-	 * WARNING: The USB Hub on the Odroid-N2 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.
-	 */
-	usb-hub-hog {
-		gpio-hog;
-		gpios = <GPIOH_4 GPIO_ACTIVE_HIGH>;
-		output-high;
-		line-name = "usb-hub-reset";
-	};
 };
 
 &i2c3 {
-- 
2.42.0

Re: [PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[Rob Herring]

On Wed, 11 Oct 2023 10:41:48 +0530, Anand Moon wrote:
> Add the binding example for the USB3.1 Genesys Logic GL3523
> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> hub.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> New patch.
> ---
>  .../bindings/usb/genesys,gl850g.yaml          | 28 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb: hub@1: 'reset-gpios' is a required property
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb: hub@1: 'vdd-supply' is a required property
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb: hub@1: 'peer-hub' is a required property
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-device.example.dtb: hub@1: 'reset-gpios' is a required property
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-device.example.dtb: hub@1: 'vdd-supply' is a required property
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-device.example.dtb: hub@1: 'peer-hub' is a required property
	from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231011051152.133257-1-linux.amoon@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[Anand Moon]

Hi Rob,

On Wed, 11 Oct 2023 at 12:01, Rob Herring <robh@kernel.org> wrote:
>
>
> On Wed, 11 Oct 2023 10:41:48 +0530, Anand Moon wrote:
> > Add the binding example for the USB3.1 Genesys Logic GL3523
> > integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> > hub.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > New patch.
> > ---
> >  .../bindings/usb/genesys,gl850g.yaml          | 28 +++++++++++++++++--
> >  1 file changed, 25 insertions(+), 3 deletions(-)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb: hub@1: 'reset-gpios' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb: hub@1: 'vdd-supply' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb: hub@1: 'peer-hub' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-device.example.dtb: hub@1: 'reset-gpios' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-device.example.dtb: hub@1: 'vdd-supply' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/usb-device.example.dtb: hub@1: 'peer-hub' is a required property
>         from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231011051152.133257-1-linux.amoon@gmail.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

Can you share an example to add two examples in this binding?
one for usb5e3,608 and other for usb5e3,610, usb5e3,620,
I have tried but I got an error for duplicate

I have tried to modify it with the following example

+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: usb5e3,608
+    then:
+      properties:
+        reset-gpios: true
+        vdd-supply: false
+        peer-hub: false
+    else:
+      $ref: usb-device.yaml
+      required:
+        - peer-hub

but it still shows me his warning,

  DTC_CHK Documentation/devicetree/bindings/usb/usb-hcd.example.dtb
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb:
hub@1: 'peer-hub' is a required property
        from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#

I could not find any binding which supports these properties.
  - reset-gpios
  - vdd-supply
  - peer-hub

Please suggest to me how to resolve this warning.

Thanks
-Anand

Re: [PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[Krzysztof Kozlowski]

On 11/10/2023 07:11, Anand Moon wrote:
> Add the binding example for the USB3.1 Genesys Logic GL3523
> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> hub.

That's not what the patch does.

> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> New patch.
> ---
>  .../bindings/usb/genesys,gl850g.yaml          | 28 +++++++++++++++++--
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> index d0927f6768a4..2f6e0c870e1d 100644
> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> @@ -22,29 +22,51 @@ properties:
>    reg: true
>  
>    reset-gpios:
> +    maxItems: 1

Why?

>      description: GPIO controlling the RESET# pin.
>  
>    vdd-supply:
>      description:
>        the regulator that provides 3.3V core power to the hub.
>  
> +  peer-hub:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to the peer hub on the controller.
> +
>  required:
>    - compatible
>    - reg
> +  - reset-gpios

Why?

> +  - vdd-supply
> +  - peer-hub
>  
>  additionalProperties: false
>  
>  examples:
>    - |
>      #include <dt-bindings/gpio/gpio.h>
> +
>      usb {
>          dr_mode = "host";
>          #address-cells = <1>;
>          #size-cells = <0>;
>  
> -        hub: hub@1 {
> -            compatible = "usb5e3,608";
> +        /* 2.0 hub on port 1 */
> +        hub_2_0: hub@1 {
> +            compatible = "usb5e3,610";
>              reg = <1>;
> -            reset-gpios = <&pio 7 2 GPIO_ACTIVE_LOW>;
> +            vdd-supply = <&vcc_5v>;
> +            peer-hub = <&hub_3_0>;
> +            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
> +        };
> +
> +        /* 3.1 hub on port 4 */
> +        hub_3_0: hub@2 {
> +            compatible = "usb5e3,620";
> +            reg = <2>;
> +            vdd-supply = <&vcc_5v>;
> +            peer-hub = <&hub_2_0>;
> +            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;

Really, what is happening here?

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[Anand Moon]

Hi Krzysztof,

On Thu, 12 Oct 2023 at 13:13, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 11/10/2023 07:11, Anand Moon wrote:
> > Add the binding example for the USB3.1 Genesys Logic GL3523
> > integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> > hub.
>
> That's not what the patch does.

Ok I have tried to add an example below the original changes
but the device tree complained of duplicate entries. Hence I
modified these changes.

This change was requested to update the peer-hub example below.
[0] https://lore.kernel.org/all/9fe7d0d2-3582-4b62-be9b-aa9134c18023@linaro.org/

>
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > New patch.
> > ---
> >  .../bindings/usb/genesys,gl850g.yaml          | 28 +++++++++++++++++--
> >  1 file changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > index d0927f6768a4..2f6e0c870e1d 100644
> > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > @@ -22,29 +22,51 @@ properties:
> >    reg: true
> >
> >    reset-gpios:
> > +    maxItems: 1
>
> Why?

Following another example, I added this and will drop this.
>
> >      description: GPIO controlling the RESET# pin.
> >
> >    vdd-supply:
> >      description:
> >        the regulator that provides 3.3V core power to the hub.
> >
> > +  peer-hub:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      phandle to the peer hub on the controller.
> > +
> >  required:
> >    - compatible
> >    - reg
> > +  - reset-gpios
>
> Why?
see below.
>
> > +  - vdd-supply
> > +  - peer-hub
> >
> >  additionalProperties: false
> >
> >  examples:
> >    - |
> >      #include <dt-bindings/gpio/gpio.h>
> > +
> >      usb {
> >          dr_mode = "host";
> >          #address-cells = <1>;
> >          #size-cells = <0>;
> >
> > -        hub: hub@1 {
> > -            compatible = "usb5e3,608";
> > +        /* 2.0 hub on port 1 */
> > +        hub_2_0: hub@1 {
> > +            compatible = "usb5e3,610";
> >              reg = <1>;
> > -            reset-gpios = <&pio 7 2 GPIO_ACTIVE_LOW>;
> > +            vdd-supply = <&vcc_5v>;
> > +            peer-hub = <&hub_3_0>;
> > +            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
> > +        };
> > +
> > +        /* 3.1 hub on port 4 */
> > +        hub_3_0: hub@2 {
> > +            compatible = "usb5e3,620";
> > +            reg = <2>;
> > +            vdd-supply = <&vcc_5v>;
> > +            peer-hub = <&hub_2_0>;
> > +            reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>;
>
> Really, what is happening here?

USB hub GL3523-QFN76 supports two pins CHIP_EN and RST_N pins
so RST_N (GPIOH_4) is used to reset the USB hub,
earlier we were using gpio-hog to reset the hub.

>
> Best regards,
> Krzysztof
>

Thanks
-Anand

Re: [PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[Krzysztof Kozlowski]

On 12/10/2023 18:37, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Thu, 12 Oct 2023 at 13:13, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 11/10/2023 07:11, Anand Moon wrote:
>>> Add the binding example for the USB3.1 Genesys Logic GL3523
>>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
>>> hub.
>>
>> That's not what the patch does.
> 
> Ok I have tried to add an example below the original changes
> but the device tree complained of duplicate entries. Hence I
> modified these changes.
> 
> This change was requested to update the peer-hub example below.
> [0] https://lore.kernel.org/all/9fe7d0d2-3582-4b62-be9b-aa9134c18023@linaro.org/

Neil did not ask you to add it to the example but to the binding.
Existing example should be extended, but that's byproduct of main change.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub

[Anand Moon]

Hi Krzysztof,

On Thu, 12 Oct 2023 at 23:30, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 12/10/2023 18:37, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > On Thu, 12 Oct 2023 at 13:13, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 11/10/2023 07:11, Anand Moon wrote:
> >>> Add the binding example for the USB3.1 Genesys Logic GL3523
> >>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed
> >>> hub.
> >>
> >> That's not what the patch does.
> >
> > Ok I have tried to add an example below the original changes
> > but the device tree complained of duplicate entries. Hence I
> > modified these changes.
> >
> > This change was requested to update the peer-hub example below.
> > [0] https://lore.kernel.org/all/9fe7d0d2-3582-4b62-be9b-aa9134c18023@linaro.org/
>
> Neil did not ask you to add it to the example but to the binding.
> Existing example should be extended, but that's byproduct of main change.
>

Do you want me to add a new binding file to support this example.

# Documentation/devicetree/bindings/usb/genesys,gpn76.yaml

> Best regards,
> Krzysztof
>

Thanks
-Anand