From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nithin Nayak Sujir Date: Thu, 06 Feb 2014 00:10:23 +0000 Subject: Re: [patch] tg3: cleanup an error path in tg3_phy_reset_5703_4_5() Message-Id: <52F2D2EF.2010508@broadcom.com> List-Id: References: <20140205132920.GE800@elgon.mountain> In-Reply-To: <20140205132920.GE800@elgon.mountain> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter Cc: Michael Chan , netdev@vger.kernel.org, kernel-janitors@vger.kernel.org On 02/05/2014 05:29 AM, Dan Carpenter wrote: > In the original code, if tg3_readphy() fails then it does an unnecessary > check to verify "err" is still zero and then returns -EBUSY. > > My static checker complains about the unnecessary "if (!err)" check and > anyway it is better to propagate the -EBUSY error code from > tg3_readphy() instead of hard coding it here. And really the original > code is confusing to look at. > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index e2ca03e23dc1..c466e12b601e 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -2609,13 +2609,14 @@ static int tg3_phy_reset_5703_4_5(struct tg3 *tp) > > tg3_writephy(tp, MII_CTRL1000, phy9_orig); > > - if (!tg3_readphy(tp, MII_TG3_EXT_CTRL, ®32)) { > - reg32 &= ~0x3000; > - tg3_writephy(tp, MII_TG3_EXT_CTRL, reg32); > - } else if (!err) > - err = -EBUSY; > + err = tg3_readphy(tp, MII_TG3_EXT_CTRL, ®32); > + if (err) > + return err; > > - return err; > + reg32 &= ~0x3000; > + tg3_writephy(tp, MII_TG3_EXT_CTRL, reg32); > + > + return 0; > } > > static void tg3_carrier_off(struct tg3 *tp) > Acked-by: Nithin Nayak Sujir From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nithin Nayak Sujir Subject: Re: [patch] tg3: cleanup an error path in tg3_phy_reset_5703_4_5() Date: Wed, 5 Feb 2014 16:10:23 -0800 Message-ID: <52F2D2EF.2010508@broadcom.com> References: <20140205132920.GE800@elgon.mountain> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Cc: Michael Chan , , To: Dan Carpenter Return-path: Received: from mail-gw1-out.broadcom.com ([216.31.210.62]:50432 "EHLO mail-gw1-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751192AbaBFAKo (ORCPT ); Wed, 5 Feb 2014 19:10:44 -0500 In-Reply-To: <20140205132920.GE800@elgon.mountain> Sender: netdev-owner@vger.kernel.org List-ID: On 02/05/2014 05:29 AM, Dan Carpenter wrote: > In the original code, if tg3_readphy() fails then it does an unnecessary > check to verify "err" is still zero and then returns -EBUSY. > > My static checker complains about the unnecessary "if (!err)" check and > anyway it is better to propagate the -EBUSY error code from > tg3_readphy() instead of hard coding it here. And really the original > code is confusing to look at. > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index e2ca03e23dc1..c466e12b601e 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -2609,13 +2609,14 @@ static int tg3_phy_reset_5703_4_5(struct tg3 *tp) > > tg3_writephy(tp, MII_CTRL1000, phy9_orig); > > - if (!tg3_readphy(tp, MII_TG3_EXT_CTRL, ®32)) { > - reg32 &= ~0x3000; > - tg3_writephy(tp, MII_TG3_EXT_CTRL, reg32); > - } else if (!err) > - err = -EBUSY; > + err = tg3_readphy(tp, MII_TG3_EXT_CTRL, ®32); > + if (err) > + return err; > > - return err; > + reg32 &= ~0x3000; > + tg3_writephy(tp, MII_TG3_EXT_CTRL, reg32); > + > + return 0; > } > > static void tg3_carrier_off(struct tg3 *tp) > Acked-by: Nithin Nayak Sujir