ethoc driver updates

ethoc driver updates

[Jonas Bonn]

Here's a set of patches that fix up some aspects of the ethoc network
driver:

i) device tree configuration
ii) reworked interrupt handling
iii) cleanups

This has been tested on an OpenRISC 1200 board; the OpenRISC Linux port
is using device trees now, hence the device tree bits.

/Jonas

[PATCH 1/8] ethoc: Add device tree configuration

[Jonas Bonn]

This patch adds the ability to describe ethernet devices via a flattened
device tree.  As device tree remains an optional feature, these bits all
need to be guarded by CONFIG_OF ifdefs.

MAC address is settable via the device tree parameter "local-mac-address";
however, the selection of the phy id is limited to probing, for now.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   31 ++++++++++++++++++++++++++++++-
 1 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index c5a2fe0..6b4b9e1 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <net/ethoc.h>
 
 static int buffer_size = 0x8000; /* 32 KBytes */
@@ -986,7 +987,21 @@ static int __devinit ethoc_probe(struct platform_device *pdev)
 			(struct ethoc_platform_data *)pdev->dev.platform_data;
 		memcpy(netdev->dev_addr, pdata->hwaddr, IFHWADDRLEN);
 		priv->phy_id = pdata->phy_id;
-	}
+	} else {
+		priv->phy_id = -1;
+
+#ifdef CONFIG_OF
+		{
+		uint8_t* mac;
+
+		mac = (uint8_t*)of_get_property(pdev->dev.of_node,
+						"local-mac-address",
+						NULL);
+		if (mac)
+			memcpy(netdev->dev_addr, mac, IFHWADDRLEN);
+		}
+#endif
+	} 
 
 	/* Check that the given MAC address is valid. If it isn't, read the
 	 * current MAC from the controller. */
@@ -1113,6 +1128,16 @@ static int ethoc_resume(struct platform_device *pdev)
 # define ethoc_resume  NULL
 #endif
 
