[PATCH 0/6] more sky2 patches for 2.6.22

[PATCH 0/6] more sky2 patches for 2.6.22

[Stephen Hemminger]

These are less serious than the last group, and are mostly
one liners.  I decided to give up blacklisting the 88e8056
problem in the driver, if the driver can't identify what configuration
is good/bad, then it has no business trying to enforce the policy.


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

[PATCH 1/6] sky2: remove Gigabyte 88e8056 restriction

[Stephen Hemminger]

[-- Attachment #1: sky2-no-dmi.patch --]
[-- Type: text/plain, Size: 1989 bytes --]

The problems with Gigabyte motherboards are system configuration dependent.
Since it works fine for some users, it doesn't make sense to deprive
them.

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

---
 drivers/net/sky2.c |   30 ------------------------------
 1 file changed, 30 deletions(-)

--- 2.6.21-rc1.orig/drivers/net/sky2.c	2007-05-14 12:06:07.000000000 -0700
+++ 2.6.21-rc1/drivers/net/sky2.c	2007-05-14 12:06:25.000000000 -0700
@@ -40,7 +40,6 @@
 #include <linux/if_vlan.h>
 #include <linux/prefetch.h>
 #include <linux/mii.h>
-#include <linux/dmi.h>
 
 #include <asm/irq.h>
 
@@ -151,8 +150,6 @@ static const char *yukon2_name[] = {
 	"FE",		/* 0xb7 */
 };
 
-static int dmi_blacklisted;
-
 /* Access to external PHY */
 static int gm_phy_write(struct sky2_hw *hw, unsigned port, u16 reg, u16 val)
 {
@@ -2534,17 +2531,6 @@ static int __devinit sky2_init(struct sk
 		return -EOPNOTSUPP;
 	}
 
-
-	/* Some Gigabyte motherboards have 88e8056 but cause problems
-	 * There is some unresolved hardware related problem that causes
-	 * descriptor errors and receive data corruption.
-	 */
-	if (hw->chip_id == CHIP_ID_YUKON_EC_U && dmi_blacklisted) {
-		dev_err(&hw->pdev->dev,
-			"88E8056 on this motherboard not supported\n");
-		return -EOPNOTSUPP;
-	}
-
 	hw->pmd_type = sky2_read8(hw, B2_PMD_TYP);
 	hw->ports = 1;
 	t8 = sky2_read8(hw, B2_Y2_HW_RES);
@@ -3910,24 +3896,8 @@ static struct pci_driver sky2_driver = {
 	.shutdown = sky2_shutdown,
 };
 
-static struct dmi_system_id __initdata broken_dmi_table[] = {
-	{
-		.ident = "Gigabyte 965P-S3",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Gigabyte Technology Co., Ltd."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "965P-S3"),
-
-		},
-	},
-	{ }
-};
-
 static int __init sky2_init_module(void)
 {
-	/* Look for sick motherboards */
-	if (dmi_check_system(broken_dmi_table))
-		dmi_blacklisted = 1;
-
 	return pci_register_driver(&sky2_driver);
 }
 

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

[PATCH 2/6] sky2: PHY register settings

[Stephen Hemminger]

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

Align the PHY setup of the sky2 driver with the vendor sk98lin (10.0.4.3)
driver. The PHY register settings are mostly black magic, even with access
to the documentation it isn't clear what the right values are. The changes
are mostly comments, the code change only affects the Yukon FE (100 mbit only)
version.

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

---
 drivers/net/sky2.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--- 2.6.21-rc1.orig/drivers/net/sky2.c	2007-05-14 12:06:25.000000000 -0700
+++ 2.6.21-rc1/drivers/net/sky2.c	2007-05-14 12:10:13.000000000 -0700
@@ -304,10 +304,13 @@ static void sky2_phy_init(struct sky2_hw
 			   PHY_M_EC_MAC_S_MSK);
 		ectrl |= PHY_M_EC_MAC_S(MAC_TX_CLK_25_MHZ);
 
+		/* on PHY 88E1040 Rev.D0 (and newer) downshift control changed */
 		if (hw->chip_id == CHIP_ID_YUKON_EC)
+			/* set downshift counter to 3x and enable downshift */
 			ectrl |= PHY_M_EC_DSC_2(2) | PHY_M_EC_DOWN_S_ENA;
 		else
-			ectrl |= PHY_M_EC_M_DSC(2) | PHY_M_EC_S_DSC(3);
+			/* set master & slave downshift counter to 1x */
+			ectrl |= PHY_M_EC_M_DSC(0) | PHY_M_EC_S_DSC(1);
 
 		gm_phy_write(hw, port, PHY_MARV_EXT_CTRL, ectrl);
 	}
@@ -324,10 +327,12 @@ static void sky2_phy_init(struct sky2_hw
 			/* enable automatic crossover */
 			ctrl |= PHY_M_PC_MDI_XMODE(PHY_M_PC_ENA_AUTO);
 
+			/* downshift on PHY 88E1112 and 88E1149 is changed */
 			if (sky2->autoneg == AUTONEG_ENABLE
 			    && (hw->chip_id == CHIP_ID_YUKON_XL
 				|| hw->chip_id == CHIP_ID_YUKON_EC_U
 				|| hw->chip_id == CHIP_ID_YUKON_EX)) {
+				/* set downshift counter to 3x and enable downshift */
 				ctrl &= ~PHY_M_PC_DSC_MSK;
 				ctrl |= PHY_M_PC_DSC(2) | PHY_M_PC_DOWN_S_ENA;
 			}

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

[PATCH 3/6] sky2: keep track of receive alloc failures

[Stephen Hemminger]

[-- Attachment #1: sky2-rx_dropped --]
[-- Type: text/plain, Size: 792 bytes --]

When driver can't allocate receive buffer it drops incoming
packet. Keep a counter.

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

---
 drivers/net/sky2.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- 2.6.21-rc1.orig/drivers/net/sky2.c	2007-05-14 12:10:13.000000000 -0700
+++ 2.6.21-rc1/drivers/net/sky2.c	2007-05-14 12:10:16.000000000 -0700
@@ -2141,8 +2141,10 @@ static int sky2_status_intr(struct sky2_
 		switch (le->opcode & ~HW_OWNER) {
 		case OP_RXSTAT:
 			skb = sky2_receive(dev, length, status);
-			if (!skb)
+			if (unlikely(!skb)) {
+				sky2->net_stats.rx_dropped++;
 				goto force_update;
+			}
 
 			skb->protocol = eth_type_trans(skb, dev);
 			sky2->net_stats.rx_packets++;

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

[PATCH 4/6] sky2: MIB counter overflow handling

[Stephen Hemminger]

[-- Attachment #1: sky2-mib-ov.patch --]
[-- Type: text/plain, Size: 889 bytes --]

Make sure that if we ever get a MIB counter overflow interrupt (normally
masked off), that the IRQ is cleared.

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

---
 drivers/net/sky2.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- 2.6.21-rc1.orig/drivers/net/sky2.c	2007-05-14 12:10:16.000000000 -0700
+++ 2.6.21-rc1/drivers/net/sky2.c	2007-05-14 12:10:18.000000000 -0700
@@ -2345,6 +2345,12 @@ static void sky2_mac_intr(struct sky2_hw
 		printk(KERN_INFO PFX "%s: mac interrupt status 0x%x\n",
 		       dev->name, status);
 
+	if (status & GM_IS_RX_CO_OV)
+		gma_read16(hw, port, GM_RX_IRQ_SRC);
+
+	if (status & GM_IS_TX_CO_OV)
+		gma_read16(hw, port, GM_TX_IRQ_SRC);
+
 	if (status & GM_IS_RX_FF_OR) {
 		++sky2->net_stats.rx_fifo_errors;
 		sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_CLI_RX_FO);

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

[PATCH 5/6] sky2: remove dual port workaround

[Stephen Hemminger]

[-- Attachment #1: sky2-napi-ok.patch --]
[-- Type: text/plain, Size: 935 bytes --]

This workaround was added to deal with NAPI core and how
it affected dual port shared polling. It turned out not to
be necessary. Stopping device 0 only doesn't stop NAPI from
working completely after that.

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

---
 drivers/net/sky2.c |    7 -------
 1 file changed, 7 deletions(-)

--- 2.6.21-rc1.orig/drivers/net/sky2.c	2007-05-14 12:10:18.000000000 -0700
+++ 2.6.21-rc1/drivers/net/sky2.c	2007-05-14 12:10:22.000000000 -0700
@@ -1579,13 +1579,6 @@ static int sky2_down(struct net_device *
 	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@linux-foundation.org>

[PATCH 6/6] sky2: memory barriers change

[Stephen Hemminger]

[-- Attachment #1: sky2-more-mb.patch --]
[-- Type: text/plain, Size: 2109 bytes --]

Do some memory barrier changes for safety/perfomance:
Don't need read after update to index, mmiowb() followed by read at end
of irq is sufficient.

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

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

--- 2.6.21-rc1.orig/drivers/net/sky2.c	2007-05-14 12:10:22.000000000 -0700
+++ 2.6.21-rc1/drivers/net/sky2.c	2007-05-14 12:10:24.000000000 -0700
@@ -844,10 +844,12 @@ static inline struct tx_ring_info *tx_le
 /* Update chip's next pointer */
 static inline void sky2_put_idx(struct sky2_hw *hw, unsigned q, u16 idx)
 {
-	q = Y2_QADDR(q, PREF_UNIT_PUT_IDX);
+	/* Make sure write' to descriptors are complete before we tell hardware */
 	wmb();
-	sky2_write16(hw, q, idx);
-	sky2_read16(hw, q);
+	sky2_write16(hw, Y2_QADDR(q, PREF_UNIT_PUT_IDX), idx);
+
+	/* Synchronize I/O on since next processor may write to tail */
+	mmiowb();
 }
 
 
@@ -979,6 +981,7 @@ stopped:
 
 	/* reset the Rx prefetch unit */
 	sky2_write32(hw, Y2_QADDR(rxq, PREF_UNIT_CTRL), PREF_UNIT_RST_SET);
+	mmiowb();
 }
 
 /* Clean out receive buffer area, assumes receiver hardware stopped */
@@ -1198,7 +1201,7 @@ static int sky2_rx_start(struct sky2_por
 	}
 
 	/* Tell chip about available buffers */
-	sky2_write16(hw, Y2_QADDR(rxq, PREF_UNIT_PUT_IDX), sky2->rx_put);
+	sky2_put_idx(hw, rxq, sky2->rx_put);
 	return 0;
 nomem:
 	sky2_rx_clean(sky2);
@@ -1540,6 +1543,8 @@ static void sky2_tx_complete(struct sky2
 	}
 
 	sky2->tx_cons = idx;
+	smp_mb();
+
 	if (tx_avail(sky2) > MAX_SKB_TX_LE + 4)
 		netif_wake_queue(dev);
 }
@@ -2218,6 +2223,7 @@ force_update:
 
 	/* Fully processed status ring so clear irq */
 	sky2_write32(hw, STAT_CTRL, SC_STAT_CLR_IRQ);
+	mmiowb();
 
 exit_loop:
 	if (buf_write[0]) {
@@ -2442,6 +2448,7 @@ static int sky2_poll(struct net_device *
 	if (work_done < work_limit) {
 		netif_rx_complete(dev0);
 
+		/* end of interrupt, re-enables also acts as I/O synchronization */
 		sky2_read32(hw, B0_Y2_SP_LISR);
 		return 0;
 	} else {

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

Re: [PATCH 1/6] sky2: remove Gigabyte 88e8056 restriction

[Jeff Garzik]

Stephen Hemminger wrote:
> The problems with Gigabyte motherboards are system configuration dependent.
> Since it works fine for some users, it doesn't make sense to deprive
> them.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

applied 1-6, but I'm watching this change closely.  This area of code 
has seen quite a bit of change-it-then-change-it-back activity :/

	Jeff