Setting CPU clock frequency on early boot

Setting CPU clock frequency on early boot

[Alexey Brodkin]

Hello,

I'd like to get some feedback on our idea as well as check
if somebody faces similar situations and if so what would be the best
way to implement some generic solution that suits everyone.

So that's our problem:
1. On power-on hardware might start clocking CPU with either
   too high frequency (such that CPU may get stuck at some point)
   or too low frequency.

   That all sounds stupid but let me elaborate a bit here.
   I'm talking about FPGA-based devboards firmware for which
   (here I mean just image loaded in FPGA with CPU implementation
   but not some software yet) might not be stable or be even experimental.

   For example we may deal with dual-core or quad-core designs.
   Former might be OK running @100MHz and latter is only usable
   @75MHz and lower. The simplest solution might be to use some safe
   value before something like CPUfreq kicks in. But we don't yet have
   CPUfreq for ARC (we do plan to get it working sometime soon) which
   means simple change of CPU frequency once time-keeping infrastructure
   was brought-up is not an option... I.e. we'll end up with the system running
   much slower compared what could have been possible.

2. Up until now we used to do dirty hacks in early platform init code.
   Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
    1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
       boot stage still so no expanded DevTree yet exists).
    2) Check how many cores we have and which freq is usable
    3) Update PLL settings right in place if new freq != existing in PLL.

   Even though it is proven to work but with more platforms in the pipeline
   we'll need to copy-paste pretty much the same stuff across all affected
   plats. Which is not nice.

   Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
   Thus acting on PLL registers right in place was the only thing we were able
   to do. Now with introduction of normal clk driver
   (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
   it and have a cleaner and more universal solution to the problem.

   That's how it could be done - http://patchwork.ozlabs.org/patch/801240/
   Basically in architecture's time_init() we check if there's explicitly
   specified "clock-frequency" parameter in cpu's node in Device Tree and
   if there's one we set it via just instantiated clk driver.

We may indeed proceed with mentioned solution for ARC but if that makes
sense for somebody else it might worth getting something similar in generic
init code. Any thoughts?

All comments and suggestions are more than welcome.

-Alexey

Setting CPU clock frequency on early boot

[Alexey Brodkin]

SGVsbG8sDQoNCkknZCBsaWtlIHRvIGdldCBzb21lIGZlZWRiYWNrIG9uIG91ciBpZGVhIGFzIHdl
bGwgYXMgY2hlY2sNCmlmIHNvbWVib2R5IGZhY2VzIHNpbWlsYXIgc2l0dWF0aW9ucyBhbmQgaWYg
c28gd2hhdCB3b3VsZCBiZSB0aGUgYmVzdA0Kd2F5IHRvIGltcGxlbWVudCBzb21lIGdlbmVyaWMg
c29sdXRpb24gdGhhdCBzdWl0cyBldmVyeW9uZS4NCg0KU28gdGhhdCdzIG91ciBwcm9ibGVtOg0K
MS4gT24gcG93ZXItb24gaGFyZHdhcmUgbWlnaHQgc3RhcnQgY2xvY2tpbmcgQ1BVIHdpdGggZWl0
aGVyDQrCoCDCoHRvbyBoaWdoIGZyZXF1ZW5jeSAoc3VjaCB0aGF0IENQVSBtYXkgZ2V0IHN0dWNr
IGF0IHNvbWUgcG9pbnQpDQrCoCDCoG9yIHRvbyBsb3cgZnJlcXVlbmN5Lg0KDQrCoCDCoFRoYXQg
YWxsIHNvdW5kcyBzdHVwaWQgYnV0IGxldCBtZSBlbGFib3JhdGUgYSBiaXQgaGVyZS4NCsKgIMKg
SSdtIHRhbGtpbmcgYWJvdXQgRlBHQS1iYXNlZCBkZXZib2FyZHMgZmlybXdhcmUgZm9yIHdoaWNo
DQrCoCDCoChoZXJlIEkgbWVhbiBqdXN0IGltYWdlIGxvYWRlZCBpbiBGUEdBIHdpdGggQ1BVIGlt
cGxlbWVudGF0aW9uDQrCoCDCoGJ1dCBub3Qgc29tZSBzb2Z0d2FyZSB5ZXQpIG1pZ2h0IG5vdCBi
ZSBzdGFibGUgb3IgYmUgZXZlbiBleHBlcmltZW50YWwuDQoNCsKgIMKgRm9yIGV4YW1wbGUgd2Ug
bWF5IGRlYWwgd2l0aCBkdWFsLWNvcmUgb3IgcXVhZC1jb3JlIGRlc2lnbnMuDQrCoCDCoEZvcm1l
ciBtaWdodCBiZSBPSyBydW5uaW5nIEAxMDBNSHogYW5kIGxhdHRlciBpcyBvbmx5IHVzYWJsZQ0K
wqAgwqBANzVNSHogYW5kIGxvd2VyLiBUaGUgc2ltcGxlc3Qgc29sdXRpb24gbWlnaHQgYmUgdG8g
dXNlIHNvbWUgc2FmZQ0KwqAgwqB2YWx1ZSBiZWZvcmUgc29tZXRoaW5nIGxpa2UgQ1BVZnJlcSBr
aWNrcyBpbi4gQnV0IHdlIGRvbid0IHlldCBoYXZlDQrCoCDCoENQVWZyZXEgZm9yIEFSQyAod2Ug
ZG8gcGxhbiB0byBnZXQgaXQgd29ya2luZyBzb21ldGltZSBzb29uKSB3aGljaA0KwqAgwqBtZWFu
cyBzaW1wbGUgY2hhbmdlIG9mIENQVSBmcmVxdWVuY3kgb25jZSB0aW1lLWtlZXBpbmcgaW5mcmFz
dHJ1Y3R1cmUNCsKgIMKgd2FzIGJyb3VnaHQtdXAgaXMgbm90IGFuIG9wdGlvbi4uLiBJLmUuIHdl
J2xsIGVuZCB1cCB3aXRoIHRoZSBzeXN0ZW0gcnVubmluZw0KwqAgwqBtdWNoIHNsb3dlciBjb21w
YXJlZCB3aGF0IGNvdWxkIGhhdmUgYmVlbiBwb3NzaWJsZS4NCg0KMi4gVXAgdW50aWwgbm93IHdl
IHVzZWQgdG8gZG8gZGlydHkgaGFja3MgaW4gZWFybHkgcGxhdGZvcm0gaW5pdCBjb2RlLg0KwqAg
wqBOYW1lbHkgKHNlZcKgYXhzMTAzX2Vhcmx5X2luaXQoKSBpbsKgYXJjaC9hcmMvcGxhdC1heHMx
MHgvYXhzMTB4LmMpOg0KwqAgwqAgMSkgUmVhZCBDUFUncyAiY2xvY2stZnJlcXVlbmN5IiBmcm9t
IC5kdGIgKHJlbWVtYmVyIHdlJ3JlIG9uIHZlcnkgZWFybHkNCsKgIMKgIMKgIMKgYm9vdCBzdGFn
ZSBzdGlsbCBzbyBubyBleHBhbmRlZCBEZXZUcmVlIHlldCBleGlzdHMpLg0KwqAgwqAgMikgQ2hl
Y2sgaG93IG1hbnkgY29yZXMgd2UgaGF2ZSBhbmQgd2hpY2ggZnJlcSBpcyB1c2FibGUNCsKgIMKg
IDMpIFVwZGF0ZSBQTEwgc2V0dGluZ3MgcmlnaHQgaW4gcGxhY2UgaWYgbmV3IGZyZXEgIT0gZXhp
c3RpbmcgaW4gUExMLg0KDQrCoCDCoEV2ZW4gdGhvdWdoIGl0IGlzIHByb3ZlbiB0byB3b3JrIGJ1
dCB3aXRoIG1vcmUgcGxhdGZvcm1zIGluIHRoZSBwaXBlbGluZQ0KwqAgwqB3ZSdsbCBuZWVkIHRv
IGNvcHktcGFzdGUgcHJldHR5IG11Y2ggdGhlIHNhbWUgc3R1ZmYgYWNyb3NzIGFsbCBhZmZlY3Rl
ZA0KwqAgwqBwbGF0cy4gV2hpY2ggaXMgbm90IG5pY2UuDQoNCsKgIMKgTW9yZW92ZXIgYmFjayBp
biB0aGUgZGF5IHdlIGRpZG4ndCBoYXZlIGEgcHJvcGVyIGNsayBkcml2ZXIgZm9yIENQVSdzIFBM
TC4NCsKgIMKgVGh1cyBhY3Rpbmcgb24gUExMIHJlZ2lzdGVycyByaWdodCBpbiBwbGFjZSB3YXMg
dGhlIG9ubHkgdGhpbmcgd2Ugd2VyZSBhYmxlDQrCoCDCoHRvIGRvLiBOb3cgd2l0aCBpbnRyb2R1
Y3Rpb24gb2Ygbm9ybWFsIGNsayBkcml2ZXINCsKgIMKgKHNlZcKgZHJpdmVycy9jbGsvYXhzMTB4
L3BsbF9jbG9jay5jIGluIGxpbnV4LW5leHQpIHdlJ2QgbGlrZSB0byB1dGlsaXplDQrCoCDCoGl0
IGFuZCBoYXZlIGEgY2xlYW5lciBhbmQgbW9yZSB1bml2ZXJzYWwgc29sdXRpb24gdG8gdGhlIHBy
b2JsZW0uDQoNCsKgIMKgVGhhdCdzIGhvdyBpdCBjb3VsZCBiZSBkb25lIC3CoGh0dHA6Ly9wYXRj
aHdvcmsub3psYWJzLm9yZy9wYXRjaC84MDEyNDAvDQrCoCDCoEJhc2ljYWxseSBpbiBhcmNoaXRl
Y3R1cmUncyB0aW1lX2luaXQoKSB3ZSBjaGVjayBpZiB0aGVyZSdzIGV4cGxpY2l0bHkNCsKgIMKg
c3BlY2lmaWVkICJjbG9jay1mcmVxdWVuY3kiIHBhcmFtZXRlciBpbiBjcHUncyBub2RlIGluIERl
dmljZSBUcmVlIGFuZA0KwqAgwqBpZiB0aGVyZSdzIG9uZSB3ZSBzZXQgaXQgdmlhIGp1c3QgaW5z
dGFudGlhdGVkIGNsayBkcml2ZXIuDQoNCldlIG1heSBpbmRlZWQgcHJvY2VlZCB3aXRoIG1lbnRp
b25lZCBzb2x1dGlvbiBmb3IgQVJDIGJ1dCBpZiB0aGF0IG1ha2VzDQpzZW5zZSBmb3Igc29tZWJv
ZHkgZWxzZSBpdCBtaWdodCB3b3J0aCBnZXR0aW5nIHNvbWV0aGluZyBzaW1pbGFyIGluIGdlbmVy
aWMNCmluaXQgY29kZS4gQW55IHRob3VnaHRzPw0KDQpBbGwgY29tbWVudHMgYW5kIHN1Z2dlc3Rp
b25zIGFyZSBtb3JlIHRoYW4gd2VsY29tZS4NCg0KLUFsZXhleQ==

Setting CPU clock frequency on early boot

[Alexey Brodkin]

Hello,

I'd like to get some feedback on our idea as well as check
if somebody faces similar situations and if so what would be the best
way to implement some generic solution that suits everyone.

So that's our problem:
1. On power-on hardware might start clocking CPU with either
? ?too high frequency (such that CPU may get stuck at some point)
? ?or too low frequency.

? ?That all sounds stupid but let me elaborate a bit here.
? ?I'm talking about FPGA-based devboards firmware for which
? ?(here I mean just image loaded in FPGA with CPU implementation
? ?but not some software yet) might not be stable or be even experimental.

? ?For example we may deal with dual-core or quad-core designs.
? ?Former might be OK running @100MHz and latter is only usable
? ?@75MHz and lower. The simplest solution might be to use some safe
? ?value before something like CPUfreq kicks in. But we don't yet have
? ?CPUfreq for ARC (we do plan to get it working sometime soon) which
? ?means simple change of CPU frequency once time-keeping infrastructure
? ?was brought-up is not an option... I.e. we'll end up with the system running
? ?much slower compared what could have been possible.

2. Up until now we used to do dirty hacks in early platform init code.
? ?Namely (see?axs103_early_init() in?arch/arc/plat-axs10x/axs10x.c):
? ? 1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
? ? ? ?boot stage still so no expanded DevTree yet exists).
? ? 2) Check how many cores we have and which freq is usable
? ? 3) Update PLL settings right in place if new freq != existing in PLL.

