Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2
Subsystems (2)
Hi,
wake on LAN doesn't work with 2.6.33-rc4-git2 on the following hardware:
01:00.0 Ethernet controller [0200]: Marvell Technology Group Ltd. 88E8053 PCI-E Gigabit Ethernet Controller [11ab:4362] (rev 22)
It worked with 2.6.32. In my suspend script, I enable WoL using ethtool:
ethtool -s eth0 wol g
in dmesg, I get these messages at suspend:
sky2 eth0: disabling interface
sky2 0000:01:00.0: PME# enabled
sky2 0000:01:00.0: wake-up capability enabled by ACPI
and at resume:
sky2 0000:01:00.0: wake-up capability disabled by ACPI sky2 0000:01:00.0: PME# disabled
Regards,
Tino
Quoted 19 lines · On Fri, 15 Jan 2010 10:10:53 +0100 Tino Keitel <[email protected]> wrote:
Hi,
wake on LAN doesn't work with 2.6.33-rc4-git2 on the following hardware:
01:00.0 Ethernet controller [0200]: Marvell Technology Group Ltd. 88E8053 PCI-E Gigabit Ethernet Controller [11ab:4362] (rev 22)
It worked with 2.6.32. In my suspend script, I enable WoL using ethtool:
ethtool -s eth0 wol g
in dmesg, I get these messages at suspend:
sky2 eth0: disabling interface
sky2 0000:01:00.0: PME# enabled
sky2 0000:01:00.0: wake-up capability enabled by ACPI
Quoted 7 lines
and at resume:
sky2 0000:01:00.0: wake-up capability disabled by ACPI sky2 0000:01:00.0: PME# disabled
Regards,
Tino
This has already been reported.
Rafael has been doing the generic PM stuff. Look for thread,
[Bug 14730] sky2 won't work after suspend/resume cycle
Quoted 27 lines · Stephen Hemminger
Quoted 24 lines · On Fri, 15 Jan 2010 10:10:53 +0100 Tino Keitel <[email protected]> wrote:
Hi,
wake on LAN doesn't work with 2.6.33-rc4-git2 on the following hardware:
01:00.0 Ethernet controller [0200]: Marvell Technology Group Ltd. 88E8053 PCI-E Gigabit Ethernet Controller [11ab:4362] (rev 22)
It worked with 2.6.32. In my suspend script, I enable WoL using ethtool:
ethtool -s eth0 wol g
in dmesg, I get these messages at suspend:
sky2 eth0: disabling interface
sky2 0000:01:00.0: PME# enabled
sky2 0000:01:00.0: wake-up capability enabled by ACPIand at resume:
sky2 0000:01:00.0: wake-up capability disabled by ACPI sky2 0000:01:00.0: PME# disabled
Quoted 8 lines
Quoted 2 lines
Regards,
TinoThis has already been reported.
Rafael has been doing the generic PM stuff. Look for thread,
[Bug 14730] sky2 won't work after suspend/resume cycle
This has been fixed already and the bug appears to be different.
No idea what it is at the moment.
Tino, please check if reverting commit dc1a94ae1749d14c55f8b54e9d92bd89df82d51a helps, although the messages indicate that the generic PCI-side does its job.
Rafael
Quoted 1 line · Rafael J. Wysocki
On Friday 15 January 2010, Stephen Hemminger wrote:
[...]
Quoted 7 lines
Quoted 5 lines
This has already been reported.
Rafael has been doing the generic PM stuff. Look for thread,
[Bug 14730] sky2 won't work after suspend/resume cycleThis has been fixed already and the bug appears to be different.
Yes, sounds different. The interface works after a resume. My issue is that the computer just won't resume at all using wake on LAN.
Quoted 6 lines
No idea what it is at the moment.
Tino, please check if reverting commit
dc1a94ae1749d14c55f8b54e9d92bd89df82d51ahelps, although the messages indicate that the generic PCI-side does its job.
No luck with the commit reverted, WoL still doesn't work.
Regards,
Tino
Quoted 17 lines · Tino Keitel
Quoted 1 line · Rafael J. Wysocki
On Friday 15 January 2010, Stephen Hemminger wrote:
[...]
Quoted 7 lines
Quoted 5 lines
This has already been reported.
Rafael has been doing the generic PM stuff. Look for thread,
[Bug 14730] sky2 won't work after suspend/resume cycleThis has been fixed already and the bug appears to be different.
Yes, sounds different. The interface works after a resume. My issue is that the computer just won't resume at all using wake on LAN.
Quoted 1 line
Quoted 7 lines
Quoted 5 lines
No idea what it is at the moment.
Tino, please check if reverting commit
dc1a94ae1749d14c55f8b54e9d92bd89df82d51ahelps, although the messages indicate that the generic PCI-side does its job.No luck with the commit reverted, WoL still doesn't work.
Well, this commit is the only PCI core change affecting WoL after 2.6.32 IIRC.
Rafael
Tino Keitel
Acked-by Rafael J. WysockiHi,
reverting this commit fixes WoL for me:
commit 166a0fd4c788ec7f10ca8194ec6d526afa12db75
Author: stephen hemminger <[email protected]> Date: Mon Dec 14 08:50:12 2009 +0000
sky2: leave PCI config space writeable
Since power management is done by PCI subsystem as well as driver, don't toggle the bit that disables PCI register writes.
Regards,
Tino
Quoted 5 lines · Tino Keitel
Hi,
reverting this commit fixes WoL for me:
commit
166a0fd4c788ec7f10ca8194ec6d526afa12db75
Hi Tino,
Does this fix WoL also? Compile tested only...
thanks,
Mike
Subject: [PATCH] sky2: Disable writes to PCI space before suspend
Tino Keitel found that reverting 166a0fd4c fixes WoL for him. This suggests that PCI config should not be writable when suspended.
--- drivers/net/sky2.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/sky2.c +3 -0
@@ -4789,6 +4789,9 @@ static int sky2_suspend(struct pci_dev *pdev, pm_message_t state)sky2_power_aux (hw);rtnl_unlock (); +/* disable writes to PCI config again */ +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF ); +pci_save_state (pdev);pci_enable_wake (pdev,pci_choose_state (pdev, state), wol);pci_set_power_state (pdev,pci_choose_state (pdev, state));
-- 1.5.6.5
Quoted 10 lines · Mike McCormack
Quoted 5 lines · Tino Keitel
Hi,
reverting this commit fixes WoL for me:
commit
166a0fd4c788ec7f10ca8194ec6d526afa12db75Hi Tino,
Does this fix WoL also? Compile tested only...
No, still no wakeup on WoL packets.
Regards,
Tino
Does it work better if config bit is set here?
--- drivers/net/sky2.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/sky2.c +3 -0
@@ -283,6 +283,9 @@ static void sky2_power_aux(struct sky2_h/* turn off "driver loaded LED" */ sky2_write16 (hw,B0_CTST ,Y2_LED_STAT_OFF ); + +/* disable writes to PCI config again */ +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF ); }static void sky2_gmac_reset (struct sky2_hw *hw,unsigned port)
Quoted 1 line · Stephen Hemminger
Does it work better if config bit is set here?
No, still the same problem.
Regards,
Tino
Quoted 8 lines · Tino Keitel
Hi,
reverting this commit fixes WoL for me:
commit
166a0fd4c788ec7f10ca8194ec6d526afa12db75Author: stephen hemminger <[email protected]> Date: Mon Dec 14 08:50:12 2009 +0000
Quoted 8 lines
sky2: leave PCI config space writeable
Since power management is done by PCI subsystem as well as driver, don't toggle the bit that disables PCI register writes.
Signed-off-by Stephen Hemminger <[email protected]>Signed-off-by David S. Miller <[email protected]>
Hmm, this is kind of interesting.
Perhaps we clear the WoL setting somewhere we shouldn't, but the "disable PCI register writes" bit prevents this from actually happening?
Rafael
Quoted 18 lines · On Mon, 18 Jan 2010 22:34:55 +0100 "Rafael J. Wysocki" <[email protected]> wrote:
Quoted 8 lines · Tino Keitel
Hi,
reverting this commit fixes WoL for me:
commit
166a0fd4c788ec7f10ca8194ec6d526afa12db75Author: stephen hemminger <[email protected]> Date: Mon Dec 14 08:50:12 2009 +0000Quoted 8 lines
sky2: leave PCI config space writeable
Since power management is done by PCI subsystem as well as driver, don't toggle the bit that disables PCI register writes.
Signed-off-by Stephen Hemminger <[email protected]>Signed-off-by David S. Miller <[email protected]>
Quoted 6 lines
Hmm, this is kind of interesting.
Perhaps we clear the WoL setting somewhere we shouldn't, but the "disable PCI register writes" bit prevents this from actually happening?
Rafael
How about getting a register dump of pci config space with good/bad version?
Quoted 2 lines · Stephen Hemminger
On Mon, 18 Jan 2010 22:34:55 +0100
"Rafael J. Wysocki" <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 4 lines
Perhaps we clear the WoL setting somewhere we shouldn't, but the "disable PCI register writes" bit prevents this from actually happening?
Rafael
How about getting a register dump of pci config space with good/bad version?
At which point should the dump be placed? Somewhere as near to the actual suspend as possible?
Regards,
Tino
Is this better worse or the same? It make sure that some registers are set on resume that might matter.
drivers/net/sky2.c +13 -6
@@ -3002,11 +3002,17 @@ static void sky2_reset(struct sky2_hw *hu32 hwe_mask =Y2_HWE_ALL_MASK ;/* disable ASF */ -if (hw->chip_id ==CHIP_ID_YUKON_EX ) { +if (hw->chip_id ==CHIP_ID_YUKON_EX || + hw->chip_id ==CHIP_ID_YUKON_SUPR ) { +/* stop the watchdog */ +sky2_write32 (hw,CPU_WDOG ,0 ); + status =sky2_read16 (hw,HCU_CCSR ); status &= ~(HCU_CCSR_AHB_RST |HCU_CCSR_CPU_RST_MODE |HCU_CCSR_UC_STATE_MSK ); + status &= ~HCU_CCSR_CPU_CLK_DIVIDE_MSK ;sky2_write16 (hw,HCU_CCSR , status); +sky2_write32 (hw,CPU_WDOG ,0 ); }else sky2_write8 (hw,B28_Y2_ASF_STAT_CMD ,Y2_ASF_RESET );sky2_write16 (hw,B0_CTST ,Y2_ASF_DISABLE );
@@ -4807,11 +4813,12 @@ static int sky2_resume(struct pci_dev *ppci_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 || - hw->chip_id ==CHIP_ID_YUKON_FE_P ) -sky2_pci_write32 (hw,PCI_DEV_REG3 ,0 ); +/* Enable all clocks and check for bad PCI access */ + err =pci_write_config_dword (pdev,PCI_DEV_REG3 ,0 ); +if (err) + goto out; + +sky2_write8 (hw,B0_CTST ,CS_RST_CLR );sky2_reset (hw);sky2_write32 (hw,B0_IMSK ,Y2_IS_BASE );
Quoted 2 lines · Stephen Hemminger
Is this better worse or the same? It make sure that some registers are set on resume that might matter.
I see not difference.
Regards,
Tino
Quoted 5 lines · Tino Keitel
Quoted 2 lines · Stephen Hemminger
Is this better worse or the same? It make sure that some registers are set on resume that might matter.
I see not difference.
Hmm. Perhaps it's better to revert commit 166a0fd4c788ec7f10ca8194ec6d526afa12db75 for now, so that 2.6.33 works on the Tino's machine, and figure out why it broke the WoL before (eventually) re-applying it?
Rafael
Quoted 5 lines · On Sun, 24 Jan 2010 21:35:29 +0100 "Rafael J. Wysocki" <[email protected]> wrote:
Quoted 4 lines · Tino Keitel
Quoted 2 lines · Stephen Hemminger
Is this better worse or the same? It make sure that some registers are set on resume that might matter.
Quoted 7 lines
Quoted 1 line
I see not difference.
Hmm. Perhaps it's better to revert commit
166a0fd4c788ec7f10ca8194ec6d526afa12db75for now, so that 2.6.33 works on the Tino's machine, and figure out why it broke the WoL before (eventually) re-applying it?Rafael
Agreed
PATCH 1/1 sky2: revert config space change
In reply to Rafael J. WysockiObviously, this register had some other impact that is causing the regression. Either it is masking some other access or needs to be reset in some path.
Either, way it is best to just revert the change for 2.6.33
This reverts commit 166a0fd4c788ec7f10ca8194ec6d526afa12db75.
--- drivers/net/sky2.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
drivers/net/sky2.c +13 -0
@@ -644,6 +644,7 @@ static void sky2_phy_power_up(struct sky2_hw *hw, unsigned port) {u32 reg1; +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_ON ); reg1 =sky2_pci_read32 (hw,PCI_DEV_REG1 ); reg1 &= ~phy_power[port];
@@ -651,6 +652,7 @@ static void sky2_phy_power_up(struct sky2_hw *hw, unsigned port) reg1 |= coma_mode[port];sky2_pci_write32 (hw,PCI_DEV_REG1 , reg1); +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF );sky2_pci_read32 (hw,PCI_DEV_REG1 );if (hw->chip_id ==CHIP_ID_YUKON_FE )
@@ -707,9 +709,11 @@ static void sky2_phy_power_down(struct sky2_hw *hw, unsigned port)gm_phy_write (hw, port,PHY_MARV_CTRL ,PHY_CT_PDOWN ); } +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_ON ); reg1 =sky2_pci_read32 (hw,PCI_DEV_REG1 ); reg1 |= phy_power[port];/* set PHY to PowerDown/COMA Mode */ sky2_pci_write32 (hw,PCI_DEV_REG1 , reg1); +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF ); }/* Force a renegotiation */
@@ -2149,7 +2153,9 @@ static void sky2_qlink_intr(struct sky2_hw *hw)/* reset PHY Link Detect */ phy =sky2_pci_read16 (hw,PSM_CONFIG_REG4 ); +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_ON );sky2_pci_write16 (hw,PSM_CONFIG_REG4 , phy |1 ); +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF );sky2_link_up (sky2); }
@@ -2640,6 +2646,7 @@ static void sky2_hw_intr(struct sky2_hw *hw)if (status & (Y2_IS_MST_ERR |Y2_IS_IRQ_STAT )) {u16 pci_err; +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_ON ); pci_err =sky2_pci_read16 (hw,PCI_STATUS );if (net_ratelimit ()) dev_err(&pdev->dev, "PCI hardware error (0x%x)\n",
@@ -2647,12 +2654,14 @@ static void sky2_hw_intr(struct sky2_hw *hw)sky2_pci_write16 (hw,PCI_STATUS , pci_err |PCI_STATUS_ERROR_BITS ); +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF ); }if (status &Y2_IS_PCI_EXP ) {/* PCI-Express uncorrectable Error occurred */ u32 err; +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_ON ); err =sky2_read32 (hw,Y2_CFG_AER +PCI_ERR_UNCOR_STATUS );sky2_write32 (hw,Y2_CFG_AER +PCI_ERR_UNCOR_STATUS ,0xfffffffful );
@@ -2660,6 +2669,7 @@ static void sky2_hw_intr(struct sky2_hw *hw)dev_err (&pdev->dev,"PCI Express error (0x%x)\n" , err);sky2_read32 (hw,Y2_CFG_AER +PCI_ERR_UNCOR_STATUS ); +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF ); }if (status &Y2_HWE_L1_MASK )
@@ -3038,6 +3048,7 @@ static void sky2_reset(struct sky2_hw *hw) }sky2_power_on (hw); +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF );for (i =0 ; i < hw->ports; i++) {sky2_write8 (hw,SK_REG (i,GMAC_LINK_CTRL ),GMLC_RST_SET );
@@ -3074,6 +3085,7 @@ static void sky2_reset(struct sky2_hw *hw) reg <<=PSM_CONFIG_REG4_TIMER_PHY_LINK_DETECT_BASE ;/* reset PHY Link Detect */ +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_ON );sky2_pci_write16 (hw,PSM_CONFIG_REG4 , reg |PSM_CONFIG_REG4_RST_PHY_LINK_DETECT );sky2_pci_write16 (hw,PSM_CONFIG_REG4 , reg);
@@ -3091,6 +3103,7 @@ static void sky2_reset(struct sky2_hw *hw)/* restore the PCIe Link Control register */ sky2_pci_write16 (hw, cap +PCI_EXP_LNKCTL , reg); } +sky2_write8 (hw,B2_TST_CTRL1 ,TST_CFG_WRITE_OFF );/* re-enable PEX PM in PEX PHY debug reg. 8 (clear bit 12) */ sky2_write32 (hw,Y2_PEX_PHY_DATA ,PEX_DB_ACCESS | (0x08UL <<16 ));
Signature · 1 line
1.6.3.3
David Miller
maintainerRe: [PATCH] sky2: revert config space change
From: Stephen Hemminger <[email protected]>
Date: Sun, 24 Jan 2010 20:46:06 -0800
Quoted 8 lines
Obviously, this register had some other impact that is causing the regression. Either it is masking some other access or needs to be reset in some path.
Either, way it is best to just revert the change for 2.6.33
This reverts commit
166a0fd4c788ec7f10ca8194ec6d526afa12db75.
I'll apply this, thanks Stephen.