
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 afleming@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