2023-10-11 05:12:30

by Anand Moon

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

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 <[email protected]>
---
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


2023-10-11 06:31:49

by Rob Herring (Arm)

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


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 <[email protected]>
> ---
> 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/[email protected]

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.

2023-10-12 03:21:12

by Anand Moon

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

Hi Rob,

On Wed, 11 Oct 2023 at 12:01, Rob Herring <[email protected]> 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 <[email protected]>
> > ---
> > 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/[email protected]
>
> 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

2023-10-12 07:44:19

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> 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

2023-10-12 16:37:58

by Anand Moon

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

Hi Krzysztof,

On Thu, 12 Oct 2023 at 13:13, Krzysztof Kozlowski
<[email protected]> 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/[email protected]/

>
> >
> > Signed-off-by: Anand Moon <[email protected]>
> > ---
> > 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

2023-10-12 18:00:58

by Krzysztof Kozlowski

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

On 12/10/2023 18:37, Anand Moon wrote:
> Hi Krzysztof,
>
> On Thu, 12 Oct 2023 at 13:13, Krzysztof Kozlowski
> <[email protected]> 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/[email protected]/

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

2023-10-13 00:53:54

by Anand Moon

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

Hi Krzysztof,

On Thu, 12 Oct 2023 at 23:30, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 12/10/2023 18:37, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > On Thu, 12 Oct 2023 at 13:13, Krzysztof Kozlowski
> > <[email protected]> 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/[email protected]/
>
> 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