RE: [PATCH] gianfar: Reduce logging noise seen due to phy polling if link is down

[claudiu.manoil@xxxxxxxxxxxxx]

> -----Original Message-----
> From: Guenter Roeck [mailto:linux@xxxxxxxxxxxx]
> Sent: Monday, March 02, 2015 10:03 PM
> 
[...]
> 
> Cc: Claudiu Manoil <claudiu.manoil@xxxxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
[...]
>  	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@xxxxxxxxxxxxx>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/