+#ifdef CONFIG_OF
+static struct of_device_id ethoc_match[] = {
+	{
+		.compatible = "opencores,ethoc",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, ethoc_match);
+#endif
+
 static struct platform_driver ethoc_driver = {
 	.probe   = ethoc_probe,
 	.remove  = __devexit_p(ethoc_remove),
@@ -1120,6 +1145,10 @@ static struct platform_driver ethoc_driver = {
 	.resume  = ethoc_resume,
 	.driver  = {
 		.name = "ethoc",
+		.owner = THIS_MODULE,
+#ifdef CONFIG_OF
+		.of_match_table = ethoc_match,
+#endif
 	},
 };
 
-- 
1.7.1

[PATCH 2/8] ethoc: remove unused spinlock

[Jonas Bonn]

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 6b4b9e1..fc8c044 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -185,7 +185,6 @@ MODULE_PARM_DESC(buffer_size, "DMA buffer allocation size");
  * @netdev:	pointer to network device structure
  * @napi:	NAPI structure
  * @msg_enable:	device state flags
- * @rx_lock:	receive lock
  * @lock:	device lock
  * @phy:	attached PHY
  * @mdio:	MDIO bus for PHY access
@@ -210,7 +209,6 @@ struct ethoc {
 	struct napi_struct napi;
 	u32 msg_enable;
 
-	spinlock_t rx_lock;
 	spinlock_t lock;
 
 	struct phy_device *phy;
@@ -1061,7 +1059,6 @@ static int __devinit ethoc_probe(struct platform_device *pdev)
 	/* setup NAPI */
 	netif_napi_add(netdev, &priv->napi, ethoc_poll, 64);
 
-	spin_lock_init(&priv->rx_lock);
 	spin_lock_init(&priv->lock);
 
 	ret = register_netdev(netdev);
-- 
1.7.1

[PATCH 3/8] ethoc: enable interrupts after napi_complete

[Jonas Bonn]

From: Adam Edvardsson <adam.edvardsson@orsoc.se>

Occasionally, it seems that some race is causing the interrupts to not be
reenabled otherwise with the end result that networking just stops working.
Enabling interrupts after calling napi_complete is more in line with what
other drivers do.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index fc8c044..53c03f2 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -569,8 +569,8 @@ static int ethoc_poll(struct napi_struct *napi, int budget)
 
 	work_done = ethoc_rx(priv->netdev, budget);
 	if (work_done < budget) {
-		ethoc_enable_irq(priv, INT_MASK_RX);
 		napi_complete(napi);
+		ethoc_enable_irq(priv, INT_MASK_RX);
 	}
 
 	return work_done;
-- 
1.7.1

[PATCH 4/8] ethoc: prevent overflow of rx counter

[Jonas Bonn]

Rewind cur_rx to prevent it from overflowing.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 53c03f2..7d1b5d8 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -408,6 +408,9 @@ static int ethoc_rx(struct net_device *dev, int limit)
 	struct ethoc *priv = netdev_priv(dev);
 	int count;
 
+	/* Prevent overflow of priv->cur_rx by rewinding it */	
+	priv->cur_rx = priv->cur_rx % priv->num_rx;
+
 	for (count = 0; count < limit; ++count) {
 		unsigned int entry;
 		struct ethoc_bd bd;
-- 
1.7.1

[PATCH 5/8] ethoc: Double check pending RX packet

[Jonas Bonn]

An interrupt may occur between checking bd.stat and clearing the
interrupt source register which would result in the packet going totally
unnoticed as the interrupt will be missed.  Double check bd.stat after
clearing the interrupt source register to guard against such an
occurrence.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 7d1b5d8..5afa934 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -417,8 +417,20 @@ static int ethoc_rx(struct net_device *dev, int limit)
 
 		entry = priv->num_tx + (priv->cur_rx % priv->num_rx);
 		ethoc_read_bd(priv, entry, &bd);
-		if (bd.stat & RX_BD_EMPTY)
-			break;
+		if (bd.stat & RX_BD_EMPTY) {
+			ethoc_ack_irq(priv, INT_MASK_RX);
+			/* If packet (interrupt) came in between checking
+			 * BD_EMTPY and clearing the interrupt source, then we
+			 * risk missing the packet as the RX interrupt won't
+			 * trigger right away when we reenable it; hence, check
+			 * BD_EMTPY here again to make sure there isn't such a
+			 * packet waiting for us...
+			 */
+			ethoc_read_bd(priv, entry, &bd);
+			if (bd.stat & RX_BD_EMPTY) 
+				break;
+
+		}
 
 		if (ethoc_update_rx_stats(priv, &bd) == 0) {
 			int size = bd.stat >> 16;
-- 
1.7.1

[PATCH 6/8] ethoc: rework interrupt handling

[Jonas Bonn]

The old interrupt handling was incorrect in that it did not account for the
fact that the interrupt source bits get set irregardless of whether or not
their corresponding mask is set.  This patch fixes that by masking off the
source bits for masked interrupts.

Furthermore, the handling of transmission events is moved to the NAPI polling
handler alongside the reception handler, thus preventing a whole bunch of
interrupts during heavy traffic.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   79 +++++++++++++++++++++++++++++++++------------------
 1 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 5afa934..70ca2f3 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -499,29 +499,43 @@ static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
 	return 0;
 }
 
-static void ethoc_tx(struct net_device *dev)
+static int ethoc_tx(struct net_device *dev, int limit)
 {
 	struct ethoc *priv = netdev_priv(dev);
+	int count;
+	struct ethoc_bd bd;
 
-	spin_lock(&priv->lock);
+	for (count = 0; count < limit; ++count) {
+		unsigned int entry;
 
-	while (priv->dty_tx != priv->cur_tx) {
-		unsigned int entry = priv->dty_tx % priv->num_tx;
-		struct ethoc_bd bd;
+		entry = priv->dty_tx % priv->num_tx;
 
 		ethoc_read_bd(priv, entry, &bd);
-		if (bd.stat & TX_BD_READY)
-			break;
 
-		entry = (++priv->dty_tx) % priv->num_tx;
+		if (bd.stat & TX_BD_READY || (priv->dty_tx == priv->cur_tx)) {
+			ethoc_ack_irq(priv, INT_MASK_TX);
+			/* If interrupt came in between reading in the BD
+			 * and clearing the interrupt source, then we risk 
+			 * missing the event as the TX interrupt won't trigger
+			 * right away when we reenable it; hence, check 
+			 * BD_EMPTY here again to make sure there isn't such an
+			 * event pending...
+			 */
+			ethoc_read_bd(priv, entry, &bd);
+			if (bd.stat & TX_BD_READY ||
+			    (priv->dty_tx == priv->cur_tx))
+				break;
+		}
+
 		(void)ethoc_update_tx_stats(priv, &bd);
+		priv->dty_tx++;
 	}
 
-	if ((priv->cur_tx - priv->dty_tx) <= (priv->num_tx / 2))
+	if ((priv->cur_tx - priv->dty_tx) <= (priv->num_tx / 2)) {
 		netif_wake_queue(dev);
+	}
 
-	ethoc_ack_irq(priv, INT_MASK_TX);
-	spin_unlock(&priv->lock);
+	return count;
 }
 
 static irqreturn_t ethoc_interrupt(int irq, void *dev_id)
@@ -529,32 +543,38 @@ static irqreturn_t ethoc_interrupt(int irq, void *dev_id)
 	struct net_device *dev = dev_id;
 	struct ethoc *priv = netdev_priv(dev);
 	u32 pending;
-
-	ethoc_disable_irq(priv, INT_MASK_ALL);
+	u32 mask;
+
+	/* Figure out what triggered the interrupt...
+	 * The tricky bit here is that the interrupt source bits get
+	 * set in INT_SOURCE for an event irregardless of whether that
+	 * event is masked or not.  Thus, in order to figure out what
+	 * triggered the interrupt, we need to remove the sources
+	 * for all events that are currently masked.  This behaviour
+	 * is not particularly well documented but reasonable...
+	 */
+	mask = ethoc_read(priv, INT_MASK);
 	pending = ethoc_read(priv, INT_SOURCE);
+	pending &= mask;
+
 	if (unlikely(pending == 0)) {
-		ethoc_enable_irq(priv, INT_MASK_ALL);
 		return IRQ_NONE;
 	}
 
 	ethoc_ack_irq(priv, pending);
 
+	/* We always handle the dropped packet interrupt */
 	if (pending & INT_MASK_BUSY) {
 		dev_err(&dev->dev, "packet dropped\n");
 		dev->stats.rx_dropped++;
 	}
 
-	if (pending & INT_MASK_RX) {
-		if (napi_schedule_prep(&priv->napi))
-			__napi_schedule(&priv->napi);
-	} else {
-		ethoc_enable_irq(priv, INT_MASK_RX);
+	/* Handle receive/transmit event by switching to polling */
+	if (pending & (INT_MASK_TX | INT_MASK_RX)) {
+		ethoc_disable_irq(priv, INT_MASK_TX | INT_MASK_RX);
+		napi_schedule(&priv->napi);
 	}
 
-	if (pending & INT_MASK_TX)
-		ethoc_tx(dev);
-
-	ethoc_enable_irq(priv, INT_MASK_ALL & ~INT_MASK_RX);
 	return IRQ_HANDLED;
 }
 
@@ -580,15 +600,18 @@ static int ethoc_get_mac_address(struct net_device *dev, void *addr)
 static int ethoc_poll(struct napi_struct *napi, int budget)
 {
 	struct ethoc *priv = container_of(napi, struct ethoc, napi);
-	int work_done = 0;
+	int rx_work_done = 0;
+	int tx_work_done = 0;
+
+	rx_work_done = ethoc_rx(priv->netdev, budget);
+	tx_work_done = ethoc_tx(priv->netdev, budget);
 
-	work_done = ethoc_rx(priv->netdev, budget);
-	if (work_done < budget) {
+	if (rx_work_done < budget && tx_work_done < budget) {
 		napi_complete(napi);
-		ethoc_enable_irq(priv, INT_MASK_RX);
+		ethoc_enable_irq(priv, INT_MASK_TX | INT_MASK_RX);
 	}
 
-	return work_done;
+	return rx_work_done;
 }
 
 static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
-- 
1.7.1

[PATCH 7/8] ethoc: rework mdio read/write

[Jonas Bonn]

MDIO read and write were checking whether a timeout had expired to determine
whether to recheck the result of the MDIO operation.  Under heavy CPU usage,
however, it was possible for the timeout to expire before the routine got
around to be able to check a second time even, thus erroneousy returning an
-EBUSY.

This patch changes the the MDIO IO routines to try up to five times to complete
the operation before giving up, thus lessening the dependency on CPU load.

This resolves a problem whereby a ping flood would keep the CPU so busy that
the above problem would manifest itself; the MDIO command to check link status
would fail and the interface would erroneously be shut down.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 70ca2f3..4f7c9ad 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -616,13 +616,13 @@ static int ethoc_poll(struct napi_struct *napi, int budget)
 
 static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
 {
-	unsigned long timeout = jiffies + ETHOC_MII_TIMEOUT;
 	struct ethoc *priv = bus->priv;
+	int i;
 
 	ethoc_write(priv, MIIADDRESS, MIIADDRESS_ADDR(phy, reg));
 	ethoc_write(priv, MIICOMMAND, MIICOMMAND_READ);
 
-	while (time_before(jiffies, timeout)) {
+	for (i=0; i < 5; i++) {
 		u32 status = ethoc_read(priv, MIISTATUS);
 		if (!(status & MIISTATUS_BUSY)) {
 			u32 data = ethoc_read(priv, MIIRX_DATA);
@@ -630,8 +630,7 @@ static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
 			ethoc_write(priv, MIICOMMAND, 0);
 			return data;
 		}
-
-		schedule();
+		usleep_range(100,200);
 	}
 
 	return -EBUSY;
@@ -639,22 +638,21 @@ static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
 
 static int ethoc_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 {
-	unsigned long timeout = jiffies + ETHOC_MII_TIMEOUT;
 	struct ethoc *priv = bus->priv;
+	int i;
 
 	ethoc_write(priv, MIIADDRESS, MIIADDRESS_ADDR(phy, reg));
 	ethoc_write(priv, MIITX_DATA, val);
 	ethoc_write(priv, MIICOMMAND, MIICOMMAND_WRITE);
 
-	while (time_before(jiffies, timeout)) {
+	for (i=0; i < 5; i++) {
 		u32 stat = ethoc_read(priv, MIISTATUS);
 		if (!(stat & MIISTATUS_BUSY)) {
 			/* reset MII command register */
 			ethoc_write(priv, MIICOMMAND, 0);
 			return 0;
 		}
-
-		schedule();
+		usleep_range(100,200);
 	}
 
 	return -EBUSY;
-- 
1.7.1

[PATCH 8/8] ethoc: fix function return type

[Jonas Bonn]

update_ethoc_tx_stats doesn't need to return anything so make its return
type void in order to avoid an unnecessary cast when the function is called.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 4f7c9ad..b12c588 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -466,7 +466,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
 	return count;
 }
 
-static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
+static void ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
 {
 	struct net_device *netdev = dev->netdev;
 
@@ -496,7 +496,6 @@ static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
 	netdev->stats.collisions += (bd->stat >> 4) & 0xf;
 	netdev->stats.tx_bytes += bd->stat >> 16;
 	netdev->stats.tx_packets++;
-	return 0;
 }
 
 static int ethoc_tx(struct net_device *dev, int limit)
@@ -527,7 +526,7 @@ static int ethoc_tx(struct net_device *dev, int limit)
 				break;
 		}
 
-		(void)ethoc_update_tx_stats(priv, &bd);
+		ethoc_update_tx_stats(priv, &bd);
 		priv->dty_tx++;
 	}
 
-- 
1.7.1

Re: [PATCH 4/8] ethoc: prevent overflow of rx counter

[Ben Hutchings]

On Wed, 2010-11-24 at 14:40 +0100, Jonas Bonn wrote:
> Rewind cur_rx to prevent it from overflowing.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
> ---
>  drivers/net/ethoc.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
> index 53c03f2..7d1b5d8 100644
> --- a/drivers/net/ethoc.c
> +++ b/drivers/net/ethoc.c
> @@ -408,6 +408,9 @@ static int ethoc_rx(struct net_device *dev, int limit)
>  	struct ethoc *priv = netdev_priv(dev);
>  	int count;
>  
> +	/* Prevent overflow of priv->cur_rx by rewinding it */	
> +	priv->cur_rx = priv->cur_rx % priv->num_rx;
> +

Division is expensive; you should either use masking (if priv->num_rx is
guaranteed to be a power of 2) or check for overflow whenever you
increment priv->cur_rx:

	if (++priv->cur_rx == priv->num_rx)
		priv->cur_rx = 0;

Ben.

>  	for (count = 0; count < limit; ++count) {
>  		unsigned int entry;
>  		struct ethoc_bd bd;

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 4/8] ethoc: prevent overflow of rx counter

[Eric Dumazet]

Le mercredi 24 novembre 2010 à 14:40 +0100, Jonas Bonn a écrit :
> Rewind cur_rx to prevent it from overflowing.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
> ---
>  drivers/net/ethoc.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
> index 53c03f2..7d1b5d8 100644
> --- a/drivers/net/ethoc.c
> +++ b/drivers/net/ethoc.c
> @@ -408,6 +408,9 @@ static int ethoc_rx(struct net_device *dev, int limit)
>  	struct ethoc *priv = netdev_priv(dev);
>  	int count;
>  
> +	/* Prevent overflow of priv->cur_rx by rewinding it */	
> +	priv->cur_rx = priv->cur_rx % priv->num_rx;
> +
>  	for (count = 0; count < limit; ++count) {
>  		unsigned int entry;
>  		struct ethoc_bd bd;


Hmm... please try following code instead (no divides)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index c5a2fe0..591b698 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -413,7 +413,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
 		unsigned int entry;
 		struct ethoc_bd bd;
 
-		entry = priv->num_tx + (priv->cur_rx % priv->num_rx);
+		entry = priv->num_tx + priv->cur_rx;
 		ethoc_read_bd(priv, entry, &bd);
 		if (bd.stat & RX_BD_EMPTY)
 			break;
@@ -446,7 +446,8 @@ static int ethoc_rx(struct net_device *dev, int limit)
 		bd.stat &= ~RX_BD_STATS;
 		bd.stat |=  RX_BD_EMPTY;
 		ethoc_write_bd(priv, entry, &bd);
-		priv->cur_rx++;
+		if (++priv->cur_rx == priv->num_rx)
+			priv->cur_rx = 0;
 	}
 
 	return count;

Re: [PATCH 3/8] ethoc: enable interrupts after napi_complete

[Laurent Chavey]

ethoc_rx can return a value == budget
if work_done == budget, interrupt
are not re-enabled

in net_rx_action, the check for budget <= 0 will
cause the poll loop to not be called, but will removed
the callback to be removed from the poll_list.

could this result in interrupt not being re-enabled ?

Re: [PATCH 3/8] ethoc: enable interrupts after napi_complete

[Laurent Chavey]

actually my previous comments are not correct.
the check for work_done < budget will
only cause an extra call with work_done == 0
if no more work is done.
so that will work.

--
--------------------------------------------------------------------------------

Re: [PATCH 1/8] ethoc: Add device tree configuration

[David Miller]

From: Jonas Bonn <jonas@southpole.se>
Date: Wed, 24 Nov 2010 14:40:51 +0100

> This patch adds the ability to describe ethernet devices via a flattened
> device tree.  As device tree remains an optional feature, these bits all
> need to be guarded by CONFIG_OF ifdefs.
> 
> MAC address is settable via the device tree parameter "local-mac-address";
> however, the selection of the phy id is limited to probing, for now.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
 ...
> +	} 

Trailing whitespace.

Re: [PATCH 4/8] ethoc: prevent overflow of rx counter

[David Miller]

From: Jonas Bonn <jonas@southpole.se>
Date: Wed, 24 Nov 2010 14:40:54 +0100

> Rewind cur_rx to prevent it from overflowing.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
...
> +	/* Prevent overflow of priv->cur_rx by rewinding it */	

Trailing whitespace, also please integrate the feedback from Ben
and Eric Dumazet about making this computation less expensive.

Re: [PATCH 5/8] ethoc: Double check pending RX packet

[David Miller]

From: Jonas Bonn <jonas@southpole.se>
Date: Wed, 24 Nov 2010 14:40:55 +0100

> An interrupt may occur between checking bd.stat and clearing the
> interrupt source register which would result in the packet going totally
> unnoticed as the interrupt will be missed.  Double check bd.stat after
> clearing the interrupt source register to guard against such an
> occurrence.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
 ...
> +			if (bd.stat & RX_BD_EMPTY) 

Trailing whitespace.

> +				break;
> +
> +		}

Unnecessary empty line.

Re: [PATCH 6/8] ethoc: rework interrupt handling

[David Miller]

From: Jonas Bonn <jonas@southpole.se>
Date: Wed, 24 Nov 2010 14:40:56 +0100

> The old interrupt handling was incorrect in that it did not account for the
> fact that the interrupt source bits get set irregardless of whether or not
> their corresponding mask is set.  This patch fixes that by masking off the
> source bits for masked interrupts.
> 
> Furthermore, the handling of transmission events is moved to the NAPI polling
> handler alongside the reception handler, thus preventing a whole bunch of
> interrupts during heavy traffic.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>

> +			 * and clearing the interrupt source, then we risk 
 ...
> +			 * right away when we reenable it; hence, check 

Trailing whitespace.

> -	if ((priv->cur_tx - priv->dty_tx) <= (priv->num_tx / 2))
> +	if ((priv->cur_tx - priv->dty_tx) <= (priv->num_tx / 2)) {
>  		netif_wake_queue(dev);
> +	}
>  

One-line statement does not require braces.