linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] natsemi: NAPI support
@ 2005-12-04 22:47 Mark Brown
       [not found] ` <20051204231209.GA28949@electric-eye.fr.zoreil.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2005-12-04 22:47 UTC (permalink / raw)
  To: Jeff Garzik, Tim Hockin; +Cc: Harald Welte, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 8906 bytes --]

This patch against 2.6.14 converts the natsemi driver to use NAPI.  It
was originally based on one written by Harald Welte, though it has since
been modified quite a bit, most extensively in order to remove the
ability to disable NAPI since none of the other drivers seem to provide
that functionality any more.

Signed-off-by: Mark Brown <broonie@sirena.org.uk>

--- linux-2.6.14/drivers/net/natsemi.c.orig	2005-11-29 19:29:12.000000000 +0000
+++ linux/drivers/net/natsemi.c	2005-12-04 22:13:33.000000000 +0000
@@ -3,6 +3,7 @@
 	Written/copyright 1999-2001 by Donald Becker.
 	Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com)
 	Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com)
+	Portions copyright 2004 Harald Welte <laforge@gnumonks.org>
 
 	This software may be used and distributed according to the terms of
 	the GNU General Public License (GPL), incorporated herein by reference.
@@ -136,7 +137,6 @@
 	TODO:
 	* big endian support with CFG:BEM instead of cpu_to_le32
 	* support for an external PHY
-	* NAPI
 */
 
 #include <linux/config.h>
@@ -160,6 +160,7 @@
 #include <linux/mii.h>
 #include <linux/crc32.h>
 #include <linux/bitops.h>
+#include <linux/prefetch.h>
 #include <asm/processor.h>	/* Processor type for cache alignment. */
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -183,8 +184,6 @@
 				 NETIF_MSG_TX_ERR)
 static int debug = -1;
 
-/* Maximum events (Rx packets, etc.) to handle at each interrupt. */
-static int max_interrupt_work = 20;
 static int mtu;
 
 /* Maximum number of multicast addresses to filter (vs. rx-all-multicast).
@@ -251,14 +250,11 @@
 MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver");
 MODULE_LICENSE("GPL");
 
-module_param(max_interrupt_work, int, 0);
 module_param(mtu, int, 0);
 module_param(debug, int, 0);
 module_param(rx_copybreak, int, 0);
 module_param_array(options, int, NULL, 0);
 module_param_array(full_duplex, int, NULL, 0);
-MODULE_PARM_DESC(max_interrupt_work, 
-	"DP8381x maximum events handled per interrupt");
 MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)");
 MODULE_PARM_DESC(debug, "DP8381x default debug level");
 MODULE_PARM_DESC(rx_copybreak, 
@@ -691,6 +687,8 @@
 	/* Based on MTU+slack. */
 	unsigned int rx_buf_sz;
 	int oom;
+	/* Interrupt status */
+	u32 intr_status;
 	/* Do not touch the nic registers */
 	int hands_off;
 	/* external phy that is used: only valid if dev->if_port != PORT_TP */
@@ -748,7 +746,8 @@
 static int start_tx(struct sk_buff *skb, struct net_device *dev);
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs);
 static void netdev_error(struct net_device *dev, int intr_status);
-static void netdev_rx(struct net_device *dev);
+static int natsemi_poll(struct net_device *dev, int *budget);
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do);
 static void netdev_tx_done(struct net_device *dev);
 static int natsemi_change_mtu(struct net_device *dev, int new_mtu);
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -776,6 +775,18 @@
 	return (void __iomem *) dev->base_addr;
 }
 
+static inline void natsemi_irq_enable(struct net_device *dev)
+{
+	writel(1, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
+static inline void natsemi_irq_disable(struct net_device *dev)
+{
+	writel(0, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
 static void move_int_phy(struct net_device *dev, int addr)
 {
 	struct netdev_private *np = netdev_priv(dev);
@@ -879,6 +890,7 @@
 	spin_lock_init(&np->lock);
 	np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG;
 	np->hands_off = 0;
+	np->intr_status = 0;
 
 	/* Initial port:
 	 * - If the nic was configured to use an external phy and if find_mii
@@ -932,6 +944,9 @@
 	dev->do_ioctl = &netdev_ioctl;
 	dev->tx_timeout = &tx_timeout;
 	dev->watchdog_timeo = TX_TIMEOUT;
+	dev->poll = natsemi_poll;
+	dev->weight = 64;
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	dev->poll_controller = &natsemi_poll_controller;
 #endif
@@ -2158,68 +2173,93 @@
 	}
 }
 
-/* The interrupt handler does all of the Rx thread work and cleans up
-   after the Tx thread. */
+/* The interrupt handler doesn't actually handle interrupts itself, it
+ * schedules a NAPI poll if there is anything to do. */
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs)
 {
 	struct net_device *dev = dev_instance;
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem * ioaddr = ns_ioaddr(dev);
-	int boguscnt = max_interrupt_work;
-	unsigned int handled = 0;
 
 	if (np->hands_off)
 		return IRQ_NONE;
-	do {
-		/* Reading automatically acknowledges all int sources. */
-		u32 intr_status = readl(ioaddr + IntrStatus);
+	
+	/* Reading automatically acknowledges. */
+	np->intr_status = readl(ioaddr + IntrStatus);
 
-		if (netif_msg_intr(np))
-			printk(KERN_DEBUG
-				"%s: Interrupt, status %#08x, mask %#08x.\n",
-				dev->name, intr_status,
-				readl(ioaddr + IntrMask));
+	if (netif_msg_intr(np))
+		printk(KERN_DEBUG
+		       "%s: Interrupt, status %#08x, mask %#08x.\n",
+		       dev->name, np->intr_status,
+		       readl(ioaddr + IntrMask));
 
-		if (intr_status == 0)
-			break;
-		handled = 1;
+	if (!np->intr_status) 
+		return IRQ_NONE;
 
-		if (intr_status &
-		   (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
-		    IntrRxErr | IntrRxOverrun)) {
-			netdev_rx(dev);
-		}
+	prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]);
+
+	if (netif_rx_schedule_prep(dev)) {
+		/* Disable interrupts and register for poll */
+		natsemi_irq_disable(dev);
+		__netif_rx_schedule(dev);
+	}
+	return IRQ_HANDLED;
+}
+
+/* This is the NAPI poll routine.  As well as the standard RX handling
+ * it also handles all other interrupts that the chip might raise.
+ */
+static int natsemi_poll(struct net_device *dev, int *budget)
+{
+	struct netdev_private *np = netdev_priv(dev);
+	void __iomem * ioaddr = ns_ioaddr(dev);
 
-		if (intr_status &
-		   (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
+	int work_to_do = min(*budget, dev->quota);
+	int work_done = 0;
+
+	spin_lock_irq(&np->lock);
+	if (np->hands_off) {
+		spin_unlock_irq(&np->lock);
+		return 0;
+	}
+	spin_unlock_irq(&np->lock);
+
+	do {
+		if (np->intr_status &
+		    (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
 			spin_lock(&np->lock);
 			netdev_tx_done(dev);
 			spin_unlock(&np->lock);
 		}
-
+		
 		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & IntrAbnormalSummary)
-			netdev_error(dev, intr_status);
-
-		if (--boguscnt < 0) {
-			if (netif_msg_intr(np))
-				printk(KERN_WARNING
-					"%s: Too much work at interrupt, "
-					"status=%#08x.\n",
-					dev->name, intr_status);
-			break;
+		if (np->intr_status & IntrAbnormalSummary)
+			netdev_error(dev, np->intr_status);
+		
+		if (np->intr_status &
+		    (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
+		     IntrRxErr | IntrRxOverrun)) {
+			netdev_rx(dev, &work_done, work_to_do);
 		}
-	} while (1);
+		
+		*budget -= work_done;
+		dev->quota -= work_done;
 
-	if (netif_msg_intr(np))
-		printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name);
+		if (work_done >= work_to_do)
+			return 1;
 
-	return IRQ_RETVAL(handled);
+		np->intr_status = readl(ioaddr + IntrStatus);
+	} while (np->intr_status);
+
+	netif_rx_complete(dev);
+	natsemi_irq_enable(dev);
+
+	return 0;
 }
 
 /* This routine is logically part of the interrupt handler, but separated
    for clarity and better register allocation. */
-static void netdev_rx(struct net_device *dev)
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do)
 {
 	struct netdev_private *np = netdev_priv(dev);
 	int entry = np->cur_rx % RX_RING_SIZE;
@@ -2237,6 +2277,12 @@
 				entry, desc_status);
 		if (--boguscnt < 0)
 			break;
+
+		if (*work_done >= work_to_do)
+			break;
+
+		(*work_done)++;
+
 		pkt_len = (desc_status & DescSizeMask) - 4;
 		if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){
 			if (desc_status & DescMore) {
@@ -2293,7 +2339,7 @@
 				np->rx_skbuff[entry] = NULL;
 			}
 			skb->protocol = eth_type_trans(skb, dev);
-			netif_rx(skb);
+			netif_receive_skb(skb);
 			dev->last_rx = jiffies;
 			np->stats.rx_packets++;
 			np->stats.rx_bytes += pkt_len;
@@ -3175,6 +3221,8 @@
 		disable_irq(dev->irq);
 		spin_lock_irq(&np->lock);
 
+		netif_poll_disable(dev);
+
 		writel(0, ioaddr + IntrEnable);
 		np->hands_off = 1;
 		natsemi_stop_rxtx(dev);
@@ -3235,6 +3283,7 @@
 		mod_timer(&np->timer, jiffies + 1*HZ);
 	}
 	netif_device_attach(dev);
+	netif_poll_enable(dev);
 out:
 	rtnl_unlock();
 	return 0;

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] natsemi: NAPI support
       [not found] ` <20051204231209.GA28949@electric-eye.fr.zoreil.com>
