| Message ID | 20220418082738.11301-3-ychuang3@nuvoton.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add initial support for MA35D1 SoC | expand |
On 18/04/2022 10:27, Jacky Huang wrote: > Add documentation to describe Nuvoton MA35D1 clock driver bindings. > You skipped the review tag, so I assume because of amount of changes. Usually it is nice to mention it... > Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> > --- > .../bindings/clock/nuvoton,ma35d1-clk.yaml | 63 +++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml > > diff --git a/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml > new file mode 100644 > index 000000000000..d0d37c5e84af > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml > @@ -0,0 +1,63 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/nuvoton,ma35d1-clk.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton MA35D1 Clock Control Module Binding > + > +maintainers: > + - Chi-Fang Li <cfli0@nuvoton.com> > + - Jacky Huang <ychuang3@nuvoton.com> > + > +description: | > + The MA35D1 clock controller generates clocks for the whole chip, > + including system clocks and all peripheral clocks. > + > + See also: > + include/dt-bindings/clock/ma35d1-clk.h > + > +properties: > + compatible: > + const: nuvoton,ma35d1-clk > + > + reg: > + maxItems: 1 > + > + "#clock-cells": > + const: 1 > + > + assigned-clocks: What about clocks? This depends on clocks. What clocks do you want to assign if they are not an input to the device? > + minItems: 5 > + maxItems: 5 This is different than before. minItems should not be here. Why do you need assigned-clocks in the binding at all? > + > + assigned-clock-rates: > + minItems: 5 > + maxItems: 5 > + > + nuvoton,clk-pll-mode: > + A list of PLL operation mode corresponding to DDRPLL, APLL, EPLL, > + and VPLL in sequential. This does not look like a binding which was tested. Read "writing-schema" and test your bindings. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 5 No need for minItems. > + maxItems: 5 > + items: > + enum: [ 0, 1, 2 ] You need to describe the values in description, what's their meaning. > + > +required: > + - compatible > + - reg > + - "#clock-cells" > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> > + > + clk: clock-controller@40460200 { > + compatible = "nuvoton,ma35d1-clk"; > + reg = <0x0 0x40460200 0x0 0x100>; > + #clock-cells = <1>; > + }; > +... Best regards, Krzysztof
On Mon, 18 Apr 2022 16:27:35 +0800, Jacky Huang wrote: > Add documentation to describe Nuvoton MA35D1 clock driver bindings. > > Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> > --- > .../bindings/clock/nuvoton,ma35d1-clk.yaml | 63 +++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.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: ./Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml:41:9: [error] syntax error: mapping values are not allowed here (syntax) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.example.dts' Traceback (most recent call last): File "/usr/local/bin/dt-extract-example", line 52, in <module> binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load return constructor.get_single_data() File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data node = self.composer.get_single_node() File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context in "<unicode string>", line 41, column 9 make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml: mapping values are not allowed in this context in "<unicode string>", line 41, column 9 /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml: ignoring, error parsing file make: *** [Makefile:1401: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. 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.
On 2022/4/18 下午 09:35, Rob Herring wrote: > On Mon, 18 Apr 2022 16:27:35 +0800, Jacky Huang wrote: >> Add documentation to describe Nuvoton MA35D1 clock driver bindings. >> >> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >> --- >> .../bindings/clock/nuvoton,ma35d1-clk.yaml | 63 +++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.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: > ./Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml:41:9: [error] syntax error: mapping values are not allowed here (syntax) > > dtschema/dtc warnings/errors: > make[1]: *** Deleting file 'Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.example.dts' > Traceback (most recent call last): > File "/usr/local/bin/dt-extract-example", line 52, in <module> > binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) > File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load > return constructor.get_single_data() > File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data > node = self.composer.get_single_node() > File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node > File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document > File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node > File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node > File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node > File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node > File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event > ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context > in "<unicode string>", line 41, column 9 > make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.example.dts] Error 1 > make[1]: *** Waiting for unfinished jobs.... > ./Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml: mapping values are not allowed in this context > in "<unicode string>", line 41, column 9 > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml: ignoring, error parsing file > make: *** [Makefile:1401: dt_binding_check] Error 2 > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/patch/ > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > 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. > Thanks for your review. I see this error and know where the problem is. I will fix it in the next version. Sincerely, Jacky Huang
On 2022/4/18 下午 08:18, Krzysztof Kozlowski wrote: > On 18/04/2022 10:27, Jacky Huang wrote: >> Add documentation to describe Nuvoton MA35D1 clock driver bindings. >> > You skipped the review tag, so I assume because of amount of changes. > Usually it is nice to mention it... I search the mail loop and find the "Reviewed-by" tag. Now I know I should add the review tag to my patch. Thanks for your reminding. >> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >> --- >> .../bindings/clock/nuvoton,ma35d1-clk.yaml | 63 +++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml >> >> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml >> new file mode 100644 >> index 000000000000..d0d37c5e84af >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml >> @@ -0,0 +1,63 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Fnuvoton%2Cma35d1-clk.yaml%23&data=05%7C01%7Cychuang3%40nuvoton.com%7C345b237bf1254018654b08da213588f9%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637858811062058468%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gCnLGCaMeUI4kdu23m0T9g6eGPd37z8%2BatQQb%2Ftnxb4%3D&reserved=0 >> +$schema: https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cychuang3%40nuvoton.com%7C345b237bf1254018654b08da213588f9%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637858811062058468%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=dj6FannQd0GxVJ%2BGlAjqs08SbNzPKi6ibdyLxLfR4q4%3D&reserved=0 >> + >> +title: Nuvoton MA35D1 Clock Control Module Binding >> + >> +maintainers: >> + - Chi-Fang Li <cfli0@nuvoton.com> >> + - Jacky Huang <ychuang3@nuvoton.com> >> + >> +description: | >> + The MA35D1 clock controller generates clocks for the whole chip, >> + including system clocks and all peripheral clocks. >> + >> + See also: >> + include/dt-bindings/clock/ma35d1-clk.h >> + >> +properties: >> + compatible: >> + const: nuvoton,ma35d1-clk >> + >> + reg: >> + maxItems: 1 >> + >> + "#clock-cells": >> + const: 1 >> + >> + assigned-clocks: > What about clocks? This depends on clocks. What clocks do you want to > assign if they are not an input to the device? The clock source of all PLLs are from external 24 MHz crystal. Yes, I should add clocks such as clocks = <&hxt_24m> and add a node hxt_24m: hxt_24m { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <24000000>; clock-output-names = "hxt_24m"; }; >> + minItems: 5 >> + maxItems: 5 > This is different than before. minItems should not be here. > > Why do you need assigned-clocks in the binding at all? The clock controller is equipped with 5 PLLs, which generated clocks for CPU, DDR, and various peripheral bus. The assigned-clocks describe these PLL output clocks. I will remove the minItems. >> + >> + assigned-clock-rates: >> + minItems: 5 >> + maxItems: 5 >> + >> + nuvoton,clk-pll-mode: >> + A list of PLL operation mode corresponding to DDRPLL, APLL, EPLL, >> + and VPLL in sequential. > This does not look like a binding which was tested. Read > "writing-schema" and test your bindings. "nuvoton,clk-pll-mode" is a nonstandard property used to describe the operation mode of corresponding PLLs. (According to Device tree Specification section "2.2.4 Properties" Nonstandard property names should specify a unique string prefix, such as a stock ticker symbol, identifying the name of the company or organization that defined the property. Examples: fsl,channel-fifo-len ibm,ppc-interrupt-server#s linux,network-index) >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + minItems: 5 > No need for minItems. Yes, I will remove it. >> + maxItems: 5 >> + items: >> + enum: [ 0, 1, 2 ] > You need to describe the values in description, what's their meaning. OK, I will add description about the values represented for PLL operation modes. >> + >> +required: >> + - compatible >> + - reg >> + - "#clock-cells" >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> >> + >> + clk: clock-controller@40460200 { >> + compatible = "nuvoton,ma35d1-clk"; >> + reg = <0x0 0x40460200 0x0 0x100>; >> + #clock-cells = <1>; >> + }; >> +... > > Best regards, > Krzysztof Thanks for your review. Sincerely, Jacky Huang
On 19/04/2022 12:12, Jacky Huang wrote: >>> + >>> + assigned-clock-rates: >>> + minItems: 5 >>> + maxItems: 5 >>> + >>> + nuvoton,clk-pll-mode: >>> + A list of PLL operation mode corresponding to DDRPLL, APLL, EPLL, >>> + and VPLL in sequential. >> This does not look like a binding which was tested. Read >> "writing-schema" and test your bindings. > > "nuvoton,clk-pll-mode" is a nonstandard property used to describe the > operation mode of > corresponding PLLs. > > (According to Device tree Specification section "2.2.4 Properties" > Nonstandard property names should specify a unique string prefix, such > as a stock ticker symbol, identifying the name of > the company or organization that defined the property. Examples: I am not saying about property name. I replied under some description below which fails to build. Instead please test your bindings. Best regards, Krzysztof
On 2022/4/19 下午 06:39, Krzysztof Kozlowski wrote: > On 19/04/2022 12:12, Jacky Huang wrote: >>>> + >>>> + assigned-clock-rates: >>>> + minItems: 5 >>>> + maxItems: 5 >>>> + >>>> + nuvoton,clk-pll-mode: >>>> + A list of PLL operation mode corresponding to DDRPLL, APLL, EPLL, >>>> + and VPLL in sequential. >>> This does not look like a binding which was tested. Read >>> "writing-schema" and test your bindings. >> "nuvoton,clk-pll-mode" is a nonstandard property used to describe the >> operation mode of >> corresponding PLLs. >> >> (According to Device tree Specification section "2.2.4 Properties" >> Nonstandard property names should specify a unique string prefix, such >> as a stock ticker symbol, identifying the name of >> the company or organization that defined the property. Examples: > I am not saying about property name. I replied under some description > below which fails to build. > > Instead please test your bindings. > > Best regards, > Krzysztof OK, I got it. I found the error by dt_binding_check. I will fix them in the next version. Thank you very much. Jacky Huang
diff --git a/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml new file mode 100644 index 000000000000..d0d37c5e84af --- /dev/null +++ b/Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/nuvoton,ma35d1-clk.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 Clock Control Module Binding + +maintainers: + - Chi-Fang Li <cfli0@nuvoton.com> + - Jacky Huang <ychuang3@nuvoton.com> + +description: | + The MA35D1 clock controller generates clocks for the whole chip, + including system clocks and all peripheral clocks. + + See also: + include/dt-bindings/clock/ma35d1-clk.h + +properties: + compatible: + const: nuvoton,ma35d1-clk + + reg: + maxItems: 1 + + "#clock-cells": + const: 1 + + assigned-clocks: + minItems: 5 + maxItems: 5 + + assigned-clock-rates: + minItems: 5 + maxItems: 5 + + nuvoton,clk-pll-mode: + A list of PLL operation mode corresponding to DDRPLL, APLL, EPLL, + and VPLL in sequential. + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 5 + maxItems: 5 + items: + enum: [ 0, 1, 2 ] + +required: + - compatible + - reg + - "#clock-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + + clk: clock-controller@40460200 { + compatible = "nuvoton,ma35d1-clk"; + reg = <0x0 0x40460200 0x0 0x100>; + #clock-cells = <1>; + }; +...
Add documentation to describe Nuvoton MA35D1 clock driver bindings. Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> --- .../bindings/clock/nuvoton,ma35d1-clk.yaml | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,ma35d1-clk.yaml