* [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver @ 2015-01-26 8:14 Fugang Duan 2015-01-26 17:14 ` Florian Fainelli 0 siblings, 1 reply; 4+ messages in thread From: Fugang Duan @ 2015-01-26 8:14 UTC (permalink / raw) To: davem; +Cc: netdev, s.hauer, f.fainelli, bhutchings, stephen, b38611 The current .phy_detach() function only unbind generic phy driver, which causes specifical driver suspend/resume function still work like Atheros AT803X PHYs. For example: ifconfig eth0 down echo mem > /sys/power/status After eth0 interface down, driver call phy_detach to unbind phy driver, and then do suspend/resume operation, at803x_suspend()/at803x_resume() functions still get called that call mdio bus read/write function. When eth0 interface down, MAC driver may close all clocks and mdio bus cannot work. So the issue happens. The patch can unbind generic and specifical driver. Signed-off-by: Fugang Duan <B38611@freescale.com> --- drivers/net/phy/phy_device.c | 15 ++------------- 1 files changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 3fc91e8..8adbc5d 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -672,8 +672,6 @@ EXPORT_SYMBOL(phy_attach); */ void phy_detach(struct phy_device *phydev) { - int i; - if (phydev->bus->dev.driver) module_put(phydev->bus->dev.driver->owner); @@ -681,17 +679,8 @@ void phy_detach(struct phy_device *phydev) phydev->attached_dev = NULL; phy_suspend(phydev); - /* If the device had no specific driver before (i.e. - it - * was using the generic driver), we unbind the device - * from the generic driver so that there's a chance a - * real driver could be loaded - */ - for (i = 0; i < ARRAY_SIZE(genphy_driver); i++) { - if (phydev->dev.driver == &genphy_driver[i].driver) { - device_release_driver(&phydev->dev); - break; - } - } + if (phydev->dev.driver) + device_release_driver(&phydev->dev); } EXPORT_SYMBOL(phy_detach); -- 1.7.8 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver 2015-01-26 8:14 [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver Fugang Duan @ 2015-01-26 17:14 ` Florian Fainelli 2015-01-26 17:47 ` Florian Fainelli 0 siblings, 1 reply; 4+ messages in thread From: Florian Fainelli @ 2015-01-26 17:14 UTC (permalink / raw) To: Fugang Duan, davem; +Cc: netdev, s.hauer, bhutchings, stephen On 26/01/15 00:14, Fugang Duan wrote: > The current .phy_detach() function only unbind generic phy driver, which causes > specifical driver suspend/resume function still work like Atheros AT803X PHYs. > > For example: > ifconfig eth0 down > echo mem > /sys/power/status > > After eth0 interface down, driver call phy_detach to unbind phy driver, and then > do suspend/resume operation, at803x_suspend()/at803x_resume() functions still get > called that call mdio bus read/write function. When eth0 interface down, MAC driver > may close all clocks and mdio bus cannot work. So the issue happens. I was just hitting this problem on Friday evening and was about to submit a similar change. Thanks! > > The patch can unbind generic and specifical driver. > > Signed-off-by: Fugang Duan <B38611@freescale.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com> Tested-by: Florian Fainelli <f.fainelli@gmail.com> > --- > drivers/net/phy/phy_device.c | 15 ++------------- > 1 files changed, 2 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index 3fc91e8..8adbc5d 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -672,8 +672,6 @@ EXPORT_SYMBOL(phy_attach); > */ > void phy_detach(struct phy_device *phydev) > { > - int i; > - > if (phydev->bus->dev.driver) > module_put(phydev->bus->dev.driver->owner); > > @@ -681,17 +679,8 @@ void phy_detach(struct phy_device *phydev) > phydev->attached_dev = NULL; > phy_suspend(phydev); > > - /* If the device had no specific driver before (i.e. - it > - * was using the generic driver), we unbind the device > - * from the generic driver so that there's a chance a > - * real driver could be loaded > - */ > - for (i = 0; i < ARRAY_SIZE(genphy_driver); i++) { > - if (phydev->dev.driver == &genphy_driver[i].driver) { > - device_release_driver(&phydev->dev); > - break; > - } > - } > + if (phydev->dev.driver) > + device_release_driver(&phydev->dev); > } > EXPORT_SYMBOL(phy_detach); > > -- Florian ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver 2015-01-26 17:14 ` Florian Fainelli @ 2015-01-26 17:47 ` Florian Fainelli 2015-01-27 2:27 ` fugang.duan 0 siblings, 1 reply; 4+ messages in thread From: Florian Fainelli @ 2015-01-26 17:47 UTC (permalink / raw) To: Fugang Duan, davem; +Cc: netdev, s.hauer, bhutchings, stephen On 26/01/15 09:14, Florian Fainelli wrote: > On 26/01/15 00:14, Fugang Duan wrote: >> The current .phy_detach() function only unbind generic phy driver, which causes >> specifical driver suspend/resume function still work like Atheros AT803X PHYs. >> >> For example: >> ifconfig eth0 down >> echo mem > /sys/power/status >> >> After eth0 interface down, driver call phy_detach to unbind phy driver, and then >> do suspend/resume operation, at803x_suspend()/at803x_resume() functions still get >> called that call mdio bus read/write function. When eth0 interface down, MAC driver >> may close all clocks and mdio bus cannot work. So the issue happens. > > I was just hitting this problem on Friday evening and was about to > submit a similar change. Thanks! > >> >> The patch can unbind generic and specifical driver. >> >> Signed-off-by: Fugang Duan <B38611@freescale.com> > > Acked-by: Florian Fainelli <f.fainelli@gmail.com> > Tested-by: Florian Fainelli <f.fainelli@gmail.com> Humm, this breaks a sequence of ifconfig down then up, the driver is removed, and we never get to probe it again, does this work for you using the FEC driver? Another way to solve this double suspend problem is to make sure that we track whether the PHY has already been suspended. -- Florian ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver 2015-01-26 17:47 ` Florian Fainelli @ 2015-01-27 2:27 ` fugang.duan 0 siblings, 0 replies; 4+ messages in thread From: fugang.duan @ 2015-01-27 2:27 UTC (permalink / raw) To: Florian Fainelli, davem@davemloft.net Cc: netdev@vger.kernel.org, s.hauer@pengutronix.de, bhutchings@solarflare.com, stephen@networkplumber.org From: Florian Fainelli <f.fainelli@gmail.com> Sent: Tuesday, January 27, 2015 1:47 AM > To: Duan Fugang-B38611; davem@davemloft.net > Cc: netdev@vger.kernel.org; s.hauer@pengutronix.de; > bhutchings@solarflare.com; stephen@networkplumber.org > Subject: Re: [PATCH net-next 1/1] net: phy: unbind phy device from > generic and specifical driver > > On 26/01/15 09:14, Florian Fainelli wrote: > > On 26/01/15 00:14, Fugang Duan wrote: > >> The current .phy_detach() function only unbind generic phy driver, > >> which causes specifical driver suspend/resume function still work like > Atheros AT803X PHYs. > >> > >> For example: > >> ifconfig eth0 down > >> echo mem > /sys/power/status > >> > >> After eth0 interface down, driver call phy_detach to unbind phy > >> driver, and then do suspend/resume operation, > >> at803x_suspend()/at803x_resume() functions still get called that call > >> mdio bus read/write function. When eth0 interface down, MAC driver may > close all clocks and mdio bus cannot work. So the issue happens. > > > > I was just hitting this problem on Friday evening and was about to > > submit a similar change. Thanks! > > > >> > >> The patch can unbind generic and specifical driver. > >> > >> Signed-off-by: Fugang Duan <B38611@freescale.com> > > > > Acked-by: Florian Fainelli <f.fainelli@gmail.com> > > Tested-by: Florian Fainelli <f.fainelli@gmail.com> > > Humm, this breaks a sequence of ifconfig down then up, the driver is > removed, and we never get to probe it again, does this work for you using > the FEC driver? > It works fine for FEC driver with below flow case. echo enabled > /sys/class/tty/ttymxc0/power/wakeup echo mem > /sys/power/state ifconfig eth0 down echo mem > /sys/power/state ifconfig eth0 up ping 10.192.242.204 //ping work fine echo mem > /sys/power/state ... I know the reason, the first fec driver use Atheros 8031 ethernet special phy driver, like: fec 2188000.ethernet eth0: Freescale FEC PHY driver [Atheros 8031 ethernet] (mii_bus:phy_addr=2188000.ethernet:01, irq=-1) After down and up, fec use generic phy driver, but it still can work fine: fec 2188000.ethernet eth0: Freescale FEC PHY driver [Generic PHY] (mii_bus:phy_addr=2188000.ethernet:01, irq=-1) After down and up, in here, phy driver only is initialized as generic phy driver. int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, u32 flags, phy_interface_t interface) { ... /* Assume that if there is no driver, that it doesn't * exist, and we should use the genphy driver. */ if (NULL == d->driver) { if (phydev->is_c45) d->driver = &genphy_driver[GENPHY_DRV_10G].driver; else d->driver = &genphy_driver[GENPHY_DRV_1G].driver; err = d->driver->probe(d); if (err >= 0) err = device_bind_driver(d); if (err) return err; } ... } So, I suggest to use your patches set. Regards, Andy ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-27 2:27 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-26 8:14 [PATCH net-next 1/1] net: phy: unbind phy device from generic and specifical driver Fugang Duan 2015-01-26 17:14 ` Florian Fainelli 2015-01-26 17:47 ` Florian Fainelli 2015-01-27 2:27 ` fugang.duan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).