From mboxrd@z Thu Jan 1 00:00:00 1970 From: mturquette@linaro.org (Mike Turquette) Date: Wed, 10 Sep 2014 09:52:11 -0700 Subject: [PATCH resend 1/4] clk: hix5hd2: add complex clk In-Reply-To: References: <1409031970-4821-1-git-send-email-zhangfei.gao@linaro.org> <1409031970-4821-2-git-send-email-zhangfei.gao@linaro.org> <20140903173753.11368.10916@quantum> Message-ID: <20140910165211.19023.39726@quantum> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Quoting ??? (2014-09-04 23:37:25) > > > 2014-09-04 1:37 GMT+08:00 Mike Turquette : > > Quoting Zhangfei Gao (2014-08-25 22:46:07) > > +static int clk_ether_enable(struct clk_hw *hw) > > +{ > > +? ? ? ?struct hix5hd2_clk_complex *clk = to_complex_clk(hw); > > +? ? ? ?u32 val; > > + > > +? ? ? ?val = readl_relaxed(clk->ctrl_reg); > > +? ? ? ?val |= clk->ctrl_clk_mask | clk->ctrl_rst_mask; > > +? ? ? ?writel_relaxed(val, clk->ctrl_reg); > > +? ? ? ?val &= ~(clk->ctrl_rst_mask); > > +? ? ? ?writel_relaxed(val, clk->ctrl_reg); > > + > > +? ? ? ?val = readl_relaxed(clk->phy_reg); > > +? ? ? ?val |= clk->phy_clk_mask; > > +? ? ? ?val &= ~(clk->phy_rst_mask); > > +? ? ? ?writel_relaxed(val, clk->phy_reg); > > +? ? ? ?mdelay(10); > > + > > +? ? ? ?val &= ~(clk->phy_clk_mask); > > +? ? ? ?val |= clk->phy_rst_mask; > > +? ? ? ?writel_relaxed(val, clk->phy_reg); > > +? ? ? ?mdelay(10); > > + > > +? ? ? ?val |= clk->phy_clk_mask; > > +? ? ? ?val &= ~(clk->phy_rst_mask); > > +? ? ? ?writel_relaxed(val, clk->phy_reg); > > +? ? ? ?mdelay(30); > > With all of these mdelays, I wonder if you should use .prepare and > .unprepare instead? Does the Ethernet driver call clk_{en|dis}able from > interrupt context? > > ? > Thank you for the advise. > > In hix5hd2 soc, these mdelays are necessary for resetting the Ethernet ?phy > device. The hardware need some time to be stable.It's difficult to use .prepare > and .unprepare instead, because they are embeded in several places among the > whole sequence. Even though some code segment could be put into ?the .prepare > callback, mdelays should still be reserved. So we hope to keep this manner if > it's ok. OK. I wonder if you should be using the reset controller framework to control the reset of your phy? Some clock drivers are also reset drivers since bits for controlling that stuff are often combined in the same register space. As an example, take a look at: drivers/clk/qcom/gcc-apq8084.c > > The Ethernet driver won't call clk_enable and clk_disable from interrupt > context. Good to know. clk_enable and clk_disable are designed to be called safely from interrupt context. clk_prepare and clk_unprepare often enable/disable a clock, but are designed for use in a regular process context (e.g. we might sleep or schedule). So depending on how long it takes you to enable/disable your Ethernet clock you might want to migrate to those callbacks instead. Regards, Mike > > ? > > > +? ? ? ?return 0; > > +} > > + > > +static void clk_ether_disable(struct clk_hw *hw) > > +{ > > +? ? ? ?struct hix5hd2_clk_complex *clk = to_complex_clk(hw); > > +? ? ? ?u32 val; > > + > > +? ? ? ?val = readl_relaxed(clk->ctrl_reg); > > +? ? ? ?val &= ~(clk->ctrl_clk_mask); > > +? ? ? ?writel_relaxed(val, clk->ctrl_reg); > > +} > > + > > +static struct clk_ops clk_ether_ops = { > > +? ? ? ?.enable = clk_ether_enable, > > +? ? ? ?.disable = clk_ether_disable, > > +}; > > + > > +static int clk_complex_enable(struct clk_hw *hw) > > +{ > > +? ? ? ?struct hix5hd2_clk_complex *clk = to_complex_clk(hw); > > +? ? ? ?u32 val; > > + > > +? ? ? ?val = readl_relaxed(clk->ctrl_reg); > > +? ? ? ?val |= clk->ctrl_clk_mask; > > +? ? ? ?val &= ~(clk->ctrl_rst_mask); > > +? ? ? ?writel_relaxed(val, clk->ctrl_reg); > > + > > +? ? ? ?val = readl_relaxed(clk->phy_reg); > > +? ? ? ?val |= clk->phy_clk_mask; > > +? ? ? ?val &= ~(clk->phy_rst_mask); > > +? ? ? ?writel_relaxed(val, clk->phy_reg); > > + > > +? ? ? ?return 0; > > +} > > + > > +static void clk_complex_disable(struct clk_hw *hw) > > +{ > > +? ? ? ?struct hix5hd2_clk_complex *clk = to_complex_clk(hw); > > +? ? ? ?u32 val; > > + > > +? ? ? ?val = readl_relaxed(clk->ctrl_reg); > > +? ? ? ?val |= clk->ctrl_rst_mask; > > +? ? ? ?val &= ~(clk->ctrl_clk_mask); > > +? ? ? ?writel_relaxed(val, clk->ctrl_reg); > > + > > +? ? ? ?val = readl_relaxed(clk->phy_reg); > > +? ? ? ?val |= clk->phy_rst_mask; > > +? ? ? ?val &= ~(clk->phy_clk_mask); > > +? ? ? ?writel_relaxed(val, clk->phy_reg); > > +} > > + > > +static struct clk_ops clk_complex_ops = { > > +? ? ? ?.enable = clk_complex_enable, > > +? ? ? ?.disable = clk_complex_disable, > > +}; > > These enable/disable callbacks look good, with no delays. > > Regards, > Mike > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Turquette Subject: Re: [PATCH resend 1/4] clk: hix5hd2: add complex clk Date: Wed, 10 Sep 2014 09:52:11 -0700 Message-ID: <20140910165211.19023.39726@quantum> References: <1409031970-4821-1-git-send-email-zhangfei.gao@linaro.org> <1409031970-4821-2-git-send-email-zhangfei.gao@linaro.org> <20140903173753.11368.10916@quantum> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: =?utf-8?b?6Jab5bu65oiQ?= Cc: devicetree@vger.kernel.org, haifeng.yan@linaro.org, xuwei5@hisilicon.com, Haojian Zhuang , Jiancheng Xue , Zhangfei Gao , linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org UXVvdGluZyDolpvlu7rmiJAgKDIwMTQtMDktMDQgMjM6Mzc6MjUpCj4gCj4gCj4gMjAxNC0wOS0w NCAxOjM3IEdNVCswODowMCBNaWtlIFR1cnF1ZXR0ZSA8bXR1cnF1ZXR0ZUBsaW5hcm8ub3JnPjoK PiAKPiAgICAgUXVvdGluZyBaaGFuZ2ZlaSBHYW8gKDIwMTQtMDgtMjUgMjI6NDY6MDcpCj4gICAg ID4gK3N0YXRpYyBpbnQgY2xrX2V0aGVyX2VuYWJsZShzdHJ1Y3QgY2xrX2h3ICpodykKPiAgICAg PiArewo+ICAgICA+ICvCoCDCoCDCoCDCoHN0cnVjdCBoaXg1aGQyX2Nsa19jb21wbGV4ICpjbGsg PSB0b19jb21wbGV4X2Nsayhodyk7Cj4gICAgID4gK8KgIMKgIMKgIMKgdTMyIHZhbDsKPiAgICAg PiArCj4gICAgID4gK8KgIMKgIMKgIMKgdmFsID0gcmVhZGxfcmVsYXhlZChjbGstPmN0cmxfcmVn KTsKPiAgICAgPiArwqAgwqAgwqAgwqB2YWwgfD0gY2xrLT5jdHJsX2Nsa19tYXNrIHwgY2xrLT5j dHJsX3JzdF9tYXNrOwo+ICAgICA+ICvCoCDCoCDCoCDCoHdyaXRlbF9yZWxheGVkKHZhbCwgY2xr LT5jdHJsX3JlZyk7Cj4gICAgID4gK8KgIMKgIMKgIMKgdmFsICY9IH4oY2xrLT5jdHJsX3JzdF9t YXNrKTsKPiAgICAgPiArwqAgwqAgwqAgwqB3cml0ZWxfcmVsYXhlZCh2YWwsIGNsay0+Y3RybF9y ZWcpOwo+ICAgICA+ICsKPiAgICAgPiArwqAgwqAgwqAgwqB2YWwgPSByZWFkbF9yZWxheGVkKGNs ay0+cGh5X3JlZyk7Cj4gICAgID4gK8KgIMKgIMKgIMKgdmFsIHw9IGNsay0+cGh5X2Nsa19tYXNr Owo+ICAgICA+ICvCoCDCoCDCoCDCoHZhbCAmPSB+KGNsay0+cGh5X3JzdF9tYXNrKTsKPiAgICAg PiArwqAgwqAgwqAgwqB3cml0ZWxfcmVsYXhlZCh2YWwsIGNsay0+cGh5X3JlZyk7Cj4gICAgID4g K8KgIMKgIMKgIMKgbWRlbGF5KDEwKTsKPiAgICAgPiArCj4gICAgID4gK8KgIMKgIMKgIMKgdmFs ICY9IH4oY2xrLT5waHlfY2xrX21hc2spOwo+ICAgICA+ICvCoCDCoCDCoCDCoHZhbCB8PSBjbGst PnBoeV9yc3RfbWFzazsKPiAgICAgPiArwqAgwqAgwqAgwqB3cml0ZWxfcmVsYXhlZCh2YWwsIGNs ay0+cGh5X3JlZyk7Cj4gICAgID4gK8KgIMKgIMKgIMKgbWRlbGF5KDEwKTsKPiAgICAgPiArCj4g ICAgID4gK8KgIMKgIMKgIMKgdmFsIHw9IGNsay0+cGh5X2Nsa19tYXNrOwo+ICAgICA+ICvCoCDC oCDCoCDCoHZhbCAmPSB+KGNsay0+cGh5X3JzdF9tYXNrKTsKPiAgICAgPiArwqAgwqAgwqAgwqB3 cml0ZWxfcmVsYXhlZCh2YWwsIGNsay0+cGh5X3JlZyk7Cj4gICAgID4gK8KgIMKgIMKgIMKgbWRl bGF5KDMwKTsKPiAKPiAgICAgV2l0aCBhbGwgb2YgdGhlc2UgbWRlbGF5cywgSSB3b25kZXIgaWYg eW91IHNob3VsZCB1c2UgLnByZXBhcmUgYW5kCj4gICAgIC51bnByZXBhcmUgaW5zdGVhZD8gRG9l cyB0aGUgRXRoZXJuZXQgZHJpdmVyIGNhbGwgY2xrX3tlbnxkaXN9YWJsZSBmcm9tCj4gICAgIGlu dGVycnVwdCBjb250ZXh0Pwo+IAo+IMKgCj4gVGhhbmsgeW91IGZvciB0aGUgYWR2aXNlLgo+IAo+ IEluIGhpeDVoZDIgc29jLCB0aGVzZSBtZGVsYXlzIGFyZSBuZWNlc3NhcnkgZm9yIHJlc2V0dGlu ZyB0aGUgRXRoZXJuZXQgwqBwaHkKPiBkZXZpY2UuIFRoZSBoYXJkd2FyZSBuZWVkIHNvbWUgdGlt ZSB0byBiZSBzdGFibGUuSXQncyBkaWZmaWN1bHQgdG8gdXNlIC5wcmVwYXJlCj4gYW5kIC51bnBy ZXBhcmUgaW5zdGVhZCwgYmVjYXVzZSB0aGV5IGFyZSBlbWJlZGVkIGluIHNldmVyYWwgcGxhY2Vz IGFtb25nIHRoZQo+IHdob2xlIHNlcXVlbmNlLiBFdmVuIHRob3VnaCBzb21lIGNvZGUgc2VnbWVu dCBjb3VsZCBiZSBwdXQgaW50byDCoHRoZSAucHJlcGFyZQo+IGNhbGxiYWNrLCBtZGVsYXlzIHNo b3VsZCBzdGlsbCBiZSByZXNlcnZlZC4gU28gd2UgaG9wZSB0byBrZWVwIHRoaXMgbWFubmVyIGlm Cj4gaXQncyBvay4KCk9LLiBJIHdvbmRlciBpZiB5b3Ugc2hvdWxkIGJlIHVzaW5nIHRoZSByZXNl dCBjb250cm9sbGVyIGZyYW1ld29yayB0byBjb250cm9sIHRoZQpyZXNldCBvZiB5b3VyIHBoeT8g U29tZSBjbG9jayBkcml2ZXJzIGFyZSBhbHNvIHJlc2V0IGRyaXZlcnMgc2luY2UgYml0cwpmb3Ig Y29udHJvbGxpbmcgdGhhdCBzdHVmZiBhcmUgb2Z0ZW4gY29tYmluZWQgaW4gdGhlIHNhbWUgcmVn aXN0ZXIKc3BhY2UuIEFzIGFuIGV4YW1wbGUsIHRha2UgYSBsb29rIGF0OgoKZHJpdmVycy9jbGsv cWNvbS9nY2MtYXBxODA4NC5jCgo+IAo+IFRoZSBFdGhlcm5ldCBkcml2ZXIgd29uJ3QgY2FsbCBj bGtfZW5hYmxlIGFuZCBjbGtfZGlzYWJsZSBmcm9tIGludGVycnVwdAo+IGNvbnRleHQuCgpHb29k IHRvIGtub3cuIGNsa19lbmFibGUgYW5kIGNsa19kaXNhYmxlIGFyZSBkZXNpZ25lZCB0byBiZSBj YWxsZWQKc2FmZWx5IGZyb20gaW50ZXJydXB0IGNvbnRleHQuIGNsa19wcmVwYXJlIGFuZCBjbGtf dW5wcmVwYXJlIG9mdGVuCmVuYWJsZS9kaXNhYmxlIGEgY2xvY2ssIGJ1dCBhcmUgZGVzaWduZWQg Zm9yIHVzZSBpbiBhIHJlZ3VsYXIgcHJvY2Vzcwpjb250ZXh0IChlLmcuIHdlIG1pZ2h0IHNsZWVw IG9yIHNjaGVkdWxlKS4gU28gZGVwZW5kaW5nIG9uIGhvdyBsb25nIGl0CnRha2VzIHlvdSB0byBl bmFibGUvZGlzYWJsZSB5b3VyIEV0aGVybmV0IGNsb2NrIHlvdSBtaWdodCB3YW50IHRvCm1pZ3Jh dGUgdG8gdGhvc2UgY2FsbGJhY2tzIGluc3RlYWQuCgpSZWdhcmRzLApNaWtlCgo+IAo+ICAgICDC oAo+IAo+ICAgICA+ICvCoCDCoCDCoCDCoHJldHVybiAwOwo+ICAgICA+ICt9Cj4gICAgID4gKwo+ ICAgICA+ICtzdGF0aWMgdm9pZCBjbGtfZXRoZXJfZGlzYWJsZShzdHJ1Y3QgY2xrX2h3ICpodykK PiAgICAgPiArewo+ICAgICA+ICvCoCDCoCDCoCDCoHN0cnVjdCBoaXg1aGQyX2Nsa19jb21wbGV4 ICpjbGsgPSB0b19jb21wbGV4X2Nsayhodyk7Cj4gICAgID4gK8KgIMKgIMKgIMKgdTMyIHZhbDsK PiAgICAgPiArCj4gICAgID4gK8KgIMKgIMKgIMKgdmFsID0gcmVhZGxfcmVsYXhlZChjbGstPmN0 cmxfcmVnKTsKPiAgICAgPiArwqAgwqAgwqAgwqB2YWwgJj0gfihjbGstPmN0cmxfY2xrX21hc2sp Owo+ICAgICA+ICvCoCDCoCDCoCDCoHdyaXRlbF9yZWxheGVkKHZhbCwgY2xrLT5jdHJsX3JlZyk7 Cj4gICAgID4gK30KPiAgICAgPiArCj4gICAgID4gK3N0YXRpYyBzdHJ1Y3QgY2xrX29wcyBjbGtf ZXRoZXJfb3BzID0gewo+ICAgICA+ICvCoCDCoCDCoCDCoC5lbmFibGUgPSBjbGtfZXRoZXJfZW5h YmxlLAo+ICAgICA+ICvCoCDCoCDCoCDCoC5kaXNhYmxlID0gY2xrX2V0aGVyX2Rpc2FibGUsCj4g ICAgID4gK307Cj4gICAgID4gKwo+ICAgICA+ICtzdGF0aWMgaW50IGNsa19jb21wbGV4X2VuYWJs ZShzdHJ1Y3QgY2xrX2h3ICpodykKPiAgICAgPiArewo+ICAgICA+ICvCoCDCoCDCoCDCoHN0cnVj dCBoaXg1aGQyX2Nsa19jb21wbGV4ICpjbGsgPSB0b19jb21wbGV4X2Nsayhodyk7Cj4gICAgID4g K8KgIMKgIMKgIMKgdTMyIHZhbDsKPiAgICAgPiArCj4gICAgID4gK8KgIMKgIMKgIMKgdmFsID0g cmVhZGxfcmVsYXhlZChjbGstPmN0cmxfcmVnKTsKPiAgICAgPiArwqAgwqAgwqAgwqB2YWwgfD0g Y2xrLT5jdHJsX2Nsa19tYXNrOwo+ICAgICA+ICvCoCDCoCDCoCDCoHZhbCAmPSB+KGNsay0+Y3Ry bF9yc3RfbWFzayk7Cj4gICAgID4gK8KgIMKgIMKgIMKgd3JpdGVsX3JlbGF4ZWQodmFsLCBjbGst PmN0cmxfcmVnKTsKPiAgICAgPiArCj4gICAgID4gK8KgIMKgIMKgIMKgdmFsID0gcmVhZGxfcmVs YXhlZChjbGstPnBoeV9yZWcpOwo+ICAgICA+ICvCoCDCoCDCoCDCoHZhbCB8PSBjbGstPnBoeV9j bGtfbWFzazsKPiAgICAgPiArwqAgwqAgwqAgwqB2YWwgJj0gfihjbGstPnBoeV9yc3RfbWFzayk7 Cj4gICAgID4gK8KgIMKgIMKgIMKgd3JpdGVsX3JlbGF4ZWQodmFsLCBjbGstPnBoeV9yZWcpOwo+ ICAgICA+ICsKPiAgICAgPiArwqAgwqAgwqAgwqByZXR1cm4gMDsKPiAgICAgPiArfQo+ICAgICA+ ICsKPiAgICAgPiArc3RhdGljIHZvaWQgY2xrX2NvbXBsZXhfZGlzYWJsZShzdHJ1Y3QgY2xrX2h3 ICpodykKPiAgICAgPiArewo+ICAgICA+ICvCoCDCoCDCoCDCoHN0cnVjdCBoaXg1aGQyX2Nsa19j b21wbGV4ICpjbGsgPSB0b19jb21wbGV4X2Nsayhodyk7Cj4gICAgID4gK8KgIMKgIMKgIMKgdTMy IHZhbDsKPiAgICAgPiArCj4gICAgID4gK8KgIMKgIMKgIMKgdmFsID0gcmVhZGxfcmVsYXhlZChj bGstPmN0cmxfcmVnKTsKPiAgICAgPiArwqAgwqAgwqAgwqB2YWwgfD0gY2xrLT5jdHJsX3JzdF9t YXNrOwo+ICAgICA+ICvCoCDCoCDCoCDCoHZhbCAmPSB+KGNsay0+Y3RybF9jbGtfbWFzayk7Cj4g ICAgID4gK8KgIMKgIMKgIMKgd3JpdGVsX3JlbGF4ZWQodmFsLCBjbGstPmN0cmxfcmVnKTsKPiAg ICAgPiArCj4gICAgID4gK8KgIMKgIMKgIMKgdmFsID0gcmVhZGxfcmVsYXhlZChjbGstPnBoeV9y ZWcpOwo+ICAgICA+ICvCoCDCoCDCoCDCoHZhbCB8PSBjbGstPnBoeV9yc3RfbWFzazsKPiAgICAg PiArwqAgwqAgwqAgwqB2YWwgJj0gfihjbGstPnBoeV9jbGtfbWFzayk7Cj4gICAgID4gK8KgIMKg IMKgIMKgd3JpdGVsX3JlbGF4ZWQodmFsLCBjbGstPnBoeV9yZWcpOwo+ICAgICA+ICt9Cj4gICAg ID4gKwo+ICAgICA+ICtzdGF0aWMgc3RydWN0IGNsa19vcHMgY2xrX2NvbXBsZXhfb3BzID0gewo+ ICAgICA+ICvCoCDCoCDCoCDCoC5lbmFibGUgPSBjbGtfY29tcGxleF9lbmFibGUsCj4gICAgID4g K8KgIMKgIMKgIMKgLmRpc2FibGUgPSBjbGtfY29tcGxleF9kaXNhYmxlLAo+ICAgICA+ICt9Owo+ IAo+ICAgICBUaGVzZSBlbmFibGUvZGlzYWJsZSBjYWxsYmFja3MgbG9vayBnb29kLCB3aXRoIG5v IGRlbGF5cy4KPiAKPiAgICAgUmVnYXJkcywKPiAgICAgTWlrZQo+IAo+IAoKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5lbCBtYWls aW5nIGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0 cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo=