[Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Rafael J. Wysocki]

On Friday 27 March 2009, Rafael J. Wysocki wrote:
> On Friday 27 March 2009, Tvrtko A. Ursulin wrote:
> > On Thursday 26 March 2009 23:08:59 Rafael J. Wysocki wrote:
> > > On Friday 27 March 2009, Tvrtko A. Ursulin wrote:
> > > > On Thursday 26 March 2009 22:38:55 Pavel Machek wrote:
> > > > > On Thu 2009-03-26 22:01:03, Tvrtko A. Ursulin wrote:
> > > > > > On Thursday 26 March 2009 21:35:29 Rafael J. Wysocki wrote:
> > > > > > > On Thursday 26 March 2009, Pavel Machek wrote:
> > > > > > > > On Thu 2009-03-26 20:55:23, Tvrtko A. Ursulin wrote:
> > > > > > > > > On Thursday 26 March 2009 20:18:40 Pavel Machek wrote:
> > > > > > > > > > On Thu 2009-03-26 19:20:32, Tvrtko A. Ursulin wrote:
> > > > > > > > > > > Hi guys,
> > > > > > > > > > > I have been running custom compiled 2.6.28.7 in OpenSUSE
> > > > > > > > > > > 11.1 and using hiberation/resume daily as configured by the
> > > > > > > > > > > distro. Yesterday I upgraded to 2.6.29 (make oldconfig from
> > > > > > > > > > > 2.6.28.7, attached) and after resume networking (at least)
> > > > > > > > > > > does not work. Reboot at this point takes very long because
> > > > > > > > > > > of timeouts on CIFS and whatelse.
> > > > > > > > > > >
> > > > > > > > > > > Below is how /var/log/messagess looks for one cycle
> > > > > > > > > > > (hibernate-resume-reboot) and I am also attaching kernel
> > > > > > > > > > > config and /var/log/pm-suspend.log.
> > > > > > > > > > >
> > > > > > > > > > > You will probably notice evil nvidia.ko in there but don't
> > > > > > > > > > > panic, it all happens without it as well. Tested from
> > > > > > > > > > > runlevel 3 and by running pm-hibernate. After resume from
> > > > > > > > > > > that NIC LEDs were off. I tried /etc/init.d/network restart
> > > > > > > > > > > but that hung so I ^C and tried rmmod forcedeth && modprobe
> > > > > > > > > > > forcedeth && ifup eth0 and after some time network went
> > > > > > > > > > > live again. Haven't had time to test further though, so
> > > > > > > > > > > this is all pretty rough.
> > > > > > > > > >
> > > > > > > > > > Ok, can you try to rmmod forcedeth before hibernation, then
> > > > > > > > > > insmod it after resume?
> > > > > > > > >
> > > > > > > > > Sure, after insmod and /etc/init.d/network start following
> > > > > > > > > resume network was fine. Guess I could kludge that sequence
> > > > > > > > > into pm scripts somehow but would rather avoid that since it
> > > > > > > > > slows down the cycle and would make scripts non-stock.
> > > > > > > >
> > > > > > > > Ok, I guess you need to talk to forcedeth maintainers then...
> > > > > > > > sounds like a forcedeth problem to me.
> > > > > > >
> > > > > > > I have a box with forcedeth somewhere here, I should be able to
> > > > > > > check that in the next few days.
> > > > > >
> > > > > > Not a lot activity in forcedeth.c from 2.6.28 to 2.6.29 so I will CC
> > > > > > two guys who commited changes in that period. Ayaz, Tobias, forcedeth
> > > > > > does not seem to come up after hibernation well for me. Actual
> > > > > > hardware is:
> > > > > >
> > > > > > 00:0f.0 Ethernet controller: nVidia Corporation MCP73 Ethernet (rev
> > > > > > a2)
> > > > > >
> > > > > > Anything more I can do just shout!
> > > > >
> > > > > Actually yes. Testing forcedeth from 2.6.28 in 2.6.29 would be nice
> > > > > way to prove problem is indeed in that module.
> > > >
> > > > Good idea, I had to massage it a bit due to some API changes, but the end
> > > > result is a correct resume which proves that the problem really is in
> > > > forcedeth.
> > >
> > > Well, since you can easily check what commits changed forcedeth between
> > > 2.6.28 and 2.6.29, you can find the one that introduced the problem for
> > > you.

