[patch 07/30] bcm43xx: Drain TX status before starting IRQs

[patch 07/30] bcm43xx: Drain TX status before starting IRQs

[Chris Wright]

[-- Attachment #1: bcm43xx-drain-tx-status-before-starting-irqs.patch --]
[-- Type: text/plain, Size: 1772 bytes --]

-stable review patch.  If anyone has any objections, please let us know.
------------------

From: Michael Buesch <mb@bu3sch.de>

Drain the Microcode TX-status-FIFO before we enable IRQs.
This is required, because the FIFO may still have entries left
from a previous run. Those would immediately fire after enabling
IRQs and would lead to an oops in the DMA TXstatus handling code.

Cc: "John W. Linville" <linville@tuxdriver.com>
Signed-off-by: Michael Buesch <mb@bu3sch.de>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Chris Wright <chrisw@sous-sol.org>
---
 drivers/net/wireless/bcm43xx/bcm43xx_main.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

--- linux-2.6.18.2.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ linux-2.6.18.2/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -1463,6 +1463,23 @@ static void handle_irq_transmit_status(s
 	}
 }
 
+static void drain_txstatus_queue(struct bcm43xx_private *bcm)
+{
+	u32 dummy;
+
+	if (bcm->current_core->rev < 5)
+		return;
+	/* Read all entries from the microcode TXstatus FIFO
+	 * and throw them away.
+	 */
+	while (1) {
+		dummy = bcm43xx_read32(bcm, BCM43xx_MMIO_XMITSTAT_0);
+		if (!dummy)
+			break;
+		dummy = bcm43xx_read32(bcm, BCM43xx_MMIO_XMITSTAT_1);
+	}
+}
+
 static void bcm43xx_generate_noise_sample(struct bcm43xx_private *bcm)
 {
 	bcm43xx_shm_write16(bcm, BCM43xx_SHM_SHARED, 0x408, 0x7F7F);
@@ -3517,6 +3534,7 @@ int bcm43xx_select_wireless_core(struct 
 	bcm43xx_macfilter_clear(bcm, BCM43xx_MACFILTER_ASSOC);
 	bcm43xx_macfilter_set(bcm, BCM43xx_MACFILTER_SELF, (u8 *)(bcm->net_dev->dev_addr));
 	bcm43xx_security_init(bcm);
+	drain_txstatus_queue(bcm);
 	ieee80211softmac_start(bcm->net_dev);
 
 	/* Let's go! Be careful after enabling the IRQs.

--

Re: [patch 07/30] bcm43xx: Drain TX status before starting IRQs

[Larry Finger]

Chris Wright wrote:
> -stable review patch.  If anyone has any objections, please let us know.
> ------------------
> 
> From: Michael Buesch <mb@bu3sch.de>
> 
> Drain the Microcode TX-status-FIFO before we enable IRQs.
> This is required, because the FIFO may still have entries left
> from a previous run. Those would immediately fire after enabling
> IRQs and would lead to an oops in the DMA TXstatus handling code.
> 
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Signed-off-by: Michael Buesch <mb@bu3sch.de>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
> ---

Chris,

We have a report of a regression between 2.6.19-rc3 and -rc5. As this patch seems to be the only one 
that could cause the problem, please pull it from -stable while we sort out the difficulty.

Thanks,

Larry

Re: [patch 07/30] bcm43xx: Drain TX status before starting IRQs

[Chris Wright]

* Larry Finger (Larry.Finger@lwfinger.net) wrote:
> We have a report of a regression between 2.6.19-rc3 and -rc5. As this patch 
> seems to be the only one that could cause the problem, please pull it from 
> -stable while we sort out the difficulty.

Thanks a lot for the heads up Larry, dropping this one.
-chris

Re: [patch 07/30] bcm43xx: Drain TX status before starting IRQs

[Larry Finger]

Chris Wright wrote:
> -stable review patch.  If anyone has any objections, please let us know.
> ------------------
> 
> From: Michael Buesch <mb@bu3sch.de>
> 
> Drain the Microcode TX-status-FIFO before we enable IRQs.
> This is required, because the FIFO may still have entries left
> from a previous run. Those would immediately fire after enabling
> IRQs and would lead to an oops in the DMA TXstatus handling code.
> 
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Signed-off-by: Michael Buesch <mb@bu3sch.de>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
> ---

Chris,

The regression turns out to be a locking problem involving bcm43xx, wpa_supplicant, and 
NetworkManager. The exact cause is unknown; however, this patch is clearly not the problem. Please 
reinstate it for inclusion in -stable.

Thanks,

Larry

Re: [patch 07/30] bcm43xx: Drain TX status before starting IRQs

[Chris Wright]

* Larry Finger (Larry.Finger@lwfinger.net) wrote:
> The regression turns out to be a locking problem involving bcm43xx, 
> wpa_supplicant, and NetworkManager. The exact cause is unknown; however, 
> this patch is clearly not the problem. Please reinstate it for inclusion in 
> -stable.

Thanks for the follow-up, Larry.  It's queued for next -stable.

thanks,
-chris

Re: [patch 07/30] bcm43xx: Drain TX status before starting IRQs

[Dan Williams]

On Sat, 2006-11-18 at 13:06 -0600, Larry Finger wrote:
> Chris Wright wrote:
> > -stable review patch.  If anyone has any objections, please let us know.
> > ------------------
> > 
> > From: Michael Buesch <mb@bu3sch.de>
> > 
> > Drain the Microcode TX-status-FIFO before we enable IRQs.
> > This is required, because the FIFO may still have entries left
> > from a previous run. Those would immediately fire after enabling
> > IRQs and would lead to an oops in the DMA TXstatus handling code.
> > 
> > Cc: "John W. Linville" <linville@tuxdriver.com>
> > Signed-off-by: Michael Buesch <mb@bu3sch.de>
> > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> > Signed-off-by: Chris Wright <chrisw@sous-sol.org>
> > ---
> 
> Chris,
> 
> The regression turns out to be a locking problem involving bcm43xx, wpa_supplicant, and 
> NetworkManager. The exact cause is unknown; however, this patch is clearly not the problem. Please 
> reinstate it for inclusion in -stable.

NM should be using wpa_supplicant underneath.  But depending on the NM
version, NM may be issuing any one of SIWENCODE (only to clear keys),
[S|G]IWSCAN, GIWRANGE, GIWAP, [S|G]IWMODE, [S|G]IWFREQ.  Mainly, NM
cleans up after wpa_supplicant, gets information about the current
connection, and does scans.  All other connection setup and handling is
done by wpa_supplicant.  But note that NM will do any of the above
operations at any time, no matter what wpa_supplicant is doing at that
time.  So the locking in the driver needs to be right, but it should be
right anyway regardless of whether either one or both of NM and
wpa_supplicant is in the picture...

Dan

> Thanks,
> 
> Larry
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html