? ?Even though it is proven to work but with more platforms in the pipeline
? ?we'll need to copy-paste pretty much the same stuff across all affected
? ?plats. Which is not nice.

? ?Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
? ?Thus acting on PLL registers right in place was the only thing we were able
? ?to do. Now with introduction of normal clk driver
? ?(see?drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
? ?it and have a cleaner and more universal solution to the problem.

? ?That's how it could be done -?http://patchwork.ozlabs.org/patch/801240/
? ?Basically in architecture's time_init() we check if there's explicitly
? ?specified "clock-frequency" parameter in cpu's node in Device Tree and
? ?if there's one we set it via just instantiated clk driver.

We may indeed proceed with mentioned solution for ARC but if that makes
sense for somebody else it might worth getting something similar in generic
init code. Any thoughts?

All comments and suggestions are more than welcome.

-Alexey

Re: Setting CPU clock frequency on early boot

[Rob Herring]

On Tue, Sep 5, 2017 at 10:37 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hello,
>
> I'd like to get some feedback on our idea as well as check
> if somebody faces similar situations and if so what would be the best
> way to implement some generic solution that suits everyone.
>
> So that's our problem:
> 1. On power-on hardware might start clocking CPU with either
>    too high frequency (such that CPU may get stuck at some point)
>    or too low frequency.
>
>    That all sounds stupid but let me elaborate a bit here.
>    I'm talking about FPGA-based devboards firmware for which
>    (here I mean just image loaded in FPGA with CPU implementation
>    but not some software yet) might not be stable or be even experimental.
>
>    For example we may deal with dual-core or quad-core designs.
>    Former might be OK running @100MHz and latter is only usable
>    @75MHz and lower. The simplest solution might be to use some safe
>    value before something like CPUfreq kicks in. But we don't yet have
>    CPUfreq for ARC (we do plan to get it working sometime soon) which
>    means simple change of CPU frequency once time-keeping infrastructure
>    was brought-up is not an option... I.e. we'll end up with the system running
>    much slower compared what could have been possible.
>
> 2. Up until now we used to do dirty hacks in early platform init code.
>    Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
>     1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
>        boot stage still so no expanded DevTree yet exists).
>     2) Check how many cores we have and which freq is usable
>     3) Update PLL settings right in place if new freq != existing in PLL.
>
>    Even though it is proven to work but with more platforms in the pipeline
>    we'll need to copy-paste pretty much the same stuff across all affected
>    plats. Which is not nice.
>
>    Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
>    Thus acting on PLL registers right in place was the only thing we were able
>    to do. Now with introduction of normal clk driver
>    (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
>    it and have a cleaner and more universal solution to the problem.
>
>    That's how it could be done - http://patchwork.ozlabs.org/patch/801240/
>    Basically in architecture's time_init() we check if there's explicitly
>    specified "clock-frequency" parameter in cpu's node in Device Tree and
>    if there's one we set it via just instantiated clk driver.

The patch looks generally okay. I'd move all the logic to the clock
driver unless perhaps how to set the cpu freq is defined by the
architecture.

> We may indeed proceed with mentioned solution for ARC but if that makes
> sense for somebody else it might worth getting something similar in generic
> init code. Any thoughts?

If any ARM platforms are doing something similar, then it's done in
their clock driver via of_clk_init. Or they just wait for cpufreq to
kick in and expect the bootloader has initialized things to a
reasonable frequency.

Rob

Setting CPU clock frequency on early boot

[Rob Herring]

On Tue, Sep 5, 2017 at 10:37 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hello,
>
> I'd like to get some feedback on our idea as well as check
> if somebody faces similar situations and if so what would be the best
> way to implement some generic solution that suits everyone.
>
> So that's our problem:
> 1. On power-on hardware might start clocking CPU with either
>    too high frequency (such that CPU may get stuck at some point)
>    or too low frequency.
>
>    That all sounds stupid but let me elaborate a bit here.
>    I'm talking about FPGA-based devboards firmware for which
>    (here I mean just image loaded in FPGA with CPU implementation
>    but not some software yet) might not be stable or be even experimental.
>
>    For example we may deal with dual-core or quad-core designs.
>    Former might be OK running @100MHz and latter is only usable
>    @75MHz and lower. The simplest solution might be to use some safe
>    value before something like CPUfreq kicks in. But we don't yet have
>    CPUfreq for ARC (we do plan to get it working sometime soon) which
>    means simple change of CPU frequency once time-keeping infrastructure
>    was brought-up is not an option... I.e. we'll end up with the system running
>    much slower compared what could have been possible.
>
> 2. Up until now we used to do dirty hacks in early platform init code.
>    Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
>     1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
>        boot stage still so no expanded DevTree yet exists).
>     2) Check how many cores we have and which freq is usable
>     3) Update PLL settings right in place if new freq != existing in PLL.
>
>    Even though it is proven to work but with more platforms in the pipeline
>    we'll need to copy-paste pretty much the same stuff across all affected
>    plats. Which is not nice.
>
>    Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
>    Thus acting on PLL registers right in place was the only thing we were able
>    to do. Now with introduction of normal clk driver
>    (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
>    it and have a cleaner and more universal solution to the problem.
>
>    That's how it could be done - http://patchwork.ozlabs.org/patch/801240/
>    Basically in architecture's time_init() we check if there's explicitly
>    specified "clock-frequency" parameter in cpu's node in Device Tree and
>    if there's one we set it via just instantiated clk driver.

The patch looks generally okay. I'd move all the logic to the clock
driver unless perhaps how to set the cpu freq is defined by the
architecture.

> We may indeed proceed with mentioned solution for ARC but if that makes
> sense for somebody else it might worth getting something similar in generic
> init code. Any thoughts?

If any ARM platforms are doing something similar, then it's done in
their clock driver via of_clk_init. Or they just wait for cpufreq to
kick in and expect the bootloader has initialized things to a
reasonable frequency.

Rob

Re: Setting CPU clock frequency on early boot

[Vineet Gupta]

On 09/05/2017 03:04 PM, Rob Herring wrote:
> On Tue, Sep 5, 2017 at 10:37 AM, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
>> Hello,
>>
>> I'd like to get some feedback on our idea as well as check
>> if somebody faces similar situations and if so what would be the best
>> way to implement some generic solution that suits everyone.
>>
>> So that's our problem:
>> 1. On power-on hardware might start clocking CPU with either
>>     too high frequency (such that CPU may get stuck at some point)
>>     or too low frequency.
>>
>>     That all sounds stupid but let me elaborate a bit here.
>>     I'm talking about FPGA-based devboards firmware for which
>>     (here I mean just image loaded in FPGA with CPU implementation
>>     but not some software yet) might not be stable or be even experimental.
>>
>>     For example we may deal with dual-core or quad-core designs.
>>     Former might be OK running @100MHz and latter is only usable
>>     @75MHz and lower. The simplest solution might be to use some safe
>>     value before something like CPUfreq kicks in. But we don't yet have
>>     CPUfreq for ARC (we do plan to get it working sometime soon)

But even if we had cpufreq driver going - I don't think it would be usable for 
doing large freq switches, since in current implementations of SoCs (or fpga), the 
clk/pll etc driving core (and all timers etc) are not fixed like say ARM. And as 
discussed before (and pointed to by tglx), timer subsys can't tolerate (on 
purpose) such large drifts.

