From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932190AbdJVUP6 (ORCPT ); Sun, 22 Oct 2017 16:15:58 -0400 Received: from mout.web.de ([212.227.17.12]:52742 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932086AbdJVUP4 (ORCPT ); Sun, 22 Oct 2017 16:15:56 -0400 Subject: Re: [PATCH 1/2] extcon: max14577: Use common error handling code in max14577_muic_set_path() To: Krzysztof Kozlowski , kernel-janitors@vger.kernel.org Cc: Bartlomiej Zolnierkiewicz , Chanwoo Choi , MyungJoo Ham , LKML References: <5cdefd84-066d-5379-43c8-76f5d3b8f7c4@users.sourceforge.net> <20171022185941.mzplggaick742ylf@kozik-lap> From: SF Markus Elfring Message-ID: Date: Sun, 22 Oct 2017 22:15:42 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171022185941.mzplggaick742ylf@kozik-lap> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K0:tCBsypW6xuY/toIm4PRgxhVVqTQmBYUHyisdF65p/o9GEnulbdO 0MbdfanPW0WrXF6PJweQq7hUOSh3m00mJ50Q2vKTRgwGsmluM3NU4ayao2ikuK96LmKFj6h c9y3MefTwFSUxmW1HcM2bQXRBfyh8ua8ac4XQ5wTcnRrjsurY6rDWwmFZfb5N31yOljsD8k di0u8YcLgD9jPlJzCP7uQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:CAfOe1C2riQ=:/tmgmFDRbZ9TSScGJ16RKL 2Nl4YVgQohlL1035bxmiwym3129Xmrq10PMWcPvhgIBkTzKzAgU87xgIRtiYG4swdeFfsM7va ufhl80h0e3JXP/q/4lbzcCE2SHCbxtF2/lRokl2adlaG193ltCYpx0QSXuQyWLBPmuZ1x0S1o O/WJMSqLQuoRNM4PABJhyYzxnZMsv1bZlweznvxI+HuiEfZcPjAlW6WoPTUlf1P4UEN/J9lcc JEs2RFAdAFW0Hmxwz8TGiZGuUFxG92Poxr6bp1fj7yHUO5fuipMkB74TIPEjVjcqsleVZmp1V 5oukckZGV4E4mWJy2eCaud3xmwNa4N2TSgMgirtn1D4qseDBjifZJzMYrO6CIGBVTgMxg2xy6 zC29NBqLXIbiiBLjrt+HmVMPROWVCCp97rtvbG/O5fTML0MPnOA9l7Bflg1x9SE+rA9oDbB+m xmm5ekcKBbPwHmvQhTfh8yb41tG3e/R1Ei8qBN3NgrRTRWHun8tibbtdqEcYpTwGQxNthrQRG ASooGPNjUnBaAr75FUUjTuJqW8B4qDOROOOCfY4iI0xrFVBtPXb31R/g1Ondydk/aalLkxna/ qB0ex1Rkrk125yKnUCNIFDkN512XweK9dzgM2fW4xnhIbXJezLS8qDh7e4mzdYWObuZvJ3Fb6 /qm/2NtZF+AOXu05RqswlBkahdO8DN1JZyGUZGUGh6TOmSMKc2HsTIcTu/TabkypKcNUTyVjl QyUIuboXxB2M5pXHkUsEaJGUK3dII97LvBRNmRdoWMIG6yX/jpfx3OLUDV9IK4JacXO/7UmNf gjyJdhZTjW3aTG/Fe7Fa5S/I9VocDGA/BZl2U0iBFuKi2KOtw8= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> From: Markus Elfring >> Date: Sun, 22 Oct 2017 19:33:58 +0200 >> >> Add a jump target so that a bit of exception handling can be better reused >> at the end of this function. >> >> This issue was detected by using the Coccinelle software. >> >> Signed-off-by: Markus Elfring >> --- >> drivers/extcon/extcon-max14577.c | 22 ++++++++++------------ >> 1 file changed, 10 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c >> index f6414b7fa5bc..3d4bf5d23236 100644 >> --- a/drivers/extcon/extcon-max14577.c >> +++ b/drivers/extcon/extcon-max14577.c >> @@ -211,10 +211,8 @@ static int max14577_muic_set_path(struct max14577_muic_info *info, >> ret = max14577_update_reg(info->max14577->regmap, >> MAX14577_MUIC_REG_CONTROL1, >> CLEAR_IDBEN_MICEN_MASK, CTRL1_SW_OPEN); >> - if (ret < 0) { >> - dev_err(info->dev, "failed to update MUIC register\n"); >> - return ret; >> - } >> + if (ret < 0) >> + goto report_failure; > > No, one exit path just to report error does not seem to be more readable. I got an other development opinion on this aspect. > Instead printing error after the register access looks to me > as common pattern, easy to maintain. Do you care if corresponding messages are different? > This patch does not bring improvement, in my opinion. How do you generally think about the change possibility for a bit of code reduction? >> >> if (attached) >> ctrl1 = val; >> @@ -224,10 +222,8 @@ static int max14577_muic_set_path(struct max14577_muic_info *info, >> ret = max14577_update_reg(info->max14577->regmap, >> MAX14577_MUIC_REG_CONTROL1, >> CLEAR_IDBEN_MICEN_MASK, ctrl1); >> - if (ret < 0) { >> - dev_err(info->dev, "failed to update MUIC register\n"); >> - return ret; >> - } >> + if (ret < 0) >> + goto report_failure; >> >> if (attached) >> ctrl2 |= CTRL2_CPEN_MASK; /* LowPwr=0, CPEn=1 */ >> @@ -237,16 +233,18 @@ static int max14577_muic_set_path(struct max14577_muic_info *info, >> ret = max14577_update_reg(info->max14577->regmap, >> MAX14577_REG_CONTROL2, >> CTRL2_LOWPWR_MASK | CTRL2_CPEN_MASK, ctrl2); >> - if (ret < 0) { >> - dev_err(info->dev, "failed to update MUIC register\n"); >> - return ret; >> - } >> + if (ret < 0) >> + goto report_failure; >> >> dev_dbg(info->dev, >> "CONTROL1 : 0x%02x, CONTROL2 : 0x%02x, state : %s\n", >> ctrl1, ctrl2, attached ? "attached" : "detached"); >> >> return 0; >> + >> +report_failure: >> + dev_err(info->dev, "failed to update MUIC register\n"); >> + return ret; >> } >> >> /* >> -- >> 2.14.2 >> Would you like to take another look at remaining open issues in source files from the pattern “extcon-max…”? Regards, Markus