> -----Original Message----- > From: Guenter Roeck [mailto:[email protected]] > Sent: Monday, March 02, 2015 10:03 PM > [...] > > Cc: Claudiu Manoil <[email protected]> > Signed-off-by: Guenter Roeck <[email protected]> > --- [...] > 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 <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
