[PATCH 0/6] sky2: v1.13 patches

[PATCH 0/6] sky2: v1.13 patches

[Stephen Hemminger]

This set of patches fixes all the problems observed so far on
my machines. The biggest one was not doing transmit flow control
correctly.

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

[PATCH 1/6] sky2: dont flush good pause frames

[Stephen Hemminger]

[-- Attachment #1: sky2-pause-flush.patch --]
[-- Type: text/plain, Size: 778 bytes --]

Don't mark pause frames as errors. This problem caused transmitter not
to pause and would effectively take out a gigabit switch because the
it can't handle overrun. 

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

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

--- sky2-dev.orig/drivers/net/sky2.h	2007-02-13 15:08:30.000000000 -0800
+++ sky2-dev/drivers/net/sky2.h	2007-02-13 15:12:52.000000000 -0800
@@ -1589,7 +1589,7 @@
 
 	GMR_FS_ANY_ERR	= GMR_FS_RX_FF_OV | GMR_FS_CRC_ERR |
 			  GMR_FS_FRAGMENT | GMR_FS_LONG_ERR |
-		  	  GMR_FS_MII_ERR | GMR_FS_GOOD_FC | GMR_FS_BAD_FC |
+		  	  GMR_FS_MII_ERR | GMR_FS_BAD_FC |
 			  GMR_FS_UN_SIZE | GMR_FS_JABBER,
 };
 

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

Re: [PATCH 1/6] sky2: dont flush good pause frames

[Jeff Garzik]

Stephen Hemminger wrote:
> Don't mark pause frames as errors. This problem caused transmitter not
> to pause and would effectively take out a gigabit switch because the
> it can't handle overrun. 
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

applied 1-6

[PATCH 2/6] sky2: no need to reset pause bits on shutdown

[Stephen Hemminger]

[-- Attachment #1: sky2-no-asym-down.patch --]
[-- Type: text/plain, Size: 801 bytes --]

Resetting the pause bits on shutdown is not necessary.
The code was inherited from the vendor driver, and it is currently #ifdef'd
out there as well.

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

--- sky2-dev.orig/drivers/net/sky2.c	2007-02-13 15:08:31.000000000 -0800
+++ sky2-dev/drivers/net/sky2.c	2007-02-13 15:13:03.000000000 -0800
@@ -1742,13 +1742,6 @@
 	reg &= ~(GM_GPCR_RX_ENA | GM_GPCR_TX_ENA);
 	gma_write16(hw, port, GM_GP_CTRL, reg);
 
-	if (sky2->flow_status == FC_RX) {
-		/* restore Asymmetric Pause bit */
-		gm_phy_write(hw, port, PHY_MARV_AUNE_ADV,
-			     gm_phy_read(hw, port, PHY_MARV_AUNE_ADV)
-			     | PHY_M_AN_ASP);
-	}
-
 	netif_carrier_off(sky2->netdev);
 	netif_stop_queue(sky2->netdev);
 

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

[PATCH 3/6] sky2: flow control negotiation for Yukon-FE

[Stephen Hemminger]

[-- Attachment #1: sky2-fe-fc.patch --]
[-- Type: text/plain, Size: 2609 bytes --]

The Yukon-FE chip doesn't do gigabit and has a differen PHY internally.
On this chip, phy status register doesn't properly reflect the result
of flow control negotiation. To workaround the problem and avoid having
to have so much chip dependent code; compute the result of flow control
by looking at the local and remote advertised bits.

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

--- sky2-dev.orig/drivers/net/sky2.c	2007-02-14 10:01:41.000000000 -0800
+++ sky2-dev/drivers/net/sky2.c	2007-02-14 13:32:00.000000000 -0800
@@ -1766,10 +1766,10 @@
 {
 	struct sky2_hw *hw = sky2->hw;
 	unsigned port = sky2->port;
-	u16 lpa;
+	u16 advert, lpa;
 
+	advert = gm_phy_read(hw, port, PHY_MARV_AUNE_ADV);
 	lpa = gm_phy_read(hw, port, PHY_MARV_AUNE_LP);
-
 	if (lpa & PHY_M_AN_RF) {
 		printk(KERN_ERR PFX "%s: remote fault", sky2->netdev->name);
 		return -1;
@@ -1784,20 +1784,40 @@
 	sky2->speed = sky2_phy_speed(hw, aux);
 	sky2->duplex = (aux & PHY_M_PS_FULL_DUP) ? DUPLEX_FULL : DUPLEX_HALF;
 
-	/* Pause bits are offset (9..8) */
-	if (hw->chip_id == CHIP_ID_YUKON_XL
-	    || hw->chip_id == CHIP_ID_YUKON_EC_U
-	    || hw->chip_id == CHIP_ID_YUKON_EX)
-		aux >>= 6;
-
-	sky2->flow_status = sky2_flow(aux & PHY_M_PS_RX_P_EN,
-				      aux & PHY_M_PS_TX_P_EN);
+	/* Since the pause result bits seem to in different positions on
+	 * different chips. look at registers.
+	 */
+	if (!sky2_is_copper(hw)) {
+		/* Shift for bits in fiber PHY */
+		advert &= ~(ADVERTISE_PAUSE_CAP|ADVERTISE_PAUSE_ASYM);
+		lpa &= ~(LPA_PAUSE_CAP|LPA_PAUSE_ASYM);
+
+		if (advert & ADVERTISE_1000XPAUSE)
+			advert |= ADVERTISE_PAUSE_CAP;
+		if (advert & ADVERTISE_1000XPSE_ASYM)
+			advert |= ADVERTISE_PAUSE_ASYM;
+		if (lpa & LPA_1000XPAUSE)
+			lpa |= LPA_PAUSE_CAP;
+		if (lpa & LPA_1000XPAUSE_ASYM)
+			lpa |= LPA_PAUSE_ASYM;
+	}
+
+	sky2->flow_status = FC_NONE;
+	if (advert & ADVERTISE_PAUSE_CAP) {
+		if (lpa & LPA_PAUSE_CAP)
+			sky2->flow_status = FC_BOTH;
+		else if (advert & ADVERTISE_PAUSE_ASYM)
+			sky2->flow_status = FC_RX;
+	} else if (advert & ADVERTISE_PAUSE_ASYM) {
+		if ((lpa & LPA_PAUSE_CAP) && (lpa & LPA_PAUSE_ASYM))
+			sky2->flow_status = FC_TX;
+	}
 
 	if (sky2->duplex == DUPLEX_HALF && sky2->speed < SPEED_1000
 	    && !(hw->chip_id == CHIP_ID_YUKON_EC_U || hw->chip_id == CHIP_ID_YUKON_EX))
 		sky2->flow_status = FC_NONE;
 
-	if (aux & PHY_M_PS_RX_P_EN)
+	if (sky2->flow_status & FC_TX)
 		sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_PAUSE_ON);
 	else
 		sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_PAUSE_OFF);

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

[PATCH 4/6] sky2: transmit timeout

[Stephen Hemminger]

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

The transmit timeout code could hang, and it would not clear out
problems if the hardware was stuck.  Change the code to effectively do
a device down/up similar to the suspend/resume code.

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

--- sky2-dev.orig/drivers/net/sky2.c	2007-02-15 11:44:39.000000000 -0800
+++ sky2-dev/drivers/net/sky2.c	2007-02-15 12:47:05.000000000 -0800
@@ -1866,16 +1866,13 @@
 	spin_unlock(&sky2->phy_lock);
 }
 
-
 /* Transmit timeout is only called if we are running, carrier is up
  * and tx queue is full (stopped).
- * Called with netif_tx_lock held.
  */
 static void sky2_tx_timeout(struct net_device *dev)
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 	struct sky2_hw *hw = sky2->hw;
-	u32 imask;
 
 	if (netif_msg_timer(sky2))
 		printk(KERN_ERR PFX "%s: tx timeout\n", dev->name);
@@ -1885,19 +1882,8 @@
 	       sky2_read16(hw, sky2->port == 0 ? STAT_TXA1_RIDX : STAT_TXA2_RIDX),
 	       sky2_read16(hw, Q_ADDR(txqaddr[sky2->port], Q_DONE)));
 
-	imask = sky2_read32(hw, B0_IMSK);	/* block IRQ in hw */
-	sky2_write32(hw, B0_IMSK, 0);
-	sky2_read32(hw, B0_IMSK);
-
-	netif_poll_disable(hw->dev[0]);		/* stop NAPI poll */
-	synchronize_irq(hw->pdev->irq);
-
-	netif_start_queue(dev);			/* don't wakeup during flush */
-	sky2_tx_complete(sky2, sky2->tx_prod);	/* Flush transmit queue */
-
-	sky2_write32(hw, B0_IMSK, imask);
-
-	sky2_phy_reinit(sky2);			/* this clears flow control etc */
+	/* can't restart safely under softirq */
+	schedule_work(&hw->restart_work);
 }
 
 static int sky2_change_mtu(struct net_device *dev, int new_mtu)
