Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Tino Keitel]

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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Stephen Hemminger]

On Fri, 15 Jan 2010 10:10:53 +0100
Tino Keitel <tino.keitel@tikei.de> 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
> 
> 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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Rafael J. Wysocki]

On Friday 15 January 2010, Stephen Hemminger wrote:
> On Fri, 15 Jan 2010 10:10:53 +0100
> Tino Keitel <tino.keitel@tikei.de> 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
> > 
> > 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

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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Tino Keitel]

On Fri, Jan 15, 2010 at 23:23:37 +0100, Rafael J. Wysocki wrote:
> On Friday 15 January 2010, Stephen Hemminger wrote:

[...]

> > 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
> 
> This 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.

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

No luck with the commit reverted, WoL still doesn't work.

Regards,
Tino

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Rafael J. Wysocki]

On Monday 18 January 2010, Tino Keitel wrote:
> On Fri, Jan 15, 2010 at 23:23:37 +0100, Rafael J. Wysocki wrote:
> > On Friday 15 January 2010, Stephen Hemminger wrote:
> 
> [...]
> 
> > > 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
> > 
> > This 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.
> 
> > 
> > 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.
> 
> 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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Tino Keitel]

Hi,

reverting this commit fixes WoL for me:

commit 166a0fd4c788ec7f10ca8194ec6d526afa12db75
Author: stephen hemminger <shemminger@vyatta.com>
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.
    
    Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
    Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
    Signed-off-by: David S. Miller <davem@davemloft.net>


Regards,
Tino

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Mike McCormack]

Tino Keitel wrote:
> 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.

Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
 drivers/net/sky2.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 4c06020..b54edec 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -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 

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Tino Keitel]

On Mon, Jan 18, 2010 at 20:59:40 +0900, Mike McCormack wrote:
> Tino Keitel wrote:
> > Hi,
> > 
> > reverting this commit fixes WoL for me:
> > 
> > commit 166a0fd4c788ec7f10ca8194ec6d526afa12db75
> 
> Hi Tino,
> 
> Does this fix WoL also?  Compile tested only...

No, still no wakeup on WoL packets.

Regards,
Tino

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Stephen Hemminger]

Does it work better if config bit is set here?

---
 drivers/net/sky2.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

--- a/drivers/net/sky2.c	2010-01-18 11:45:54.444753324 -0800
+++ b/drivers/net/sky2.c	2010-01-18 11:49:18.475018945 -0800
@@ -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)

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Tino Keitel]

On Mon, Jan 18, 2010 at 11:52:24 -0800, Stephen Hemminger wrote:
> Does it work better if config bit is set here?

No, still the same problem.

Regards,
Tino

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Rafael J. Wysocki]

On Monday 18 January 2010, Tino Keitel wrote:
> Hi,
> 
> reverting this commit fixes WoL for me:
> 
> commit 166a0fd4c788ec7f10ca8194ec6d526afa12db75
> Author: stephen hemminger <shemminger@vyatta.com>
> 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.
>     
>     Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>     Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
>     Signed-off-by: David S. Miller <davem@davemloft.net>

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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Stephen Hemminger]

On Mon, 18 Jan 2010 22:34:55 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Monday 18 January 2010, Tino Keitel wrote:
> > Hi,
> > 
> > reverting this commit fixes WoL for me:
> > 
> > commit 166a0fd4c788ec7f10ca8194ec6d526afa12db75
> > Author: stephen hemminger <shemminger@vyatta.com>
> > 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.
> >     
> >     Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >     Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
> >     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> 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?

-- 

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Tino Keitel]

On Mon, Jan 18, 2010 at 14:43:40 -0800, Stephen Hemminger wrote:
> On Mon, 18 Jan 2010 22:34:55 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:

[...]

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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Stephen Hemminger]

Is this better worse or the same? It make sure that some
registers are set on resume that might matter.


--- a/drivers/net/sky2.c	2010-01-19 10:32:47.549541152 -0800
+++ b/drivers/net/sky2.c	2010-01-19 10:49:06.726853816 -0800
@@ -3002,11 +3002,17 @@ static void sky2_reset(struct sky2_hw *h
 	u32 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 *p
 
 	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 ||
-	    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);

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Tino Keitel]

On Tue, Jan 19, 2010 at 10:51:05 -0800, Stephen Hemminger wrote:
> 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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Rafael J. Wysocki]

On Tuesday 19 January 2010, Tino Keitel wrote:
> On Tue, Jan 19, 2010 at 10:51:05 -0800, Stephen Hemminger wrote:
> > 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

Re: Regression: Wake on LAN doesn't work in sky2 with 2.6.33-rc4-git2

[Stephen Hemminger]

On Sun, 24 Jan 2010 21:35:29 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Tuesday 19 January 2010, Tino Keitel wrote:
> > On Tue, Jan 19, 2010 at 10:51:05 -0800, Stephen Hemminger wrote:
> > > 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

Agreed

-- 

[PATCH] sky2: revert config space change

[Stephen Hemminger]

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.
---
 drivers/net/sky2.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index f8f50f7..64b441c 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -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));
-- 
1.6.3.3

Re: [PATCH] sky2: revert config space change

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Sun, 24 Jan 2010 20:46:06 -0800

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