>> which
>>     means simple change of CPU frequency once time-keeping infrastructure
>>     was brought-up is not an option... I.e. we'll end up with the system running
>>     much slower compared what could have been possible.
>>
>> 2. Up until now we used to do dirty hacks in early platform init code.
>>     Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
>>      1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
>>         boot stage still so no expanded DevTree yet exists).
>>      2) Check how many cores we have and which freq is usable
>>      3) Update PLL settings right in place if new freq != existing in PLL.
>>
>>     Even though it is proven to work but with more platforms in the pipeline
>>     we'll need to copy-paste pretty much the same stuff across all affected
>>     plats. Which is not nice.
>>
>>     Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
>>     Thus acting on PLL registers right in place was the only thing we were able
>>     to do. Now with introduction of normal clk driver
>>     (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
>>     it and have a cleaner and more universal solution to the problem.
>>
>>     That's how it could be done - https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_801240_&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=c14YS-cH-kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=wuUcceY8Cz5EhVklWLAgj7RzU3rvpanujvQ3qTJK0Gw&s=N5IBjq_eCyOf_GRkZskzqGhczBPTbxLJW_MUfauKvuA&e=
>>     Basically in architecture's time_init() we check if there's explicitly
>>     specified "clock-frequency" parameter in cpu's node in Device Tree and
>>     if there's one we set it via just instantiated clk driver.
> The patch looks generally okay. I'd move all the logic to the clock
> driver unless perhaps how to set the cpu freq is defined by the
> architecture.

But the above patch is clk driver agnostic and it would have to be added each clk 
driver (axs10x, hsdk - both in linux-next) ?
Also note that this code is using a new / adhoc DT binding cpu-freq in cou node to 
do the override - is that acceptable ?

-Vineet

>
>> We may indeed proceed with mentioned solution for ARC but if that makes
>> sense for somebody else it might worth getting something similar in generic
>> init code. Any thoughts?
> If any ARM platforms are doing something similar, then it's done in
> their clock driver via of_clk_init. Or they just wait for cpufreq to
> kick in and expect the bootloader has initialized things to a
> reasonable frequency.

Setting CPU clock frequency on early boot

[Vineet Gupta]

On 09/05/2017 03:04 PM, Rob Herring wrote:
> On Tue, Sep 5, 2017 at 10:37 AM, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
>> Hello,
>>
>> I'd like to get some feedback on our idea as well as check
>> if somebody faces similar situations and if so what would be the best
>> way to implement some generic solution that suits everyone.
>>
>> So that's our problem:
>> 1. On power-on hardware might start clocking CPU with either
>>     too high frequency (such that CPU may get stuck at some point)
>>     or too low frequency.
>>
>>     That all sounds stupid but let me elaborate a bit here.
>>     I'm talking about FPGA-based devboards firmware for which
>>     (here I mean just image loaded in FPGA with CPU implementation
>>     but not some software yet) might not be stable or be even experimental.
>>
>>     For example we may deal with dual-core or quad-core designs.
>>     Former might be OK running @100MHz and latter is only usable
>>     @75MHz and lower. The simplest solution might be to use some safe
>>     value before something like CPUfreq kicks in. But we don't yet have
>>     CPUfreq for ARC (we do plan to get it working sometime soon)

But even if we had cpufreq driver going - I don't think it would be usable for 
doing large freq switches, since in current implementations of SoCs (or fpga), the 
clk/pll etc driving core (and all timers etc) are not fixed like say ARM. And as 
discussed before (and pointed to by tglx), timer subsys can't tolerate (on 
purpose) such large drifts.

>> which
>>     means simple change of CPU frequency once time-keeping infrastructure
>>     was brought-up is not an option... I.e. we'll end up with the system running
>>     much slower compared what could have been possible.
>>
>> 2. Up until now we used to do dirty hacks in early platform init code.
>>     Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
>>      1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
>>         boot stage still so no expanded DevTree yet exists).
>>      2) Check how many cores we have and which freq is usable
>>      3) Update PLL settings right in place if new freq != existing in PLL.
>>
>>     Even though it is proven to work but with more platforms in the pipeline
>>     we'll need to copy-paste pretty much the same stuff across all affected
>>     plats. Which is not nice.
>>
>>     Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
>>     Thus acting on PLL registers right in place was the only thing we were able
>>     to do. Now with introduction of normal clk driver
>>     (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
>>     it and have a cleaner and more universal solution to the problem.
>>
>>     That's how it could be done - https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_801240_&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=c14YS-cH-kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=wuUcceY8Cz5EhVklWLAgj7RzU3rvpanujvQ3qTJK0Gw&s=N5IBjq_eCyOf_GRkZskzqGhczBPTbxLJW_MUfauKvuA&e=
>>     Basically in architecture's time_init() we check if there's explicitly
>>     specified "clock-frequency" parameter in cpu's node in Device Tree and
>>     if there's one we set it via just instantiated clk driver.
> The patch looks generally okay. I'd move all the logic to the clock
> driver unless perhaps how to set the cpu freq is defined by the
> architecture.

But the above patch is clk driver agnostic and it would have to be added each clk 
driver (axs10x, hsdk - both in linux-next) ?
Also note that this code is using a new / adhoc DT binding cpu-freq in cou node to 
do the override - is that acceptable ?

-Vineet

>
>> We may indeed proceed with mentioned solution for ARC but if that makes
>> sense for somebody else it might worth getting something similar in generic
>> init code. Any thoughts?
> If any ARM platforms are doing something similar, then it's done in
> their clock driver via of_clk_init. Or they just wait for cpufreq to
> kick in and expect the bootloader has initialized things to a
> reasonable frequency.

Re: Setting CPU clock frequency on early boot

[Alexey Brodkin]

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@synopsys.com> wrote:
> > > 
> > > Hello,
> > > 
> > > I'd like to get some feedback on our idea as well as check
> > > if somebody faces similar situations and if so what would be the best
> > > way to implement some generic solution that suits everyone.
> > > 
> > > So that's our problem:
> > > 1. On power-on hardware might start clocking CPU with either
> > >     too high frequency (such that CPU may get stuck at some point)
> > >     or too low frequency.
> > > 
> > >     That all sounds stupid but let me elaborate a bit here.
> > >     I'm talking about FPGA-based devboards firmware for which
> > >     (here I mean just image loaded in FPGA with CPU implementation
> > >     but not some software yet) might not be stable or be even experimental.
> > > 
> > >     For example we may deal with dual-core or quad-core designs.
> > >     Former might be OK running @100MHz and latter is only usable
> > >     @75MHz and lower. The simplest solution might be to use some safe
> > >     value before something like CPUfreq kicks in. But we don't yet have
> > >     CPUfreq for ARC (we do plan to get it working sometime soon)
> 
> But even if we had cpufreq driver going - I don't think it would be usable for 
> doing large freq switches, since in current implementations of SoCs (or fpga), the 
> clk/pll etc driving core (and all timers etc) are not fixed like say ARM. And as 
> discussed before (and pointed to by tglx), timer subsys can't tolerate (on 
> purpose) such large drifts.

Essentially cpufreq only makes sense for "compatible" systems and unfortunately
most of our current boards are not capable due to missing constant [decoupled from
CPU frequency] clock source. But looking forward we are planning to improve on that
so that hopefully even our FPGA-based boards will be usable with cpufreq.

