Re: Regression in 2.6.27-rc7: Wake On LAN with sky2 broken

Previous thread: ACPI git tree for linux-next by Len Brown on Wednesday, September 24, 2008 - 12:35 am. (2 messages)

Next thread: Re: [PATCH 4/10] xfs: Fix error handling inwrite_super_lockfs/unlockfs by Takashi Sato on Wednesday, September 24, 2008 - 1:14 am. (1 message)
From: Tino Keitel
Date: Wednesday, September 24, 2008 - 1:05 am

Hi,

Wake On LAN from suspend to RAM doesn't work on my Mac mini Core Duo
with 2.6.27-rc7. I tested the same configuration with 2.6.26.3, and it
worked. With 2.6.27-rc7, the computer stays in suspended state. I can
still wake it up using the keyboard or power switch.

In my suspend script, I use the command "ethtool -s eth0 wol g" to
enable WOL even if it was disabled before.

Any hints what commit I should try to revert, before I start bisecting?

Regards,
Tino
--

From: Rafael J. Wysocki
Date: Wednesday, September 24, 2008 - 12:13 pm

Please try the appended patch.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>

sky2: Fix wake-on-LAN regression

* Since dev->power.should_wakeup bit is used by the PCI core to
  decide whether the device should wake up the system from sleep
  states, set/unset this bit whenever WOL is enabled/disabled using
  sky2_set_wol().

* Use pci_prepare_to_sleep() and pci_back_from_sleep() in
  ->sky2_suspend() and ->sky2_resume(), respectively, instead of the
  older less suitable interfaces.

* Prevent pci_enable_wake() from being called twice unnecessarily in
  sky2_shutdown().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

 drivers/net/sky2.c |   34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff -puN drivers/net/sky2.c~skty2-adapt-to-the-reworked-pci-pm drivers/net/sky2.c
--- a/drivers/net/sky2.c~skty2-adapt-to-the-reworked-pci-pm
+++ a/drivers/net/sky2.c
@@ -3034,7 +3034,8 @@ static int sky2_set_wol(struct net_devic
 	struct sky2_port *sky2 = netdev_priv(dev);
 	struct sky2_hw *hw = sky2->hw;
 
-	if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
+	if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
+	    || !device_can_wakeup(&hw->pdev->dev))
 		return -EOPNOTSUPP;
 
 	sky2->wol = wol->wolopts;
@@ -3045,6 +3046,8 @@ static int sky2_set_wol(struct net_devic
 		sky2_write32(hw, B0_CTST, sky2->wol
 			     ? Y2_HW_WOL_ON : Y2_HW_WOL_OFF);
 
+	device_set_wakeup_enable(&hw->pdev->dev, sky2->wol);
+
 	if (!netif_running(dev))
 		sky2_wol_init(sky2);
 	return 0;
@@ -4179,18 +4182,6 @@ static int __devinit sky2_test_msi(struc
 	return err;
 }
 
-static int __devinit pci_wake_enabled(struct pci_dev *dev)
-{
-	int pm  = pci_find_capability(dev, PCI_CAP_ID_PM);
-	u16 value;
-
-	if (!pm)
-		return 0;
-	if (pci_read_config_word(dev, pm + PCI_PM_CTRL, &value))
-		return 0;
-	return value & PCI_PM_CTRL_PME_ENABLE;
-}
-
 /* This driver supports yukon2 chipset only */
 static const char *sky2_name(u8 ...
From: Tino Keitel
Date: Wednesday, September 24, 2008 - 12:49 pm

With this patch, the computer hangs during suspend.

Regards,
Tino
--

From: Rafael J. Wysocki
Date: Wednesday, September 24, 2008 - 2:10 pm

This is not good.  It probably means that it actually tries to set up the NIC
for waking up the system, but something broke _that_ for you.

FWIW, I tried it on my test machine with sky2 and it worked.

Can you try:

# ethtool -s eth0 wol g
# echo devices > /sys/power/pm_test
# echo mem > /sys/power/state
(this should wait for 5 seconds and return to the command line)

and see if the machine hangs during this operation?

Please also attach the boot log from your system.

Thanks,
Rafael
--

From: Tino Keitel
Date: Thursday, September 25, 2008 - 12:42 pm

On Wed, Sep 24, 2008 at 23:10:22 +0200, Rafael J. Wysocki wrote:


It didn't hang. The dmesg output from the suspend attempt is here:


dmesg output after boot:

http://tikei.de/dmesg

Regards,
Tino
--

From: Tino Keitel
Date: Thursday, September 25, 2008 - 1:03 pm

Hi,

I just found a method how I can enable WOL again:

echo enabled >
/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0/power/wakeup

This device is the NIC device, of cause.

I already had another case (EHCI) where I needed to do this to be able
to wake the computer up with the USB keyboard. It seems like 2.6.27
behaves different here compared to 2.6.26, and things that worked with
2.6.26 need manual fixing to work again. Or did I make something wrong?

Regards,
Tino
--

From: Rafael J. Wysocki
Date: Thursday, September 25, 2008 - 1:38 pm

I guess the box didn't hang during suspend with this setting?


No, you didn't.

The behavior was changed by the PCI wake-up patches that fixed a couple of
problems and I'd like to fix all of the cases when the new behavior causes
issues like this to happen.

I'll send you a patch for the EHCI case later today.

Thanks,
Rafael

---
sky2: Fix WOL regression

Since dev->power.should_wakeup bit is used by the PCI core to
decide whether the device should wake up the system from sleep
states, set/unset this bit whenever WOL is enabled/disabled using
sky2_set_wol().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

 drivers/net/sky2.c |   34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff -puN drivers/net/sky2.c~skty2-adapt-to-the-reworked-pci-pm drivers/net/sky2.c
--- a/drivers/net/sky2.c~skty2-adapt-to-the-reworked-pci-pm
+++ a/drivers/net/sky2.c
@@ -3034,7 +3034,8 @@ static int sky2_set_wol(struct net_devic
 	struct sky2_port *sky2 = netdev_priv(dev);
 	struct sky2_hw *hw = sky2->hw;
 
-	if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
+	if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
+	    || !device_can_wakeup(&hw->pdev->dev))
 		return -EOPNOTSUPP;
 
 	sky2->wol = wol->wolopts;
@@ -3045,6 +3046,8 @@ static int sky2_set_wol(struct net_devic
 		sky2_write32(hw, B0_CTST, sky2->wol
 			     ? Y2_HW_WOL_ON : Y2_HW_WOL_OFF);
 
+	device_set_wakeup_enable(&hw->pdev->dev, sky2->wol);
+
 	if (!netif_running(dev))
 		sky2_wol_init(sky2);
 	return 0;
@@ -4314,7 +4305,7 @@ static int __devinit sky2_probe(struct p
 		}
 	}
 
-	wol_default = pci_wake_enabled(pdev) ? WAKE_MAGIC : 0;
+	wol_default = device_may_wakeup(&pdev->dev) ? WAKE_MAGIC : 0;
 
 	err = -ENOMEM;
 	hw = kzalloc(sizeof(*hw), GFP_KERNEL);

--

From: Tino Keitel
Date: Monday, September 29, 2008 - 2:18 pm

With the attached patch, WOL works as intended the hang also didn't
happen.

Thanks and regards,
Tino
--

Previous thread: ACPI git tree for linux-next by Len Brown on Wednesday, September 24, 2008 - 12:35 am. (2 messages)

Next thread: Re: [PATCH 4/10] xfs: Fix error handling inwrite_super_lockfs/unlockfs by Takashi Sato on Wednesday, September 24, 2008 - 1:14 am. (1 message)