From mboxrd@z Thu Jan 1 00:00:00 1970 From: walter harms Date: Sun, 26 Feb 2017 09:03:06 +0000 Subject: Re: [PATCH] drm/msm/dsi: Fix the releasing of resources in error path in 'dsi_bus_clk_enable()' Message-Id: <58B299CA.4060203@bfs.de> List-Id: References: <20170226075206.25762-1-christophe.jaillet@wanadoo.fr> In-Reply-To: <20170226075206.25762-1-christophe.jaillet@wanadoo.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Christophe JAILLET Cc: freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, kernel-janitors@vger.kernel.org, yongjun_wei@trendmicro.com.cn, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Am 26.02.2017 08:52, schrieb Christophe JAILLET: > If a 'clk_prepare_enable()' fails, then we need to disable_unprepare the > clk already handled. > > With the current implemenatation, we try to do that on the clk that has > triggered the error, which is a no-op, and leave 'msm_host->bus_clks[0]' > untouched. > > Shift by one the index array to free resources correctly. > > Signed-off-by: Christophe JAILLET > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 1fc07ce24686..eac4987f3946 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -438,7 +438,7 @@ static int dsi_bus_clk_enable(struct msm_dsi_host *msm_host) > return 0; > err: > for (; i > 0; i--) > - clk_disable_unprepare(msm_host->bus_clks[i]); > + clk_disable_unprepare(msm_host->bus_clks[i-1]); > > return ret; > } i guess it is technical correct but programmes are very bad at counting backwards so its more future proof to do something like: for (j=0;jbus_clks[j]); re, wh From mboxrd@z Thu Jan 1 00:00:00 1970 From: walter harms Subject: Re: [PATCH] drm/msm/dsi: Fix the releasing of resources in error path in 'dsi_bus_clk_enable()' Date: Sun, 26 Feb 2017 10:03:06 +0100 Message-ID: <58B299CA.4060203@bfs.de> References: <20170226075206.25762-1-christophe.jaillet@wanadoo.fr> Reply-To: wharms@bfs.de Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170226075206.25762-1-christophe.jaillet@wanadoo.fr> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Christophe JAILLET Cc: freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, kernel-janitors@vger.kernel.org, yongjun_wei@trendmicro.com.cn, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org CgpBbSAyNi4wMi4yMDE3IDA4OjUyLCBzY2hyaWViIENocmlzdG9waGUgSkFJTExFVDoKPiBJZiBh ICdjbGtfcHJlcGFyZV9lbmFibGUoKScgZmFpbHMsIHRoZW4gd2UgbmVlZCB0byBkaXNhYmxlX3Vu cHJlcGFyZSB0aGUKPiBjbGsgYWxyZWFkeSBoYW5kbGVkLgo+IAo+IFdpdGggdGhlIGN1cnJlbnQg aW1wbGVtZW5hdGF0aW9uLCB3ZSB0cnkgdG8gZG8gdGhhdCBvbiB0aGUgY2xrIHRoYXQgaGFzCj4g dHJpZ2dlcmVkIHRoZSBlcnJvciwgd2hpY2ggaXMgYSBuby1vcCwgYW5kIGxlYXZlICdtc21faG9z dC0+YnVzX2Nsa3NbMF0nCj4gdW50b3VjaGVkLgo+IAo+IFNoaWZ0IGJ5IG9uZSB0aGUgaW5kZXgg YXJyYXkgdG8gZnJlZSByZXNvdXJjZXMgY29ycmVjdGx5Lgo+IAo+IFNpZ25lZC1vZmYtYnk6IENo cmlzdG9waGUgSkFJTExFVCA8Y2hyaXN0b3BoZS5qYWlsbGV0QHdhbmFkb28uZnI+Cj4gLS0tCj4g IGRyaXZlcnMvZ3B1L2RybS9tc20vZHNpL2RzaV9ob3N0LmMgfCAyICstCj4gIDEgZmlsZSBjaGFu Z2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQo+IAo+IGRpZmYgLS1naXQgYS9kcml2 ZXJzL2dwdS9kcm0vbXNtL2RzaS9kc2lfaG9zdC5jIGIvZHJpdmVycy9ncHUvZHJtL21zbS9kc2kv ZHNpX2hvc3QuYwo+IGluZGV4IDFmYzA3Y2UyNDY4Ni4uZWFjNDk4N2YzOTQ2IDEwMDY0NAo+IC0t LSBhL2RyaXZlcnMvZ3B1L2RybS9tc20vZHNpL2RzaV9ob3N0LmMKPiArKysgYi9kcml2ZXJzL2dw dS9kcm0vbXNtL2RzaS9kc2lfaG9zdC5jCj4gQEAgLTQzOCw3ICs0MzgsNyBAQCBzdGF0aWMgaW50 IGRzaV9idXNfY2xrX2VuYWJsZShzdHJ1Y3QgbXNtX2RzaV9ob3N0ICptc21faG9zdCkKPiAgCXJl dHVybiAwOwo+ICBlcnI6Cj4gIAlmb3IgKDsgaSA+IDA7IGktLSkKPiAtCQljbGtfZGlzYWJsZV91 bnByZXBhcmUobXNtX2hvc3QtPmJ1c19jbGtzW2ldKTsKPiArCQljbGtfZGlzYWJsZV91bnByZXBh cmUobXNtX2hvc3QtPmJ1c19jbGtzW2ktMV0pOwo+ICAKPiAgCXJldHVybiByZXQ7Cj4gIH0KCmkg Z3Vlc3MgaXQgaXMgdGVjaG5pY2FsIGNvcnJlY3QgYnV0IHByb2dyYW1tZXMgYXJlCnZlcnkgYmFk IGF0IGNvdW50aW5nIGJhY2t3YXJkcyBzbyBpdHMgbW9yZSBmdXR1cmUgcHJvb2YgdG8KZG8gc29t ZXRoaW5nIGxpa2U6Cgpmb3IgKGo9MDtqPGk7aisrKQoJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKG1z bV9ob3N0LT5idXNfY2xrc1tqXSk7CgpyZSwKIHdoCgoKCgpfX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZl bEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFp bG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752067AbdBZJFq (ORCPT ); Sun, 26 Feb 2017 04:05:46 -0500 Received: from mx02-sz.bfs.de ([194.94.69.103]:43976 "EHLO mx02-sz.bfs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751983AbdBZJFg (ORCPT ); Sun, 26 Feb 2017 04:05:36 -0500 Message-ID: <58B299CA.4060203@bfs.de> Date: Sun, 26 Feb 2017 10:03:06 +0100 From: walter harms Reply-To: wharms@bfs.de User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; de; rv:1.9.1.16) Gecko/20101125 SUSE/3.0.11 Thunderbird/3.0.11 MIME-Version: 1.0 To: Christophe JAILLET CC: robdclark@gmail.com, airlied@linux.ie, architt@codeaurora.org, hali@codeaurora.org, yongjun_wei@trendmicro.com.cn, linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH] drm/msm/dsi: Fix the releasing of resources in error path in 'dsi_bus_clk_enable()' References: <20170226075206.25762-1-christophe.jaillet@wanadoo.fr> In-Reply-To: <20170226075206.25762-1-christophe.jaillet@wanadoo.fr> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 26.02.2017 08:52, schrieb Christophe JAILLET: > If a 'clk_prepare_enable()' fails, then we need to disable_unprepare the > clk already handled. > > With the current implemenatation, we try to do that on the clk that has > triggered the error, which is a no-op, and leave 'msm_host->bus_clks[0]' > untouched. > > Shift by one the index array to free resources correctly. > > Signed-off-by: Christophe JAILLET > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c > index 1fc07ce24686..eac4987f3946 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -438,7 +438,7 @@ static int dsi_bus_clk_enable(struct msm_dsi_host *msm_host) > return 0; > err: > for (; i > 0; i--) > - clk_disable_unprepare(msm_host->bus_clks[i]); > + clk_disable_unprepare(msm_host->bus_clks[i-1]); > > return ret; > } i guess it is technical correct but programmes are very bad at counting backwards so its more future proof to do something like: for (j=0;jbus_clks[j]); re, wh