* [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code
@ 2012-04-02 16:24 Srinivas KANDAGATLA
2012-04-02 16:42 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Srinivas KANDAGATLA @ 2012-04-02 16:24 UTC (permalink / raw)
To: netdev; +Cc: davem, srinivas.kandagatla
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Compile tested.
remove unnecessary code that matches this coccinelle pattern
ret = phy_write(x, y , z)
if (ret < 0)
return ret;
return 0;
As phy_write returns error code, we dont need to do not need extra check
before returning.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
drivers/net/phy/bcm63xx.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/bcm63xx.c b/drivers/net/phy/bcm63xx.c
index e16f98c..cd802eb 100644
--- a/drivers/net/phy/bcm63xx.c
+++ b/drivers/net/phy/bcm63xx.c
@@ -39,10 +39,7 @@ static int bcm63xx_config_init(struct phy_device *phydev)
MII_BCM63XX_IR_SPEED |
MII_BCM63XX_IR_LINK) |
MII_BCM63XX_IR_EN;
- err = phy_write(phydev, MII_BCM63XX_IR, reg);
- if (err < 0)
- return err;
- return 0;
+ return phy_write(phydev, MII_BCM63XX_IR, reg);
}
static int bcm63xx_ack_interrupt(struct phy_device *phydev)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code
2012-04-02 16:24 [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code Srinivas KANDAGATLA
@ 2012-04-02 16:42 ` Joe Perches
2012-04-02 16:51 ` Srinivas KANDAGATLA
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2012-04-02 16:42 UTC (permalink / raw)
To: Srinivas KANDAGATLA; +Cc: netdev, davem
On Mon, 2012-04-02 at 17:24 +0100, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> Compile tested.
> remove unnecessary code that matches this coccinelle pattern
>
> ret = phy_write(x, y , z)
> if (ret < 0)
> return ret;
> return 0;
>
> As phy_write returns error code, we dont need to do not need extra check
> before returning.
Do these really make any functional difference?
Doesn't the compiler generate the same output?
Many times, there's a code pattern that precedes these
calls has a similar pattern and changing the pattern
for the last call in a sequence can be jarring to a
reader and changing the pattern can sometimes introduce
errors as well.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code
2012-04-02 16:42 ` Joe Perches
@ 2012-04-02 16:51 ` Srinivas KANDAGATLA
2012-04-02 17:02 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Srinivas KANDAGATLA @ 2012-04-02 16:51 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, davem
On 02/04/12 17:42, Joe Perches wrote:
> On Mon, 2012-04-02 at 17:24 +0100, Srinivas KANDAGATLA wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>>
>> Compile tested.
>> remove unnecessary code that matches this coccinelle pattern
>>
>> ret = phy_write(x, y , z)
>> if (ret < 0)
>> return ret;
>> return 0;
>>
>> As phy_write returns error code, we dont need to do not need extra check
>> before returning.
>
> Do these really make any functional difference?
No it does not make any functional difference.
> Doesn't the compiler generate the same output?
>
I think it will not generate same output.
> Many times, there's a code pattern that precedes these
> calls has a similar pattern and changing the pattern
> for the last call in a sequence can be jarring to a
> reader and changing the pattern can sometimes introduce
> errors as well.
There is a purpose(error handling) of having similar pattern for the
code above last call, However there is no value for doing an additional
check before returning.
If we look at other phy files(ex:boardcom.c..), we can see they do
something similar to what the patch does in config_init.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code
2012-04-02 16:51 ` Srinivas KANDAGATLA
@ 2012-04-02 17:02 ` Joe Perches
2012-04-03 9:49 ` Srinivas KANDAGATLA
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2012-04-02 17:02 UTC (permalink / raw)
To: srinivas.kandagatla; +Cc: netdev, davem
On Mon, 2012-04-02 at 17:51 +0100, Srinivas KANDAGATLA wrote:
> On 02/04/12 17:42, Joe Perches wrote:
> > On Mon, 2012-04-02 at 17:24 +0100, Srinivas KANDAGATLA wrote:
> > Do these really make any functional difference?
> No it does not make any functional difference.
> > Doesn't the compiler generate the same output?
> I think it will not generate same output.
Yes, you are right. The code currently returns 0 for
non-error cases and you've changed it to possibly
return a positive value.
Have you checked all the callers to make sure this
doesn't introduce a new defect?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code
2012-04-02 17:02 ` Joe Perches
@ 2012-04-03 9:49 ` Srinivas KANDAGATLA
2012-04-03 23:02 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Srinivas KANDAGATLA @ 2012-04-03 9:49 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, davem
Yes, I have audited all the drivers. none of them return positive values.
In general, I think phy_write should return either 0 or an error code
and *NOT* positive values.
Any mdio bus driver returning positive values for phy_writes will break
the phylib too.
On 02/04/12 18:02, Joe Perches wrote:
> On Mon, 2012-04-02 at 17:51 +0100, Srinivas KANDAGATLA wrote:
>> On 02/04/12 17:42, Joe Perches wrote:
>>> On Mon, 2012-04-02 at 17:24 +0100, Srinivas KANDAGATLA wrote:
>>> Do these really make any functional difference?
>> No it does not make any functional difference.
>>> Doesn't the compiler generate the same output?
>> I think it will not generate same output.
> Yes, you are right. The code currently returns 0 for
> non-error cases and you've changed it to possibly
> return a positive value.
>
> Have you checked all the callers to make sure this
> doesn't introduce a new defect?
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code
2012-04-03 9:49 ` Srinivas KANDAGATLA
@ 2012-04-03 23:02 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-04-03 23:02 UTC (permalink / raw)
To: srinivas.kandagatla; +Cc: joe, netdev
From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
Date: Tue, 03 Apr 2012 10:49:41 +0100
> Yes, I have audited all the drivers. none of them return positive values.
> In general, I think phy_write should return either 0 or an error code
> and *NOT* positive values.
> Any mdio bus driver returning positive values for phy_writes will break
> the phylib too.
I've applied all three patches to net-next, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-03 23:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-02 16:24 [PATCH 3.3.0 1/3] net:phy:bcm63xx: remove unnecessary code Srinivas KANDAGATLA
2012-04-02 16:42 ` Joe Perches
2012-04-02 16:51 ` Srinivas KANDAGATLA
2012-04-02 17:02 ` Joe Perches
2012-04-03 9:49 ` Srinivas KANDAGATLA
2012-04-03 23:02 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).