Re: FW: Flaw in 3c59x.c or in Kernel?

From: David Hinds (dhinds@valinux.com)
Date: Wed Jan 05 2000 - 18:09:53 EST


The "too much work" handler in the 3c59x driver is broken... and
fixing the timer interrupt to re-enable the disabled interrupt sources
does not fix the problem.

The basic problem is that if you disable some interrupt sources in the
interrupt handler, those sources may never be re-asserted, even if you
re-enable them later. If you ack a DownComplete or UpComplete
interrupt without servicing it, you may never get another one.

Here's a patch I made up for the CardBus version of the 3c59x driver,
just yesterday. It seems to be stable... however it will generate
more "too much work" messages than the old driver did. So it probably
should not be taken as a permanent solution... but give it a try.

-- Dave Hinds

--- 3c575_cb.c 2000/01/03 22:48:39 1.32
+++ 3c575_cb.c 2000/01/05 04:19:15
@@ -481,6 +481,7 @@
         u16 capabilities, info1, info2; /* Various, from EEPROM. */
         u16 advertising; /* NWay media advertisement */
         unsigned char phys[2]; /* MII device addresses. */
+ u16 deferred;
 };
 
 /* The action to take with a media selection timer tick.
@@ -1356,6 +1359,8 @@
 
         vp->timer.expires = RUN_AT(next_tick);
         add_timer(&vp->timer);
+ if (vp->deferred)
+ outw(FakeIntr, ioaddr + EL3_CMD);
         return;
 }
 
@@ -1648,6 +1653,10 @@
         ioaddr = dev->base_addr;
         latency = inb(ioaddr + Timer);
         status = inw(ioaddr + EL3_STATUS);
+ if (status & IntReq) {
+ status |= vp->deferred;
+ vp->deferred = 0;
+ }
 
         if (status == 0xffff)
                 goto handler_exit;
@@ -1716,19 +1725,20 @@
                 }
 
                 if (--work_done < 0) {
- if ((status & (0x7fe - (UpComplete | DownComplete))) == 0) {
- /* Just ack these and return. */
- outw(AckIntr | UpComplete | DownComplete, ioaddr + EL3_CMD);
- } else {
- printk(KERN_WARNING "%s: Too much work in interrupt, status "
- "%4.4x. Temporarily disabling functions (%4.4x).\n",
- dev->name, status, (~status) & vp->status_enable);
- /* Disable all pending interrupts. */
- outw(SetStatusEnb | ((~status) & vp->status_enable), ioaddr + EL3_CMD);
- outw(AckIntr | 0x7FF, ioaddr + EL3_CMD);
- /* The timer will reenable interrupts. */
- break;
- }
+ printk(KERN_WARNING "%s: Too much work in interrupt, status "
+ "%4.4x.\n", dev->name, status);
+ /* Disable all pending interrupts. */
+ do {
+ vp->deferred |= status;
+ outw(SetStatusEnb | (~vp->deferred & vp->status_enable),
+ ioaddr + EL3_CMD);
+ outw(AckIntr | (vp->deferred & 0x7ff), ioaddr + EL3_CMD);
+ } while ((status = inw(ioaddr + EL3_CMD)) & IntLatch);
+ /* The timer will reenable interrupts. */
+ del_timer(&vp->timer);
+ vp->timer.expires = RUN_AT(1);
+ add_timer(&vp->timer);
+ break;
                 }
                 /* Acknowledge the IRQ. */
                 outw(AckIntr | IntReq | IntLatch, ioaddr + EL3_CMD);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Jan 07 2000 - 21:00:05 EST