Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210218070709.11932-1-sergio.paracuellos@gmail.com>
 <20210218070709.11932-3-sergio.paracuellos@gmail.com> <20210305224756.GA777984@robh.at.kernel.org>
 <CAMhs-H_RoA-JvT9Q1K+8tEA1vqS6HWuE-D4=kWVsoOWTwjTGbw@mail.gmail.com>
In-Reply-To: <CAMhs-H_RoA-JvT9Q1K+8tEA1vqS6HWuE-D4=kWVsoOWTwjTGbw@mail.gmail.com>
From:   Sergio Paracuellos <sergio.paracuellos@gmail.com>
Date:   Sat, 6 Mar 2021 10:54:11 +0100
Message-ID: <CAMhs-H9noK84G_PgLdL1kTS9YuEa=bKojrOojYTBtOeKy+L7RA@mail.gmail.com>
Subject: Re: [PATCH v9 2/6] dt: bindings: add mt7621-clk device tree binding documentation
To:     Rob Herring <robh@kernel.org>
Cc:     Stephen Boyd <sboyd@kernel.org>, John Crispin <john@phrozen.org>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Greg KH <gregkh@linuxfoundation.org>,
        "open list:COMMON CLK FRAMEWORK" <linux-clk@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        "open list:MIPS" <linux-mips@vger.kernel.org>,
        "open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>,
        NeilBrown <neil@brown.name>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi again,

On Sat, Mar 6, 2021 at 8:12 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi Rob,
>
> On Fri, Mar 5, 2021 at 11:47 PM Rob Herring <robh@kernel.org> wrote:
> [snip]
> > > +
> > > +  ralink,sysctl:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description:
> > > +      phandle of syscon used to control system registers
> > > +
> > > +  ralink,memctl:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description:
> > > +      phandle of syscon used to control memory registers
> >
> > I assume one of these phandles are the main registers for the clocks?
> > Make this a child node and drop that phandle.
>
> The 'ralink,sysctl' phandle is to read bootstrap register to be able
> to derive xtal and a clk gate register for the peripherals.
> The 'ralink,memctl' phandle is to read the cpu clock frequency from
> the memory controller.
>
> So there is not "main registers". I already put this as a child node
> in v4 and I was told to get rid of child nodes. I need this as a
> regmap to other DT node registers (sysctl, and memctl) to be able to
> use the driver without specific architecture operations and properly
> enable for COMPILE_TEST without dirty Makefile arch flags. Both sysctl
> and memctl has no other child nodes, and I think that's why I was told
> to avoid child nodes at the end. I explained here [0] current sysctl
> and memctl in the mt7621 device tree and my view of the need for this
> two syscons:
>
> [0]: https://lkml.org/lkml/2021/1/2/9
>
> So to avoid to send again "a previous version" on this patch, please
> guide me in the correct thing to do. Stephen, Rob, I will be really
> happy with your help :)

Since there are no other child nodes for this sysc, should merge clock
properties
with this node in the following way a valid approach:

 sysc: sysc@0 {
     compatible = "mediatek,mt7621-sysc", "syscon";
     reg = <0x0 0x100>;
     #clock-cells = <1>;
     ralink,memctl = <&memc>;
     clock-output-names = "xtal", "cpu", "bus",
                                        "50m", "125m", "150m",
                                        "250m", "270m";
};

Consumer clock:

node: node@0 {
  ...
  clocks = <&sysc MT7621_CLK_WHATEVER>;
 ...
};

If that is the case... and since 'sysc' is used as system control
registers for all the rest of the world, where should be the yaml file
with bindings placed?

Thanks in advance again for your help.

Best regards,
    Sergio Paracuellos

>
> Best regards,
>     Sergio Paracuellos
> >
> > > +
> > > +  clock-output-names:
> > > +    maxItems: 8
> > > +
> > > +required:
> > > +  - compatible
> > > +  - '#clock-cells'
> > > +  - ralink,sysctl
> > > +  - ralink,memctl
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/mt7621-clk.h>
> > > +
> > > +    pll {
> > > +      compatible = "mediatek,mt7621-clk";
> > > +      #clock-cells = <1>;
> > > +      ralink,sysctl = <&sysc>;
> > > +      ralink,memctl = <&memc>;
> > > +      clock-output-names = "xtal", "cpu", "bus",
> > > +                           "50m", "125m", "150m",
> > > +                           "250m", "270m";
> > > +    };
> > > --
> > > 2.25.1
> > >