[PATCH] b43legacy: Fix failure in rate-adjustment mechanism
A coding error present since b43legacy was incorporated into the
kernel has prevented the driver from using the rate-setting mechanism
of mac80211. The driver has been forced to remain at a 1 Mb/s rate.
Signed-off-by: Larry Finger [EMAIL PROTECTED]
Cc: Stable [EMAIL PROTECTED] [2.6.26], [2.6.25]
---
John,
Although this is not strictly a regression, it is a bug. I hope
it can be sent to 2.6.27.
Thanks,
Larry
---
Index: wireless-testing/drivers/net/wireless/b43legacy/xmit.c
===
--- wireless-testing.orig/drivers/net/wireless/b43legacy/xmit.c
+++ wireless-testing/drivers/net/wireless/b43legacy/xmit.c
@@ -629,7 +629,7 @@ void b43legacy_handle_hwtxstatus(struct
status.pm_indicated = !!(tmp 0x80);
status.intermediate = !!(tmp 0x40);
status.for_ampdu = !!(tmp 0x20);
- status.acked = !!(tmp 0x02);
+ status.acked = tmp 0x01;
b43legacy_handle_txstatus(dev, status);
}
Index: wireless-testing/drivers/net/wireless/b43legacy/main.c
===
--- wireless-testing.orig/drivers/net/wireless/b43legacy/main.c
+++ wireless-testing/drivers/net/wireless/b43legacy/main.c
@@ -744,7 +744,7 @@ static void handle_irq_transmit_status(s
while (1) {
v0 = b43legacy_read32(dev, B43legacy_MMIO_XMITSTAT_0);
- if (!(v0 0x0001))
+ if (!v0)
break;
v1 = b43legacy_read32(dev, B43legacy_MMIO_XMITSTAT_1);
@@ -758,7 +758,7 @@ static void handle_irq_transmit_status(s
stat.pm_indicated = !!(tmp 0x0080);
stat.intermediate = !!(tmp 0x0040);
stat.for_ampdu = !!(tmp 0x0020);
- stat.acked = !!(tmp 0x0002);
+ stat.acked = tmp 0x0001;
b43legacy_handle_txstatus(dev, stat);
}
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
On Saturday 06 September 2008 20:34:02 Larry Finger wrote: A coding error present since b43legacy was incorporated into the kernel has prevented the driver from using the rate-setting mechanism of mac80211. The driver has been forced to remain at a 1 Mb/s rate. Does version3 firmware have a different bitlayout for the status? Although this is not strictly a regression, it is a bug. I hope it can be sent to 2.6.27. The new rules don't allow us to fix bugs after the merge window. Only regressions. -- Greetings Michael. ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
Michael Buesch wrote: On Saturday 06 September 2008 20:34:02 Larry Finger wrote: A coding error present since b43legacy was incorporated into the kernel has prevented the driver from using the rate-setting mechanism of mac80211. The driver has been forced to remain at a 1 Mb/s rate. Does version3 firmware have a different bitlayout for the status? It seems so. I found this because I was not getting any acks back to net/mac80211/rc80211_pid_algo.c. I then reviewed the V3 specs, found that bit 0, not bit 1, contained the ack. Test prints confirmed that result. With this patch, both my BCM4306/2 and BCM4303 reach the maximum rate. With the current code, 54 Mb/s is not as fast as 36 Mb/s, but at least the algorithm is working. Larry ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
On Saturday 06 September 2008 20:52:53 Larry Finger wrote: Michael Buesch wrote: On Saturday 06 September 2008 20:34:02 Larry Finger wrote: A coding error present since b43legacy was incorporated into the kernel has prevented the driver from using the rate-setting mechanism of mac80211. The driver has been forced to remain at a 1 Mb/s rate. Does version3 firmware have a different bitlayout for the status? It seems so. I found this because I was not getting any acks back to net/mac80211/rc80211_pid_algo.c. I then reviewed the V3 specs, found that bit 0, not bit 1, contained the ack. Test prints confirmed that result. With this patch, both my BCM4306/2 and BCM4303 reach the maximum rate. With the current code, 54 Mb/s is not as fast as 36 Mb/s, but at least the algorithm is working. Yeah ok. I just asked, because it seems the _whole_ flags bitfield is rightshifted by one (so the other flags are wrong, too. See the intermediate flag) -- Greetings Michael. ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
On Saturday 06 September 2008 20:57:50 Johannes Berg wrote: On Sat, 2008-09-06 at 20:55 +0200, Michael Buesch wrote: On Saturday 06 September 2008 20:52:53 Larry Finger wrote: Michael Buesch wrote: On Saturday 06 September 2008 20:34:02 Larry Finger wrote: A coding error present since b43legacy was incorporated into the kernel has prevented the driver from using the rate-setting mechanism of mac80211. The driver has been forced to remain at a 1 Mb/s rate. Does version3 firmware have a different bitlayout for the status? It seems so. I found this because I was not getting any acks back to net/mac80211/rc80211_pid_algo.c. I then reviewed the V3 specs, found that bit 0, not bit 1, contained the ack. Test prints confirmed that result. With this patch, both my BCM4306/2 and BCM4303 reach the maximum rate. With the current code, 54 Mb/s is not as fast as 36 Mb/s, but at least the algorithm is working. Yeah ok. I just asked, because it seems the _whole_ flags bitfield is rightshifted by one (so the other flags are wrong, too. See the intermediate flag) It is, this isn't really a difference between the two but a result of you shifting it up/down due to the tx status via dma queue vs. tx status via registers thing. Yeah, that's the point. larry's patch modified both the register and dmaqueue mechanism. I think the register mechanism might be correct as-is (Or is it even dead code and it's not used by any legacy device?) -- Greetings Michael. ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
On Sat, 2008-09-06 at 20:55 +0200, Michael Buesch wrote: On Saturday 06 September 2008 20:52:53 Larry Finger wrote: Michael Buesch wrote: On Saturday 06 September 2008 20:34:02 Larry Finger wrote: A coding error present since b43legacy was incorporated into the kernel has prevented the driver from using the rate-setting mechanism of mac80211. The driver has been forced to remain at a 1 Mb/s rate. Does version3 firmware have a different bitlayout for the status? It seems so. I found this because I was not getting any acks back to net/mac80211/rc80211_pid_algo.c. I then reviewed the V3 specs, found that bit 0, not bit 1, contained the ack. Test prints confirmed that result. With this patch, both my BCM4306/2 and BCM4303 reach the maximum rate. With the current code, 54 Mb/s is not as fast as 36 Mb/s, but at least the algorithm is working. Yeah ok. I just asked, because it seems the _whole_ flags bitfield is rightshifted by one (so the other flags are wrong, too. See the intermediate flag) It is, this isn't really a difference between the two but a result of you shifting it up/down due to the tx status via dma queue vs. tx status via registers thing. johannes signature.asc Description: This is a digitally signed message part ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
Michael Buesch wrote:
On Saturday 06 September 2008 20:57:50 Johannes Berg wrote:
It is, this isn't really a difference between the two but a result of
you shifting it up/down due to the tx status via dma queue vs. tx status
via registers thing.
Yeah, that's the point. larry's patch modified both the register and dmaqueue
mechanism. I think the register mechanism might be correct as-is (Or is it
even
dead code and it's not used by any legacy device?)
I modified the patch to add printouts in both paths as shown below:
Index: linux-2.6/drivers/net/wireless/b43legacy/xmit.c
===
--- linux-2.6.orig/drivers/net/wireless/b43legacy/xmit.c
+++ linux-2.6/drivers/net/wireless/b43legacy/xmit.c
@@ -631,7 +631,8 @@ void b43legacy_handle_hwtxstatus(struct
status.pm_indicated = !!(tmp 0x80);
status.intermediate = !!(tmp 0x40);
status.for_ampdu = !!(tmp 0x20);
- status.acked = !!(tmp 0x02);
+ status.acked = tmp 0x01;
+ printk(KERN_INFO b43legacy: In b43legacy_handle_hwtxstatus,
hw-flags = 0x%X\n, hw-flags);
b43legacy_handle_txstatus(dev, status);
}
Index: linux-2.6/drivers/net/wireless/b43legacy/main.c
===
--- linux-2.6.orig/drivers/net/wireless/b43legacy/main.c
+++ linux-2.6/drivers/net/wireless/b43legacy/main.c
@@ -744,7 +744,7 @@ static void handle_irq_transmit_status(s
while (1) {
v0 = b43legacy_read32(dev, B43legacy_MMIO_XMITSTAT_0);
- if (!(v0 0x0001))
+ if (!v0)
break;
v1 = b43legacy_read32(dev, B43legacy_MMIO_XMITSTAT_1);
@@ -752,13 +752,14 @@ static void handle_irq_transmit_status(s
stat.seq = (v1 0x);
stat.phy_stat = ((v1 0x00FF) 16);
tmp = (v0 0x);
+ printk(KERN_INFO b43legacy: In
handle_irq_transmit_status, tmp 0x%X\n, tmp);
stat.frame_count = ((tmp 0xF000) 12);
stat.rts_count = ((tmp 0x0F00) 8);
stat.supp_reason = ((tmp 0x001C) 2);
stat.pm_indicated = !!(tmp 0x0080);
stat.intermediate = !!(tmp 0x0040);
stat.for_ampdu = !!(tmp 0x0020);
- stat.acked = !!(tmp 0x0002);
+ stat.acked = tmp 0x0001;
b43legacy_handle_txstatus(dev, stat);
}
What I see are lots of
b43legacy: In b43legacy_handle_hwtxstatus, hw-flags = 0x1
and this is the only one that ever triggered. ATM, I'm not sure why
handle_irq_transmit_status() is not called.
Larry
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
On Sat, 2008-09-06 at 14:36 -0500, Larry Finger wrote:
What I see are lots of
b43legacy: In b43legacy_handle_hwtxstatus, hw-flags = 0x1
and this is the only one that ever triggered. ATM, I'm not sure why
handle_irq_transmit_status() is not called.
The mechanism depends on the card revision, but according to
drivers/net/wireless/b43legacy/dma.c it's always via the dma/pio
mechanism for legacy cards:
if (dev-dev-id.revision 5) {
ring = b43legacy_setup_dmaring(dev, 3, 0, type);
if (!ring)
goto err_destroy_rx0;
dma-rx_ring3 = ring;
}
johannes
signature.asc
Description: This is a digitally signed message part
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
Johannes Berg wrote:
The mechanism depends on the card revision, but according to
drivers/net/wireless/b43legacy/dma.c it's always via the dma/pio
mechanism for legacy cards:
if (dev-dev-id.revision 5) {
ring = b43legacy_setup_dmaring(dev, 3, 0, type);
if (!ring)
goto err_destroy_rx0;
dma-rx_ring3 = ring;
}
In the V3 specs, I found
Transmit Status
When this interrupt is set, the retrieve the TransmitStatus. Note that
on cores with revision 5, the last DMA controller or PIO queue can
also also get the DMA recieve done interrupt, which also triggers the
TransmitStatus retrieval process. The driver should be prepared to
deal with both interrupts at any time, on any revision. In AP mode,
this interrupt also initiates the sending of powersave responses.
The implication is that the interrupt will only be generated if we use
the last (i.e. #5) DMA controller. As we are only using #3, no
interrupts and handle_irq_status() is dead code.
Larry
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
On Sat, 2008-09-06 at 14:59 -0500, Larry Finger wrote: In the V3 specs, I found Transmit Status When this interrupt is set, the retrieve the TransmitStatus. Note that on cores with revision 5, the last DMA controller or PIO queue can also also get the DMA recieve done interrupt, which also triggers the TransmitStatus retrieval process. The driver should be prepared to deal with both interrupts at any time, on any revision. In AP mode, this interrupt also initiates the sending of powersave responses. Yeah, this isn't entirely correct, when the core revision is 5 then the register-based TX status doesn't actually exist and the firmware always uses the FIFO-based mechanism. The implication is that the interrupt will only be generated if we use the last (i.e. #5) DMA controller. As we are only using #3, no interrupts and handle_irq_status() is dead code. Right, only core revisions 2 and 4 are supported here. johannes signature.asc Description: This is a digitally signed message part ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43legacy: Fix failure in rate-adjustment mechanism
On Sat, Sep 06, 2008 at 08:41:05PM +0200, Michael Buesch wrote: On Saturday 06 September 2008 20:34:02 Larry Finger wrote: A coding error present since b43legacy was incorporated into the kernel has prevented the driver from using the rate-setting mechanism of mac80211. The driver has been forced to remain at a 1 Mb/s rate. Does version3 firmware have a different bitlayout for the status? Although this is not strictly a regression, it is a bug. I hope it can be sent to 2.6.27. The new rules don't allow us to fix bugs after the merge window. Only regressions. I imagine the powers that be would claim it isn't a new rule, but it is a new enforcement policy... John ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
