[PATCH 0/8] sky2: driver update

[PATCH 0/8] sky2: driver update

[Stephen Hemminger]

This undoes the prematurely committed sky2 transmit logic changes
and applies Mike's later changes.
Here is a corrected patch stream please apply this to 2.6.31-rc5

-- 

[PATCH 1/8] Revert "sky2: Avoid transmits during sky2_down()"

[Stephen Hemminger]

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

This reverts commit f6caa14aa0b126d4a2933907d1519611b2a8524a.
Better solution to same problem was done in later patches.

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

---
 drivers/net/sky2.c |   14 +-------------
 drivers/net/sky2.h |    1 -
 2 files changed, 1 insertion(+), 14 deletions(-)

--- a/drivers/net/sky2.c	2009-08-06 08:25:15.485025569 -0700
+++ b/drivers/net/sky2.c	2009-08-06 08:25:49.579125976 -0700
@@ -1488,8 +1488,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;
@@ -1502,9 +1500,6 @@ static int sky2_up(struct net_device *de
 
 	sky2_set_multicast(dev);
 
-	/* 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);
 	return 0;
@@ -1538,8 +1533,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);
 }
 
@@ -1825,10 +1818,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);
 
@@ -2370,7 +2359,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)) {
 		netif_tx_lock(dev);
 		sky2_tx_complete(sky2, last);
 		netif_tx_unlock(dev);
@@ -4294,7 +4283,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-06 08:25:15.498381527 -0700
+++ b/drivers/net/sky2.h	2009-08-06 08:25:49.580038902 -0700
@@ -2051,7 +2051,6 @@ struct sky2_port {
 	u8		     duplex;	/* DUPLEX_HALF, DUPLEX_FULL */
 	u8		     rx_csum;
 	u8		     wol;
-	u8		     restarting;
  	enum flow_control    flow_mode;
  	enum flow_control    flow_status;
 

-- 

[PATCH 2/8] sky2: Avoid rewinding sky2->tx_prod

[Stephen Hemminger]

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

Rather than moving the next used transmit ring index forward
then backing it up in the case of DMA mapping failure, cleaner
to use local variable.

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-03 08:43:26.300080050 -0700
+++ b/drivers/net/sky2.c	2009-08-03 09:12:07.929961591 -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 3/8] sky2: Move tx reset functionality to sky2_tx_reset()

[Stephen Hemminger]

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

This is refactoring and should not affect functionality.

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(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 16a359d..a9bb3fa 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1804,6 +1804,31 @@ static void sky2_tx_clean(struct net_device *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 *dev)
 	      && 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);
-- 
1.5.6.5

-- 

[PATCH 4/8] sky2: Reset tx train after interrupts disabled.

[Stephen Hemminger]

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

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(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index a9bb3fa..ac303cb 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1866,8 +1866,6 @@ static int sky2_down(struct net_device *dev)
 	      && 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 +1890,8 @@ static int sky2_down(struct net_device *dev)
 	/* turn off LED's */
 	sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);

+	sky2_tx_reset(sky2);
+
 	sky2_tx_clean(dev);
 	sky2_rx_clean(sky2);

-- 
1.5.6.5

-- 

[PATCH 5/8] sky2: Remove tx locks

[Stephen Hemminger]

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

Should no longer be necessary.

Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
 drivers/net/sky2.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index ac303cb..dbedee4 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1799,9 +1799,7 @@ 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_port* sky2)
@@ -2369,11 +2367,7 @@ static inline void sky2_tx_done(struct net_device *dev, u16 last)
 {
 	struct sky2_port *sky2 = netdev_priv(dev);

-	if (netif_running(dev)) {
-		netif_tx_lock(dev);
-		sky2_tx_complete(sky2, last);
-		netif_tx_unlock(dev);
-	}
+	sky2_tx_complete(sky2, last);
 }

 static inline void sky2_skb_rx(const struct sky2_port *sky2,
-- 
1.5.6.5

-- 

[PATCH 6/8] sky2: Lock tx queue when calling sky2_down locally

[Stephen Hemminger]

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

Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
 drivers/net/sky2.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index dbedee4..972c716 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -3078,10 +3078,13 @@ static void sky2_restart(struct work_struct *work)
 	int i, err;

 	rtnl_lock();
+
 	for (i = 0; i < hw->ports; i++) {
 		dev = hw->dev[i];
-		if (netif_running(dev))
+		if (netif_running(dev)) {
+			netif_tx_lock(dev);
 			sky2_down(dev);
+		}
 	}

 	napi_disable(&hw->napi);
@@ -3099,6 +3102,8 @@ static void sky2_restart(struct work_struct *work)
 				       dev->name, err);
 				dev_close(dev);
 			}
