diff mbox

brcmfmac: fix setting AP channel with new firmwares

Message ID 1464080979-12150-1-git-send-email-zajec5@gmail.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Rafał Miłecki May 24, 2016, 9:09 a.m. UTC
Firmware for new chipsets is based on a new major version of code
internally maintained at Broadcom. E.g. brcmfmac4366b-pcie.bin (used for
BCM4366B1) is based on 10.10.69.3309 while brcmfmac43602-pcie.ap.bin was
based on 7.35.177.56.

Currently setting AP 5 GHz channel doesn't work reliably with BCM4366B1.
When setting e.g. 36 control channel with VHT80 (center channel 42)
firmware may randomly pick one of:
1) 52 control channel with 58 as center one
2) 100 control channel with 106 as center one
3) 116 control channel with 122 as center one
4) 149 control channel with 155 as center one

It seems new firmwares require setting AP mode (BRCMF_C_SET_AP) before
specifying a channel. Changing an order of firmware calls fixes the
problem.

This fix was verified with BCM4366B1 and tested for regressions on
BCM43602. It's unclear if it's needed (or correct at all) for P2P
interfaces so it leaves this code unaffected.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c  | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Arend van Spriel May 25, 2016, 9:08 p.m. UTC | #1
On 24-05-16 11:09, Rafał Miłecki wrote:
> Firmware for new chipsets is based on a new major version of code
> internally maintained at Broadcom. E.g. brcmfmac4366b-pcie.bin (used for
> BCM4366B1) is based on 10.10.69.3309 while brcmfmac43602-pcie.ap.bin was
> based on 7.35.177.56.
> 
> Currently setting AP 5 GHz channel doesn't work reliably with BCM4366B1.
> When setting e.g. 36 control channel with VHT80 (center channel 42)
> firmware may randomly pick one of:
> 1) 52 control channel with 58 as center one
> 2) 100 control channel with 106 as center one
> 3) 116 control channel with 122 as center one
> 4) 149 control channel with 155 as center one
> 
> It seems new firmwares require setting AP mode (BRCMF_C_SET_AP) before
> specifying a channel. Changing an order of firmware calls fixes the
> problem.
> 
> This fix was verified with BCM4366B1 and tested for regressions on
> BCM43602. It's unclear if it's needed (or correct at all) for P2P
> interfaces so it leaves this code unaffected.

In doing so the code reads a bit awkward so if P2P-GO works with the
changed order that would be preferable.

> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c  | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
Rafał Miłecki May 25, 2016, 9:49 p.m. UTC | #2
On 25 May 2016 at 23:08, Arend van Spriel <arend.vanspriel@broadcom.com> wrote:
> On 24-05-16 11:09, Rafał Miłecki wrote:
>> Firmware for new chipsets is based on a new major version of code
>> internally maintained at Broadcom. E.g. brcmfmac4366b-pcie.bin (used for
>> BCM4366B1) is based on 10.10.69.3309 while brcmfmac43602-pcie.ap.bin was
>> based on 7.35.177.56.
>>
>> Currently setting AP 5 GHz channel doesn't work reliably with BCM4366B1.
>> When setting e.g. 36 control channel with VHT80 (center channel 42)
>> firmware may randomly pick one of:
>> 1) 52 control channel with 58 as center one
>> 2) 100 control channel with 106 as center one
>> 3) 116 control channel with 122 as center one
>> 4) 149 control channel with 155 as center one
>>
>> It seems new firmwares require setting AP mode (BRCMF_C_SET_AP) before
>> specifying a channel. Changing an order of firmware calls fixes the
>> problem.
>>
>> This fix was verified with BCM4366B1 and tested for regressions on
>> BCM43602. It's unclear if it's needed (or correct at all) for P2P
>> interfaces so it leaves this code unaffected.
>
> In doing so the code reads a bit awkward so if P2P-GO works with the
> changed order that would be preferable.

I'd prefer to have one code path as well, but my device/firmware
doesn't support P2P so I couldn't test it.

Could you test it or check firmware code to see if it's safe to change
P2P path as well?
diff mbox

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 299a404..3d09d23 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4423,7 +4423,7 @@  brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 	struct brcmf_join_params join_params;
 	enum nl80211_iftype dev_role;
 	struct brcmf_fil_bss_enable_le bss_enable;
-	u16 chanspec;
+	u16 chanspec = chandef_to_chanspec(&cfg->d11inf, &settings->chandef);
 	bool mbss;
 	int is_11d;
 
@@ -4499,16 +4499,16 @@  brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 
 	brcmf_config_ap_mgmt_ie(ifp->vif, &settings->beacon);
 
-	if (!mbss) {
-		chanspec = chandef_to_chanspec(&cfg->d11inf,
-					       &settings->chandef);
+	if (dev_role == NL80211_IFTYPE_P2P_GO) {
 		err = brcmf_fil_iovar_int_set(ifp, "chanspec", chanspec);
 		if (err < 0) {
 			brcmf_err("Set Channel failed: chspec=%d, %d\n",
 				  chanspec, err);
 			goto exit;
 		}
+	}
 
+	if (!mbss) {
 		if (is_11d != ifp->vif->is_11d) {
 			err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
 						    is_11d);
@@ -4565,6 +4565,14 @@  brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 			brcmf_err("setting AP mode failed %d\n", err);
 			goto exit;
 		}
+		if (!mbss) {
+			err = brcmf_fil_iovar_int_set(ifp, "chanspec", chanspec);
+			if (err < 0) {
+				brcmf_err("Set Channel failed: chspec=%d, %d\n",
+					  chanspec, err);
+				goto exit;
+			}
+		}
 		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_UP, 1);
 		if (err < 0) {
 			brcmf_err("BRCMF_C_UP error (%d)\n", err);