From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jisheng Zhang Subject: Re: [PATCH 2/4] i2c: designware-platdrv: fix unbalanced clk enable and prepare Date: Thu, 21 Apr 2016 10:40:22 +0800 Message-ID: <20160421104022.2d6549a9@xhacker> References: <1460638414-5987-1-git-send-email-jszhang@marvell.com> <1460638414-5987-3-git-send-email-jszhang@marvell.com> <57177C32.4090000@linux.intel.com> <1461161760.6620.306.camel@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1461161760.6620.306.camel@linux.intel.com> 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: Andy Shevchenko Cc: wsa@the-dreams.de, linux-kernel@vger.kernel.org, Jarkko Nikula , linux-i2c@vger.kernel.org, mika.westerberg@linux.intel.com, linux-arm-kernel@lists.infradead.org List-Id: linux-i2c@vger.kernel.org RGVhciBKYXJra28sIEFuZHksCgpPbiBXZWQsIDIwIEFwciAyMDE2IDE3OjE2OjAwICswMzAwIEFu ZHkgU2hldmNoZW5rbyB3cm90ZToKCj4gT24gV2VkLCAyMDE2LTA0LTIwIGF0IDE1OjU1ICswMzAw LCBKYXJra28gTmlrdWxhIHdyb3RlOgo+ID4gT24gMDQvMTQvMjAxNiAwMzo1MyBQTSwgSmlzaGVu ZyBaaGFuZyB3cm90ZTogIAo+ID4gPiAKPiA+ID4gSWYgaTJjX2R3X3Byb2JlKCkgZmFpbCwgd2Ug c2hvdWxkIGNhbGwgaTJjX2R3X3BsYXRfcHJlcGFyZV9jbGsoKSB0bwo+ID4gPiBkaXNhYmxlIGFu ZCB1bnByZXBhcmUgdGhlIGNsaywgb3RoZXJ3aXNlIHRoZSBjbGsgZW5hYmxlIGFuZCBwcmVwYXJl Cj4gPiA+IGlzIGxlZnQgdW5iYWxhbmNlZC4KPiA+ID4gCj4gPiA+IFNpZ25lZC1vZmYtYnk6IEpp c2hlbmcgWmhhbmcgPGpzemhhbmdAbWFydmVsbC5jb20+Cj4gPiA+IC0tLQo+ID4gPiDCoCBkcml2 ZXJzL2kyYy9idXNzZXMvaTJjLWRlc2lnbndhcmUtcGxhdGRydi5jIHwgMiArKwo+ID4gPiDCoCAx IGZpbGUgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspCj4gPiA+IAo+ID4gPiBkaWZmIC0tZ2l0IGEv ZHJpdmVycy9pMmMvYnVzc2VzL2kyYy1kZXNpZ253YXJlLXBsYXRkcnYuYwo+ID4gPiBiL2RyaXZl cnMvaTJjL2J1c3Nlcy9pMmMtZGVzaWdud2FyZS1wbGF0ZHJ2LmMKPiA+ID4gaW5kZXggMDBmOWU5 OS4uMTQ4OGNlYSAxMDA2NDQKPiA+ID4gLS0tIGEvZHJpdmVycy9pMmMvYnVzc2VzL2kyYy1kZXNp Z253YXJlLXBsYXRkcnYuYwo+ID4gPiArKysgYi9kcml2ZXJzL2kyYy9idXNzZXMvaTJjLWRlc2ln bndhcmUtcGxhdGRydi5jCj4gPiA+IEBAIC0yNjgsNiArMjY4LDggQEAgcnBtX2Rpc2FibGU6Cj4g PiA+IMKgwqAJCXBtX3J1bnRpbWVfcHV0X25vaWRsZSgmcGRldi0+ZGV2KTsKPiA+ID4gwqDCoAl9 Cj4gPiA+IAo+ID4gPiArCWkyY19kd19wbGF0X3ByZXBhcmVfY2xrKGRldiwgZmFsc2UpOwo+ID4g PiArCj4gPiA+IMKgwqAJcmV0dXJuIHI7Cj4gPiA+IMKgIH0KPiA+ID4gICAKPiA+IFRoaXMgaXMg YSBiaXQgdW5jbGVhciB0byBtZSBkb2VzIGRldm1fY2xrX2dldCB0YWtlIGNhcmUgb2YgY2xrCj4g PiBkaXNhYmxpbmfCoAo+ID4gaW4gY2FzZSBvZiBwcm9iZSBlcnJvciBvciBkcml2ZXIgcmVtb3Zh bD8KPiA+IAo+ID4gSSBzZWUgQW5keSdzIDFjYjcxNWNhNDY5NCAoImkyYy1kZXNpZ253YXJlOiBt b3ZlIHRvIG1hbmFnZWQgZnVuY3Rpb25zwqAKPiA+IChkZXZtXyopIikgcmVtb3ZlZCBpdCBidXQg YXQgcXVpY2sgbG9vayBkcml2ZXJzL2Nsay9jbGstZGV2cmVzLmM6wqAKPiA+IGRldm1fY2xrX3Jl bGVhc2UoKSBjYWxscyBvbmx5IGNsa19wdXQgYW5kIEkgZG9uJ3Qgc2VlIGRpc2FibGUgaXMgZG9u ZcKgCj4gPiBkb3duIHRoZSBwYXRoLiAgCj4gCj4gVGhlIGZvbGxvd2luZyBpcyBhIG1pc3Rha2Ug b2YgdGhlIG1lbnRpb25lZCBwYXRjaC4KPiAtwqDCoMKgwqDCoMKgwqBjbGtfZGlzYWJsZV91bnBy ZXBhcmUoZGV2LT5jbGspOwo+IAo+IEkgZGlkIGF0IHRoZSBzYW1lIG1pc3Rha2UgaW4gZHdfZG1h YyBkcml2ZXIgd2hpY2ggaGFkIGJlZW4gZml4ZWQgbGF0ZXIKPiBpbiB0aGUgY29tbWl0IDhiZTRm NTIzYjQ4MCAoImRtYWVuZ2luZTogZHc6IGZpeCByZWdyZXNzaW9uIGluIGR3X3Byb2JlKCkKPiBm dW5jdGlvbiIpLgo+IAoKQXMgQW5keSBwb2ludGVkIG91dCwgbWFuYWdlZCBkZXZtX2Nsa19nZXQg Y2FuIG9ubHkgYXV0b21hdGljYWxseSBwdXQgY2xrCmJ1dCBkb2Vzbid0IHVucHJlcGFyZSBhbmQg ZGlzYWJsZSB0aGUgY2xrCgpUaGFua3MsCkppc2hlbmcKCl9fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0Cmxp bnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFk Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: jszhang@marvell.com (Jisheng Zhang) Date: Thu, 21 Apr 2016 10:40:22 +0800 Subject: [PATCH 2/4] i2c: designware-platdrv: fix unbalanced clk enable and prepare In-Reply-To: <1461161760.6620.306.camel@linux.intel.com> References: <1460638414-5987-1-git-send-email-jszhang@marvell.com> <1460638414-5987-3-git-send-email-jszhang@marvell.com> <57177C32.4090000@linux.intel.com> <1461161760.6620.306.camel@linux.intel.com> Message-ID: <20160421104022.2d6549a9@xhacker> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dear Jarkko, Andy, On Wed, 20 Apr 2016 17:16:00 +0300 Andy Shevchenko wrote: > On Wed, 2016-04-20 at 15:55 +0300, Jarkko Nikula wrote: > > On 04/14/2016 03:53 PM, Jisheng Zhang wrote: > > > > > > If i2c_dw_probe() fail, we should call i2c_dw_plat_prepare_clk() to > > > disable and unprepare the clk, otherwise the clk enable and prepare > > > is left unbalanced. > > > > > > Signed-off-by: Jisheng Zhang > > > --- > > > ? drivers/i2c/busses/i2c-designware-platdrv.c | 2 ++ > > > ? 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c > > > b/drivers/i2c/busses/i2c-designware-platdrv.c > > > index 00f9e99..1488cea 100644 > > > --- a/drivers/i2c/busses/i2c-designware-platdrv.c > > > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c > > > @@ -268,6 +268,8 @@ rpm_disable: > > > ?? pm_runtime_put_noidle(&pdev->dev); > > > ?? } > > > > > > + i2c_dw_plat_prepare_clk(dev, false); > > > + > > > ?? return r; > > > ? } > > > > > This is a bit unclear to me does devm_clk_get take care of clk > > disabling? > > in case of probe error or driver removal? > > > > I see Andy's 1cb715ca4694 ("i2c-designware: move to managed functions? > > (devm_*)") removed it but at quick look drivers/clk/clk-devres.c:? > > devm_clk_release() calls only clk_put and I don't see disable is done? > > down the path. > > The following is a mistake of the mentioned patch. > -???????clk_disable_unprepare(dev->clk); > > I did at the same mistake in dw_dmac driver which had been fixed later > in the commit 8be4f523b480 ("dmaengine: dw: fix regression in dw_probe() > function"). > As Andy pointed out, managed devm_clk_get can only automatically put clk but doesn't unprepare and disable the clk Thanks, Jisheng From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751569AbcDUCqI (ORCPT ); Wed, 20 Apr 2016 22:46:08 -0400 Received: from mx0b-0016f401.pphosted.com ([67.231.156.173]:37318 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751124AbcDUCqH convert rfc822-to-8bit (ORCPT ); Wed, 20 Apr 2016 22:46:07 -0400 Date: Thu, 21 Apr 2016 10:40:22 +0800 From: Jisheng Zhang To: Andy Shevchenko CC: Jarkko Nikula , , , , , Subject: Re: [PATCH 2/4] i2c: designware-platdrv: fix unbalanced clk enable and prepare Message-ID: <20160421104022.2d6549a9@xhacker> In-Reply-To: <1461161760.6620.306.camel@linux.intel.com> References: <1460638414-5987-1-git-send-email-jszhang@marvell.com> <1460638414-5987-3-git-send-email-jszhang@marvell.com> <57177C32.4090000@linux.intel.com> <1461161760.6620.306.camel@linux.intel.com> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-04-21_01:,, signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1603290000 definitions=main-1604210038 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Jarkko, Andy, On Wed, 20 Apr 2016 17:16:00 +0300 Andy Shevchenko wrote: > On Wed, 2016-04-20 at 15:55 +0300, Jarkko Nikula wrote: > > On 04/14/2016 03:53 PM, Jisheng Zhang wrote: > > > > > > If i2c_dw_probe() fail, we should call i2c_dw_plat_prepare_clk() to > > > disable and unprepare the clk, otherwise the clk enable and prepare > > > is left unbalanced. > > > > > > Signed-off-by: Jisheng Zhang > > > --- > > >   drivers/i2c/busses/i2c-designware-platdrv.c | 2 ++ > > >   1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c > > > b/drivers/i2c/busses/i2c-designware-platdrv.c > > > index 00f9e99..1488cea 100644 > > > --- a/drivers/i2c/busses/i2c-designware-platdrv.c > > > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c > > > @@ -268,6 +268,8 @@ rpm_disable: > > >    pm_runtime_put_noidle(&pdev->dev); > > >    } > > > > > > + i2c_dw_plat_prepare_clk(dev, false); > > > + > > >    return r; > > >   } > > > > > This is a bit unclear to me does devm_clk_get take care of clk > > disabling  > > in case of probe error or driver removal? > > > > I see Andy's 1cb715ca4694 ("i2c-designware: move to managed functions  > > (devm_*)") removed it but at quick look drivers/clk/clk-devres.c:  > > devm_clk_release() calls only clk_put and I don't see disable is done  > > down the path. > > The following is a mistake of the mentioned patch. > -       clk_disable_unprepare(dev->clk); > > I did at the same mistake in dw_dmac driver which had been fixed later > in the commit 8be4f523b480 ("dmaengine: dw: fix regression in dw_probe() > function"). > As Andy pointed out, managed devm_clk_get can only automatically put clk but doesn't unprepare and disable the clk Thanks, Jisheng