* 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
* 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
* [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
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).