[PATCH 0/4] b44 driver improvements

[PATCH 0/4] b44 driver improvements

[Stephen Hemminger]

While researching why the wired networking was so slow
on my laptop (not a driver problem); spotted these small
changes to b44 driver. Nothing urgent, maybe 2.6.23
or later material.

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

[PATCH 1/4] b44: timer power saving

[Stephen Hemminger]

[-- Attachment #1: b44-modtimer.patch --]
[-- Type: text/plain, Size: 1090 bytes --]

Make the PHY and statistic timer run on one second boundary
for powersaving.

On resume, the driver should check for link up immediately, to
get online faster (rather than waiting for the next second).

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


---
 drivers/net/b44.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

--- a/drivers/net/b44.c	2007-06-04 12:31:27.000000000 -0700
+++ b/drivers/net/b44.c	2007-06-04 12:31:34.000000000 -0700
@@ -599,8 +599,7 @@ static void b44_timer(unsigned long __op
 
 	spin_unlock_irq(&bp->lock);
 
-	bp->timer.expires = jiffies + HZ;
-	add_timer(&bp->timer);
+	mod_timer(&bp->timer, round_jiffies(jiffies + HZ));
 }
 
 static void b44_tx(struct b44 *bp)
@@ -2348,11 +2347,11 @@ static int b44_resume(struct pci_dev *pd
 	netif_device_attach(bp->dev);
 	spin_unlock_irq(&bp->lock);
 
-	bp->timer.expires = jiffies + HZ;
-	add_timer(&bp->timer);
-
 	b44_enable_ints(bp);
 	netif_wake_queue(dev);
+
+	mod_timer(&bp->timer, jiffies + 1);
+
 	return 0;
 }
 

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

Re: [PATCH 1/4] b44: timer power saving

[Michael Buesch]

On Monday 04 June 2007 22:25:37 Stephen Hemminger wrote:
> Make the PHY and statistic timer run on one second boundary
> for powersaving.
> 
> On resume, the driver should check for link up immediately, to
> get online faster (rather than waiting for the next second).
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> 
> ---
>  drivers/net/b44.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> --- a/drivers/net/b44.c	2007-06-04 12:31:27.000000000 -0700
> +++ b/drivers/net/b44.c	2007-06-04 12:31:34.000000000 -0700
> @@ -599,8 +599,7 @@ static void b44_timer(unsigned long __op
>  
>  	spin_unlock_irq(&bp->lock);
>  
> -	bp->timer.expires = jiffies + HZ;
> -	add_timer(&bp->timer);
> +	mod_timer(&bp->timer, round_jiffies(jiffies + HZ));
>  }
>  
>  static void b44_tx(struct b44 *bp)
> @@ -2348,11 +2347,11 @@ static int b44_resume(struct pci_dev *pd
>  	netif_device_attach(bp->dev);
>  	spin_unlock_irq(&bp->lock);
>  
> -	bp->timer.expires = jiffies + HZ;
> -	add_timer(&bp->timer);
> -
>  	b44_enable_ints(bp);
>  	netif_wake_queue(dev);
> +
> +	mod_timer(&bp->timer, jiffies + 1);

I don't think we need +1, if you need to fire immediately
(on the next tick). The timer core will always fire
timers that are in the past immediately.

-- 
Greetings Michael.

Re: [PATCH 1/4] b44: timer power saving

[Stephen Hemminger]

> I don't think we need +1, if you need to fire immediately
> (on the next tick). The timer core will always fire
> timers that are in the past immediately.

Thanks, but is it worth bothering to change it again?

Re: [PATCH 1/4] b44: timer power saving

[Michael Buesch]

On Wednesday 06 June 2007 23:04:07 Stephen Hemminger wrote:
> 
> > I don't think we need +1, if you need to fire immediately
> > (on the next tick). The timer core will always fire
> > timers that are in the past immediately.
> 
> Thanks, but is it worth bothering to change it again?

No, I don't think so. I just wanted to point that out.

-- 
Greetings Michael.

Re: [PATCH 1/4] b44: timer power saving

[Jeff Garzik]

Stephen Hemminger wrote:
> Make the PHY and statistic timer run on one second boundary
> for powersaving.
> 
> On resume, the driver should check for link up immediately, to
> get online faster (rather than waiting for the next second).
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> 
> ---
>  drivers/net/b44.c |    9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

applied 1-4 to #upstream

[PATCH 2/4] b44: tx bounce sizing.

[Stephen Hemminger]

[-- Attachment #1: b44-tx-bounce-size.patch --]
[-- Type: text/plain, Size: 2122 bytes --]

No need to grap full size MTU buffer for possibly small transmit
bounce buffers.

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

---
 drivers/net/b44.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

--- a/drivers/net/b44.c	2007-06-04 12:46:54.000000000 -0700
+++ b/drivers/net/b44.c	2007-06-04 12:48:21.000000000 -0700
@@ -69,7 +69,6 @@
 #define NEXT_TX(N)		(((N) + 1) & (B44_TX_RING_SIZE - 1))
 
 #define RX_PKT_BUF_SZ		(1536 + bp->rx_offset + 64)
-#define TX_PKT_BUF_SZ		(B44_MAX_MTU + ETH_HLEN + 8)
 
 /* minimum number of free TX descriptors required to wake up TX process */
 #define B44_TX_WAKEUP_THRESH		(B44_TX_RING_SIZE / 4)
@@ -968,7 +967,6 @@ static void b44_tx_timeout(struct net_de
 static int b44_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct b44 *bp = netdev_priv(dev);
-	struct sk_buff *bounce_skb;
 	int rc = NETDEV_TX_OK;
 	dma_addr_t mapping;
 	u32 len, entry, ctrl;
@@ -986,12 +984,13 @@ static int b44_start_xmit(struct sk_buff
 
 	mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
 	if (dma_mapping_error(mapping) || mapping + len > DMA_30BIT_MASK) {
+		struct sk_buff *bounce_skb;
+
 		/* Chip can't handle DMA to/from >1GB, use bounce buffer */
 		if (!dma_mapping_error(mapping))
 			pci_unmap_single(bp->pdev, mapping, len, PCI_DMA_TODEVICE);
 
-		bounce_skb = __dev_alloc_skb(TX_PKT_BUF_SZ,
-					     GFP_ATOMIC|GFP_DMA);
+		bounce_skb = __dev_alloc_skb(len, GFP_ATOMIC | GFP_DMA);
 		if (!bounce_skb)
 			goto err_out;
 
@@ -1000,13 +999,12 @@ static int b44_start_xmit(struct sk_buff
 		if (dma_mapping_error(mapping) || mapping + len > DMA_30BIT_MASK) {
 			if (!dma_mapping_error(mapping))
 				pci_unmap_single(bp->pdev, mapping,
-					 len, PCI_DMA_TODEVICE);
+						 len, PCI_DMA_TODEVICE);
 			dev_kfree_skb_any(bounce_skb);
 			goto err_out;
 		}
 
-		skb_copy_from_linear_data(skb, skb_put(bounce_skb, len),
-					  skb->len);
+		skb_copy_from_linear_data(skb, skb_put(bounce_skb, len), len);
 		dev_kfree_skb_any(skb);
 		skb = bounce_skb;
 	}

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

[PATCH 3/4] b44: packet offset is constant

[Stephen Hemminger]

[-- Attachment #1: b44-rx-offset.patch --]
[-- Type: text/plain, Size: 4360 bytes --]

The receive buffer offset is constant in this driver.

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

---
 drivers/net/b44.c |   29 ++++++++++++-----------------
 drivers/net/b44.h |    2 --
 2 files changed, 12 insertions(+), 19 deletions(-)

Index: lifebook/drivers/net/b44.c
===================================================================
--- lifebook.orig/drivers/net/b44.c	2007-06-04 13:22:12.000000000 -0700
+++ lifebook/drivers/net/b44.c	2007-06-04 13:27:47.000000000 -0700
@@ -15,6 +15,7 @@
 #include <linux/ethtool.h>
 #include <linux/mii.h>
 #include <linux/if_ether.h>
+#include <linux/if_vlan.h>
 #include <linux/etherdevice.h>
 #include <linux/pci.h>
 #include <linux/delay.h>
@@ -68,7 +69,8 @@
 	  (BP)->tx_cons - (BP)->tx_prod - TX_RING_GAP(BP))
 #define NEXT_TX(N)		(((N) + 1) & (B44_TX_RING_SIZE - 1))
 
-#define RX_PKT_BUF_SZ		(1536 + bp->rx_offset + 64)
+#define RX_PKT_OFFSET		30
+#define RX_PKT_BUF_SZ		(1536 + RX_PKT_OFFSET + 64)
 
 /* minimum number of free TX descriptors required to wake up TX process */
 #define B44_TX_WAKEUP_THRESH		(B44_TX_RING_SIZE / 4)
@@ -683,10 +685,9 @@
 	}
 
 	skb->dev = bp->dev;
-	skb_reserve(skb, bp->rx_offset);
+	rh = (struct rx_header *) skb->data;
+	skb_reserve(skb, RX_PKT_OFFSET);
 
-	rh = (struct rx_header *)
-		(skb->data - bp->rx_offset);
 	rh->len = 0;
 	rh->flags = 0;
 
@@ -696,13 +697,13 @@
 	if (src_map != NULL)
 		src_map->skb = NULL;
 
-	ctrl  = (DESC_CTRL_LEN & (RX_PKT_BUF_SZ - bp->rx_offset));
+	ctrl  = (DESC_CTRL_LEN & (RX_PKT_BUF_SZ - RX_PKT_OFFSET));
 	if (dest_idx == (B44_RX_RING_SIZE - 1))
 		ctrl |= DESC_CTRL_EOT;
 
 	dp = &bp->rx_ring[dest_idx];
 	dp->ctrl = cpu_to_le32(ctrl);
-	dp->addr = cpu_to_le32((u32) mapping + bp->rx_offset + bp->dma_offset);
+	dp->addr = cpu_to_le32((u32) mapping + RX_PKT_OFFSET + bp->dma_offset);
 
 	if (bp->flags & B44_FLAG_RX_RING_HACK)
 		b44_sync_dma_desc_for_device(bp->pdev, bp->rx_ring_dma,
@@ -781,7 +782,7 @@
 					    PCI_DMA_FROMDEVICE);
 		rh = (struct rx_header *) skb->data;
 		len = le16_to_cpu(rh->len);
-		if ((len > (RX_PKT_BUF_SZ - bp->rx_offset)) ||
+		if ((len > (RX_PKT_BUF_SZ - RX_PKT_OFFSET)) ||
 		    (rh->flags & cpu_to_le16(RX_FLAG_ERRORS))) {
 		drop_it:
 			b44_recycle_rx(bp, cons, bp->rx_prod);
@@ -813,8 +814,8 @@
 			pci_unmap_single(bp->pdev, map,
 					 skb_size, PCI_DMA_FROMDEVICE);
 			/* Leave out rx_header */
-                	skb_put(skb, len+bp->rx_offset);
-            	        skb_pull(skb,bp->rx_offset);
+                	skb_put(skb, len + RX_PKT_OFFSET);
+            	        skb_pull(skb, RX_PKT_OFFSET);
 		} else {
 			struct sk_buff *copy_skb;
 
@@ -826,7 +827,7 @@
 			skb_reserve(copy_skb, 2);
 			skb_put(copy_skb, len);
 			/* DMA sync done above, copy just the actual packet */
-			skb_copy_from_linear_data_offset(skb, bp->rx_offset,
+			skb_copy_from_linear_data_offset(skb, RX_PKT_OFFSET,
 							 copy_skb->data, len);
 			skb = copy_skb;
 		}
@@ -1393,12 +1394,12 @@
 	bw32(bp, B44_TX_WMARK, 56); /* XXX magic */
 	if (reset_kind == B44_PARTIAL_RESET) {
 		bw32(bp, B44_DMARX_CTRL, (DMARX_CTRL_ENABLE |
-				      (bp->rx_offset << DMARX_CTRL_ROSHIFT)));
+				      (RX_PKT_OFFSET << DMARX_CTRL_ROSHIFT)));
 	} else {
 		bw32(bp, B44_DMATX_CTRL, DMATX_CTRL_ENABLE);
 		bw32(bp, B44_DMATX_ADDR, bp->tx_ring_dma + bp->dma_offset);
 		bw32(bp, B44_DMARX_CTRL, (DMARX_CTRL_ENABLE |
-				      (bp->rx_offset << DMARX_CTRL_ROSHIFT)));
+				      (RX_PKT_OFFSET << DMARX_CTRL_ROSHIFT)));
 		bw32(bp, B44_DMARX_ADDR, bp->rx_ring_dma + bp->dma_offset);
 
 		bw32(bp, B44_DMARX_PTR, bp->rx_pending);
@@ -2090,11 +2091,6 @@
 
 	bp->phy_addr = eeprom[90] & 0x1f;
 
-	/* With this, plus the rx_header prepended to the data by the
-	 * hardware, we'll land the ethernet header on a 2-byte boundary.
-	 */
-	bp->rx_offset = 30;
-
 	bp->imask = IMASK_DEF;
 
 	bp->core_unit = ssb_core_unit(bp);
Index: lifebook/drivers/net/b44.h
===================================================================
--- lifebook.orig/drivers/net/b44.h	2007-06-04 13:18:25.000000000 -0700
+++ lifebook/drivers/net/b44.h	2007-06-04 13:22:15.000000000 -0700
@@ -443,8 +443,6 @@
 #define B44_FLAG_TX_RING_HACK	0x40000000
 #define B44_FLAG_WOL_ENABLE	0x80000000
 
-	u32			rx_offset;
-
 	u32			msg_enable;
 
 	struct timer_list	timer;

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

[PATCH 4/4] b44: use netdev_alloc_skb

[Stephen Hemminger]

[-- Attachment #1: b44-netdev_alloc.patch --]
[-- Type: text/plain, Size: 1313 bytes --]

Use netdev_alloc_skb rather than dev_alloc_skb when allocating
receive buffers.

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

---
 drivers/net/b44.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- a/drivers/net/b44.c	2007-06-04 12:49:11.000000000 -0700
+++ b/drivers/net/b44.c	2007-06-04 12:49:26.000000000 -0700
@@ -653,7 +653,7 @@ static int b44_alloc_rx_skb(struct b44 *
 		src_map = &bp->rx_buffers[src_idx];
 	dest_idx = dest_idx_unmasked & (B44_RX_RING_SIZE - 1);
 	map = &bp->rx_buffers[dest_idx];
-	skb = dev_alloc_skb(RX_PKT_BUF_SZ);
+	skb = netdev_alloc_skb(bp->dev, RX_PKT_BUF_SZ);
 	if (skb == NULL)
 		return -ENOMEM;
 
@@ -669,7 +669,7 @@ static int b44_alloc_rx_skb(struct b44 *
 		if (!dma_mapping_error(mapping))
 			pci_unmap_single(bp->pdev, mapping, RX_PKT_BUF_SZ,PCI_DMA_FROMDEVICE);
 		dev_kfree_skb_any(skb);
-		skb = __dev_alloc_skb(RX_PKT_BUF_SZ,GFP_DMA);
+		skb = __netdev_alloc_skb(bp->dev, RX_PKT_BUF_SZ, GFP_ATOMIC|GFP_DMA);
 		if (skb == NULL)
 			return -ENOMEM;
 		mapping = pci_map_single(bp->pdev, skb->data,
@@ -684,7 +684,6 @@ static int b44_alloc_rx_skb(struct b44 *
 		}
 	}
 
-	skb->dev = bp->dev;
 	rh = (struct rx_header *) skb->data;
 	skb_reserve(skb, RX_PKT_OFFSET);
 

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

Re: [PATCH 0/4] b44 driver improvements

[Jeff Garzik]

On Mon, Jun 04, 2007 at 01:25:36PM -0700, Stephen Hemminger wrote:
> While researching why the wired networking was so slow
> on my laptop (not a driver problem); spotted these small
> changes to b44 driver. Nothing urgent, maybe 2.6.23
> or later material.

Seems sane to me.  I'm travelling and will merge this at the beginning
of next week.  I agree netdev#upstream is appropriate, though the
b44_resume() fix may want upstreaming for 2.6.22.

	Jeff

Re: [PATCH 0/4] b44 driver improvements

[John W. Linville]

On Mon, Jun 04, 2007 at 01:25:36PM -0700, Stephen Hemminger wrote:
> While researching why the wired networking was so slow
> on my laptop (not a driver problem); spotted these small
> changes to b44 driver. Nothing urgent, maybe 2.6.23
> or later material.

ACK to the lot

John
-- 
John W. Linville
linville@tuxdriver.com