On Mon, Dec 30, 2019 at 8:07 PM Chunyan Zhang <zhang.lyra@xxxxxxxxx> wrote: > > On Fri, 27 Dec 2019 at 02:56, Rob Herring <robh@xxxxxxxxxx> wrote: > > > > On Mon, Dec 16, 2019 at 08:19:29PM +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 | 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@xxxxxxxxx> > > > + - Baolin Wang <baolin.wang7@xxxxxxxxx> > > > + - Chunyan Zhang <zhang.lyra@xxxxxxxxx> > > > + > > > +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... > > > > > + > > > + 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. > > > > > > + > > > + clock-names: > > > + description: | > > > + Clock name strings used for driver to reference. > > > > You need to list out the names. > > > > > + > > > + 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
