Skip to contentSkip to conversation

Fix an old sky2 WOL regression

PATCH In review Knut Petersen 5 participants 10 messages Mar 21, 2012 – May 6, 2012

Subsystems (3)
Networking Network drivers Marvell gigabit Ethernet drivers (skge/sky2)
lore
Share & tools
lore

Knut Petersen

author
#0 lore raw

PATCH 1/1 Fix an old sky2 WOL regression

Sky2 Wake on LAN is broken since February 2010 on a number of systems. Yes. More than two years.

We know about the problem and the cause since October 2010 (Bugzilla bug #19492). It´s commit 87b09f1f25cd1e01d7c50bf423c7fe33027d7511.

Stephen, David: You signed off that commit.

Andrew: You called it a regression in October 2010.

It has been proposed to revert the commit that caused the problem. Nothing happened.

I proposed to re-establish the old code for dmi_match()ed systems. Without success.

Now it is proposed to re-establish the old code as a configuration option. If nothing happens again I will propose a module parameter ;-)

Stephen, I don´t want to be a pain in the neck, and it is not my intention to offend you by my "attitude". But I simply cannot understand why this know regression is not fixed. The bit we talk about is documented, and in fact it was set for a number of kernel versions unconditionally. Nobody complained about ruined hardware or minor problems.

The systems affected are old enough that no manufacturer cares about them, but they are still quite usable for a lot of jobs (kernel 3.3 compile time here is below 15 minutes).

If there is a problem in the kernel and if we do know an easy solution, that solution should be commited to the kernel, no matter what is written in some random documentation, no matter if we could blame some BIOS authors. That´s the way Linux works - at least I thought so.

cu,
Knut

Quoted 1 line

From 7fae2839363277835a0ac1f3f75a788aa299e0a1 Mon Sep 17 00:00:00 2001

From: Knut Petersen <[email protected]>
Date: Wed, 21 Mar 2012 07:42:20 +0100
Subject: [PATCH] Fix an old sky2 WOL regression

Commit 87b09f1f25cd1e01d7c50bf423c7fe33027d7511 removed code to enable legacy power management mode during wake on lan initialization. According to the documentation this was the right thing to do, but it definitely broke wake on lan on a number of systems.

The regression and the cause of the problem has been reported more than _18_ months ago, see Bugzilla bug 19492. We should fix it now.

This patch introduces a kernel configuration option that allows to select inclusion of code to set legacy PM mode during wake on lan init.

Reported-by Arkadiusz Miskiewicz <[email protected]>
Signed-off-by Knut Petersen <[email protected]>
---
 drivers/net/ethernet/marvell/Kconfig |   10 ++++++++++
 drivers/net/ethernet/marvell/sky2.c  |   11 ++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)
drivers/net/ethernet/marvell/Kconfig +10 -0
index 0029934..4f97020 100644
@@ -98,6 +98,16 @@ config SKY2
 	  To compile this driver as a module, choose M here: the module
 	  will be called sky2.  This is recommended.
 
+config SKY2_PME_LEGACY
+	bool "WOL support for broken BIOSes"
+	depends on SKY2 && DEBUG_FS
+	---help---
+	  If this option is set, Yukon 2 legacy power management mode will
+	  be enabled during wake on lan setup. Some BIOS implementations
+	  need this for actual WOL support.
+
+	  If unsure, say N.
+
 config SKY2_DEBUG
 	bool "Debugging interface"
 	depends on SKY2 && DEBUG_FS
drivers/net/ethernet/marvell/sky2.c +10 -1
index 760c2b1..832589c 100644
@@ -814,7 +814,9 @@ static void sky2_wol_init(struct sky2_port *sky2)
 	unsigned port = sky2->port;
 	enum flow_control save_mode;
 	u16 ctrl;
-
+#ifdef CONFIG_SKY2_PME_LEGACY
+	u32 reg1;
+#endif
 	/* Bring hardware out of reset */
 	sky2_write16(hw, B0_CTST, CS_RST_CLR);
 	sky2_write16(hw, SK_REG(port, GMAC_LINK_CTRL), GMLC_RST_CLR);

@@ -867,6 +869,13 @@ static void sky2_wol_init(struct sky2_port *sky2)
 	/* Disable PiG firmware */
 	sky2_write16(hw, B0_CTST, Y2_HW_WOL_OFF);
 
+#ifdef CONFIG_SKY2_PME_LEGACY
+	/* Needed by some broken BIOSes*/
+	reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
+	reg1 |= PCI_Y2_PME_LEGACY;
+	sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
+#endif
+
 	/* block receiver */
 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
 	sky2_read32(hw, B0_CTST);
Signature · 1 line
1.7.9.2
#1 lore raw
Quoted 29 lines · On Wed, 21 Mar 2012 12:40:30 +0100 Knut Petersen <[email protected]>…

Sky2 Wake on LAN is broken since February 2010 on a number of systems. Yes. More than two years.

We know about the problem and the cause since October 2010 (Bugzilla bug #19492). It´s commit 87b09f1f25cd1e01d7c50bf423c7fe33027d7511.

Stephen, David: You signed off that commit.

Andrew: You called it a regression in October 2010.

It has been proposed to revert the commit that caused the problem. Nothing happened.

I proposed to re-establish the old code for dmi_match()ed systems. Without success.

Now it is proposed to re-establish the old code as a configuration option. If nothing happens again I will propose a module parameter ;-)

Stephen, I don´t want to be a pain in the neck, and it is not my intention to offend you by my "attitude". But I simply cannot understand why this know regression is not fixed. The bit we talk about is documented, and in fact it was set for a number of kernel versions unconditionally. Nobody complained about ruined hardware or minor problems.

The systems affected are old enough that no manufacturer cares about them, but they are still quite usable for a lot of jobs (kernel 3.3 compile time here is below 15 minutes).

Quoted 7 lines

If there is a problem in the kernel and if we do know an easy solution, that solution should be commited to the kernel, no matter what is written in some random documentation, no matter if we could blame some BIOS authors. That´s the way Linux works - at least I thought so.

cu,
Knut

Config options don't work for distro's.

#2 lore raw

PATCH 1/1 sky2: override for PCI legacy power management

In reply to Knut Petersen

Some BIOS's don't setup power management correctly (what else is new) and don't allow use of PCI Express power control. Add a special exception module parameter to allow working around this issue. Based on slightly different patch by Knut Petersen.

Reported-by Arkadiusz Miskiewicz <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>

---
Patch against -net (ie. 3.3.0)

drivers/net/ethernet/marvell/sky2.c +11 -0
@@ -95,6 +95,10 @@ static int disable_msi = 0;
 module_param(disable_msi, int, 0);
 MODULE_PARM_DESC(disable_msi, "Disable Message Signaled Interrupt (MSI)");
 
+static int legacy_pme = 0;
+module_param(legacy_pme, int, 0);
+MODULE_PARM_DESC(legacy_pme, "Legacy power management");
+
 static DEFINE_PCI_DEVICE_TABLE(sky2_id_table) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9000) }, /* SK-9Sxx */
 	{ PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9E00) }, /* SK-9Exx */

