Hi Andy, > Extends the mii_dev structure to participate in a full-blown MDIO and > PHY driver scheme. The mii_dev structure and miiphy calls are modified > in such a way to allow the original mii command and miiphy > infrastructure to work as before, but also to support a new set of APIs > which allow (among other things) sharing of PHY driver code and 10G support > > The mii command will continue to support normal PHY management functions > (Clause 22 of 802.3), but will not be changed to support 10G > (Clause 45). > > The basic design is similar to PHY Lib from Linux, but simplified for > U-Boot's network and driver infrastructure. > > We now have MDIO drivers and PHY drivers > > An MDIO driver provides: > read > write > reset > > A PHY driver provides: > (optionally): probe > config - initial setup, starting of auto-negotiation > startup - waiting for AN, and reading link state > shutdown - any cleanup needed > > The ethernet drivers interact with the PHY Lib using these functions: > phy_connect() > phy_config() > phy_startup() > phy_shutdown() > > Each PHY driver can be configured separately, or all at once using > phylib_all_drivers.h (added in the patch which adds the drivers) > > Signed-off-by: Andy Fleming <aflem...@freescale.com>
Looks really good - a few comments: [...] > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > new file mode 100644 > index 0000000..ca35404 > --- /dev/null > +++ b/drivers/net/phy/phy.c > @@ -0,0 +1,705 @@ > +/* > + * Generic PHY Management code > + * > + * This software may be used and distributed according to the > + * terms of the GNU Public License, Version 2, incorporated > + * herein by reference. > + * > + * Copyright 2011 Freescale Semiconductor, Inc. > + * author Andy Fleming > + * > + * Based loosely off of Linux's PHY Lib According to http://www.denx.de/wiki/U-Boot/Patches new files need GPLv2+ headers and if I check Linux, I see GPLv2+ in drivers/net/phy/phy.c (and other people have contributed under this), so please change accrodingly. > +/** > + * genphy_update_link - update link status in @phydev > + * @phydev: target phy_device struct > + * > + * Description: Update the value in phydev->link to reflect the > + * current link value. In order to do this, we need to read > + * the status register twice, keeping the second value. > + */ > +int genphy_update_link(struct phy_device *phydev) > +{ > + unsigned int mii_reg; > + > + /* > + * Wait if the link is up, and autonegotiation is in progress > + * (ie - we're capable and it's not done) > + */ > + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR); > + > + /* > + * If we already saw the link up, and it hasn't gone down, then > + * we don't need to wait for autoneg again > + */ > + if (phydev->link && mii_reg & BMSR_LSTATUS) > + return 0; > + > + if ((mii_reg & BMSR_ANEGCAPABLE) && !(mii_reg & BMSR_ANEGCOMPLETE)) { > + int i = 0; > + > + printf("%s Waiting for PHY auto negotiation to complete", > + phydev->dev->name); > + while (!(mii_reg & BMSR_ANEGCOMPLETE)) { > + /* > + * Timeout reached ? > + */ > + if (i > 5000) { > + printf(" TIMEOUT !\n"); > + phydev->link = 0; > + return 0; > + } > + > + if (ctrlc()) { > + puts("user interrupt!\n"); > + phydev->link = 0; > + return -EINTR; > + } > + > + if ((i++ % 500) == 0) > + printf("."); > + > + udelay(1000); /* 1 ms */ > + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR); > + } > + printf(" done\n"); > + phydev->link = 1; > + udelay(500000); /* another 500 ms (results in faster booting) */ I don't understand this, why does sleeping half a second speed up booting? A comment would be in order here. > + } else { > + /* Read the link a second time to clear the latched state */ > + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR); > + > + if (mii_reg & BMSR_LSTATUS) > + phydev->link = 1; > + else > + phydev->link = 0; > + } > + > + return 0; > +} [...] > +struct phy_driver *get_phy_driver(struct phy_device *phydev, > + phy_interface_t interface) > +{ > + struct list_head *entry; > + int phy_id = phydev->phy_id; > + struct phy_driver *drv = NULL; > + > + list_for_each(entry, &phy_drivers) { > + drv = list_entry(entry, struct phy_driver, list); > + if ((drv->uid & drv->mask) == (phy_id & drv->mask)) > + return drv; > + } > + > + /* If we made it here, there's no driver for this PHY */ > + if (interface == PHY_INTERFACE_MODE_XGMII) > + return &gen10g_driver; > + else > + return &genphy_driver; > +} Maybe output a warning when the generic driver is used? From what I see later we should get a message from phy_connect, correct? If this is the case, then disregard this comment ;) Apart from these minor things, the code looks really good. I'm looking forward to seeing this merged, thanks a lot! Detlev -- Als ich entführt wurde, begannen meine Eltern aktiv zu werden. Sie vermieteten mein Zimmer. -- Woody Allen -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
