[PATCH 1/2] brcmfmac: don't preset all channels as disabled

[PATCH 1/2] brcmfmac: don't preset all channels as disabled

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

During init we take care of regulatory stuff by disabling all
unavailable channels (see brcmf_construct_chaninfo) so this predisabling
them is not really required (and this patch won't change any behavior).
It will on the other hand allow more detailed runtime control over
channels which is the main reason for this change.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 895404c..45ee5b6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -138,7 +138,6 @@ static struct ieee80211_rate __wl_rates[] = {
 	.band			= NL80211_BAND_2GHZ,		\
 	.center_freq		= (_freq),			\
 	.hw_value		= (_channel),			\
-	.flags			= IEEE80211_CHAN_DISABLED,	\
 	.max_antenna_gain	= 0,				\
 	.max_power		= 30,				\
 }
@@ -147,7 +146,6 @@ static struct ieee80211_rate __wl_rates[] = {
 	.band			= NL80211_BAND_5GHZ,		\
 	.center_freq		= 5000 + (5 * (_channel)),	\
 	.hw_value		= (_channel),			\
-	.flags			= IEEE80211_CHAN_DISABLED,	\
 	.max_antenna_gain	= 0,				\
 	.max_power		= 30,				\
 }
-- 
2.10.1

[PATCH 2/2] brcmfmac: setup wiphy bands after registering it first

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

During bands setup we disable all channels that firmware doesn't support
in the current regulatory setup. If we do this before wiphy_register
it will result in copying set flags (including IEEE80211_CHAN_DISABLED)
to the orig_flags which is supposed to be persistent. We don't want this
as regulatory change may result in enabling some channels. We shouldn't
mess with orig_flags then (by changing them or ignoring them) so it's
better to just take care of their proper values.

This patch cleanups code a bit (by taking orig_flags more seriously) and
allows further improvements like disabling really unavailable channels.
We will need that e.g. if some frequencies should be disabled for good
due to hardware setup (design).

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 45ee5b6..729bf33 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6477,8 +6477,7 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
 			wiphy->bands[NL80211_BAND_5GHZ] = band;
 		}
 	}
-	err = brcmf_setup_wiphybands(wiphy);
-	return err;
+	return 0;
 }
 
 static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
@@ -6843,6 +6842,12 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
 		goto priv_out;
 	}
 
+	err = brcmf_setup_wiphybands(wiphy);
+	if (err) {
+		brcmf_err("Setting wiphy bands failed (%d)\n", err);
+		goto wiphy_unreg_out;
+	}
+
 	/* If cfg80211 didn't disable 40MHz HT CAP in wiphy_register(),
 	 * setup 40MHz in 2GHz band and enable OBSS scanning.
 	 */
-- 
2.10.1

Re: [PATCH 2/2] brcmfmac: setup wiphy bands after registering it first

[Arend Van Spriel]

On 7-1-2017 21:36, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> During bands setup we disable all channels that firmware doesn't support
> in the current regulatory setup. If we do this before wiphy_register
> it will result in copying set flags (including IEEE80211_CHAN_DISABLED)
> to the orig_flags which is supposed to be persistent. We don't want this
> as regulatory change may result in enabling some channels. We shouldn't
> mess with orig_flags then (by changing them or ignoring them) so it's
> better to just take care of their proper values.
> 
> This patch cleanups code a bit (by taking orig_flags more seriously) and
> allows further improvements like disabling really unavailable channels.
> We will need that e.g. if some frequencies should be disabled for good
> due to hardware setup (design).

I think this and previous patch are too dependent and prefer to have
them in a single patch. Despite that for both:

Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 45ee5b6..729bf33 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -6477,8 +6477,7 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
>  			wiphy->bands[NL80211_BAND_5GHZ] = band;
>  		}
>  	}
> -	err = brcmf_setup_wiphybands(wiphy);
> -	return err;
> +	return 0;
>  }
>  
>  static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
> @@ -6843,6 +6842,12 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
>  		goto priv_out;
>  	}
>  
> +	err = brcmf_setup_wiphybands(wiphy);
> +	if (err) {
> +		brcmf_err("Setting wiphy bands failed (%d)\n", err);
> +		goto wiphy_unreg_out;
> +	}
> +
>  	/* If cfg80211 didn't disable 40MHz HT CAP in wiphy_register(),
>  	 * setup 40MHz in 2GHz band and enable OBSS scanning.
>  	 */
> 

Re: [PATCH 2/2] brcmfmac: setup wiphy bands after registering it first

[Rafał Miłecki]

On 8 January 2017 at 14:00, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 7-1-2017 21:36, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> During bands setup we disable all channels that firmware doesn't support
>> in the current regulatory setup. If we do this before wiphy_register
>> it will result in copying set flags (including IEEE80211_CHAN_DISABLED)
>> to the orig_flags which is supposed to be persistent. We don't want this
>> as regulatory change may result in enabling some channels. We shouldn't
>> mess with orig_flags then (by changing them or ignoring them) so it's
>> better to just take care of their proper values.
>>
>> This patch cleanups code a bit (by taking orig_flags more seriously) and
>> allows further improvements like disabling really unavailable channels.
>> We will need that e.g. if some frequencies should be disabled for good
>> due to hardware setup (design).
>
> I think this and previous patch are too dependent and prefer to have
> them in a single patch. Despite that for both:
>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>

This time to make sure I can be easily understood I decided to use two
smaller patches & describe each of them with all the details that came
to my mind. I also made sure (and described that) that applying only
1/2 won't break anything (we never wan't to break potential bisecting
process).

I can work on merging (squashing) these 2 patches but then I need to
rework commit messages & I'll risk someone will say my description
isn't clear enough or my patch is too complex...

If there isn't a real problem (and maybe having 2 patches makes
following changes more easily) maybe let's stick to this patchset?

--=20
Rafa=C5=82

Re: [1/2] brcmfmac: don't preset all channels as disabled

[Kalle Valo]

Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> During init we take care of regulatory stuff by disabling all
> unavailable channels (see brcmf_construct_chaninfo) so this predisabling
> them is not really required (and this patch won't change any behavior).
> It will on the other hand allow more detailed runtime control over
> channels which is the main reason for this change.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

2 patches applied to wireless-drivers-next.git, thanks.

9ea0c307609f brcmfmac: don't preset all channels as disabled
ab99063f8737 brcmfmac: setup wiphy bands after registering it first

-- 
https://patchwork.kernel.org/patch/9503277/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches