From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vaishali Thakkar Subject: Re: [PATCH] drm/msm: Move call to PTR_ERR_OR_ZERO after reassignment Date: Thu, 28 Apr 2016 18:53:27 +0530 Message-ID: <57220ECF.5000005@oracle.com> References: <1461756097-20799-1-git-send-email-vaishali.thakkar@oracle.com> <20160428125301.GY32731@imgtec.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69]) by gabe.freedesktop.org (Postfix) with ESMTPS id 63D906ECF1 for ; Thu, 28 Apr 2016 13:23:46 +0000 (UTC) In-Reply-To: <20160428125301.GY32731@imgtec.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Eric Engestrom Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org CgpPbiBUaHVyc2RheSAyOCBBcHJpbCAyMDE2IDA2OjIzIFBNLCBFcmljIEVuZ2VzdHJvbSB3cm90 ZToKPiBPbiBXZWQsIEFwciAyNywgMjAxNiBhdCAwNDo1MTozN1BNICswNTMwLCBWYWlzaGFsaSBU aGFra2FyIHdyb3RlOgo+PiBIZXJlLCBhIGxvY2F0aW9uIGlzIHJlc2V0IHRvIE5VTEwgYmVmb3Jl IGJlaW5nIHBhc3NlZCB0byBQVFJfRVJSLgo+PiBTbywgUFRSX0VSUiBzaG91bGQgYmUgY2FsbGVk IGJlZm9yZSBpdHMgYXJndW1lbnQgaXMgcmVhc3NpZ25lZAo+PiB0byBOVUxMLiBGdXJ0aGVyIHRv IHNpbXBsaWZ5IHRoaW5ncyB1c2UgUFRSX0VSUl9PUl9aRVJPIGluc3RlYWQKPj4gb2YgUFRSX0VS UiBhbmQgSVNfRVJSLgo+Pgo+PiBQcm9ibGVtIGZvdW5kIHVzaW5nIENvY2NpbmVsbGUuCj4+Cj4+ IFNpZ25lZC1vZmYtYnk6IFZhaXNoYWxpIFRoYWtrYXIgPHZhaXNoYWxpLnRoYWtrYXJAb3JhY2xl LmNvbT4KPj4gLS0tCj4+ICBkcml2ZXJzL2dwdS9kcm0vbXNtL2VkcC9lZHBfY3RybC5jIHwgMTYg KysrKysrKysrLS0tLS0tLQo+PiAgMSBmaWxlIGNoYW5nZWQsIDkgaW5zZXJ0aW9ucygrKSwgNyBk ZWxldGlvbnMoLSkKPj4KPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9tc20vZWRwL2Vk cF9jdHJsLmMgYi9kcml2ZXJzL2dwdS9kcm0vbXNtL2VkcC9lZHBfY3RybC5jCj4+IGluZGV4IDgx MjAwZTkuLjc4MTY1NDEgMTAwNjQ0Cj4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9tc20vZWRwL2Vk cF9jdHJsLmMKPj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL21zbS9lZHAvZWRwX2N0cmwuYwo+PiBA QCAtMzAyLDIxICszMDIsMjMgQEAgc3RhdGljIHZvaWQgZWRwX2Nsa19kaXNhYmxlKHN0cnVjdCBl ZHBfY3RybCAqY3RybCwgdTMyIGNsa19tYXNrKQo+PiAgc3RhdGljIGludCBlZHBfcmVndWxhdG9y X2luaXQoc3RydWN0IGVkcF9jdHJsICpjdHJsKQo+PiAgewo+PiAgCXN0cnVjdCBkZXZpY2UgKmRl diA9ICZjdHJsLT5wZGV2LT5kZXY7Cj4+ICsJaW50IHJldDsKPj4gIAo+PiAgCURCRygiIik7Cj4+ ICAJY3RybC0+dmRkYV92cmVnID0gZGV2bV9yZWd1bGF0b3JfZ2V0KGRldiwgInZkZGEiKTsKPj4g LQlpZiAoSVNfRVJSKGN0cmwtPnZkZGFfdnJlZykpIHsKPj4gKwlyZXQgPSBQVFJfRVJSX09SX1pF Uk8oY3RybC0+dmRkYV92cmVnKTsKPj4gKwlpZiAocmV0KSB7Cj4+ICAJCXByX2VycigiJXM6IENv dWxkIG5vdCBnZXQgdmRkYSByZWcsIHJldCA9ICVsZFxuIiwgX19mdW5jX18sCj4+IC0JCQkJUFRS X0VSUihjdHJsLT52ZGRhX3ZyZWcpKTsKPj4gKwkJCQlyZXQpOwo+PiAgCQljdHJsLT52ZGRhX3Zy ZWcgPSBOVUxMOwo+PiAtCQlyZXR1cm4gUFRSX0VSUihjdHJsLT52ZGRhX3ZyZWcpOwo+PiArCQly ZXR1cm4gcmV0Owo+PiAgCX0KPj4gIAljdHJsLT5sdmxfdnJlZyA9IGRldm1fcmVndWxhdG9yX2dl dChkZXYsICJsdmwtdmRkIik7Cj4+IC0JaWYgKElTX0VSUihjdHJsLT5sdmxfdnJlZykpIHsKPj4g LQkJcHJfZXJyKCJDb3VsZCBub3QgZ2V0IGx2bC12ZGQgcmVnLCAlbGQiLAo+PiAtCQkJCVBUUl9F UlIoY3RybC0+bHZsX3ZyZWcpKTsKPj4gKwlyZXQgPSBQVFJfRVJSX09SX1pFUk8oY3RybC0+bHZs X3ZyZWcpOwo+PiArCWlmIChyZXQpIHsKPiAKPiBXaGlsZSB5b3UncmUgcmV3cml0aW5nIHRoZW0s IGl0IG1pZ2h0IGJlIHdvcnRoIG1ha2luZyB0aGVzZSB0d28gcHJfZXJyKCkKPiBwcmludCB0aGUg c2FtZSBmb3JtYXQsIGUuZy46Cj4gCj4gCXByX2VycigiJXM6IENvdWxkIG5vdCBnZXQgbHZsLXZk ZCByZWcsIHJldCA9ICVsZFxuIiwgX19mdW5jX18sIHJldCk7CgpEb25lLCB0aGFua3MuCgo+PiAr CQlwcl9lcnIoIkNvdWxkIG5vdCBnZXQgbHZsLXZkZCByZWcsICVsZCIsIHJldCk7Cj4+ICAJCWN0 cmwtPmx2bF92cmVnID0gTlVMTDsKPj4gLQkJcmV0dXJuIFBUUl9FUlIoY3RybC0+bHZsX3ZyZWcp Owo+PiArCQlyZXR1cm4gcmV0Owo+PiAgCX0KPj4gIAo+PiAgCXJldHVybiAwOwo+PiAtLSAKPj4g Mi4xLjQKPiAKPiBSZXZpZXdlZC1ieTogRXJpYyBFbmdlc3Ryb20gPGVyaWMuZW5nZXN0cm9tQGlt Z3RlYy5jb20+Cj4gCgotLSAKVmFpc2hhbGkKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752751AbcD1NXt (ORCPT ); Thu, 28 Apr 2016 09:23:49 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:26935 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752507AbcD1NXr (ORCPT ); Thu, 28 Apr 2016 09:23:47 -0400 Subject: Re: [PATCH] drm/msm: Move call to PTR_ERR_OR_ZERO after reassignment To: Eric Engestrom References: <1461756097-20799-1-git-send-email-vaishali.thakkar@oracle.com> <20160428125301.GY32731@imgtec.com> Cc: airlied@linux.ie, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org From: Vaishali Thakkar Message-ID: <57220ECF.5000005@oracle.com> Date: Thu, 28 Apr 2016 18:53:27 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <20160428125301.GY32731@imgtec.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday 28 April 2016 06:23 PM, Eric Engestrom wrote: > On Wed, Apr 27, 2016 at 04:51:37PM +0530, Vaishali Thakkar wrote: >> Here, a location is reset to NULL before being passed to PTR_ERR. >> So, PTR_ERR should be called before its argument is reassigned >> to NULL. Further to simplify things use PTR_ERR_OR_ZERO instead >> of PTR_ERR and IS_ERR. >> >> Problem found using Coccinelle. >> >> Signed-off-by: Vaishali Thakkar >> --- >> drivers/gpu/drm/msm/edp/edp_ctrl.c | 16 +++++++++------- >> 1 file changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/edp/edp_ctrl.c b/drivers/gpu/drm/msm/edp/edp_ctrl.c >> index 81200e9..7816541 100644 >> --- a/drivers/gpu/drm/msm/edp/edp_ctrl.c >> +++ b/drivers/gpu/drm/msm/edp/edp_ctrl.c >> @@ -302,21 +302,23 @@ static void edp_clk_disable(struct edp_ctrl *ctrl, u32 clk_mask) >> static int edp_regulator_init(struct edp_ctrl *ctrl) >> { >> struct device *dev = &ctrl->pdev->dev; >> + int ret; >> >> DBG(""); >> ctrl->vdda_vreg = devm_regulator_get(dev, "vdda"); >> - if (IS_ERR(ctrl->vdda_vreg)) { >> + ret = PTR_ERR_OR_ZERO(ctrl->vdda_vreg); >> + if (ret) { >> pr_err("%s: Could not get vdda reg, ret = %ld\n", __func__, >> - PTR_ERR(ctrl->vdda_vreg)); >> + ret); >> ctrl->vdda_vreg = NULL; >> - return PTR_ERR(ctrl->vdda_vreg); >> + return ret; >> } >> ctrl->lvl_vreg = devm_regulator_get(dev, "lvl-vdd"); >> - if (IS_ERR(ctrl->lvl_vreg)) { >> - pr_err("Could not get lvl-vdd reg, %ld", >> - PTR_ERR(ctrl->lvl_vreg)); >> + ret = PTR_ERR_OR_ZERO(ctrl->lvl_vreg); >> + if (ret) { > > While you're rewriting them, it might be worth making these two pr_err() > print the same format, e.g.: > > pr_err("%s: Could not get lvl-vdd reg, ret = %ld\n", __func__, ret); Done, thanks. >> + pr_err("Could not get lvl-vdd reg, %ld", ret); >> ctrl->lvl_vreg = NULL; >> - return PTR_ERR(ctrl->lvl_vreg); >> + return ret; >> } >> >> return 0; >> -- >> 2.1.4 > > Reviewed-by: Eric Engestrom > -- Vaishali