Hi Rob,
On Wed, 2017-09-06 at 10:25 -0500, Rob Herring wrote:
> On Wed, Sep 6, 2017 at 8:51 AM, Alexey Brodkin
> <Alexey.Brodkin@xxxxxxxxxxxx> wrote:
> >
> > Hi Vineet, Rob,
> >
> > On Tue, 2017-09-05 at 16:40 -0700, Vineet Gupta wrote:
> > >
> > > On 09/05/2017 03:04 PM, Rob Herring wrote:
> > > >
> > > >
> > > > On Tue, Sep 5, 2017 at 10:37 AM, Alexey Brodkin
> > > > <Alexey.Brodkin@xxxxxxxxxxxx> wrote:
[snip]
> > Yeah, that's an interesting question. We may indeed move more smarts to the clock driver
> > but:
> > 1. We'll have duplicate code in different clock drivers. Even today that kind of clock
> > setup is applicable to AXS10x and HSDK platforms (and they use different clock drivers).
>
> No, you could provide a common, shared function to call. Then each
> platform can opt-in. If you can make something that applies to every
> single platform now or in the future, then I'd put it in arch. If you
> have plans to decouple the timer and cpu clocks, then sounds like you
> can't.
Right so we'll implement a function which is called by a platform if required.
That way we escape copy-pasting while keeping enough flexibility for current
and future platforms.
> IMO, if it's not part of the defined CPU architecture, then don't put
> it in arch/. That's how we end up with multiple copies of the same
> thing done arbitrarily different ways because few people look across
> architectures.
So do you propose to have the function [that reads "clock-frequency" from say
CPU node and passes its value to CPU's parent clock driver] in generic
[i.e. architecture agnostic] code somewhere in "init/main.c"?
> >
> > 2. Print out of CPU frequency which is used during boot process for us is important as well
> > especially during bring-up of new HW.
> >
> > 3. If there's no dedicated "clock-frequency" parameter in CPU node we won't
> > change anything so that non-affected platforms will live as they used to.
> >
> > That said IMHO proposed implementation is what we want to kep for now.
> >
> > >
> > > Also note that this code is using a new / adhoc DT binding cpu-freq in cou node to
> > > do the override - is that acceptable ?
>
> No, I meant to point that out.
Sorry, but for me it's not clear what did you mean here.
Care to elaborate a bit more?
> > I think we'll switch to more common "clock-frequency" in the next respin.
> > Indeed "cpu-freq" might be a bit misleading.
>
> Ideally, you'd use the clock binding eventually.
Again I'm probably missing something :)
I meant we will have both clock phandle and "clock-frequency" at the same time.
Something like this:
-------------------------------->8---------------------------
cpus {
#address-cells = <1>;
#size-cells = <0>;
cpu@0 {
device_type = "cpu";
compatible = "snps,archs38";
reg = <0>;
clocks = <&core_clk>;
clock-frequency = <100000000>; <-- That's where we want to set desired value
};
...
}
core_clk: core-clk@80 {
compatible = "snps,axs10x-arc-pll-clock";
reg = <0x80 0x10>, <0x100 0x10>;
#clock-cells = <0>;
clocks = <&input_clk>;
};
-------------------------------->8---------------------------
Or alternatively we may move "clock-frequency" right to the clock's node and have
it like that:
-------------------------------->8---------------------------
core_clk: core-clk@80 {
compatible = "snps,axs10x-arc-pll-clock";
reg = <0x80 0x10>, <0x100 0x10>;
#clock-cells = <0>;
clocks = <&input_clk>;
clock-frequency = <100000000>; <-- That's where we want to set desired value
};
-------------------------------->8---------------------------
-Alexey
