[PATCH 0/9] sky2: version 1.24

[PATCH 0/9] sky2: version 1.24

[Stephen Hemminger]

This is rework of some of Mike's patches, and additional
improvement in the restart logic.  Patch against current
net-next-2.6.

-- 

[PATCH 1/9] sky2: Avoid rewinding sky2->tx_prod

[Stephen Hemminger]

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

From:	Mike McCormack <mikem@ring3k.org>

Keep sky2->tx_prod consistent since int might be examined by
an softirq poll or restart.

Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/net/sky2.c |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

--- a/drivers/net/sky2.c	2009-08-14 07:58:07.644188726 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:43.806126697 -0700
@@ -989,11 +989,11 @@ static void sky2_prefetch_init(struct sk
 	sky2_read32(hw, Y2_QADDR(qaddr, PREF_UNIT_CTRL));
 }
 
-static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2)
+static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
 {
-	struct sky2_tx_le *le = sky2->tx_le + sky2->tx_prod;
+	struct sky2_tx_le *le = sky2->tx_le + *slot;
 
-	sky2->tx_prod = RING_NEXT(sky2->tx_prod, TX_RING_SIZE);
+	*slot = RING_NEXT(*slot, TX_RING_SIZE);
 	le->ctrl = 0;
 	return le;
 }
@@ -1006,7 +1006,7 @@ static void tx_init(struct sky2_port *sk
 	sky2->tx_tcpsum = 0;
 	sky2->tx_last_mss = 0;
 
-	le = get_tx_le(sky2);
+	le = get_tx_le(sky2, &sky2->tx_prod);
 	le->addr = 0;
 	le->opcode = OP_ADDR64 | HW_OWNER;
 }