> > > which
> > >     means simple change of CPU frequency once time-keeping infrastructure
> > >     was brought-up is not an option... I.e. we'll end up with the system running
> > >     much slower compared what could have been possible.
> > > 
> > > 2. Up until now we used to do dirty hacks in early platform init code.
> > >     Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
> > >      1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
> > >         boot stage still so no expanded DevTree yet exists).
> > >      2) Check how many cores we have and which freq is usable
> > >      3) Update PLL settings right in place if new freq != existing in PLL.
> > > 
> > >     Even though it is proven to work but with more platforms in the pipeline
> > >     we'll need to copy-paste pretty much the same stuff across all affected
> > >     plats. Which is not nice.
> > > 
> > >     Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
> > >     Thus acting on PLL registers right in place was the only thing we were able
> > >     to do. Now with introduction of normal clk driver
> > >     (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
> > >     it and have a cleaner and more universal solution to the problem.
> > > 
> > >     That's how it could be done - https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_801240_&d=DwICAg&c=DPL6_X_6JkXF
> > > x7AXWqB0tg&r=c14YS-cH-
> > > kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=wuUcceY8Cz5EhVklWLAgj7RzU3rvpanujvQ3qTJK0Gw&s=N5IBjq_eCyOf_GRkZskzqGhczBPTbxLJW_MUfauKvuA&e=
> > >     Basically in architecture's time_init() we check if there's explicitly
> > >     specified "clock-frequency" parameter in cpu's node in Device Tree and
> > >     if there's one we set it via just instantiated clk driver.
> > The patch looks generally okay. I'd move all the logic to the clock
> > driver unless perhaps how to set the cpu freq is defined by the
> > architecture.

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).

 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 ?

I think we'll switch to more common "clock-frequency" in the next respin.
Indeed "cpu-freq" might be a bit misleading.

Also FWIW one MIPS platform uses something similar
(even though their property name is not standard but "mips-hpt-frequency"),
see http://elixir.free-electrons.com/linux/latest/source/arch/mips/boot/dts/brcm/bcm3368.dtsi#L10
http://elixir.free-electrons.com/linux/latest/source/arch/mips/bmips/setup.c#L141.

-Alexey

Re: Setting CPU clock frequency on early boot

[Alexey Brodkin]