@@ -2651,6 +2637,49 @@
 	sky2_write8(hw, STAT_ISR_TIMER_CTRL, TIM_START);
 }
 
+static void sky2_restart(struct work_struct *work)
+{
+	struct sky2_hw *hw = container_of(work, struct sky2_hw, restart_work);
+	struct net_device *dev;
+	int i, err;
+
+	dev_dbg(&hw->pdev->dev, "restarting\n");
+
+	del_timer_sync(&hw->idle_timer);
+
+	rtnl_lock();
+	sky2_write32(hw, B0_IMSK, 0);
+	sky2_read32(hw, B0_IMSK);
+
+	netif_poll_disable(hw->dev[0]);
+
+	for (i = 0; i < hw->ports; i++) {
+		dev = hw->dev[i];
+		if (netif_running(dev))
+			sky2_down(dev);
+	}
+
+	sky2_reset(hw);
+	sky2_write32(hw, B0_IMSK, Y2_IS_BASE);
+	netif_poll_enable(hw->dev[0]);
+
+	for (i = 0; i < hw->ports; i++) {
+		dev = hw->dev[i];
+		if (netif_running(dev)) {
+			err = sky2_up(dev);
+			if (err) {
+				printk(KERN_INFO PFX "%s: could not restart %d\n",
+				       dev->name, err);
+				dev_close(dev);
+			}
+		}
+	}
+
+	sky2_idle_start(hw);
+
+	rtnl_unlock();
+}
+
 static inline u8 sky2_wol_supported(const struct sky2_hw *hw)
 {
 	return sky2_is_copper(hw) ? (WAKE_PHY | WAKE_MAGIC) : 0;
@@ -3613,6 +3642,8 @@
 	}
 
 	setup_timer(&hw->idle_timer, sky2_idle, (unsigned long) hw);
+	INIT_WORK(&hw->restart_work, sky2_restart);
+
 	sky2_idle_start(hw);
 
 	pci_set_drvdata(pdev, hw);
@@ -3649,6 +3680,8 @@
 
 	del_timer_sync(&hw->idle_timer);
 
+	flush_scheduled_work();
+
 	sky2_write32(hw, B0_IMSK, 0);
 	synchronize_irq(hw->pdev->irq);
 
--- sky2-dev.orig/drivers/net/sky2.h	2007-02-15 11:58:51.000000000 -0800
+++ sky2-dev/drivers/net/sky2.h	2007-02-15 11:59:07.000000000 -0800
@@ -1933,6 +1933,7 @@
 	dma_addr_t   	     st_dma;
 
 	struct timer_list    idle_timer;
+	struct work_struct   restart_work;
 	int		     msi;
 	wait_queue_head_t    msi_wait;
 };

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

[PATCH 5/6] sky2: receive error handling improvements

[Stephen Hemminger]

[-- Attachment #1: sky2-rx-error-handle.patch --]
[-- Type: text/plain, Size: 892 bytes --]

Don't drop oversize frame it might be a VLAN (untagged).
Use different counter for fifo overrun vs fifo error.
Print error on fifo overrrun.

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


--- sky2-dev.orig/drivers/net/sky2.c	2007-02-15 14:33:52.000000000 -0800
+++ sky2-dev/drivers/net/sky2.c	2007-02-15 15:00:38.000000000 -0800
@@ -2056,9 +2056,6 @@
 	if (!(status & GMR_FS_RX_OK))
 		goto resubmit;
 
-	if (length > dev->mtu + ETH_HLEN)
-		goto oversize;
-
 	if (length < copybreak)
 		skb = receive_copy(sky2, re, length);
 	else
@@ -2068,14 +2065,10 @@
 
 	return skb;
 
-oversize:
-	++sky2->net_stats.rx_over_errors;
-	goto resubmit;
-
 error:
 	++sky2->net_stats.rx_errors;
 	if (status & GMR_FS_RX_FF_OV) {
-		sky2->net_stats.rx_fifo_errors++;
+		sky2->net_stats.rx_over_errors++;
 		goto resubmit;
 	}
 

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

[PATCH 6/6] sky2: v1.13

[Stephen Hemminger]

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

New version.

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

--- sky2-dev.orig/drivers/net/sky2.c	2007-02-15 15:00:38.000000000 -0800
+++ sky2-dev/drivers/net/sky2.c	2007-02-15 15:00:56.000000000 -0800
@@ -49,7 +49,7 @@
 #include "sky2.h"
 
 #define DRV_NAME		"sky2"
-#define DRV_VERSION		"1.12"
+#define DRV_VERSION		"1.13"
 #define PFX			DRV_NAME " "
 
 /*

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