* mv643xx_eth and MDIO bus PM resume @ 2010-09-11 14:07 Simon Guinot 2010-09-12 11:17 ` saeed bishara 0 siblings, 1 reply; 6+ messages in thread From: Simon Guinot @ 2010-09-11 14:07 UTC (permalink / raw) To: linux-arm-kernel Hi, I am trying to get some PM support for Kirkwood machines and I have to deal with the MDIO bus driver which trigger a kernel crash at resume. The Kirkwood SoC Ethernet driver is mv643xx_eth and don't use the PAL machine state. The PHY is handled by calling manually PHY functions. The problem is that the current mdio_bus_resume() implementation _always_ start at resume the PAL state machine even if the driver can't handle that. That's the mv643xx_eth case, which don't provide the adjust_link() method. Any hint to fix this issue is welcome. Thanks in advance. Simon -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100911/275adc68/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* mv643xx_eth and MDIO bus PM resume 2010-09-11 14:07 mv643xx_eth and MDIO bus PM resume Simon Guinot @ 2010-09-12 11:17 ` saeed bishara 2010-09-12 21:32 ` Simon Guinot 0 siblings, 1 reply; 6+ messages in thread From: saeed bishara @ 2010-09-12 11:17 UTC (permalink / raw) To: linux-arm-kernel On Sat, Sep 11, 2010 at 4:07 PM, Simon Guinot <simon@sequanux.org> wrote: > Hi, > > I am trying to get some PM support for Kirkwood machines and I have to > deal with the MDIO bus driver which trigger a kernel crash at resume. > > The Kirkwood SoC Ethernet driver is mv643xx_eth and don't use the PAL > machine state. The PHY is handled by calling manually PHY functions. > > The problem is that the current mdio_bus_resume() implementation > _always_ start at resume the PAL state machine even if the driver can't > handle that. That's the mv643xx_eth case, which don't provide the > adjust_link() method. > > Any hint to fix this issue is welcome. Simon, can you please post your patch to this driver that adds PM support? saeed ^ permalink raw reply [flat|nested] 6+ messages in thread
* mv643xx_eth and MDIO bus PM resume 2010-09-12 11:17 ` saeed bishara @ 2010-09-12 21:32 ` Simon Guinot 2010-09-12 22:30 ` [PATCH 1/4] phylib: fix PAL state machine restart on resume Simon Guinot 2010-09-13 10:56 ` mv643xx_eth and MDIO bus PM resume saeed bishara 0 siblings, 2 replies; 6+ messages in thread From: Simon Guinot @ 2010-09-12 21:32 UTC (permalink / raw) To: linux-arm-kernel Hi Saeed, On Sun, Sep 12, 2010 at 01:17:28PM +0200, saeed bishara wrote: > On Sat, Sep 11, 2010 at 4:07 PM, Simon Guinot <simon@sequanux.org> wrote: > > Hi, > > > > I am trying to get some PM support for Kirkwood machines and I have to > > deal with the MDIO bus driver which trigger a kernel crash at resume. > > > > The Kirkwood SoC Ethernet driver is mv643xx_eth and don't use the PAL > > machine state. The PHY is handled by calling manually PHY functions. > > > > The problem is that the current mdio_bus_resume() implementation > > _always_ start at resume the PAL state machine even if the driver can't > > handle that. That's the mv643xx_eth case, which don't provide the > > adjust_link() method. > > > > Any hint to fix this issue is welcome. > Simon, can you please post your patch to this driver that adds PM support? Sorry I don't have a such patch yet. For now, I am still working on the core power management support. If you want test, some patches are available at: git://lacie-nas.org/lacie-orion.git pm And the web url is: http://git.lacie-nas.org/?p=lacie-orion.git;a=shortlog;h=refs/heads/pm The next step is to add PM support for the driver mv643xx_eth. As the PM core turn off the GE unit clock, probably that some driver code is needed to handle suspend and resume. Maybe do you have a such patch ? Concerning the mdio_bus_resume() issue, I think it is not related with the Ethernet driver. At resume the MDIO bus automatically start the PAL state machine, even if the adjust_link() function is not supplied. Just add a check is probably good enough. Simon -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100912/cd3785a9/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] phylib: fix PAL state machine restart on resume 2010-09-12 21:32 ` Simon Guinot @ 2010-09-12 22:30 ` Simon Guinot 2010-09-14 3:12 ` David Miller 2010-09-13 10:56 ` mv643xx_eth and MDIO bus PM resume saeed bishara 1 sibling, 1 reply; 6+ messages in thread From: Simon Guinot @ 2010-09-12 22:30 UTC (permalink / raw) To: linux-arm-kernel From: Simon Guinot <sguinot@lacie.com> On resume, before starting the PAL state machine, check if the adjust_link() method is well supplied. If not, this would lead to a NULL pointer dereference in the phy_state_machine() function. This scenario can happen if the Ethernet driver call manually the PHY functions instead of using the PAL state machine. The mv643xx_eth driver is a such example. Signed-off-by: Simon Guinot <sguinot@lacie.com> --- drivers/net/phy/mdio_bus.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 6a6b819..6c58da2 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -308,7 +308,7 @@ static int mdio_bus_suspend(struct device *dev) * may call phy routines that try to grab the same lock, and that may * lead to a deadlock. */ - if (phydev->attached_dev) + if (phydev->attached_dev && phydev->adjust_link) phy_stop_machine(phydev); if (!mdio_bus_phy_may_suspend(phydev)) @@ -331,7 +331,7 @@ static int mdio_bus_resume(struct device *dev) return ret; no_resume: - if (phydev->attached_dev) + if (phydev->attached_dev && phydev->adjust_link) phy_start_machine(phydev, NULL); return 0; -- 1.6.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/4] phylib: fix PAL state machine restart on resume 2010-09-12 22:30 ` [PATCH 1/4] phylib: fix PAL state machine restart on resume Simon Guinot @ 2010-09-14 3:12 ` David Miller 0 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2010-09-14 3:12 UTC (permalink / raw) To: linux-arm-kernel Networking patches should be submitted to netdev at vger.kernel.org not linux-net at vger.kernel.org linux-net is for user questions, developers do not listen at that address Please resubmit your patch to the correct location, thank you. ^ permalink raw reply [flat|nested] 6+ messages in thread
* mv643xx_eth and MDIO bus PM resume 2010-09-12 21:32 ` Simon Guinot 2010-09-12 22:30 ` [PATCH 1/4] phylib: fix PAL state machine restart on resume Simon Guinot @ 2010-09-13 10:56 ` saeed bishara 1 sibling, 0 replies; 6+ messages in thread From: saeed bishara @ 2010-09-13 10:56 UTC (permalink / raw) To: linux-arm-kernel On Sun, Sep 12, 2010 at 11:32 PM, Simon Guinot <simon@sequanux.org> wrote: > Hi Saeed, > > On Sun, Sep 12, 2010 at 01:17:28PM +0200, saeed bishara wrote: >> On Sat, Sep 11, 2010 at 4:07 PM, Simon Guinot <simon@sequanux.org> wrote: >> > Hi, >> > >> > I am trying to get some PM support for Kirkwood machines and I have to >> > deal with the MDIO bus driver which trigger a kernel crash at resume. >> > >> > The Kirkwood SoC Ethernet driver is mv643xx_eth and don't use the PAL >> > machine state. The PHY is handled by calling manually PHY functions. >> > >> > The problem is that the current mdio_bus_resume() implementation >> > _always_ start at resume the PAL state machine even if the driver can't >> > handle that. That's the mv643xx_eth case, which don't provide the >> > adjust_link() method. >> > >> > Any hint to fix this issue is welcome. >> Simon, can you please post your patch to this driver that adds PM support? attached my patch for mv643xx_eth pm support. The Marvell Dove has complete pm support, but this still not merged into mainline, you can look at this git tree where you can find patches for usb, spi, pcie http://kernel.ubuntu.com/git?p=ycmiao/ubuntu-lucid.git;a=shortlog;h=refs/heads/mvl-dove saeed > > Sorry I don't have a such patch yet. For now, I am still working on the > core power management support. > > If you want test, some patches are available at: > > git://lacie-nas.org/lacie-orion.git pm > > And the web url is: > > http://git.lacie-nas.org/?p=lacie-orion.git;a=shortlog;h=refs/heads/pm > > The next step is to add PM support for the driver mv643xx_eth. As the PM > core turn off the GE unit clock, probably that some driver code is > needed to handle suspend and resume. Maybe do you have a such patch ? > > Concerning the mdio_bus_resume() issue, I think it is not related with > the Ethernet driver. At resume the MDIO bus automatically start the PAL > state machine, even if the adjust_link() function is not supplied. Just > add a check is probably good enough. > > Simon > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.10 (GNU/Linux) > > iEYEARECAAYFAkyNRvcACgkQgtp0PDeOcDrcTACfVDQPWDsoALforv2wSCJ4sbwy > 8cIAn0FDzNcMUjuhHJsp5BRsTQ8jdqIm > =I4UJ > -----END PGP SIGNATURE----- > > -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-mv643xx-add-power-management-support.patch Type: application/octet-stream Size: 3714 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100913/ebf0287c/attachment.obj> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-14 3:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-11 14:07 mv643xx_eth and MDIO bus PM resume Simon Guinot 2010-09-12 11:17 ` saeed bishara 2010-09-12 21:32 ` Simon Guinot 2010-09-12 22:30 ` [PATCH 1/4] phylib: fix PAL state machine restart on resume Simon Guinot 2010-09-14 3:12 ` David Miller 2010-09-13 10:56 ` mv643xx_eth and MDIO bus PM resume saeed bishara
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).