| Message ID | 20221228100321.15949-9-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,01/11] dt-bindings: usb: Add device id for Genesys Logic hub controller | expand |
On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote: > The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that > features 4 downstream ports, an internal 5V regulator and has > external reset pin. > > Add a device tree binding for its USB protocol part. > The internal LDO is not covered by this and can just be modelled > as a fixed regulator. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > .../bindings/usb/vialab,vl817q7.yaml | 47 +++++++++++++++++++ > 1 file changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > new file mode 100644 > index 000000000000..4ae995160fd5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > @@ -0,0 +1,47 @@ > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > +%YAML 1.2 > +--- > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Via labs VL817Q7 USB 3.1 hub controller > + > +maintainers: > + - Anand Moon <linux.amoon@gmail.com> > + > +allOf: > + - $ref: usb-device.yaml# > + > +properties: > + compatible: > + enum: > + - vialab,usb2109 This isn't a valid compatible string for USB devices (should be "usb<vid>,<pid>"). Same for the other binding. Also the bindings should go before the driver changes in the series. Johan
Hi Johan Thanks for your review comments. On Wed, 28 Dec 2022 at 16:32, Johan Hovold <johan@kernel.org> wrote: > > On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote: > > The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that > > features 4 downstream ports, an internal 5V regulator and has > > external reset pin. > > > > Add a device tree binding for its USB protocol part. > > The internal LDO is not covered by this and can just be modelled > > as a fixed regulator. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > .../bindings/usb/vialab,vl817q7.yaml | 47 +++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > > > diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > new file mode 100644 > > index 000000000000..4ae995160fd5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > @@ -0,0 +1,47 @@ > > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Via labs VL817Q7 USB 3.1 hub controller > > + > > +maintainers: > > + - Anand Moon <linux.amoon@gmail.com> > > + > > +allOf: > > + - $ref: usb-device.yaml# > > + > > +properties: > > + compatible: > > + enum: > > + - vialab,usb2109 > > This isn't a valid compatible string for USB devices (should be > "usb<vid>,<pid>"). > > Same for the other binding. > ok, I will change this in the next version. > Also the bindings should go before the driver changes in the series. > > Johan Thanks -Anand
On Wed, 28 Dec 2022 10:03:17 +0000, Anand Moon wrote: > The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that > features 4 downstream ports, an internal 5V regulator and has > external reset pin. > > Add a device tree binding for its USB protocol part. > The internal LDO is not covered by this and can just be modelled > as a fixed regulator. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > .../bindings/usb/vialab,vl817q7.yaml | 47 +++++++++++++++++++ > 1 file changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > 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/vialab,vl817q7.yaml: '$id' is a required property hint: Metaschema for devicetree binding documentation from schema $id: http://devicetree.org/meta-schemas/base.yaml# Error: Documentation/devicetree/bindings/usb/vialab,vl817q7.example.dts:26.17-18 syntax error FATAL ERROR: Unable to parse input tree make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/usb/vialab,vl817q7.example.dtb] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1508: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221228100321.15949-9-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.
On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote: > The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that > features 4 downstream ports, an internal 5V regulator and has > external reset pin. > > Add a device tree binding for its USB protocol part. > The internal LDO is not covered by this and can just be modelled > as a fixed regulator. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > .../bindings/usb/vialab,vl817q7.yaml | 47 +++++++++++++++++++ > 1 file changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > new file mode 100644 > index 000000000000..4ae995160fd5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > @@ -0,0 +1,47 @@ > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > +%YAML 1.2 > +--- > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Via labs VL817Q7 USB 3.1 hub controller nit: VIA Labs VL817-Q7 > + > +maintainers: > + - Anand Moon <linux.amoon@gmail.com> > + > +allOf: > + - $ref: usb-device.yaml# > + > +properties: > + compatible: > + enum: > + - vialab,usb2109 This is not a valid compatible string as Johan already noted. Besides that the VL817-Q7 provides both a 3.1 and a 2.0 USB hub, which are enumerated separately. Please also add a compatible string for the 2.0 hub (assuming 0x2109 is the 3.1 hub). > + > + reg: true > + > + reset-gpios: > + description: GPIO controlling the RESET# pin. > + > + vdd-supply: > + description: > + the regulator that provides 5.0V core power to the hub. > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + usb { > + dr_mode = "host"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + hub: hub@1 { > + compatible = "vialab,usb2109" > + reg = <1>; > + reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>; > + }; Please also add a node for the other hub and link the two nodes with each other through the 'peer-hub' property. See realtek,rts5411.yaml for reference.
Hi Matthias, On Thu, 5 Jan 2023 at 04:07, Matthias Kaehlcke <mka@chromium.org> wrote: > > On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote: > > The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that > > features 4 downstream ports, an internal 5V regulator and has > > external reset pin. > > > > Add a device tree binding for its USB protocol part. > > The internal LDO is not covered by this and can just be modelled > > as a fixed regulator. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > .../bindings/usb/vialab,vl817q7.yaml | 47 +++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > > > diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > new file mode 100644 > > index 000000000000..4ae995160fd5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml > > @@ -0,0 +1,47 @@ > > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Via labs VL817Q7 USB 3.1 hub controller > > nit: VIA Labs VL817-Q7 > Ok > > + > > +maintainers: > > + - Anand Moon <linux.amoon@gmail.com> > > + > > +allOf: > > + - $ref: usb-device.yaml# > > + > > +properties: > > + compatible: > > + enum: > > + - vialab,usb2109 > > This is not a valid compatible string as Johan already noted. > > Besides that the VL817-Q7 provides both a 3.1 and a 2.0 USB hub, which > are enumerated separately. Please also add a compatible string for the > 2.0 hub (assuming 0x2109 is the 3.1 hub). > Yes, correct, actually, I would like to rename this file to vialab,vl817.yaml since vialab,vl817-q7 is used for USB 3.1 hub and vialab,vl817-q5 is used for USB 2.0 hub. [0] https://datasheet.lcsc.com/lcsc/1808111624_VIA-Tech-VL817-Q7-B0_C209756.pdf > > + > > + reg: true > > + > > + reset-gpios: > > + description: GPIO controlling the RESET# pin. > > + > > + vdd-supply: > > + description: > > + the regulator that provides 5.0V core power to the hub. > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + usb { > > + dr_mode = "host"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + hub: hub@1 { > > + compatible = "vialab,usb2109" > > + reg = <1>; > > + reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>; > > + }; > > Please also add a node for the other hub and link the two nodes with > each other through the 'peer-hub' property. See realtek,rts5411.yaml > for reference. Ok, I will update the example according, Thanks -Anand.
diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml new file mode 100644 index 000000000000..4ae995160fd5 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause +%YAML 1.2 +--- +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Via labs VL817Q7 USB 3.1 hub controller + +maintainers: + - Anand Moon <linux.amoon@gmail.com> + +allOf: + - $ref: usb-device.yaml# + +properties: + compatible: + enum: + - vialab,usb2109 + + reg: true + + reset-gpios: + description: GPIO controlling the RESET# pin. + + vdd-supply: + description: + the regulator that provides 5.0V core power to the hub. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + usb { + dr_mode = "host"; + #address-cells = <1>; + #size-cells = <0>; + + hub: hub@1 { + compatible = "vialab,usb2109" + reg = <1>; + reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>; + }; + };
The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that features 4 downstream ports, an internal 5V regulator and has external reset pin. Add a device tree binding for its USB protocol part. The internal LDO is not covered by this and can just be modelled as a fixed regulator. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- .../bindings/usb/vialab,vl817q7.yaml | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml