From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko =?utf-8?q?St=C3=BCbner?=) Date: Thu, 4 Jul 2013 16:34:10 +0200 Subject: [PATCH v2 1/6] misc: sram: fix error path in sram_probe In-Reply-To: <1372151074.3981.21.camel@pizza.hi.pengutronix.de> References: <201306251046.04873.heiko@sntech.de> <201306251046.38993.heiko@sntech.de> <1372151074.3981.21.camel@pizza.hi.pengutronix.de> Message-ID: <201307041634.11248.heiko@sntech.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Philipp, Am Dienstag, 25. Juni 2013, 11:04:34 schrieb Philipp Zabel: > Hi Heiko, > > Am Dienstag, den 25.06.2013, 10:46 +0200 schrieb Heiko St?bner: > > The pool is created thru devm_gen_pool_create, so the call to > > gen_pool_destroy is not necessary. > > Instead the sram-clock must be turned off again if it exists. > > > > Signed-off-by: Heiko Stuebner > > --- > > > > drivers/misc/sram.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c > > index d87cc91..afe66571 100644 > > --- a/drivers/misc/sram.c > > +++ b/drivers/misc/sram.c > > @@ -68,7 +68,8 @@ static int sram_probe(struct platform_device *pdev) > > > > ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base, > > > > res->start, size, -1); > > > > if (ret < 0) { > > > > - gen_pool_destroy(sram->pool); > > Right, thanks. > > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > > > return ret; > > > > } > > In light of the following patch, I'd rather move the > clk_prepare_enable() call after gen_pool_add_virt() and its error path. I'm not sure, but isn't moving the clock enablement below the pool allocation producing a race condition? I.e. can the case happen that some other part wants to allocate part of the newly generated pool already, while the subsequent gen_pool_add_virt calls from the following patch are still running? ... And what will happen in this case, when the sram clock is still disabled? Thanks Heiko From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko =?utf-8?q?St=C3=BCbner?= Subject: Re: [PATCH v2 1/6] misc: sram: fix error path in sram_probe Date: Thu, 4 Jul 2013 16:34:10 +0200 Message-ID: <201307041634.11248.heiko@sntech.de> References: <201306251046.04873.heiko@sntech.de> <201306251046.38993.heiko@sntech.de> <1372151074.3981.21.camel@pizza.hi.pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1372151074.3981.21.camel-/rZezPiN1rtR6QfukMTsflXZhhPuCNm+@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Philipp Zabel Cc: Russell King , Greg Kroah-Hartman , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Grant Likely , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: devicetree@vger.kernel.org SGkgUGhpbGlwcCwKCkFtIERpZW5zdGFnLCAyNS4gSnVuaSAyMDEzLCAxMTowNDozNCBzY2hyaWVi IFBoaWxpcHAgWmFiZWw6Cj4gSGkgSGVpa28sCj4gCj4gQW0gRGllbnN0YWcsIGRlbiAyNS4wNi4y MDEzLCAxMDo0NiArMDIwMCBzY2hyaWViIEhlaWtvIFN0w7xibmVyOgo+ID4gVGhlIHBvb2wgaXMg Y3JlYXRlZCB0aHJ1IGRldm1fZ2VuX3Bvb2xfY3JlYXRlLCBzbyB0aGUgY2FsbCB0bwo+ID4gZ2Vu X3Bvb2xfZGVzdHJveSBpcyBub3QgbmVjZXNzYXJ5Lgo+ID4gSW5zdGVhZCB0aGUgc3JhbS1jbG9j ayBtdXN0IGJlIHR1cm5lZCBvZmYgYWdhaW4gaWYgaXQgZXhpc3RzLgo+ID4gCj4gPiBTaWduZWQt b2ZmLWJ5OiBIZWlrbyBTdHVlYm5lciA8aGVpa29Ac250ZWNoLmRlPgo+ID4gLS0tCj4gPiAKPiA+ ICBkcml2ZXJzL21pc2Mvc3JhbS5jIHwgICAgMyArKy0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgMiBp bnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJz L21pc2Mvc3JhbS5jIGIvZHJpdmVycy9taXNjL3NyYW0uYwo+ID4gaW5kZXggZDg3Y2M5MS4uYWZl NjY1NzEgMTAwNjQ0Cj4gPiAtLS0gYS9kcml2ZXJzL21pc2Mvc3JhbS5jCj4gPiArKysgYi9kcml2 ZXJzL21pc2Mvc3JhbS5jCj4gPiBAQCAtNjgsNyArNjgsOCBAQCBzdGF0aWMgaW50IHNyYW1fcHJv YmUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldikKPiA+IAo+ID4gIAlyZXQgPSBnZW5fcG9v bF9hZGRfdmlydChzcmFtLT5wb29sLCAodW5zaWduZWQgbG9uZyl2aXJ0X2Jhc2UsCj4gPiAgCQo+ ID4gIAkJCQlyZXMtPnN0YXJ0LCBzaXplLCAtMSk7Cj4gPiAgCQo+ID4gIAlpZiAocmV0IDwgMCkg ewo+ID4gCj4gPiAtCQlnZW5fcG9vbF9kZXN0cm95KHNyYW0tPnBvb2wpOwo+IAo+IFJpZ2h0LCB0 aGFua3MuCj4gCj4gPiArCQlpZiAoc3JhbS0+Y2xrKQo+ID4gKwkJCWNsa19kaXNhYmxlX3VucHJl cGFyZShzcmFtLT5jbGspOwo+ID4gCj4gPiAgCQlyZXR1cm4gcmV0Owo+ID4gIAkKPiA+ICAJfQo+ IAo+IEluIGxpZ2h0IG9mIHRoZSBmb2xsb3dpbmcgcGF0Y2gsIEknZCByYXRoZXIgbW92ZSB0aGUK PiBjbGtfcHJlcGFyZV9lbmFibGUoKSBjYWxsIGFmdGVyIGdlbl9wb29sX2FkZF92aXJ0KCkgYW5k IGl0cyBlcnJvciBwYXRoLgoKSSdtIG5vdCBzdXJlLCBidXQgaXNuJ3QgbW92aW5nIHRoZSBjbG9j ayBlbmFibGVtZW50IGJlbG93IHRoZSBwb29sIGFsbG9jYXRpb24gCnByb2R1Y2luZyBhIHJhY2Ug Y29uZGl0aW9uPwoKSS5lLiBjYW4gdGhlIGNhc2UgaGFwcGVuIHRoYXQgc29tZSBvdGhlciBwYXJ0 IHdhbnRzIHRvIGFsbG9jYXRlIHBhcnQgb2YgdGhlIApuZXdseSBnZW5lcmF0ZWQgcG9vbCBhbHJl YWR5LCB3aGlsZSB0aGUgc3Vic2VxdWVudCBnZW5fcG9vbF9hZGRfdmlydCBjYWxscyAKZnJvbSB0 aGUgZm9sbG93aW5nIHBhdGNoIGFyZSBzdGlsbCBydW5uaW5nPyAuLi4gQW5kIHdoYXQgd2lsbCBo YXBwZW4gaW4gdGhpcyAKY2FzZSwgd2hlbiB0aGUgc3JhbSBjbG9jayBpcyBzdGlsbCBkaXNhYmxl ZD8KCgpUaGFua3MKSGVpa28KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18KZGV2aWNldHJlZS1kaXNjdXNzIG1haWxpbmcgbGlzdApkZXZpY2V0cmVlLWRpc2N1 c3NAbGlzdHMub3psYWJzLm9yZwpodHRwczovL2xpc3RzLm96bGFicy5vcmcvbGlzdGluZm8vZGV2 aWNldHJlZS1kaXNjdXNzCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756779Ab3GDOea (ORCPT ); Thu, 4 Jul 2013 10:34:30 -0400 Received: from gloria.sntech.de ([95.129.55.99]:52960 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756202Ab3GDOe3 (ORCPT ); Thu, 4 Jul 2013 10:34:29 -0400 From: Heiko =?utf-8?q?St=C3=BCbner?= To: Philipp Zabel Subject: Re: [PATCH v2 1/6] misc: sram: fix error path in sram_probe Date: Thu, 4 Jul 2013 16:34:10 +0200 User-Agent: KMail/1.13.7 (Linux/3.2.0-3-686-pae; KDE/4.8.4; i686; ; ) Cc: Arnd Bergmann , Olof Johansson , "linux-arm-kernel@lists.infradead.org" , Grant Likely , Rob Herring , devicetree-discuss@lists.ozlabs.org, Russell King , linux-kernel@vger.kernel.org, "Greg Kroah-Hartman" References: <201306251046.04873.heiko@sntech.de> <201306251046.38993.heiko@sntech.de> <1372151074.3981.21.camel@pizza.hi.pengutronix.de> In-Reply-To: <1372151074.3981.21.camel@pizza.hi.pengutronix.de> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 8bit Message-Id: <201307041634.11248.heiko@sntech.de> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Philipp, Am Dienstag, 25. Juni 2013, 11:04:34 schrieb Philipp Zabel: > Hi Heiko, > > Am Dienstag, den 25.06.2013, 10:46 +0200 schrieb Heiko Stübner: > > The pool is created thru devm_gen_pool_create, so the call to > > gen_pool_destroy is not necessary. > > Instead the sram-clock must be turned off again if it exists. > > > > Signed-off-by: Heiko Stuebner > > --- > > > > drivers/misc/sram.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c > > index d87cc91..afe66571 100644 > > --- a/drivers/misc/sram.c > > +++ b/drivers/misc/sram.c > > @@ -68,7 +68,8 @@ static int sram_probe(struct platform_device *pdev) > > > > ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base, > > > > res->start, size, -1); > > > > if (ret < 0) { > > > > - gen_pool_destroy(sram->pool); > > Right, thanks. > > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > > > return ret; > > > > } > > In light of the following patch, I'd rather move the > clk_prepare_enable() call after gen_pool_add_virt() and its error path. I'm not sure, but isn't moving the clock enablement below the pool allocation producing a race condition? I.e. can the case happen that some other part wants to allocate part of the newly generated pool already, while the subsequent gen_pool_add_virt calls from the following patch are still running? ... And what will happen in this case, when the sram clock is still disabled? Thanks Heiko