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��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f