| Message ID | 20191216121932.22967-4-zhang.lyra@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add clocks for Unisoc's SC9863A | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Mon, Dec 16, 2019 at 08:19:29PM +0800, Chunyan Zhang wrote: > From: Chunyan Zhang <chunyan.zhang@unisoc.com> > > add a new bindings to describe sc9863a clock compatible string. > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com> > --- > .../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... > + > + 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. > + > + 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. > + > +required: > + - compatible > + - '#clock-cells' > + > +examples: > + - | > + ap_clk: clock-controller@21500000 { > + compatible = "sprd,sc9863a-ap-clk"; > + reg = <0 0x21500000 0 0x1000>; > + clocks = <&ext_32k>, <&ext_26m>; > + clock-names = "ext-32k", "ext-26m"; > + #clock-cells = <1>; > + }; > + > + - | > + apahb_gate: apahb-gate { > + compatible = "sprd,sc9863a-apahb-gate"; > + sprd,syscon = <&ap_ahb_regs>; > + #clock-cells = <1>; > + }; > + > +... > -- > 2.20.1 >
On Fri, 27 Dec 2019 at 02:56, Rob Herring <robh@kernel.org> wrote: > > On Mon, Dec 16, 2019 at 08:19:29PM +0800, Chunyan Zhang wrote: > > From: Chunyan Zhang <chunyan.zhang@unisoc.com> > > > > add a new bindings to describe sc9863a clock compatible string. > > > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com> > > --- > > .../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... > > > + > > + 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. Thanks, Chunyan > > > + > > +required: > > + - compatible > > + - '#clock-cells' > > + > > +examples: > > + - | > > + ap_clk: clock-controller@21500000 { > > + compatible = "sprd,sc9863a-ap-clk"; > > + reg = <0 0x21500000 0 0x1000>; > > + clocks = <&ext_32k>, <&ext_26m>; > > + clock-names = "ext-32k", "ext-26m"; > > + #clock-cells = <1>; > > + }; > > + > > + - | > > + apahb_gate: apahb-gate { > > + compatible = "sprd,sc9863a-apahb-gate"; > > + sprd,syscon = <&ap_ahb_regs>; > > + #clock-cells = <1>; > > + }; > > + > > +... > > -- > > 2.20.1 > >
On Mon, Dec 30, 2019 at 8:07 PM Chunyan Zhang <zhang.lyra@gmail.com> wrote: > > On Fri, 27 Dec 2019 at 02:56, Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Dec 16, 2019 at 08:19:29PM +0800, Chunyan Zhang wrote: > > > From: Chunyan Zhang <chunyan.zhang@unisoc.com> > > > > > > add a new bindings to describe sc9863a clock compatible string. > > > > > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com> > > > --- > > > .../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... > > > > > + > > > + 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
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 + + clocks: + description: | + The input parent clock(s) phandle for this clock, only list fixed + clocks which are decleared in devicetree. + + clock-names: + description: | + Clock name strings used for driver to reference. + + 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. + +required: + - compatible + - '#clock-cells' + +examples: + - | + ap_clk: clock-controller@21500000 { + compatible = "sprd,sc9863a-ap-clk"; + reg = <0 0x21500000 0 0x1000>; + clocks = <&ext_32k>, <&ext_26m>; + clock-names = "ext-32k", "ext-26m"; + #clock-cells = <1>; + }; + + - | + apahb_gate: apahb-gate { + compatible = "sprd,sc9863a-apahb-gate"; + sprd,syscon = <&ap_ahb_regs>; + #clock-cells = <1>; + }; + +...