diff mbox

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

Message ID 200809242113.52256.rjw@sisk.pl
State RFC, archived
Delegated to: Jeff Garzik
Headers show

Commit Message

Rafael J. Wysocki Sept. 24, 2008, 7:13 p.m. UTC 
On Wednesday, 24 of September 2008, Tino Keitel wrote:
> 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?

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

Comments

Tino Keitel Sept. 24, 2008, 7:49 p.m. UTC | #1 
On Wed, Sep 24, 2008 at 21:13:51 +0200, Rafael J. Wysocki wrote:
> On Wednesday, 24 of September 2008, Tino Keitel wrote:
> > 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?
> 
> Please try the appended patch.

With this patch, the computer hangs during suspend.

Regards,
Tino
--
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 Sept. 24, 2008, 9:10 p.m. UTC | #2 
On Wednesday, 24 of September 2008, Tino Keitel wrote:
> On Wed, Sep 24, 2008 at 21:13:51 +0200, Rafael J. Wysocki wrote:
> > On Wednesday, 24 of September 2008, Tino Keitel wrote:
> > > 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?
> > 
> > Please try the appended patch.
> 
> With this patch, the computer hangs during suspend.

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
--
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
Tino Keitel Sept. 25, 2008, 7:42 p.m. UTC | #3 
On Wed, Sep 24, 2008 at 23:10:22 +0200, Rafael J. Wysocki wrote:

[...]

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

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

http://tikei.de/dmesg_suspend

> 
> Please also attach the boot log from your system.

dmesg output after boot:

http://tikei.de/dmesg

Regards,
Tino
--
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
Tino Keitel Sept. 25, 2008, 8:03 p.m. UTC | #4 
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
--
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

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 chipid, char *buf, int sz)
 {
@@ -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);
@@ -4479,7 +4470,7 @@  static void __devexit sky2_remove(struct
 static int sky2_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct sky2_hw *hw = pci_get_drvdata(pdev);
-	int i, wol = 0;
+	int i;
 
 	if (!hw)
 		return 0;
@@ -4497,8 +4488,6 @@  static int sky2_suspend(struct pci_dev *
 
 		if (sky2->wol)
 			sky2_wol_init(sky2);
-
-		wol |= sky2->wol;
 	}
 
 	sky2_write32(hw, B0_IMSK, 0);
@@ -4506,8 +4495,7 @@  static int sky2_suspend(struct pci_dev *
 	sky2_power_aux(hw);
 
 	pci_save_state(pdev);
-	pci_enable_wake(pdev, pci_choose_state(pdev, state), wol);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
+	pci_prepare_to_sleep(pdev);
 
 	return 0;
 }
@@ -4520,7 +4508,7 @@  static int sky2_resume(struct pci_dev *p
 	if (!hw)
 		return 0;
 
-	err = pci_set_power_state(pdev, PCI_D0);
+	err = pci_back_from_sleep(pdev);
 	if (err)
 		goto out;
 
@@ -4528,8 +4516,6 @@  static int sky2_resume(struct pci_dev *p
 	if (err)
 		goto out;
 
-	pci_enable_wake(pdev, PCI_D0, 0);
-
 	/* Re-enable all clocks */
 	if (hw->chip_id == CHIP_ID_YUKON_EX ||
 	    hw->chip_id == CHIP_ID_YUKON_EC_U ||
@@ -4588,8 +4574,8 @@  static void sky2_shutdown(struct pci_dev
 	if (wol)
 		sky2_power_aux(hw);
 
-	pci_enable_wake(pdev, PCI_D3hot, wol);
-	pci_enable_wake(pdev, PCI_D3cold, wol);
+	if (pci_enable_wake(pdev, PCI_D3cold, wol))
+		pci_enable_wake(pdev, PCI_D3hot, wol);
 
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);