Messages in this thread | | | Date | Tue, 19 Apr 2022 18:12:08 +0800 | Subject | Re: [PATCH v3 2/5] dt-bindings: clock: Document MA35D1 clock controller bindings | From | 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
| |