diff mbox

mwifiex: add module parameter to disable 40MHZ support in 2.4G band

Message ID 1501751215-12742-1-git-send-email-huxinming820@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Xinming Hu Aug. 3, 2017, 9:06 a.m. UTC 
From: Xinming Hu <huxm@marvell.com>

This patch provide a new module parameter disable_2g4_40m, with
which driver will not report 40M capability for 2.4GHZ to cfg80211.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

Kalle Valo Aug. 3, 2017, 9:26 a.m. UTC | #1 
Xinming Hu <huxinming820@gmail.com> writes:

> From: Xinming Hu <huxm@marvell.com>
>
> This patch provide a new module parameter disable_2g4_40m, with
> which driver will not report 40M capability for 2.4GHZ to cfg80211.
>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index 2be7817..820475a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -25,6 +25,10 @@
>  static char *reg_alpha2;
>  module_param(reg_alpha2, charp, 0);
>  
> +static bool disable_2g4_40m;
> +module_param(disable_2g4_40m, bool, 0000);

This is bool, good.

> +MODULE_PARM_DESC(disable_2g4_40m, "2.4G 40M support disable:1, enable:0");

This is not really a readable description for someone not familiar with
Wi-Fi, maybe this instead:

"disable 40 Mhz channels on 2.4 GHz band (disable:1, enable:0)"

> @@ -2755,7 +2759,7 @@ static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
>   */
>  static void
>  mwifiex_setup_ht_caps(struct ieee80211_sta_ht_cap *ht_info,
> -		      struct mwifiex_private *priv)
> +		      struct mwifiex_private *priv, int disable_40m)

But here you use int, that's a bit strange. Why not bool?
Arend van Spriel Aug. 3, 2017, 9:49 a.m. UTC | #2 
On 03-08-17 11:26, Kalle Valo wrote:
> Xinming Hu <huxinming820@gmail.com> writes:
> 
>> From: Xinming Hu <huxm@marvell.com>
>>
>> This patch provide a new module parameter disable_2g4_40m, with
>> which driver will not report 40M capability for 2.4GHZ to cfg80211.
>>
>> Signed-off-by: Xinming Hu <huxm@marvell.com>
>> Signed-off-by: Cathy Luo <cluo@marvell.com>
>> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
>> ---
>>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 22 ++++++++++++++--------
>>  1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> index 2be7817..820475a 100644
>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>> @@ -25,6 +25,10 @@
>>  static char *reg_alpha2;
>>  module_param(reg_alpha2, charp, 0);
>>  
>> +static bool disable_2g4_40m;
>> +module_param(disable_2g4_40m, bool, 0000);
> 
> This is bool, good.
> 
>> +MODULE_PARM_DESC(disable_2g4_40m, "2.4G 40M support disable:1, enable:0");
> 
> This is not really a readable description for someone not familiar with
> Wi-Fi, maybe this instead:
> 
> "disable 40 Mhz channels on 2.4 GHz band (disable:1, enable:0)"
> 
>> @@ -2755,7 +2759,7 @@ static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
>>   */
>>  static void
>>  mwifiex_setup_ht_caps(struct ieee80211_sta_ht_cap *ht_info,
>> -		      struct mwifiex_private *priv)
>> +		      struct mwifiex_private *priv, int disable_40m)
> 
> But here you use int, that's a bit strange. Why not bool?

Here is what is in net/wireless/core.c:

static bool cfg80211_disable_40mhz_24ghz;
module_param(cfg80211_disable_40mhz_24ghz, bool, 0644);
MODULE_PARM_DESC(cfg80211_disable_40mhz_24ghz,
                 "Disable 40MHz support in the 2.4GHz band");

which seems exactly the same thing and ends up doing:

                /*
                 * Since cfg80211_disable_40mhz_24ghz is global, we can
                 * modify the sband's ht data even if the driver uses a
                 * global structure for that.
                 */
                if (cfg80211_disable_40mhz_24ghz &&
                    band == NL80211_BAND_2GHZ &&
                    sband->ht_cap.ht_supported) {
                        sband->ht_cap.cap &=
~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
                        sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SGI_40;
                }

