[PATCH 0/4]: Spidernet transmit patches

[PATCH 0/4]: Spidernet transmit patches

[Linas Vepstas]

The following set of four patches provide some more spidernet fixes.
Most important are a pair of patches to stop the transmit queue when it
is full, and to actually turn off transmit interrupts during NAPI(!)

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>

[PATCH 0/4]: Spidernet transmit patches

[Linas Vepstas]

The following set of four patches provide some more spidernet fixes.
Most important are a pair of patches to stop the transmit queue when it
is full, and to actually turn off transmit interrupts during NAPI(!)

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>

[PATCH 1/4]: Spidernet stop queue when queue is full

[Linas Vepstas]

Subject: [PATCH 1/4]: Spidernet stop queue when queue is full.

This patch adds a call to netif_stop_queue() when there is
no more room for more packets on the transmit queue.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>

----
 drivers/net/spider_net.c |   19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

Index: linux-2.6.18-mm2/drivers/net/spider_net.c
===================================================================
--- linux-2.6.18-mm2.orig/drivers/net/spider_net.c	2006-10-02 12:12:56.000000000 -0500
+++ linux-2.6.18-mm2/drivers/net/spider_net.c	2006-10-02 18:59:43.000000000 -0500
@@ -686,6 +686,7 @@ spider_net_prepare_tx_descr(struct spide
 	/* Chain the bus address, so that the DMA engine finds this descr. */
 	descr->prev->next_descr_addr = descr->bus_addr;
 
+	card->netdev->trans_start = jiffies;
 	return 0;
 }
 
