diff mbox

[Bugme-new,Bug,12886] New: skge wake on lan

Message ID 200903172223.14608.rjw@sisk.pl
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Rafael J. Wysocki March 17, 2009, 9:23 p.m. UTC
On Tuesday 17 March 2009, Tomi Orava wrote:
> 
> Hi,
> 
> > Please tell me what exactly you do to set up WoL.
> 
> The only thing I do is I configure the WOL bios option
> to on ie. no OS based configuration settings are touched.

Hmm.

> The previously mentioned patch reverted the ethtool output is
> (ie. this is a working 2.6.28.7 system atm):

I wonder what it shows if that commit is not reverted (in fact it is necessary
on some other systems for things to work correctly).

Also, please check if the appended patch helps (of course, without reverting
the commit).

---
 drivers/net/skge.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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

Comments

Tomi Orava March 18, 2009, 7:32 a.m. UTC | #1
Hi again,

>> The previously mentioned patch reverted the ethtool output is
>> (ie. this is a working 2.6.28.7 system atm):
>
> I wonder what it shows if that commit is not reverted (in fact it is
> necessary
> on some other systems for things to work correctly).

Ethtool eth0 shows on latest 2.6.28.8 kernel with your manually merged
patch below:

Settings for eth0:
        Supported ports: [ TP ]
        Supported link modes:   10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Half 1000baseT/Full
        Supports auto-negotiation: Yes
        Advertised link modes:  10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Half 1000baseT/Full
        Advertised auto-negotiation: Yes
        Speed: 1000Mb/s
        Duplex: Full
        Port: Twisted Pair
        PHYAD: 0
        Transceiver: internal
        Auto-negotiation: on
        Supports Wake-on: pg
        Wake-on: g
        Current message level: 0x00000037 (55)
        Link detected: yes

Ie. looks like exactly the same as with the commit reverted.

