| Message ID | 20231011051152.133257-1-linux.amoon@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
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.
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
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
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
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
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
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>; }; };
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(-)