SGkgVmluZWV0LCBSb2IsDQoNCk9uIFR1ZSwgMjAxNy0wOS0wNSBhdCAxNjo0MCAtMDcwMCwgVmlu
ZWV0IEd1cHRhIHdyb3RlOg0KPiBPbiAwOS8wNS8yMDE3IDAzOjA0IFBNLCBSb2IgSGVycmluZyB3
cm90ZToNCj4gPiANCj4gPiBPbiBUdWUsIFNlcCA1LCAyMDE3IGF0IDEwOjM3IEFNLCBBbGV4ZXkg
QnJvZGtpbg0KPiA+IDxBbGV4ZXkuQnJvZGtpbkBzeW5vcHN5cy5jb20+IHdyb3RlOg0KPiA+ID4g
DQo+ID4gPiBIZWxsbywNCj4gPiA+IA0KPiA+ID4gSSdkIGxpa2UgdG8gZ2V0IHNvbWUgZmVlZGJh
Y2sgb24gb3VyIGlkZWEgYXMgd2VsbCBhcyBjaGVjaw0KPiA+ID4gaWYgc29tZWJvZHkgZmFjZXMg
c2ltaWxhciBzaXR1YXRpb25zIGFuZCBpZiBzbyB3aGF0IHdvdWxkIGJlIHRoZSBiZXN0DQo+ID4g
PiB3YXkgdG8gaW1wbGVtZW50IHNvbWUgZ2VuZXJpYyBzb2x1dGlvbiB0aGF0IHN1aXRzIGV2ZXJ5
b25lLg0KPiA+ID4gDQo+ID4gPiBTbyB0aGF0J3Mgb3VyIHByb2JsZW06DQo+ID4gPiAxLiBPbiBw
b3dlci1vbiBoYXJkd2FyZSBtaWdodCBzdGFydCBjbG9ja2luZyBDUFUgd2l0aCBlaXRoZXINCj4g
PiA+IMKgwqDCoMKgdG9vIGhpZ2ggZnJlcXVlbmN5IChzdWNoIHRoYXQgQ1BVIG1heSBnZXQgc3R1
Y2sgYXQgc29tZSBwb2ludCkNCj4gPiA+IMKgwqDCoMKgb3IgdG9vIGxvdyBmcmVxdWVuY3kuDQo+
ID4gPiANCj4gPiA+IMKgwqDCoMKgVGhhdCBhbGwgc291bmRzIHN0dXBpZCBidXQgbGV0IG1lIGVs
YWJvcmF0ZSBhIGJpdCBoZXJlLg0KPiA+ID4gwqDCoMKgwqBJJ20gdGFsa2luZyBhYm91dCBGUEdB
LWJhc2VkIGRldmJvYXJkcyBmaXJtd2FyZSBmb3Igd2hpY2gNCj4gPiA+IMKgwqDCoMKgKGhlcmUg
SSBtZWFuIGp1c3QgaW1hZ2UgbG9hZGVkIGluIEZQR0Egd2l0aCBDUFUgaW1wbGVtZW50YXRpb24N
Cj4gPiA+IMKgwqDCoMKgYnV0IG5vdCBzb21lIHNvZnR3YXJlIHlldCkgbWlnaHQgbm90IGJlIHN0
YWJsZSBvciBiZSBldmVuIGV4cGVyaW1lbnRhbC4NCj4gPiA+IA0KPiA+ID4gwqDCoMKgwqBGb3Ig
ZXhhbXBsZSB3ZSBtYXkgZGVhbCB3aXRoIGR1YWwtY29yZSBvciBxdWFkLWNvcmUgZGVzaWducy4N
Cj4gPiA+IMKgwqDCoMKgRm9ybWVyIG1pZ2h0IGJlIE9LIHJ1bm5pbmcgQDEwME1IeiBhbmQgbGF0
dGVyIGlzIG9ubHkgdXNhYmxlDQo+ID4gPiDCoMKgwqDCoEA3NU1IeiBhbmQgbG93ZXIuIFRoZSBz
aW1wbGVzdCBzb2x1dGlvbiBtaWdodCBiZSB0byB1c2Ugc29tZSBzYWZlDQo+ID4gPiDCoMKgwqDC
oHZhbHVlIGJlZm9yZSBzb21ldGhpbmcgbGlrZSBDUFVmcmVxIGtpY2tzIGluLiBCdXQgd2UgZG9u
J3QgeWV0IGhhdmUNCj4gPiA+IMKgwqDCoMKgQ1BVZnJlcSBmb3IgQVJDICh3ZSBkbyBwbGFuIHRv
IGdldCBpdCB3b3JraW5nIHNvbWV0aW1lIHNvb24pDQo+IA0KPiBCdXQgZXZlbiBpZiB3ZSBoYWQg
Y3B1ZnJlcSBkcml2ZXIgZ29pbmcgLSBJIGRvbid0IHRoaW5rIGl0IHdvdWxkIGJlIHVzYWJsZSBm
b3LCoA0KPiBkb2luZyBsYXJnZSBmcmVxIHN3aXRjaGVzLCBzaW5jZSBpbiBjdXJyZW50IGltcGxl
bWVudGF0aW9ucyBvZiBTb0NzIChvciBmcGdhKSwgdGhlwqANCj4gY2xrL3BsbCBldGMgZHJpdmlu
ZyBjb3JlIChhbmQgYWxsIHRpbWVycyBldGMpIGFyZSBub3QgZml4ZWQgbGlrZSBzYXkgQVJNLiBB
bmQgYXPCoA0KPiBkaXNjdXNzZWQgYmVmb3JlIChhbmQgcG9pbnRlZCB0byBieSB0Z2x4KSwgdGlt
ZXIgc3Vic3lzIGNhbid0IHRvbGVyYXRlIChvbsKgDQo+IHB1cnBvc2UpIHN1Y2ggbGFyZ2UgZHJp
ZnRzLg0KDQpFc3NlbnRpYWxseSBjcHVmcmVxIG9ubHkgbWFrZXMgc2Vuc2UgZm9yICJjb21wYXRp
YmxlIiBzeXN0ZW1zIGFuZCB1bmZvcnR1bmF0ZWx5DQptb3N0IG9mIG91ciBjdXJyZW50IGJvYXJk
cyBhcmUgbm90IGNhcGFibGUgZHVlIHRvIG1pc3NpbmcgY29uc3RhbnQgW2RlY291cGxlZCBmcm9t
DQpDUFUgZnJlcXVlbmN5XSBjbG9jayBzb3VyY2UuIEJ1dCBsb29raW5nIGZvcndhcmQgd2UgYXJl
IHBsYW5uaW5nIHRvIGltcHJvdmUgb24gdGhhdA0Kc28gdGhhdCBob3BlZnVsbHkgZXZlbiBvdXIg
RlBHQS1iYXNlZCBib2FyZHMgd2lsbCBiZSB1c2FibGUgd2l0aCBjcHVmcmVxLg0KDQo+ID4gPiB3
aGljaA0KPiA+ID4gwqDCoMKgwqBtZWFucyBzaW1wbGUgY2hhbmdlIG9mIENQVSBmcmVxdWVuY3kg
b25jZSB0aW1lLWtlZXBpbmcgaW5mcmFzdHJ1Y3R1cmUNCj4gPiA+IMKgwqDCoMKgd2FzIGJyb3Vn
aHQtdXAgaXMgbm90IGFuIG9wdGlvbi4uLiBJLmUuIHdlJ2xsIGVuZCB1cCB3aXRoIHRoZSBzeXN0
ZW0gcnVubmluZw0KPiA+ID4gwqDCoMKgwqBtdWNoIHNsb3dlciBjb21wYXJlZCB3aGF0IGNvdWxk
IGhhdmUgYmVlbiBwb3NzaWJsZS4NCj4gPiA+IA0KPiA+ID4gMi4gVXAgdW50aWwgbm93IHdlIHVz
ZWQgdG8gZG8gZGlydHkgaGFja3MgaW4gZWFybHkgcGxhdGZvcm0gaW5pdCBjb2RlLg0KPiA+ID4g
wqDCoMKgwqBOYW1lbHkgKHNlZSBheHMxMDNfZWFybHlfaW5pdCgpIGluIGFyY2gvYXJjL3BsYXQt
YXhzMTB4L2F4czEweC5jKToNCj4gPiA+IMKgwqDCoMKgwqAxKSBSZWFkIENQVSdzICJjbG9jay1m
cmVxdWVuY3kiIGZyb20gLmR0YiAocmVtZW1iZXIgd2UncmUgb24gdmVyeSBlYXJseQ0KPiA+ID4g
wqDCoMKgwqDCoMKgwqDCoGJvb3Qgc3RhZ2Ugc3RpbGwgc28gbm8gZXhwYW5kZWQgRGV2VHJlZSB5
ZXQgZXhpc3RzKS4NCj4gPiA+IMKgwqDCoMKgwqAyKSBDaGVjayBob3cgbWFueSBjb3JlcyB3ZSBo
YXZlIGFuZCB3aGljaCBmcmVxIGlzIHVzYWJsZQ0KPiA+ID4gwqDCoMKgwqDCoDMpIFVwZGF0ZSBQ
TEwgc2V0dGluZ3MgcmlnaHQgaW4gcGxhY2UgaWYgbmV3IGZyZXEgIT0gZXhpc3RpbmcgaW4gUExM
Lg0KPiA+ID4gDQo+ID4gPiDCoMKgwqDCoEV2ZW4gdGhvdWdoIGl0IGlzIHByb3ZlbiB0byB3b3Jr
IGJ1dCB3aXRoIG1vcmUgcGxhdGZvcm1zIGluIHRoZSBwaXBlbGluZQ0KPiA+ID4gwqDCoMKgwqB3
ZSdsbCBuZWVkIHRvIGNvcHktcGFzdGUgcHJldHR5IG11Y2ggdGhlIHNhbWUgc3R1ZmYgYWNyb3Nz
IGFsbCBhZmZlY3RlZA0KPiA+ID4gwqDCoMKgwqBwbGF0cy4gV2hpY2ggaXMgbm90IG5pY2UuDQo+
ID4gPiANCj4gPiA+IMKgwqDCoMKgTW9yZW92ZXIgYmFjayBpbiB0aGUgZGF5IHdlIGRpZG4ndCBo
YXZlIGEgcHJvcGVyIGNsayBkcml2ZXIgZm9yIENQVSdzIFBMTC4NCj4gPiA+IMKgwqDCoMKgVGh1
cyBhY3Rpbmcgb24gUExMIHJlZ2lzdGVycyByaWdodCBpbiBwbGFjZSB3YXMgdGhlIG9ubHkgdGhp
bmcgd2Ugd2VyZSBhYmxlDQo+ID4gPiDCoMKgwqDCoHRvIGRvLiBOb3cgd2l0aCBpbnRyb2R1Y3Rp
b24gb2Ygbm9ybWFsIGNsayBkcml2ZXINCj4gPiA+IMKgwqDCoMKgKHNlZSBkcml2ZXJzL2Nsay9h
eHMxMHgvcGxsX2Nsb2NrLmMgaW4gbGludXgtbmV4dCkgd2UnZCBsaWtlIHRvIHV0aWxpemUNCj4g
PiA+IMKgwqDCoMKgaXQgYW5kIGhhdmUgYSBjbGVhbmVyIGFuZCBtb3JlIHVuaXZlcnNhbCBzb2x1
dGlvbiB0byB0aGUgcHJvYmxlbS4NCj4gPiA+IA0KPiA+ID4gwqDCoMKgwqBUaGF0J3MgaG93IGl0
IGNvdWxkIGJlIGRvbmUgLSBodHRwczovL3VybGRlZmVuc2UucHJvb2Zwb2ludC5jb20vdjIvdXJs
P3U9aHR0cC0zQV9fcGF0Y2h3b3JrLm96bGFicy5vcmdfcGF0Y2hfODAxMjQwXyZkPUR3SUNBZyZj
PURQTDZfWF82SmtYRg0KPiA+ID4geDdBWFdxQjB0ZyZyPWMxNFlTLWNILQ0KPiA+ID4ga2RoVE9X
ODlLb3pGaEJ0QkpnczF6WHNjWm9qRVpRMFRIcyZtPXd1VWNjZVk4Q3o1RWhWa2xXTEFnajdSelUz
cnZwYW51anZRM3FUSkswR3cmcz1ONUlCanFfZUN5T2ZfR1JrWnNrenFHaGN6QlBUYnhMSldfTVVm
YXVLdnVBJmU9DQo+ID4gPiDCoMKgwqDCoEJhc2ljYWxseSBpbiBhcmNoaXRlY3R1cmUncyB0aW1l
X2luaXQoKSB3ZSBjaGVjayBpZiB0aGVyZSdzIGV4cGxpY2l0bHkNCj4gPiA+IMKgwqDCoMKgc3Bl
Y2lmaWVkICJjbG9jay1mcmVxdWVuY3kiIHBhcmFtZXRlciBpbiBjcHUncyBub2RlIGluIERldmlj
ZSBUcmVlIGFuZA0KPiA+ID4gwqDCoMKgwqBpZiB0aGVyZSdzIG9uZSB3ZSBzZXQgaXQgdmlhIGp1
c3QgaW5zdGFudGlhdGVkIGNsayBkcml2ZXIuDQo+ID4gVGhlIHBhdGNoIGxvb2tzIGdlbmVyYWxs
eSBva2F5LiBJJ2QgbW92ZSBhbGwgdGhlIGxvZ2ljIHRvIHRoZSBjbG9jaw0KPiA+IGRyaXZlciB1
bmxlc3MgcGVyaGFwcyBob3cgdG8gc2V0IHRoZSBjcHUgZnJlcSBpcyBkZWZpbmVkIGJ5IHRoZQ0K
PiA+IGFyY2hpdGVjdHVyZS4NCg0KWWVhaCwgdGhhdCdzIGFuIGludGVyZXN0aW5nIHF1ZXN0aW9u
LiBXZSBtYXkgaW5kZWVkIG1vdmUgbW9yZSBzbWFydHMgdG8gdGhlIGNsb2NrIGRyaXZlcg0KYnV0
Og0KwqAxLiBXZSdsbCBoYXZlIGR1cGxpY2F0ZSBjb2RlIGluIGRpZmZlcmVudCBjbG9jayBkcml2
ZXJzLiBFdmVuIHRvZGF5IHRoYXQga2luZCBvZiBjbG9jaw0KwqAgwqAgc2V0dXAgaXMgYXBwbGlj
YWJsZSB0byBBWFMxMHggYW5kIEhTREsgcGxhdGZvcm1zIChhbmQgdGhleSB1c2UgZGlmZmVyZW50
IGNsb2NrIGRyaXZlcnMpLg0KDQrCoDIuIFByaW50IG91dCBvZiBDUFUgZnJlcXVlbmN5IHdoaWNo
IGlzIHVzZWQgZHVyaW5nIGJvb3QgcHJvY2VzcyBmb3IgdXMgaXMgaW1wb3J0YW50IGFzIHdlbGwN
CsKgIMKgIGVzcGVjaWFsbHkgZHVyaW5nIGJyaW5nLXVwIG9mIG5ldyBIVy4NCg0KwqAzLiBJZiB0
aGVyZSdzIG5vIGRlZGljYXRlZCAiY2xvY2stZnJlcXVlbmN5IiBwYXJhbWV0ZXIgaW4gQ1BVIG5v
ZGUgd2Ugd29uJ3QNCsKgIMKgIGNoYW5nZSBhbnl0aGluZyBzbyB0aGF0IG5vbi1hZmZlY3RlZCBw
bGF0Zm9ybXMgd2lsbCBsaXZlIGFzIHRoZXkgdXNlZCB0by4NCg0KVGhhdCBzYWlkIElNSE8gcHJv
cG9zZWQgaW1wbGVtZW50YXRpb24gaXMgd2hhdCB3ZSB3YW50IHRvIGtlcCBmb3Igbm93Lg0KDQo+
IEFsc28gbm90ZSB0aGF0IHRoaXMgY29kZSBpcyB1c2luZyBhIG5ldyAvIGFkaG9jIERUIGJpbmRp
bmcgY3B1LWZyZXEgaW4gY291IG5vZGUgdG/CoA0KPiBkbyB0aGUgb3ZlcnJpZGUgLSBpcyB0aGF0
IGFjY2VwdGFibGUgPw0KDQpJIHRoaW5rIHdlJ2xsIHN3aXRjaCB0byBtb3JlIGNvbW1vbiAiY2xv
Y2stZnJlcXVlbmN5IiBpbiB0aGUgbmV4dCByZXNwaW4uDQpJbmRlZWQgImNwdS1mcmVxIiBtaWdo
dCBiZSBhIGJpdCBtaXNsZWFkaW5nLg0KDQpBbHNvIEZXSVcgb25lIE1JUFMgcGxhdGZvcm0gdXNl
cyBzb21ldGhpbmcgc2ltaWxhcg0KKGV2ZW4gdGhvdWdoIHRoZWlyIHByb3BlcnR5IG5hbWUgaXMg
bm90IHN0YW5kYXJkIGJ1dCAibWlwcy1ocHQtZnJlcXVlbmN5IiksDQpzZWXCoGh0dHA6Ly9lbGl4
aXIuZnJlZS1lbGVjdHJvbnMuY29tL2xpbnV4L2xhdGVzdC9zb3VyY2UvYXJjaC9taXBzL2Jvb3Qv
ZHRzL2JyY20vYmNtMzM2OC5kdHNpI0wxMA0KaHR0cDovL2VsaXhpci5mcmVlLWVsZWN0cm9ucy5j
b20vbGludXgvbGF0ZXN0L3NvdXJjZS9hcmNoL21pcHMvYm1pcHMvc2V0dXAuYyNMMTQxLg0KDQot
QWxleGV5