>
> Also, please check if the appended patch helps (of course, without
> reverting
> the commit).
>
> ---
>  drivers/net/skge.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/net/skge.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/skge.c
> +++ linux-2.6/drivers/net/skge.c
> @@ -3856,8 +3856,9 @@ static struct net_device *skge_devinit(s
>  	skge->speed = -1;
>  	skge->advertising = skge_supported_modes(hw);
>
> -	if (device_may_wakeup(&hw->pdev->dev))
> -		skge->wol = wol_supported(hw) & WAKE_MAGIC;
> +	skge->wol = wol_supported(hw) & WAKE_MAGIC;
> +	if (skge->wol && device_can_wakeup(&hw->pdev->dev))
> +		device_set_wakeup_enable(&hw->pdev->dev, true);
>
>  	hw->dev[port] = dev;

Hmm, looks like this patch is against a 2.6.28.8 tree with the commit
reverted. I merged the couple lines as following.

line 3859:

        // if (pci_wake_enabled(hw->pdev))
        //      skge->wol = wol_supported(hw) & WAKE_MAGIC;
        skge->wol = wol_supported(hw) & WAKE_MAGIC;
        if (skge->wol && device_can_wakeup(&hw->pdev->dev))
                device_set_wakeup_enable(&hw->pdev->dev, true);

and this really seems to fix the problem. The system is able to wake
up via etherwake just fine.

Regards,
Tomi Orava
Rafael J. Wysocki March 18, 2009, 3:14 p.m. UTC | #2
On Wednesday 18 March 2009, Tomi Orava wrote:
> Hi again,
> 
> >> The previously mentioned patch reverted the ethtool output is
> >> (ie. this is a working 2.6.28.7 system atm):
> >
> > I wonder what it shows if that commit is not reverted (in fact it is
> > necessary
> > on some other systems for things to work correctly).
> 
> Ethtool eth0 shows on latest 2.6.28.8 kernel with your manually merged
> patch below:

Hm, are you sure the commit in question _is_ in 2.6.28.y?  I can't see it in
there.  It is in the current Linus' tree, but not in v2.6.28 and surely not in
-stable.  Do you use a distro kernel?

> Settings for eth0:
>         Supported ports: [ TP ]
>         Supported link modes:   10baseT/Half 10baseT/Full
>                                 100baseT/Half 100baseT/Full
>                                 1000baseT/Half 1000baseT/Full
>         Supports auto-negotiation: Yes
>         Advertised link modes:  10baseT/Half 10baseT/Full
>                                 100baseT/Half 100baseT/Full
>                                 1000baseT/Half 1000baseT/Full
>         Advertised auto-negotiation: Yes
>         Speed: 1000Mb/s
>         Duplex: Full
>         Port: Twisted Pair
>         PHYAD: 0
>         Transceiver: internal
>         Auto-negotiation: on
>         Supports Wake-on: pg
>         Wake-on: g
>         Current message level: 0x00000037 (55)
>         Link detected: yes
> 
> Ie. looks like exactly the same as with the commit reverted.

OK

> >
> > Also, please check if the appended patch helps (of course, without
> > reverting
> > the commit).
> >
> > ---
> >  drivers/net/skge.c |    5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > Index: linux-2.6/drivers/net/skge.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/net/skge.c
> > +++ linux-2.6/drivers/net/skge.c
> > @@ -3856,8 +3856,9 @@ static struct net_device *skge_devinit(s
> >  	skge->speed = -1;
> >  	skge->advertising = skge_supported_modes(hw);
> >
> > -	if (device_may_wakeup(&hw->pdev->dev))
> > -		skge->wol = wol_supported(hw) & WAKE_MAGIC;
> > +	skge->wol = wol_supported(hw) & WAKE_MAGIC;
> > +	if (skge->wol && device_can_wakeup(&hw->pdev->dev))
> > +		device_set_wakeup_enable(&hw->pdev->dev, true);
> >
> >  	hw->dev[port] = dev;
> 
> Hmm, looks like this patch is against a 2.6.28.8 tree with the commit
> reverted. I merged the couple lines as following.
> 
> line 3859:
> 
>         // if (pci_wake_enabled(hw->pdev))
>         //      skge->wol = wol_supported(hw) & WAKE_MAGIC;
>         skge->wol = wol_supported(hw) & WAKE_MAGIC;
>         if (skge->wol && device_can_wakeup(&hw->pdev->dev))
>                 device_set_wakeup_enable(&hw->pdev->dev, true);
> 
> and this really seems to fix the problem. The system is able to wake
> up via etherwake just fine.

Well, that's because commit 5177b3240a6608fc0c9c05cc32f4855c6540f8d5 changed
the default from "wake-up enabled by default" to "wake-up disabled by default".
The latter is safer IMO, but I'll prepare a (better) patch to change it back to
the old behavior.

You should be able to set up WoL by running 'ethtool -s eth0 wol g' from your
init scripts even without reverting the patch.  Please verify if that works.
--
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
Tomi Orava March 19, 2009, 1:03 p.m. UTC | #3
Hi,


> Hm, are you sure the commit in question _is_ in 2.6.28.y?  I can't see it
> in
> there.  It is in the current Linus' tree, but not in v2.6.28 and surely
> not in
> -stable.  Do you use a distro kernel?

My bad! It seems that I've got it backwards and therefore must
retest everything. The fact is that the stable series 2.6.28.x
kernels did not have a working WOL in my motherboard.
However, I think I messed up initially by patching instead of
reverting the mentioned commit into the stable series 2.6.28.x
kernel which seemed to fix the problem.

Therefore, could you wait for a short while, and I'll try to run
the couple of tests again in the evening and then get back to
you.

Regards,
Tomi Orava

>
>> Settings for eth0:
>>         Supported ports: [ TP ]
>>         Supported link modes:   10baseT/Half 10baseT/Full
>>                                 100baseT/Half 100baseT/Full
>>                                 1000baseT/Half 1000baseT/Full
>>         Supports auto-negotiation: Yes
>>         Advertised link modes:  10baseT/Half 10baseT/Full
>>                                 100baseT/Half 100baseT/Full
>>                                 1000baseT/Half 1000baseT/Full
>>         Advertised auto-negotiation: Yes
>>         Speed: 1000Mb/s
>>         Duplex: Full
>>         Port: Twisted Pair
>>         PHYAD: 0
>>         Transceiver: internal
>>         Auto-negotiation: on
>>         Supports Wake-on: pg
>>         Wake-on: g
>>         Current message level: 0x00000037 (55)
>>         Link detected: yes
>>
>> Ie. looks like exactly the same as with the commit reverted.
>
> OK
>
>> >
>> > Also, please check if the appended patch helps (of course, without
>> > reverting
>> > the commit).
>> >
>> > ---
>> >  drivers/net/skge.c |    5 +++--
>> >  1 file changed, 3 insertions(+), 2 deletions(-)
>> >
>> > Index: linux-2.6/drivers/net/skge.c
>> > ===================================================================
>> > --- linux-2.6.orig/drivers/net/skge.c
>> > +++ linux-2.6/drivers/net/skge.c
>> > @@ -3856,8 +3856,9 @@ static struct net_device *skge_devinit(s
>> >  	skge->speed = -1;
>> >  	skge->advertising = skge_supported_modes(hw);
>> >
>> > -	if (device_may_wakeup(&hw->pdev->dev))
>> > -		skge->wol = wol_supported(hw) & WAKE_MAGIC;
>> > +	skge->wol = wol_supported(hw) & WAKE_MAGIC;
>> > +	if (skge->wol && device_can_wakeup(&hw->pdev->dev))
>> > +		device_set_wakeup_enable(&hw->pdev->dev, true);
>> >
>> >  	hw->dev[port] = dev;
>>
>> Hmm, looks like this patch is against a 2.6.28.8 tree with the commit
>> reverted. I merged the couple lines as following.
>>
>> line 3859:
>>
>>         // if (pci_wake_enabled(hw->pdev))
>>         //      skge->wol = wol_supported(hw) & WAKE_MAGIC;
>>         skge->wol = wol_supported(hw) & WAKE_MAGIC;
>>         if (skge->wol && device_can_wakeup(&hw->pdev->dev))
>>                 device_set_wakeup_enable(&hw->pdev->dev, true);
>>
>> and this really seems to fix the problem. The system is able to wake
>> up via etherwake just fine.
>
> Well, that's because commit 5177b3240a6608fc0c9c05cc32f4855c6540f8d5
> changed
> the default from "wake-up enabled by default" to "wake-up disabled by
> default".
> The latter is safer IMO, but I'll prepare a (better) patch to change it
> back to
> the old behavior.
>
> You should be able to set up WoL by running 'ethtool -s eth0 wol g' from
> your
> init scripts even without reverting the patch.  Please verify if that
> works.
>
Tomi Orava March 19, 2009, 4:23 p.m. UTC | #4
Hi,

> Hm, are you sure the commit in question _is_ in 2.6.28.y?  I can't see it in
> there.  It is in the current Linus' tree, but not in v2.6.28 and surely not in
> -stable.  Do you use a distro kernel?

Ok, I've now verified that I did indeed mess up with the commit
mentioned previously. In other words the stable series 2.6.28.x
doesn't support the suspend feature on my Asus A7V880 at all.
The bios option is not respected at all and the command
'ethtool -s eth0 wol g' doesn't enable the suspend any better.

However, the commit taken from main branch actually makes also
the stable series to work with this motherboard. No additional
changes are needed, it just works out of the box.

Sorry about the mess up.

Regards,
Tomi Orava

>>> Also, please check if the appended patch helps (of course, without
>>> reverting
>>> the commit).
>>>
>>> ---
>>>  drivers/net/skge.c |    5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> Index: linux-2.6/drivers/net/skge.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/net/skge.c
>>> +++ linux-2.6/drivers/net/skge.c
>>> @@ -3856,8 +3856,9 @@ static struct net_device *skge_devinit(s
>>>  	skge->speed = -1;
>>>  	skge->advertising = skge_supported_modes(hw);
>>>
>>> -	if (device_may_wakeup(&hw->pdev->dev))
>>> -		skge->wol = wol_supported(hw) & WAKE_MAGIC;
>>> +	skge->wol = wol_supported(hw) & WAKE_MAGIC;
>>> +	if (skge->wol && device_can_wakeup(&hw->pdev->dev))
>>> +		device_set_wakeup_enable(&hw->pdev->dev, true);
>>>
>>>  	hw->dev[port] = dev;
>> Hmm, looks like this patch is against a 2.6.28.8 tree with the commit
>> reverted. I merged the couple lines as following.
>>
>> line 3859:
>>
>>         // if (pci_wake_enabled(hw->pdev))
>>         //      skge->wol = wol_supported(hw) & WAKE_MAGIC;
>>         skge->wol = wol_supported(hw) & WAKE_MAGIC;
>>         if (skge->wol && device_can_wakeup(&hw->pdev->dev))
>>                 device_set_wakeup_enable(&hw->pdev->dev, true);
>>
>> and this really seems to fix the problem. The system is able to wake
>> up via etherwake just fine.
> 
> Well, that's because commit 5177b3240a6608fc0c9c05cc32f4855c6540f8d5 changed
> the default from "wake-up enabled by default" to "wake-up disabled by default".
> The latter is safer IMO, but I'll prepare a (better) patch to change it back to
> the old behavior.
> 
> You should be able to set up WoL by running 'ethtool -s eth0 wol g' from your
> init scripts even without reverting the patch.  Please verify if that works.

--
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 March 19, 2009, 8:59 p.m. UTC | #5
On Thursday 19 March 2009, Tomi Orava wrote:
> Hi,
> 
> > Hm, are you sure the commit in question _is_ in 2.6.28.y?  I can't see it in
> > there.  It is in the current Linus' tree, but not in v2.6.28 and surely not in
> > -stable.  Do you use a distro kernel?
> 
> Ok, I've now verified that I did indeed mess up with the commit
> mentioned previously. In other words the stable series 2.6.28.x
> doesn't support the suspend feature on my Asus A7V880 at all.
> The bios option is not respected at all and the command
> 'ethtool -s eth0 wol g' doesn't enable the suspend any better.
> 
> However, the commit taken from main branch actually makes also
> the stable series to work with this motherboard. No additional
> changes are needed, it just works out of the box.
> 
> Sorry about the mess up.

No problem.

Does it mean we can close the bug?

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
Tomi Orava March 20, 2009, 3:59 a.m. UTC | #6
On 03/19/2009 10:59 PM, Rafael J. Wysocki wrote:
> On Thursday 19 March 2009, Tomi Orava wrote:
>> Hi,
>>
>>> Hm, are you sure the commit in question _is_ in 2.6.28.y?  I can't see it in
>>> there.  It is in the current Linus' tree, but not in v2.6.28 and surely not in
>>> -stable.  Do you use a distro kernel?
>> Ok, I've now verified that I did indeed mess up with the commit
>> mentioned previously. In other words the stable series 2.6.28.x
>> doesn't support the suspend feature on my Asus A7V880 at all.
>> The bios option is not respected at all and the command
>> 'ethtool -s eth0 wol g' doesn't enable the suspend any better.
>>
>> However, the commit taken from main branch actually makes also
>> the stable series to work with this motherboard. No additional
>> changes are needed, it just works out of the box.
>>
>> Sorry about the mess up.
> 
> No problem.
> 
> Does it mean we can close the bug?

Yes, as it does look like the next stable release should
contain the fix by default.

Thanks & Regards,
Tomi Orava
--
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
diff mbox

Patch

Index: linux-2.6/drivers/net/skge.c
===================================================================
--- linux-2.6.orig/drivers/net/skge.c
+++ linux-2.6/drivers/net/skge.c
@@ -3856,8 +3856,9 @@  static struct net_device *skge_devinit(s
 	skge->speed = -1;
 	skge->advertising = skge_supported_modes(hw);
 
-	if (device_may_wakeup(&hw->pdev->dev))
-		skge->wol = wol_supported(hw) & WAKE_MAGIC;
+	skge->wol = wol_supported(hw) & WAKE_MAGIC;
+	if (skge->wol && device_can_wakeup(&hw->pdev->dev))
+		device_set_wakeup_enable(&hw->pdev->dev, true);
 
 	hw->dev[port] = dev;