[PATCH] bcm43xx: Drain TX status before starting IRQs

[PATCH] bcm43xx: Drain TX status before starting IRQs

[Michael Buesch]

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.

Signed-off-by: Michael Buesch <mb@bu3sch.de>

--

Please consider also pushing this into the -stable tree.
The bug is not likely to trigger, but at least ben
triggered it in the past. Anyway, it can't hurt much to
drain the FIFO before running the device.

Note that this is diffed against 2.6.18.1 and not 2.6.18
as the diff prolog suggests. I just forgot to rename
the directory. ;)


Index: linux-2.6.18/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
--- linux-2.6.18.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c	2006-10-15 21:10:37.000000000 +0200
+++ linux-2.6.18/drivers/net/wireless/bcm43xx/bcm43xx_main.c	2006-10-19 17:17:16.000000000 +0200
@@ -1463,6 +1463,21 @@ static void handle_irq_transmit_status(s
 	}
 }
 
+static void drain_txstatus_queue(struct bcm43xx_private *bcm)
+{
+	u32 dummy;
+
+	/* 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);
@@ -3509,6 +3524,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.


-- 
Greetings Michael.

Re: [PATCH] bcm43xx: Drain TX status before starting IRQs

[Benjamin Herrenschmidt]

On Thu, 2006-10-19 at 17:29 +0200, Michael Buesch wrote:
> 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.
> 
> Signed-off-by: Michael Buesch <mb-fseUSCV1ubazQB+pC5nmwQ@public.gmane.org>

Great, thanks. Note that I haven't yet hit the bug since I updated the
firmware, so it could be a mix of firmware and that problem, though it's
not been long enough to be a clear results.

In any case, that patch should go in.

Cheers,
Ben.

Re: [PATCH] bcm43xx: Drain TX status before starting IRQs

[Larry Finger]

Michael Buesch wrote:
> 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.
> 
> Signed-off-by: Michael Buesch <mb@bu3sch.de>
> 
> --
> 
> Please consider also pushing this into the -stable tree.
> The bug is not likely to trigger, but at least ben
> triggered it in the past. Anyway, it can't hurt much to
> drain the FIFO before running the device.
> 
> Note that this is diffed against 2.6.18.1 and not 2.6.18
> as the diff prolog suggests. I just forgot to rename
> the directory. ;)

Has anyone used this patch, particularly with WPA encryption? When I try it, wpa_supplicant 
immediately uses 90+% of the cpu and never actually authenticates with my AP. I wonder if it is 
something with my system.

Thanks,

Larry

Re: [PATCH] bcm43xx: Drain TX status before starting IRQs

[Johannes Berg]

On Wed, 2006-11-01 at 23:46 -0600, Larry Finger wrote:

> Has anyone used this patch, particularly with WPA encryption? When I try it, wpa_supplicant 
> immediately uses 90+% of the cpu and never actually authenticates with my AP. I wonder if it is 
> something with my system.

Unless wpa supplicant is stupid [1] this cannot cause it any problems.

johannes

[1] by that, I mean sending out frames, turning off and on the
interface, and then hoping to get a transmit status for frames sent
before off/on cycle.

Re: [PATCH] bcm43xx: Drain TX status before starting IRQs

[Jouni Malinen]

On Thu, Nov 02, 2006 at 09:45:46AM +0100, Johannes Berg wrote:
> On Wed, 2006-11-01 at 23:46 -0600, Larry Finger wrote:
> > Has anyone used this patch, particularly with WPA encryption? When I try it, wpa_supplicant 
> > immediately uses 90+% of the cpu and never actually authenticates with my AP. I wonder if it is 
> > something with my system.

What does wpa_supplicant do here? Have you looked at the debug log? Is
it in some kind of busy loop doing something?

> Unless wpa supplicant is stupid [1] this cannot cause it any problems.
> 
> johannes
> 
> [1] by that, I mean sending out frames, turning off and on the
> interface, and then hoping to get a transmit status for frames sent
> before off/on cycle.

wpa_supplicant does not receive or query TX status (at least yet; in the
user space MLME case this is likely to change at some point).

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] bcm43xx: Drain TX status before starting IRQs

[Larry Finger]

Jouni Malinen wrote:
> On Thu, Nov 02, 2006 at 09:45:46AM +0100, Johannes Berg wrote:
>> On Wed, 2006-11-01 at 23:46 -0600, Larry Finger wrote:
>>> Has anyone used this patch, particularly with WPA encryption? When I try it, wpa_supplicant 
>>> immediately uses 90+% of the cpu and never actually authenticates with my AP. I wonder if it is 
>>> something with my system.
> 
> What does wpa_supplicant do here? Have you looked at the debug log? Is
> it in some kind of busy loop doing something?

I started wpa_supplicant in the foreground with the -w and -ddd options. When the BCM4306 card was 
plugged in, the following was output (this is hand transcribed and may not be exact):

Added interface wlan0
Ignore event for foreign ifindex 8
l2_packet_receive - recvfrom: Network is down
Ignore event for foreign ifindex 8
State: DISCONNECTED -> SCANNING
Starting AP scan (specific SSID)
Scan SSID - hexdump_ascii (len = 6)
         6c 77 66 64 6a 66               lwfdjf
ioctl[SIOGSIWSCAN]: No such device
Failed to initiate AP scan.
Setting scan request: 10 sec 0 usec
Ignore event for foreign ifindex 8

At this point, the system was in the state described earlier. I could not kill wpa_supplicant nor do 
anything else, and had to power off to recover.

Larry

[PATCH] bcm43xx: Drain TX status before starting IRQs

[Larry Finger]

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.

Signed-off-by: Michael Buesch <mb@bu3sch.de>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Index: linux-2.6.18/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
--- linux-2.6.18.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ linux-2.6.18/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.

[PATCH] bcm43xx: Drain TX status before starting IRQs

[Larry Finger]

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.

Signed-off-by: Michael Buesch <mb@bu3sch.de>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

John,

Please apply this to wireless-2.6 and push it to 2.6.19. It has already
been sent to -stable for inclusion in 2.6.18.3. This patch replaces one
with the same name that was sent by Michael on October 19. It had a bug,
fixed in this version, that would lock up certain core revisions.

Larry

Index: wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ wireless-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -1467,6 +1467,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);
@@ -3569,6 +3586,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.