diff options
author | broonie@sirena.org.uk <broonie@sirena.org.uk> | 2007-03-14 15:49:14 -0400 |
---|---|---|
committer | Jeff Garzik <jeff@garzik.org> | 2007-03-15 10:59:54 -0400 |
commit | 069f8256362b7a17da532f0631cee73b4cfee65b (patch) | |
tree | ee8e9e4a3b78de04e15a71a4b4216bfc050b3a8a /drivers/net/natsemi.c | |
parent | 14fdd90ef2ec1878d6851ec4dd8d5abb2cef098c (diff) |
natsemi: Fix NAPI for interrupt sharing
The interrupt status register for the natsemi chips is clear on read and
was read unconditionally from both the interrupt and from the NAPI poll
routine, meaning that if the interrupt service routine was called (for
example, due to a shared interrupt) while a NAPI poll was scheduled
interrupts could be missed. This patch fixes that by ensuring that the
interrupt status register is only read by the interrupt handler when
interrupts are enabled from the chip.
It also reverts a workaround for this problem from the netpoll hook and
improves the trace for interrupt events.
Thanks to Sergei Shtylyov <sshtylyov@ru.mvista.com> for spotting the
issue, Mark Huth <mhuth@mvista.com> for a simpler method and Simon
Blake <simon@citylink.co.nz> for testing resources.
Signed-Off-By: Mark Brown <broonie@sirena.org.uk>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Diffstat (limited to 'drivers/net/natsemi.c')
-rw-r--r-- | drivers/net/natsemi.c | 38 |
1 files changed, 20 insertions, 18 deletions
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c index 2888880df3b7..5f88200724e6 100644 --- a/drivers/net/natsemi.c +++ b/drivers/net/natsemi.c | |||
@@ -2119,28 +2119,35 @@ static irqreturn_t intr_handler(int irq, void *dev_instance) | |||
2119 | struct netdev_private *np = netdev_priv(dev); | 2119 | struct netdev_private *np = netdev_priv(dev); |
2120 | void __iomem * ioaddr = ns_ioaddr(dev); | 2120 | void __iomem * ioaddr = ns_ioaddr(dev); |
2121 | 2121 | ||
2122 | if (np->hands_off) | 2122 | /* Reading IntrStatus automatically acknowledges so don't do |
2123 | * that while interrupts are disabled, (for example, while a | ||
2124 | * poll is scheduled). */ | ||
2125 | if (np->hands_off || !readl(ioaddr + IntrEnable)) | ||
2123 | return IRQ_NONE; | 2126 | return IRQ_NONE; |
2124 | 2127 | ||
2125 | /* Reading automatically acknowledges. */ | ||
2126 | np->intr_status = readl(ioaddr + IntrStatus); | 2128 | np->intr_status = readl(ioaddr + IntrStatus); |
2127 | 2129 | ||
2130 | if (!np->intr_status) | ||
2131 | return IRQ_NONE; | ||
2132 | |||
2128 | if (netif_msg_intr(np)) | 2133 | if (netif_msg_intr(np)) |
2129 | printk(KERN_DEBUG | 2134 | printk(KERN_DEBUG |
2130 | "%s: Interrupt, status %#08x, mask %#08x.\n", | 2135 | "%s: Interrupt, status %#08x, mask %#08x.\n", |
2131 | dev->name, np->intr_status, | 2136 | dev->name, np->intr_status, |
2132 | readl(ioaddr + IntrMask)); | 2137 | readl(ioaddr + IntrMask)); |
2133 | 2138 | ||
2134 | if (!np->intr_status) | ||
2135 | return IRQ_NONE; | ||
2136 | |||
2137 | prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]); | 2139 | prefetch(&np->rx_skbuff[np->cur_rx % RX_RING_SIZE]); |
2138 | 2140 | ||
2139 | if (netif_rx_schedule_prep(dev)) { | 2141 | if (netif_rx_schedule_prep(dev)) { |
2140 | /* Disable interrupts and register for poll */ | 2142 | /* Disable interrupts and register for poll */ |
2141 | natsemi_irq_disable(dev); | 2143 | natsemi_irq_disable(dev); |
2142 | __netif_rx_schedule(dev); | 2144 | __netif_rx_schedule(dev); |
2143 | } | 2145 | } else |
2146 | printk(KERN_WARNING | ||
2147 | "%s: Ignoring interrupt, status %#08x, mask %#08x.\n", | ||
2148 | dev->name, np->intr_status, | ||
2149 | readl(ioaddr + IntrMask)); | ||
2150 | |||
2144 | return IRQ_HANDLED; | 2151 | return IRQ_HANDLED; |
2145 | } | 2152 | } |
2146 | 2153 | ||
@@ -2156,6 +2163,12 @@ static int natsemi_poll(struct net_device *dev, int *budget) | |||
2156 | int work_done = 0; | 2163 | int work_done = 0; |
2157 | 2164 | ||
2158 | do { | 2165 | do { |
2166 | if (netif_msg_intr(np)) | ||
2167 | printk(KERN_DEBUG | ||
2168 | "%s: Poll, status %#08x, mask %#08x.\n", | ||
2169 | dev->name, np->intr_status, | ||
2170 | readl(ioaddr + IntrMask)); | ||
2171 | |||
2159 | if (np->intr_status & | 2172 | if (np->intr_status & |
2160 | (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { | 2173 | (IntrTxDone | IntrTxIntr | IntrTxIdle | IntrTxErr)) { |
2161 | spin_lock(&np->lock); | 2174 | spin_lock(&np->lock); |
@@ -2399,19 +2412,8 @@ static struct net_device_stats *get_stats(struct net_device *dev) | |||
2399 | #ifdef CONFIG_NET_POLL_CONTROLLER | 2412 | #ifdef CONFIG_NET_POLL_CONTROLLER |
2400 | static void natsemi_poll_controller(struct net_device *dev) | 2413 | static void natsemi_poll_controller(struct net_device *dev) |
2401 | { | 2414 | { |
2402 | struct netdev_private *np = netdev_priv(dev); | ||
2403 | |||
2404 | disable_irq(dev->irq); | 2415 | disable_irq(dev->irq); |
2405 | 2416 | intr_handler(dev->irq, dev); | |
2406 | /* | ||
2407 | * A real interrupt might have already reached us at this point | ||
2408 | * but NAPI might still haven't called us back. As the interrupt | ||
2409 | * status register is cleared by reading, we should prevent an | ||
2410 | * interrupt loss in this case... | ||
2411 | */ | ||
2412 | if (!np->intr_status) | ||
2413 | intr_handler(dev->irq, dev); | ||
2414 | |||
2415 | enable_irq(dev->irq); | 2417 | enable_irq(dev->irq); |
2416 | } | 2418 | } |
2417 | #endif | 2419 | #endif |