* Setting CPU clock frequency on early boot @ 2017-09-05 15:37 Alexey Brodkin 2017-09-05 22:03 ` Rob Herring 0 siblings, 1 reply; 7+ messages in thread From: Alexey Brodkin @ 2017-09-05 15:37 UTC (permalink / raw) To: linux-clk@vger.kernel.org Cc: linux-arch@vger.kernel.org, mturquette@baylibre.com, sboyd@codeaurora.org, linux-snps-arc@lists.infradead.org, devicetree@vger.kernel.org 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== ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting CPU clock frequency on early boot 2017-09-05 15:37 Setting CPU clock frequency on early boot Alexey Brodkin @ 2017-09-05 22:03 ` Rob Herring 2017-09-05 23:40 ` Vineet Gupta 0 siblings, 1 reply; 7+ messages in thread From: Rob Herring @ 2017-09-05 22:03 UTC (permalink / raw) To: Alexey Brodkin Cc: linux-clk@vger.kernel.org, linux-arch@vger.kernel.org, mturquette@baylibre.com, sboyd@codeaurora.org, linux-snps-arc@lists.infradead.org, devicetree@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting CPU clock frequency on early boot 2017-09-05 22:03 ` Rob Herring @ 2017-09-05 23:40 ` Vineet Gupta 2017-09-06 13:51 ` Alexey Brodkin 0 siblings, 1 reply; 7+ messages in thread From: Vineet Gupta @ 2017-09-05 23:40 UTC (permalink / raw) To: Rob Herring, Alexey Brodkin Cc: linux-arch@vger.kernel.org, devicetree@vger.kernel.org, mturquette@baylibre.com, sboyd@codeaurora.org, linux-snps-arc@lists.infradead.org, linux-clk@vger.kernel.org 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting CPU clock frequency on early boot 2017-09-05 23:40 ` Vineet Gupta @ 2017-09-06 13:51 ` Alexey Brodkin 2017-09-06 15:25 ` Rob Herring 0 siblings, 1 reply; 7+ messages in thread From: Alexey Brodkin @ 2017-09-06 13:51 UTC (permalink / raw) To: robh@kernel.org, Vineet Gupta Cc: linux-clk@vger.kernel.org, linux-arch@vger.kernel.org, mturquette@baylibre.com, sboyd@codeaurora.org, devicetree@vger.kernel.org, linux-snps-arc@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting CPU clock frequency on early boot 2017-09-06 13:51 ` Alexey Brodkin @ 2017-09-06 15:25 ` Rob Herring 2017-09-06 16:22 ` Alexey Brodkin 0 siblings, 1 reply; 7+ messages in thread From: Rob Herring @ 2017-09-06 15:25 UTC (permalink / raw) To: Alexey Brodkin Cc: Vineet Gupta, linux-clk@vger.kernel.org, linux-arch@vger.kernel.org, mturquette@baylibre.com, sboyd@codeaurora.org, devicetree@vger.kernel.org, linux-snps-arc@lists.infradead.org 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting CPU clock frequency on early boot 2017-09-06 15:25 ` Rob Herring @ 2017-09-06 16:22 ` Alexey Brodkin 2017-09-06 19:20 ` Rob Herring 0 siblings, 1 reply; 7+ messages in thread From: Alexey Brodkin @ 2017-09-06 16:22 UTC (permalink / raw) To: robh@kernel.org Cc: linux-snps-arc@lists.infradead.org, Vineet.Gupta1@synopsys.com, linux-arch@vger.kernel.org, mturquette@baylibre.com, sboyd@codeaurora.org, linux-clk@vger.kernel.org, devicetree@vger.kernel.org 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== ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting CPU clock frequency on early boot 2017-09-06 16:22 ` Alexey Brodkin @ 2017-09-06 19:20 ` Rob Herring 0 siblings, 0 replies; 7+ messages in thread From: Rob Herring @ 2017-09-06 19:20 UTC (permalink / raw) To: Alexey Brodkin Cc: linux-snps-arc@lists.infradead.org, Vineet.Gupta1@synopsys.com, linux-arch@vger.kernel.org, mturquette@baylibre.com, sboyd@codeaurora.org, linux-clk@vger.kernel.org, devicetree@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-06 19:20 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-05 15:37 Setting CPU clock frequency on early boot Alexey Brodkin 2017-09-05 22:03 ` Rob Herring 2017-09-05 23:40 ` Vineet Gupta 2017-09-06 13:51 ` Alexey Brodkin 2017-09-06 15:25 ` Rob Herring 2017-09-06 16:22 ` Alexey Brodkin 2017-09-06 19:20 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox