Skip to contentSkip to conversation

sky2: patches for 2.6.22

PATCH Applied Stephen Hemminger 3 participants 14 messages May 24, 2007 – May 30, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

cover sky2: patches for 2.6.22

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

Signature · 1 line
Stephen Hemminger <[email protected]>
author
#1 lore raw

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

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 <[email protected]>
drivers/net/sky2.c +1 -1
@@ -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);
drivers/net/sky2.h +0 -22
@@ -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 */
 };
Signature · 1 line
Stephen Hemminger <[email protected]>
#2 lore raw

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

Quoted 6 lines · Stephen Hemminger

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 <[email protected]>

applied 1-3

author
#3 lore raw

PATCH 2/7 sky2: checksum offload plus vlan bug

In reply to Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
drivers/net/sky2.c +1 -1
@@ -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;
Signature · 1 line
Stephen Hemminger <[email protected]>
author
#4 lore raw

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

In reply to Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
drivers/net/sky2.c +1 -1
@@ -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);
Signature · 1 line
Stephen Hemminger <[email protected]>
author
#5 lore raw

PATCH 4/7 sky2: PHY page register fixes

In reply to Stephen Hemminger

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 <[email protected]>
---
 drivers/net/sky2.c |   25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)
drivers/net/sky2.c +9 -22
@@ -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);
Signature · 1 line
Stephen Hemminger <[email protected]>
#6 lore raw

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

Quoted 7 lines · Stephen Hemminger

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 <[email protected]>

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

author
#7 lore raw

PATCH 5/7 sky2: enable IRQ on duplex renegotiation

In reply to Stephen Hemminger

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 <[email protected]>
drivers/net/sky2.h +1 -1
@@ -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,
 };
Signature · 1 line
Stephen Hemminger <[email protected]>
Applied
#8 lore raw

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

Quoted 4 lines · Stephen Hemminger

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 <[email protected]>

applied

author
#9 lore raw

PATCH 6/7 sky2: enable clocks before probe

In reply to Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/sky2.c |  102 ++++++++++++++++++++++++++++++++++++++---------------
 drivers/net/sky2.h |   19 +++++++++
 2 files changed, 92 insertions(+), 29 deletions(-)
drivers/net/sky2.c +4 -6
@@ -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 <[email protected]>\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);
Signature · 1 line
Stephen Hemminger <[email protected]>
#10 lore raw

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

Quoted 34 lines · Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/sky2.c |  102 ++++++++++++++++++++++++++++++++++++++---------------
 drivers/net/sky2.h |   19 +++++++++
 2 files changed, 92 insertions(+), 29 deletions(-)
drivers/net/sky2.c +3 -4
@@ -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 <[email protected]>\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;

drivers/net/sky2.c static int sky2_resume(struct pci_dev *p · line 3824

Quoted 6 lines
drivers/net/sky2.c +1 -2
@@ -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.

author
#11 lore raw

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

Quoted 41 lines · Jeff Garzik
Quoted 40 lines · Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/sky2.c |  102 
++++++++++++++++++++++++++++++++++++++---------------
 drivers/net/sky2.h |   19 +++++++++
 2 files changed, 92 insertions(+), 29 deletions(-)
drivers/net/sky2.c 2007-05-24 15:22:33.000000000 -0700 renamed from drivers/net/sky2.c 2007-05-24 15:22:14.000000000 -0700 +0 -0
@@ -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
<[email protected]>\n");

drivers/net/sky2.c 2007-05-24 15:22:33.000000000 -0700 renamed from drivers/net/sky2.c 2007-05-24 15:22:14.000000000 -0700 +0 -2
-    /* Make sure and enable all clocks */
-    if (hw->chip_id == CHIP_ID_YUKON_EX || hw->chip_id ==

CHIP_ID_YUKON_EC_U)

drivers/net/sky2.c 2007-05-24 15:22:33.000000000 -0700 renamed from drivers/net/sky2.c 2007-05-24 15:22:14.000000000 -0700 +3 -2
-        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);
Quoted 9 lines

drivers/net/sky2.c 2007-05-24 15:22:33.000000000 -0700 static int sky2_resume(struct pci_dev *p · line 3825

Quoted 5 lines
drivers/net/sky2.c 2007-05-24 15:22:33.000000000 -0700 renamed from drivers/net/sky2.c 2007-05-24 15:22:14.000000000 -0700 +1 -2
     /* 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.

author
#12 lore raw

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

In reply to Jeff Garzik
Quoted 37 lines · On Wed, 30 May 2007 09:56:18 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 36 lines · Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/sky2.c |  102 ++++++++++++++++++++++++++++++++++++++---------------
 drivers/net/sky2.h |   19 +++++++++
 2 files changed, 92 insertions(+), 29 deletions(-)
drivers/net/sky2.c +3 -4
@@ -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 <[email protected]>\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);
Quoted 9 lines

drivers/net/sky2.c static int sky2_resume(struct pci_dev *p · line 3824

Quoted 4 lines
drivers/net/sky2.c +1 -2
 	/* 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.

Signature · 1 line
Stephen Hemminger <[email protected]>
author
#13 lore raw

PATCH 7/7 sky2: version 1.15

In reply to Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/sky2.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/sky2.c +1 -1
@@ -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 " "
 
 /*
Signature · 1 line
Stephen Hemminger <[email protected]>