| Message ID | 20231128061118.575847-3-ychuang570808@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for nuvoton ma35d1 pin control | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On Tue, 28 Nov 2023 06:11:16 +0000, Jacky Huang wrote: > From: Jacky Huang <ychuang3@nuvoton.com> > > Add documentation to describe nuvoton ma35d1 pin control and GPIO. > > Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> > --- > .../pinctrl/nuvoton,ma35d1-pinctrl.yaml | 189 ++++++++++++++++++ > 1 file changed, 189 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.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: Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.example.dts:27:18: fatal error: dt-bindings/pinctrl/ma35d1-pinfunc.h: No such file or directory 27 | #include <dt-bindings/pinctrl/ma35d1-pinfunc.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2 make: *** [Makefile:234: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231128061118.575847-3-ychuang570808@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 28/11/2023 07:11, Jacky Huang wrote: > From: Jacky Huang <ychuang3@nuvoton.com> > > Add documentation to describe nuvoton ma35d1 pin control and GPIO. > > Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> > --- Your changelog said: > - Remove ma35d1-pinfunc.h which is unused after update definition of 'nuvoton,pins'. You forgot to add: " - Do not test the bindings before sending" I assume none of the driver changes compile either. > .../pinctrl/nuvoton,ma35d1-pinctrl.yaml | 189 ++++++++++++++++++ > 1 file changed, 189 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml > > diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml > new file mode 100644 > index 000000000000..84287293a726 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml > @@ -0,0 +1,189 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/nuvoton,ma35d1-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton MA35D1 pin control and GPIO > + > +maintainers: > + - Shan-Chun Hung <schung@nuvoton.com> > + > +properties: > + compatible: > + enum: > + - nuvoton,ma35d1-pinctrl > + > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 1 > + > + nuvoton,sys: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + phandle of the system-management node. > + > + ranges: true > + > +allOf: > + - $ref: pinctrl.yaml# allOf goes before additionalProperties. > + > +required: > + - compatible > + - nuvoton,sys This goes after patternProperties > + > +patternProperties: > + "^gpio@[0-9a-f]+$": > + type: object > + additionalProperties: false > + properties: > + gpio-controller: true > + > + '#gpio-cells': > + const: 2 > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 2 > + > + interrupts: > + description: > + The interrupt outputs to sysirq. > + maxItems: 1 > + > + required: > + - gpio-controller > + - '#gpio-cells' > + - reg > + - clocks > + - interrupt-controller > + - '#interrupt-cells' > + - interrupts > + > + "^pin-[a-z0-9]+$": > + type: object > + description: > + A pinctrl node should contain at least one subnodes representing the > + pinctrl groups available on the machine. Each subnode will list the > + pins it needs, and how they should be configured, with regard to muxer > + configuration, pullups, drive strength, input enable/disable and input > + schmitt. > + > + allOf: Drop allOf, just $ref > + - $ref: pincfg-node.yaml# > + > + properties: > + bias-disable: true Drop this and other "true", why do you need them here? > + > + bias-pull-down: true > + > + bias-pull-up: true > + > + power-source: > + description: | > + Valid arguments are described as below: > + 0: power supply of 1.8V > + 1: power supply of 3.3V > + enum: [0, 1] > + > + drive-strength-microamp: > + oneOf: > + - enum: [ 2900, 4400, 5800, 7300, 8600, 10100, 11500, 13000 ] > + description: 1.8V I/O driving strength > + - enum: [ 17100, 25600, 34100, 42800, 48000, 56000, 77000, 82000 ] > + description: 3.3V I/O driving strength > + > + input-enable: true > + > + input-schmitt-enable: true > + > + unevaluatedProperties: false Best regards, Krzysztof
Dear Krzysztof, Thanks for your review. On 2023/11/28 下午 03:34, Krzysztof Kozlowski wrote: > On 28/11/2023 07:11, Jacky Huang wrote: >> From: Jacky Huang <ychuang3@nuvoton.com> >> >> Add documentation to describe nuvoton ma35d1 pin control and GPIO. >> >> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >> --- > Your changelog said: > >> - Remove ma35d1-pinfunc.h which is unused after update definition of > 'nuvoton,pins'. > > You forgot to add: > > " - Do not test the bindings before sending" > > I assume none of the driver changes compile either. It's my mistake. I forgot to remove 'ma35d1-pinfunc.h' from my local copy, and as a consequence, the 'dt_binding_check' did not catch this error. I will fix this. >> .../pinctrl/nuvoton,ma35d1-pinctrl.yaml | 189 ++++++++++++++++++ >> 1 file changed, 189 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml >> new file mode 100644 >> index 000000000000..84287293a726 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml >> @@ -0,0 +1,189 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,ma35d1-pinctrl.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Nuvoton MA35D1 pin control and GPIO >> + >> +maintainers: >> + - Shan-Chun Hung <schung@nuvoton.com> >> + >> +properties: >> + compatible: >> + enum: >> + - nuvoton,ma35d1-pinctrl >> + >> + '#address-cells': >> + const: 1 >> + >> + '#size-cells': >> + const: 1 >> + >> + nuvoton,sys: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: >> + phandle of the system-management node. >> + >> + ranges: true >> + >> +allOf: >> + - $ref: pinctrl.yaml# > allOf goes before additionalProperties. > >> + >> +required: >> + - compatible >> + - nuvoton,sys > This goes after patternProperties I will fix the above two as: allOf: - $ref: pinctrl.yaml# required: - compatible - nuvoton,sys additionalProperties: false examples: >> + >> +patternProperties: >> + "^gpio@[0-9a-f]+$": >> + type: object >> + additionalProperties: false >> + properties: >> + gpio-controller: true >> + >> + '#gpio-cells': >> + const: 2 >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + interrupt-controller: true >> + >> + '#interrupt-cells': >> + const: 2 >> + >> + interrupts: >> + description: >> + The interrupt outputs to sysirq. >> + maxItems: 1 >> + >> + required: >> + - gpio-controller >> + - '#gpio-cells' >> + - reg >> + - clocks >> + - interrupt-controller >> + - '#interrupt-cells' >> + - interrupts >> + >> + "^pin-[a-z0-9]+$": >> + type: object >> + description: >> + A pinctrl node should contain at least one subnodes representing the >> + pinctrl groups available on the machine. Each subnode will list the >> + pins it needs, and how they should be configured, with regard to muxer >> + configuration, pullups, drive strength, input enable/disable and input >> + schmitt. >> + >> + allOf: > Drop allOf, just $ref OK, I will fix it. >> + - $ref: pincfg-node.yaml# >> + >> + properties: >> + bias-disable: true > Drop this and other "true", why do you need them here? We are following the conventions used in other pinctrl documents, such as 'realtek,rtd1315e-pinctrl.yaml' and 'xlnx,zynqmp-pinctrl.yaml'. After comparing various pinctrl documents, I noticed that they all express it as 'bias-disable: true'. Therefore, may I keep the current format? >> + >> + bias-pull-down: true >> + >> + bias-pull-up: true >> + >> + power-source: >> + description: | >> + Valid arguments are described as below: >> + 0: power supply of 1.8V >> + 1: power supply of 3.3V >> + enum: [0, 1] >> + >> + drive-strength-microamp: >> + oneOf: >> + - enum: [ 2900, 4400, 5800, 7300, 8600, 10100, 11500, 13000 ] >> + description: 1.8V I/O driving strength >> + - enum: [ 17100, 25600, 34100, 42800, 48000, 56000, 77000, 82000 ] >> + description: 3.3V I/O driving strength >> + >> + input-enable: true >> + >> + input-schmitt-enable: true >> + >> + unevaluatedProperties: false > > > Best regards, > Krzysztof > Best Regards, Jacky Huang
On 28/11/2023 09:29, Jacky Huang wrote: > Dear Krzysztof, > > Thanks for your review. > > > On 2023/11/28 下午 03:34, Krzysztof Kozlowski wrote: >> On 28/11/2023 07:11, Jacky Huang wrote: >>> From: Jacky Huang <ychuang3@nuvoton.com> >>> >>> Add documentation to describe nuvoton ma35d1 pin control and GPIO. >>> >>> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >>> --- >> Your changelog said: >> >>> - Remove ma35d1-pinfunc.h which is unused after update definition of >> 'nuvoton,pins'. >> >> You forgot to add: >> >> " - Do not test the bindings before sending" >> >> I assume none of the driver changes compile either. > > It's my mistake. I forgot to remove 'ma35d1-pinfunc.h' from my local > copy, and as a consequence, the 'dt_binding_check' did not catch this > error. I will fix this. But then git status would point you that tree is not clean and you did not finish commiting. .. >>> + >>> +allOf: >>> + - $ref: pinctrl.yaml# >> allOf goes before additionalProperties. >> >>> + >>> +required: >>> + - compatible >>> + - nuvoton,sys >> This goes after patternProperties > > I will fix the above two as: > > allOf: > - $ref: pinctrl.yaml# Look: >> allOf goes before additionalProperties. Open example-schema. .. > >>> + - $ref: pincfg-node.yaml# >>> + >>> + properties: >>> + bias-disable: true >> Drop this and other "true", why do you need them here? > > We are following the conventions used in other pinctrl documents, such as > 'realtek,rtd1315e-pinctrl.yaml' and 'xlnx,zynqmp-pinctrl.yaml'. But they are quite different there. > > After comparing various pinctrl documents, I noticed that they all express > it as 'bias-disable: true'. Therefore, may I keep the current format? No, you cannot copy pieces of other binding, selectively ignoring the rest. Look how these other bindings are constructed - they have additionalProperties, which you don't. Drop all these true properties if the only reason of them being here is they were copied. Best regards, Krzysztof
Dear Krzysztof, Thanks for your review. On 2023/11/28 下午 04:33, Krzysztof Kozlowski wrote: > On 28/11/2023 09:29, Jacky Huang wrote: >> Dear Krzysztof, >> >> Thanks for your review. >> >> >> On 2023/11/28 下午 03:34, Krzysztof Kozlowski wrote: >>> On 28/11/2023 07:11, Jacky Huang wrote: >>>> From: Jacky Huang <ychuang3@nuvoton.com> >>>> >>>> Add documentation to describe nuvoton ma35d1 pin control and GPIO. >>>> >>>> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >>>> --- >>> Your changelog said: >>> >>>> - Remove ma35d1-pinfunc.h which is unused after update definition of >>> 'nuvoton,pins'. >>> >>> You forgot to add: >>> >>> " - Do not test the bindings before sending" >>> >>> I assume none of the driver changes compile either. >> It's my mistake. I forgot to remove 'ma35d1-pinfunc.h' from my local >> copy, and as a consequence, the 'dt_binding_check' did not catch this >> error. I will fix this. > But then git status would point you that tree is not clean and you did > not finish commiting. > > .. > Yes, I should have been more careful in checking. >>>> + >>>> +allOf: >>>> + - $ref: pinctrl.yaml# >>> allOf goes before additionalProperties. >>> >>>> + >>>> +required: >>>> + - compatible >>>> + - nuvoton,sys >>> This goes after patternProperties >> I will fix the above two as: >> >> allOf: >> - $ref: pinctrl.yaml# > Look: > >>> allOf goes before additionalProperties. > Open example-schema. > > .. I found that 'pinctrl.yaml' is not required for this document, so I will drop it. >>>> + - $ref: pincfg-node.yaml# >>>> + >>>> + properties: >>>> + bias-disable: true >>> Drop this and other "true", why do you need them here? >> We are following the conventions used in other pinctrl documents, such as >> 'realtek,rtd1315e-pinctrl.yaml' and 'xlnx,zynqmp-pinctrl.yaml'. > But they are quite different there. > >> After comparing various pinctrl documents, I noticed that they all express >> it as 'bias-disable: true'. Therefore, may I keep the current format? > No, you cannot copy pieces of other binding, selectively ignoring the > rest. Look how these other bindings are constructed - they have > additionalProperties, which you don't. > > Drop all these true properties if the only reason of them being here is > they were copied. > > > > Best regards, > Krzysztof > OK, I will drop these properties. Best Regards, Jacky Huang
On 28/11/2023 10:32, Jacky Huang wrote: >>>> +required: >>>>> + - compatible >>>>> + - nuvoton,sys >>>> This goes after patternProperties >>> I will fix the above two as: >>> >>> allOf: >>> - $ref: pinctrl.yaml# >> Look: >> >>>> allOf goes before additionalProperties. >> Open example-schema. >> >> .. > > I found that 'pinctrl.yaml' is not required for this document, so I will > drop it. Why is it not required? I don't understand where this discussion is going. Best regards, Krzysztof
On 2023/11/28 下午 05:35, Krzysztof Kozlowski wrote: > On 28/11/2023 10:32, Jacky Huang wrote: >>>>> +required: >>>>>> + - compatible >>>>>> + - nuvoton,sys >>>>> This goes after patternProperties >>>> I will fix the above two as: >>>> >>>> allOf: >>>> - $ref: pinctrl.yaml# >>> Look: >>> >>>>> allOf goes before additionalProperties. >>> Open example-schema. >>> >>> .. >> I found that 'pinctrl.yaml' is not required for this document, so I will >> drop it. > Why is it not required? I don't understand where this discussion is going. > > > > Best regards, > Krzysztof > I read the description of 'pinctrl.yaml'. Yes, it is required. So, I will add it to allOf: - $ref: pinctrl.yaml# properties: compatible: enum: - nuvoton,ma35d1-pinctrl '#address-cells': const: 1 ... Best Regards, Jacky Huang
diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml new file mode 100644 index 000000000000..84287293a726 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,ma35d1-pinctrl.yaml @@ -0,0 +1,189 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/nuvoton,ma35d1-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 pin control and GPIO + +maintainers: + - Shan-Chun Hung <schung@nuvoton.com> + +properties: + compatible: + enum: + - nuvoton,ma35d1-pinctrl + + '#address-cells': + const: 1 + + '#size-cells': + const: 1 + + nuvoton,sys: + $ref: /schemas/types.yaml#/definitions/phandle + description: + phandle of the system-management node. + + ranges: true + +allOf: + - $ref: pinctrl.yaml# + +required: + - compatible + - nuvoton,sys + +patternProperties: + "^gpio@[0-9a-f]+$": + type: object + additionalProperties: false + properties: + gpio-controller: true + + '#gpio-cells': + const: 2 + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + interrupt-controller: true + + '#interrupt-cells': + const: 2 + + interrupts: + description: + The interrupt outputs to sysirq. + maxItems: 1 + + required: + - gpio-controller + - '#gpio-cells' + - reg + - clocks + - interrupt-controller + - '#interrupt-cells' + - interrupts + + "^pin-[a-z0-9]+$": + type: object + description: + A pinctrl node should contain at least one subnodes representing the + pinctrl groups available on the machine. Each subnode will list the + pins it needs, and how they should be configured, with regard to muxer + configuration, pullups, drive strength, input enable/disable and input + schmitt. + + allOf: + - $ref: pincfg-node.yaml# + + properties: + bias-disable: true + + bias-pull-down: true + + bias-pull-up: true + + power-source: + description: | + Valid arguments are described as below: + 0: power supply of 1.8V + 1: power supply of 3.3V + enum: [0, 1] + + drive-strength-microamp: + oneOf: + - enum: [ 2900, 4400, 5800, 7300, 8600, 10100, 11500, 13000 ] + description: 1.8V I/O driving strength + - enum: [ 17100, 25600, 34100, 42800, 48000, 56000, 77000, 82000 ] + description: 3.3V I/O driving strength + + input-enable: true + + input-schmitt-enable: true + + unevaluatedProperties: false + + "-grp$": + type: object + description: + Pinctrl node's client devices use subnodes for desired pin configuration. + Client device subnodes use below standard properties. + properties: + nuvoton,pins: + description: + Each entry consists of 4 parameters and represents the mux and config + setting for one pin. + $ref: /schemas/types.yaml#/definitions/uint32-matrix + minItems: 1 + items: + items: + - minimum: 0 + maximum: 13 + description: + Pin bank. + - minimum: 0 + maximum: 15 + description: + Pin bank index. + - minimum: 0 + maximum: 15 + description: + Mux 0 means GPIO and mux 1 to 15 means the specific device function. + - description: + The phandle of a node contains the generic pinconfig options + to use as described in pinctrl-bindings.txt. + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + #include <dt-bindings/pinctrl/ma35d1-pinfunc.h> + + pinctrl@40040000 { + compatible = "nuvoton,ma35d1-pinctrl"; + #address-cells = <1>; + #size-cells = <1>; + nuvoton,sys = <&sys>; + ranges = <0 0x40040000 0xc00>; + + gpio@40040000 { + reg = <0x0 0x40>; + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk GPA_GATE>; + gpio-controller; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + }; + + pcfg_default: pin-default { + slew-rate = <0>; + input-schmitt-disable; + bias-disable; + power-source = <1>; + drive-strength-microamp = <17100>; + }; + + uart-grp { + uart11-pins { + nuvoton,pins = <11 0 2 &pcfg_default>, + <11 1 2 &pcfg_default>, + <11 2 2 &pcfg_default>, + <11 3 2 &pcfg_default>; + }; + + uart12-pins { + nuvoton,pins = <8 1 2 &pcfg_default>, + <8 2 2 &pcfg_default>, + <8 3 2 &pcfg_default>; + }; + }; + };