@ 2005-12-05 23:23   ` Mark Brown
  2005-12-06  0:19     ` Francois Romieu
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2005-12-05 23:23 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 638 bytes --]

On Mon, Dec 05, 2005 at 12:12:09AM +0100, Francois Romieu wrote:

> -> netif_poll_disable() may sleep while a spinlock is held.

So it can, thanks.

> Btw, the poll/close routines seem racy with each other.

I had been under the impression that the stack was supposed to make sure
that no poll() is running before the driver close() gets called?  I
could well be missing something there, though.  Indeed, now that I think
about it the calls netif_poll_disable() in suspend() ought to mean that
we don't need to look at hands_off inside poll().

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] natsemi: NAPI support
  2005-12-05 23:23   ` Mark Brown
@ 2005-12-06  0:19     ` Francois Romieu
  2005-12-06 21:17       ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Francois Romieu @ 2005-12-06  0:19 UTC (permalink / raw)
  To: broonie; +Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel

Mark Brown <broonie@sirena.org.uk> :
[...]
> I had been under the impression that the stack was supposed to make sure
> that no poll() is running before the driver close() gets called ?

Not exactly. dev_close() waits a bit but it can not be sure that the
device driver will not schedule ->poll() from its irq handler later.

--
Ueimor

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] natsemi: NAPI support
  2005-12-06  0:19     ` Francois Romieu
@ 2005-12-06 21:17       ` Mark Brown
  2005-12-06 21:56         ` Francois Romieu
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2005-12-06 21:17 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

On Tue, Dec 06, 2005 at 01:19:34AM +0100, Francois Romieu wrote:
> Mark Brown <broonie@sirena.org.uk> :

> > I had been under the impression that the stack was supposed to make sure
> > that no poll() is running before the driver close() gets called?

> Not exactly. dev_close() waits a bit but it can not be sure that the
> device driver will not schedule ->poll() from its irq handler later.

Prior to waiting dev_close() clears __LINK_STATE_START which will cause
netif_rx_schedule_prep() to return false.  As far as I can see that
should prevent the interrupt handler scheduling any further poll() calls?

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] natsemi: NAPI support
  2005-12-06 21:17       ` Mark Brown
@ 2005-12-06 21:56         ` Francois Romieu
  2005-12-09 10:48           ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Francois Romieu @ 2005-12-06 21:56 UTC (permalink / raw)
  To: Mark Brown; +Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel

Mark Brown <broonie@sirena.org.uk> :
[...]
> Prior to waiting dev_close() clears __LINK_STATE_START which will cause
> netif_rx_schedule_prep() to return false.
> As far as I can see that should prevent the interrupt handler scheduling
> any further poll() calls?

netif_rx_schedule_prep return netif_running(dev) &&
dev_close              clear_bit(__LINK_STATE_START, &dev->state);
dev_close              smp_mb__after_clear_bit(); /* Commit netif_running(). */
dev_close              while (test_bit(__LINK_STATE_RX_SCHED, &dev->state)) {
dev_close                      /* No hurry. */
dev_close                      msleep(1);
dev_close              }
dev_close              
netif_rx_schedule_prep !test_and_set_bit(__LINK_STATE_RX_SCHED, &dev->state);

--
Ueimor

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] natsemi: NAPI support
  2005-12-06 21:56         ` Francois Romieu
@ 2005-12-09 10:48           ` Mark Brown
  2005-12-12 23:55             ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2005-12-09 10:48 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Jeff Garzik, Tim Hockin, Harald Welte, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Tue, Dec 06, 2005 at 10:56:19PM +0100, Francois Romieu wrote:

> netif_rx_schedule_prep return netif_running(dev) &&
> dev_close              clear_bit(__LINK_STATE_START, &dev->state);

Oh, of course - thanks for bearing wth me.  Will fix that too and
resubmit.

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] natsemi: NAPI support
  2005-12-09 10:48           ` Mark Brown
@ 2005-12-12 23:55             ` Mark Brown
  2005-12-13  0:25               ` Jeff Garzik
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2005-12-12 23:55 UTC (permalink / raw)
  To: Tim Hockin; +Cc: Francois Romieu, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 10414 bytes --]

This patch against 2.6.14 converts the natsemi driver to use NAPI.  It
was originally based on one written by Harald Welte, though it has since
been modified quite a bit, most extensively in order to remove the
ability to disable NAPI since none of the other drivers seem to provide
that functionality any more.

Signed-off-by: Mark Brown <broonie@sirena.org.uk>

---

This revision of the patch:
 - Doesn't sleep with the device spinlock held in suspend().
 - Improves the synchronisation between poll() and the shutdown paths.

--- linux-2.6.14/drivers/net/natsemi.c.orig	2005-11-29 19:29:12.000000000 +0000
+++ linux/drivers/net/natsemi.c	2005-12-11 14:55:48.000000000 +0000
@@ -3,6 +3,7 @@
 	Written/copyright 1999-2001 by Donald Becker.
 	Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com)
 	Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com)
+	Portions copyright 2004 Harald Welte <laforge@gnumonks.org>
 
 	This software may be used and distributed according to the terms of
 	the GNU General Public License (GPL), incorporated herein by reference.
@@ -135,8 +136,6 @@
 
 	TODO:
 	* big endian support with CFG:BEM instead of cpu_to_le32
-	* support for an external PHY
-	* NAPI
 */
 
 #include <linux/config.h>
@@ -160,6 +159,7 @@
 #include <linux/mii.h>
 #include <linux/crc32.h>
 #include <linux/bitops.h>
+#include <linux/prefetch.h>
 #include <asm/processor.h>	/* Processor type for cache alignment. */
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -183,8 +183,6 @@
 				 NETIF_MSG_TX_ERR)
 static int debug = -1;
 
-/* Maximum events (Rx packets, etc.) to handle at each interrupt. */
-static int max_interrupt_work = 20;
 static int mtu;
 
 /* Maximum number of multicast addresses to filter (vs. rx-all-multicast).
@@ -251,14 +249,11 @@ MODULE_AUTHOR("Donald Becker <becker@scy
 MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver");
 MODULE_LICENSE("GPL");
 
-module_param(max_interrupt_work, int, 0);
 module_param(mtu, int, 0);
 module_param(debug, int, 0);
 module_param(rx_copybreak, int, 0);
 module_param_array(options, int, NULL, 0);
 module_param_array(full_duplex, int, NULL, 0);
-MODULE_PARM_DESC(max_interrupt_work, 
-	"DP8381x maximum events handled per interrupt");
 MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)");
 MODULE_PARM_DESC(debug, "DP8381x default debug level");
 MODULE_PARM_DESC(rx_copybreak, 
@@ -691,6 +686,8 @@ struct netdev_private {
 	/* Based on MTU+slack. */
 	unsigned int rx_buf_sz;
 	int oom;
+	/* Interrupt status */
+	u32 intr_status;
 	/* Do not touch the nic registers */
 	int hands_off;
 	/* external phy that is used: only valid if dev->if_port != PORT_TP */
@@ -748,7 +745,8 @@ static void init_registers(struct net_de
 static int start_tx(struct sk_buff *skb, struct net_device *dev);
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs);
 static void netdev_error(struct net_device *dev, int intr_status);
-static void netdev_rx(struct net_device *dev);
+static int natsemi_poll(struct net_device *dev, int *budget);
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do);
 static void netdev_tx_done(struct net_device *dev);
 static int natsemi_change_mtu(struct net_device *dev, int new_mtu);
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -776,6 +774,18 @@ static inline void __iomem *ns_ioaddr(st
 	return (void __iomem *) dev->base_addr;
 }
 
