On Mon, Dec 30, 2019 at 8:07 PM Chunyan Zhang [off-list ref] wrote:

On Fri, 27 Dec 2019 at 02:56, Rob Herring [off-list ref] wrote:

quoted

On Mon, Dec 16, 2019 at 08:19:29PM +0800, Chunyan Zhang wrote:

quoted

From: Chunyan Zhang <redacted>

add a new bindings to describe sc9863a clock compatible string.

Signed-off-by: Chunyan Zhang <redacted>
---
 .../bindings/clock/sprd,sc9863a-clk.yaml      | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.yaml

diff --git a/Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.yaml b/Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.yaml
new file mode 100644
index 000000000000..881f0a0287e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.yaml

@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2019 Unisoc Inc.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/clock/sprd,sc9863a-clk.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: SC9863A Clock Control Unit Device Tree Bindings
+
+maintainers:
+  - Orson Zhai <orsonzhai@gmail.com>
+  - Baolin Wang <baolin.wang7@gmail.com>
+  - Chunyan Zhang <zhang.lyra@gmail.com>
+
+properties:
+  "#clock-cells":
+    const: 1
+
+  compatible :
+    enum:
+      - sprd,sc9863a-ap-clk
+      - sprd,sc9863a-pmu-gate
+      - sprd,sc9863a-pll
+      - sprd,sc9863a-mpll
+      - sprd,sc9863a-rpll
+      - sprd,sc9863a-dpll
+      - sprd,sc9863a-aon-clk
+      - sprd,sc9863a-apahb-gate
+      - sprd,sc9863a-aonapb-gate
+      - sprd,sc9863a-mm-gate
+      - sprd,sc9863a-mm-clk
+      - sprd,sc9863a-vspahb-gate
+      - sprd,sc9863a-apapb-gate

These will probably need to be split to separate schemas for the reasons
below...

quoted

+
+  clocks:
+    description: |
+      The input parent clock(s) phandle for this clock, only list fixed
+      clocks which are decleared in devicetree.

typo.

You need to define how many clocks.

Ok, will add a define of maxItems.

quoted

quoted

+
+  clock-names:
+    description: |
+      Clock name strings used for driver to reference.

You need to list out the names.

quoted

+
+  reg:
+    description: |
+      Contain the registers base address and length. It must be configured
+      only if no 'sprd,syscon' under the node.
+
+  sprd,syscon:
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+    description: |
+      The phandle to the syscon which is in the same address area with
+      the clock, and so we can get regmap for the clocks from the
+      syscon device.

It is preferred to make the clock node a child of the syscon and then
you don't need this property.

According to the hardware topology, any clocks are not belonged to
syscon, like described here, this phandle is only used to get virtual
map address for clocks which have the same phsical address base with
one syscon.

In the past, clocks were defined like below:
    apahb_gate: apahb-gate {
      compatible = "sprd,sc9863a-apahb-gate";
      reg = <0 0x20e00000 0 0x1000>;
      #clock-cells = <1>;
    };

And there was also a syscon which had the same base address like below:
ap_ahb_regs: syscon@20e00000 {
compatible = "sprd,sc9863a-glbregs", "syscon";
reg = <0 0x20e00000 0 0x4000>;
};

To avoid one phsical address was remapped more than one time, I think
using the mapped address by syscon directly would be better.
Any other suggestions are very appreciated.

I'm only suggesting you change the location of the node and get the
syscon by getting the parent node. IOW, do this:

ap_ahb_regs: syscon@20e00000 {
    compatible = "sprd,sc9863a-glbregs", "syscon";
    reg = <0 0x20e00000 0 0x4000>;

    apahb_gate: apahb-gate {
        compatible = "sprd,sc9863a-apahb-gate";
        #clock-cells = <1>;
    };
};

Now, if there is a sub range of registers for the clocks that are
contiguous, then apahb-gate should have a 'reg' property too (and
ranges in the parent). Linux will not use that, but it's a more
complete description of the h/w.

Rob