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

login
register
mail settings
Submitter Ed Swierk
Date April 3, 2009, 5:41 p.m.
Message ID <1238780473.19305.3.camel@localhost.localdomain>
Download mbox | patch
Permalink /patch/25582/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Ed Swierk - April 3, 2009, 5:41 p.m.
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>
--


--
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
Rafael J. Wysocki - April 3, 2009, 8:26 p.m.
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
--
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
Tvrtko A. Ursulin - April 4, 2009, 1:04 p.m.
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
--
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
Rafael J. Wysocki - April 4, 2009, 1:07 p.m.
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
--
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

Patch

--- 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);