+static inline void natsemi_irq_enable(struct net_device *dev)
+{
+	writel(1, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
+static inline void natsemi_irq_disable(struct net_device *dev)
+{
+	writel(0, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
 static void move_int_phy(struct net_device *dev, int addr)
 {
 	struct netdev_private *np = netdev_priv(dev);
@@ -879,6 +889,7 @@ static int __devinit natsemi_probe1 (str
 	spin_lock_init(&np->lock);
 	np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG;
 	np->hands_off = 0;
+	np->intr_status = 0;
 
 	/* Initial port:
 	 * - If the nic was configured to use an external phy and if find_mii
@@ -932,6 +943,9 @@ static int __devinit natsemi_probe1 (str
 	dev->do_ioctl = &netdev_ioctl;
 	dev->tx_timeout = &tx_timeout;
 	dev->watchdog_timeo = TX_TIMEOUT;
+	dev->poll = natsemi_poll;
+	dev->weight = 64;
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	dev->poll_controller = &natsemi_poll_controller;
 #endif
@@ -2158,68 +2172,92 @@ static void netdev_tx_done(struct net_de
 	}
 }
 
-/* The interrupt handler does all of the Rx thread work and cleans up
-   after the Tx thread. */
+/* The interrupt handler doesn't actually handle interrupts itself, it
+ * schedules a NAPI poll if there is anything to do. */
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs)
 {
 	struct net_device *dev = dev_instance;
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem * ioaddr = ns_ioaddr(dev);
-	int boguscnt = max_interrupt_work;
-	unsigned int handled = 0;
 
 	if (np->hands_off)
 		return IRQ_NONE;
-	do {
-		/* Reading automatically acknowledges all int sources. */
-		u32 intr_status = readl(ioaddr + IntrStatus);
+	
+	/* Reading automatically acknowledges. */
+	np->intr_status = readl(ioaddr + IntrStatus);
 
-		if (netif_msg_intr(np))
-			printk(KERN_DEBUG
-				"%s: Interrupt, status %#08x, mask %#08x.\n",
-				dev->name, intr_status,
-				readl(ioaddr + IntrMask));
+	if (netif_msg_intr(np))
+		printk(KERN_DEBUG
+		       "%s: Interrupt, status %#08x, mask %#08x.\n",
+		       dev->name, np->intr_status,
+		       readl(ioaddr + IntrMask));
 
-		if (intr_status == 0)
-			break;
-		handled = 1;
+	if (!np->intr_status) 
+		return IRQ_NONE;
 
-		if (intr_status &
-		   (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
-		    IntrRxErr | IntrRxOverrun)) {
-			netdev_rx(dev);
-		}
+	prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]);
+
+	if (netif_rx_schedule_prep(dev)) {
+		/* Disable interrupts and register for poll */
+		natsemi_irq_disable(dev);
+		__netif_rx_schedule(dev);
+	}
+	return IRQ_HANDLED;
+}
 
-		if (intr_status &
-		   (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
+/* This is the NAPI poll routine.  As well as the standard RX handling
+ * it also handles all other interrupts that the chip might raise.
+ */
+static int natsemi_poll(struct net_device *dev, int *budget)
+{
+	struct netdev_private *np = netdev_priv(dev);
+	void __iomem * ioaddr = ns_ioaddr(dev);
+
+	int work_to_do = min(*budget, dev->quota);
+	int work_done = 0;
+
+	do {
+		if (np->intr_status &
+		    (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
 			spin_lock(&np->lock);
 			netdev_tx_done(dev);
 			spin_unlock(&np->lock);
 		}
 
 		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & IntrAbnormalSummary)
-			netdev_error(dev, intr_status);
-
-		if (--boguscnt < 0) {
-			if (netif_msg_intr(np))
-				printk(KERN_WARNING
-					"%s: Too much work at interrupt, "
-					"status=%#08x.\n",
-					dev->name, intr_status);
-			break;
+		if (np->intr_status & IntrAbnormalSummary)
+			netdev_error(dev, np->intr_status);
+		
+		if (np->intr_status &
+		    (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
+		     IntrRxErr | IntrRxOverrun)) {
+			netdev_rx(dev, &work_done, work_to_do);
 		}
-	} while (1);
+		
+		*budget -= work_done;
+		dev->quota -= work_done;
 
-	if (netif_msg_intr(np))
-		printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name);
+		if (work_done >= work_to_do)
+			return 1;
+
+		np->intr_status = readl(ioaddr + IntrStatus);
+	} while (np->intr_status);
 
-	return IRQ_RETVAL(handled);
+	netif_rx_complete(dev);
+
+	/* Reenable interrupts providing nothing is trying to shut
+	 * the chip down. */
+	spin_lock(&np->lock);
+	if (!np->hands_off && netif_running(dev))
+		natsemi_irq_enable(dev);
+	spin_unlock(&np->lock);
+
+	return 0;
 }
 
 /* This routine is logically part of the interrupt handler, but separated
    for clarity and better register allocation. */
-static void netdev_rx(struct net_device *dev)
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do)
 {
 	struct netdev_private *np = netdev_priv(dev);
 	int entry = np->cur_rx % RX_RING_SIZE;
@@ -2237,6 +2275,12 @@ static void netdev_rx(struct net_device 
 				entry, desc_status);
 		if (--boguscnt < 0)
 			break;
+
+		if (*work_done >= work_to_do)
+			break;
+
+		(*work_done)++;
+
 		pkt_len = (desc_status & DescSizeMask) - 4;
 		if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){
 			if (desc_status & DescMore) {
@@ -2293,7 +2337,7 @@ static void netdev_rx(struct net_device 
 				np->rx_skbuff[entry] = NULL;
 			}
 			skb->protocol = eth_type_trans(skb, dev);
-			netif_rx(skb);
+			netif_receive_skb(skb);
 			dev->last_rx = jiffies;
 			np->stats.rx_packets++;
 			np->stats.rx_bytes += pkt_len;
@@ -3074,9 +3118,7 @@ static int netdev_close(struct net_devic
 	del_timer_sync(&np->timer);
 	disable_irq(dev->irq);
 	spin_lock_irq(&np->lock);
-	/* Disable interrupts, and flush posted writes */
-	writel(0, ioaddr + IntrEnable);
-	readl(ioaddr + IntrEnable);
+	natsemi_irq_disable(dev);
 	np->hands_off = 1;
 	spin_unlock_irq(&np->lock);
 	enable_irq(dev->irq);
@@ -3158,6 +3200,9 @@ static void __devexit natsemi_remove1 (s
  *	* netdev_timer: timer stopped by natsemi_suspend.
  *	* intr_handler: doesn't acquire the spinlock. suspend calls
  *		disable_irq() to enforce synchronization.
+ *      * natsemi_poll: checks before reenabling interrupts.  suspend
+ *              sets hands_off, disables interrupts and then waits with
+ *              netif_poll_disable().
  *
  * Interrupts must be disabled, otherwise hands_off can cause irq storms.
  */
@@ -3183,6 +3228,8 @@ static int natsemi_suspend (struct pci_d
 		spin_unlock_irq(&np->lock);
 		enable_irq(dev->irq);
 
+		netif_poll_disable(dev);
+
 		/* Update the error counts. */
 		__get_stats(dev);
 
@@ -3235,6 +3282,7 @@ static int natsemi_resume (struct pci_de
 		mod_timer(&np->timer, jiffies + 1*HZ);
 	}
 	netif_device_attach(dev);
+	netif_poll_enable(dev);
 out:
 	rtnl_unlock();
 	return 0;

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] natsemi: NAPI support
  2005-12-12 23:55             ` Mark Brown
@ 2005-12-13  0:25               ` Jeff Garzik
  2005-12-13 22:59                 ` Mark Brown
  2005-12-21 23:48                 ` Resubmit: " Mark Brown
  0 siblings, 2 replies; 12+ messages in thread
From: Jeff Garzik @ 2005-12-13  0:25 UTC (permalink / raw)
  To: Mark Brown; +Cc: Tim Hockin, Francois Romieu, netdev, linux-kernel

Mark Brown wrote:
> This patch against 2.6.14 converts the natsemi driver to use NAPI.  It
> was originally based on one written by Harald Welte, though it has since
> been modified quite a bit, most extensively in order to remove the
> ability to disable NAPI since none of the other drivers seem to provide
> that functionality any more.
> 
> Signed-off-by: Mark Brown <broonie@sirena.org.uk>

Was it updated per the comments you received on the first posting?

	Jeff




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] natsemi: NAPI support
  2005-12-13  0:25               ` Jeff Garzik
@ 2005-12-13 22:59                 ` Mark Brown
  2005-12-21 23:48                 ` Resubmit: " Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Mark Brown @ 2005-12-13 22:59 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Tim Hockin, Francois Romieu, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 255 bytes --]

On Mon, Dec 12, 2005 at 07:25:20PM -0500, Jeff Garzik wrote:

> Was it updated per the comments you received on the first posting?

I think I addressed everything, yes.

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Resubmit: [PATCH] natsemi: NAPI support
  2005-12-13  0:25               ` Jeff Garzik
  2005-12-13 22:59                 ` Mark Brown
@ 2005-12-21 23:48                 ` Mark Brown
  2006-01-04  7:32                   ` Mark Brown
  1 sibling, 1 reply; 12+ messages in thread
From: Mark Brown @ 2005-12-21 23:48 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Tim Hockin, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 10477 bytes --]

This patch against 2.6.14 converts the natsemi driver to use NAPI.  It
was originally based on one written by Harald Welte, though it has since
been modified quite a bit, most extensively in order to remove the
ability to disable NAPI since none of the other drivers seem to provide
that functionality any more.

Signed-off-by: Mark Brown <broonie@sirena.org.uk>

---

This revision of the patch:
 - Doesn't sleep with the device spinlock held in suspend().
 - Improves the synchronisation between poll() and the shutdown paths.

and should address all the issues people previously raised.

--- linux-2.6.14/drivers/net/natsemi.c.orig	2005-11-29 19:29:12.000000000 +0000
+++ linux/drivers/net/natsemi.c	2005-12-11 14:55:48.000000000 +0000
@@ -3,6 +3,7 @@
 	Written/copyright 1999-2001 by Donald Becker.
 	Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com)
 	Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com)
+	Portions copyright 2004 Harald Welte <laforge@gnumonks.org>
 
 	This software may be used and distributed according to the terms of
 	the GNU General Public License (GPL), incorporated herein by reference.
@@ -135,8 +136,6 @@
 
 	TODO:
 	* big endian support with CFG:BEM instead of cpu_to_le32
-	* support for an external PHY
-	* NAPI
 */
 
 #include <linux/config.h>
@@ -160,6 +159,7 @@
 #include <linux/mii.h>
 #include <linux/crc32.h>
 #include <linux/bitops.h>
+#include <linux/prefetch.h>
 #include <asm/processor.h>	/* Processor type for cache alignment. */
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -183,8 +183,6 @@
 				 NETIF_MSG_TX_ERR)
 static int debug = -1;
 
-/* Maximum events (Rx packets, etc.) to handle at each interrupt. */
-static int max_interrupt_work = 20;
 static int mtu;
 
 /* Maximum number of multicast addresses to filter (vs. rx-all-multicast).
@@ -251,14 +249,11 @@ MODULE_AUTHOR("Donald Becker <becker@scy
 MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver");
 MODULE_LICENSE("GPL");
 
-module_param(max_interrupt_work, int, 0);
 module_param(mtu, int, 0);
 module_param(debug, int, 0);
 module_param(rx_copybreak, int, 0);
 module_param_array(options, int, NULL, 0);
 module_param_array(full_duplex, int, NULL, 0);
-MODULE_PARM_DESC(max_interrupt_work, 
-	"DP8381x maximum events handled per interrupt");
 MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)");
 MODULE_PARM_DESC(debug, "DP8381x default debug level");
 MODULE_PARM_DESC(rx_copybreak, 
@@ -691,6 +686,8 @@ struct netdev_private {
 	/* Based on MTU+slack. */
 	unsigned int rx_buf_sz;
 	int oom;
+	/* Interrupt status */
+	u32 intr_status;
 	/* Do not touch the nic registers */
 	int hands_off;
 	/* external phy that is used: only valid if dev->if_port != PORT_TP */
@@ -748,7 +745,8 @@ static void init_registers(struct net_de
 static int start_tx(struct sk_buff *skb, struct net_device *dev);
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs);
 static void netdev_error(struct net_device *dev, int intr_status);
-static void netdev_rx(struct net_device *dev);
+static int natsemi_poll(struct net_device *dev, int *budget);
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do);
 static void netdev_tx_done(struct net_device *dev);
 static int natsemi_change_mtu(struct net_device *dev, int new_mtu);
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -776,6 +774,18 @@ static inline void __iomem *ns_ioaddr(st
 	return (void __iomem *) dev->base_addr;
 }
 
+static inline void natsemi_irq_enable(struct net_device *dev)
+{
+	writel(1, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
+static inline void natsemi_irq_disable(struct net_device *dev)
+{
+	writel(0, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
 static void move_int_phy(struct net_device *dev, int addr)
 {
 	struct netdev_private *np = netdev_priv(dev);
@@ -879,6 +889,7 @@ static int __devinit natsemi_probe1 (str
 	spin_lock_init(&np->lock);
 	np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG;
 	np->hands_off = 0;
+	np->intr_status = 0;
 
 	/* Initial port:
 	 * - If the nic was configured to use an external phy and if find_mii
@@ -932,6 +943,9 @@ static int __devinit natsemi_probe1 (str
 	dev->do_ioctl = &netdev_ioctl;
 	dev->tx_timeout = &tx_timeout;
 	dev->watchdog_timeo = TX_TIMEOUT;
+	dev->poll = natsemi_poll;
+	dev->weight = 64;
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	dev->poll_controller = &natsemi_poll_controller;
 #endif
@@ -2158,68 +2172,92 @@ static void netdev_tx_done(struct net_de
 	}
 }
 
-/* The interrupt handler does all of the Rx thread work and cleans up
-   after the Tx thread. */
+/* The interrupt handler doesn't actually handle interrupts itself, it
+ * schedules a NAPI poll if there is anything to do. */
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs)
 {
 	struct net_device *dev = dev_instance;
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem * ioaddr = ns_ioaddr(dev);
-	int boguscnt = max_interrupt_work;
-	unsigned int handled = 0;
 
 	if (np->hands_off)
 		return IRQ_NONE;
-	do {
-		/* Reading automatically acknowledges all int sources. */
-		u32 intr_status = readl(ioaddr + IntrStatus);
+	
+	/* Reading automatically acknowledges. */
+	np->intr_status = readl(ioaddr + IntrStatus);
 
-		if (netif_msg_intr(np))
-			printk(KERN_DEBUG
-				"%s: Interrupt, status %#08x, mask %#08x.\n",
-				dev->name, intr_status,
-				readl(ioaddr + IntrMask));
+	if (netif_msg_intr(np))
+		printk(KERN_DEBUG
+		       "%s: Interrupt, status %#08x, mask %#08x.\n",
+		       dev->name, np->intr_status,
+		       readl(ioaddr + IntrMask));
 
-		if (intr_status == 0)
-			break;
-		handled = 1;
+	if (!np->intr_status) 
+		return IRQ_NONE;
 
-		if (intr_status &
-		   (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
-		    IntrRxErr | IntrRxOverrun)) {
-			netdev_rx(dev);
-		}
+	prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]);
+
+	if (netif_rx_schedule_prep(dev)) {
+		/* Disable interrupts and register for poll */
+		natsemi_irq_disable(dev);
+		__netif_rx_schedule(dev);
+	}
+	return IRQ_HANDLED;
+}
 
-		if (intr_status &
-		   (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
+/* This is the NAPI poll routine.  As well as the standard RX handling
+ * it also handles all other interrupts that the chip might raise.
+ */
+static int natsemi_poll(struct net_device *dev, int *budget)
+{
+	struct netdev_private *np = netdev_priv(dev);
+	void __iomem * ioaddr = ns_ioaddr(dev);
+
+	int work_to_do = min(*budget, dev->quota);
+	int work_done = 0;
+
+	do {
+		if (np->intr_status &
+		    (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
 			spin_lock(&np->lock);
 			netdev_tx_done(dev);
 			spin_unlock(&np->lock);
 		}
 
 		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & IntrAbnormalSummary)
-			netdev_error(dev, intr_status);
-
-		if (--boguscnt < 0) {
-			if (netif_msg_intr(np))
-				printk(KERN_WARNING
-					"%s: Too much work at interrupt, "
-					"status=%#08x.\n",
-					dev->name, intr_status);
-			break;
+		if (np->intr_status & IntrAbnormalSummary)
+			netdev_error(dev, np->intr_status);
+		
+		if (np->intr_status &
+		    (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
+		     IntrRxErr | IntrRxOverrun)) {
+			netdev_rx(dev, &work_done, work_to_do);
 		}
-	} while (1);
+		
+		*budget -= work_done;
+		dev->quota -= work_done;
 
-	if (netif_msg_intr(np))
-		printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name);
+		if (work_done >= work_to_do)
+			return 1;
+
+		np->intr_status = readl(ioaddr + IntrStatus);
+	} while (np->intr_status);
 
-	return IRQ_RETVAL(handled);
+	netif_rx_complete(dev);
+
+	/* Reenable interrupts providing nothing is trying to shut
+	 * the chip down. */
+	spin_lock(&np->lock);
+	if (!np->hands_off && netif_running(dev))
+		natsemi_irq_enable(dev);
+	spin_unlock(&np->lock);
+
+	return 0;
 }
 
 /* This routine is logically part of the interrupt handler, but separated
    for clarity and better register allocation. */
-static void netdev_rx(struct net_device *dev)
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do)
 {
 	struct netdev_private *np = netdev_priv(dev);
 	int entry = np->cur_rx % RX_RING_SIZE;
@@ -2237,6 +2275,12 @@ static void netdev_rx(struct net_device 
 				entry, desc_status);
 		if (--boguscnt < 0)
 			break;
+
+		if (*work_done >= work_to_do)
+			break;
+
+		(*work_done)++;
+
 		pkt_len = (desc_status & DescSizeMask) - 4;
 		if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){
 			if (desc_status & DescMore) {
@@ -2293,7 +2337,7 @@ static void netdev_rx(struct net_device 
 				np->rx_skbuff[entry] = NULL;
 			}
 			skb->protocol = eth_type_trans(skb, dev);
-			netif_rx(skb);
+			netif_receive_skb(skb);
 			dev->last_rx = jiffies;
 			np->stats.rx_packets++;
 			np->stats.rx_bytes += pkt_len;
@@ -3074,9 +3118,7 @@ static int netdev_close(struct net_devic
 	del_timer_sync(&np->timer);
 	disable_irq(dev->irq);
 	spin_lock_irq(&np->lock);
-	/* Disable interrupts, and flush posted writes */
-	writel(0, ioaddr + IntrEnable);
-	readl(ioaddr + IntrEnable);
+	natsemi_irq_disable(dev);
 	np->hands_off = 1;
 	spin_unlock_irq(&np->lock);
 	enable_irq(dev->irq);
@@ -3158,6 +3200,9 @@ static void __devexit natsemi_remove1 (s
  *	* netdev_timer: timer stopped by natsemi_suspend.
  *	* intr_handler: doesn't acquire the spinlock. suspend calls
  *		disable_irq() to enforce synchronization.
+ *      * natsemi_poll: checks before reenabling interrupts.  suspend
+ *              sets hands_off, disables interrupts and then waits with
+ *              netif_poll_disable().
  *
  * Interrupts must be disabled, otherwise hands_off can cause irq storms.
  */
@@ -3183,6 +3228,8 @@ static int natsemi_suspend (struct pci_d
 		spin_unlock_irq(&np->lock);
 		enable_irq(dev->irq);
 
+		netif_poll_disable(dev);
+
 		/* Update the error counts. */
 		__get_stats(dev);
 
@@ -3235,6 +3282,7 @@ static int natsemi_resume (struct pci_de
 		mod_timer(&np->timer, jiffies + 1*HZ);
 	}
 	netif_device_attach(dev);
+	netif_poll_enable(dev);
 out:
 	rtnl_unlock();
 	return 0;

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Resubmit: [PATCH] natsemi: NAPI support
  2005-12-21 23:48                 ` Resubmit: " Mark Brown
@ 2006-01-04  7:32                   ` Mark Brown
  2006-01-04 13:40                     ` Harald Welte
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2006-01-04  7:32 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Tim Hockin, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 10414 bytes --]

This patch against 2.6.14 converts the natsemi driver to use NAPI.  It
was originally based on one written by Harald Welte, though it has since
been modified quite a bit, most extensively in order to remove the
ability to disable NAPI since none of the other drivers seem to provide
that functionality any more.

Signed-off-by: Mark Brown <broonie@sirena.org.uk>

---

This revision of the patch:
 - Doesn't sleep with the device spinlock held in suspend().
 - Improves the synchronisation between poll() and the shutdown paths.

--- linux-2.6.14/drivers/net/natsemi.c.orig	2005-11-29 19:29:12.000000000 +0000
+++ linux/drivers/net/natsemi.c	2005-12-11 14:55:48.000000000 +0000
@@ -3,6 +3,7 @@
 	Written/copyright 1999-2001 by Donald Becker.
 	Portions copyright (c) 2001,2002 Sun Microsystems (thockin@sun.com)
 	Portions copyright 2001,2002 Manfred Spraul (manfred@colorfullife.com)
+	Portions copyright 2004 Harald Welte <laforge@gnumonks.org>
 
 	This software may be used and distributed according to the terms of
 	the GNU General Public License (GPL), incorporated herein by reference.
@@ -135,8 +136,6 @@
 
 	TODO:
 	* big endian support with CFG:BEM instead of cpu_to_le32
-	* support for an external PHY
-	* NAPI
 */
 
 #include <linux/config.h>
@@ -160,6 +159,7 @@
 #include <linux/mii.h>
 #include <linux/crc32.h>
 #include <linux/bitops.h>
+#include <linux/prefetch.h>
 #include <asm/processor.h>	/* Processor type for cache alignment. */
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -183,8 +183,6 @@
 				 NETIF_MSG_TX_ERR)
 static int debug = -1;
 
-/* Maximum events (Rx packets, etc.) to handle at each interrupt. */
-static int max_interrupt_work = 20;
 static int mtu;
 
 /* Maximum number of multicast addresses to filter (vs. rx-all-multicast).
@@ -251,14 +249,11 @@ MODULE_AUTHOR("Donald Becker <becker@scy
 MODULE_DESCRIPTION("National Semiconductor DP8381x series PCI Ethernet driver");
 MODULE_LICENSE("GPL");
 
-module_param(max_interrupt_work, int, 0);
 module_param(mtu, int, 0);
 module_param(debug, int, 0);
 module_param(rx_copybreak, int, 0);
 module_param_array(options, int, NULL, 0);
 module_param_array(full_duplex, int, NULL, 0);
-MODULE_PARM_DESC(max_interrupt_work, 
-	"DP8381x maximum events handled per interrupt");
 MODULE_PARM_DESC(mtu, "DP8381x MTU (all boards)");
 MODULE_PARM_DESC(debug, "DP8381x default debug level");
 MODULE_PARM_DESC(rx_copybreak, 
@@ -691,6 +686,8 @@ struct netdev_private {
 	/* Based on MTU+slack. */
 	unsigned int rx_buf_sz;
 	int oom;
+	/* Interrupt status */
+	u32 intr_status;
 	/* Do not touch the nic registers */
 	int hands_off;
 	/* external phy that is used: only valid if dev->if_port != PORT_TP */
@@ -748,7 +745,8 @@ static void init_registers(struct net_de
 static int start_tx(struct sk_buff *skb, struct net_device *dev);
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *regs);
 static void netdev_error(struct net_device *dev, int intr_status);
-static void netdev_rx(struct net_device *dev);
+static int natsemi_poll(struct net_device *dev, int *budget);
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do);
 static void netdev_tx_done(struct net_device *dev);
 static int natsemi_change_mtu(struct net_device *dev, int new_mtu);
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -776,6 +774,18 @@ static inline void __iomem *ns_ioaddr(st
 	return (void __iomem *) dev->base_addr;
 }
 
+static inline void natsemi_irq_enable(struct net_device *dev)
+{
+	writel(1, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
+static inline void natsemi_irq_disable(struct net_device *dev)
+{
+	writel(0, ns_ioaddr(dev) + IntrEnable);
+	readl(ns_ioaddr(dev) + IntrEnable);
+}
+
 static void move_int_phy(struct net_device *dev, int addr)
 {
 	struct netdev_private *np = netdev_priv(dev);
@@ -879,6 +889,7 @@ static int __devinit natsemi_probe1 (str
 	spin_lock_init(&np->lock);
 	np->msg_enable = (debug >= 0) ? (1<<debug)-1 : NATSEMI_DEF_MSG;
 	np->hands_off = 0;
+	np->intr_status = 0;
 
 	/* Initial port:
 	 * - If the nic was configured to use an external phy and if find_mii
@@ -932,6 +943,9 @@ static int __devinit natsemi_probe1 (str
 	dev->do_ioctl = &netdev_ioctl;
 	dev->tx_timeout = &tx_timeout;
 	dev->watchdog_timeo = TX_TIMEOUT;
+	dev->poll = natsemi_poll;
+	dev->weight = 64;
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	dev->poll_controller = &natsemi_poll_controller;
 #endif
@@ -2158,68 +2172,92 @@ static void netdev_tx_done(struct net_de
 	}
 }
 
-/* The interrupt handler does all of the Rx thread work and cleans up
-   after the Tx thread. */
+/* The interrupt handler doesn't actually handle interrupts itself, it
+ * schedules a NAPI poll if there is anything to do. */
 static irqreturn_t intr_handler(int irq, void *dev_instance, struct pt_regs *rgs)
 {
 	struct net_device *dev = dev_instance;
 	struct netdev_private *np = netdev_priv(dev);
 	void __iomem * ioaddr = ns_ioaddr(dev);
-	int boguscnt = max_interrupt_work;
-	unsigned int handled = 0;
 
 	if (np->hands_off)
 		return IRQ_NONE;
-	do {
-		/* Reading automatically acknowledges all int sources. */
-		u32 intr_status = readl(ioaddr + IntrStatus);
+	
+	/* Reading automatically acknowledges. */
+	np->intr_status = readl(ioaddr + IntrStatus);
 
-		if (netif_msg_intr(np))
-			printk(KERN_DEBUG
-				"%s: Interrupt, status %#08x, mask %#08x.\n",
-				dev->name, intr_status,
-				readl(ioaddr + IntrMask));
+	if (netif_msg_intr(np))
+		printk(KERN_DEBUG
+		       "%s: Interrupt, status %#08x, mask %#08x.\n",
+		       dev->name, np->intr_status,
+		       readl(ioaddr + IntrMask));
 
-		if (intr_status == 0)
-			break;
-		handled = 1;
+	if (!np->intr_status) 
+		return IRQ_NONE;
 
-		if (intr_status &
-		   (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
-		    IntrRxErr | IntrRxOverrun)) {
-			netdev_rx(dev);
-		}
+	prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]);
+
+	if (netif_rx_schedule_prep(dev)) {
+		/* Disable interrupts and register for poll */
+		natsemi_irq_disable(dev);
+		__netif_rx_schedule(dev);
+	}
+	return IRQ_HANDLED;
+}
 
-		if (intr_status &
-		   (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
+/* This is the NAPI poll routine.  As well as the standard RX handling
+ * it also handles all other interrupts that the chip might raise.
+ */
+static int natsemi_poll(struct net_device *dev, int *budget)
+{
+	struct netdev_private *np = netdev_priv(dev);
+	void __iomem * ioaddr = ns_ioaddr(dev);
+
+	int work_to_do = min(*budget, dev->quota);
+	int work_done = 0;
+
+	do {
+		if (np->intr_status &
+		    (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) {
 			spin_lock(&np->lock);
 			netdev_tx_done(dev);
 			spin_unlock(&np->lock);
 		}
 
 		/* Abnormal error summary/uncommon events handlers. */
-		if (intr_status & IntrAbnormalSummary)
-			netdev_error(dev, intr_status);
-
-		if (--boguscnt < 0) {
-			if (netif_msg_intr(np))
-				printk(KERN_WARNING
-					"%s: Too much work at interrupt, "
-					"status=%#08x.\n",
-					dev->name, intr_status);
-			break;
+		if (np->intr_status & IntrAbnormalSummary)
+			netdev_error(dev, np->intr_status);
+		
+		if (np->intr_status &
+		    (IntrRxDone | IntrRxIntr | RxStatusFIFOOver |
+		     IntrRxErr | IntrRxOverrun)) {
+			netdev_rx(dev, &work_done, work_to_do);
 		}
-	} while (1);
+		
+		*budget -= work_done;
+		dev->quota -= work_done;
 
-	if (netif_msg_intr(np))
-		printk(KERN_DEBUG "%s: exiting interrupt.\n", dev->name);
+		if (work_done >= work_to_do)
+			return 1;
+
+		np->intr_status = readl(ioaddr + IntrStatus);
+	} while (np->intr_status);
 
-	return IRQ_RETVAL(handled);
+	netif_rx_complete(dev);
+
+	/* Reenable interrupts providing nothing is trying to shut
+	 * the chip down. */
+	spin_lock(&np->lock);
+	if (!np->hands_off && netif_running(dev))
+		natsemi_irq_enable(dev);
+	spin_unlock(&np->lock);
+
+	return 0;
 }
 
 /* This routine is logically part of the interrupt handler, but separated
    for clarity and better register allocation. */
-static void netdev_rx(struct net_device *dev)
+static void netdev_rx(struct net_device *dev, int *work_done, int work_to_do)
 {
 	struct netdev_private *np = netdev_priv(dev);
 	int entry = np->cur_rx % RX_RING_SIZE;
@@ -2237,6 +2275,12 @@ static void netdev_rx(struct net_device 
 				entry, desc_status);
 		if (--boguscnt < 0)
 			break;
+
+		if (*work_done >= work_to_do)
+			break;
+
+		(*work_done)++;
+
 		pkt_len = (desc_status & DescSizeMask) - 4;
 		if ((desc_status&(DescMore|DescPktOK|DescRxLong)) != DescPktOK){
 			if (desc_status & DescMore) {
@@ -2293,7 +2337,7 @@ static void netdev_rx(struct net_device 
 				np->rx_skbuff[entry] = NULL;
 			}
 			skb->protocol = eth_type_trans(skb, dev);
-			netif_rx(skb);
+			netif_receive_skb(skb);
 			dev->last_rx = jiffies;
 			np->stats.rx_packets++;
 			np->stats.rx_bytes += pkt_len;
@@ -3074,9 +3118,7 @@ static int netdev_close(struct net_devic
 	del_timer_sync(&np->timer);
 	disable_irq(dev->irq);
 	spin_lock_irq(&np->lock);
-	/* Disable interrupts, and flush posted writes */
-	writel(0, ioaddr + IntrEnable);
-	readl(ioaddr + IntrEnable);
+	natsemi_irq_disable(dev);
 	np->hands_off = 1;
 	spin_unlock_irq(&np->lock);
 	enable_irq(dev->irq);
@@ -3158,6 +3200,9 @@ static void __devexit natsemi_remove1 (s
  *	* netdev_timer: timer stopped by natsemi_suspend.
  *	* intr_handler: doesn't acquire the spinlock. suspend calls
  *		disable_irq() to enforce synchronization.
+ *      * natsemi_poll: checks before reenabling interrupts.  suspend
+ *              sets hands_off, disables interrupts and then waits with
+ *              netif_poll_disable().
  *
  * Interrupts must be disabled, otherwise hands_off can cause irq storms.
  */
@@ -3183,6 +3228,8 @@ static int natsemi_suspend (struct pci_d
 		spin_unlock_irq(&np->lock);
 		enable_irq(dev->irq);
 
+		netif_poll_disable(dev);
+
 		/* Update the error counts. */
 		__get_stats(dev);
 
@@ -3235,6 +3282,7 @@ static int natsemi_resume (struct pci_de
 		mod_timer(&np->timer, jiffies + 1*HZ);
 	}
 	netif_device_attach(dev);
+	netif_poll_enable(dev);
 out:
 	rtnl_unlock();
 	return 0;

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Resubmit: [PATCH] natsemi: NAPI support
  2006-01-04  7:32                   ` Mark Brown
@ 2006-01-04 13:40                     ` Harald Welte
  0 siblings, 0 replies; 12+ messages in thread
From: Harald Welte @ 2006-01-04 13:40 UTC (permalink / raw)
  To: Jeff Garzik, Tim Hockin, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On Wed, Jan 04, 2006 at 07:32:49AM +0000, Mark Brown wrote:
> This patch against 2.6.14 converts the natsemi driver to use NAPI.  It
> was originally based on one written by Harald Welte, though it has since
> been modified quite a bit, most extensively in order to remove the
> ability to disable NAPI since none of the other drivers seem to provide
> that functionality any more.

Mark, sorry for not responding earlier to your emails with regard to
your natsemi patch.

Thanks for pushing this persistently.  From reviewing your patch, I
personally thin it's fine to be merged.

I will test it on my natsemi based boxes later today.

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2006-01-04 13:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-12-04 22:47 [PATCH] natsemi: NAPI support Mark Brown
     [not found] ` <20051204231209.GA28949@electric-eye.fr.zoreil.com>
2005-12-05 23:23   ` Mark Brown
2005-12-06  0:19     ` Francois Romieu
2005-12-06 21:17       ` Mark Brown
2005-12-06 21:56         ` Francois Romieu
2005-12-09 10:48           ` Mark Brown
2005-12-12 23:55             ` Mark Brown
2005-12-13  0:25               ` Jeff Garzik
2005-12-13 22:59                 ` Mark Brown
2005-12-21 23:48                 ` Resubmit: " Mark Brown
2006-01-04  7:32                   ` Mark Brown
2006-01-04 13:40                     ` Harald Welte

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).