I was able to reproduce the problem and identify the commit that broke the
resume of forcedeth, which turned out to be:

commit cb52deba12f27af90a46d2f8667a64888118a888
Author: Ed Swierk <eswierk@arastra.com>
Date:   Mon Dec 1 12:24:43 2008 +0000

    forcedeth: power down phy when interface is down

    Signed-off-by: Ed Swierk <eswierk@arastra.com>
    Tested-by: Arthur Jones <ajones@riverbed.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Since I have no slightest idea of what this commit is supposed to achieve,
I can only ask for reverting it.  It reverts cleanly, BTW.

Thanks,
Rafael

Re: [Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Tvrtko A. Ursulin]

On Friday 27 March 2009 20:09:20 Rafael J. Wysocki wrote:
> I was able to reproduce the problem and identify the commit that broke the
> resume of forcedeth, which turned out to be:
>
> commit cb52deba12f27af90a46d2f8667a64888118a888
> Author: Ed Swierk <eswierk@arastra.com>
> Date:   Mon Dec 1 12:24:43 2008 +0000
>
>     forcedeth: power down phy when interface is down
>
>     Signed-off-by: Ed Swierk <eswierk@arastra.com>
>     Tested-by: Arthur Jones <ajones@riverbed.com>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Since I have no slightest idea of what this commit is supposed to achieve,
> I can only ask for reverting it.  It reverts cleanly, BTW.

Reverting this fixes it for me as well.

Thanks for your effort!

Tvrtko

Re: [Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Tvrtko Ursulin]

On Sunday 29 March 2009 09:03:57 Tvrtko A. Ursulin wrote:
> On Friday 27 March 2009 20:09:20 Rafael J. Wysocki wrote:
> > I was able to reproduce the problem and identify the commit that broke
> > the resume of forcedeth, which turned out to be:
> >
> > commit cb52deba12f27af90a46d2f8667a64888118a888
> > Author: Ed Swierk <eswierk@arastra.com>
> > Date:   Mon Dec 1 12:24:43 2008 +0000
> >
> >     forcedeth: power down phy when interface is down
> >
> >     Signed-off-by: Ed Swierk <eswierk@arastra.com>
> >     Tested-by: Arthur Jones <ajones@riverbed.com>
> >     Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >     Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > Since I have no slightest idea of what this commit is supposed to
> > achieve, I can only ask for reverting it.  It reverts cleanly, BTW.
>
> Reverting this fixes it for me as well.
>
> Thanks for your effort!

Would reverting this be a -stable material?

Tvrtko

Re: [Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Ed Swierk]

On Fri, Mar 27, 2009 at 1:09 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> I was able to reproduce the problem and identify the commit that broke the
> resume of forcedeth, which turned out to be:
>
> commit cb52deba12f27af90a46d2f8667a64888118a888
> Author: Ed Swierk <eswierk@arastra.com>
> Date:   Mon Dec 1 12:24:43 2008 +0000
>
>    forcedeth: power down phy when interface is down
>
>    Signed-off-by: Ed Swierk <eswierk@arastra.com>
>    Tested-by: Arthur Jones <ajones@riverbed.com>
>    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>    Signed-off-by: David S. Miller <davem@davemloft.net>
>
> Since I have no slightest idea of what this commit is supposed to achieve,
> I can only ask for reverting it.  It reverts cleanly, BTW.

The change causes forcedeth to bring down the physical link when an
interface goes down; leaving it up causes the switch at the other end
to think the port is still active, with potentially random speed and
duplex parameters.

It's possible that the forcedeth driver needs to reset autonegotiation
after bringing it up the link again.

Can you please try this on a 2.6.29 kernel that's exhibiting the
symptoms you describe, after resuming the machine from hibernation:

  ethtool -s eth1 autoneg off speed 100 duplex full
  ethtool -s eth1 autoneg on

