Skip to contentSkip to conversation

b44 driver improvements

PATCH In review Stephen Hemminger 4 participants 11 messages Jun 4, 2007 – Jun 13, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

cover b44 driver improvements

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.

Signature · 1 line
Stephen Hemminger <[email protected]>
author
#1 lore raw

PATCH 1/4 b44: timer power saving

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 <[email protected]>
---
 drivers/net/b44.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/net/b44.c +4 -5
@@ -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;
 }
Signature · 1 line
Stephen Hemminger <[email protected]>
#2 lore raw

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

Quoted 26 lines · 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 <[email protected]>
---
 drivers/net/b44.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/net/b44.c +1 -2
@@ -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)

drivers/net/b44.c static int b44_resume(struct pci_dev *pd · line 2353

Quoted 10 lines
drivers/net/b44.c +2 -3
@@ -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.

Signature · 1 line
Greetings Michael.
author
#3 lore raw

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

Quoted 4 lines

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?

#4 lore raw

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

Quoted 6 lines · On Wednesday 06 June 2007 23:04:07 Stephen Hemminger wrote:
Quoted 4 lines

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.

Signature · 1 line
Greetings Michael.
#5 lore raw

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

In reply to Stephen Hemminger
Quoted 6 lines · Stephen Hemminger

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

Quoted 6 lines
Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/b44.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

applied 1-4 to #upstream

author
#6 lore raw

PATCH 2/4 b44: tx bounce sizing.

In reply to Stephen Hemminger

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

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/b44.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/net/b44.c +5 -7
@@ -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;
 	}
Signature · 1 line
Stephen Hemminger <[email protected]>
author
#7 lore raw

PATCH 3/4 b44: packet offset is constant

In reply to Stephen Hemminger

The receive buffer offset is constant in this driver.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/b44.c |   29 ++++++++++++-----------------
 drivers/net/b44.h |    2 --
 2 files changed, 12 insertions(+), 19 deletions(-)
drivers/net/b44.c +13 -17
Index: lifebook/drivers/net/b44.c
===================================================================
@@ -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);
drivers/net/b44.h +0 -2
Index: lifebook/drivers/net/b44.h
===================================================================
@@ -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;
Signature · 1 line
Stephen Hemminger <[email protected]>
author
#8 lore raw

PATCH 4/4 b44: use netdev_alloc_skb

In reply to Stephen Hemminger

Use netdev_alloc_skb rather than dev_alloc_skb when allocating receive buffers.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/b44.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/net/b44.c +2 -3
@@ -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);
Signature · 1 line
Stephen Hemminger <[email protected]>
#9 lore raw
In reply to Stephen Hemminger
Quoted 4 lines · 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.

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

#10 lore raw
In reply to Stephen Hemminger
Quoted 4 lines · 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.

ACK to the lot

John

Signature · 2 lines
John W. Linville
[email protected]