From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933266AbdDEQGV (ORCPT ); Wed, 5 Apr 2017 12:06:21 -0400 Received: from smtprelay.synopsys.com ([198.182.60.111]:42981 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933074AbdDEQGR (ORCPT ); Wed, 5 Apr 2017 12:06:17 -0400 From: Vlad Zakharov To: "sboyd@codeaurora.org" CC: "mark.rutland@arm.com" , "linux-kernel@vger.kernel.org" , "Jose.Abreu@synopsys.com" , "mturquette@baylibre.com" , "devicetree@vger.kernel.org" , "linux-clk@vger.kernel.org" , "linux-snps-arc@lists.infradead.org" Subject: Re: [PATCH v2] clk/axs10x: introduce AXS10X pll driver Thread-Topic: [PATCH v2] clk/axs10x: introduce AXS10X pll driver Thread-Index: AQHSjEP84oiIqPaSbUyCNi/neN7GXaG2H7yAgADzSQA= Date: Wed, 5 Apr 2017 16:06:11 +0000 Message-ID: <1491408370.9650.24.camel@synopsys.com> References: <1487682670-4164-1-git-send-email-vzakhar@synopsys.com> <20170405013525.GJ18246@codeaurora.org> In-Reply-To: <20170405013525.GJ18246@codeaurora.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.121.8.100] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v35G732O023936 Hi Stephen, On Tue, 2017-04-04 at 18:35 -0700, Stephen Boyd wrote: > > +     .pll_table = (struct pll_of_table []){ > > +             { > > +                     .prate = 27000000, > > Can this be another clk in the framework instead of hardcoding > the parent rate? In fact there is another clk in the framework that represents this parent clock. But this field is needed to get appropriate pll_cfg_table as it depends on parent clock frequency. Below in pll_cfg_get function we are searching for the correct table comparing .parent_node field with real hardware parent clock frequency: ---------------------------------->8------------------------------------ for (i = 0; pll_table[i].prate != 0; i++)     if (pll_table[i].prate == prate)         return pll_table[i].pll_cfg_table; ---------------------------------->8------------------------------------ > > > +                     .pll_cfg_table = (struct pll_cfg []){ > > +                             { 25200000, 1, 84, 90 }, > > +                             { 50000000, 1, 100, 54 }, > > +                             { 74250000, 1, 44, 16 }, > > +                             { }, > > +                     }, > > +             }, > > +             /* Used as list limiter */ > > +             { }, > > There's only ever one, so I'm confused why we're making a list. By this patch we only add support of core arc pll and pgu pll and today they are clocked by the only parent clocks introduced here. But other plls on axs10x may be driven by different or configurable clocks, so in such cases we will have more than one entry in this list. And we are going to add more supported plls to this driver in the nearest future. > > + > > +     clk = clk_register(NULL, &pll_clk->hw); > > +     if (IS_ERR(clk)) { > > +             pr_err("failed to register %s clock (%ld)\n", > > +                             node->name, PTR_ERR(clk)); > > +             kfree(pll_clk); > > +             return; > > +     } > > + > > +     of_clk_add_provider(node, of_clk_src_simple_get, clk); > > Can you please use the clk_hw based provider and clk registration > functions? Sure. Could you be so kind to explain what is the difference between hw and non-hw based provider and clk registration functions please? In which cases they are preferred?  > > > +} > > + > > +CLK_OF_DECLARE(axs10x_pll_clock, "snps,axs10x-arc-pll-clock", of_pll_clk_setup); > > Does this need to be CLK_OF_DECLARE_DRIVER? I mean does the > driver need to probe and also have this of declare happen? Is the > PLL special and needs to be used for the timers? It is special and is used for the timers, so we have to CLK_OF_DECLARE it. On the other hand similar pll is used to drive PGU clock frequency and other subsystems and so we add usual probe func. -- Best regards, Vlad Zakharov From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vlad Zakharov Subject: Re: [PATCH v2] clk/axs10x: introduce AXS10X pll driver Date: Wed, 5 Apr 2017 16:06:11 +0000 Message-ID: <1491408370.9650.24.camel@synopsys.com> References: <1487682670-4164-1-git-send-email-vzakhar@synopsys.com> <20170405013525.GJ18246@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170405013525.GJ18246@codeaurora.org> Content-Language: en-US Content-ID: Sender: linux-clk-owner@vger.kernel.org To: "sboyd@codeaurora.org" Cc: "mark.rutland@arm.com" , "linux-kernel@vger.kernel.org" , "Jose.Abreu@synopsys.com" , "mturquette@baylibre.com" , "devicetree@vger.kernel.org" , "linux-clk@vger.kernel.org" , "linux-snps-arc@lists.infradead.org" List-Id: devicetree@vger.kernel.org SGkgU3RlcGhlbiwNCg0KT24gVHVlLCAyMDE3LTA0LTA0IGF0IDE4OjM1IC0wNzAwLCBTdGVwaGVu IEJveWQgd3JvdGU6DQo+ID4gK8KgwqDCoMKgwqAucGxsX3RhYmxlID0gKHN0cnVjdCBwbGxfb2Zf dGFibGUgW10pew0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHsNCj4gPiArwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgLnByYXRlID0gMjcwMDAwMDAsDQo+ IA0KPiBDYW4gdGhpcyBiZSBhbm90aGVyIGNsayBpbiB0aGUgZnJhbWV3b3JrIGluc3RlYWQgb2Yg aGFyZGNvZGluZw0KPiB0aGUgcGFyZW50IHJhdGU/DQoNCkluIGZhY3QgdGhlcmUgaXMgYW5vdGhl ciBjbGsgaW4gdGhlIGZyYW1ld29yayB0aGF0IHJlcHJlc2VudHMgdGhpcyBwYXJlbnQgY2xvY2su IEJ1dCB0aGlzIGZpZWxkIGlzIG5lZWRlZCB0byBnZXQNCmFwcHJvcHJpYXRlIHBsbF9jZmdfdGFi bGUgYXMgaXQgZGVwZW5kcyBvbiBwYXJlbnQgY2xvY2sgZnJlcXVlbmN5LiBCZWxvdyBpbiBwbGxf Y2ZnX2dldCBmdW5jdGlvbiB3ZSBhcmUgc2VhcmNoaW5nIGZvcg0KdGhlIGNvcnJlY3QgdGFibGUg Y29tcGFyaW5nIC5wYXJlbnRfbm9kZSBmaWVsZCB3aXRoIHJlYWwgaGFyZHdhcmUgcGFyZW50IGNs b2NrIGZyZXF1ZW5jeToNCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0+OC0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KZm9yIChpID0gMDsgcGxsX3RhYmxlW2ld LnByYXRlICE9IDA7IGkrKykNCsKgIMKgIGlmIChwbGxfdGFibGVbaV0ucHJhdGUgPT0gcHJhdGUp DQrCoCDCoCDCoCDCoCByZXR1cm4gcGxsX3RhYmxlW2ldLnBsbF9jZmdfdGFibGU7DQotLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tPjgtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0NCg0KPiANCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgLnBsbF9jZmdfdGFibGUgPSAoc3RydWN0IHBsbF9jZmcgW10pew0KPiA+ICvCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgeyAyNTIw MDAwMCwgMSwgODQsIDkwIH0sDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB7IDUwMDAwMDAwLCAxLCAxMDAsIDU0IH0sDQo+ID4g K8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqB7IDc0MjUwMDAwLCAxLCA0NCwgMTYgfSwNCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHsgfSwNCj4gPiArwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgfSwNCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqB9LA0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoC8qIFVzZWQgYXMgbGlz dCBsaW1pdGVyICovDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgeyB9LA0KPiANCj4g VGhlcmUncyBvbmx5IGV2ZXIgb25lLCBzbyBJJ20gY29uZnVzZWQgd2h5IHdlJ3JlIG1ha2luZyBh IGxpc3QuDQoNCkJ5IHRoaXMgcGF0Y2ggd2Ugb25seSBhZGQgc3VwcG9ydCBvZiBjb3JlIGFyYyBw bGwgYW5kIHBndSBwbGwgYW5kIHRvZGF5IHRoZXkgYXJlIGNsb2NrZWQgYnkgdGhlIG9ubHkgcGFy ZW50IGNsb2Nrcw0KaW50cm9kdWNlZCBoZXJlLiBCdXQgb3RoZXIgcGxscyBvbiBheHMxMHggbWF5 IGJlIGRyaXZlbiBieSBkaWZmZXJlbnQgb3IgY29uZmlndXJhYmxlIGNsb2Nrcywgc28gaW4gc3Vj aCBjYXNlcyB3ZSB3aWxsDQpoYXZlIG1vcmUgdGhhbiBvbmUgZW50cnkgaW4gdGhpcyBsaXN0LiBB bmQgd2UgYXJlIGdvaW5nIHRvIGFkZCBtb3JlIHN1cHBvcnRlZCBwbGxzIHRvIHRoaXMgZHJpdmVy IGluIHRoZSBuZWFyZXN0IGZ1dHVyZS4NCg0KPiA+ICsNCj4gPiArwqDCoMKgwqDCoGNsayA9IGNs a19yZWdpc3RlcihOVUxMLCAmcGxsX2Nsay0+aHcpOw0KPiA+ICvCoMKgwqDCoMKgaWYgKElTX0VS UihjbGspKSB7DQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgcHJfZXJyKCJmYWlsZWQg dG8gcmVnaXN0ZXIgJXMgY2xvY2sgKCVsZClcbiIsDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBub2RlLT5uYW1lLCBQVFJfRVJS KGNsaykpOw0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGtmcmVlKHBsbF9jbGspOw0K PiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHJldHVybjsNCj4gPiArwqDCoMKgwqDCoH0N Cj4gPiArDQo+ID4gK8KgwqDCoMKgwqBvZl9jbGtfYWRkX3Byb3ZpZGVyKG5vZGUsIG9mX2Nsa19z cmNfc2ltcGxlX2dldCwgY2xrKTsNCj4gDQo+IENhbiB5b3UgcGxlYXNlIHVzZSB0aGUgY2xrX2h3 IGJhc2VkIHByb3ZpZGVyIGFuZCBjbGsgcmVnaXN0cmF0aW9uDQo+IGZ1bmN0aW9ucz8NCg0KU3Vy ZS4gQ291bGQgeW91IGJlIHNvIGtpbmQgdG8gZXhwbGFpbiB3aGF0IGlzIHRoZSBkaWZmZXJlbmNl IGJldHdlZW4gaHcgYW5kIG5vbi1odyBiYXNlZCBwcm92aWRlciBhbmQgY2xrIHJlZ2lzdHJhdGlv bg0KZnVuY3Rpb25zIHBsZWFzZT8gSW4gd2hpY2ggY2FzZXMgdGhleSBhcmUgcHJlZmVycmVkP8Kg DQoNCj4gDQo+ID4gK30NCj4gPiArDQo+ID4gK0NMS19PRl9ERUNMQVJFKGF4czEweF9wbGxfY2xv Y2ssICJzbnBzLGF4czEweC1hcmMtcGxsLWNsb2NrIiwgb2ZfcGxsX2Nsa19zZXR1cCk7DQo+IA0K PiBEb2VzIHRoaXMgbmVlZCB0byBiZSBDTEtfT0ZfREVDTEFSRV9EUklWRVI/IEkgbWVhbiBkb2Vz IHRoZQ0KPiBkcml2ZXIgbmVlZCB0byBwcm9iZSBhbmQgYWxzbyBoYXZlIHRoaXMgb2YgZGVjbGFy ZSBoYXBwZW4/IElzIHRoZQ0KPiBQTEwgc3BlY2lhbCBhbmQgbmVlZHMgdG8gYmUgdXNlZCBmb3Ig dGhlIHRpbWVycz8NCg0KSXQgaXMgc3BlY2lhbCBhbmQgaXMgdXNlZCBmb3IgdGhlIHRpbWVycywg c28gd2UgaGF2ZSB0byBDTEtfT0ZfREVDTEFSRSBpdC4gT24gdGhlIG90aGVyIGhhbmQgc2ltaWxh ciBwbGwgaXMgdXNlZCB0bw0KZHJpdmUgUEdVIGNsb2NrIGZyZXF1ZW5jeSBhbmQgb3RoZXIgc3Vi c3lzdGVtcyBhbmQgc28gd2UgYWRkIHVzdWFsIHByb2JlIGZ1bmMuDQoNCi0tIA0KQmVzdCByZWdh cmRzLA0KVmxhZCBaYWtoYXJvdiA8dnpha2hhckBzeW5vcHN5cy5jb20+ From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vlad Zakharov To: "sboyd@codeaurora.org" CC: "mark.rutland@arm.com" , "linux-kernel@vger.kernel.org" , "Jose.Abreu@synopsys.com" , "mturquette@baylibre.com" , "devicetree@vger.kernel.org" , "linux-clk@vger.kernel.org" , "linux-snps-arc@lists.infradead.org" Subject: Re: [PATCH v2] clk/axs10x: introduce AXS10X pll driver Date: Wed, 5 Apr 2017 16:06:11 +0000 Message-ID: <1491408370.9650.24.camel@synopsys.com> References: <1487682670-4164-1-git-send-email-vzakhar@synopsys.com> <20170405013525.GJ18246@codeaurora.org> In-Reply-To: <20170405013525.GJ18246@codeaurora.org> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 List-ID: SGkgU3RlcGhlbiwNCg0KT24gVHVlLCAyMDE3LTA0LTA0IGF0IDE4OjM1IC0wNzAwLCBTdGVwaGVu IEJveWQgd3JvdGU6DQo+ID4gK8KgwqDCoMKgwqAucGxsX3RhYmxlID0gKHN0cnVjdCBwbGxfb2Zf dGFibGUgW10pew0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHsNCj4gPiArwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgLnByYXRlID0gMjcwMDAwMDAsDQo+ IA0KPiBDYW4gdGhpcyBiZSBhbm90aGVyIGNsayBpbiB0aGUgZnJhbWV3b3JrIGluc3RlYWQgb2Yg aGFyZGNvZGluZw0KPiB0aGUgcGFyZW50IHJhdGU/DQoNCkluIGZhY3QgdGhlcmUgaXMgYW5vdGhl ciBjbGsgaW4gdGhlIGZyYW1ld29yayB0aGF0IHJlcHJlc2VudHMgdGhpcyBwYXJlbnQgY2xvY2su IEJ1dCB0aGlzIGZpZWxkIGlzIG5lZWRlZCB0byBnZXQNCmFwcHJvcHJpYXRlIHBsbF9jZmdfdGFi bGUgYXMgaXQgZGVwZW5kcyBvbiBwYXJlbnQgY2xvY2sgZnJlcXVlbmN5LiBCZWxvdyBpbiBwbGxf Y2ZnX2dldCBmdW5jdGlvbiB3ZSBhcmUgc2VhcmNoaW5nIGZvcg0KdGhlIGNvcnJlY3QgdGFibGUg Y29tcGFyaW5nIC5wYXJlbnRfbm9kZSBmaWVsZCB3aXRoIHJlYWwgaGFyZHdhcmUgcGFyZW50IGNs b2NrIGZyZXF1ZW5jeToNCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0+OC0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KZm9yIChpID0gMDsgcGxsX3RhYmxlW2ld LnByYXRlICE9IDA7IGkrKykNCsKgIMKgIGlmIChwbGxfdGFibGVbaV0ucHJhdGUgPT0gcHJhdGUp DQrCoCDCoCDCoCDCoCByZXR1cm4gcGxsX3RhYmxlW2ldLnBsbF9jZmdfdGFibGU7DQotLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tPjgtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0NCg0KPiANCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgLnBsbF9jZmdfdGFibGUgPSAoc3RydWN0IHBsbF9jZmcgW10pew0KPiA+ICvCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgeyAyNTIw MDAwMCwgMSwgODQsIDkwIH0sDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB7IDUwMDAwMDAwLCAxLCAxMDAsIDU0IH0sDQo+ID4g K8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqB7IDc0MjUwMDAwLCAxLCA0NCwgMTYgfSwNCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHsgfSwNCj4gPiArwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgfSwNCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqB9LA0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoC8qIFVzZWQgYXMgbGlz dCBsaW1pdGVyICovDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgeyB9LA0KPiANCj4g VGhlcmUncyBvbmx5IGV2ZXIgb25lLCBzbyBJJ20gY29uZnVzZWQgd2h5IHdlJ3JlIG1ha2luZyBh IGxpc3QuDQoNCkJ5IHRoaXMgcGF0Y2ggd2Ugb25seSBhZGQgc3VwcG9ydCBvZiBjb3JlIGFyYyBw bGwgYW5kIHBndSBwbGwgYW5kIHRvZGF5IHRoZXkgYXJlIGNsb2NrZWQgYnkgdGhlIG9ubHkgcGFy ZW50IGNsb2Nrcw0KaW50cm9kdWNlZCBoZXJlLiBCdXQgb3RoZXIgcGxscyBvbiBheHMxMHggbWF5 IGJlIGRyaXZlbiBieSBkaWZmZXJlbnQgb3IgY29uZmlndXJhYmxlIGNsb2Nrcywgc28gaW4gc3Vj aCBjYXNlcyB3ZSB3aWxsDQpoYXZlIG1vcmUgdGhhbiBvbmUgZW50cnkgaW4gdGhpcyBsaXN0LiBB bmQgd2UgYXJlIGdvaW5nIHRvIGFkZCBtb3JlIHN1cHBvcnRlZCBwbGxzIHRvIHRoaXMgZHJpdmVy IGluIHRoZSBuZWFyZXN0IGZ1dHVyZS4NCg0KPiA+ICsNCj4gPiArwqDCoMKgwqDCoGNsayA9IGNs a19yZWdpc3RlcihOVUxMLCAmcGxsX2Nsay0+aHcpOw0KPiA+ICvCoMKgwqDCoMKgaWYgKElTX0VS UihjbGspKSB7DQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgcHJfZXJyKCJmYWlsZWQg dG8gcmVnaXN0ZXIgJXMgY2xvY2sgKCVsZClcbiIsDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBub2RlLT5uYW1lLCBQVFJfRVJS KGNsaykpOw0KPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGtmcmVlKHBsbF9jbGspOw0K PiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHJldHVybjsNCj4gPiArwqDCoMKgwqDCoH0N Cj4gPiArDQo+ID4gK8KgwqDCoMKgwqBvZl9jbGtfYWRkX3Byb3ZpZGVyKG5vZGUsIG9mX2Nsa19z cmNfc2ltcGxlX2dldCwgY2xrKTsNCj4gDQo+IENhbiB5b3UgcGxlYXNlIHVzZSB0aGUgY2xrX2h3 IGJhc2VkIHByb3ZpZGVyIGFuZCBjbGsgcmVnaXN0cmF0aW9uDQo+IGZ1bmN0aW9ucz8NCg0KU3Vy ZS4gQ291bGQgeW91IGJlIHNvIGtpbmQgdG8gZXhwbGFpbiB3aGF0IGlzIHRoZSBkaWZmZXJlbmNl IGJldHdlZW4gaHcgYW5kIG5vbi1odyBiYXNlZCBwcm92aWRlciBhbmQgY2xrIHJlZ2lzdHJhdGlv bg0KZnVuY3Rpb25zIHBsZWFzZT8gSW4gd2hpY2ggY2FzZXMgdGhleSBhcmUgcHJlZmVycmVkP8Kg DQoNCj4gDQo+ID4gK30NCj4gPiArDQo+ID4gK0NMS19PRl9ERUNMQVJFKGF4czEweF9wbGxfY2xv Y2ssICJzbnBzLGF4czEweC1hcmMtcGxsLWNsb2NrIiwgb2ZfcGxsX2Nsa19zZXR1cCk7DQo+IA0K PiBEb2VzIHRoaXMgbmVlZCB0byBiZSBDTEtfT0ZfREVDTEFSRV9EUklWRVI/IEkgbWVhbiBkb2Vz IHRoZQ0KPiBkcml2ZXIgbmVlZCB0byBwcm9iZSBhbmQgYWxzbyBoYXZlIHRoaXMgb2YgZGVjbGFy ZSBoYXBwZW4/IElzIHRoZQ0KPiBQTEwgc3BlY2lhbCBhbmQgbmVlZHMgdG8gYmUgdXNlZCBmb3Ig dGhlIHRpbWVycz8NCg0KSXQgaXMgc3BlY2lhbCBhbmQgaXMgdXNlZCBmb3IgdGhlIHRpbWVycywg c28gd2UgaGF2ZSB0byBDTEtfT0ZfREVDTEFSRSBpdC4gT24gdGhlIG90aGVyIGhhbmQgc2ltaWxh ciBwbGwgaXMgdXNlZCB0bw0KZHJpdmUgUEdVIGNsb2NrIGZyZXF1ZW5jeSBhbmQgb3RoZXIgc3Vi c3lzdGVtcyBhbmQgc28gd2UgYWRkIHVzdWFsIHByb2JlIGZ1bmMuDQoNCi0tIA0KQmVzdCByZWdh cmRzLA0KVmxhZCBaYWtoYXJvdiA8dnpha2hhckBzeW5vcHN5cy5jb20+ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vladislav.Zakharov@synopsys.com (Vlad Zakharov) Date: Wed, 5 Apr 2017 16:06:11 +0000 Subject: [PATCH v2] clk/axs10x: introduce AXS10X pll driver In-Reply-To: <20170405013525.GJ18246@codeaurora.org> References: <1487682670-4164-1-git-send-email-vzakhar@synopsys.com> <20170405013525.GJ18246@codeaurora.org> List-ID: Message-ID: <1491408370.9650.24.camel@synopsys.com> To: linux-snps-arc@lists.infradead.org Hi Stephen, On Tue, 2017-04-04@18:35 -0700, Stephen Boyd wrote: > > +?????.pll_table = (struct pll_of_table []){ > > +?????????????{ > > +?????????????????????.prate = 27000000, > > Can this be another clk in the framework instead of hardcoding > the parent rate? In fact there is another clk in the framework that represents this parent clock. But this field is needed to get appropriate pll_cfg_table as it depends on parent clock frequency. Below in pll_cfg_get function we are searching for the correct table comparing .parent_node field with real hardware parent clock frequency: ---------------------------------->8------------------------------------ for (i = 0; pll_table[i].prate != 0; i++) ? ? if (pll_table[i].prate == prate) ? ? ? ? return pll_table[i].pll_cfg_table; ---------------------------------->8------------------------------------ > > > +?????????????????????.pll_cfg_table = (struct pll_cfg []){ > > +?????????????????????????????{ 25200000, 1, 84, 90 }, > > +?????????????????????????????{ 50000000, 1, 100, 54 }, > > +?????????????????????????????{ 74250000, 1, 44, 16 }, > > +?????????????????????????????{ }, > > +?????????????????????}, > > +?????????????}, > > +?????????????/* Used as list limiter */ > > +?????????????{ }, > > There's only ever one, so I'm confused why we're making a list. By this patch we only add support of core arc pll and pgu pll and today they are clocked by the only parent clocks introduced here. But other plls on axs10x may be driven by different or configurable clocks, so in such cases we will have more than one entry in this list. And we are going to add more supported plls to this driver in the nearest future. > > + > > +?????clk = clk_register(NULL, &pll_clk->hw); > > +?????if (IS_ERR(clk)) { > > +?????????????pr_err("failed to register %s clock (%ld)\n", > > +?????????????????????????????node->name, PTR_ERR(clk)); > > +?????????????kfree(pll_clk); > > +?????????????return; > > +?????} > > + > > +?????of_clk_add_provider(node, of_clk_src_simple_get, clk); > > Can you please use the clk_hw based provider and clk registration > functions? Sure. Could you be so kind to explain what is the difference between hw and non-hw based provider and clk registration functions please? In which cases they are preferred?? > > > +} > > + > > +CLK_OF_DECLARE(axs10x_pll_clock, "snps,axs10x-arc-pll-clock", of_pll_clk_setup); > > Does this need to be CLK_OF_DECLARE_DRIVER? I mean does the > driver need to probe and also have this of declare happen? Is the > PLL special and needs to be used for the timers? It is special and is used for the timers, so we have to CLK_OF_DECLARE it. On the other hand similar pll is used to drive PGU clock frequency and other subsystems and so we add usual probe func. -- Best regards, Vlad Zakharov