[PATCH 0/7] sky2: patches for 2.6.22

[PATCH 0/7] sky2: patches for 2.6.22

[Stephen Hemminger]

The following are non-critical fixes found during review
of code. I don't expect them to fix the outstanding bugzilla
or mailing list problem reports. Mostly they are of the form,
"don't touch bits that aren't used on that chip".

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[PATCH 1/7] sky2: dont set bogus bit in PHY register

[Stephen Hemminger]

[-- Attachment #1: sky2-gphy-ctrl.patch --]
[-- Type: text/plain, Size: 2242 bytes --]

This code inherited from the sk98lin driver is incorrect on the Yukon2.
The GPHY_CTRL register values are specific to the internal PHY of the chip
and the values used were leftovers.
Driver was setting bit 13 which is now the INT polarity for the PHY!

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


--- a/drivers/net/sky2.c	2007-05-23 08:44:09.000000000 -0700
+++ b/drivers/net/sky2.c	2007-05-23 08:47:11.000000000 -0700
@@ -658,7 +658,7 @@ static void sky2_mac_init(struct sky2_hw
 	const u8 *addr = hw->dev[port]->dev_addr;
 
 	sky2_write32(hw, SK_REG(port, GPHY_CTRL), GPC_RST_SET);
-	sky2_write32(hw, SK_REG(port, GPHY_CTRL), GPC_RST_CLR|GPC_ENA_PAUSE);
+	sky2_write32(hw, SK_REG(port, GPHY_CTRL), GPC_RST_CLR);
 
 	sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_CLR);
 
--- a/drivers/net/sky2.h	2007-05-23 08:44:09.000000000 -0700
+++ b/drivers/net/sky2.h	2007-05-23 08:47:11.000000000 -0700
@@ -1732,28 +1732,6 @@ enum {
 
 /*	GPHY_CTRL		32 bit	GPHY Control Reg (YUKON only) */
 enum {
-	GPC_SEL_BDT	= 1<<28, /* Select Bi-Dir. Transfer for MDC/MDIO */
-	GPC_INT_POL_HI	= 1<<27, /* IRQ Polarity is Active HIGH */
-	GPC_75_OHM	= 1<<26, /* Use 75 Ohm Termination instead of 50 */
-	GPC_DIS_FC	= 1<<25, /* Disable Automatic Fiber/Copper Detection */
-	GPC_DIS_SLEEP	= 1<<24, /* Disable Energy Detect */
-	GPC_HWCFG_M_3	= 1<<23, /* HWCFG_MODE[3] */
-	GPC_HWCFG_M_2	= 1<<22, /* HWCFG_MODE[2] */
-	GPC_HWCFG_M_1	= 1<<21, /* HWCFG_MODE[1] */
-	GPC_HWCFG_M_0	= 1<<20, /* HWCFG_MODE[0] */
-	GPC_ANEG_0	= 1<<19, /* ANEG[0] */
-	GPC_ENA_XC	= 1<<18, /* Enable MDI crossover */
-	GPC_DIS_125	= 1<<17, /* Disable 125 MHz clock */
-	GPC_ANEG_3	= 1<<16, /* ANEG[3] */
-	GPC_ANEG_2	= 1<<15, /* ANEG[2] */
-	GPC_ANEG_1	= 1<<14, /* ANEG[1] */
-	GPC_ENA_PAUSE	= 1<<13, /* Enable Pause (SYM_OR_REM) */
-	GPC_PHYADDR_4	= 1<<12, /* Bit 4 of Phy Addr */
-	GPC_PHYADDR_3	= 1<<11, /* Bit 3 of Phy Addr */
-	GPC_PHYADDR_2	= 1<<10, /* Bit 2 of Phy Addr */
-	GPC_PHYADDR_1	= 1<<9,	 /* Bit 1 of Phy Addr */
-	GPC_PHYADDR_0	= 1<<8,	 /* Bit 0 of Phy Addr */
-						/* Bits  7..2:	reserved */
 	GPC_RST_CLR	= 1<<1,	/* Clear GPHY Reset */
 	GPC_RST_SET	= 1<<0,	/* Set   GPHY Reset */
 };

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[PATCH 2/7] sky2: checksum offload plus vlan bug

[Stephen Hemminger]

[-- Attachment #1: sky2-vlan-ins.patch --]
[-- Type: text/plain, Size: 688 bytes --]

Driver was not correctly setting up transmit descriptor when doing
VLAN tag insertion with checksum offload.

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

--- a/drivers/net/sky2.c	2007-05-22 10:42:36.000000000 -0700
+++ b/drivers/net/sky2.c	2007-05-22 10:43:12.000000000 -0700
@@ -1432,7 +1432,7 @@ static int sky2_xmit_frame(struct sk_buf
 		tcpsum = offset << 16;		/* sum start */
 		tcpsum |= offset + skb->csum_offset;	/* sum write */
 
-		ctrl = CALSUM | WR_SUM | INIT_SUM | LOCK_SUM;
+		ctrl |= CALSUM | WR_SUM | INIT_SUM | LOCK_SUM;
 		if (ip_hdr(skb)->protocol == IPPROTO_UDP)
 			ctrl |= UDPTCP;
 

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[PATCH 3/7] sky2: program proper register for fiber PHY

[Stephen Hemminger]

[-- Attachment #1: sky2-fiber-reg.patch --]
[-- Type: text/plain, Size: 698 bytes --]

Driver was reading value from one register, setting bit and then
writing the wrong register.

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

--- a/drivers/net/sky2.c	2007-05-22 10:43:12.000000000 -0700
+++ b/drivers/net/sky2.c	2007-05-22 10:44:32.000000000 -0700
@@ -364,7 +364,7 @@ static void sky2_phy_init(struct sky2_hw
 			/* for SFP-module set SIGDET polarity to low */
 			ctrl = gm_phy_read(hw, port, PHY_MARV_PHY_CTRL);
 			ctrl |= PHY_M_FIB_SIGD_POL;
-			gm_phy_write(hw, port, PHY_MARV_CTRL, ctrl);
+			gm_phy_write(hw, port, PHY_MARV_PHY_CTRL, ctrl);
 		}
 
 		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, pg);

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[PATCH 4/7] sky2: PHY page register fixes

[Stephen Hemminger]

[-- Attachment #1: sky2-phy-page.patch --]
[-- Type: text/plain, Size: 6530 bytes --]

Several of the PHY registers are multiplexed; access to
register must be proceeded by setting page register.

The driver setup is safer if this is done before the access
rather than depending on the last value.

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

---
 drivers/net/sky2.c |   25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

--- a/drivers/net/sky2.c	2007-05-22 10:44:32.000000000 -0700
+++ b/drivers/net/sky2.c	2007-05-22 10:44:34.000000000 -0700
@@ -252,6 +252,7 @@ static void sky2_gmac_reset(struct sky2_
 	/* disable all GMAC IRQ's */
 	sky2_write8(hw, SK_REG(port, GMAC_IRQ_MSK), 0);
 	/* disable PHY IRQs */
+	gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 	gm_phy_write(hw, port, PHY_MARV_INT_MASK, 0);
 
 	gma_write16(hw, port, GM_MC_ADDR_H1, 0);	/* clear MC hash */
@@ -292,7 +293,7 @@ static const u16 gm_fc_disable[] = {
 static void sky2_phy_init(struct sky2_hw *hw, unsigned port)
 {
 	struct sky2_port *sky2 = netdev_priv(hw->dev[port]);
-	u16 ctrl, ct1000, adv, pg, ledctrl, ledover, reg;
+	u16 ctrl, ct1000, adv, ledctrl, ledover, reg;
 
 	if (sky2->autoneg == AUTONEG_ENABLE
 	    && !(hw->chip_id == CHIP_ID_YUKON_XL
@@ -315,6 +316,7 @@ static void sky2_phy_init(struct sky2_hw
 		gm_phy_write(hw, port, PHY_MARV_EXT_CTRL, ectrl);
 	}
 
+	gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 	ctrl = gm_phy_read(hw, port, PHY_MARV_PHY_CTRL);
 	if (sky2_is_copper(hw)) {
 		if (hw->chip_id == CHIP_ID_YUKON_FE) {
@@ -348,8 +350,6 @@ static void sky2_phy_init(struct sky2_hw
 
 	/* special setup for PHY 88E1112 Fiber */
 	if (hw->chip_id == CHIP_ID_YUKON_XL && !sky2_is_copper(hw)) {
-		pg = gm_phy_read(hw, port, PHY_MARV_EXT_ADR);
-
 		/* Fiber: select 1000BASE-X only mode MAC Specific Ctrl Reg. */
 		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 2);
 		ctrl = gm_phy_read(hw, port, PHY_MARV_PHY_CTRL);
@@ -367,7 +367,6 @@ static void sky2_phy_init(struct sky2_hw
 			gm_phy_write(hw, port, PHY_MARV_PHY_CTRL, ctrl);
 		}
 
-		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, pg);
 	}
 
 	ctrl = PHY_CT_RESET;
@@ -463,8 +462,6 @@ static void sky2_phy_init(struct sky2_hw
 		break;
 
 	case CHIP_ID_YUKON_XL:
-		pg = gm_phy_read(hw, port, PHY_MARV_EXT_ADR);
-
 		/* select page 3 to access LED control register */
 		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 3);
 
@@ -483,15 +480,10 @@ static void sky2_phy_init(struct sky2_hw
 			      PHY_M_POLC_INIT_CTRL(2) |
 			      PHY_M_POLC_STA1_CTRL(2) |
 			      PHY_M_POLC_STA0_CTRL(2)));
-
-		/* restore page register */
-		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, pg);
 		break;
 
 	case CHIP_ID_YUKON_EC_U:
 	case CHIP_ID_YUKON_EX:
-		pg = gm_phy_read(hw, port, PHY_MARV_EXT_ADR);
-
 		/* select page 3 to access LED control register */
 		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 3);
 
@@ -505,8 +497,7 @@ static void sky2_phy_init(struct sky2_hw
 		/* set Blink Rate in LED Timer Control Register */
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK,
 			     ledctrl | PHY_M_LED_BLINK_RT(BLINK_84MS));
-		/* restore page register */
-		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, pg);
+
 		break;
 
 	default:
@@ -529,8 +520,6 @@ static void sky2_phy_init(struct sky2_hw
 		gm_phy_write(hw, port, 0x18, 0xa204);
 		gm_phy_write(hw, port, 0x17, 0x2002);
 
-		/* set page register to 0 */
-		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 	} else if (hw->chip_id != CHIP_ID_YUKON_EX) {
 		gm_phy_write(hw, port, PHY_MARV_LED_CTRL, ledctrl);
 
@@ -545,6 +534,7 @@ static void sky2_phy_init(struct sky2_hw
 	}
 
 	/* Enable phy interrupt on auto-negotiation complete (or link up) */
+	gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 	if (sky2->autoneg == AUTONEG_ENABLE)
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_IS_AN_COMPL);
 	else
@@ -663,6 +653,7 @@ static void sky2_mac_init(struct sky2_hw
 	sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_CLR);
 
 	if (hw->chip_id == CHIP_ID_YUKON_XL && hw->chip_rev == 0 && port == 1) {
+		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 		/* WA DEV_472 -- looks like crossed wires on port 2 */
 		/* clear GMAC 1 Control reset */
 		sky2_write8(hw, SK_REG(0, GMAC_CTRL), GMC_RST_CLR);
@@ -1690,6 +1681,7 @@ static void sky2_link_up(struct sky2_por
 	reg |= GM_GPCR_RX_ENA | GM_GPCR_TX_ENA;
 	gma_write16(hw, port, GM_GP_CTRL, reg);
 
+	gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 	gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_DEF_MSK);
 
 	netif_carrier_on(sky2->netdev);
@@ -1738,6 +1730,7 @@ static void sky2_link_down(struct sky2_p
 	unsigned port = sky2->port;
 	u16 reg;
 
+	gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 	gm_phy_write(hw, port, PHY_MARV_INT_MASK, 0);
 
 	reg = gma_read16(hw, port, GM_GP_CTRL);
@@ -1838,6 +1831,7 @@ static void sky2_phy_intr(struct sky2_hw
 		return;
 
 	spin_lock(&sky2->phy_lock);
+	gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 0);
 	istatus = gm_phy_read(hw, port, PHY_MARV_INT_STAT);
 	phystat = gm_phy_read(hw, port, PHY_MARV_PHY_STAT);
 
@@ -3085,11 +3079,9 @@ static void sky2_set_multicast(struct ne
  */
 static void sky2_led(struct sky2_hw *hw, unsigned port, int on)
 {
-	u16 pg;
 
 	switch (hw->chip_id) {
 	case CHIP_ID_YUKON_XL:
-		pg = gm_phy_read(hw, port, PHY_MARV_EXT_ADR);
 		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 3);
 		gm_phy_write(hw, port, PHY_MARV_PHY_CTRL,
 			     on ? (PHY_M_LEDC_LOS_CTRL(1) |
@@ -3098,7 +3090,6 @@ static void sky2_led(struct sky2_hw *hw,
 				   PHY_M_LEDC_STA0_CTRL(7))
 			     : 0);
 
-		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, pg);
 		break;
 
 	default:
@@ -3127,10 +3118,8 @@ static int sky2_phys_id(struct net_devic
 	/* save initial values */
 	spin_lock_bh(&sky2->phy_lock);
 	if (hw->chip_id == CHIP_ID_YUKON_XL) {
-		u16 pg = gm_phy_read(hw, port, PHY_MARV_EXT_ADR);
 		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 3);
 		ledctrl = gm_phy_read(hw, port, PHY_MARV_PHY_CTRL);
-		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, pg);
 	} else {
 		ledctrl = gm_phy_read(hw, port, PHY_MARV_LED_CTRL);
 		ledover = gm_phy_read(hw, port, PHY_MARV_LED_OVER);
@@ -3150,10 +3139,8 @@ static int sky2_phys_id(struct net_devic
 
 	/* resume regularly scheduled programming */
 	if (hw->chip_id == CHIP_ID_YUKON_XL) {
-		u16 pg = gm_phy_read(hw, port, PHY_MARV_EXT_ADR);
 		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, 3);
 		gm_phy_write(hw, port, PHY_MARV_PHY_CTRL, ledctrl);
-		gm_phy_write(hw, port, PHY_MARV_EXT_ADR, pg);
 	} else {
 		gm_phy_write(hw, port, PHY_MARV_LED_CTRL, ledctrl);
 		gm_phy_write(hw, port, PHY_MARV_LED_OVER, ledover);

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[PATCH 5/7] sky2: enable IRQ on duplex renegotiation

[Stephen Hemminger]

[-- Attachment #1: sky2-duplex-irq.patch --]
[-- Type: text/plain, Size: 618 bytes --]

Don't want IRQ on FIFO error because there is nothing useful to do with it.
But do want IRQ on duplex change.

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

--- a/drivers/net/sky2.h	2007-05-22 10:42:36.000000000 -0700
+++ b/drivers/net/sky2.h	2007-05-22 10:44:35.000000000 -0700
@@ -1149,7 +1149,7 @@ enum {
 	PHY_M_IS_JABBER		= 1<<0, /* Jabber */
 
 	PHY_M_DEF_MSK		= PHY_M_IS_LSP_CHANGE | PHY_M_IS_LST_CHANGE
-				 | PHY_M_IS_FIFO_ERROR,
+				 | PHY_M_IS_DUP_CHANGE,
 	PHY_M_AN_MSK	       = PHY_M_IS_AN_ERROR | PHY_M_IS_AN_COMPL,
 };
 

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[PATCH 6/7] sky2: enable clocks before probe

[Stephen Hemminger]

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

Some chips need to have internal clocks enabled (via PCI config)
before the PCI space is readable.

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

---
 drivers/net/sky2.c |  102 ++++++++++++++++++++++++++++++++++++++---------------
 drivers/net/sky2.h |   19 +++++++++
 2 files changed, 92 insertions(+), 29 deletions(-)

--- a/drivers/net/sky2.c	2007-05-24 15:22:14.000000000 -0700
+++ b/drivers/net/sky2.c	2007-05-24 15:22:33.000000000 -0700
@@ -2524,10 +2524,6 @@ static int __devinit sky2_init(struct sk
 		dev_warn(&hw->pdev->dev, "this driver not yet tested on this chip type\n"
 			 "Please report success or failure to <netdev@vger.kernel.org>\n");
 
-	/* Make sure and enable all clocks */
-	if (hw->chip_id == CHIP_ID_YUKON_EX || hw->chip_id == CHIP_ID_YUKON_EC_U)
-		sky2_pci_write32(hw, PCI_DEV_REG3, 0);
-
 	hw->chip_rev = (sky2_read8(hw, B2_MAC_CFG) & CFG_CHIP_R_MSK) >> 4;
 
 	/* This rev is really old, and requires untested workarounds */
@@ -3638,6 +3634,9 @@ static int __devinit sky2_probe(struct p
 	if (!hw->st_le)
 		goto err_out_iounmap;
 
+	/* On Yukon EC-U and EX need to force enabling all clocks */
+	pci_write_config_dword(pdev, PCI_DEV_REG3, 0);
+
 	err = sky2_init(hw);
 	if (err)
 		goto err_out_iounmap;
@@ -3822,8 +3821,7 @@ 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)
-		sky2_pci_write32(hw, PCI_DEV_REG3, 0);
+	pci_write_config_dword(pdev, PCI_DEV_REG3, 0);
 
 	sky2_reset(hw);
 

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

[PATCH 7/7] sky2: version 1.15

[Stephen Hemminger]

[-- Attachment #1: sky2-1.15 --]
[-- Type: text/plain, Size: 552 bytes --]

Why? Most bug reports on vendor kernels with backported
drivers.

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

--- a/drivers/net/sky2.c	2007-05-24 11:20:26.000000000 -0700
+++ b/drivers/net/sky2.c	2007-05-24 11:20:29.000000000 -0700
@@ -50,7 +50,7 @@
 #include "sky2.h"
 
 #define DRV_NAME		"sky2"
-#define DRV_VERSION		"1.14"
+#define DRV_VERSION		"1.15"
 #define PFX			DRV_NAME " "
 
 /*

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH 1/7] sky2: dont set bogus bit in PHY register

[Jeff Garzik]

Stephen Hemminger wrote:
> This code inherited from the sk98lin driver is incorrect on the Yukon2.
> The GPHY_CTRL register values are specific to the internal PHY of the chip
> and the values used were leftovers.
> Driver was setting bit 13 which is now the INT polarity for the PHY!
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

applied 1-3

Re: [PATCH 5/7] sky2: enable IRQ on duplex renegotiation

[Jeff Garzik]

Stephen Hemminger wrote:
> Don't want IRQ on FIFO error because there is nothing useful to do with it.
> But do want IRQ on duplex change.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

applied

Re: [PATCH 6/7] sky2: enable clocks before probe

[Jeff Garzik]

Stephen Hemminger wrote:
> Some chips need to have internal clocks enabled (via PCI config)
> before the PCI space is readable.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> ---
>  drivers/net/sky2.c |  102 ++++++++++++++++++++++++++++++++++++++---------------
>  drivers/net/sky2.h |   19 +++++++++
>  2 files changed, 92 insertions(+), 29 deletions(-)
> 
> --- a/drivers/net/sky2.c	2007-05-24 15:22:14.000000000 -0700
> +++ b/drivers/net/sky2.c	2007-05-24 15:22:33.000000000 -0700
> @@ -2524,10 +2524,6 @@ static int __devinit sky2_init(struct sk
>  		dev_warn(&hw->pdev->dev, "this driver not yet tested on this chip type\n"
>  			 "Please report success or failure to <netdev@vger.kernel.org>\n");
>  
> -	/* Make sure and enable all clocks */
> -	if (hw->chip_id == CHIP_ID_YUKON_EX || hw->chip_id == CHIP_ID_YUKON_EC_U)
> -		sky2_pci_write32(hw, PCI_DEV_REG3, 0);
> -
>  	hw->chip_rev = (sky2_read8(hw, B2_MAC_CFG) & CFG_CHIP_R_MSK) >> 4;
>  
>  	/* This rev is really old, and requires untested workarounds */
> @@ -3638,6 +3634,9 @@ static int __devinit sky2_probe(struct p
>  	if (!hw->st_le)
>  		goto err_out_iounmap;
>  
> +	/* On Yukon EC-U and EX need to force enabling all clocks */
> +	pci_write_config_dword(pdev, PCI_DEV_REG3, 0);
> +
>  	err = sky2_init(hw);
>  	if (err)
>  		goto err_out_iounmap;
> @@ -3822,8 +3821,7 @@ 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)
> -		sky2_pci_write32(hw, PCI_DEV_REG3, 0);
> +	pci_write_config_dword(pdev, PCI_DEV_REG3, 0);


No explanation as to why the chip identity tests went away.  And in one 
of your code comments, it seems to imply that the chip tests are still 
present.

Re: [PATCH 4/7] sky2: PHY page register fixes

[Jeff Garzik]

Stephen Hemminger wrote:
> Several of the PHY registers are multiplexed; access to
> register must be proceeded by setting page register.
> 
> The driver setup is safer if this is done before the access
> rather than depending on the last value.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

NAK for 2.622 - this appears to be a highly speculative cleanup-style 
change that is not an appropriate submission so late in the Release 
Candidate cycle.

I would have expected this sort of change to have been brewing for a 
little while in -mm, via netdev#upstream perhaps.  As it is, this change 
has been zero testing exposure, which is not something we want in a -rc2 
or -rc3.

	Jeff

Re: [PATCH 6/7] sky2: enable clocks before probe

[Stephen Hemminger]

Jeff Garzik wrote:
> Stephen Hemminger wrote:
>> Some chips need to have internal clocks enabled (via PCI config)
>> before the PCI space is readable.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
>>
>> ---
>>  drivers/net/sky2.c |  102 
>> ++++++++++++++++++++++++++++++++++++++---------------
>>  drivers/net/sky2.h |   19 +++++++++
>>  2 files changed, 92 insertions(+), 29 deletions(-)
>>
>> --- a/drivers/net/sky2.c    2007-05-24 15:22:14.000000000 -0700
>> +++ b/drivers/net/sky2.c    2007-05-24 15:22:33.000000000 -0700
>> @@ -2524,10 +2524,6 @@ static int __devinit sky2_init(struct sk
>>          dev_warn(&hw->pdev->dev, "this driver not yet tested on this 
>> chip type\n"
>>               "Please report success or failure to 
>> <netdev@vger.kernel.org>\n");
>>  
>> -    /* Make sure and enable all clocks */
>> -    if (hw->chip_id == CHIP_ID_YUKON_EX || hw->chip_id == 
>> CHIP_ID_YUKON_EC_U)
>> -        sky2_pci_write32(hw, PCI_DEV_REG3, 0);
>> -
>>      hw->chip_rev = (sky2_read8(hw, B2_MAC_CFG) & CFG_CHIP_R_MSK) >> 4;
>>  
>>      /* This rev is really old, and requires untested workarounds */
>> @@ -3638,6 +3634,9 @@ static int __devinit sky2_probe(struct p
>>      if (!hw->st_le)
>>          goto err_out_iounmap;
>>  
>> +    /* On Yukon EC-U and EX need to force enabling all clocks */
>> +    pci_write_config_dword(pdev, PCI_DEV_REG3, 0);
>> +
>>      err = sky2_init(hw);
>>      if (err)
>>          goto err_out_iounmap;
>> @@ -3822,8 +3821,7 @@ 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)
>> -        sky2_pci_write32(hw, PCI_DEV_REG3, 0);
>> +    pci_write_config_dword(pdev, PCI_DEV_REG3, 0);
>
>
> No explanation as to why the chip identity tests went away.  And in 
> one of your code comments, it seems to imply that the chip tests are 
> still present.
>
Because you can't read chip idenity on 88e8071 when clocks are disabled 
(catch-22), and clearing
registers on other chips is no problem.

Re: [PATCH 6/7] sky2: enable clocks before probe

[Stephen Hemminger]

On Wed, 30 May 2007 09:56:18 -0400
Jeff Garzik <jgarzik@pobox.com> wrote:

> Stephen Hemminger wrote:
> > Some chips need to have internal clocks enabled (via PCI config)
> > before the PCI space is readable.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> > 
> > ---
> >  drivers/net/sky2.c |  102 ++++++++++++++++++++++++++++++++++++++---------------
> >  drivers/net/sky2.h |   19 +++++++++
> >  2 files changed, 92 insertions(+), 29 deletions(-)
> > 
> > --- a/drivers/net/sky2.c	2007-05-24 15:22:14.000000000 -0700
> > +++ b/drivers/net/sky2.c	2007-05-24 15:22:33.000000000 -0700
> > @@ -2524,10 +2524,6 @@ static int __devinit sky2_init(struct sk
> >  		dev_warn(&hw->pdev->dev, "this driver not yet tested on this chip type\n"
> >  			 "Please report success or failure to <netdev@vger.kernel.org>\n");
> >  
> > -	/* Make sure and enable all clocks */
> > -	if (hw->chip_id == CHIP_ID_YUKON_EX || hw->chip_id == CHIP_ID_YUKON_EC_U)
> > -		sky2_pci_write32(hw, PCI_DEV_REG3, 0);
> > -
> >  	hw->chip_rev = (sky2_read8(hw, B2_MAC_CFG) & CFG_CHIP_R_MSK) >> 4;
> >  
> >  	/* This rev is really old, and requires untested workarounds */
> > @@ -3638,6 +3634,9 @@ static int __devinit sky2_probe(struct p
> >  	if (!hw->st_le)
> >  		goto err_out_iounmap;
> >  
> > +	/* On Yukon EC-U and EX need to force enabling all clocks */
> > +	pci_write_config_dword(pdev, PCI_DEV_REG3, 0);
> > +
> >  	err = sky2_init(hw);
> >  	if (err)
> >  		goto err_out_iounmap;
> > @@ -3822,8 +3821,7 @@ 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)
> > -		sky2_pci_write32(hw, PCI_DEV_REG3, 0);
> > +	pci_write_config_dword(pdev, PCI_DEV_REG3, 0);
> 
> 
> No explanation as to why the chip identity tests went away.  And in one 
> of your code comments, it seems to imply that the chip tests are still 
> present.

I'll send a new version with better comments in next batch.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>