Re: [PATCH v5 3/7] dt-bindings: clk: sprd: add bindings for sc9863a clock controller

[Chunyan Zhang]

On Wed, 26 Feb 2020 at 23:26, Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Wed, Feb 19, 2020 at 12:09:11PM +0800, Chunyan Zhang wrote:
> > From: Chunyan Zhang <chunyan.zhang@xxxxxxxxxx>
> >
> > add a new bindings to describe sc9863a clock compatible string.
> >
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@xxxxxxxxxx>
> > ---
> >  .../bindings/clock/sprd,sc9863a-clk.yaml      | 110 ++++++++++++++++++
> >  1 file changed, 110 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..b31569b524e5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/sprd,sc9863a-clk.yaml
> > @@ -0,0 +1,110 @@
> > +# 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@xxxxxxxxx>
> > +  - Baolin Wang <baolin.wang7@xxxxxxxxx>
> > +  - Chunyan Zhang <zhang.lyra@xxxxxxxxx>
> > +
> > +properties:
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  compatible :
> > +    enum:
> > +      - sprd,sc9863a-ap-clk
> > +      - sprd,sc9863a-aon-clk
> > +      - sprd,sc9863a-apahb-gate
> > +      - sprd,sc9863a-pmu-gate
> > +      - sprd,sc9863a-aonapb-gate
> > +      - sprd,sc9863a-pll
> > +      - sprd,sc9863a-mpll
> > +      - sprd,sc9863a-rpll
> > +      - sprd,sc9863a-dpll
> > +      - sprd,sc9863a-mm-gate
> > +      - sprd,sc9863a-apapb-gate
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 4
> > +    description: |
> > +      The input parent clock(s) phandle for this clock, only list fixed
> > +      clocks which are declared in devicetree.
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 4
> > +    description: |
> > +      Clock name strings used for driver to reference.
>
> Drop this. That's all 'clock-names'.

Ok.

>
> > +    items:
> > +      - const: ext-26m
> > +      - const: ext-32k
> > +      - const: ext-4m
> > +      - const: rco-100m
> > +
> > +  reg:
> > +    description: |
> > +      Contain the registers base address and length.
>
> Drop this. You need to define how many entries (maxItems: 1).

Ok.

>
> > +
> > +required:
> > +  - compatible
> > +  - '#clock-cells'
> > +
> > +if:
> > +  properties:
> > +    compatible:
> > +      enum:
> > +        - sprd,sc9863a-ap-clk
> > +        - sprd,sc9863a-aon-clk
> > +then:
> > +  required:
> > +    - reg
> > +
> > +else:
> > +  description: |
> > +    Other SC9863a clock nodes should be the child of a syscon node with
> > +    the required property:
> > +
> > +    - compatible: Should be the following:
> > +                  "sprd,sc9863a-glbregs", "syscon", "simple-mfd"
> > +
> > +    The 'reg' property is also required if there is a sub range of
> > +    registers for the clocks that are contiguous.
>
> Which ones are these? You should be able to define that exactly starting
> with the example below.

This is for the second example below, which clocks are under syscon node.

>
> > +
> > +examples:
> > +  - |
> > +    ap_clk: clock-controller@21500000 {
> > +      compatible = "sprd,sc9863a-ap-clk";
> > +      reg = <0 0x21500000 0 0x1000>;
> > +      clocks = <&ext_26m>, <&ext_32k>;
> > +      clock-names = "ext-26m", "ext-32k";
> > +      #clock-cells = <1>;
> > +    };
> > +
> > +  - |
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      ap_ahb_regs: syscon@20e00000 {
> > +        compatible = "sprd,sc9863a-glbregs", "syscon", "simple-mfd";
> > +        reg = <0 0x20e00000 0 0x4000>;
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        ranges = <0 0 0x20e00000 0x4000>;
> > +
> > +        apahb_gate: apahb-gate@0 {
> > +          compatible = "sprd,sc9863a-apahb-gate";
> > +          reg = <0x0 0x1020>;
> > +          #clock-cells = <1>;
>
> Doesn't this block have input clocks?

Since it switched to the new way of referencing parent, some clocks
whose parents are all in the same driver don't need to get their
parent from DT.

Thanks,
Chunyan