+			else
+				netif_tx_unlock(dev);
 		}
 	}

@@ -3697,8 +3702,10 @@ static int sky2_set_ringparam(struct net_device *dev,
 	    ering->tx_pending > TX_RING_SIZE - 1)
 		return -EINVAL;

-	if (netif_running(dev))
+	if (netif_running(dev)) {
+		netif_tx_lock(dev);
 		sky2_down(dev);
+	}

 	sky2->rx_pending = ering->rx_pending;
 	sky2->tx_pending = ering->tx_pending;
@@ -3707,6 +3714,8 @@ static int sky2_set_ringparam(struct net_device *dev,
 		err = sky2_up(dev);
 		if (err)
 			dev_close(dev);
+		else
+			netif_tx_unlock(dev);
 	}

 	return err;
-- 
1.5.6.5

-- 

[PATCH 7/8] 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-06 08:29:58.560356154 -0700
+++ b/drivers/net/sky2.c	2009-08-06 08:32:06.128478111 -0700
@@ -4635,6 +4635,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);
@@ -4652,6 +4653,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);
@@ -4721,6 +4723,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++) {
@@ -4735,6 +4738,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 8/8] sky2: hold spinlock around phy_power_down

[Stephen Hemminger]

[-- Attachment #1: sky2-phy-lock.patch --]
[-- Type: text/plain, Size: 853 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 <mikem@ring3k.org>

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


--- a/drivers/net/sky2.c	2009-08-06 08:40:56.097359280 -0700
+++ b/drivers/net/sky2.c	2009-08-06 08:41:38.949377326 -0700
@@ -1883,7 +1883,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);

-- 

Re: [PATCH 0/8] sky2: driver update

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 06 Aug 2009 09:12:13 -0700

> This undoes the prematurely committed sky2 transmit logic changes
> and applies Mike's later changes.
> Here is a corrected patch stream please apply this to 2.6.31-rc5

I have to pass on this set, too many problems:

1) You didn't provide explicit "From: " lines for patches where
   you were not the author Stephen.  I could guess from the
   Signoff lines but I'd rather not.

2) I can almost swallow the cleanups if they make the bug fix
   coming up afterwards to be understood more easily.

   But I'm not going to apply patches right now that remove locking
   beause it "seems safe" or "should be ok".  That's absolutely not
   appropriate right now unless it fixes a performance regression
   explicitly listed in the formal regression list.  This is in
   reference to patch #5.

   If this is a prerequisite for patch #6 in some way (that's my
   guess), please make this more explicit, or even better change
   words like "should" (that sounds like "I'm not sure") into
   "is" when the commit message describes how the locks are no
   longer needed at that spot.

   Another idea is to combine #5 and #6 and explain the dependency
   in the commit message.

Also, to be honest, this kind of commit count is way too high at this
late stage of the RC series.  Nothing in here is super critical,
and none of it (to my knowledge) is in the regression list either.

And I informed you in private that the main reason I kept asking you
to be more responsive over the past month or so to sky2 patches and
bug reports was so that patches trickled in gradually, instead of
being dumped all at once way too late in the RC series.

Re: [PATCH 0/8] sky2: driver update

[Stephen Hemminger]

On Sun, 09 Aug 2009 21:09:43 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 06 Aug 2009 09:12:13 -0700
> 
> > This undoes the prematurely committed sky2 transmit logic changes
> > and applies Mike's later changes.
> > Here is a corrected patch stream please apply this to 2.6.31-rc5
> 
> I have to pass on this set, too many problems:
> 
> 1) You didn't provide explicit "From: " lines for patches where
>    you were not the author Stephen.  I could guess from the
>    Signoff lines but I'd rather not.

What is the mechanics for this, does patchwork accept From:
lines in message body?

Re: [PATCH 0/8] sky2: driver update

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 10 Aug 2009 08:48:22 -0700

> On Sun, 09 Aug 2009 21:09:43 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: Stephen Hemminger <shemminger@vyatta.com>
>> Date: Thu, 06 Aug 2009 09:12:13 -0700
>> 
>> > This undoes the prematurely committed sky2 transmit logic changes
>> > and applies Mike's later changes.
>> > Here is a corrected patch stream please apply this to 2.6.31-rc5
>> 
>> I have to pass on this set, too many problems:
>> 
>> 1) You didn't provide explicit "From: " lines for patches where
>>    you were not the author Stephen.  I could guess from the
>>    Signoff lines but I'd rather not.
> 
> What is the mechanics for this, does patchwork accept From:
> lines in message body?

Yes, this is exactly how it works, git format-patch will even
generate these for you :-)