Hi Arend/Kalle, > -----Original Message----- > From: Kalle Valo [mailto:kvalo@xxxxxxxxxxxxxx] > Sent: 2017年8月3日 18:00 > To: Arend van Spriel > Cc: Xinming Hu; Linux Wireless; Brian Norris; Dmitry Torokhov; > rajatja@xxxxxxxxxx; 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@xxxxxxxxxxxx> writes: > > > On 03-08-17 11:26, Kalle Valo wrote: > >> Xinming Hu <huxinming820@xxxxxxxxx> writes: > >> > >>> From: Xinming Hu <huxm@xxxxxxxxxxx> > >>> > >>> 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@xxxxxxxxxxx> > >>> Signed-off-by: Cathy Luo <cluo@xxxxxxxxxxx> > >>> Signed-off-by: Ganapathi Bhat <gbhat@xxxxxxxxxxx> > >>> --- > >>> 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