@@ -1565,7 +1565,8 @@ static int sky2_xmit_frame(struct sk_buf
 	struct sky2_hw *hw = sky2->hw;
 	struct sky2_tx_le *le = NULL;
 	struct tx_ring_info *re;
-	unsigned i, len, first_slot;
+	unsigned i, len;
+	u16 slot;
 	dma_addr_t mapping;
 	u16 mss;
 	u8 ctrl;
@@ -1579,14 +1580,14 @@ static int sky2_xmit_frame(struct sk_buf
 	if (pci_dma_mapping_error(hw->pdev, mapping))
 		goto mapping_error;
 
-	first_slot = sky2->tx_prod;
+	slot = sky2->tx_prod;
 	if (unlikely(netif_msg_tx_queued(sky2)))
 		printk(KERN_DEBUG "%s: tx queued, slot %u, len %d\n",
-		       dev->name, first_slot, skb->len);
+		       dev->name, slot, skb->len);
 
 	/* Send high bits if needed */
 	if (sizeof(dma_addr_t) > sizeof(u32)) {
-		le = get_tx_le(sky2);
+		le = get_tx_le(sky2, &slot);
 		le->addr = cpu_to_le32(upper_32_bits(mapping));
 		le->opcode = OP_ADDR64 | HW_OWNER;
 	}
@@ -1599,7 +1600,7 @@ static int sky2_xmit_frame(struct sk_buf
 			mss += ETH_HLEN + ip_hdrlen(skb) + tcp_hdrlen(skb);
 
   		if (mss != sky2->tx_last_mss) {
-  			le = get_tx_le(sky2);
+			le = get_tx_le(sky2, &slot);
   			le->addr = cpu_to_le32(mss);
 
 			if (hw->flags & SKY2_HW_NEW_LE)
@@ -1615,7 +1616,7 @@ static int sky2_xmit_frame(struct sk_buf
 	/* Add VLAN tag, can piggyback on LRGLEN or ADDR64 */
 	if (sky2->vlgrp && vlan_tx_tag_present(skb)) {
 		if (!le) {
-			le = get_tx_le(sky2);
+			le = get_tx_le(sky2, &slot);
 			le->addr = 0;
 			le->opcode = OP_VLAN|HW_OWNER;
 		} else
@@ -1644,7 +1645,7 @@ static int sky2_xmit_frame(struct sk_buf
 			if (tcpsum != sky2->tx_tcpsum) {
 				sky2->tx_tcpsum = tcpsum;
 
-				le = get_tx_le(sky2);
+				le = get_tx_le(sky2, &slot);
 				le->addr = cpu_to_le32(tcpsum);
 				le->length = 0;	/* initial checksum value */
 				le->ctrl = 1;	/* one packet */
@@ -1653,7 +1654,7 @@ static int sky2_xmit_frame(struct sk_buf
 		}
 	}
 
-	le = get_tx_le(sky2);
+	le = get_tx_le(sky2, &slot);
 	le->addr = cpu_to_le32((u32) mapping);
 	le->length = cpu_to_le16(len);
 	le->ctrl = ctrl;
@@ -1674,13 +1675,13 @@ static int sky2_xmit_frame(struct sk_buf
 			goto mapping_unwind;
 
 		if (sizeof(dma_addr_t) > sizeof(u32)) {
-			le = get_tx_le(sky2);
+			le = get_tx_le(sky2, &slot);
 			le->addr = cpu_to_le32(upper_32_bits(mapping));
 			le->ctrl = 0;
 			le->opcode = OP_ADDR64 | HW_OWNER;
 		}
 
-		le = get_tx_le(sky2);
+		le = get_tx_le(sky2, &slot);
 		le->addr = cpu_to_le32((u32) mapping);
 		le->length = cpu_to_le16(frag->size);
 		le->ctrl = ctrl;
@@ -1694,6 +1695,8 @@ static int sky2_xmit_frame(struct sk_buf
 
 	le->ctrl |= EOP;
 
+	sky2->tx_prod = slot;
+
 	if (tx_avail(sky2) <= MAX_SKB_TX_LE)
 		netif_stop_queue(dev);
 
@@ -1702,7 +1705,7 @@ static int sky2_xmit_frame(struct sk_buf
 	return NETDEV_TX_OK;
 
 mapping_unwind:
-	for (i = first_slot; i != sky2->tx_prod; i = RING_NEXT(i, TX_RING_SIZE)) {
+	for (i = sky2->tx_prod; i != slot; i = RING_NEXT(i, TX_RING_SIZE)) {
 		le = sky2->tx_le + i;
 		re = sky2->tx_ring + i;
 
@@ -1722,7 +1725,6 @@ mapping_unwind:
 		}
 	}
 
-	sky2->tx_prod = first_slot;
 mapping_error:
 	if (net_ratelimit())
 		dev_warn(&hw->pdev->dev, "%s: tx mapping error\n", dev->name);

-- 

[PATCH 2/9] sky2: Move tx reset functionality to sky2_tx_reset()

[Stephen Hemminger]

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

From:	Mike McCormack <mikem@ring3k.org>

This is pure refactoring.

Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/net/sky2.c |   44 ++++++++++++++++++++++++++------------------
 1 files changed, 26 insertions(+), 18 deletions(-)

--- a/drivers/net/sky2.c	2009-08-14 07:58:43.806126697 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:44.833439109 -0700
@@ -1804,6 +1804,31 @@ static void sky2_tx_clean(struct net_dev
 	netif_tx_unlock_bh(dev);
 }
 
+static void sky2_tx_reset(struct sky2_port* sky2)
+{
+	unsigned port = sky2->port;
+	struct sky2_hw *hw = sky2->hw;
+
+	/* Disable Force Sync bit and Enable Alloc bit */
+	sky2_write8(hw, SK_REG(port, TXA_CTRL),
+		    TXA_DIS_FSYNC | TXA_DIS_ALLOC | TXA_STOP_RC);
+
+	/* Stop Interval Timer and Limit Counter of Tx Arbiter */
+	sky2_write32(hw, SK_REG(port, TXA_ITI_INI), 0L);
+	sky2_write32(hw, SK_REG(port, TXA_LIM_INI), 0L);
+
+	/* Reset the PCI FIFO of the async Tx queue */
+	sky2_write32(hw, Q_ADDR(txqaddr[port], Q_CSR),
+		     BMU_RST_SET | BMU_FIFO_RST);
+
+	/* Reset the Tx prefetch units */
+	sky2_write32(hw, Y2_QADDR(txqaddr[port], PREF_UNIT_CTRL),
+		     PREF_UNIT_RST_SET);
+
+	sky2_write32(hw, RB_ADDR(txqaddr[port], RB_CTRL), RB_RST_SET);
+	sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);
+}
+
 /* Network shutdown */
 static int sky2_down(struct net_device *dev)
 {
@@ -1841,26 +1866,9 @@ static int sky2_down(struct net_device *
 	      && port == 0 && hw->dev[1] && netif_running(hw->dev[1])))
 		sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_SET);
 
-	/* Disable Force Sync bit and Enable Alloc bit */
-	sky2_write8(hw, SK_REG(port, TXA_CTRL),
-		    TXA_DIS_FSYNC | TXA_DIS_ALLOC | TXA_STOP_RC);
-
-	/* Stop Interval Timer and Limit Counter of Tx Arbiter */
-	sky2_write32(hw, SK_REG(port, TXA_ITI_INI), 0L);
-	sky2_write32(hw, SK_REG(port, TXA_LIM_INI), 0L);
-
-	/* Reset the PCI FIFO of the async Tx queue */
-	sky2_write32(hw, Q_ADDR(txqaddr[port], Q_CSR),
-		     BMU_RST_SET | BMU_FIFO_RST);
-
-	/* Reset the Tx prefetch units */
-	sky2_write32(hw, Y2_QADDR(txqaddr[port], PREF_UNIT_CTRL),
-		     PREF_UNIT_RST_SET);
-
-	sky2_write32(hw, RB_ADDR(txqaddr[port], RB_CTRL), RB_RST_SET);
+	sky2_tx_reset(sky2);
 
 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
-	sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);
 
 	/* Force any delayed status interrrupt and NAPI */
 	sky2_write32(hw, STAT_LEV_TIMER_CNT, 0);

-- 

[PATCH 3/9] sky2: Reset tx train after interrupts disabled.

[Stephen Hemminger]

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

From:	Mike McCormack <mikem@ring3k.org>

Reseting the tx chain too soon results in invalid tx queue positions
being delivered in the status queue.  This also makes sure there's no
overlap between the cleanup done by sky2_tx_clean() and
sky2_tx_done().

Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/net/sky2.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/net/sky2.c	2009-08-14 07:58:44.833439109 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:45.736296852 -0700
@@ -1804,11 +1804,8 @@ static void sky2_tx_clean(struct net_dev
 	netif_tx_unlock_bh(dev);
 }
 
-static void sky2_tx_reset(struct sky2_port* sky2)
+static void sky2_tx_reset(struct sky2_hw *hw, unsigned port)
 {
-	unsigned port = sky2->port;
-	struct sky2_hw *hw = sky2->hw;
-
 	/* Disable Force Sync bit and Enable Alloc bit */
 	sky2_write8(hw, SK_REG(port, TXA_CTRL),
 		    TXA_DIS_FSYNC | TXA_DIS_ALLOC | TXA_STOP_RC);
@@ -1866,8 +1863,6 @@ static int sky2_down(struct net_device *
 	      && port == 0 && hw->dev[1] && netif_running(hw->dev[1])))
 		sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_SET);
 
-	sky2_tx_reset(sky2);
-
 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
 
 	/* Force any delayed status interrrupt and NAPI */
@@ -1892,6 +1887,8 @@ static int sky2_down(struct net_device *
 	/* turn off LED's */
 	sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);
 
+	sky2_tx_reset(hw, port);
+
 	sky2_tx_clean(dev);
 	sky2_rx_clean(sky2);
 

-- 

[PATCH 4/9] sky2: hold spinlock around phy_power_down

[Stephen Hemminger]

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

Avoid any possible problems with accessing PHY registers on shutdown.
This is a purely theoretical issue and is not related to any of the
outstanding bug reports. Since receiver and transmitter are already
shutdown and phy interrupts for this device are already disabled, 
there should already be enough protection. Suggested by Mike McCormack.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/sky2.c	2009-08-14 07:58:45.736296852 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:46.664005121 -0700
@@ -1882,7 +1882,9 @@ static int sky2_down(struct net_device *
 	synchronize_irq(hw->pdev->irq);
 	napi_synchronize(&hw->napi);
 
+	spin_lock_bh(&sky2->phy_lock);
 	sky2_phy_power_down(hw, port);
+	spin_unlock_bh(&sky2->phy_lock);
 
 	/* turn off LED's */
 	sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);

-- 

[PATCH 5/9] sky2: hold RTNL when doing suspend/shutdown operations

[Stephen Hemminger]

[-- Attachment #1: sky2-wol-rtnl.patch --]
[-- Type: text/plain, Size: 1197 bytes --]

The suspend and shutdown code plays with shared state. Use consistent
locking, for extra protection. 

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/sky2.c	2009-08-14 07:58:46.664005121 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:47.421479764 -0700
@@ -4631,6 +4631,7 @@ static int sky2_suspend(struct pci_dev *
 	del_timer_sync(&hw->watchdog_timer);
 	cancel_work_sync(&hw->restart_work);
 
+	rtnl_lock();
 	for (i = 0; i < hw->ports; i++) {
 		struct net_device *dev = hw->dev[i];
 		struct sky2_port *sky2 = netdev_priv(dev);
@@ -4648,6 +4649,7 @@ static int sky2_suspend(struct pci_dev *
 	sky2_write32(hw, B0_IMSK, 0);
 	napi_disable(&hw->napi);
 	sky2_power_aux(hw);
+	rtnl_unlock();
 
 	pci_save_state(pdev);
 	pci_enable_wake(pdev, pci_choose_state(pdev, state), wol);
@@ -4717,6 +4719,7 @@ static void sky2_shutdown(struct pci_dev
 	if (!hw)
 		return;
 
+	rtnl_lock();
 	del_timer_sync(&hw->watchdog_timer);
 
 	for (i = 0; i < hw->ports; i++) {
@@ -4731,6 +4734,7 @@ static void sky2_shutdown(struct pci_dev
 
 	if (wol)
 		sky2_power_aux(hw);
+	rtnl_unlock();
 
 	pci_enable_wake(pdev, PCI_D3hot, wol);
 	pci_enable_wake(pdev, PCI_D3cold, wol);

-- 

[PATCH 6/9] sky2: cleanup restart operations

[Stephen Hemminger]

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

This unifies the places that bounce the device (suspend/resume
and restart). And makes the operations have the same semantics
as normal dev_open/dev_stop.

This also avoids setting the multicast addresses twice when
device is brought up.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/net/sky2.c |  120 +++++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 63 deletions(-)

--- a/drivers/net/sky2.c	2009-08-14 07:58:47.421479764 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:48.448220547 -0700
@@ -1498,10 +1498,9 @@ static int sky2_up(struct net_device *de
 	sky2_write32(hw, B0_IMSK, imask);
 	sky2_read32(hw, B0_IMSK);
 
-	sky2_set_multicast(dev);
-
 	if (netif_msg_ifup(sky2))
 		printk(KERN_INFO PFX "%s: enabling interface\n", dev->name);
+
 	return 0;
 
 err_out:
@@ -3076,18 +3075,46 @@ static void sky2_reset(struct sky2_hw *h
 	sky2_write8(hw, STAT_ISR_TIMER_CTRL, TIM_START);
 }
 
+/* Take device down (offline).
+ * Equivalent to doing dev_stop() but this does not
+ * inform upper layers of the transistion.
+ */
+static void sky2_detach(struct net_device *dev)
+{
+	if (netif_running(dev)) {
+		netif_device_detach(dev);	/* stop txq */
+		sky2_down(dev);
+	}
+}
+
+/* Bring device back after doing sky2_detach */
+static int sky2_reattach(struct net_device *dev)
+{
+	int err = 0;
+
+	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);
+		} else {
+			netif_device_attach(dev);
+			sky2_set_multicast(dev);
+		}
+	}
+
+	return err;
+}
+
 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;
+	int i;
 
 	rtnl_lock();
-	for (i = 0; i < hw->ports; i++) {
-		dev = hw->dev[i];
-		if (netif_running(dev))
-			sky2_down(dev);
-	}
+	for (i = 0; i < hw->ports; i++)
+		sky2_detach(hw->dev[i]);
 
 	napi_disable(&hw->napi);
 	sky2_write32(hw, B0_IMSK, 0);
@@ -3095,17 +3122,8 @@ static void sky2_restart(struct work_str
 	sky2_write32(hw, B0_IMSK, Y2_IS_BASE);
 	napi_enable(&hw->napi);
 
-	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);
-			}
-		}
-	}
+	for (i = 0; i < hw->ports; i++)
+		sky2_reattach(hw->dev[i]);
 
 	rtnl_unlock();
 }
@@ -3694,7 +3712,6 @@ static int sky2_set_ringparam(struct net
 			      struct ethtool_ringparam *ering)
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
-	int err = 0;
 
 	if (ering->rx_pending > RX_MAX_PENDING ||
 	    ering->rx_pending < 8 ||
@@ -3702,19 +3719,12 @@ static int sky2_set_ringparam(struct net
 	    ering->tx_pending > TX_RING_SIZE - 1)
 		return -EINVAL;
 
-	if (netif_running(dev))
-		sky2_down(dev);
+	sky2_detach(dev);
 
 	sky2->rx_pending = ering->rx_pending;
 	sky2->tx_pending = ering->tx_pending;
 
-	if (netif_running(dev)) {
-		err = sky2_up(dev);
-		if (err)
-			dev_close(dev);
-	}
-
-	return err;
+	return sky2_reattach(dev);
 }
 
 static int sky2_get_regs_len(struct net_device *dev)
@@ -4636,9 +4646,7 @@ static int sky2_suspend(struct pci_dev *
 		struct net_device *dev = hw->dev[i];
 		struct sky2_port *sky2 = netdev_priv(dev);
 
-		netif_device_detach(dev);
-		if (netif_running(dev))
-			sky2_down(dev);
+		sky2_detach(dev);
 
 		if (sky2->wol)
 			sky2_wol_init(sky2);
@@ -4686,25 +4694,18 @@ static int sky2_resume(struct pci_dev *p
 	sky2_write32(hw, B0_IMSK, Y2_IS_BASE);
 	napi_enable(&hw->napi);
 
+	rtnl_lock();
 	for (i = 0; i < hw->ports; i++) {
-		struct net_device *dev = hw->dev[i];
-
-		netif_device_attach(dev);
-		if (netif_running(dev)) {
-			err = sky2_up(dev);
-			if (err) {
-				printk(KERN_ERR PFX "%s: could not up: %d\n",
-				       dev->name, err);
-				rtnl_lock();
-				dev_close(dev);
-				rtnl_unlock();
-				goto out;
-			}
-		}
+		err = sky2_reattach(hw->dev[i]);
+		if (err)
+			goto out;
 	}
+	rtnl_unlock();
 
 	return 0;
 out:
+	rtnl_unlock();
+
 	dev_err(&pdev->dev, "resume failed (%d)\n", err);
 	pci_disable_device(pdev);
 	return err;

-- 

[PATCH 7/9] sky2: lock less transmit completion

[Stephen Hemminger]

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

Transmit completion can safely run lockless against transmit start.
In the normal case, completion is done from NAPI and only looks
at elements that are at the tail of the ring.  When doing shutdown
or reset, the transmiter should be completely block by NAPI disable
and blocking of transmit queue. 

Based on earlier work by Mike McCormack.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
 drivers/net/sky2.c |  120 +++++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 63 deletions(-)

--- a/drivers/net/sky2.c	2009-08-14 07:58:48.448220547 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:49.410251274 -0700
@@ -1734,8 +1734,12 @@ mapping_error:
 /*
  * Free ring elements from starting at tx_cons until "done"
  *
- * NB: the hardware will tell us about partial completion of multi-part
+ * NB:
+ *  1. The hardware will tell us about partial completion of multi-part
  *     buffers so make sure not to free skb to early.
+ *  2. This may run in parallel start_xmit because the it only
+ *     looks at the tail of the queue of FIFO (tx_cons), not
+ *     the head (tx_prod)
  */
 static void sky2_tx_complete(struct sky2_port *sky2, u16 done)
 {
@@ -1793,16 +1797,6 @@ static void sky2_tx_complete(struct sky2
 		netif_wake_queue(dev);
 }
 
-/* Cleanup all untransmitted buffers, assume transmitter not running */
-static void sky2_tx_clean(struct net_device *dev)
-{
-	struct sky2_port *sky2 = netdev_priv(dev);
-
-	netif_tx_lock_bh(dev);
-	sky2_tx_complete(sky2, sky2->tx_prod);
-	netif_tx_unlock_bh(dev);
-}
-
 static void sky2_tx_reset(struct sky2_hw *hw, unsigned port)
 {
 	/* Disable Force Sync bit and Enable Alloc bit */
@@ -1890,7 +1884,9 @@ static int sky2_down(struct net_device *
 
 	sky2_tx_reset(hw, port);
 
-	sky2_tx_clean(dev);
+	/* Free any pending frames stuck in HW queue */
+	sky2_tx_complete(sky2, sky2->tx_prod);
+
 	sky2_rx_clean(sky2);
 
 	pci_free_consistent(hw->pdev, RX_LE_BYTES,
@@ -2367,11 +2363,8 @@ static inline void sky2_tx_done(struct n
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 
-	if (netif_running(dev)) {
-		netif_tx_lock(dev);
+	if (netif_running(dev))
 		sky2_tx_complete(sky2, last);
-		netif_tx_unlock(dev);
-	}
 }
 
 static inline void sky2_skb_rx(const struct sky2_port *sky2,

-- 

[PATCH 8/9] sky2: fix pause negotiation

[Stephen Hemminger]

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

The sky2 driver combines auto speed negotiation with automatic negotiation
of pause parameters; but the ethtool interface expects them to be
split. This patch allows autonegotiation to be used for speed, but
manually disable flow control.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/sky2.c	2009-08-14 07:58:49.410251274 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:50.471356910 -0700
@@ -321,7 +321,7 @@ static void sky2_phy_init(struct sky2_hw
 	struct sky2_port *sky2 = netdev_priv(hw->dev[port]);
 	u16 ctrl, ct1000, adv, pg, ledctrl, ledover, reg;
 
-	if (sky2->autoneg == AUTONEG_ENABLE &&
+	if ( (sky2->flags & SKY2_FLAG_AUTO_SPEED) &&
 	    !(hw->flags & SKY2_HW_NEWER_PHY)) {
 		u16 ectrl = gm_phy_read(hw, port, PHY_MARV_EXT_CTRL);
 
@@ -363,7 +363,7 @@ static void sky2_phy_init(struct sky2_hw
 			ctrl |= PHY_M_PC_MDI_XMODE(PHY_M_PC_ENA_AUTO);
 
 			/* downshift on PHY 88E1112 and 88E1149 is changed */
-			if (sky2->autoneg == AUTONEG_ENABLE
+			if ( (sky2->flags & SKY2_FLAG_AUTO_SPEED)
 			    && (hw->flags & SKY2_HW_NEWER_PHY)) {
 				/* set downshift counter to 3x and enable downshift */
 				ctrl &= ~PHY_M_PC_DSC_MSK;
@@ -408,7 +408,7 @@ static void sky2_phy_init(struct sky2_hw
 	adv = PHY_AN_CSMA;
 	reg = 0;
 
-	if (sky2->autoneg == AUTONEG_ENABLE) {
+	if (sky2->flags & SKY2_FLAG_AUTO_SPEED) {
 		if (sky2_is_copper(hw)) {
 			if (sky2->advertising & ADVERTISED_1000baseT_Full)
 				ct1000 |= PHY_M_1000C_AFD;
@@ -423,14 +423,11 @@ static void sky2_phy_init(struct sky2_hw
 			if (sky2->advertising & ADVERTISED_10baseT_Half)
 				adv |= PHY_M_AN_10_HD;
 
-			adv |= copper_fc_adv[sky2->flow_mode];
 		} else {	/* special defines for FIBER (88E1040S only) */
 			if (sky2->advertising & ADVERTISED_1000baseT_Full)
 				adv |= PHY_M_AN_1000X_AFD;
 			if (sky2->advertising & ADVERTISED_1000baseT_Half)
 				adv |= PHY_M_AN_1000X_AHD;
-
-			adv |= fiber_fc_adv[sky2->flow_mode];
 		}
 
 		/* Restart Auto-negotiation */
@@ -439,8 +436,8 @@ static void sky2_phy_init(struct sky2_hw
 		/* forced speed/duplex settings */
 		ct1000 = PHY_M_1000C_MSE;
 
-		/* Disable auto update for duplex flow control and speed */
-		reg |= GM_GPCR_AU_ALL_DIS;
+		/* Disable auto update for duplex flow control and duplex */
+		reg |= GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_SPD_DIS;
 
 		switch (sky2->speed) {
 		case SPEED_1000:
@@ -458,8 +455,15 @@ static void sky2_phy_init(struct sky2_hw
 			ctrl |= PHY_CT_DUP_MD;
 		} else if (sky2->speed < SPEED_1000)
 			sky2->flow_mode = FC_NONE;
+	}
 
-
+	if (sky2->flags & SKY2_FLAG_AUTO_PAUSE) {
+		if (sky2_is_copper(hw))
+			adv |= copper_fc_adv[sky2->flow_mode];
+		else
+			adv |= fiber_fc_adv[sky2->flow_mode];
+	} else {
+		reg |= GM_GPCR_AU_FCT_DIS;
  		reg |= gm_fc_disable[sky2->flow_mode];
 
 		/* Forward pause packets to GMAC? */
@@ -594,7 +598,8 @@ static void sky2_phy_init(struct sky2_hw
 		/* no effect on Yukon-XL */
 		gm_phy_write(hw, port, PHY_MARV_LED_CTRL, ledctrl);
 
-		if (sky2->autoneg == AUTONEG_DISABLE || sky2->speed == SPEED_100) {
+		if ( !(sky2->flags & SKY2_FLAG_AUTO_SPEED)
+		     || sky2->speed == SPEED_100) {
 			/* turn on 100 Mbps LED (LED_LINK100) */
 			ledover |= PHY_M_LED_MO_100(MO_LED_ON);
 		}
@@ -605,7 +610,7 @@ static void sky2_phy_init(struct sky2_hw
 	}
 
 	/* Enable phy interrupt on auto-negotiation complete (or link up) */
-	if (sky2->autoneg == AUTONEG_ENABLE)
+	if (sky2->flags & SKY2_FLAG_AUTO_SPEED)
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_IS_AN_COMPL);
 	else
 		gm_phy_write(hw, port, PHY_MARV_INT_MASK, PHY_M_DEF_MSK);
@@ -661,7 +666,9 @@ static void sky2_phy_power_down(struct s
 
 	/* setup General Purpose Control Register */
 	gma_write16(hw, port, GM_GP_CTRL,
-		    GM_GPCR_FL_PASS | GM_GPCR_SPEED_100 | GM_GPCR_AU_ALL_DIS);
+		    GM_GPCR_FL_PASS | GM_GPCR_SPEED_100 |
+		    GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_FCT_DIS |
+		    GM_GPCR_AU_SPD_DIS);
 
 	if (hw->chip_id != CHIP_ID_YUKON_EC) {
 		if (hw->chip_id == CHIP_ID_YUKON_EC_U) {
@@ -1117,7 +1124,8 @@ static void rx_set_checksum(struct sky2_
 
 	sky2_write32(sky2->hw,
 		     Q_ADDR(rxqaddr[sky2->port], Q_CSR),
-		     sky2->rx_csum ? BMU_ENA_RX_CHKSUM : BMU_DIS_RX_CHKSUM);
+		     (sky2->flags & SKY2_FLAG_RX_CHECKSUM)
+		     ? BMU_ENA_RX_CHKSUM : BMU_DIS_RX_CHKSUM);
 }
 
 /*
@@ -2076,7 +2084,7 @@ static void sky2_phy_intr(struct sky2_hw
 		printk(KERN_INFO PFX "%s: phy interrupt status 0x%x 0x%x\n",
 		       sky2->netdev->name, istatus, phystat);
 
-	if (sky2->autoneg == AUTONEG_ENABLE && (istatus & PHY_M_IS_AN_COMPL)) {
+	if (istatus & PHY_M_IS_AN_COMPL) {
 		if (sky2_autoneg_done(sky2, phystat) == 0)
 			sky2_link_up(sky2);
 		goto out;
@@ -2442,7 +2450,7 @@ static int sky2_status_intr(struct sky2_
 
 			/* This chip reports checksum status differently */
 			if (hw->flags & SKY2_HW_NEW_LE) {
-				if (sky2->rx_csum &&
+				if ((sky2->flags & SKY2_FLAG_RX_CHECKSUM) &&
 				    (le->css & (CSS_ISIPV4 | CSS_ISIPV6)) &&
 				    (le->css & CSS_TCPUDPCSOK))
 					skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -2469,7 +2477,7 @@ static int sky2_status_intr(struct sky2_
 			/* fall through */
 #endif
 		case OP_RXCHKS:
-			if (!sky2->rx_csum)
+			if (!(sky2->flags & SKY2_FLAG_RX_CHECKSUM))
 				break;
 
 			/* If this happens then driver assuming wrong format */
@@ -2494,7 +2502,8 @@ static int sky2_status_intr(struct sky2_
 				printk(KERN_NOTICE PFX "%s: hardware receive "
 				       "checksum problem (status = %#x)\n",
 				       dev->name, status);
-				sky2->rx_csum = 0;
+				sky2->flags &= ~SKY2_FLAG_RX_CHECKSUM;
+
 				sky2_write32(sky2->hw,
 					     Q_ADDR(rxqaddr[port], Q_CSR),
 					     BMU_DIS_RX_CHKSUM);
@@ -3195,7 +3204,8 @@ static int sky2_get_settings(struct net_
 	}
 
 	ecmd->advertising = sky2->advertising;
-	ecmd->autoneg = sky2->autoneg;
+	ecmd->autoneg = (sky2->flags & SKY2_FLAG_AUTO_SPEED)
+		? AUTONEG_ENABLE : AUTONEG_DISABLE;
 	ecmd->duplex = sky2->duplex;
 	return 0;
 }
@@ -3207,6 +3217,7 @@ static int sky2_set_settings(struct net_
 	u32 supported = sky2_supported_modes(hw);
 
 	if (ecmd->autoneg == AUTONEG_ENABLE) {
+		sky2->flags |= SKY2_FLAG_AUTO_SPEED;
 		ecmd->advertising = supported;
 		sky2->duplex = -1;
 		sky2->speed = -1;
@@ -3248,9 +3259,9 @@ static int sky2_set_settings(struct net_
 
 		sky2->speed = ecmd->speed;
 		sky2->duplex = ecmd->duplex;
+		sky2->flags &= ~SKY2_FLAG_AUTO_SPEED;
 	}
 
-	sky2->autoneg = ecmd->autoneg;
 	sky2->advertising = ecmd->advertising;
 
 	if (netif_running(dev)) {
@@ -3320,14 +3331,17 @@ static u32 sky2_get_rx_csum(struct net_d
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 
-	return sky2->rx_csum;
+	return !!(sky2->flags & SKY2_FLAG_RX_CHECKSUM);
 }
 
 static int sky2_set_rx_csum(struct net_device *dev, u32 data)
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 
-	sky2->rx_csum = data;
+	if (data)
+		sky2->flags |= SKY2_FLAG_RX_CHECKSUM;
+	else
+		sky2->flags &= ~SKY2_FLAG_RX_CHECKSUM;
 
 	sky2_write32(sky2->hw, Q_ADDR(rxqaddr[sky2->port], Q_CSR),
 		     data ? BMU_ENA_RX_CHKSUM : BMU_DIS_RX_CHKSUM);
@@ -3345,7 +3359,7 @@ static int sky2_nway_reset(struct net_de
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 
-	if (!netif_running(dev) || sky2->autoneg != AUTONEG_ENABLE)
+	if (!netif_running(dev) || !(sky2->flags & SKY2_FLAG_AUTO_SPEED))
 		return -EINVAL;
 
 	sky2_phy_reinit(sky2);
@@ -3585,7 +3599,8 @@ static void sky2_get_pauseparam(struct n
 		ecmd->tx_pause = ecmd->rx_pause = 1;
 	}
 
-	ecmd->autoneg = sky2->autoneg;
+	ecmd->autoneg = (sky2->flags & SKY2_FLAG_AUTO_PAUSE)
+		? AUTONEG_ENABLE : AUTONEG_DISABLE;
 }
 
 static int sky2_set_pauseparam(struct net_device *dev,
@@ -3593,7 +3608,11 @@ static int sky2_set_pauseparam(struct ne
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 
-	sky2->autoneg = ecmd->autoneg;
+	if (ecmd->autoneg == AUTONEG_ENABLE)
+		sky2->flags |= SKY2_FLAG_AUTO_PAUSE;
+	else
+		sky2->flags &= ~SKY2_FLAG_AUTO_PAUSE;
+
 	sky2->flow_mode = sky2_flow(ecmd->rx_pause, ecmd->tx_pause);
 
 	if (netif_running(dev))
@@ -4283,13 +4302,15 @@ static __devinit struct net_device *sky2
 	sky2->msg_enable = netif_msg_init(debug, default_msg);
 
 	/* Auto speed and flow control */
-	sky2->autoneg = AUTONEG_ENABLE;
+	sky2->flags = SKY2_FLAG_AUTO_SPEED | SKY2_FLAG_AUTO_PAUSE;
+	if (hw->chip_id != CHIP_ID_YUKON_XL)
+		sky2->flags |= SKY2_FLAG_RX_CHECKSUM;
+
 	sky2->flow_mode = FC_BOTH;
 
 	sky2->duplex = -1;
 	sky2->speed = -1;
 	sky2->advertising = sky2_supported_modes(hw);
-	sky2->rx_csum = (hw->chip_id != CHIP_ID_YUKON_XL);
 	sky2->wol = wol;
 
 	spin_lock_init(&sky2->phy_lock);
--- a/drivers/net/sky2.h	2009-08-14 07:58:07.357063846 -0700
+++ b/drivers/net/sky2.h	2009-08-14 07:58:50.472100125 -0700
@@ -1583,7 +1583,6 @@ enum {
 };
 
 #define GM_GPCR_SPEED_1000	(GM_GPCR_GIGS_ENA | GM_GPCR_SPEED_100)
-#define GM_GPCR_AU_ALL_DIS	(GM_GPCR_AU_DUP_DIS | GM_GPCR_AU_FCT_DIS|GM_GPCR_AU_SPD_DIS)
 
 /*	GM_TX_CTRL			16 bit r/w	Transmit Control Register */
 enum {
@@ -2042,15 +2041,18 @@ struct sky2_port {
 		u8	fifo_lev;
 	} check;
 
-
 	dma_addr_t	     rx_le_map;
 	dma_addr_t	     tx_le_map;
+
 	u16		     advertising;	/* ADVERTISED_ bits */
-	u16		     speed;	/* SPEED_1000, SPEED_100, ... */
-	u8		     autoneg;	/* AUTONEG_ENABLE, AUTONEG_DISABLE */
-	u8		     duplex;	/* DUPLEX_HALF, DUPLEX_FULL */
-	u8		     rx_csum;
-	u8		     wol;
+	u16		     speed;		/* SPEED_1000, SPEED_100, ... */
+	u8		     wol;		/* WAKE_ bits */
+	u8		     duplex;		/* DUPLEX_HALF, DUPLEX_FULL */
+	u16		     flags;
+#define SKY2_FLAG_RX_CHECKSUM		0x0001
+#define SKY2_FLAG_AUTO_SPEED		0x0002
+#define SKY2_FLAG_AUTO_PAUSE		0x0004
+
  	enum flow_control    flow_mode;
  	enum flow_control    flow_status;
 

-- 

[PATCH 9/9] sky2: version 1.24

[Stephen Hemminger]

[-- Attachment #1: sky2-v1.24.patch --]
[-- Type: text/plain, Size: 343 bytes --]

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/sky2.c	2009-08-14 07:58:50.471356910 -0700
+++ b/drivers/net/sky2.c	2009-08-14 07:58:51.564040271 -0700
@@ -50,7 +50,7 @@
 #include "sky2.h"
 
 #define DRV_NAME		"sky2"
-#define DRV_VERSION		"1.23"
+#define DRV_VERSION		"1.24"
 #define PFX			DRV_NAME " "
 
 /*

-- 

Re: [PATCH 0/9] sky2: version 1.24

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 14 Aug 2009 08:15:11 -0700

> This is rework of some of Mike's patches, and additional
> improvement in the restart logic.

Patches themselves look good.

> Patch against current net-next-2.6.

Ummm... no, it isn't.

I merged net-2.6 into net-next-2.6 yesterday, which brought in various
sky2 fixes.  This caused conflicts which I had to hand merge starting
in patch #6.

For example, in patch #6:

@@ -1498,10 +1498,9 @@ static int sky2_up(struct net_device *de
 	sky2_write32(hw, B0_IMSK, imask);
 	sky2_read32(hw, B0_IMSK);
 
-	sky2_set_multicast(dev);
-
 	if (netif_msg_ifup(sky2))
 		printk(KERN_INFO PFX "%s: enabling interface\n", dev->name);
+
 	return 0;
 
 err_out:

There is a netif_wake_queue() call right before the if (netif_msg_ifup)

Then, in patch #7:

@@ -2367,11 +2363,8 @@ static inline void sky2_tx_done(struct n
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 
-	if (netif_running(dev)) {
-		netif_tx_lock(dev);
+	if (netif_running(dev))
 		sky2_tx_complete(sky2, last);
-		netif_tx_unlock(dev);
-	}
 }
 
 static inline void sky2_skb_rx(const struct sky2_port *sky2,


That if () test has an extra condition "&& !sky2->restarting",
I retained in when applying that patch.

Then, in patch #8:

@@ -2042,15 +2041,18 @@ struct sky2_port {
 		u8	fifo_lev;
 	} check;
 
-
 	dma_addr_t	     rx_le_map;
 	dma_addr_t	     tx_le_map;
+
 	u16		     advertising;	/* ADVERTISED_ bits */
-	u16		     speed;	/* SPEED_1000, SPEED_100, ... */
-	u8		     autoneg;	/* AUTONEG_ENABLE, AUTONEG_DISABLE */
-	u8		     duplex;	/* DUPLEX_HALF, DUPLEX_FULL */
-	u8		     rx_csum;
-	u8		     wol;
+	u16		     speed;		/* SPEED_1000, SPEED_100, ... */
+	u8		     wol;		/* WAKE_ bits */
+	u8		     duplex;		/* DUPLEX_HALF, DUPLEX_FULL */
+	u16		     flags;
+#define SKY2_FLAG_RX_CHECKSUM		0x0001
+#define SKY2_FLAG_AUTO_SPEED		0x0002
+#define SKY2_FLAG_AUTO_PAUSE		0x0004
+
  	enum flow_control    flow_mode;
  	enum flow_control    flow_status;

This is missing the "->restarting" member, I retained it while
applying this patch.

Even if whatever net-next-2.6 tree you patches against didn't have
those fixes, you should have anticipated them and known that I'd
have to end up sorting out the merge mess afterwards.

Anyways I pushed it all out, send me fixup patches if necessary.

[PATCH] sky2: remove restarting workaround flag

[Stephen Hemminger]

The whole restarting flag was introduced by Mike McCormack
and was a temporary duct tape patch around issues with transmits
inflight during restart. The problems it was covering are now
fixed and the code should have been reverted.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/sky2.c	2009-08-14 16:29:56.716322334 -0700
+++ b/drivers/net/sky2.c	2009-08-14 16:30:05.681690475 -0700
@@ -1496,8 +1496,6 @@ static int sky2_up(struct net_device *de
 	sky2_set_vlan_mode(hw, port, sky2->vlgrp != NULL);
 #endif
 
-	sky2->restarting = 0;
-
 	err = sky2_rx_start(sky2);
 	if (err)
 		goto err_out;
@@ -1508,9 +1506,6 @@ static int sky2_up(struct net_device *de
 	sky2_write32(hw, B0_IMSK, imask);
 	sky2_read32(hw, B0_IMSK);
 
-	/* wake queue incase we are restarting */
-	netif_wake_queue(dev);
-
 	if (netif_msg_ifup(sky2))
 		printk(KERN_INFO PFX "%s: enabling interface\n", dev->name);
 
@@ -1545,8 +1540,6 @@ static inline int tx_dist(unsigned tail,
 /* Number of list elements available for next tx */
 static inline int tx_avail(const struct sky2_port *sky2)
 {
-	if (unlikely(sky2->restarting))
-		return 0;
 	return sky2->tx_pending - tx_dist(sky2->tx_cons, sky2->tx_prod);
 }
 
@@ -1850,10 +1843,6 @@ static int sky2_down(struct net_device *
 	if (netif_msg_ifdown(sky2))
 		printk(KERN_INFO PFX "%s: disabling interface\n", dev->name);
 
-	/* explicitly shut off tx incase we're restarting */
-	sky2->restarting = 1;
-	netif_tx_disable(dev);
-
 	/* Force flow control off */
 	sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_PAUSE_OFF);
 
@@ -2382,7 +2371,7 @@ static inline void sky2_tx_done(struct n
 {
 	struct sky2_port *sky2 = netdev_priv(dev);
 
-	if (likely(netif_running(dev) && !sky2->restarting))
+	if (netif_running(dev))
 		sky2_tx_complete(sky2, last);
 }
 
@@ -4327,7 +4316,6 @@ static __devinit struct net_device *sky2
 	spin_lock_init(&sky2->phy_lock);
 	sky2->tx_pending = TX_DEF_PENDING;
 	sky2->rx_pending = RX_DEF_PENDING;
-	sky2->restarting = 0;
 
 	hw->dev[port] = dev;
 
--- a/drivers/net/sky2.h	2009-08-14 16:29:56.728439978 -0700
+++ b/drivers/net/sky2.h	2009-08-14 16:30:08.825275706 -0700
@@ -2053,7 +2053,6 @@ struct sky2_port {
 #define SKY2_FLAG_AUTO_SPEED		0x0002
 #define SKY2_FLAG_AUTO_PAUSE		0x0004
 
-	u8		     restarting;
  	enum flow_control    flow_mode;
  	enum flow_control    flow_status;
 



-- 

Re: [PATCH] sky2: remove restarting workaround flag

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 14 Aug 2009 16:33:17 -0700

> The whole restarting flag was introduced by Mike McCormack
> and was a temporary duct tape patch around issues with transmits
> inflight during restart. The problems it was covering are now
> fixed and the code should have been reverted.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied, thanks.