| Message ID | 1227794220-19872-1-git-send-email-peppe.cavallaro@st.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Giuseppe CAVALLARO <peppe.cavallaro@st.com> Date: Thu, 27 Nov 2008 14:57:00 +0100 > This patch adds the power management support into the physical > abstraction layer. > > Suspend and resume functions respectively turns on/off the bit 11 > into the PHY Basic mode control register. > Generic PHY device starts supporting PM. > > In order to support the wake-on LAN and avoid to put in power down > the PHY device, the MDIO is aware of what the Ethernet device wants to do. > > Voluntary, no CONFIG_PM defines were added into the sources. > Also generic suspend/resume functions are exported to allow > other drivers use them (such as genphy_config_aneg etc.). > > Within the phy_driver_register function, we need to remove the > memset. It overrides the device driver owner and it is not good. > > Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> Applied. We should audit all of the phy_driver_register() callers to make sure the removal of that memset() won't cause any problems. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
A while ago Andy Fleming had replied to one of my patches expressing an interest in implementing WOL (which I expanded to mean "more control of the lifetime of the phy") within the phylib. I responded that there were two problems with that approach. Very briefly, the problems were that the phylib did not have enough information available to determine what power state to put the phy into (think management firmware), and that turning off the phy might interfere with the normal operation of some MAC registers. I'm sorry that thread died so abruptly. My concerns remain though. At the time the phy is initially probed, there is no way to gather enough information to determine whether or not it is safe to power down the phy. The earliest such information can be communicated is through the flags parameter of the phy_connect() function. I can't speak for the other uses of the phylib, but for tg3 it makes a lot of sense to place phy power management in the hands of the driver that controls it. If you agree, can we devise a way to bypass the suspend callback? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 536bda1..cd71e78 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -284,9 +284,12 @@ static int mdio_bus_suspend(struct device * dev, pm_message_t state) { int ret = 0; struct device_driver *drv = dev->driver; + struct phy_driver *phydrv = to_phy_driver(drv); + struct phy_device *phydev = to_phy_device(dev); - if (drv && drv->suspend) - ret = drv->suspend(dev, state); + if ((!device_may_wakeup(phydev->dev.parent)) && + (phydrv && phydrv->suspend)) + ret = phydrv->suspend(phydev); return ret; } @@ -295,9 +298,12 @@ static int mdio_bus_resume(struct device * dev) { int ret = 0; struct device_driver *drv = dev->driver; + struct phy_driver *phydrv = to_phy_driver(drv); + struct phy_device *phydev = to_phy_device(dev); - if (drv && drv->resume) - ret = drv->resume(dev); + if ((!device_may_wakeup(phydev->dev.parent)) && + (phydrv && phydrv->resume)) + ret = phydrv->resume(phydev); return ret; } diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 25acbbd..b51cebb 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -779,7 +779,35 @@ static int genphy_config_init(struct phy_device *phydev) return 0; } +int genphy_suspend(struct phy_device *phydev) +{ + int value; + + mutex_lock(&phydev->lock); + + value = phy_read(phydev, MII_BMCR); + phy_write(phydev, MII_BMCR, (value | BMCR_PDOWN)); + + mutex_unlock(&phydev->lock); + + return 0; +} +EXPORT_SYMBOL(genphy_suspend); +int genphy_resume(struct phy_device *phydev) +{ + int value; + + mutex_lock(&phydev->lock); + + value = phy_read(phydev, MII_BMCR); + phy_write(phydev, MII_BMCR, (value & ~BMCR_PDOWN)); + + mutex_unlock(&phydev->lock); + + return 0; +} +EXPORT_SYMBOL(genphy_resume); /** * phy_probe - probe and init a PHY device @@ -855,7 +883,6 @@ int phy_driver_register(struct phy_driver *new_driver) { int retval; - memset(&new_driver->driver, 0, sizeof(new_driver->driver)); new_driver->driver.name = new_driver->name; new_driver->driver.bus = &mdio_bus_type; new_driver->driver.probe = phy_probe; @@ -890,6 +917,8 @@ static struct phy_driver genphy_driver = { .features = 0, .config_aneg = genphy_config_aneg, .read_status = genphy_read_status, + .suspend = genphy_suspend, + .resume = genphy_resume, .driver = {.owner= THIS_MODULE, }, }; diff --git a/include/linux/phy.h b/include/linux/phy.h index 77c4ed6..d7e54d9 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -467,6 +467,8 @@ int genphy_restart_aneg(struct phy_device *phydev); int genphy_config_aneg(struct phy_device *phydev); int genphy_update_link(struct phy_device *phydev); int genphy_read_status(struct phy_device *phydev); +int genphy_suspend(struct phy_device *phydev); +int genphy_resume(struct phy_device *phydev); void phy_driver_unregister(struct phy_driver *drv); int phy_driver_register(struct phy_driver *new_driver); void phy_prepare_link(struct phy_device *phydev,
This patch adds the power management support into the physical abstraction layer. Suspend and resume functions respectively turns on/off the bit 11 into the PHY Basic mode control register. Generic PHY device starts supporting PM. In order to support the wake-on LAN and avoid to put in power down the PHY device, the MDIO is aware of what the Ethernet device wants to do. Voluntary, no CONFIG_PM defines were added into the sources. Also generic suspend/resume functions are exported to allow other drivers use them (such as genphy_config_aneg etc.). Within the phy_driver_register function, we need to remove the memset. It overrides the device driver owner and it is not good. Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> --- drivers/net/phy/mdio_bus.c | 14 ++++++++++---- drivers/net/phy/phy_device.c | 31 ++++++++++++++++++++++++++++++- include/linux/phy.h | 2 ++ 3 files changed, 42 insertions(+), 5 deletions(-)