Skip to contentSkip to conversation

smc91x driver: fix bug: print warning only in interrupt mode

PATCH NAK [email protected] 3 participants 3 messages Mar 28, 2008 – Mar 31, 2008

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

[email protected]

author
#0 lore raw

PATCH 1/1 smc91x driver: fix bug: print warning only in interrupt mode

From: Sonic Zhang <[email protected]>

http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=3956

Signed-off-by Sonic Zhang <[email protected]>
Signed-off-by Bryan Wu <[email protected]>
Cc Jeff Garzik <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 drivers/net/smc91x.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/net/smc91x.c renamed from drivers/net/smc91x.c~smc91x-driver-fix-bug-print-warning-only-in-interrupt-mode +2 -0
diff -puN drivers/net/smc91x.c~smc91x-driver-fix-bug-print-warning-only-in-interrupt-mode drivers/net/smc91x.c
@@ -1326,9 +1326,11 @@ static irqreturn_t smc_interrupt(int irq
 	SMC_SET_INT_MASK(lp, mask);
 	spin_unlock(&lp->lock);
 
+#ifndef CONFIG_NET_POLL_CONTROLLER
 	if (timeout == MAX_IRQ_LOOPS)
 		PRINTK("%s: spurious interrupt (mask = 0x%02x)\n",
 		       dev->name, mask);
+#endif
 	DBG(3, "%s: Interrupt done (%d loops)\n",
 	       dev->name, MAX_IRQ_LOOPS - timeout);

_

NAK
#1 lore raw
Quoted 17 lines · [email protected] wrote:

From: Sonic Zhang <[email protected]>

http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=3956

Signed-off-by Sonic Zhang <[email protected]>
Signed-off-by Bryan Wu <[email protected]>
Cc Jeff Garzik <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 drivers/net/smc91x.c |    2 ++
 1 file changed, 2 insertions(+)

diff -puN drivers/net/smc91x.c~smc91x-driver-fix-bug-print-warning-only-in-interrupt-mode drivers/net/smc91x.c

drivers/net/smc91x.c static irqreturn_t smc_interrupt(int irq · line 1335

Quoted 10 lines
drivers/net/smc91x.c renamed from drivers/net/smc91x.c~smc91x-driver-fix-bug-print-warning-only-in-interrupt-mode +2 -0
@@ -1326,9 +1326,11 @@ static irqreturn_t smc_interrupt(int irq
 	SMC_SET_INT_MASK(lp, mask);
 	spin_unlock(&lp->lock);
 
+#ifndef CONFIG_NET_POLL_CONTROLLER
 	if (timeout == MAX_IRQ_LOOPS)
 		PRINTK("%s: spurious interrupt (mask = 0x%02x)\n",
 		       dev->name, mask);
+#endif
 	DBG(3, "%s: Interrupt done (%d loops)\n",
 	       dev->name, MAX_IRQ_LOOPS - timeout);

NAK

Either this code is OK or it isn't. This patch simply hides a problem inside a configuration the developer probably doesn't use.

What's the _real_ problem? Looking at the tracker, I'd guess that some events need to be masked, but are not?

If the hardware is asserting events continually, the current code does the right thing -- keep processing interrupt events flagged, until (a) no more events asserted or (b) max loop count reached.

So the question is to find out why you are hitting the timeout, and what to do about it.

NAK
#2 lore raw

In netpoll mode, smc_interrupt() is invoked repeatedly till a net packet is received. Timeout is meaning less here.

Sonic

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Saturday, March 29, 2008 9:45 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [patch (for 2.6.25?) 1/1] smc91x driver: fix bug: print warning only in interrupt mode

Quoted 22 lines · [email protected] wrote:

From: Sonic Zhang <[email protected]>

http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=Tr
ackerItemEdit&tracker_item_id=3956

Signed-off-by Sonic Zhang <[email protected]>
Signed-off-by Bryan Wu <[email protected]>
Cc Jeff Garzik <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 drivers/net/smc91x.c |    2 ++
 1 file changed, 2 insertions(+)

diff -puN drivers/net/smc91x.c~smc91x-driver-fix-bug-print-warning-only-in-inter rupt-mode drivers/net/smc91x.c
---
a/drivers/net/smc91x.c~smc91x-driver-fix-bug-print-warning-only-in-int errupt-mode
+++ a/drivers/net/smc91x.c

drivers/net/smc91x.c static irqreturn_t smc_interrupt(int irq · line 1335

Quoted 10 lines
drivers/net/smc91x.c renamed from drivers/net/smc91x.c~smc91x-driver-fix-bug-print-warning-only-in-interrupt-mode +2 -0
@@ -1326,9 +1326,11 @@ static irqreturn_t smc_interrupt(int irq
 	SMC_SET_INT_MASK(lp, mask);
 	spin_unlock(&lp->lock);
 
+#ifndef CONFIG_NET_POLL_CONTROLLER
 	if (timeout == MAX_IRQ_LOOPS)
 		PRINTK("%s: spurious interrupt (mask = 0x%02x)\n",
 		       dev->name, mask);
+#endif
 	DBG(3, "%s: Interrupt done (%d loops)\n",
 	       dev->name, MAX_IRQ_LOOPS - timeout);

NAK

Either this code is OK or it isn't. This patch simply hides a problem inside a configuration the developer probably doesn't use.

What's the _real_ problem? Looking at the tracker, I'd guess that some events need to be masked, but are not?

If the hardware is asserting events continually, the current code does the right thing -- keep processing interrupt events flagged, until (a) no more events asserted or (b) max loop count reached.

So the question is to find out why you are hitting the timeout, and what to do about it.