@@ -857,29 +858,23 @@ spider_net_xmit(struct sk_buff *skb, str
 
 	spider_net_release_tx_chain(card, 0);
 
-	if (chain->head->next == chain->tail->prev) {
-		card->netdev_stats.tx_dropped++;
-		result = NETDEV_TX_LOCKED;
-		goto out;
-	}
-
-	if (spider_net_get_descr_status(descr) != SPIDER_NET_DESCR_NOT_IN_USE) {
-		card->netdev_stats.tx_dropped++;
+	if ((chain->head->next == chain->tail->prev) ||
+	   (spider_net_get_descr_status(descr) != SPIDER_NET_DESCR_NOT_IN_USE)) {
 		result = NETDEV_TX_LOCKED;
 		goto out;
 	}
 
 	if (spider_net_prepare_tx_descr(card, skb) != 0) {
-		card->netdev_stats.tx_dropped++;
 		result = NETDEV_TX_BUSY;
 		goto out;
 	}
 
-	result = NETDEV_TX_OK;
 	spider_net_kick_tx_dma(card);
+	return NETDEV_TX_OK;
 
 out:
-	netif_wake_queue(netdev);
+	card->netdev_stats.tx_dropped++;
+	netif_stop_queue(netdev);
 	return result;
 }
 
@@ -898,6 +893,8 @@ spider_net_cleanup_tx_ring(struct spider
 	if ((spider_net_release_tx_chain(card, 0) != 0) &&
 	    (card->netdev->flags & IFF_UP))
 		spider_net_kick_tx_dma(card);
+
+	netif_wake_queue(card->netdev);
 }
 
 /**

[PATCH 2/4]: Spidernet fix register field definitions

[Linas Vepstas]

This patch fixes the names of a few fields in the DMA control 
register. There is no functional change.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>

----

 drivers/net/spider_net.c |    2 +-
 drivers/net/spider_net.h |   14 +++++++++-----
 2 files changed, 10 insertions(+), 6 deletions(-)

Index: linux-2.6.18-mm2/drivers/net/spider_net.c
===================================================================
--- linux-2.6.18-mm2.orig/drivers/net/spider_net.c	2006-10-02 18:59:43.000000000 -0500
+++ linux-2.6.18-mm2/drivers/net/spider_net.c	2006-10-02 19:06:55.000000000 -0500
@@ -1639,7 +1639,7 @@ spider_net_enable_card(struct spider_net
 			     SPIDER_NET_INT2_MASK_VALUE);
 
 	spider_net_write_reg(card, SPIDER_NET_GDTDMACCNTR,
-			     SPIDER_NET_GDTDCEIDIS);
+			     SPIDER_NET_GDTBSTA);
 }
 
 /**
Index: linux-2.6.18-mm2/drivers/net/spider_net.h
===================================================================
--- linux-2.6.18-mm2.orig/drivers/net/spider_net.h	2006-10-02 18:58:11.000000000 -0500
+++ linux-2.6.18-mm2/drivers/net/spider_net.h	2006-10-02 19:06:55.000000000 -0500
@@ -191,7 +191,9 @@ extern char spider_net_driver_name[];
 #define SPIDER_NET_MACMODE_VALUE	0x00000001
 #define SPIDER_NET_BURSTLMT_VALUE	0x00000200 /* about 16 us */
 
-/* 1(0)					enable r/tx dma
+/* DMAC control register GDMACCNTR
+ *
+ * 1(0)				enable r/tx dma
  *  0000000				fixed to 0
  *
  *         000000			fixed to 0
@@ -200,6 +202,7 @@ extern char spider_net_driver_name[];
  *
  *                 000000		fixed to 0
  *                       00		burst alignment: 128 bytes
+ *                       11		burst alignment: 1024 bytes
  *
  *                         00000	fixed to 0
  *                              0	descr writeback size 32 bytes
@@ -210,10 +213,11 @@ extern char spider_net_driver_name[];
 #define SPIDER_NET_DMA_RX_VALUE		0x80000000
 #define SPIDER_NET_DMA_RX_FEND_VALUE	0x00030003
 /* to set TX_DMA_EN */
-#define SPIDER_NET_TX_DMA_EN		0x80000000
-#define SPIDER_NET_GDTDCEIDIS		0x00000300
-#define SPIDER_NET_DMA_TX_VALUE		SPIDER_NET_TX_DMA_EN | \
-					SPIDER_NET_GDTDCEIDIS
+#define SPIDER_NET_TX_DMA_EN           0x80000000
+#define SPIDER_NET_GDTBSTA             0x00000300
+#define SPIDER_NET_GDTDCEIDIS          0x00000002
+#define SPIDER_NET_DMA_TX_VALUE        SPIDER_NET_TX_DMA_EN | \
+                                       SPIDER_NET_GDTBSTA
 #define SPIDER_NET_DMA_TX_FEND_VALUE	0x00030003
 
 /* SPIDER_NET_UA_DESCR_VALUE is OR'ed with the unicast address */

[PATCH 3/4]: Spidernet transmit interupt mitigation

[Linas Vepstas]

For small packets, the transmit interrupt settings were 
accidentally generating too many interrupts. This patch
turns off all transmit-related interrupts when the tx
queue is mostly empty.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: James K Lewis <jklewis@us.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>

----
 drivers/net/spider_net.c |    8 +++++---
 drivers/net/spider_net.h |    7 +++----
 2 files changed, 8 insertions(+), 7 deletions(-)

Index: linux-2.6.18-mm2/drivers/net/spider_net.c
===================================================================
--- linux-2.6.18-mm2.orig/drivers/net/spider_net.c	2006-10-02 19:06:55.000000000 -0500
+++ linux-2.6.18-mm2/drivers/net/spider_net.c	2006-10-02 19:09:38.000000000 -0500
@@ -707,7 +707,9 @@ spider_net_set_low_watermark(struct spid
 		descr = descr->next;
 		cnt++;
 	}
-	if (cnt == 0)
+
+	/* If TX queue is short, don't even bother with interrupts */
+	if (cnt < tx_descriptors/4)
 		return;
 
 	/* Set low-watermark 3/4th's of the way into the queue. */
@@ -716,7 +718,7 @@ spider_net_set_low_watermark(struct spid
 	for (i=0;i<cnt; i++)
 		descr = descr->next;
 
-	/* Set the new watermark, clear the old wtermark */
+	/* Set the new watermark, clear the old watermark */
 	spin_lock_irqsave(&card->tx_chain.lock, flags);
 	descr->dmac_cmd_status |= SPIDER_NET_DESCR_TXDESFLG;
 	if (card->low_watermark && card->low_watermark != descr)
@@ -1639,7 +1641,7 @@ spider_net_enable_card(struct spider_net
 			     SPIDER_NET_INT2_MASK_VALUE);
 
 	spider_net_write_reg(card, SPIDER_NET_GDTDMACCNTR,
-			     SPIDER_NET_GDTBSTA);
+			     SPIDER_NET_GDTBSTA | SPIDER_NET_GDTDCEIDIS);
 }
 
 /**
Index: linux-2.6.18-mm2/drivers/net/spider_net.h
===================================================================
--- linux-2.6.18-mm2.orig/drivers/net/spider_net.h	2006-10-02 19:06:55.000000000 -0500
+++ linux-2.6.18-mm2/drivers/net/spider_net.h	2006-10-02 19:09:38.000000000 -0500
@@ -217,7 +217,8 @@ extern char spider_net_driver_name[];
 #define SPIDER_NET_GDTBSTA             0x00000300
 #define SPIDER_NET_GDTDCEIDIS          0x00000002
 #define SPIDER_NET_DMA_TX_VALUE        SPIDER_NET_TX_DMA_EN | \
-                                       SPIDER_NET_GDTBSTA
+                                       SPIDER_NET_GDTBSTA   | \
+                                       SPIDER_NET_GDTDCEIDIS
 #define SPIDER_NET_DMA_TX_FEND_VALUE	0x00030003
 
 /* SPIDER_NET_UA_DESCR_VALUE is OR'ed with the unicast address */