This should reset autonegotiation manually and bring the link. Let me
know how it goes.

Thanks,
--Ed

Re: [Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Ed Swierk]

On Fri, 2009-04-03 at 08:24 -0700, Ed Swierk wrote:
> On Fri, Mar 27, 2009 at 1:09 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > I was able to reproduce the problem and identify the commit that broke the
> > resume of forcedeth, which turned out to be:
> >
> > commit cb52deba12f27af90a46d2f8667a64888118a888
> > Author: Ed Swierk <eswierk@arastra.com>
> > Date:   Mon Dec 1 12:24:43 2008 +0000
> >
> >    forcedeth: power down phy when interface is down
> >
> >    Signed-off-by: Ed Swierk <eswierk@arastra.com>
> >    Tested-by: Arthur Jones <ajones@riverbed.com>
> >    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >    Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > Since I have no slightest idea of what this commit is supposed to achieve,
> > I can only ask for reverting it.  It reverts cleanly, BTW.
> 
> The change causes forcedeth to bring down the physical link when an
> interface goes down; leaving it up causes the switch at the other end
> to think the port is still active, with potentially random speed and
> duplex parameters.
> 
> It's possible that the forcedeth driver needs to reset autonegotiation
> after bringing it up the link again.
> 
> Can you please try this on a 2.6.29 kernel that's exhibiting the
> symptoms you describe, after resuming the machine from hibernation:
> 
>   ethtool -s eth1 autoneg off speed 100 duplex full
>   ethtool -s eth1 autoneg on
> 
> This should reset autonegotiation manually and bring the link. Let me
> know how it goes.

Also please try this patch; it fixes the problem on my test system (a
DFI board with an nVidia MCP55).

Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>
--
--- linux-2.6.29.x86_64/drivers/net/forcedeth.c.orig	2009-03-23 16:12:14.000000000 -0700
+++ linux-2.6.29.x86_64/drivers/net/forcedeth.c	2009-04-03 10:11:26.839614710 -0700
@@ -5995,6 +5995,9 @@
 	for (i = 0;i <= np->register_size/sizeof(u32); i++)
 		writel(np->saved_config_space[i], base+i*sizeof(u32));
 
