[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 3 Mar 2015 14:42:10 +0000
From: "claudiu.manoil@...escale.com" <claudiu.manoil@...escale.com>
To: Guenter Roeck <linux@...ck-us.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "David S. Miller" <davem@...emloft.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] gianfar: Reduce logging noise seen due to phy polling if
link is down
> -----Original Message-----
> From: Guenter Roeck [mailto:linux@...ck-us.net]
> Sent: Monday, March 02, 2015 10:03 PM
>
[...]
>
> Cc: Claudiu Manoil <claudiu.manoil@...escale.com>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
[...]
> if (unlikely(phydev->link != priv->oldlink ||
> - phydev->duplex != priv->oldduplex ||
> - phydev->speed != priv->oldspeed))
> + (phydev->link && (phydev->duplex != priv->oldduplex ||
> + phydev->speed != priv->oldspeed))))
> gfar_update_link_state(priv);
> }
I did a quick check and, indeed, phydev->duplex or phydev->speed
may change even if phydev->link is 0. I could reproduce the issue
with the following test for an eth with a polling mode phy:
1) initially link is up - autoneg on, speed 1000;
2) taking the link down - link down message printed only once;
3) turning autoneg to off for the same eth - link down message
is printed continuously;
So, given this, I agree that the driver needs this protection before
accessing the phydev->duplex/speed fields, namely to check first whether
phydev->link is 1. If link is 0, phydev->speed/duplex may be bogus.
Thanks for spotting this.
Reviewed-by: Claudiu Manoil <claudiu.manoil@...escale.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists