sky2 WOL fix
PATCH 1/1 sky2 WOL fix
This patch fixes the regression introduced with 2.6.38 on my system, the patch is safe as the reintroduced code is only executed after the motherboard model is identified by dmi.
Probably the dmi_system_id list will grow, a hot candidate is the ASUS P5LD2 motherboard, see Debian bug #647560
Nick: Try the patch with your system added to dmi_system_id need_legacy. Feel free to send the output of dmidecode if you cannot do that yourself.
cu,
Knut
Quoted 1 line
From
f3a7086638b0ee1f9d9e9028a188d96646671088Mon Sep 17 00:00:00 2001
From: Knut Petersen <[email protected]>
Date: Tue, 14 Feb 2012 08:35:17 +0100
Subject: [PATCH] Fix an old sky2 WOL regression
Wake-On-Lan was broken for a few systems with commit 87b09f1f25cd1e01d7c50bf423c7fe33027d7511 more than a year ago.
This commit fixes the regression for the AOpen i915GMm-HFS motherboard. It reintroduces the old code, but that code is only executed for those system included in a dmi_system_id list. Currently there is only one entry in that list, but it probably will grow.
--- drivers/net/ethernet/marvell/sky2.c | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-)
drivers/net/ethernet/marvell/sky2.c +21 -0
@@ -44,6 +44,7 @@#include <linux/prefetch.h> #include <linux/debugfs.h> #include <linux/mii.h> +#include <linux/dmi.h> #include <asm/irq.h>
@@ -807,6 +808,18 @@ static void sky2_phy_reinit(struct sky2_port *sky2)spin_unlock_bh (&sky2->phy_lock); } +/* A few systems do need PCI_Y2_PME_LEGACY for WOL, add them here */ +static const struct dmi_system_id need_legacy[] = { + { + .ident ="AOpen i915GMm-HFS" , + .matches = { +DMI_MATCH (DMI_BOARD_VENDOR ,"AOpen" ), +DMI_MATCH (DMI_BOARD_NAME ,"i915GMm-HFS" ), + }, + }, + {NULL } +}; +/* Put device in state to listen for Wake On Lan */ static void sky2_wol_init (struct sky2_port *sky2) {
@@ -814,6 +827,7 @@ static void sky2_wol_init(struct sky2_port *sky2)unsigned port = sky2->port;enum flow_control save_mode;u16 ctrl; +u32 reg1;/* Bring hardware out of reset */ sky2_write16 (hw,B0_CTST ,CS_RST_CLR );
@@ -867,6 +881,13 @@ static void sky2_wol_init(struct sky2_port *sky2)/* Disable PiG firmware */ sky2_write16 (hw,B0_CTST ,Y2_HW_WOL_OFF ); +/* set legacy pm bit on those systems that need it */ +if (dmi_check_system (need_legacy)) { + 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 );
Signature · 1 line
1.7.9
Quoted 9 lines · On Tue, 14 Feb 2012 09:25:02 +0100 Knut Petersen <[email protected]>…
From
f3a7086638b0ee1f9d9e9028a188d96646671088Mon Sep 17 00:00:00 2001
From: Knut Petersen <[email protected]>
Date: Tue, 14 Feb 2012 08:35:17 +0100
Subject: [PATCH] Fix an old sky2 WOL regressionWake-On-Lan was broken for a few systems with commit
87b09f1f25cd1e01d7c50bf423c7fe33027d7511more than a year ago.
Quoted 7 lines
This commit fixes the regression for the AOpen i915GMm-HFS motherboard. It reintroduces the old code, but that code is only executed for those system included in a
dmi_system_idlist. Currently there is only one entry in that list, but it probably will grow.Signed-off-by Knut Petersen <[email protected]>
You might also want to check with AOpen to see if there is a BIOS update. Often the BIOS programs bits on the onboard card to deal with power management issues.
Am 15.02.2012 18:16, schrieb Stephen Hemminger:
Quoted 18 lines
Quoted 15 lines · On Tue, 14 Feb 2012 09:25:02 +0100 Knut Petersen<[email protected]>…
From
f3a7086638b0ee1f9d9e9028a188d96646671088Mon Sep 17 00:00:00 2001
From: Knut Petersen<[email protected]>
Date: Tue, 14 Feb 2012 08:35:17 +0100
Subject: [PATCH] Fix an old sky2 WOL regressionWake-On-Lan was broken for a few systems with commit
87b09f1f25cd1e01d7c50bf423c7fe33027d7511more than a year ago.This commit fixes the regression for the AOpen i915GMm-HFS motherboard. It reintroduces the old code, but that code is only executed for those system included in a
dmi_system_idlist. Currently there is only one entry in that list, but it probably will grow.
Quoted 5 lines
Quoted 1 line
Signed-off-by Knut Petersen<[email protected]>You might also want to check with AOpen to see if there is a BIOS update. Often the BIOS programs bits on the onboard card to deal with power management issues.
[I added some people to the cc list that commented on bug #19492]
The last BIOS AOpen published for that board is more than six years old. No. I will not bother them with this problem.
Your commit broke existing support for WOL on a number of boards. This is known to you and to bugzilla.kernel org since 2010. A number of people complained about that, but nothing happened.
Neither your original commit description nor the description of the PXE_LegNat_Sel I found at other places indicate that setting PXE_LegNat_Sel could do any harm.
Stephen: It´s a real regression, the fix is known. If you do not want to go back to setting PXE_LegNat_Sel, you really should allow dmi_matched fixes.
Mirko, Ralph: Any comments from Marvell?
cu,
Knut
Quoted 7 lines · On Fri, 17 Feb 2012 19:21:53 +0100 Knut Petersen <[email protected]>…
Your commit broke existing support for WOL on a number of boards. This is known to you and to bugzilla.kernel org since 2010. A number of people complained about that, but nothing happened.
Neither your original commit description nor the description of the
PXE_LegNat_SelI found at other places indicate that settingPXE_LegNat_Selcould do any harm.
Please lose the attitude. Any support of sky2 is done as a minor side project by me. People find a problem, I put in one fix, then someone else complains and another fix goes in. The regressions happen a lot in power management because the hardware is not well documented in this area and it is a complex interaction of kernel, BIOS, motherboard, and general shittiness of implementations. Because of that I am more likely to believe that what the Marvell code does is more based on experience on other platforms. The out-of-tree Marvell driver contains bits that are part of the Windows driver.
Quoted 12 lines · Stephen Hemminger
Quoted 7 lines · On Fri, 17 Feb 2012 19:21:53 +0100 Knut Petersen <[email protected]>…
Your commit broke existing support for WOL on a number of boards. This is known to you and to bugzilla.kernel org since 2010. A number of people complained about that, but nothing happened.
Neither your original commit description nor the description of the
PXE_LegNat_SelI found at other places indicate that settingPXE_LegNat_Selcould do any harm.
Quoted 8 lines
Please lose the attitude. Any support of sky2 is done as a minor side project by me. People find a problem, I put in one fix, then someone else complains and another fix goes in. The regressions happen a lot in power management because the hardware is not well documented in this area and it is a complex interaction of kernel, BIOS, motherboard, and general shittiness of implementations. Because of that I am more likely to believe that what the Marvell code does is more based on experience on other platforms. The out-of-tree Marvell driver contains bits that are part of the Windows driver.
Attitudes aside, the general problem here is that someone (which I believe is you, Stephen, based solely on the commit) committed a patch two years ago which broke WoL support for a number of users. I have no doubt that you had some reason for making these changes, but a bug was filed about this in 10/02/2010, followed by identification and confirmation of both the problem and a solution 1 week later, and absolutely nothing has been done since then. Here's the bug:
https://bugzilla.kernel.org/show_bug.cgi?id=19492
I think we all understand that you volunteer your time for this, and I'm most appreciative of your (and all kernel developer's) efforts, but that's a long time for a known and easily fixable regression to be ignored, without even a single comment on the issue since the fix was identified. And I will admit that I'm also pretty frustrated by the need to re-patch the kernel on my server every single time I update it to fix an obvious and known regression that, prior 2.6.34, worked perfectly fine.
To Knut's points, my own motherboard (from Asus) is no longer supported by the vendor either at this point, so a fix from that side is not likely. Is there any particular reason, any known side effects, why this newer patch from Knut cannot be applied? As thing's stand now, there is already a definite problem, so unless there's a risk of creating a larger, more widespread problem I think it'd make sense to apply the patch and fix the current issue.
Clearly I'm no developer, but as a user that's been affected by this problem for over a year and a half now I thought it worth chiming in. I hope some resolution can be reached on this.
Thanks.
Signature · 1 line
Jared
Quoted 30 lines · On Fri, 17 Feb 2012 14:13:05 -0600 Jared <[email protected]> wrote:
Quoted 12 lines · Stephen Hemminger
Quoted 7 lines · On Fri, 17 Feb 2012 19:21:53 +0100 Knut Petersen <[email protected]>…
Your commit broke existing support for WOL on a number of boards. This is known to you and to bugzilla.kernel org since 2010. A number of people complained about that, but nothing happened.
Neither your original commit description nor the description of the
PXE_LegNat_SelI found at other places indicate that settingPXE_LegNat_Selcould do any harm.Quoted 8 lines
Please lose the attitude. Any support of sky2 is done as a minor side project by me. People find a problem, I put in one fix, then someone else complains and another fix goes in. The regressions happen a lot in power management because the hardware is not well documented in this area and it is a complex interaction of kernel, BIOS, motherboard, and general shittiness of implementations. Because of that I am more likely to believe that what the Marvell code does is more based on experience on other platforms. The out-of-tree Marvell driver contains bits that are part of the Windows driver.
Attitudes aside, the general problem here is that someone (which I believe is you, Stephen, based solely on the commit) committed a patch two years ago which broke WoL support for a number of users. I have no doubt that you had some reason for making these changes, but a bug was filed about this in 10/02/2010, followed by identification and confirmation of both the problem and a solution 1 week later, and absolutely nothing has been done since then. Here's the bug:
Quoted 1 line
Kernel bugzilla is ignored by network developers.
Sorry, but that's life.
Quoted 17 lines
I think we all understand that you volunteer your time for this, and I'm most appreciative of your (and all kernel developer's) efforts, but that's a long time for a known and easily fixable regression to be ignored, without even a single comment on the issue since the fix was identified. And I will admit that I'm also pretty frustrated by the need to re-patch the kernel on my server every single time I update it to fix an obvious and known regression that, prior 2.6.34, worked perfectly fine.
To Knut's points, my own motherboard (from Asus) is no longer supported by the vendor either at this point, so a fix from that side is not likely. Is there any particular reason, any known side effects, why this newer patch from Knut cannot be applied? As thing's stand now, there is already a definite problem, so unless there's a risk of creating a larger, more widespread problem I think it'd make sense to apply the patch and fix the current issue.
Quoted 3 lines
Clearly I'm no developer, but as a user that's been affected by this problem for over a year and a half now I thought it worth chiming in. I hope some resolution can be reached on this.
Going back to the bug report:
05:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8053 PCI-E Gigabit Ethernet Controller (rev 15)
Subsystem: ASUSTeK Computer Inc. Marvell 88E8053 Gigabit Ethernet
controller PCIe (Asus)
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 32 bytes
Interrupt: pin A routed to IRQ 36
Region 0: Memory at d9000000 (64-bit, non-prefetchable) [size=16K] Region 2: I/O ports at 9000 [size=256]
Expansion ROM at d8000000 [disabled] [size=128K]
Capabilities: [48] Power Management version 2
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mAPME(D0+,D1+,D2+,D3hot+,D3cold+)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=1 PME-
The root cause of the system (in the bug report) not doing WOL is that the device has not been configured to enable Power Management Enable. This is a BIOS function and the driver (like most drivers) gets the setting of PCI wakeup from core PCI layer. The core PCI gets it's values from the BIOS.
The documentation of that bit is for Bit 15 of "Our Register 1"
0 = PCI Express PME mechanism: Wake# can be asserted only
when Vmain is not available (in L2 Link state). Once Wake# has been asserted, it must continue to be driven low until Vmain is restored. After deassertion of Wake# the PCI Express link is initialized. The the adapter sends a PM_PME message signaling the system to clear the PME status bit.
1 = Debug mode of PME mechanism. This is only a debug mode.
The behavior of the WAKE# pin is dependent on wake events and is independent of Vmain. No PM_PME message is generated in debug mode. The PME mechanism is the same as in PCI or PCI-X mode.
Reset by Power On Reset
So you patch is just setting the bit that overrides proper PME support and allows wake to work even if disabled in BIOS.