Setting CPU clock frequency on early boot

[Alexey Brodkin]

Hi Vineet, Rob,

On Tue, 2017-09-05@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@synopsys.com> wrote:
> > > 
> > > Hello,
> > > 
> > > I'd like to get some feedback on our idea as well as check
> > > if somebody faces similar situations and if so what would be the best
> > > way to implement some generic solution that suits everyone.
> > > 
> > > So that's our problem:
> > > 1. On power-on hardware might start clocking CPU with either
> > > ????too high frequency (such that CPU may get stuck at some point)
> > > ????or too low frequency.
> > > 
> > > ????That all sounds stupid but let me elaborate a bit here.
> > > ????I'm talking about FPGA-based devboards firmware for which
> > > ????(here I mean just image loaded in FPGA with CPU implementation
> > > ????but not some software yet) might not be stable or be even experimental.
> > > 
> > > ????For example we may deal with dual-core or quad-core designs.
> > > ????Former might be OK running @100MHz and latter is only usable
> > > ????@75MHz and lower. The simplest solution might be to use some safe
> > > ????value before something like CPUfreq kicks in. But we don't yet have
> > > ????CPUfreq for ARC (we do plan to get it working sometime soon)
> 
> But even if we had cpufreq driver going - I don't think it would be usable for?
> doing large freq switches, since in current implementations of SoCs (or fpga), the?
> clk/pll etc driving core (and all timers etc) are not fixed like say ARM. And as?
> discussed before (and pointed to by tglx), timer subsys can't tolerate (on?
> purpose) such large drifts.

Essentially cpufreq only makes sense for "compatible" systems and unfortunately
most of our current boards are not capable due to missing constant [decoupled from
CPU frequency] clock source. But looking forward we are planning to improve on that
so that hopefully even our FPGA-based boards will be usable with cpufreq.

> > > which
> > > ????means simple change of CPU frequency once time-keeping infrastructure
> > > ????was brought-up is not an option... I.e. we'll end up with the system running
> > > ????much slower compared what could have been possible.
> > > 
> > > 2. Up until now we used to do dirty hacks in early platform init code.
> > > ????Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
> > > ?????1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
> > > ????????boot stage still so no expanded DevTree yet exists).
> > > ?????2) Check how many cores we have and which freq is usable
> > > ?????3) Update PLL settings right in place if new freq != existing in PLL.
> > > 
> > > ????Even though it is proven to work but with more platforms in the pipeline
> > > ????we'll need to copy-paste pretty much the same stuff across all affected
> > > ????plats. Which is not nice.
> > > 
> > > ????Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
> > > ????Thus acting on PLL registers right in place was the only thing we were able
> > > ????to do. Now with introduction of normal clk driver
> > > ????(see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
> > > ????it and have a cleaner and more universal solution to the problem.
> > > 
> > > ????That's how it could be done - https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_801240_&d=DwICAg&c=DPL6_X_6JkXF
> > > x7AXWqB0tg&r=c14YS-cH-
> > > kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=wuUcceY8Cz5EhVklWLAgj7RzU3rvpanujvQ3qTJK0Gw&s=N5IBjq_eCyOf_GRkZskzqGhczBPTbxLJW_MUfauKvuA&e=
> > > ????Basically in architecture's time_init() we check if there's explicitly
> > > ????specified "clock-frequency" parameter in cpu's node in Device Tree and
> > > ????if there's one we set it via just instantiated clk driver.
> > The patch looks generally okay. I'd move all the logic to the clock
> > driver unless perhaps how to set the cpu freq is defined by the
> > architecture.

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).

?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 ?

I think we'll switch to more common "clock-frequency" in the next respin.
Indeed "cpu-freq" might be a bit misleading.

Also FWIW one MIPS platform uses something similar
(even though their property name is not standard but "mips-hpt-frequency"),
see?http://elixir.free-electrons.com/linux/latest/source/arch/mips/boot/dts/brcm/bcm3368.dtsi#L10
http://elixir.free-electrons.com/linux/latest/source/arch/mips/bmips/setup.c#L141.

-Alexey

Re: Setting CPU clock frequency on early boot

[Rob Herring]

On Wed, Sep 6, 2017 at 8:51 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> 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@synopsys.com> wrote:
>> > >
>> > > Hello,
>> > >
>> > > I'd like to get some feedback on our idea as well as check
>> > > if somebody faces similar situations and if so what would be the best
>> > > way to implement some generic solution that suits everyone.
>> > >
>> > > So that's our problem:
>> > > 1. On power-on hardware might start clocking CPU with either
>> > >     too high frequency (such that CPU may get stuck at some point)
>> > >     or too low frequency.
>> > >
>> > >     That all sounds stupid but let me elaborate a bit here.
>> > >     I'm talking about FPGA-based devboards firmware for which
>> > >     (here I mean just image loaded in FPGA with CPU implementation
>> > >     but not some software yet) might not be stable or be even experimental.
>> > >
>> > >     For example we may deal with dual-core or quad-core designs.
>> > >     Former might be OK running @100MHz and latter is only usable
>> > >     @75MHz and lower. The simplest solution might be to use some safe
>> > >     value before something like CPUfreq kicks in. But we don't yet have
>> > >     CPUfreq for ARC (we do plan to get it working sometime soon)
>>
>> But even if we had cpufreq driver going - I don't think it would be usable for
>> doing large freq switches, since in current implementations of SoCs (or fpga), the
>> clk/pll etc driving core (and all timers etc) are not fixed like say ARM. And as
>> discussed before (and pointed to by tglx), timer subsys can't tolerate (on
>> purpose) such large drifts.
>
> Essentially cpufreq only makes sense for "compatible" systems and unfortunately
> most of our current boards are not capable due to missing constant [decoupled from
> CPU frequency] clock source. But looking forward we are planning to improve on that
> so that hopefully even our FPGA-based boards will be usable with cpufreq.
>
>> > > which
>> > >     means simple change of CPU frequency once time-keeping infrastructure
>> > >     was brought-up is not an option... I.e. we'll end up with the system running
>> > >     much slower compared what could have been possible.
>> > >
>> > > 2. Up until now we used to do dirty hacks in early platform init code.
>> > >     Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
>> > >      1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
>> > >         boot stage still so no expanded DevTree yet exists).
>> > >      2) Check how many cores we have and which freq is usable
>> > >      3) Update PLL settings right in place if new freq != existing in PLL.
>> > >
>> > >     Even though it is proven to work but with more platforms in the pipeline
>> > >     we'll need to copy-paste pretty much the same stuff across all affected
>> > >     plats. Which is not nice.
>> > >
>> > >     Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
>> > >     Thus acting on PLL registers right in place was the only thing we were able
>> > >     to do. Now with introduction of normal clk driver
>> > >     (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
>> > >     it and have a cleaner and more universal solution to the problem.
>> > >
>> > >     That's how it could be done - https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_801240_&d=DwICAg&c=DPL6_X_6JkXF
>> > > x7AXWqB0tg&r=c14YS-cH-
>> > > kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=wuUcceY8Cz5EhVklWLAgj7RzU3rvpanujvQ3qTJK0Gw&s=N5IBjq_eCyOf_GRkZskzqGhczBPTbxLJW_MUfauKvuA&e=
>> > >     Basically in architecture's time_init() we check if there's explicitly
>> > >     specified "clock-frequency" parameter in cpu's node in Device Tree and
>> > >     if there's one we set it via just instantiated clk driver.
>> > The patch looks generally okay. I'd move all the logic to the clock
>> > driver unless perhaps how to set the cpu freq is defined by the
>> > architecture.
>
> 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.

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.

>  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.

> 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. "clock-frequency" is
still allowed, but not both. You have to pick. Either you have simple
needs or you don't...

>
> Also FWIW one MIPS platform uses something similar
> (even though their property name is not standard but "mips-hpt-frequency"),
> see http://elixir.free-electrons.com/linux/latest/source/arch/mips/boot/dts/brcm/bcm3368.dtsi#L10
> http://elixir.free-electrons.com/linux/latest/source/arch/mips/bmips/setup.c#L141.

That's fine, but you need a good reason.

Rob

Setting CPU clock frequency on early boot

[Rob Herring]

On Wed, Sep 6, 2017 at 8:51 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hi Vineet, Rob,
>
> On Tue, 2017-09-05@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@synopsys.com> wrote:
>> > >
>> > > Hello,
>> > >
>> > > I'd like to get some feedback on our idea as well as check
>> > > if somebody faces similar situations and if so what would be the best
>> > > way to implement some generic solution that suits everyone.
>> > >
>> > > So that's our problem:
>> > > 1. On power-on hardware might start clocking CPU with either
>> > >     too high frequency (such that CPU may get stuck at some point)
>> > >     or too low frequency.
>> > >
>> > >     That all sounds stupid but let me elaborate a bit here.
>> > >     I'm talking about FPGA-based devboards firmware for which
>> > >     (here I mean just image loaded in FPGA with CPU implementation
>> > >     but not some software yet) might not be stable or be even experimental.
>> > >
>> > >     For example we may deal with dual-core or quad-core designs.
>> > >     Former might be OK running @100MHz and latter is only usable
>> > >     @75MHz and lower. The simplest solution might be to use some safe
>> > >     value before something like CPUfreq kicks in. But we don't yet have
>> > >     CPUfreq for ARC (we do plan to get it working sometime soon)
>>
>> But even if we had cpufreq driver going - I don't think it would be usable for
>> doing large freq switches, since in current implementations of SoCs (or fpga), the
>> clk/pll etc driving core (and all timers etc) are not fixed like say ARM. And as
>> discussed before (and pointed to by tglx), timer subsys can't tolerate (on
>> purpose) such large drifts.
>
> Essentially cpufreq only makes sense for "compatible" systems and unfortunately
> most of our current boards are not capable due to missing constant [decoupled from
> CPU frequency] clock source. But looking forward we are planning to improve on that
> so that hopefully even our FPGA-based boards will be usable with cpufreq.
>
>> > > which
>> > >     means simple change of CPU frequency once time-keeping infrastructure
>> > >     was brought-up is not an option... I.e. we'll end up with the system running
>> > >     much slower compared what could have been possible.
>> > >
>> > > 2. Up until now we used to do dirty hacks in early platform init code.
>> > >     Namely (see axs103_early_init() in arch/arc/plat-axs10x/axs10x.c):
>> > >      1) Read CPU's "clock-frequency" from .dtb (remember we're on very early
>> > >         boot stage still so no expanded DevTree yet exists).
>> > >      2) Check how many cores we have and which freq is usable
>> > >      3) Update PLL settings right in place if new freq != existing in PLL.
>> > >
>> > >     Even though it is proven to work but with more platforms in the pipeline
>> > >     we'll need to copy-paste pretty much the same stuff across all affected
>> > >     plats. Which is not nice.
>> > >
>> > >     Moreover back in the day we didn't have a proper clk driver for CPU's PLL.
>> > >     Thus acting on PLL registers right in place was the only thing we were able
>> > >     to do. Now with introduction of normal clk driver
>> > >     (see drivers/clk/axs10x/pll_clock.c in linux-next) we'd like to utilize
>> > >     it and have a cleaner and more universal solution to the problem.
>> > >
>> > >     That's how it could be done - https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_801240_&d=DwICAg&c=DPL6_X_6JkXF
>> > > x7AXWqB0tg&r=c14YS-cH-
>> > > kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=wuUcceY8Cz5EhVklWLAgj7RzU3rvpanujvQ3qTJK0Gw&s=N5IBjq_eCyOf_GRkZskzqGhczBPTbxLJW_MUfauKvuA&e=
>> > >     Basically in architecture's time_init() we check if there's explicitly
>> > >     specified "clock-frequency" parameter in cpu's node in Device Tree and
>> > >     if there's one we set it via just instantiated clk driver.
>> > The patch looks generally okay. I'd move all the logic to the clock
>> > driver unless perhaps how to set the cpu freq is defined by the
>> > architecture.
>
> 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.

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.

