* bcm43xx-mac80211: a fix for the shared interrupt problem
@ 2007-07-28 3:36 Larry Finger
2007-07-28 16:48 ` Michael Buesch
0 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2007-07-28 3:36 UTC (permalink / raw)
To: Michael Buesch; +Cc: Bcm43xx-dev, linux-wireless
Michael,
I was in the right area with my previous fix to the "irq 11: nobody cared"
message. I think for shared interrupts, skipping out if the status is not
BCM43xx_STAT_STARTED is too severe. I added a message printing out the status
whenever it skipped out, and found many thousands of interrupts with the
status equal to BCM43xx_STAT_INITIALIZED. The patch below makes the necessary
change and modifies a couple of asserts that depend on it.
This patch has been tested on BCM4306 and BCM4318 with shared interrupts,
and BCM4311 without.
Thanks,
Larry
----
Index: wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
===================================================================
--- wireless-mb.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
+++ wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
@@ -1374,7 +1374,7 @@ static void bcm43xx_interrupt_tasklet(st
spin_lock_irqsave(&dev->wl->irq_lock, flags);
- assert(bcm43xx_status(dev) == BCM43xx_STAT_STARTED);
+ assert(bcm43xx_status(dev) >= BCM43xx_STAT_INITIALIZED);
reason = dev->irq_reason;
for (i = 0; i < ARRAY_SIZE(dma_reason); i++) {
@@ -1512,7 +1512,7 @@ static irqreturn_t bcm43xx_interrupt_han
spin_lock(&dev->wl->irq_lock);
- if (bcm43xx_status(dev) < BCM43xx_STAT_STARTED)
+ if (bcm43xx_status(dev) < BCM43xx_STAT_INITIALIZED)
goto out;
reason = bcm43xx_read32(dev, BCM43xx_MMIO_GEN_IRQ_REASON);
if (reason == 0xffffffff) /* shared IRQ */
@@ -1522,7 +1522,7 @@ static irqreturn_t bcm43xx_interrupt_han
if (!reason)
goto out;
- assert(bcm43xx_status(dev) == BCM43xx_STAT_STARTED);
+ assert(bcm43xx_status(dev) >= BCM43xx_STAT_INITIALIZED);
dev->dma_reason[0] = bcm43xx_read32(dev, BCM43xx_MMIO_DMA0_REASON)
& 0x0001DC00;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bcm43xx-mac80211: a fix for the shared interrupt problem
2007-07-28 3:36 bcm43xx-mac80211: a fix for the shared interrupt problem Larry Finger
@ 2007-07-28 16:48 ` Michael Buesch
2007-07-28 20:53 ` Larry Finger
0 siblings, 1 reply; 6+ messages in thread
From: Michael Buesch @ 2007-07-28 16:48 UTC (permalink / raw)
To: Larry Finger; +Cc: Bcm43xx-dev, linux-wireless
On Saturday 28 July 2007 05:36:04 Larry Finger wrote:
> Michael,
>
> I was in the right area with my previous fix to the "irq 11: nobody cared"
> message. I think for shared interrupts, skipping out if the status is not
> BCM43xx_STAT_STARTED is too severe. I added a message printing out the status
> whenever it skipped out, and found many thousands of interrupts with the
> status equal to BCM43xx_STAT_INITIALIZED. The patch below makes the necessary
> change and modifies a couple of asserts that depend on it.
>
> This patch has been tested on BCM4306 and BCM4318 with shared interrupts,
> and BCM4311 without.
>
> Thanks,
>
> Larry
> ----
>
>
> Index: wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> ===================================================================
> --- wireless-mb.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> +++ wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> @@ -1374,7 +1374,7 @@ static void bcm43xx_interrupt_tasklet(st
>
> spin_lock_irqsave(&dev->wl->irq_lock, flags);
>
> - assert(bcm43xx_status(dev) == BCM43xx_STAT_STARTED);
> + assert(bcm43xx_status(dev) >= BCM43xx_STAT_INITIALIZED);
>
> reason = dev->irq_reason;
> for (i = 0; i < ARRAY_SIZE(dma_reason); i++) {
> @@ -1512,7 +1512,7 @@ static irqreturn_t bcm43xx_interrupt_han
>
> spin_lock(&dev->wl->irq_lock);
>
> - if (bcm43xx_status(dev) < BCM43xx_STAT_STARTED)
> + if (bcm43xx_status(dev) < BCM43xx_STAT_INITIALIZED)
> goto out;
> reason = bcm43xx_read32(dev, BCM43xx_MMIO_GEN_IRQ_REASON);
> if (reason == 0xffffffff) /* shared IRQ */
> @@ -1522,7 +1522,7 @@ static irqreturn_t bcm43xx_interrupt_han
> if (!reason)
> goto out;
>
> - assert(bcm43xx_status(dev) == BCM43xx_STAT_STARTED);
> + assert(bcm43xx_status(dev) >= BCM43xx_STAT_INITIALIZED);
>
> dev->dma_reason[0] = bcm43xx_read32(dev, BCM43xx_MMIO_DMA0_REASON)
> & 0x0001DC00;
>
>
That's not the right bugfix.
If the interface is not STAT_STARTED, it should _not_ generate IRQs.
The bug is elsewhere in the init routines.
Please find out which IRQ bits are generated for these spurious IRQs.
--
Greetings Michael.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bcm43xx-mac80211: a fix for the shared interrupt problem
2007-07-28 16:48 ` Michael Buesch
@ 2007-07-28 20:53 ` Larry Finger
2007-07-28 23:03 ` Michael Buesch
0 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2007-07-28 20:53 UTC (permalink / raw)
To: Michael Buesch; +Cc: linux-wireless, Bcm43xx-dev
Michael Buesch wrote:
>
> That's not the right bugfix.
> If the interface is not STAT_STARTED, it should _not_ generate IRQs.
> The bug is elsewhere in the init routines.
> Please find out which IRQ bits are generated for these spurious IRQs.
>
I trapped those entries where the interface was not STAT_STARTED and got 2 such interrupts, each
with the IRQ bits set to 0x00000001 (READY). They occur between the "30-bit DMA initialized" and the
"Wireless interface started" messages.
Larry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bcm43xx-mac80211: a fix for the shared interrupt problem
2007-07-28 20:53 ` Larry Finger
@ 2007-07-28 23:03 ` Michael Buesch
2007-07-28 23:58 ` Larry Finger
0 siblings, 1 reply; 6+ messages in thread
From: Michael Buesch @ 2007-07-28 23:03 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, Bcm43xx-dev
On Saturday 28 July 2007 22:53:21 Larry Finger wrote:
> Michael Buesch wrote:
> >
> > That's not the right bugfix.
> > If the interface is not STAT_STARTED, it should _not_ generate IRQs.
> > The bug is elsewhere in the init routines.
> > Please find out which IRQ bits are generated for these spurious IRQs.
> >
>
> I trapped those entries where the interface was not STAT_STARTED and got 2 such interrupts, each
> with the IRQ bits set to 0x00000001 (READY). They occur between the "30-bit DMA initialized" and the
> "Wireless interface started" messages.
Ok, seems like we are missing some dummy reads to drain
this IRQ. I'll look at this tomorrow.
Thanks for tracking this down.
--
Greetings Michael.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bcm43xx-mac80211: a fix for the shared interrupt problem
2007-07-28 23:03 ` Michael Buesch
@ 2007-07-28 23:58 ` Larry Finger
2007-07-29 8:54 ` Michael Buesch
0 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2007-07-28 23:58 UTC (permalink / raw)
To: Michael Buesch; +Cc: linux-wireless, Bcm43xx-dev
Michael Buesch wrote:
>
> Ok, seems like we are missing some dummy reads to drain
> this IRQ. I'll look at this tomorrow.
> Thanks for tracking this down.
As we shouldn't get any interrupts until STAT_STARTED is reached, I wondered why interrupts were
enabled so early. As far as I can tell, none of the steps from where interrupts are enabled to where
STAT_STARTED is set depend on having them enabled; therefore, I tried a patch that delayed the
enabling of interrupts until after we had reached STAT_STARTED. It works for all my systems.
I'm beginning to think that the shared interrupts have nothing to do with the problem, but it is
more likely caused by peculiarities in the PCMCIA bridge in my ancient laptop.
Larry
Index: wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
===================================================================
--- wireless-mb.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
+++ wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
@@ -3014,13 +3014,13 @@ static int bcm43xx_wireless_core_start(s
dev->dev->irq);
goto out;
}
- bcm43xx_interrupt_enable(dev, dev->irq_savedstate);
bcm43xx_mac_enable(dev);
bcm43xx_periodic_tasks_setup(dev);
ieee80211_start_queues(dev->wl->hw);
bcm43xx_set_status(dev, BCM43xx_STAT_STARTED);
+ bcm43xx_interrupt_enable(dev, dev->irq_savedstate);
bcmdbg(dev->wl, "Wireless interface started\n");
out:
return err;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: bcm43xx-mac80211: a fix for the shared interrupt problem
2007-07-28 23:58 ` Larry Finger
@ 2007-07-29 8:54 ` Michael Buesch
0 siblings, 0 replies; 6+ messages in thread
From: Michael Buesch @ 2007-07-29 8:54 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, Bcm43xx-dev
On Sunday 29 July 2007 01:58, Larry Finger wrote:
> Michael Buesch wrote:
> >
> > Ok, seems like we are missing some dummy reads to drain
> > this IRQ. I'll look at this tomorrow.
> > Thanks for tracking this down.
>
> As we shouldn't get any interrupts until STAT_STARTED is reached, I wondered why interrupts were
> enabled so early. As far as I can tell, none of the steps from where interrupts are enabled to where
> STAT_STARTED is set depend on having them enabled; therefore, I tried a patch that delayed the
> enabling of interrupts until after we had reached STAT_STARTED. It works for all my systems.
>
> I'm beginning to think that the shared interrupts have nothing to do with the problem, but it is
> more likely caused by peculiarities in the PCMCIA bridge in my ancient laptop.
>
> Larry
>
> Index: wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> ===================================================================
> --- wireless-mb.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> +++ wireless-mb/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> @@ -3014,13 +3014,13 @@ static int bcm43xx_wireless_core_start(s
> dev->dev->irq);
> goto out;
> }
> - bcm43xx_interrupt_enable(dev, dev->irq_savedstate);
> bcm43xx_mac_enable(dev);
>
> bcm43xx_periodic_tasks_setup(dev);
>
> ieee80211_start_queues(dev->wl->hw);
> bcm43xx_set_status(dev, BCM43xx_STAT_STARTED);
> + bcm43xx_interrupt_enable(dev, dev->irq_savedstate);
> bcmdbg(dev->wl, "Wireless interface started\n");
> out:
> return err;
>
Yes, that seems to be the correct fix.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-07-29 8:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-28 3:36 bcm43xx-mac80211: a fix for the shared interrupt problem Larry Finger
2007-07-28 16:48 ` Michael Buesch
2007-07-28 20:53 ` Larry Finger
2007-07-28 23:03 ` Michael Buesch
2007-07-28 23:58 ` Larry Finger
2007-07-29 8:54 ` Michael Buesch
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).