netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Richard Cochran <richardcochran@gmail.com>,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: [PATCH] net/cpsw: don't disable_irqs() after an interrupt has been received.
Date: Tue, 16 Apr 2013 20:21:00 +0200	[thread overview]
Message-ID: <1366136460-30732-1-git-send-email-bigeasy@linutronix.de> (raw)

This driver has usually four interrupts registered with the same ISR.
The ISR masks the interrupt source in DMA engine and CPSW, disables the
interrupt and schedules NAPI. After the NAPI routine completes, the
source is activated and interrupts are re-enabled.
With threaded-interrupts enabled, the enable/disable might go wrong.
After the RX interrupt arrived, the core marks the interrupt pending. If
the TX interrupt arrives before the RX started, then both lines are
marked pending and both disable the interrupt which then is disabled
twice. In NAPI complete the interrupt is enabled only once which means
the four interrupts are still masked and the device plays dead.

While playing with this on my beagle bone I didn't understand why
the interrupts are deactivated in the first place. According to my
testing, after calling cpsw_intr_disable() the interrupt is not active
anymore. I haven't seen the ISR being invoked again until after
cpsw_poll() enabled them (except for a different interrupt number).
Therefore I remove this.

In my testing I didn't notice anything unusual except one thing: I start
a wget of a 126MiB file (which is sttored on MMC) from the beagle bone.
On the first invocation I receive almost steady 5MiB/sec. In the
following invocations of wget I see the performance floating between
1MiB/sec and 4MiB/sec. It usually ends with overall around 2MiB/sec.
I couldn't notice nothing wrong. The only difference compared to the
first invocation is (most likely) that the file is now served from
memory and not read from MMC which should perform better but not worse.
After executing "while ((1)); do /bin/true; done" on the beagle bone
while the file was downloaded, I noticed that the speed rose to 9MiB/sec
- 10/MiB/sec. Now this looks like power/idle optimization on the beagle
bone side.

Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c |   16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index e2ba702..acb229c 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -133,19 +133,6 @@ do {								\
 #define CPSW_CMINTMAX_INTVL	(1000 / CPSW_CMINTMIN_CNT)
 #define CPSW_CMINTMIN_INTVL	((1000 / CPSW_CMINTMAX_CNT) + 1)
 
-#define cpsw_enable_irq(priv)	\
-	do {			\
-		u32 i;		\
-		for (i = 0; i < priv->num_irqs; i++) \
-			enable_irq(priv->irqs_table[i]); \
-	} while (0);
-#define cpsw_disable_irq(priv)	\
-	do {			\
-		u32 i;		\
-		for (i = 0; i < priv->num_irqs; i++) \
-			disable_irq_nosync(priv->irqs_table[i]); \
-	} while (0);
-
 #define cpsw_slave_index(priv)				\
 		((priv->data.dual_emac) ? priv->emac_port :	\
 		priv->data.active_slave)
@@ -513,13 +500,11 @@ static irqreturn_t cpsw_interrupt(int irq, void *dev_id)
 
 	if (likely(netif_running(priv->ndev))) {
 		cpsw_intr_disable(priv);
-		cpsw_disable_irq(priv);
 		napi_schedule(&priv->napi);
 	} else {
 		priv = cpsw_get_slave_priv(priv, 1);
 		if (likely(priv) && likely(netif_running(priv->ndev))) {
 			cpsw_intr_disable(priv);
-			cpsw_disable_irq(priv);
 			napi_schedule(&priv->napi);
 		}
 	}
@@ -540,7 +525,6 @@ static int cpsw_poll(struct napi_struct *napi, int budget)
 		napi_complete(napi);
 		cpsw_intr_enable(priv);
 		cpdma_ctlr_eoi(priv->dma, CPDMA_EOI_RX);
-		cpsw_enable_irq(priv);
 	}
 
 	if (num_rx || num_tx)
-- 
1.7.10.4

             reply	other threads:[~2013-04-16 18:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-16 18:21 Sebastian Andrzej Siewior [this message]
2013-04-17  6:19 ` [PATCH] net/cpsw: don't disable_irqs() after an interrupt has been received Mugunthan V N
2013-04-17  7:44   ` Sebastian Andrzej Siewior
2013-04-17  8:46     ` Mugunthan V N
2013-04-17  9:02       ` Sebastian Andrzej Siewior
2013-04-17 10:08         ` Mugunthan V N
2013-04-17 10:49           ` Sebastian Andrzej Siewior
2013-04-17 11:44             ` Mugunthan V N
2013-04-17 11:55             ` D-Link DUB E100 C1 not working when pinged by "ping 172.17.0.1 -c 1 -s 1965" Dean Jenkins
2013-04-17 21:12           ` [PATCH] net/cpsw: don't disable_irqs() after an interrupt has been received Sebastian Andrzej Siewior
2013-04-17  9:19 ` Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1366136460-30732-1-git-send-email-bigeasy@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=davem@davemloft.net \
    --cc=mugunthanvnm@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).