[patch 0/6] sky2 driver update (v1.11)

[patch 0/6] sky2 driver update (v1.11)

[Stephen Hemminger]

Patches are in order of severity. 1-3 are bug fixes, 4 is a cleanup
of the power state code, and 5 adds wake on lan support.

IMHO, it is bad security policy to allow wake on lan to enabled by default.
The sky2 driver doesn't do WOL until enabled with ethtool.

--
Stephen Hemminger <shemminger@osdl.org>

[patch 1/6] sky2: dual port NAPI problem

[Stephen Hemminger]

[-- Attachment #1: sky2-dual-port-napi.patch --]
[-- Type: text/plain, Size: 754 bytes --]

Shutting down port 0 disables the NAPI poll used by both ports.
The long term fix will be to separate NAPI object from net device
until then just reenable if needed.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>


--- sky2-2.6.orig/drivers/net/sky2.c	2006-12-18 12:46:59.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.c	2006-12-18 12:47:30.000000000 -0800
@@ -1511,6 +1511,13 @@
 	imask &= ~portirq_msk[port];
 	sky2_write32(hw, B0_IMSK, imask);
 
+	/*
+	 * Both ports share the NAPI poll on port 0, so if necessary undo the
+	 * the disable that is done in dev_close.
+	 */
+	if (sky2->port == 0 && hw->ports > 1)
+		netif_poll_enable(dev);
+
 	sky2_gmac_reset(hw, port);
 
 	/* Stop transmitter */

--
Stephen Hemminger <shemminger@osdl.org>

Re: [patch 1/6] sky2: dual port NAPI problem

[Jeff Garzik]

applied 1-3 to upstream-fixes

[patch 2/6] sky2: power management/MSI workaround

[Stephen Hemminger]

[-- Attachment #1: sky2-msi-resume.patch --]
[-- Type: text/plain, Size: 1504 bytes --]

MSI doesn't work properly on resume on many platforms because the
BIOS goes and changes it back to INTx mode after the sky2 driver has
restored in resume. 

It is really a bug in the base power management resume code, and 
this workaround is temporary until the change to PM code works it's way
through the release process.  The PM fix is non-trivial since it needs
to change when non-boot CPU's are enabled.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

--- sky2-2.6.orig/drivers/net/sky2.c	2006-12-18 14:22:56.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.c	2006-12-18 14:31:43.000000000 -0800
@@ -3638,6 +3638,29 @@
 out:
 	return err;
 }
+
+/* BIOS resume runs after device (it's a bug in PM)
+ * as a temporary workaround on suspend/resume leave MSI disabled
+ */
+static int sky2_suspend_late(struct pci_dev *pdev, pm_message_t state)
+{
+	struct sky2_hw *hw = pci_get_drvdata(pdev);
+
+	free_irq(pdev->irq, hw);
+	if (hw->msi) {
+		pci_disable_msi(pdev);
+		hw->msi = 0;
+	}
+	return 0;
+}
+
+static int sky2_resume_early(struct pci_dev *pdev)
+{
+	struct sky2_hw *hw = pci_get_drvdata(pdev);
+	struct net_device *dev = hw->dev[0];
+
+	return request_irq(pdev->irq, sky2_intr, IRQF_SHARED, dev->name, hw);
+}
 #endif
 
 static struct pci_driver sky2_driver = {
@@ -3648,6 +3671,8 @@
 #ifdef CONFIG_PM
 	.suspend = sky2_suspend,
 	.resume = sky2_resume,
+	.suspend_late = sky2_suspend_late,
+	.resume_early = sky2_resume_early,
 #endif
 };
 

--
Stephen Hemminger <shemminger@osdl.org>

[patch 3/6] sky2: phy power down needs PCI config write enabled

[Stephen Hemminger]

[-- Attachment #1: sky2-phy-power-write.patch --]
[-- Type: text/plain, Size: 948 bytes --]

In order to change PCI registers (via the iomap'd window),
it needs to be enabled; this wasn't being done in sky2_phy_power
the function that turns on/off power to the PHY.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 drivers/net/sky2.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- sky2-2.6.orig/drivers/net/sky2.c	2006-12-19 15:33:38.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.c	2006-12-19 15:34:31.000000000 -0800
@@ -569,8 +569,8 @@
 	if (hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev > 1)
 		onoff = !onoff;
 
+	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
 	reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
-
 	if (onoff)
 		/* Turn off phy power saving */
 		reg1 &= ~phy_power[port];
@@ -579,6 +579,7 @@
 
 	sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
 	sky2_pci_read32(hw, PCI_DEV_REG1);
+	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
 	udelay(100);
 }
 

--
Stephen Hemminger <shemminger@osdl.org>

[patch 4/6] sky2: better power state management

[Stephen Hemminger]

[-- Attachment #1: sky2-power-state.patch --]
[-- Type: text/plain, Size: 6938 bytes --]

Improve power management and error handling by using pci_set_power_state(),
instead of driver doing PCI PM register changes in the driver.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/net/sky2.c |  140 +++++++++++++++++++++--------------------------------
 drivers/net/sky2.h |    1 
 2 files changed, 57 insertions(+), 84 deletions(-)

--- sky2-2.6.orig/drivers/net/sky2.c	2006-12-20 12:45:37.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.c	2006-12-20 12:45:40.000000000 -0800
@@ -192,76 +192,52 @@
 	return v;
 }
 
-static void sky2_set_power_state(struct sky2_hw *hw, pci_power_t state)
-{
-	u16 power_control;
-	int vaux;
-
-	pr_debug("sky2_set_power_state %d\n", state);
-	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
-
-	power_control = sky2_pci_read16(hw, hw->pm_cap + PCI_PM_PMC);
-	vaux = (sky2_read16(hw, B0_CTST) & Y2_VAUX_AVAIL) &&
-		(power_control & PCI_PM_CAP_PME_D3cold);
-
-	power_control = sky2_pci_read16(hw, hw->pm_cap + PCI_PM_CTRL);
-
-	power_control |= PCI_PM_CTRL_PME_STATUS;
-	power_control &= ~(PCI_PM_CTRL_STATE_MASK);
-
-	switch (state) {
-	case PCI_D0:
-		/* switch power to VCC (WA for VAUX problem) */
-		sky2_write8(hw, B0_POWER_CTRL,
-			    PC_VAUX_ENA | PC_VCC_ENA | PC_VAUX_OFF | PC_VCC_ON);
 
-		/* disable Core Clock Division, */
-		sky2_write32(hw, B2_Y2_CLK_CTRL, Y2_CLK_DIV_DIS);
-
-		if (hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev > 1)
-			/* enable bits are inverted */
-			sky2_write8(hw, B2_Y2_CLK_GATE,
-				    Y2_PCI_CLK_LNK1_DIS | Y2_COR_CLK_LNK1_DIS |
-				    Y2_CLK_GAT_LNK1_DIS | Y2_PCI_CLK_LNK2_DIS |
-				    Y2_COR_CLK_LNK2_DIS | Y2_CLK_GAT_LNK2_DIS);
-		else
-			sky2_write8(hw, B2_Y2_CLK_GATE, 0);
+static void sky2_power_on(struct sky2_hw *hw)
+{
+	/* switch power to VCC (WA for VAUX problem) */
+	sky2_write8(hw, B0_POWER_CTRL,
+		    PC_VAUX_ENA | PC_VCC_ENA | PC_VAUX_OFF | PC_VCC_ON);
 
-		if (hw->chip_id == CHIP_ID_YUKON_EC_U) {
-			u32 reg1;
+	/* disable Core Clock Division, */
+	sky2_write32(hw, B2_Y2_CLK_CTRL, Y2_CLK_DIV_DIS);
 
-			sky2_pci_write32(hw, PCI_DEV_REG3, 0);
-			reg1 = sky2_pci_read32(hw, PCI_DEV_REG4);
-			reg1 &= P_ASPM_CONTROL_MSK;
-			sky2_pci_write32(hw, PCI_DEV_REG4, reg1);
-			sky2_pci_write32(hw, PCI_DEV_REG5, 0);
-		}
+	if (hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev > 1)
+		/* enable bits are inverted */
+		sky2_write8(hw, B2_Y2_CLK_GATE,
+			    Y2_PCI_CLK_LNK1_DIS | Y2_COR_CLK_LNK1_DIS |
+			    Y2_CLK_GAT_LNK1_DIS | Y2_PCI_CLK_LNK2_DIS |
+			    Y2_COR_CLK_LNK2_DIS | Y2_CLK_GAT_LNK2_DIS);
+	else
+		sky2_write8(hw, B2_Y2_CLK_GATE, 0);
 
-		break;
+	if (hw->chip_id == CHIP_ID_YUKON_EC_U) {
+		u32 reg1;
 
-	case PCI_D3hot:
-	case PCI_D3cold:
-		if (hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev > 1)
-			sky2_write8(hw, B2_Y2_CLK_GATE, 0);
-		else
-			/* enable bits are inverted */
-			sky2_write8(hw, B2_Y2_CLK_GATE,
-				    Y2_PCI_CLK_LNK1_DIS | Y2_COR_CLK_LNK1_DIS |
-				    Y2_CLK_GAT_LNK1_DIS | Y2_PCI_CLK_LNK2_DIS |
-				    Y2_COR_CLK_LNK2_DIS | Y2_CLK_GAT_LNK2_DIS);
-
-		/* switch power to VAUX */
-		if (vaux && state != PCI_D3cold)
-			sky2_write8(hw, B0_POWER_CTRL,
-				    (PC_VAUX_ENA | PC_VCC_ENA |
-				     PC_VAUX_ON | PC_VCC_OFF));
-		break;
-	default:
-		printk(KERN_ERR PFX "Unknown power state %d\n", state);
+		sky2_pci_write32(hw, PCI_DEV_REG3, 0);
+		reg1 = sky2_pci_read32(hw, PCI_DEV_REG4);
+		reg1 &= P_ASPM_CONTROL_MSK;
+		sky2_pci_write32(hw, PCI_DEV_REG4, reg1);
+		sky2_pci_write32(hw, PCI_DEV_REG5, 0);
 	}
+}
 
-	sky2_pci_write16(hw, hw->pm_cap + PCI_PM_CTRL, power_control);
-	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
+static void sky2_power_aux(struct sky2_hw *hw)
+{
+	if (hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev > 1)
+		sky2_write8(hw, B2_Y2_CLK_GATE, 0);
+	else
+		/* enable bits are inverted */
+		sky2_write8(hw, B2_Y2_CLK_GATE,
+			    Y2_PCI_CLK_LNK1_DIS | Y2_COR_CLK_LNK1_DIS |
+			    Y2_CLK_GAT_LNK1_DIS | Y2_PCI_CLK_LNK2_DIS |
+			    Y2_COR_CLK_LNK2_DIS | Y2_CLK_GAT_LNK2_DIS);
+
+	/* switch power to VAUX */
+	if (sky2_read16(hw, B0_CTST) & Y2_VAUX_AVAIL)
+		sky2_write8(hw, B0_POWER_CTRL,
+			    (PC_VAUX_ENA | PC_VCC_ENA |
+			     PC_VAUX_ON | PC_VCC_OFF));
 }
 
 static void sky2_gmac_reset(struct sky2_hw *hw, unsigned port)
@@ -2480,7 +2456,7 @@
 			++hw->ports;
 	}
 
-	sky2_set_power_state(hw, PCI_D0);
+	sky2_power_on(hw);
 
 	for (i = 0; i < hw->ports; i++) {
 		sky2_write8(hw, SK_REG(i, GMAC_LINK_CTRL), GMLC_RST_SET);
@@ -3376,7 +3352,7 @@
 {
 	struct net_device *dev, *dev1 = NULL;
 	struct sky2_hw *hw;
-	int err, pm_cap, using_dac = 0;
+	int err, using_dac = 0;
 
 	err = pci_enable_device(pdev);
 	if (err) {
@@ -3394,15 +3370,6 @@
 
 	pci_set_master(pdev);
 
-	/* Find power-management capability. */
-	pm_cap = pci_find_capability(pdev, PCI_CAP_ID_PM);
-	if (pm_cap == 0) {
-		printk(KERN_ERR PFX "Cannot find PowerManagement capability, "
-		       "aborting.\n");
-		err = -EIO;
-		goto err_out_free_regions;
-	}
-
 	if (sizeof(dma_addr_t) > sizeof(u32) &&
 	    !(err = pci_set_dma_mask(pdev, DMA_64BIT_MASK))) {
 		using_dac = 1;
@@ -3438,7 +3405,6 @@
 		       pci_name(pdev));
 		goto err_out_free_hw;
 	}
-	hw->pm_cap = pm_cap;
 
 #ifdef __BIG_ENDIAN
 	/* The sk98lin vendor driver uses hardware byte swapping but
@@ -3555,7 +3521,8 @@
 		unregister_netdev(dev1);
 	unregister_netdev(dev0);
 
-	sky2_set_power_state(hw, PCI_D3hot);
+	sky2_power_aux(hw);
+
 	sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);
 	sky2_write8(hw, B0_CTST, CS_RST_SET);
 	sky2_read8(hw, B0_CTST);
@@ -3581,10 +3548,6 @@
 {
 	struct sky2_hw *hw = pci_get_drvdata(pdev);
 	int i;
-	pci_power_t pstate = pci_choose_state(pdev, state);
-
-	if (!(pstate == PCI_D3hot || pstate == PCI_D3cold))
-		return -EINVAL;
 
 	del_timer_sync(&hw->idle_timer);
 	netif_poll_disable(hw->dev[0]);
@@ -3599,8 +3562,10 @@
 	}
 
 	sky2_write32(hw, B0_IMSK, 0);
+	sky2_power_aux(hw);
 	pci_save_state(pdev);
-	sky2_set_power_state(hw, pstate);
+	pci_set_power_state(pdev, pci_choose_state(pdev, state));
+
 	return 0;
 }
 
@@ -3609,9 +3574,15 @@
 	struct sky2_hw *hw = pci_get_drvdata(pdev);
 	int i, err;
 
-	pci_restore_state(pdev);
+	err = pci_set_power_state(pdev, PCI_D0);
+	if (err)
+		goto out;
+
+	err = pci_restore_state(pdev);
+	if (err)
+		goto out;
+
 	pci_enable_wake(pdev, PCI_D0, 0);
-	sky2_set_power_state(hw, PCI_D0);
 
 	err = sky2_reset(hw);
 	if (err)
@@ -3636,7 +3607,10 @@
 
 	netif_poll_enable(hw->dev[0]);
 	sky2_idle_start(hw);
+	return 0;
 out:
+	printk(KERN_ERR PFX "%s: resume failed (%d)\n", pci_name(pdev), err);
+	pci_disable_device(pdev);
 	return err;
 }
 
--- sky2-2.6.orig/drivers/net/sky2.h	2006-12-20 12:45:22.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.h	2006-12-20 12:45:40.000000000 -0800
@@ -1887,7 +1887,6 @@
 	struct pci_dev	     *pdev;
 	struct net_device    *dev[2];
 
-	int		     pm_cap;
 	u8	     	     chip_id;
 	u8		     chip_rev;
 	u8		     pmd_type;

--
Stephen Hemminger <shemminger@osdl.org>

Re: [patch 4/6] sky2: better power state management

[Jeff Garzik]

Stephen Hemminger wrote:
> Improve power management and error handling by using pci_set_power_state(),
> instead of driver doing PCI PM register changes in the driver.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

applied

[patch 5/6] sky2: add Wake On Lan support

[Stephen Hemminger]

[-- Attachment #1: sky2-wol2.patch --]
[-- Type: text/plain, Size: 9014 bytes --]

This adds basic magic packet wake on lan support to the sky2 driver.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 drivers/net/sky2.c |  149 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/net/sky2.h |   28 ++-------
 2 files changed, 150 insertions(+), 27 deletions(-)

--- sky2-2.6.orig/drivers/net/sky2.c	2006-12-20 12:45:40.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.c	2006-12-18 13:31:22.000000000 -0800
@@ -567,6 +567,73 @@
 	spin_unlock_bh(&sky2->phy_lock);
 }
 
+/* Put device in state to listen for Wake On Lan */
+static void sky2_wol_init(struct sky2_port *sky2)
+{
+	struct sky2_hw *hw = sky2->hw;
+	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);
+	sky2_write16(hw, SK_REG(port, GMAC_LINK_CTRL), GMLC_RST_CLR);
+
+	sky2_write8(hw, SK_REG(port, GPHY_CTRL), GPC_RST_CLR);
+	sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_CLR);
+
+	/* Force to 10/100
+	 * sky2_reset will re-enable on resume
+	 */
+	save_mode = sky2->flow_mode;
+	ctrl = sky2->advertising;
+
+	sky2->advertising &= ~(ADVERTISED_1000baseT_Half|ADVERTISED_1000baseT_Full);
+	sky2->flow_mode = FC_NONE;
+	sky2_phy_power(hw, port, 1);
+	sky2_phy_reinit(sky2);
+
+	sky2->flow_mode = save_mode;
+	sky2->advertising = ctrl;
+
+	/* Set GMAC to no flow control and auto update for speed/duplex */
+	gma_write16(hw, port, GM_GP_CTRL,
+		    GM_GPCR_FC_TX_DIS|GM_GPCR_TX_ENA|GM_GPCR_RX_ENA|
+		    GM_GPCR_DUP_FULL|GM_GPCR_FC_RX_DIS|GM_GPCR_AU_FCT_DIS);
+
+	/* Set WOL address */
+	memcpy_toio(hw->regs + WOL_REGS(port, WOL_MAC_ADDR),
+		    sky2->netdev->dev_addr, ETH_ALEN);
+
+	/* Turn on appropriate WOL control bits */
+	sky2_write16(hw, WOL_REGS(port, WOL_CTRL_STAT), WOL_CTL_CLEAR_RESULT);
+	ctrl = 0;
+	if (sky2->wol & WAKE_PHY)
+		ctrl |= WOL_CTL_ENA_PME_ON_LINK_CHG|WOL_CTL_ENA_LINK_CHG_UNIT;
+	else
+		ctrl |= WOL_CTL_DIS_PME_ON_LINK_CHG|WOL_CTL_DIS_LINK_CHG_UNIT;
+
+	if (sky2->wol & WAKE_MAGIC)
+		ctrl |= WOL_CTL_ENA_PME_ON_MAGIC_PKT|WOL_CTL_ENA_MAGIC_PKT_UNIT;
+	else
+		ctrl |= WOL_CTL_DIS_PME_ON_MAGIC_PKT|WOL_CTL_DIS_MAGIC_PKT_UNIT;;
+
+	ctrl |= WOL_CTL_DIS_PME_ON_PATTERN|WOL_CTL_DIS_PATTERN_UNIT;
+	sky2_write16(hw, WOL_REGS(port, WOL_CTRL_STAT), ctrl);
+
+	/* Turn on legacy PCI-Express PME mode */
+	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
+	reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
+	reg1 |= PCI_Y2_PME_LEGACY;
+	sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
+	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
+
+	/* block receiver */
+	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
+
+}
+
 static void sky2_mac_init(struct sky2_hw *hw, unsigned port)
 {
 	struct sky2_port *sky2 = netdev_priv(hw->dev[port]);
@@ -2543,6 +2610,38 @@
 	return 0;
 }
 
+static inline u8 sky2_wol_supported(const struct sky2_hw *hw)
+{
+	return sky2_is_copper(hw) ? (WAKE_PHY | WAKE_MAGIC) : 0;
+}
+
+static void sky2_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	const struct sky2_port *sky2 = netdev_priv(dev);
+
+	wol->supported = sky2_wol_supported(sky2->hw);
+	wol->wolopts = sky2->wol;
+}
+
+static int sky2_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct sky2_port *sky2 = netdev_priv(dev);
+	struct sky2_hw *hw = sky2->hw;
+
+	if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
+		return -EOPNOTSUPP;
+
+	sky2->wol = wol->wolopts;
+
+	if (hw->chip_id == CHIP_ID_YUKON_EC_U)
+		sky2_write32(hw, B0_CTST, sky2->wol
+			     ? Y2_HW_WOL_ON : Y2_HW_WOL_OFF);
+
+	if (!netif_running(dev))
+		sky2_wol_init(sky2);
+	return 0;
+}
+
 static u32 sky2_supported_modes(const struct sky2_hw *hw)
 {
 	if (sky2_is_copper(hw)) {
@@ -3167,7 +3266,9 @@
 static const struct ethtool_ops sky2_ethtool_ops = {
 	.get_settings = sky2_get_settings,
 	.set_settings = sky2_set_settings,
-	.get_drvinfo = sky2_get_drvinfo,
+	.get_drvinfo  = sky2_get_drvinfo,
+	.get_wol      = sky2_get_wol,
+	.set_wol      = sky2_set_wol,
 	.get_msglevel = sky2_get_msglevel,
 	.set_msglevel = sky2_set_msglevel,
 	.nway_reset   = sky2_nway_reset,
@@ -3547,23 +3648,29 @@
 static int sky2_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct sky2_hw *hw = pci_get_drvdata(pdev);
-	int i;
+	int i, wol = 0;
 
 	del_timer_sync(&hw->idle_timer);
 	netif_poll_disable(hw->dev[0]);
 
 	for (i = 0; i < hw->ports; i++) {
 		struct net_device *dev = hw->dev[i];
+		struct sky2_port *sky2 = netdev_priv(dev);
 
-		if (netif_running(dev)) {
+		if (netif_running(dev))
 			sky2_down(dev);
-			netif_device_detach(dev);
-		}
+
+		if (sky2->wol)
+			sky2_wol_init(sky2);
+
+		wol |= sky2->wol;
 	}
 
 	sky2_write32(hw, B0_IMSK, 0);
 	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));
 
 	return 0;
@@ -3593,8 +3700,6 @@
 	for (i = 0; i < hw->ports; i++) {
 		struct net_device *dev = hw->dev[i];
 		if (netif_running(dev)) {
-			netif_device_attach(dev);
-
 			err = sky2_up(dev);
 			if (err) {
 				printk(KERN_ERR PFX "%s: could not up: %d\n",
@@ -3638,6 +3743,35 @@
 }
 #endif
 
+static void sky2_shutdown(struct pci_dev *pdev)
+{
+	struct sky2_hw *hw = pci_get_drvdata(pdev);
+	int i, wol = 0;
+
+	del_timer_sync(&hw->idle_timer);
+	netif_poll_disable(hw->dev[0]);
+
+	for (i = 0; i < hw->ports; i++) {
+		struct net_device *dev = hw->dev[i];
+		struct sky2_port *sky2 = netdev_priv(dev);
+
+		if (sky2->wol) {
+			wol = 1;
+			sky2_wol_init(sky2);
+		}
+	}
+
+	if (wol)
+		sky2_power_aux(hw);
+
+	pci_enable_wake(pdev, PCI_D3hot, wol);
+	pci_enable_wake(pdev, PCI_D3cold, wol);
+
+	pci_disable_device(pdev);
+	pci_set_power_state(pdev, PCI_D3hot);
+
+}
+
 static struct pci_driver sky2_driver = {
 	.name = DRV_NAME,
 	.id_table = sky2_id_table,
@@ -3649,6 +3783,7 @@
 	.suspend_late = sky2_suspend_late,
 	.resume_early = sky2_resume_early,
 #endif
+	.shutdown = sky2_shutdown,
 };
 
 static int __init sky2_init_module(void)
--- sky2-2.6.orig/drivers/net/sky2.h	2006-12-20 12:45:40.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.h	2006-12-19 13:57:45.000000000 -0800
@@ -32,6 +32,7 @@
 	PCI_Y2_PHY1_COMA = 1<<28, /* Set PHY 1 to Coma Mode (YUKON-2) */
 	PCI_Y2_PHY2_POWD = 1<<27, /* Set PHY 2 to Power Down (YUKON-2) */
 	PCI_Y2_PHY1_POWD = 1<<26, /* Set PHY 1 to Power Down (YUKON-2) */
+	PCI_Y2_PME_LEGACY= 1<<15, /* PCI Express legacy power management mode */
 };
 
 enum pci_dev_reg_2 {
@@ -837,33 +838,27 @@
 	GMAC_LINK_CTRL	= 0x0f10,/* 16 bit	Link Control Reg */
 
 /* Wake-up Frame Pattern Match Control Registers (YUKON only) */
-
-	WOL_REG_OFFS	= 0x20,/* HW-Bug: Address is + 0x20 against spec. */
-
 	WOL_CTRL_STAT	= 0x0f20,/* 16 bit	WOL Control/Status Reg */
 	WOL_MATCH_CTL	= 0x0f22,/*  8 bit	WOL Match Control Reg */
 	WOL_MATCH_RES	= 0x0f23,/*  8 bit	WOL Match Result Reg */
 	WOL_MAC_ADDR	= 0x0f24,/* 32 bit	WOL MAC Address */
-	WOL_PATT_PME	= 0x0f2a,/*  8 bit	WOL PME Match Enable (Yukon-2) */
-	WOL_PATT_ASFM	= 0x0f2b,/*  8 bit	WOL ASF Match Enable (Yukon-2) */
 	WOL_PATT_RPTR	= 0x0f2c,/*  8 bit	WOL Pattern Read Pointer */
 
 /* WOL Pattern Length Registers (YUKON only) */
-
 	WOL_PATT_LEN_LO	= 0x0f30,/* 32 bit	WOL Pattern Length 3..0 */
 	WOL_PATT_LEN_HI	= 0x0f34,/* 24 bit	WOL Pattern Length 6..4 */
 
 /* WOL Pattern Counter Registers (YUKON only) */
-
-
 	WOL_PATT_CNT_0	= 0x0f38,/* 32 bit	WOL Pattern Counter 3..0 */
 	WOL_PATT_CNT_4	= 0x0f3c,/* 24 bit	WOL Pattern Counter 6..4 */
 };
+#define WOL_REGS(port, x)	(x + (port)*0x80)
 
 enum {
 	WOL_PATT_RAM_1	= 0x1000,/*  WOL Pattern RAM Link 1 */
 	WOL_PATT_RAM_2	= 0x1400,/*  WOL Pattern RAM Link 2 */
 };
+#define WOL_PATT_RAM_BASE(port)	(WOL_PATT_RAM_1 + (port)*0x400)
 
 enum {
 	BASE_GMAC_1	= 0x2800,/* GMAC 1 registers */
@@ -1715,14 +1710,17 @@
 	GM_IS_RX_COMPL	= 1<<0,	/* Frame Reception Complete */
 
 #define GMAC_DEF_MSK     GM_IS_TX_FF_UR
+};
 
 /*	GMAC_LINK_CTRL	16 bit	GMAC Link Control Reg (YUKON only) */
-						/* Bits 15.. 2:	reserved */
+enum {						/* Bits 15.. 2:	reserved */
 	GMLC_RST_CLR	= 1<<1,	/* Clear GMAC Link Reset */
 	GMLC_RST_SET	= 1<<0,	/* Set   GMAC Link Reset */
+};
 
 
 /*	WOL_CTRL_STAT	16 bit	WOL Control/Status Reg */
+enum {
 	WOL_CTL_LINK_CHG_OCC		= 1<<15,
 	WOL_CTL_MAGIC_PKT_OCC		= 1<<14,
 	WOL_CTL_PATTERN_OCC		= 1<<13,
@@ -1741,17 +1739,6 @@
 	WOL_CTL_DIS_PATTERN_UNIT	= 1<<0,
 };
 
-#define WOL_CTL_DEFAULT				\
-	(WOL_CTL_DIS_PME_ON_LINK_CHG |	\
-	WOL_CTL_DIS_PME_ON_PATTERN |	\
-	WOL_CTL_DIS_PME_ON_MAGIC_PKT |	\
-	WOL_CTL_DIS_LINK_CHG_UNIT |		\
-	WOL_CTL_DIS_PATTERN_UNIT |		\
-	WOL_CTL_DIS_MAGIC_PKT_UNIT)
-
-/*	WOL_MATCH_CTL	 8 bit	WOL Match Control Reg */
-#define WOL_CTL_PATT_ENA(x)	(1 << (x))
-
 
 /* Control flags */
 enum {
@@ -1875,6 +1862,7 @@
 	u8		     autoneg;	/* AUTONEG_ENABLE, AUTONEG_DISABLE */
 	u8		     duplex;	/* DUPLEX_HALF, DUPLEX_FULL */
 	u8		     rx_csum;
+	u8		     wol;
  	enum flow_control    flow_mode;
  	enum flow_control    flow_status;
 

--
Stephen Hemminger <shemminger@osdl.org>

[patch 6/6] sky2: version 1.11

[Stephen Hemminger]

[-- Attachment #1: sky2-v1.11 --]
[-- Type: text/plain, Size: 497 bytes --]

Version update

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/net/sky2.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- sky2-2.6.orig/drivers/net/sky2.c	2006-12-19 14:20:46.000000000 -0800
+++ sky2-2.6/drivers/net/sky2.c	2006-12-19 14:20:51.000000000 -0800
@@ -49,7 +49,7 @@
 #include "sky2.h"
 
 #define DRV_NAME		"sky2"
-#define DRV_VERSION		"1.10"
+#define DRV_VERSION		"1.11"
 #define PFX			DRV_NAME " "
 
 /*

--
Stephen Hemminger <shemminger@osdl.org>

Re: [patch 0/6] sky2 driver update (v1.11)

[Jeff Garzik]

Stephen Hemminger wrote:
> Patches are in order of severity. 1-3 are bug fixes, 4 is a cleanup
> of the power state code, and 5 adds wake on lan support.
> 
> IMHO, it is bad security policy to allow wake on lan to enabled by default.
> The sky2 driver doesn't do WOL until enabled with ethtool.

While in general I agree with you on the security principle, this seems 
like it might break working setups.

WOL is a partnership between the motherboard and NIC.  The motherboard 
must support WOL, or its useless.  And since the motherboard must 
support WOL, it normally has an on/off switch in BIOS.

As such, you're overriding the admin's chosen BIOS setting here.

	Jeff

Re: [patch 0/6] sky2 driver update (v1.11)

[Stephen Hemminger]

On Tue, 26 Dec 2006 16:44:24 -0500
Jeff Garzik <jgarzik@pobox.com> wrote:

> Stephen Hemminger wrote:
> > Patches are in order of severity. 1-3 are bug fixes, 4 is a cleanup
> > of the power state code, and 5 adds wake on lan support.
> > 
> > IMHO, it is bad security policy to allow wake on lan to enabled by default.
> > The sky2 driver doesn't do WOL until enabled with ethtool.
> 
> While in general I agree with you on the security principle, this seems 
> like it might break working setups.
> 
> WOL is a partnership between the motherboard and NIC.  The motherboard 
> must support WOL, or its useless.  And since the motherboard must 
> support WOL, it normally has an on/off switch in BIOS.
> 
> As such, you're overriding the admin's chosen BIOS setting here.
> 
> 	Jeff

But there is no way to read the BIOS settings.

If BIOS was being smart enough to actually, setup the chip, then I can
look at chip registers on startup and see if it is enabled there.


-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [patch 0/6] sky2 driver update (v1.11)

[Tino Keitel]

On Mon, Jan 01, 2007 at 10:36:44 -0800, Stephen Hemminger wrote:
> On Tue, 26 Dec 2006 16:44:24 -0500
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
> > Stephen Hemminger wrote:
> > > Patches are in order of severity. 1-3 are bug fixes, 4 is a cleanup
> > > of the power state code, and 5 adds wake on lan support.
> > > 
> > > IMHO, it is bad security policy to allow wake on lan to enabled by default.
> > > The sky2 driver doesn't do WOL until enabled with ethtool.
> > 
> > While in general I agree with you on the security principle, this seems 
> > like it might break working setups.
> > 
> > WOL is a partnership between the motherboard and NIC.  The motherboard 
> > must support WOL, or its useless.  And since the motherboard must 
> > support WOL, it normally has an on/off switch in BIOS.
> > 
> > As such, you're overriding the admin's chosen BIOS setting here.
> > 
> > 	Jeff
> 
> But there is no way to read the BIOS settings.
> 
> If BIOS was being smart enough to actually, setup the chip, then I can
> look at chip registers on startup and see if it is enabled there.

Some computers even don't have such a BIOS settings (like my Mac mini).

Btw., I just built 2.6.20-rc3 with patches 4 and 5 and wake on LAN now
works. Thanks for your work.

Regards,
Tino

Re: [patch 0/6] sky2 driver update (v1.11)

[Tino Keitel]

On Tue, Jan 02, 2007 at 20:10:15 +0100, Tino Keitel wrote:

[...]

> Btw., I just built 2.6.20-rc3 with patches 4 and 5 and wake on LAN now
> works. Thanks for your work.

Hi,

I had some failures during resume from suspend with 2.6.20-rc3 and
-rc4. I enabled pm_trace and it looks like the sky2 driver is the
culprit:

hash matches drivers/base/power/resume.c:56
hash matches device 0000:01:00.0

$ lspci | grep 01:00.0
01:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8053
PCI-E Gigabit Ethernet Controller (rev 22)

I removed the patches and had no resume failure so far.

Regards,
Tino

Re: [patch 0/6] sky2 driver update (v1.11)

[Stephen Hemminger]

On Sat, 13 Jan 2007 14:03:29 +0100
Tino Keitel <tino.keitel@tikei.de> wrote:

> On Tue, Jan 02, 2007 at 20:10:15 +0100, Tino Keitel wrote:
> 
> [...]
> 
> > Btw., I just built 2.6.20-rc3 with patches 4 and 5 and wake on LAN now
> > works. Thanks for your work.
> 
> Hi,
> 
> I had some failures during resume from suspend with 2.6.20-rc3 and
> -rc4. I enabled pm_trace and it looks like the sky2 driver is the
> culprit:
> 
> hash matches drivers/base/power/resume.c:56
> hash matches device 0000:01:00.0
> 
> $ lspci | grep 01:00.0
> 01:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8053
> PCI-E Gigabit Ethernet Controller (rev 22)
> 
> I removed the patches and had no resume failure so far.
> 
> Regards,
> Tino
> 

What kind of failures, did the system just not come up?
Did you have WOL enabled or not?

The new code checks for pci_ errors on resume and it could
be that the errors were always there.

-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [patch 0/6] sky2 driver update (v1.11)

[Tino Keitel]

On Mon, Jan 15, 2007 at 10:21:49 -0800, Stephen Hemminger wrote:
> On Sat, 13 Jan 2007 14:03:29 +0100
> Tino Keitel <tino.keitel@tikei.de> wrote:
> 
> > On Tue, Jan 02, 2007 at 20:10:15 +0100, Tino Keitel wrote:
> > 
> > [...]
> > 
> > > Btw., I just built 2.6.20-rc3 with patches 4 and 5 and wake on LAN now
> > > works. Thanks for your work.
> > 
> > Hi,
> > 
> > I had some failures during resume from suspend with 2.6.20-rc3 and
> > -rc4. I enabled pm_trace and it looks like the sky2 driver is the
> > culprit:
> > 
> > hash matches drivers/base/power/resume.c:56
> > hash matches device 0000:01:00.0
> > 
> > $ lspci | grep 01:00.0
> > 01:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8053
> > PCI-E Gigabit Ethernet Controller (rev 22)
> > 
> > I removed the patches and had no resume failure so far.
> > 
> > Regards,
> > Tino
> > 
> 
> What kind of failures, did the system just not come up?

Yes, screen stayed dark and machine was dead. However, it was hardly
reproducable. I set up a suspend/resume loop for an hour without
failures. Then, when I just wanted to suspend for a while, resume
failed.

> Did you have WOL enabled or not?

I had WOL enabled.

Regards,
Tino

Re: [patch 0/6] sky2 driver update (v1.11)

[Gerd v. Egidy]

> > > IMHO, it is bad security policy to allow wake on lan to enabled by
> > > default. The sky2 driver doesn't do WOL until enabled with ethtool.
> >
> > While in general I agree with you on the security principle, this seems
> > like it might break working setups.
> >
> > WOL is a partnership between the motherboard and NIC.  The motherboard
> > must support WOL, or its useless.  And since the motherboard must
> > support WOL, it normally has an on/off switch in BIOS.
> >
> > As such, you're overriding the admin's chosen BIOS setting here.
>
> But there is no way to read the BIOS settings.

true.

> If BIOS was being smart enough to actually, setup the chip, then I can
> look at chip registers on startup and see if it is enabled there.

If the BIOS doesn't setup the chip, WOL won't work if you plug in the power 
cord (instead of just using atx poweroff) and is thus nearly useless. 
Correct?

So I'd propose to read the chip registers and set them to the state they were 
in on bootup.

Kind regards,

Gerd