@@ -326,9 +327,7 @@ enum spider_net_int2_status {
 	SPIDER_NET_GRISPDNGINT
 };
 
-#define SPIDER_NET_TXINT	( (1 << SPIDER_NET_GTTEDINT) | \
-				  (1 << SPIDER_NET_GDTDCEINT) | \
-				  (1 << SPIDER_NET_GDTFDCINT) )
+#define SPIDER_NET_TXINT	( (1 << SPIDER_NET_GDTFDCINT) )
 
 /* we rely on flagged descriptor interrupts*/
 #define SPIDER_NET_RXINT	( (1 << SPIDER_NET_GDAFDCINT) | \

[PATCH 4/4]: Spidernet module parm permissions

[Linas Vepstas]

The module param permsissions should bw read-only, not writable.

Signed-off-by: James K Lewis <jklewis@us.ibm.com>
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>

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

Index: linux-2.6.18-mm2/drivers/net/spider_net.c
===================================================================
--- linux-2.6.18-mm2.orig/drivers/net/spider_net.c	2006-10-02 19:09:38.000000000 -0500
+++ linux-2.6.18-mm2/drivers/net/spider_net.c	2006-10-02 19:14:48.000000000 -0500
@@ -60,8 +60,8 @@ MODULE_VERSION(VERSION);
 static int rx_descriptors = SPIDER_NET_RX_DESCRIPTORS_DEFAULT;
 static int tx_descriptors = SPIDER_NET_TX_DESCRIPTORS_DEFAULT;
 
-module_param(rx_descriptors, int, 0644);
-module_param(tx_descriptors, int, 0644);
+module_param(rx_descriptors, int, 0444);
+module_param(tx_descriptors, int, 0444);
 
 MODULE_PARM_DESC(rx_descriptors, "number of descriptors used " \
 		 "in rx chains");

Re: [PATCH 1/4]: Spidernet stop queue when queue is full

[Arnd Bergmann]

On Tuesday 03 October 2006 22:57, Linas Vepstas wrote:
> +=A0=A0=A0=A0=A0=A0=A0if ((chain->head->next =3D=3D chain->tail->prev) ||
> +=A0=A0=A0=A0=A0=A0=A0 =A0 (spider_net_get_descr_status(descr) !=3D SPIDE=
R_NET_DESCR_NOT_IN_USE)) {
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0result =3D NETDEV_TX_LOCK=
ED;
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0goto out;
> =A0=A0=A0=A0=A0=A0=A0=A0}

=2E..

> =A0out:
> -=A0=A0=A0=A0=A0=A0=A0netif_wake_queue(netdev);
> +=A0=A0=A0=A0=A0=A0=A0card->netdev_stats.tx_dropped++;
> +=A0=A0=A0=A0=A0=A0=A0netif_stop_queue(netdev);
> =A0=A0=A0=A0=A0=A0=A0=A0return result;
> =A0}

Hmm, this looks a little strange to me. I would assume that we should not
stop the queue when the device is locked, but only when it is busy.

I would assume though that the fix is to return NETDEV_TX_BUSY instead
of NETDEV_TX_LOCKED in the case above, while the netif_stop_queue()
is correct here.

	Arnd <><

Re: [PATCH 1/4]: Spidernet stop queue when queue is full

[Arnd Bergmann]

On Tuesday 03 October 2006 22:57, Linas Vepstas wrote:
> +       if ((chain->head->next == chain->tail->prev) ||
> +          (spider_net_get_descr_status(descr) != SPIDER_NET_DESCR_NOT_IN_USE)) {
>                 result = NETDEV_TX_LOCKED;
>                 goto out;
>         }

...

>  out:
> -       netif_wake_queue(netdev);
> +       card->netdev_stats.tx_dropped++;
> +       netif_stop_queue(netdev);
>         return result;
>  }

Hmm, this looks a little strange to me. I would assume that we should not
stop the queue when the device is locked, but only when it is busy.

I would assume though that the fix is to return NETDEV_TX_BUSY instead
of NETDEV_TX_LOCKED in the case above, while the netif_stop_queue()
is correct here.

	Arnd <><

Re: [PATCH 1/4]: Spidernet stop queue when queue is full

[Linas Vepstas]

On Wed, Oct 04, 2006 at 12:19:42AM +0200, Arnd Bergmann wrote:
> On Tuesday 03 October 2006 22:57, Linas Vepstas wrote:
> >                 result = NETDEV_TX_LOCKED;
> 
> Hmm, this looks a little strange to me. 

Right. This was left-over cruft from back when.

I'll fix this and resend the whole series tommorrow or friday, 
I've got a few more minor performnace tweaks, an implementation
of NETIF_F_SG, and another fix or two, etc.

--linas