| From b0cf4dfb7cd21556efd9a6a67edcba0840b4d98d Mon Sep 17 00:00:00 2001 |
| From: Ben Hutchings <ben@decadent.org.uk> |
| Date: Wed, 7 Apr 2010 20:55:47 -0700 |
| Subject: 3c503: Fix IRQ probing |
| |
| From: Ben Hutchings <ben@decadent.org.uk> |
| |
| commit b0cf4dfb7cd21556efd9a6a67edcba0840b4d98d upstream. |
| |
| The driver attempts to select an IRQ for the NIC automatically by |
| testing which of the supported IRQs are available and then probing |
| each available IRQ with probe_irq_{on,off}(). There are obvious race |
| conditions here, besides which: |
| 1. The test for availability is done by passing a NULL handler, which |
| now always returns -EINVAL, thus the device cannot be opened: |
| <http://bugs.debian.org/566522> |
| 2. probe_irq_off() will report only the first ISA IRQ handled, |
| potentially leading to a false negative. |
| |
| There was another bug that meant it ignored all error codes from |
| request_irq() except -EBUSY, so it would 'succeed' despite this |
| (possibly causing conflicts with other ISA devices). This was fixed |
| by ab08999d6029bb2c79c16be5405d63d2bedbdfea 'WARNING: some |
| request_irq() failures ignored in el2_open()', which exposed bug 1. |
| |
| This patch: |
| 1. Replaces the use of probe_irq_{on,off}() with a real interrupt handler |
| 2. Adds a delay before checking the interrupt-seen flag |
| 3. Disables interrupts on all failure paths |
| 4. Distinguishes error codes from the second request_irq() call, |
| consistently with the first |
| |
| Compile-tested only. |
| |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| drivers/net/3c503.c | 42 ++++++++++++++++++++++++++++++------------ |
| 1 file changed, 30 insertions(+), 12 deletions(-) |
| |
| --- a/drivers/net/3c503.c |
| +++ b/drivers/net/3c503.c |
| @@ -380,6 +380,12 @@ out: |
| return retval; |
| } |
| |
| +static irqreturn_t el2_probe_interrupt(int irq, void *seen) |
| +{ |
| + *(bool *)seen = true; |
| + return IRQ_HANDLED; |
| +} |
| + |
| static int |
| el2_open(struct net_device *dev) |
| { |
| @@ -391,23 +397,35 @@ el2_open(struct net_device *dev) |
| |
| outb(EGACFR_NORM, E33G_GACFR); /* Enable RAM and interrupts. */ |
| do { |
| - retval = request_irq(*irqp, NULL, 0, "bogus", dev); |
| - if (retval >= 0) { |
| + bool seen; |
| + |
| + retval = request_irq(*irqp, el2_probe_interrupt, 0, |
| + dev->name, &seen); |
| + if (retval == -EBUSY) |
| + continue; |
| + if (retval < 0) |
| + goto err_disable; |
| + |
| /* Twinkle the interrupt, and check if it's seen. */ |
| - unsigned long cookie = probe_irq_on(); |
| + seen = false; |
| + smp_wmb(); |
| outb_p(0x04 << ((*irqp == 9) ? 2 : *irqp), E33G_IDCFR); |
| outb_p(0x00, E33G_IDCFR); |
| - if (*irqp == probe_irq_off(cookie) && /* It's a good IRQ line! */ |
| - ((retval = request_irq(dev->irq = *irqp, |
| - eip_interrupt, 0, |
| - dev->name, dev)) == 0)) |
| - break; |
| - } else { |
| - if (retval != -EBUSY) |
| - return retval; |
| - } |
| + msleep(1); |
| + free_irq(*irqp, el2_probe_interrupt); |
| + if (!seen) |
| + continue; |
| + |
| + retval = request_irq(dev->irq = *irqp, eip_interrupt, 0, |
| + dev->name, dev); |
| + if (retval == -EBUSY) |
| + continue; |
| + if (retval < 0) |
| + goto err_disable; |
| } while (*++irqp); |
| + |
| if (*irqp == 0) { |
| + err_disable: |
| outb(EGACFR_IRQOFF, E33G_GACFR); /* disable interrupts. */ |
| return -EAGAIN; |
| } |