>  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.

> 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. "clock-frequency" is
still allowed, but not both. You have to pick. Either you have simple
needs or you don't...

>
> Also FWIW one MIPS platform uses something similar
> (even though their property name is not standard but "mips-hpt-frequency"),
> see http://elixir.free-electrons.com/linux/latest/source/arch/mips/boot/dts/brcm/bcm3368.dtsi#L10
> http://elixir.free-electrons.com/linux/latest/source/arch/mips/bmips/setup.c#L141.

That's fine, but you need a good reason.

Rob

Re: Setting CPU clock frequency on early boot

[Alexey Brodkin]

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@synopsys.com> 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@synopsys.com> 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

Re: Setting CPU clock frequency on early boot

[Alexey Brodkin]

SGkgUm9iLA0KDQpPbiBXZWQsIDIwMTctMDktMDYgYXQgMTA6MjUgLTA1MDAsIFJvYiBIZXJyaW5n
IHdyb3RlOg0KPiBPbiBXZWQsIFNlcCA2LCAyMDE3IGF0IDg6NTEgQU0sIEFsZXhleSBCcm9ka2lu
DQo+IDxBbGV4ZXkuQnJvZGtpbkBzeW5vcHN5cy5jb20+IHdyb3RlOg0KPiA+IA0KPiA+IEhpIFZp
bmVldCwgUm9iLA0KPiA+IA0KPiA+IE9uIFR1ZSwgMjAxNy0wOS0wNSBhdCAxNjo0MCAtMDcwMCwg
VmluZWV0IEd1cHRhIHdyb3RlOg0KPiA+ID4gDQo+ID4gPiBPbiAwOS8wNS8yMDE3IDAzOjA0IFBN
LCBSb2IgSGVycmluZyB3cm90ZToNCj4gPiA+ID4gDQo+ID4gPiA+IA0KPiA+ID4gPiBPbiBUdWUs
IFNlcCA1LCAyMDE3IGF0IDEwOjM3IEFNLCBBbGV4ZXkgQnJvZGtpbg0KPiA+ID4gPiA8QWxleGV5
LkJyb2RraW5Ac3lub3BzeXMuY29tPiB3cm90ZToNCg0KW3NuaXBdDQoNCj4gPiBZZWFoLCB0aGF0
J3MgYW4gaW50ZXJlc3RpbmcgcXVlc3Rpb24uIFdlIG1heSBpbmRlZWQgbW92ZSBtb3JlIHNtYXJ0
cyB0byB0aGUgY2xvY2sgZHJpdmVyDQo+ID4gYnV0Og0KPiA+IMKgMS4gV2UnbGwgaGF2ZSBkdXBs
aWNhdGUgY29kZSBpbiBkaWZmZXJlbnQgY2xvY2sgZHJpdmVycy4gRXZlbiB0b2RheSB0aGF0IGtp
bmQgb2YgY2xvY2sNCj4gPiDCoMKgwqDCoHNldHVwIGlzIGFwcGxpY2FibGUgdG8gQVhTMTB4IGFu
ZCBIU0RLIHBsYXRmb3JtcyAoYW5kIHRoZXkgdXNlIGRpZmZlcmVudCBjbG9jayBkcml2ZXJzKS4N
Cj4gDQo+IE5vLCB5b3UgY291bGQgcHJvdmlkZSBhIGNvbW1vbiwgc2hhcmVkIGZ1bmN0aW9uIHRv
IGNhbGwuIFRoZW4gZWFjaA0KPiBwbGF0Zm9ybSBjYW4gb3B0LWluLiBJZiB5b3UgY2FuIG1ha2Ug
c29tZXRoaW5nIHRoYXQgYXBwbGllcyB0byBldmVyeQ0KPiBzaW5nbGUgcGxhdGZvcm0gbm93IG9y
IGluIHRoZSBmdXR1cmUsIHRoZW4gSSdkIHB1dCBpdCBpbiBhcmNoLiBJZiB5b3UNCj4gaGF2ZSBw
bGFucyB0byBkZWNvdXBsZSB0aGUgdGltZXIgYW5kIGNwdSBjbG9ja3MsIHRoZW4gc291bmRzIGxp
a2UgeW91DQo+IGNhbid0Lg0KDQpSaWdodCBzbyB3ZSdsbCBpbXBsZW1lbnQgYSBmdW5jdGlvbiB3
aGljaCBpcyBjYWxsZWQgYnkgYSBwbGF0Zm9ybSBpZiByZXF1aXJlZC4NClRoYXQgd2F5IHdlIGVz
Y2FwZSBjb3B5LXBhc3Rpbmcgd2hpbGUga2VlcGluZyBlbm91Z2ggZmxleGliaWxpdHkgZm9yIGN1
cnJlbnQNCmFuZCBmdXR1cmUgcGxhdGZvcm1zLg0KDQo+IElNTywgaWYgaXQncyBub3QgcGFydCBv
ZiB0aGUgZGVmaW5lZCBDUFUgYXJjaGl0ZWN0dXJlLCB0aGVuIGRvbid0IHB1dA0KPiBpdCBpbiBh
cmNoLy4gVGhhdCdzIGhvdyB3ZSBlbmQgdXAgd2l0aCBtdWx0aXBsZSBjb3BpZXMgb2YgdGhlIHNh
bWUNCj4gdGhpbmcgZG9uZSBhcmJpdHJhcmlseSBkaWZmZXJlbnQgd2F5cyBiZWNhdXNlIGZldyBw
ZW9wbGUgbG9vayBhY3Jvc3MNCj4gYXJjaGl0ZWN0dXJlcy4NCg0KU28gZG8geW91IHByb3Bvc2Ug
dG8gaGF2ZSB0aGUgZnVuY3Rpb24gW3RoYXQgcmVhZHMgImNsb2NrLWZyZXF1ZW5jeSIgZnJvbSBz
YXkNCkNQVSBub2RlIGFuZCBwYXNzZXMgaXRzIHZhbHVlIHRvIENQVSdzIHBhcmVudCBjbG9jayBk
cml2ZXJdIGluIGdlbmVyaWMNCltpLmUuIGFyY2hpdGVjdHVyZSBhZ25vc3RpY10gY29kZSBzb21l
d2hlcmUgaW4gImluaXQvbWFpbi5jIj8NCg0KPiA+IA0KPiA+IMKgMi4gUHJpbnQgb3V0IG9mIENQ
VSBmcmVxdWVuY3kgd2hpY2ggaXMgdXNlZCBkdXJpbmcgYm9vdCBwcm9jZXNzIGZvciB1cyBpcyBp
bXBvcnRhbnQgYXMgd2VsbA0KPiA+IMKgwqDCoMKgZXNwZWNpYWxseSBkdXJpbmcgYnJpbmctdXAg
b2YgbmV3IEhXLg0KPiA+IA0KPiA+IMKgMy4gSWYgdGhlcmUncyBubyBkZWRpY2F0ZWQgImNsb2Nr
LWZyZXF1ZW5jeSIgcGFyYW1ldGVyIGluIENQVSBub2RlIHdlIHdvbid0DQo+ID4gwqDCoMKgwqBj
aGFuZ2UgYW55dGhpbmcgc28gdGhhdCBub24tYWZmZWN0ZWQgcGxhdGZvcm1zIHdpbGwgbGl2ZSBh
cyB0aGV5IHVzZWQgdG8uDQo+ID4gDQo+ID4gVGhhdCBzYWlkIElNSE8gcHJvcG9zZWQgaW1wbGVt
ZW50YXRpb24gaXMgd2hhdCB3ZSB3YW50IHRvIGtlcCBmb3Igbm93Lg0KPiA+IA0KPiA+ID4gDQo+
ID4gPiBBbHNvIG5vdGUgdGhhdCB0aGlzIGNvZGUgaXMgdXNpbmcgYSBuZXcgLyBhZGhvYyBEVCBi
aW5kaW5nIGNwdS1mcmVxIGluIGNvdSBub2RlIHRvDQo+ID4gPiBkbyB0aGUgb3ZlcnJpZGUgLSBp
cyB0aGF0IGFjY2VwdGFibGUgPw0KPiANCj4gTm8sIEkgbWVhbnQgdG8gcG9pbnQgdGhhdCBvdXQu
DQoNClNvcnJ5LCBidXQgZm9yIG1lIGl0J3Mgbm90IGNsZWFyIHdoYXQgZGlkIHlvdSBtZWFuIGhl
cmUuDQpDYXJlIHRvIGVsYWJvcmF0ZSBhIGJpdCBtb3JlPw0KDQo+ID4gSSB0aGluayB3ZSdsbCBz
d2l0Y2ggdG8gbW9yZSBjb21tb24gImNsb2NrLWZyZXF1ZW5jeSIgaW4gdGhlIG5leHQgcmVzcGlu
Lg0KPiA+IEluZGVlZCAiY3B1LWZyZXEiIG1pZ2h0IGJlIGEgYml0IG1pc2xlYWRpbmcuDQo+IA0K
PiBJZGVhbGx5LCB5b3UnZCB1c2UgdGhlIGNsb2NrIGJpbmRpbmcgZXZlbnR1YWxseS4NCg0KQWdh
aW4gSSdtIHByb2JhYmx5IG1pc3Npbmcgc29tZXRoaW5nIDopDQoNCkkgbWVhbnQgd2Ugd2lsbCBo
YXZlIGJvdGggY2xvY2sgcGhhbmRsZSBhbmQgImNsb2NrLWZyZXF1ZW5jeSIgYXQgdGhlIHNhbWUg
dGltZS4NClNvbWV0aGluZyBsaWtlIHRoaXM6DQotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLT44LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQoJY3B1cyB7DQoJCSNhZGRyZXNzLWNl
bGxzID0gPDE+Ow0KCQkjc2l6ZS1jZWxscyA9IDwwPjsNCg0KCQljcHVAMCB7DQoJCQlkZXZpY2Vf
dHlwZSA9ICJjcHUiOw0KCQkJY29tcGF0aWJsZSA9ICJzbnBzLGFyY2hzMzgiOw0KCQkJcmVnID0g
PDA+Ow0KCQkJY2xvY2tzID0gPCZjb3JlX2Nsaz47DQoJCQljbG9jay1mcmVxdWVuY3kgPSA8MTAw
MDAwMDAwPjsgwqA8LS0gVGhhdCdzIHdoZXJlIHdlIHdhbnQgdG8gc2V0IGRlc2lyZWQgdmFsdWUN
CgkJfTsNCgkuLi4NCgl9DQoNCgljb3JlX2NsazogY29yZS1jbGtAODAgew0KCQljb21wYXRpYmxl
ID0gInNucHMsYXhzMTB4LWFyYy1wbGwtY2xvY2siOw0KCQlyZWcgPSA8MHg4MCAweDEwPiwgPDB4
MTAwIDB4MTA+Ow0KCQkjY2xvY2stY2VsbHMgPSA8MD47DQoJCWNsb2NrcyA9IDwmaW5wdXRfY2xr
PjsNCgl9Ow0KLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0+OC0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLQ0KDQpPciBhbHRlcm5hdGl2ZWx5IHdlIG1heSBtb3ZlICJjbG9jay1mcmVx
dWVuY3kiIHJpZ2h0IHRvIHRoZSBjbG9jaydzIG5vZGUgYW5kIGhhdmUNCml0IGxpa2UgdGhhdDoN
Ci0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tPjgtLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0NCgljb3JlX2NsazrCoGNvcmUtY2xrQDgwwqB7DQoJCWNvbXBhdGlibGUgPSAic25wcyxh
eHMxMHgtYXJjLXBsbC1jbG9jayI7DQoJCXJlZyA9IDwweDgwIDB4MTA+LCA8MHgxMDAgMHgxMD47
DQoJCQ0KI2Nsb2NrLWNlbGxzID0gPDA+Ow0KCQljbG9ja3MgPSA8JmlucHV0X2Nsaz47DQoJCWNs
b2NrLWZyZXF1ZW5jeSA9IDwxMDAwMDAwMDA+OyDCoDwtLSBUaGF0J3Mgd2hlcmUgd2Ugd2FudCB0
byBzZXQgZGVzaXJlZCB2YWx1ZQ0KCQ0KfTsNCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tPjgtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCg0KLUFsZXhleQ==