Regards,
Arend
Kalle Valo Aug. 3, 2017, 9:59 a.m. UTC | #3 
Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 03-08-17 11:26, Kalle Valo wrote:
>> Xinming Hu <huxinming820@gmail.com> writes:
>> 
>>> From: Xinming Hu <huxm@marvell.com>
>>>
>>> This patch provide a new module parameter disable_2g4_40m, with
>>> which driver will not report 40M capability for 2.4GHZ to cfg80211.
>>>
>>> Signed-off-by: Xinming Hu <huxm@marvell.com>
>>> Signed-off-by: Cathy Luo <cluo@marvell.com>
>>> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
>>> ---
>>>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 22 ++++++++++++++--------
>>>  1 file changed, 14 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>> b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>> index 2be7817..820475a 100644
>>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
>>> @@ -25,6 +25,10 @@
>>>  static char *reg_alpha2;
>>>  module_param(reg_alpha2, charp, 0);
>>>  
>>> +static bool disable_2g4_40m;
>>> +module_param(disable_2g4_40m, bool, 0000);
>> 
>> This is bool, good.
>> 
>>> +MODULE_PARM_DESC(disable_2g4_40m, "2.4G 40M support disable:1, enable:0");
>> 
>> This is not really a readable description for someone not familiar with
>> Wi-Fi, maybe this instead:
>> 
>> "disable 40 Mhz channels on 2.4 GHz band (disable:1, enable:0)"
>> 
>>> @@ -2755,7 +2759,7 @@ static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
>>>   */
>>>  static void
>>>  mwifiex_setup_ht_caps(struct ieee80211_sta_ht_cap *ht_info,
>>> -		      struct mwifiex_private *priv)
>>> +		      struct mwifiex_private *priv, int disable_40m)
>> 
>> But here you use int, that's a bit strange. Why not bool?
>
> Here is what is in net/wireless/core.c:
>
> static bool cfg80211_disable_40mhz_24ghz;
> module_param(cfg80211_disable_40mhz_24ghz, bool, 0644);
> MODULE_PARM_DESC(cfg80211_disable_40mhz_24ghz,
>                  "Disable 40MHz support in the 2.4GHz band");
>
> which seems exactly the same thing and ends up doing:
>
>                 /*
>                  * Since cfg80211_disable_40mhz_24ghz is global, we can
>                  * modify the sband's ht data even if the driver uses a
>                  * global structure for that.
>                  */
>                 if (cfg80211_disable_40mhz_24ghz &&
>                     band == NL80211_BAND_2GHZ &&
>                     sband->ht_cap.ht_supported) {
>                         sband->ht_cap.cap &=
> ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
>                         sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SGI_40;
>                 }

Good catch! So the module parameter for the driver is not needed, right?
Xinming Hu Aug. 3, 2017, 10:26 a.m. UTC | #4 
Hi Arend/Kalle,

> -----Original Message-----

> From: Kalle Valo [mailto:kvalo@codeaurora.org]

> Sent: 2017年8月3日 18:00

> To: Arend van Spriel

> Cc: Xinming Hu; Linux Wireless; Brian Norris; Dmitry Torokhov;

> rajatja@google.com; Zhiyuan Yang; Tim Song; Cathy Luo; Ganapathi Bhat;

> Xinming Hu

> Subject: Re: [PATCH] mwifiex: add module parameter to disable 40MHZ

> support in 2.4G band

> 

> Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> 

> > On 03-08-17 11:26, Kalle Valo wrote:

> >> Xinming Hu <huxinming820@gmail.com> writes:

> >>

> >>> From: Xinming Hu <huxm@marvell.com>

> >>>

> >>> This patch provide a new module parameter disable_2g4_40m, with

> >>> which driver will not report 40M capability for 2.4GHZ to cfg80211.

> >>>

> >>> Signed-off-by: Xinming Hu <huxm@marvell.com>

> >>> Signed-off-by: Cathy Luo <cluo@marvell.com>

> >>> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>

> >>> ---

> >>>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 22

> >>> ++++++++++++++--------

> >>>  1 file changed, 14 insertions(+), 8 deletions(-)

> >>>

> >>> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c

> >>> b/drivers/net/wireless/marvell/mwifiex/cfg80211.c

> >>> index 2be7817..820475a 100644

> >>> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c

> >>> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c

> >>> @@ -25,6 +25,10 @@

> >>>  static char *reg_alpha2;

> >>>  module_param(reg_alpha2, charp, 0);

> >>>

> >>> +static bool disable_2g4_40m;

> >>> +module_param(disable_2g4_40m, bool, 0000);

> >>

> >> This is bool, good.

> >>