@@ -867,6 +871,13 @@ static void sky2_wol_init(struct sky2_po
 	/* Disable PiG firmware */
 	sky2_write16(hw, B0_CTST, Y2_HW_WOL_OFF);
 
+	/* Needed by some broken BIOSes, use PCI rather than PCI-e for WOL */
+	if (legacy_pme) {
+		u32 reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
+		reg1 |= PCI_Y2_PME_LEGACY;
+		sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
+	}
+
 	/* block receiver */
 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
 	sky2_read32(hw, B0_CTST);
#4 lore raw

Re: [PATCH] sky2: override for PCI legacy power management

In reply to Stephen Hemminger
Quoted 7 lines · Stephen Hemminger

Some BIOS's don't setup power management correctly (what else is new) and don't allow use of PCI Express power control. Add a special exception module parameter to allow working around this issue. Based on slightly different patch by Knut Petersen.

Reported-by Arkadiusz Miskiewicz <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>

Is there a problem report URL you can include here?

It looks like this requires a user to figure out that he might be suffering from this problem, then use this module parameter to work around it. How would a user figure that out? Can we do it automatically to save him the trouble?

Quoted 35 lines

---
Patch against -net (ie. 3.3.0)

drivers/net/ethernet/marvell/sky2.c       2012-03-21 08:25:52.400929532 -0700 renamed from drivers/net/ethernet/marvell/sky2.c       2012-01-10 10:56:56.855156017 -0800 +4 -0
@@ -95,6 +95,10 @@ static int disable_msi = 0;
  module_param(disable_msi, int, 0);
  MODULE_PARM_DESC(disable_msi, "Disable Message Signaled Interrupt (MSI)");
 
+static int legacy_pme = 0;
+module_param(legacy_pme, int, 0);
+MODULE_PARM_DESC(legacy_pme, "Legacy power management");
+
  static DEFINE_PCI_DEVICE_TABLE(sky2_id_table) = {

       { PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9000) }, /* SK-9Sxx */ { PCI_DEVICE(PCI_VENDOR_ID_SYSKONNECT, 0x9E00) }, /* SK-9Exx */

drivers/net/ethernet/marvell/sky2.c       2012-03-21 08:25:52.400929532 -0700 renamed from drivers/net/ethernet/marvell/sky2.c       2012-01-10 10:56:56.855156017 -0800 +7 -1
@@ -867,6 +871,13 @@ static void sky2_wol_init(struct sky2_po
        /* Disable PiG firmware */
        sky2_write16(hw, B0_CTST, Y2_HW_WOL_OFF);
 
+       /* Needed by some broken BIOSes, use PCI rather than PCI-e for WOL */
+       if (legacy_pme) {
+               u32 reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
+               reg1 |= PCI_Y2_PME_LEGACY;
+               sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
+       }
+
        /* block receiver */
        sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
        sky2_read32(hw, B0_CTST);
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

#5 lore raw

Re: [PATCH] sky2: override for PCI legacy power management

Quoted 10 lines · On Wed, 21 Mar 2012 14:22:01 -0600 Bjorn Helgaas <[email protected]> wrote:
Quoted 7 lines · Stephen Hemminger

Some BIOS's don't setup power management correctly (what else is new) and don't allow use of PCI Express power control. Add a special exception module parameter to allow working around this issue. Based on slightly different patch by Knut Petersen.

Reported-by Arkadiusz Miskiewicz <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>
Quoted 6 lines

Is there a problem report URL you can include here?

It looks like this requires a user to figure out that he might be suffering from this problem, then use this module parameter to work around it. How would a user figure that out? Can we do it automatically to save him the trouble?

I am not a power management expert. Looks like a BIOS issue where the BIOS has configured the device to disable power management but the user wants to override that value. There is no method to determine when the BIOS is broken versus when the BIOS setting is correct and we should follow what it says.

#6 lore raw

Re: [PATCH] sky2: override for PCI legacy power management

Quoted 20 lines · Stephen Hemminger
Quoted 10 lines · On Wed, 21 Mar 2012 14:22:01 -0600 Bjorn Helgaas <[email protected]> wrote:
Quoted 7 lines · Stephen Hemminger

Some BIOS's don't setup power management correctly (what else is new) and don't allow use of PCI Express power control. Add a special exception module parameter to allow working around this issue. Based on slightly different patch by Knut Petersen.

Reported-by Arkadiusz Miskiewicz <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>
Quoted 6 lines

Is there a problem report URL you can include here?

It looks like this requires a user to figure out that he might be suffering from this problem, then use this module parameter to work around it.  How would a user figure that out?  Can we do it automatically to save him the trouble?

Quoted 5 lines

I am not a power management expert. Looks like a BIOS issue where the BIOS has configured the device to disable power management but the user wants to override that value. There is no method to determine when the BIOS is broken versus when the BIOS setting is correct and we should follow what it says.

I'm not a power management expert either. I was just wondering whether the known broken BIOSes could be encoded in a blacklist or something, because it looks like a case where a user might have to request help or debug the problem again before discovering this flag.

Bjorn

author
#7 lore raw

Re: [PATCH] sky2: override for PCI legacy power management

In reply to Bjorn Helgaas

Am 21.03.2012 21:22, schrieb Bjorn Helgaas:

Quoted 1 line

It looks like this requires a user to figure out that he might be suffering from this problem, then use this module parameter to work around it. How would a user figure that out? Can we do it automatically to save him the trouble?

It´s easy to dmi_match() known broken systems - I have dmidecode outputs of four systems that definitely need the patch. Two ASUSTek P5* mainboards with AMI BIOSes, two AOpen i915G* mainboards with Award/Phoenix BIOSes.

cu,
Knut

#8 lore raw

Re: [PATCH] sky2: override for PCI legacy power management

Knut Petersen wrote, a few months ago:

Quoted 5 lines

It´s easy to dmi_match() known broken systems - I have dmidecode outputs of four systems that definitely need the patch.

Two ASUSTek P5* mainboards with AMI BIOSes, two AOpen i915G* mainboards with Award/Phoenix BIOSes.

Yes, please. Could you attach those to [1]?

Thanks,
Jonathan

[1] https://bugzilla.kernel.org/show_bug.cgi?id=19492

maintainer
#9 lore raw

Re: [PATCH] sky2: override for PCI legacy power management

In reply to Stephen Hemminger

From: Stephen Hemminger <[email protected]>
Date: Wed, 21 Mar 2012 08:32:05 -0700

Quoted 7 lines

Some BIOS's don't setup power management correctly (what else is new) and don't allow use of PCI Express power control. Add a special exception module parameter to allow working around this issue. Based on slightly different patch by Knut Petersen.

Reported-by Arkadiusz Miskiewicz <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>

Applied and queued up for -stable.