Re: [PATCH v3 2/5] dt-bindings: clock: Document MA35D1 clock controller bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxx>
---
  .../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&amp;data=05%7C01%7Cychuang3%40nuvoton.com%7C345b237bf1254018654b08da213588f9%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637858811062058468%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=gCnLGCaMeUI4kdu23m0T9g6eGPd37z8%2BatQQb%2Ftnxb4%3D&amp;reserved=0
+$schema: https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Cychuang3%40nuvoton.com%7C345b237bf1254018654b08da213588f9%7Ca3f24931d4034b4a94f17d83ac638e07%7C0%7C0%7C637858811062058468%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=dj6FannQd0GxVJ%2BGlAjqs08SbNzPKi6ibdyLxLfR4q4%3D&amp;reserved=0
+
+title: Nuvoton MA35D1 Clock Control Module Binding
+
+maintainers:
+  - Chi-Fang Li <cfli0@xxxxxxxxxxx>
+  - Jacky Huang <ychuang3@xxxxxxxxxxx>
+
+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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux