On Mon, Sep 5, 2011 at 12:24 PM, Vladimir Zapolskiy vz@mleia.com wrote:
This change slightly improves readability of the phydev speed/duplex assignment logic.
Shoot, I just saw this patch in my tree. It's incorrect.
@@ -318,13 +318,10 @@ static int genphy_parse_link(struct phy_device *phydev) lpa = phy_read(phydev, MDIO_DEVAD_NONE, MII_ADVERTISE); lpa &= phy_read(phydev, MDIO_DEVAD_NONE, MII_LPA);
- if (lpa & (LPA_100FULL | LPA_100HALF)) {
- if (lpa & (LPA_100FULL | LPA_100HALF))
phydev->speed = SPEED_100;
- if (lpa & LPA_100FULL)
- phydev->duplex = DUPLEX_FULL;
- } else if (lpa & LPA_10FULL)
- if (lpa & (LPA_100FULL | LPA_10FULL))
phydev->duplex = DUPLEX_FULL;
The lines weren't redundant. The logic is (and probably should be better commented):
Find the intersection of the advertised capabilities of both sides of the link (lpa)
From that intersection, find the highest capability we can run at
(that will be the negotiated link)
Now imagine that the intersection (lpa) is (LPA_100HALF | LPA_10FULL).
The code will now set phydev->speed to 100, and phydev->duplex to 1, but this link does not support 100FULL.
Andy