+	/* restore phy state, including autoneg */
+	phy_init(dev);
+
 	netif_device_attach(dev);
 	if (netif_running(dev)) {
 		rc = nv_open(dev);

Re: [Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Rafael J. Wysocki]

On Friday 03 April 2009, Ed Swierk wrote:
> On Fri, 2009-04-03 at 08:24 -0700, Ed Swierk wrote:
> > On Fri, Mar 27, 2009 at 1:09 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > I was able to reproduce the problem and identify the commit that broke the
> > > resume of forcedeth, which turned out to be:
> > >
> > > commit cb52deba12f27af90a46d2f8667a64888118a888
> > > Author: Ed Swierk <eswierk@arastra.com>
> > > Date:   Mon Dec 1 12:24:43 2008 +0000
> > >
> > >    forcedeth: power down phy when interface is down
> > >
> > >    Signed-off-by: Ed Swierk <eswierk@arastra.com>
> > >    Tested-by: Arthur Jones <ajones@riverbed.com>
> > >    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > >    Signed-off-by: David S. Miller <davem@davemloft.net>
> > >
> > > Since I have no slightest idea of what this commit is supposed to achieve,
> > > I can only ask for reverting it.  It reverts cleanly, BTW.
> > 
> > The change causes forcedeth to bring down the physical link when an
> > interface goes down; leaving it up causes the switch at the other end
> > to think the port is still active, with potentially random speed and
> > duplex parameters.
> > 
> > It's possible that the forcedeth driver needs to reset autonegotiation
> > after bringing it up the link again.
> > 
> > Can you please try this on a 2.6.29 kernel that's exhibiting the
> > symptoms you describe, after resuming the machine from hibernation:
> > 
> >   ethtool -s eth1 autoneg off speed 100 duplex full
> >   ethtool -s eth1 autoneg on
> > 
> > This should reset autonegotiation manually and bring the link. Let me
> > know how it goes.
> 
> Also please try this patch; it fixes the problem on my test system (a
> DFI board with an nVidia MCP55).
> 
> Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>
> --
> --- linux-2.6.29.x86_64/drivers/net/forcedeth.c.orig	2009-03-23 16:12:14.000000000 -0700
> +++ linux-2.6.29.x86_64/drivers/net/forcedeth.c	2009-04-03 10:11:26.839614710 -0700
> @@ -5995,6 +5995,9 @@
>  	for (i = 0;i <= np->register_size/sizeof(u32); i++)
>  		writel(np->saved_config_space[i], base+i*sizeof(u32));
>  
> +	/* restore phy state, including autoneg */
> +	phy_init(dev);
> +
>  	netif_device_attach(dev);
>  	if (netif_running(dev)) {
>  		rc = nv_open(dev);

Tvrtko, can you test it, please?  I'll be away from my NVidia box during the
next couple of days.

Thanks,
Rafael

Re: [Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Tvrtko A. Ursulin]

On Friday 03 April 2009 18:41:12 Ed Swierk wrote:
> On Fri, 2009-04-03 at 08:24 -0700, Ed Swierk wrote:
> > On Fri, Mar 27, 2009 at 1:09 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > I was able to reproduce the problem and identify the commit that broke
> > > the resume of forcedeth, which turned out to be:
> > >
> > > commit cb52deba12f27af90a46d2f8667a64888118a888
> > > Author: Ed Swierk <eswierk@arastra.com>
> > > Date:   Mon Dec 1 12:24:43 2008 +0000
> > >
> > >    forcedeth: power down phy when interface is down
> > >
> > >    Signed-off-by: Ed Swierk <eswierk@arastra.com>
> > >    Tested-by: Arthur Jones <ajones@riverbed.com>
> > >    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > >    Signed-off-by: David S. Miller <davem@davemloft.net>
> > >
> > > Since I have no slightest idea of what this commit is supposed to
> > > achieve, I can only ask for reverting it.  It reverts cleanly, BTW.
> >
> > The change causes forcedeth to bring down the physical link when an
> > interface goes down; leaving it up causes the switch at the other end
> > to think the port is still active, with potentially random speed and
> > duplex parameters.
> >
> > It's possible that the forcedeth driver needs to reset autonegotiation
> > after bringing it up the link again.
> >
> > Can you please try this on a 2.6.29 kernel that's exhibiting the
> > symptoms you describe, after resuming the machine from hibernation:
> >
> >   ethtool -s eth1 autoneg off speed 100 duplex full
> >   ethtool -s eth1 autoneg on
> >
> > This should reset autonegotiation manually and bring the link. Let me
> > know how it goes.
>
> Also please try this patch; it fixes the problem on my test system (a
> DFI board with an nVidia MCP55).
>
> Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>
> --
> --- linux-2.6.29.x86_64/drivers/net/forcedeth.c.orig	2009-03-23
> 16:12:14.000000000 -0700 +++
> linux-2.6.29.x86_64/drivers/net/forcedeth.c	2009-04-03 10:11:26.839614710
> -0700 @@ -5995,6 +5995,9 @@
>  	for (i = 0;i <= np->register_size/sizeof(u32); i++)
>  		writel(np->saved_config_space[i], base+i*sizeof(u32));
>
> +	/* restore phy state, including autoneg */
> +	phy_init(dev);
> +
>  	netif_device_attach(dev);
>  	if (netif_running(dev)) {
>  		rc = nv_open(dev);

Works for me on MCP73 as well.

Although NIC LED is still on after hibernation - seems to go off shortly 
before power off and then comes back on. Could be firmware issue I guess, 
this motherboard has always gulped power when off.

Tvrtko

Re: [Regression in 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Rafael J. Wysocki]

On Saturday 04 April 2009, Tvrtko A. Ursulin wrote:
> On Friday 03 April 2009 18:41:12 Ed Swierk wrote:
> > On Fri, 2009-04-03 at 08:24 -0700, Ed Swierk wrote:
> > > On Fri, Mar 27, 2009 at 1:09 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > > I was able to reproduce the problem and identify the commit that broke
> > > > the resume of forcedeth, which turned out to be:
> > > >
> > > > commit cb52deba12f27af90a46d2f8667a64888118a888
> > > > Author: Ed Swierk <eswierk@arastra.com>
> > > > Date:   Mon Dec 1 12:24:43 2008 +0000
> > > >
> > > >    forcedeth: power down phy when interface is down
> > > >
> > > >    Signed-off-by: Ed Swierk <eswierk@arastra.com>
> > > >    Tested-by: Arthur Jones <ajones@riverbed.com>
> > > >    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > > >    Signed-off-by: David S. Miller <davem@davemloft.net>
> > > >
> > > > Since I have no slightest idea of what this commit is supposed to
> > > > achieve, I can only ask for reverting it.  It reverts cleanly, BTW.
> > >
> > > The change causes forcedeth to bring down the physical link when an
> > > interface goes down; leaving it up causes the switch at the other end
> > > to think the port is still active, with potentially random speed and
> > > duplex parameters.
> > >
> > > It's possible that the forcedeth driver needs to reset autonegotiation
> > > after bringing it up the link again.
> > >
> > > Can you please try this on a 2.6.29 kernel that's exhibiting the
> > > symptoms you describe, after resuming the machine from hibernation:
> > >
> > >   ethtool -s eth1 autoneg off speed 100 duplex full
> > >   ethtool -s eth1 autoneg on
> > >
> > > This should reset autonegotiation manually and bring the link. Let me
> > > know how it goes.
> >
> > Also please try this patch; it fixes the problem on my test system (a
> > DFI board with an nVidia MCP55).
> >
> > Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>
> > --
> > --- linux-2.6.29.x86_64/drivers/net/forcedeth.c.orig	2009-03-23
> > 16:12:14.000000000 -0700 +++
> > linux-2.6.29.x86_64/drivers/net/forcedeth.c	2009-04-03 10:11:26.839614710
> > -0700 @@ -5995,6 +5995,9 @@
> >  	for (i = 0;i <= np->register_size/sizeof(u32); i++)
> >  		writel(np->saved_config_space[i], base+i*sizeof(u32));
> >
> > +	/* restore phy state, including autoneg */
> > +	phy_init(dev);
> > +
> >  	netif_device_attach(dev);
> >  	if (netif_running(dev)) {
> >  		rc = nv_open(dev);
> 
> Works for me on MCP73 as well.

Great, thanks for testing.

I think the patch should go to Linus ASAP and to -stable as a regression fix.

Thanks,
Rafael

Re: [Regression, in, 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[David Miller]

Please send a version of this patch that actually applies to
Linus's tree, the line numbers et al. are a lot different
now.

I'll backport and submit that to -stable.

Thanks.

Re: [Regression, in, 2.6.29] forcedeth doesn't work after resume from hibernation (was: Re: Resume after hibernation regression in 2.6.29)

[Ed Swierk]

On Sat, 2009-04-04 at 16:54 -0700, David Miller wrote:
> Please send a version of this patch that actually applies to
> Linus's tree, the line numbers et al. are a lot different
> now.
> 
> I'll backport and submit that to -stable.
> 
> Thanks.

Reset phy state on resume, fixing a regression caused by powering down
the phy on hibernate.

Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>
---
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index a858c6f..663a7b3 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -6117,6 +6117,9 @@ static int nv_resume(struct pci_dev *pdev)
 
 	pci_write_config_dword(pdev, NV_MSI_PRIV_OFFSET, NV_MSI_PRIV_VALUE);
 
+	/* restore phy state, including autoneg */
+	phy_init(dev);
+
 	netif_device_attach(dev);
 	if (netif_running(dev)) {
 		rc = nv_open(dev);

Re: [Regression, in, 2.6.29] forcedeth doesn't work after resume from hibernation

[David Miller]

From: Ed Swierk <eswierk@aristanetworks.com>
Date: Mon, 06 Apr 2009 07:03:32 -0700

> Reset phy state on resume, fixing a regression caused by powering down
> the phy on hibernate.
> 
> Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>

Applied, thanks.