lkml.org 
[lkml]   [2020]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH V2 3/6] dt-bindings: clk: sprd: add bindings for sc9863a clock controller
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

\
 
 \ /
  Last update: 2020-01-02 23:43    [W:0.051 / U:4.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site