Re: Setting CPU clock frequency on early boot

[Alexey Brodkin]

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@synopsys.com> 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@synopsys.com> 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

Setting CPU clock frequency on early boot

[Alexey Brodkin]

Hi Rob,

On Wed, 2017-09-06@10:25 -0500, Rob Herring wrote:
> On Wed, Sep 6, 2017 at 8:51 AM, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
> > 
> > Hi Vineet, Rob,
> > 
> > On Tue, 2017-09-05@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@synopsys.com> 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 at 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 at 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 at 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

Re: Setting CPU clock frequency on early boot

[Rob Herring]

On Wed, Sep 6, 2017 at 11:22 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> 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@synopsys.com> 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@synopsys.com> 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"?

No, just like you said above and have the platform's clock driver
initialize frequencies.

I haven't seen anyone else want such a thing as generally the
bootloader should initialize things to something reasonable.

>> >  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 noticed the same thing and meant to highlight that in my first
reply. Use clock-frequency, don't invent something new.

>> > 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 :)

"clock-frequency" property and the clock bindings (i.e. clocks =
<&phandle>) are generally mutually exclusive at least within clock
consumer side. There's the assigned-clocks binding which provides the
ability to set both parent clocks and frequency.

> 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

I think this is how it should be done if you use clock-frequency prop.
This is inline with how fixed-clock binding is done.

Rob

Setting CPU clock frequency on early boot

[Rob Herring]

On Wed, Sep 6, 2017 at 11:22 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:
> Hi Rob,
>
> On Wed, 2017-09-06@10:25 -0500, Rob Herring wrote:
>> On Wed, Sep 6, 2017 at 8:51 AM, Alexey Brodkin
>> <Alexey.Brodkin@synopsys.com> wrote:
>> >
>> > Hi Vineet, Rob,
>> >
>> > On Tue, 2017-09-05@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@synopsys.com> 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"?

No, just like you said above and have the platform's clock driver
initialize frequencies.

I haven't seen anyone else want such a thing as generally the
bootloader should initialize things to something reasonable.

>> >  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 noticed the same thing and meant to highlight that in my first
reply. Use clock-frequency, don't invent something new.

>> > 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 :)

"clock-frequency" property and the clock bindings (i.e. clocks =
<&phandle>) are generally mutually exclusive at least within clock
consumer side. There's the assigned-clocks binding which provides the
ability to set both parent clocks and frequency.

> 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 at 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 at 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 at 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

I think this is how it should be done if you use clock-frequency prop.
This is inline with how fixed-clock binding is done.

Rob