* [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
@ 2016-05-15 14:23 Adrian Chadd
2016-05-15 14:32 ` Michael Büsch
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Adrian Chadd @ 2016-05-15 14:23 UTC (permalink / raw)
To: b43-dev; +Cc: linux-wireless
Author: Adrian Chadd <adrian@freebsd.org>
Date: Sun May 15 07:15:54 2016 -0700
[b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
Check the current PHY operating mode (gmode) to see if we should
fall back from 6MB OFDM to 11MB CCK. For 5GHz operation this isn't
allowed.
Note, the fallback lookup is only done for RTS rates; normal fallback
rates are done via mac80211 and aren't affected by this change.
Signed-off-by: Adrian Chadd <adrian@freebsd.org>
diff --git a/drivers/net/wireless/broadcom/b43/xmit.c
b/drivers/net/wireless/broadcom/b43/xmit.c
index f620126..fbf0e92 100644
--- a/drivers/net/wireless/broadcom/b43/xmit.c
+++ b/drivers/net/wireless/broadcom/b43/xmit.c
@@ -205,7 +205,7 @@ static u16 b43_generate_tx_phy_ctl1(struct
b43_wldev *dev, u8 bitrate)
return control;
}
-static u8 b43_calc_fallback_rate(u8 bitrate)
+static u8 b43_calc_fallback_rate(u8 bitrate, int gmode)
{
switch (bitrate) {
case B43_CCK_RATE_1MB:
@@ -216,8 +216,16 @@ static u8 b43_calc_fallback_rate(u8 bitrate)
return B43_CCK_RATE_2MB;
case B43_CCK_RATE_11MB:
return B43_CCK_RATE_5MB;
+
+ /*
+ * Don't just fallback to CCK; it may be in 5GHz operation
+ * and falling back to CCK won't work out very well.
+ */
case B43_OFDM_RATE_6MB:
- return B43_CCK_RATE_5MB;
+ if (gmode)
+ return B43_CCK_RATE_5MB;
+ else
+ return B43_OFDM_RATE_6MB;
case B43_OFDM_RATE_9MB:
return B43_OFDM_RATE_6MB;
case B43_OFDM_RATE_12MB:
@@ -438,7 +446,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
rts_rate = rts_cts_rate ? rts_cts_rate->hw_value :
B43_CCK_RATE_1MB;
rts_rate_ofdm = b43_is_ofdm_rate(rts_rate);
- rts_rate_fb = b43_calc_fallback_rate(rts_rate);
+ rts_rate_fb = b43_calc_fallback_rate(rts_rate, phy->gmode);
rts_rate_fb_ofdm = b43_is_ofdm_rate(rts_rate_fb);
if (rates[0].flags & IEEE80211_TX_RC_USE_CTS_PROTECT) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
2016-05-15 14:23 [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM Adrian Chadd
@ 2016-05-15 14:32 ` Michael Büsch
2016-05-16 11:41 ` Kalle Valo
2016-05-15 14:39 ` Michael Büsch
2016-05-15 17:09 ` Rafał Miłecki
2 siblings, 1 reply; 6+ messages in thread
From: Michael Büsch @ 2016-05-15 14:32 UTC (permalink / raw)
To: Adrian Chadd; +Cc: b43-dev, linux-wireless, Kalle Valo
[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]
On Sun, 15 May 2016 07:23:25 -0700
Adrian Chadd <adrian@freebsd.org> wrote:
> Author: Adrian Chadd <adrian@freebsd.org>
> Date: Sun May 15 07:15:54 2016 -0700
>
> [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
>
> Check the current PHY operating mode (gmode) to see if we should
> fall back from 6MB OFDM to 11MB CCK. For 5GHz operation this isn't
> allowed.
>
> Note, the fallback lookup is only done for RTS rates; normal fallback
> rates are done via mac80211 and aren't affected by this change.
>
> Signed-off-by: Adrian Chadd <adrian@freebsd.org>
This makes sense. I guess you tested this on actual hardware?
In the final submission please send this to Kalle Valo and add [PATCH]
to the subject, so tools can pick this up.
> diff --git a/drivers/net/wireless/broadcom/b43/xmit.c
> b/drivers/net/wireless/broadcom/b43/xmit.c
> index f620126..fbf0e92 100644
> --- a/drivers/net/wireless/broadcom/b43/xmit.c
> +++ b/drivers/net/wireless/broadcom/b43/xmit.c
> @@ -205,7 +205,7 @@ static u16 b43_generate_tx_phy_ctl1(struct
> b43_wldev *dev, u8 bitrate)
> return control;
> }
>
> -static u8 b43_calc_fallback_rate(u8 bitrate)
> +static u8 b43_calc_fallback_rate(u8 bitrate, int gmode)
> {
> switch (bitrate) {
> case B43_CCK_RATE_1MB:
> @@ -216,8 +216,16 @@ static u8 b43_calc_fallback_rate(u8 bitrate)
> return B43_CCK_RATE_2MB;
> case B43_CCK_RATE_11MB:
> return B43_CCK_RATE_5MB;
> +
> + /*
> + * Don't just fallback to CCK; it may be in 5GHz operation
> + * and falling back to CCK won't work out very well.
> + */
> case B43_OFDM_RATE_6MB:
> - return B43_CCK_RATE_5MB;
> + if (gmode)
> + return B43_CCK_RATE_5MB;
> + else
> + return B43_OFDM_RATE_6MB;
> case B43_OFDM_RATE_9MB:
> return B43_OFDM_RATE_6MB;
> case B43_OFDM_RATE_12MB:
> @@ -438,7 +446,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
>
> rts_rate = rts_cts_rate ? rts_cts_rate->hw_value :
> B43_CCK_RATE_1MB;
> rts_rate_ofdm = b43_is_ofdm_rate(rts_rate);
> - rts_rate_fb = b43_calc_fallback_rate(rts_rate);
> + rts_rate_fb = b43_calc_fallback_rate(rts_rate, phy->gmode);
> rts_rate_fb_ofdm = b43_is_ofdm_rate(rts_rate_fb);
>
> if (rates[0].flags & IEEE80211_TX_RC_USE_CTS_PROTECT) {
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
2016-05-15 14:23 [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM Adrian Chadd
2016-05-15 14:32 ` Michael Büsch
@ 2016-05-15 14:39 ` Michael Büsch
2016-05-15 14:44 ` Adrian Chadd
2016-05-15 17:09 ` Rafał Miłecki
2 siblings, 1 reply; 6+ messages in thread
From: Michael Büsch @ 2016-05-15 14:39 UTC (permalink / raw)
To: Adrian Chadd; +Cc: b43-dev, linux-wireless, Kalle Valo
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
On Sun, 15 May 2016 07:23:25 -0700
Adrian Chadd <adrian@freebsd.org> wrote:
> @@ -438,7 +446,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
>
> rts_rate = rts_cts_rate ? rts_cts_rate->hw_value :
> B43_CCK_RATE_1MB;
I just noticed that this is incorrect for 5GHz, too.
Maybe there are more such bugs.
--
Michael
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
2016-05-15 14:39 ` Michael Büsch
@ 2016-05-15 14:44 ` Adrian Chadd
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Chadd @ 2016-05-15 14:44 UTC (permalink / raw)
To: Michael Büsch; +Cc: b43-dev, linux-wireless, Kalle Valo
On 15 May 2016 at 07:39, Michael Büsch <m@bues.ch> wrote:
> On Sun, 15 May 2016 07:23:25 -0700
> Adrian Chadd <adrian@freebsd.org> wrote:
>
>> @@ -438,7 +446,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
>>
>> rts_rate = rts_cts_rate ? rts_cts_rate->hw_value :
>> B43_CCK_RATE_1MB;
>
> I just noticed that this is incorrect for 5GHz, too.
> Maybe there are more such bugs.
There are other bugs, yes. I'll do some more digging and submit
follow-up patches, including for the rts_rate selection.
I've tested this on BCM4321 on FreeBSD; it came up in a code review of
b43 whilst I was learning about the driver/hardware. I've tested the
change on FreeBSD and it works correctly. I'll setup a separate PC
linux environment next week to test things on.
-adrian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
2016-05-15 14:23 [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM Adrian Chadd
2016-05-15 14:32 ` Michael Büsch
2016-05-15 14:39 ` Michael Büsch
@ 2016-05-15 17:09 ` Rafał Miłecki
2 siblings, 0 replies; 6+ messages in thread
From: Rafał Miłecki @ 2016-05-15 17:09 UTC (permalink / raw)
To: Adrian Chadd; +Cc: b43-dev, linux-wireless
Hi,
Unfortunately your patch is malformed and I couldn't apply it locally.
It seems your mailer broke lines longer than 80 chars and there are
some wrong indents in the description. After fixing up some line break
I got following commit in my git log:
Author: Adrian Chadd <adrian@freebsd.org>
Date: Sun May 15 07:23:25 2016 -0700
don't unconditionally fall back to CCK if the rate is 6MB OFDM.
Author: Adrian Chadd <adrian@freebsd.org>
Date: Sun May 15 07:15:54 2016 -0700
[b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
Check the current PHY operating mode (gmode) to see if we should
fall back from 6MB OFDM to 11MB CCK. For 5GHz operation this isn't
allowed.
Note, the fallback lookup is only done for RTS rates; normal fallback
rates are done via mac80211 and aren't affected by this change.
Signed-off-by: Adrian Chadd <adrian@freebsd.org>
Please fixup these problems, send patch to yourself, save it and see
if you can apply it with "git am" this time. If that works, please
resend to the mailing list.
On 15 May 2016 at 16:23, Adrian Chadd <adrian@freebsd.org> wrote:
> Author: Adrian Chadd <adrian@freebsd.org>
> Date: Sun May 15 07:15:54 2016 -0700
>
> [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
Please use "b43:" prefix and fix indent.
> Check the current PHY operating mode (gmode) to see if we should
> fall back from 6MB OFDM to 11MB CCK. For 5GHz operation this isn't
> allowed.
>
> Note, the fallback lookup is only done for RTS rates; normal fallback
> rates are done via mac80211 and aren't affected by this change.
>
> Signed-off-by: Adrian Chadd <adrian@freebsd.org>
Fix indent.
> diff --git a/drivers/net/wireless/broadcom/b43/xmit.c
> b/drivers/net/wireless/broadcom/b43/xmit.c
> index f620126..fbf0e92 100644
> --- a/drivers/net/wireless/broadcom/b43/xmit.c
> +++ b/drivers/net/wireless/broadcom/b43/xmit.c
> @@ -205,7 +205,7 @@ static u16 b43_generate_tx_phy_ctl1(struct
> b43_wldev *dev, u8 bitrate)
> return control;
Unexpected line break.
> }
>
> -static u8 b43_calc_fallback_rate(u8 bitrate)
> +static u8 b43_calc_fallback_rate(u8 bitrate, int gmode)
> {
> switch (bitrate) {
> case B43_CCK_RATE_1MB:
> @@ -216,8 +216,16 @@ static u8 b43_calc_fallback_rate(u8 bitrate)
> return B43_CCK_RATE_2MB;
> case B43_CCK_RATE_11MB:
> return B43_CCK_RATE_5MB;
> +
> + /*
> + * Don't just fallback to CCK; it may be in 5GHz operation
> + * and falling back to CCK won't work out very well.
> + */
> case B43_OFDM_RATE_6MB:
> - return B43_CCK_RATE_5MB;
> + if (gmode)
> + return B43_CCK_RATE_5MB;
> + else
> + return B43_OFDM_RATE_6MB;
> case B43_OFDM_RATE_9MB:
> return B43_OFDM_RATE_6MB;
> case B43_OFDM_RATE_12MB:
> @@ -438,7 +446,7 @@ int b43_generate_txhdr(struct b43_wldev *dev,
>
> rts_rate = rts_cts_rate ? rts_cts_rate->hw_value :
> B43_CCK_RATE_1MB;
Unexpected line break.
> rts_rate_ofdm = b43_is_ofdm_rate(rts_rate);
> - rts_rate_fb = b43_calc_fallback_rate(rts_rate);
> + rts_rate_fb = b43_calc_fallback_rate(rts_rate, phy->gmode);
> rts_rate_fb_ofdm = b43_is_ofdm_rate(rts_rate_fb);
>
> if (rates[0].flags & IEEE80211_TX_RC_USE_CTS_PROTECT) {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM.
2016-05-15 14:32 ` Michael Büsch
@ 2016-05-16 11:41 ` Kalle Valo
0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2016-05-16 11:41 UTC (permalink / raw)
To: Michael Büsch; +Cc: Adrian Chadd, b43-dev, linux-wireless
Michael Büsch <m@bues.ch> writes:
> In the final submission please send this to Kalle Valo and add [PATCH]
> to the subject, so tools can pick this up.
And remember to CC linux-wireless so that patchwork sees the patch. I
skip the patches sent via email and only follow patchwork:
https://patchwork.kernel.org/project/linux-wireless/list/?state=*
--
Kalle Valo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-05-16 11:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-15 14:23 [b43] don't unconditionally fall back to CCK if the rate is 6MB OFDM Adrian Chadd
2016-05-15 14:32 ` Michael Büsch
2016-05-16 11:41 ` Kalle Valo
2016-05-15 14:39 ` Michael Büsch
2016-05-15 14:44 ` Adrian Chadd
2016-05-15 17:09 ` Rafał Miłecki
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).