> >>> +MODULE_PARM_DESC(disable_2g4_40m, "2.4G 40M support disable:1,

> >>> +enable:0");

> >>

> >> This is not really a readable description for someone not familiar

> >> with Wi-Fi, maybe this instead:

> >>

> >> "disable 40 Mhz channels on 2.4 GHz band (disable:1, enable:0)"

> >>

> >>> @@ -2755,7 +2759,7 @@ static void mwifiex_setup_vht_caps(struct

> ieee80211_sta_vht_cap *vht_info,

> >>>   */

> >>>  static void

> >>>  mwifiex_setup_ht_caps(struct ieee80211_sta_ht_cap *ht_info,

> >>> -		      struct mwifiex_private *priv)

> >>> +		      struct mwifiex_private *priv, int disable_40m)

> >>

> >> But here you use int, that's a bit strange. Why not bool?

> >

> > Here is what is in net/wireless/core.c:

> >

> > static bool cfg80211_disable_40mhz_24ghz;

> > module_param(cfg80211_disable_40mhz_24ghz, bool, 0644);

> > MODULE_PARM_DESC(cfg80211_disable_40mhz_24ghz,

> >                  "Disable 40MHz support in the 2.4GHz band");

> >

> > which seems exactly the same thing and ends up doing:

> >

> >                 /*

> >                  * Since cfg80211_disable_40mhz_24ghz is global, we

> can

> >                  * modify the sband's ht data even if the driver uses a

> >                  * global structure for that.

> >                  */

> >                 if (cfg80211_disable_40mhz_24ghz &&

> >                     band == NL80211_BAND_2GHZ &&

> >                     sband->ht_cap.ht_supported) {

> >                         sband->ht_cap.cap &=

> > ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;

> >                         sband->ht_cap.cap &=

> ~IEEE80211_HT_CAP_SGI_40;

> >                 }

> 

> Good catch! So the module parameter for the driver is not needed, right?


Great, that is what we want! Thanks for the point.

> 

> --

> Kalle Valo
diff mbox

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 2be7817..820475a 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -25,6 +25,10 @@ 
 static char *reg_alpha2;
 module_param(reg_alpha2, charp, 0);
 
+static bool disable_2g4_40m;
+module_param(disable_2g4_40m, bool, 0000);
+MODULE_PARM_DESC(disable_2g4_40m, "2.4G 40M support disable:1, enable:0");
+
 static const struct ieee80211_iface_limit mwifiex_ap_sta_limits[] = {
 	{
 		.max = 3, .types = BIT(NL80211_IFTYPE_STATION) |
@@ -2755,7 +2759,7 @@  static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
  */
 static void
 mwifiex_setup_ht_caps(struct ieee80211_sta_ht_cap *ht_info,
-		      struct mwifiex_private *priv)
+		      struct mwifiex_private *priv, int disable_40m)
 {
 	int rx_mcs_supp;
 	struct ieee80211_mcs_info mcs_set;
@@ -2769,7 +2773,7 @@  static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
 	memset(&ht_info->mcs, 0, sizeof(ht_info->mcs));
 
 	/* Fill HT capability information */
-	if (ISSUPP_CHANWIDTH40(adapter->hw_dot_11n_dev_cap))
+	if (ISSUPP_CHANWIDTH40(adapter->hw_dot_11n_dev_cap) && !disable_40m)
 		ht_info->cap |= IEEE80211_HT_CAP_SUP_WIDTH_20_40;
 	else
 		ht_info->cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
@@ -2779,7 +2783,7 @@  static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
 	else
 		ht_info->cap &= ~IEEE80211_HT_CAP_SGI_20;
 
-	if (ISSUPP_SHORTGI40(adapter->hw_dot_11n_dev_cap))
+	if (ISSUPP_SHORTGI40(adapter->hw_dot_11n_dev_cap) && !disable_40m)
 		ht_info->cap |= IEEE80211_HT_CAP_SGI_40;
 	else
 		ht_info->cap &= ~IEEE80211_HT_CAP_SGI_40;
@@ -2799,7 +2803,8 @@  static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
 	else
 		ht_info->cap &= ~IEEE80211_HT_CAP_GRN_FLD;
 
-	if (ISENABLED_40MHZ_INTOLERANT(adapter->hw_dot_11n_dev_cap))
+	if (ISENABLED_40MHZ_INTOLERANT(adapter->hw_dot_11n_dev_cap) &&
+	    !disable_40m)
 		ht_info->cap |= IEEE80211_HT_CAP_40MHZ_INTOLERANT;
 	else
 		ht_info->cap &= ~IEEE80211_HT_CAP_40MHZ_INTOLERANT;
@@ -2819,7 +2824,7 @@  static void mwifiex_setup_vht_caps(struct ieee80211_sta_vht_cap *vht_info,
 	memset(&mcs[rx_mcs_supp], 0,
 	       sizeof(struct ieee80211_mcs_info) - rx_mcs_supp);
 	if (priv->bss_mode == NL80211_IFTYPE_STATION ||
-	    ISSUPP_CHANWIDTH40(adapter->hw_dot_11n_dev_cap))
+	    (ISSUPP_CHANWIDTH40(adapter->hw_dot_11n_dev_cap) && !disable_40m))
 		/* Set MCS32 for infra mode or ad-hoc mode with 40MHz support */
 		SETHT_MCS32(mcs_set.rx_mask);
 
@@ -2974,14 +2979,15 @@  struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
 	if (ret)
 		goto err_sta_init;
 
-	mwifiex_setup_ht_caps(&wiphy->bands[NL80211_BAND_2GHZ]->ht_cap, priv);
-	if (adapter->is_hw_11ac_capable)
+	mwifiex_setup_ht_caps(&wiphy->bands[NL80211_BAND_2GHZ]->ht_cap, priv,
+			      disable_2g4_40m);
+	if (adapter->is_hw_11ac_capable && !disable_2g4_40m)
 		mwifiex_setup_vht_caps(
 			&wiphy->bands[NL80211_BAND_2GHZ]->vht_cap, priv);
 
 	if (adapter->config_bands & BAND_A)
 		mwifiex_setup_ht_caps(
-			&wiphy->bands[NL80211_BAND_5GHZ]->ht_cap, priv);
+			&wiphy->bands[NL80211_BAND_5GHZ]->ht_cap, priv, 0);
 
 	if ((adapter->config_bands & BAND_A) && adapter->is_hw_11ac_capable)
 		mwifiex_setup_vht_caps(