Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH 0921/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 11:58 UTC (permalink / raw)
  To: m, jejb, martin.petersen, m.chehab, pawel, m.szyprowski,
	kyungmin.park, k.kozlowski
  Cc: linux-wireless, linux-kernel, chuansheng.liu, baolex.ni

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 drivers/ssb/pcmcia.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssb/pcmcia.c b/drivers/ssb/pcmcia.c
index f03422b..e69296a 100644
--- a/drivers/ssb/pcmcia.c
+++ b/drivers/ssb/pcmcia.c
@@ -764,7 +764,7 @@ static ssize_t ssb_pcmcia_attr_sprom_store(struct device *pcmciadev,
 				    ssb_pcmcia_sprom_write_all);
 }
 
-static DEVICE_ATTR(ssb_sprom, 0600,
+static DEVICE_ATTR(ssb_sprom, S_IRUSR | S_IWUSR,
 		   ssb_pcmcia_attr_sprom_show,
 		   ssb_pcmcia_attr_sprom_store);
 
-- 
2.9.2


^ permalink raw reply related

* [PATCH 0954/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:02 UTC (permalink / raw)
  To: Larry.Finger, Jes.Sorensen, gregkh, m.chehab, m.szyprowski,
	kyungmin.park, k.kozlowski
  Cc: linux-wireless, devel, linux-kernel, chuansheng.liu, baolex.ni,
	shivanib134, punitvara, joseph.bisch, bhaktipriya96

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 drivers/staging/rtl8723au/os_dep/os_intfs.c | 66 ++++++++++++++---------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/rtl8723au/os_dep/os_intfs.c b/drivers/staging/rtl8723au/os_dep/os_intfs.c
index b8848c2..eac8edc 100644
--- a/drivers/staging/rtl8723au/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723au/os_dep/os_intfs.c
@@ -53,7 +53,7 @@ static int rtw_ips_mode = IPS_NORMAL;
 
 static int rtw_smart_ps = 2;
 
-module_param(rtw_ips_mode, int, 0644);
+module_param(rtw_ips_mode, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(rtw_ips_mode, "The default IPS mode");
 
 static int rtw_long_retry_lmt = 7;
@@ -112,62 +112,62 @@ static int rtw_80211d;
 
 static int rtw_regulatory_id = 0xff;/*  Regulatory tab id, 0xff = follow efuse's setting */
 
-module_param(rtw_regulatory_id, int, 0644);
+module_param(rtw_regulatory_id, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 static char *ifname = "wlan%d";
-module_param(ifname, charp, 0644);
+module_param(ifname, charp, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(ifname, "The default name to allocate for first interface");
 
 static char *if2name = "wlan%d";
-module_param(if2name, charp, 0644);
+module_param(if2name, charp, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(if2name, "The default name to allocate for second interface");
 
-module_param(rtw_channel_plan, int, 0644);
-module_param(rtw_chip_version, int, 0644);
-module_param(rtw_rfintfs, int, 0644);
-module_param(rtw_channel, int, 0644);
-module_param(rtw_wmm_enable, int, 0644);
-module_param(rtw_vrtl_carrier_sense, int, 0644);
-module_param(rtw_vcs_type, int, 0644);
-module_param(rtw_busy_thresh, int, 0644);
-module_param(rtw_ht_enable, int, 0644);
-module_param(rtw_cbw40_enable, int, 0644);
-module_param(rtw_ampdu_enable, int, 0644);
-module_param(rtw_rx_stbc, int, 0644);
-module_param(rtw_ampdu_amsdu, int, 0644);
+module_param(rtw_channel_plan, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_chip_version, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_rfintfs, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_channel, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_wmm_enable, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_vrtl_carrier_sense, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_vcs_type, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_busy_thresh, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_ht_enable, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_cbw40_enable, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_ampdu_enable, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_rx_stbc, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_ampdu_amsdu, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
-module_param(rtw_lowrate_two_xmit, int, 0644);
+module_param(rtw_lowrate_two_xmit, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
-module_param(rtw_rf_config, int, 0644);
-module_param(rtw_power_mgnt, int, 0644);
-module_param(rtw_smart_ps, int, 0644);
-module_param(rtw_low_power, int, 0644);
-module_param(rtw_wifi_spec, int, 0644);
+module_param(rtw_rf_config, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_power_mgnt, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_smart_ps, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_low_power, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_wifi_spec, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
-module_param(rtw_antdiv_cfg, int, 0644);
+module_param(rtw_antdiv_cfg, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
-module_param(rtw_enusbss, int, 0644);
-module_param(rtw_hwpdn_mode, int, 0644);
-module_param(rtw_hwpwrp_detect, int, 0644);
+module_param(rtw_enusbss, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_hwpdn_mode, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+module_param(rtw_hwpwrp_detect, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
-module_param(rtw_hw_wps_pbc, int, 0644);
+module_param(rtw_hw_wps_pbc, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
 static uint rtw_max_roaming_times = 2;
-module_param(rtw_max_roaming_times, uint, 0644);
+module_param(rtw_max_roaming_times, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(rtw_max_roaming_times, "The max roaming times to try");
 
-module_param(rtw_80211d, int, 0644);
+module_param(rtw_80211d, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(rtw_80211d, "Enable 802.11d mechanism");
 
 #ifdef CONFIG_8723AU_BT_COEXIST
-module_param(rtw_btcoex_enable, int, 0644);
+module_param(rtw_btcoex_enable, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(rtw_btcoex_enable, "Enable BT co-existence mechanism");
 #endif
 
 static uint rtw_notch_filter;
-module_param(rtw_notch_filter, uint, 0644);
+module_param(rtw_notch_filter, uint, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(rtw_notch_filter, "0:Disable, 1:Enable, 2:Enable only for P2P");
-module_param_named(debug, rtw_debug, int, 0444);
+module_param_named(debug, rtw_debug, int, S_IRUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(debug, "Set debug level (1-9) (default 1)");
 
 static int netdev_close(struct net_device *pnetdev);
-- 
2.9.2


^ permalink raw reply related

* [PATCH 1133/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:19 UTC (permalink / raw)
  To: johannes, davem, kadlec, kaber, m.szyprowski, kyungmin.park,
	k.kozlowski
  Cc: linux-wireless, netdev, linux-kernel, chuansheng.liu, baolex.ni,
	aryabinin

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 net/rfkill/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 884027f..53bf4f9 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -114,7 +114,7 @@ static DEFINE_MUTEX(rfkill_global_mutex);
 static LIST_HEAD(rfkill_fds);	/* list of open fds of /dev/rfkill */
 
 static unsigned int rfkill_default_state = 1;
-module_param_named(default_state, rfkill_default_state, uint, 0444);
+module_param_named(default_state, rfkill_default_state, uint, S_IRUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(default_state,
 		 "Default initial state for all radio types, 0 = radio off");
 
-- 
2.9.2


^ permalink raw reply related

* [PATCH 1112/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:17 UTC (permalink / raw)
  To: johannes, davem, jmorris, yoshfuji, kaber, m.szyprowski,
	kyungmin.park, k.kozlowski
  Cc: linux-wireless, netdev, linux-kernel, chuansheng.liu, baolex.ni,
	aarcange, aryabinin

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 net/mac80211/mlme.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 8d426f6..ca2f846 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -42,12 +42,12 @@
 #define IEEE80211_ASSOC_MAX_TRIES	3
 
 static int max_nullfunc_tries = 2;
-module_param(max_nullfunc_tries, int, 0644);
+module_param(max_nullfunc_tries, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(max_nullfunc_tries,
 		 "Maximum nullfunc tx tries before disconnecting (reason 4).");
 
 static int max_probe_tries = 5;
-module_param(max_probe_tries, int, 0644);
+module_param(max_probe_tries, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(max_probe_tries,
 		 "Maximum probe tries before disconnecting (reason 4).");
 
@@ -60,7 +60,7 @@ MODULE_PARM_DESC(max_probe_tries,
  * default to what we want.
  */
 static int beacon_loss_count = 7;
-module_param(beacon_loss_count, int, 0644);
+module_param(beacon_loss_count, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(beacon_loss_count,
 		 "Number of beacon intervals before we decide beacon was lost.");
 
@@ -75,7 +75,7 @@ MODULE_PARM_DESC(beacon_loss_count,
  * checking the connection still works.
  */
 static int probe_wait_ms = 500;
-module_param(probe_wait_ms, int, 0644);
+module_param(probe_wait_ms, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(probe_wait_ms,
 		 "Maximum time(ms) to wait for probe response"
 		 " before disconnecting (reason 4).");
-- 
2.9.2


^ permalink raw reply related

* [PATCH 1114/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:17 UTC (permalink / raw)
  To: johannes, davem, jmorris, yoshfuji, kaber, m.szyprowski,
	kyungmin.park, k.kozlowski
  Cc: linux-wireless, netdev, linux-kernel, chuansheng.liu, baolex.ni,
	aarcange, aryabinin

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 net/mac80211/rc80211_minstrel_ht.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 30fbabf..1b9d495 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -138,7 +138,7 @@
 
 #ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
 static bool minstrel_vht_only = true;
-module_param(minstrel_vht_only, bool, 0644);
+module_param(minstrel_vht_only, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(minstrel_vht_only,
 		 "Use only VHT rates when VHT is supported by sta.");
 #endif
-- 
2.9.2


^ permalink raw reply related

* [PATCH 1141/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:19 UTC (permalink / raw)
  To: johannes, davem, linux-wimax, anna.schumaker, kadlec,
	m.szyprowski, kyungmin.park, k.kozlowski
  Cc: linux-wireless, netdev, linux-kernel, chuansheng.liu, baolex.ni,
	arnd, aryabinin

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 net/wireless/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index ecca389..711ab8c 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -48,7 +48,7 @@ static struct dentry *ieee80211_debugfs_dir;
 struct workqueue_struct *cfg80211_wq;
 
 static bool cfg80211_disable_40mhz_24ghz;
-module_param(cfg80211_disable_40mhz_24ghz, bool, 0644);
+module_param(cfg80211_disable_40mhz_24ghz, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(cfg80211_disable_40mhz_24ghz,
 		 "Disable 40MHz support in the 2.4GHz band");
 
-- 
2.9.2


^ permalink raw reply related

* [PATCH 1142/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:20 UTC (permalink / raw)
  To: johannes, davem, linux-wimax, anna.schumaker, kadlec,
	m.szyprowski, kyungmin.park, k.kozlowski
  Cc: linux-wireless, netdev, linux-kernel, chuansheng.liu, baolex.ni,
	arnd, aryabinin

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 net/wireless/reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..1435e1b 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -261,7 +261,7 @@ static const struct ieee80211_regdomain *cfg80211_world_regdom =
 static char *ieee80211_regdom = "00";
 static char user_alpha2[2];
 
-module_param(ieee80211_regdom, charp, 0444);
+module_param(ieee80211_regdom, charp, S_IRUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
 
 static void reg_free_request(struct regulatory_request *request)
-- 
2.9.2


^ permalink raw reply related

* [PATCH 1113/1285] Replace numeric parameter like 0444 with macro
From: Baole Ni @ 2016-08-02 12:17 UTC (permalink / raw)
  To: johannes, davem, jmorris, yoshfuji, kaber, m.szyprowski,
	kyungmin.park, k.kozlowski
  Cc: linux-wireless, netdev, linux-kernel, chuansheng.liu, baolex.ni,
	aarcange, aryabinin

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 net/mac80211/rate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 206698b..43bd119 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -25,7 +25,7 @@ static LIST_HEAD(rate_ctrl_algs);
 static DEFINE_MUTEX(rate_ctrl_mutex);
 
 static char *ieee80211_default_rc_algo = CONFIG_MAC80211_RC_DEFAULT;
-module_param(ieee80211_default_rc_algo, charp, 0644);
+module_param(ieee80211_default_rc_algo, charp, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(ieee80211_default_rc_algo,
 		 "Default rate control algorithm for mac80211 to use");
 
-- 
2.9.2


^ permalink raw reply related

* Re: [RFC] ath10k: silence firmware file probing warnings
From: Luis R. Rodriguez @ 2016-08-02 14:16 UTC (permalink / raw)
  To: Valo, Kalle
  Cc: Luis R. Rodriguez, Stanislaw Gruszka, Prarit Bhargava,
	Arend Van Spriel, Greg Kroah-Hartman, Ming Lei, linux-wireless,
	ath10k, mmarek@suse.com, michal.kazior@tieto.com,
	Arend van Spriel, Emmanuel Grumbach
In-Reply-To: <874m738m02.fsf@kamboji.qca.qualcomm.com>

On Tue, Aug 02, 2016 at 11:10:22AM +0000, Valo, Kalle wrote:
> "Luis R. Rodriguez" <mcgrof@kernel.org> writes:
> 
> > I was considering this as a future extension to the firmware API
> > through the new extensible firmware API, the sysdata API.
> 
> I think Linus mentioned this already, but I want to reiterate anyway.
> The name "sysdata" is horrible, I didn't have any idea what it means
> until I read your description. Please continue to use the term
> "firmware", anyone already know what it means.

We've gone well past using the firmware API for firmware though, if
we use it for 802.11 to replace CRDA for instance its really odd to
be calling it firmware. But sure... I will rebrand again to firmware...

  Luis

^ permalink raw reply

* Wireless Workshop accepted into the 2016 Linux Kernel Summit and Linux Plumbers Conference
From: Jake Edge @ 2016-08-02 14:45 UTC (permalink / raw)
  To: lpc-announce; +Cc: linux-wireless


It might well be that wireless networking recently made the transition
from an ubiquitous networking technology to the dominant networking
technology, at least from the viewpoint of end-user devices.  Part of
this trend is the use of wireless in automobiles, and this workshop
will look at Wireless Access in Vehicular Environments (WAVE),
also know as IEEE 802.11p.  In addition, the bufferbloat problem [1]
is starting to focus on the more difficult wireless environment, and to
that end, this workshop will discuss FQ/Codel [2] integration, testing,
and development.  As usual, the workshop will encompass the full 802.11
stack, not just the kernel portions, and therefore wpa_supplicant [3]
will also be on the agenda.

Please join us for a timely and important discussion [4]!

KS [5] will be held October 31-November 1 and LPC [6] will be held
November 1-4, both in Santa Fe, New Mexico, US.

[1] https://en.wikipedia.org/wiki/Bufferbloat
[2] https://en.wikipedia.org/wiki/CoDel
[3] https://w1.fi/wpa_supplicant/
[4] http://wiki.linuxplumbersconf.org/2016:wireless
[5] https://events.linuxfoundation.org/events/linux-kernel-summit
[6] http://www.linuxplumbersconf.org/2016/

^ permalink raw reply

* Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.
From: arvind Yadav @ 2016-08-02 15:48 UTC (permalink / raw)
  To: Arnd Bergmann, linuxppc-dev
  Cc: Scott Wood, qiang.zhao@freescale.com, viresh.kumar@linaro.org,
	zajec5@gmail.com, linux-wireless@vger.kernel.org,
	David.Laight@aculab.com, netdev@vger.kernel.org,
	scottwood@freescale.com, akpm@linux-foundation.org,
	davem@davemloft.net, linux@roeck-us.net
In-Reply-To: <2484583.95xFmO7xir@wuerfel>



On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote:
> On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote:
>> On 08/01/2016 02:02 AM, Arnd Bergmann wrote:
>>>> diff --git a/include/linux/err.h b/include/linux/err.h
>>>> index 1e35588..c2a2789 100644
>>>> --- a/include/linux/err.h
>>>> +++ b/include/linux/err.h
>>>> @@ -18,7 +18,17 @@
>>>>   
>>>>   #ifndef __ASSEMBLY__
>>>>   
>>>> -#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
>>>> +#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
>>>> +
>>>> +static inline int is_error_check(unsigned long error)
>>> Please leave the existing macro alone. I think you were looking for
>>> something specific to the return code of qe_muram_alloc() function,
>>> so please add a helper in that subsystem if you need it, not in
>>> the generic header files.
>> qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.  The
>> problem is certain callers that store the return value in a u32.  Why
>> not just fix those callers to store it in unsigned long (at least until
>> error checking is done)?
>>
> Yes, that would also address another problem with code like
>
>           kfree((void *)ugeth->tx_bd_ring_offset[i]);
>
> which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
> that also holds the return value of qe_muram_alloc.
>
> 	Arnd
Yes, we will fix caller. Caller api is not safe on 64bit.
Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int,
but it should be unsigned long. Need to work on it.

Arvind

^ permalink raw reply

* Re: [PATCH 0786/1285] Replace numeric parameter like 0444 with macro
From: Shanker Wang @ 2016-08-02 15:50 UTC (permalink / raw)
  To: Baole Ni
  Cc: kvalo, luciano.coelho, linuxwifi, m.chehab, pawel, m.szyprowski,
	kyungmin.park, k.kozlowski, libertas-dev, linux-wireless, netdev,
	LKML, chuansheng.liu
In-Reply-To: <20160802114618.2152-1-baolex.ni@intel.com>

I don’t think macros is clearer, and the meaning of those so called 
“magic numbers” like 0644 is clear enough. People are used to that.
As a result, it is not meaningful to replace them with macro.


> 在 2016年8月2日,19:46,Baole Ni <baolex.ni@intel.com> 写道:
> 
> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
> 
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> Signed-off-by: Baole Ni <baolex.ni@intel.com>
> ---
> drivers/net/wireless/marvell/libertas/mesh.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/libertas/mesh.c b/drivers/net/wireless/marvell/libertas/mesh.c
> index d0c881d..ae4f0a5 100644
> --- a/drivers/net/wireless/marvell/libertas/mesh.c
> +++ b/drivers/net/wireless/marvell/libertas/mesh.c
> @@ -297,19 +297,19 @@ static ssize_t lbs_mesh_set(struct device *dev,
>  * lbs_mesh attribute to be exported per ethX interface
>  * through sysfs (/sys/class/net/ethX/lbs_mesh)
>  */
> -static DEVICE_ATTR(lbs_mesh, 0644, lbs_mesh_get, lbs_mesh_set);
> +static DEVICE_ATTR(lbs_mesh, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, lbs_mesh_get, lbs_mesh_set);
> 
> /*
>  * anycast_mask attribute to be exported per mshX interface
>  * through sysfs (/sys/class/net/mshX/anycast_mask)
>  */
> -static DEVICE_ATTR(anycast_mask, 0644, lbs_anycast_get, lbs_anycast_set);
> +static DEVICE_ATTR(anycast_mask, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, lbs_anycast_get, lbs_anycast_set);
> 
> /*
>  * prb_rsp_limit attribute to be exported per mshX interface
>  * through sysfs (/sys/class/net/mshX/prb_rsp_limit)
>  */
> -static DEVICE_ATTR(prb_rsp_limit, 0644, lbs_prb_rsp_limit_get,
> +static DEVICE_ATTR(prb_rsp_limit, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, lbs_prb_rsp_limit_get,
> 		lbs_prb_rsp_limit_set);
> 
> static struct attribute *lbs_mesh_sysfs_entries[] = {
> @@ -764,13 +764,13 @@ static ssize_t capability_set(struct device *dev, struct device_attribute *attr,
> }
> 
> 
> -static DEVICE_ATTR(bootflag, 0644, bootflag_get, bootflag_set);
> -static DEVICE_ATTR(boottime, 0644, boottime_get, boottime_set);
> -static DEVICE_ATTR(channel, 0644, channel_get, channel_set);
> -static DEVICE_ATTR(mesh_id, 0644, mesh_id_get, mesh_id_set);
> -static DEVICE_ATTR(protocol_id, 0644, protocol_id_get, protocol_id_set);
> -static DEVICE_ATTR(metric_id, 0644, metric_id_get, metric_id_set);
> -static DEVICE_ATTR(capability, 0644, capability_get, capability_set);
> +static DEVICE_ATTR(bootflag, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, bootflag_get, bootflag_set);
> +static DEVICE_ATTR(boottime, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, boottime_get, boottime_set);
> +static DEVICE_ATTR(channel, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, channel_get, channel_set);
> +static DEVICE_ATTR(mesh_id, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, mesh_id_get, mesh_id_set);
> +static DEVICE_ATTR(protocol_id, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, protocol_id_get, protocol_id_set);
> +static DEVICE_ATTR(metric_id, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, metric_id_get, metric_id_set);
> +static DEVICE_ATTR(capability, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, capability_get, capability_set);
> 
> static struct attribute *boot_opts_attrs[] = {
> 	&dev_attr_bootflag.attr,
> -- 
> 2.9.2
> 


^ permalink raw reply

* Re: [PATCH 0768/1285] Replace numeric parameter like 0444 with macro
From: Kalle Valo @ 2016-08-02 17:52 UTC (permalink / raw)
  To: Baole Ni; +Cc: linux-wireless
In-Reply-To: <20160802114428.860-1-baolex.ni@intel.com>

(Trimming CC)

Baole Ni <baolex.ni@intel.com> writes:

> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> Signed-off-by: Baole Ni <baolex.ni@intel.com>
> ---
>  drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Over 1000 patches? It's totally inapproriate to spam people's inboxes
(including mine) like this. PLEASE first study how kernel development
works and then start with something small, not like this.

I'm dropping all patches from this patchset.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH] Staging: rtl8723au: rtw_xmit: fixed coding style issues
From: Jes Sorensen @ 2016-08-02 18:03 UTC (permalink / raw)
  To: Shiva Kerdel
  Cc: Larry.Finger, gregkh, geliangtang, linux-wireless, devel,
	linux-kernel
In-Reply-To: <20160802175721.2453-1-shiva@exdev.nl>

Shiva Kerdel <shiva@exdev.nl> writes:
> Fixed some coding style issues that were detected as errors.
>
> Signed-off-by: Shiva Kerdel <shiva@exdev.nl>

You have already been told this once by Greg. Describe what you are
fixing in the commit message, and don't fix more than one type of bug
per commit.

Jes

> ---
>  drivers/staging/rtl8723au/core/rtw_xmit.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c b/drivers/staging/rtl8723au/core/rtw_xmit.c
> index 3de40cf..4ea7c5f 100644
> --- a/drivers/staging/rtl8723au/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8723au/core/rtw_xmit.c
> @@ -174,7 +174,7 @@ int _rtw_init_xmit_priv23a(struct xmit_priv *pxmitpriv,
>  	rtw_alloc_hwxmits23a(padapter);
>  	rtw_init_hwxmits23a(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>  
> -	for (i = 0; i < 4; i ++)
> +	for (i = 0; i < 4; i++)
>  		pxmitpriv->wmm_para_seq[i] = i;
>  
>  	sema_init(&pxmitpriv->tx_retevt, 0);
> @@ -640,7 +640,7 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
>  	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
>  	int curfragnum, length;
>  	u8 *pframe, *payload, mic[8];
> -	u8 priority[4]= {0x0, 0x0, 0x0, 0x0};
> +	u8 priority[4] = {0x0, 0x0, 0x0, 0x0};
>  	u8 hw_hdr_offset = 0;
>  	int bmcst = is_multicast_ether_addr(pattrib->ra);
>  
> @@ -667,10 +667,10 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
>  	if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
>  		/* encode mic code */
>  		if (stainfo) {
> -			u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
> -					 0x0, 0x0, 0x0, 0x0,
> -					 0x0, 0x0, 0x0, 0x0,
> -					 0x0, 0x0, 0x0, 0x0};
> +			u8 null_key[16] = {0x0, 0x0, 0x0, 0x0,
> +					   0x0, 0x0, 0x0, 0x0,
> +					   0x0, 0x0, 0x0, 0x0,
> +					   0x0, 0x0, 0x0, 0x0};
>  
>  			pframe = pxmitframe->buf_addr + hw_hdr_offset;
>  
> @@ -902,8 +902,7 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
>  
>  			if (psta->qos_option)
>  				qos_option = true;
> -		}
> -		else {
> +		} else {
>  			RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
>  				 "fw_state:%x is not allowed to xmit frame\n",
>  				 get_fwstate(pmlmepriv));
> @@ -1190,7 +1189,7 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
>  		pdata += mem_sz;
>  		data_len -= mem_sz;
>  
> -		if ((pattrib->icv_len >0) && (pattrib->bswenc)) {
> +		if ((pattrib->icv_len > 0) && (pattrib->bswenc)) {
>  			memcpy(pframe, pattrib->icv, pattrib->icv_len);
>  			pframe += pattrib->icv_len;
>  		}
> @@ -1766,7 +1765,6 @@ void rtw_alloc_hwxmits23a(struct rtw_adapter *padapter)
>  		hwxmits[4] .sta_queue = &pxmitpriv->be_pending;
>  
>  	} else if (pxmitpriv->hwxmit_entry == 4) {
> -
>  		/* pxmitpriv->vo_txqueue.head = 0; */
>  		/* hwxmits[0] .phwtxqueue = &pxmitpriv->vo_txqueue; */
>  		hwxmits[0] .sta_queue = &pxmitpriv->vo_pending;
> @@ -1952,18 +1950,16 @@ int xmitframe_enqueue_for_sleeping_sta23a(struct rtw_adapter *padapter, struct x
>  			/* spin_unlock_bh(&psta->sleep_q.lock); */
>  
>  			ret = true;
> -
>  		}
>  
>  		spin_unlock_bh(&psta->sleep_q.lock);
>  
>  		return ret;
> -
>  	}
>  
>  	spin_lock_bh(&psta->sleep_q.lock);
>  
> -	if (psta->state&WIFI_SLEEP_STATE) {
> +	if (psta->state & WIFI_SLEEP_STATE) {
>  		u8 wmmps_ac = 0;
>  
>  		if (pstapriv->sta_dz_bitmap & CHKBIT(psta->aid)) {
> @@ -2017,9 +2013,7 @@ int xmitframe_enqueue_for_sleeping_sta23a(struct rtw_adapter *padapter, struct x
>  			/*  */
>  
>  			ret = true;
> -
>  		}
> -
>  	}
>  
>  	spin_unlock_bh(&psta->sleep_q.lock);
> @@ -2159,7 +2153,7 @@ void wakeup_sta_to_xmit23a(struct rtw_adapter *padapter, struct sta_info *psta)
>  		/* update BCN for TIM IE */
>  		update_mask = BIT(0);
>  
> -		if (psta->state&WIFI_SLEEP_STATE)
> +		if (psta->state & WIFI_SLEEP_STATE)
>  			psta->state ^= WIFI_SLEEP_STATE;
>  
>  		if (psta->state & WIFI_STA_ALIVE_CHK_STATE) {

^ permalink raw reply

* [PATCH] Staging: rtl8723au: rtw_xmit: Fixed operators spacing style issues
From: Shiva Kerdel @ 2016-08-02 18:05 UTC (permalink / raw)
  To: Larry.Finger
  Cc: Jes.Sorensen, gregkh, geliangtang, linux-wireless, devel,
	linux-kernel, Shiva Kerdel

Fixed spaces around operators to fix their coding style issues.

Signed-off-by: Shiva Kerdel <shiva@exdev.nl>
---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c b/drivers/staging/rtl8723au/core/rtw_xmit.c
index 3de40cf..56228a5 100644
--- a/drivers/staging/rtl8723au/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723au/core/rtw_xmit.c
@@ -174,7 +174,7 @@ int _rtw_init_xmit_priv23a(struct xmit_priv *pxmitpriv,
 	rtw_alloc_hwxmits23a(padapter);
 	rtw_init_hwxmits23a(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
 
-	for (i = 0; i < 4; i ++)
+	for (i = 0; i < 4; i++)
 		pxmitpriv->wmm_para_seq[i] = i;
 
 	sema_init(&pxmitpriv->tx_retevt, 0);
@@ -421,7 +421,7 @@ static int update_attrib(struct rtw_adapter *padapter,
 	struct security_priv *psecuritypriv = &padapter->securitypriv;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	int res = _SUCCESS;
-	struct ethhdr *ehdr = (struct ethhdr *) skb->data;
+	struct ethhdr *ehdr = (struct ethhdr *)skb->data;
 
 	pattrib->ether_type = ntohs(ehdr->h_proto);
 
@@ -640,7 +640,7 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 	int curfragnum, length;
 	u8 *pframe, *payload, mic[8];
-	u8 priority[4]= {0x0, 0x0, 0x0, 0x0};
+	u8 priority[4] = {0x0, 0x0, 0x0, 0x0};
 	u8 hw_hdr_offset = 0;
 	int bmcst = is_multicast_ether_addr(pattrib->ra);
 
@@ -667,10 +667,10 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 	if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
 		/* encode mic code */
 		if (stainfo) {
-			u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
-					 0x0, 0x0, 0x0, 0x0,
-					 0x0, 0x0, 0x0, 0x0,
-					 0x0, 0x0, 0x0, 0x0};
+			u8 null_key[16] = {0x0, 0x0, 0x0, 0x0,
+					   0x0, 0x0, 0x0, 0x0,
+					   0x0, 0x0, 0x0, 0x0,
+					   0x0, 0x0, 0x0, 0x0};
 
 			pframe = pxmitframe->buf_addr + hw_hdr_offset;
 
@@ -961,11 +961,11 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
 					/* DBG_8723A("tx ampdu seqnum(%d) < tx_seq(%d)\n", pattrib->seqnum, tx_seq); */
 					pattrib->ampdu_en = false;/* AGG BK */
 				} else if (SN_EQUAL(pattrib->seqnum, tx_seq)) {
-					psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq+1)&0xfff;
+					psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (tx_seq + 1)&0xfff;
 					pattrib->ampdu_en = true;/* AGG EN */
 				} else {
 					/* DBG_8723A("tx ampdu over run\n"); */
-					psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum+1)&0xfff;
+					psta->BA_starting_seqctrl[pattrib->priority & 0x0f] = (pattrib->seqnum + 1)&0xfff;
 					pattrib->ampdu_en = true;/* AGG EN */
 				}
 			}
@@ -1164,8 +1164,8 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 			RT_TRACE(_module_rtl871x_xmit_c_, _drv_notice_,
 				 "rtw_xmiaframe_coalesce23a: keyid =%d pattrib->iv[3]=%.2x pframe =%.2x %.2x %.2x %.2x\n",
 				 padapter->securitypriv.dot11PrivacyKeyIndex,
-				 pattrib->iv[3], *pframe, *(pframe+1),
-				 *(pframe+2), *(pframe+3));
+				 pattrib->iv[3], *pframe, *(pframe + 1),
+				 *(pframe + 2), *(pframe + 3));
 			pframe += pattrib->iv_len;
 			mpdu_len -= pattrib->iv_len;
 		}
@@ -1190,7 +1190,7 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 		pdata += mem_sz;
 		data_len -= mem_sz;
 
-		if ((pattrib->icv_len >0) && (pattrib->bswenc)) {
+		if ((pattrib->icv_len > 0) && (pattrib->bswenc)) {
 			memcpy(pframe, pattrib->icv, pattrib->icv_len);
 			pframe += pattrib->icv_len;
 		}
-- 
2.9.2


^ permalink raw reply related

* Re: [PATCH 0768/1285] Replace numeric parameter like 0444 with macro
From: Arend van Spriel @ 2016-08-02 18:34 UTC (permalink / raw)
  To: Kalle Valo, Baole Ni; +Cc: linux-wireless
In-Reply-To: <87k2fz5a9c.fsf@kamboji.qca.qualcomm.com>



On 02-08-16 19:52, Kalle Valo wrote:
> (Trimming CC)
> 
> Baole Ni <baolex.ni@intel.com> writes:
> 
>> I find that the developers often just specified the numeric value
>> when calling a macro which is defined with a parameter for access permission.
>> As we know, these numeric value for access permission have had the corresponding macro,
>> and that using macro can improve the robustness and readability of the code,
>> thus, I suggest replacing the numeric parameter with the macro.
>>
>> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
>> Signed-off-by: Baole Ni <baolex.ni@intel.com>
>> ---
>>  drivers/net/wireless/ath/wcn36xx/main.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Over 1000 patches? It's totally inapproriate to spam people's inboxes
> (including mine) like this. PLEASE first study how kernel development
> works and then start with something small, not like this.
> 
> I'm dropping all patches from this patchset.

And this type of change has been tried before and burned by Linus as
everybody with a bit of *nix background knows the meaning of the
numerical access permissions.

Regards,
Arend

^ permalink raw reply

* [PATCH 1/1 v2] rtlwifi: remove superfluous condition
From: Heinrich Schuchardt @ 2016-08-02 19:26 UTC (permalink / raw)
  To: Larry Finger, Chaoming Li, Kalle Valo
  Cc: linux-wireless, netdev, linux-kernel, Heinrich Schuchardt
In-Reply-To: <4e6de16f-b42c-920a-60a2-fcc88e282dc0@lwfinger.net>

If sta == NULL, the changed line will not be reached.
So no need to check that sta != NULL here.

v2:
	fix typo

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/realtek/rtlwifi/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
index 41f77f8..7aee5ebb1 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -1135,7 +1135,7 @@ static void rtl_op_bss_info_changed(struct ieee80211_hw *hw,
 					mac->mode = WIRELESS_MODE_AC_24G;
 			}
 
-			if (vif->type == NL80211_IFTYPE_STATION && sta)
+			if (vif->type == NL80211_IFTYPE_STATION)
 				rtlpriv->cfg->ops->update_rate_tbl(hw, sta, 0);
 			rcu_read_unlock();
 
-- 
2.1.4


^ permalink raw reply related

* Re: [v4] Fix to avoid IS_ERR_VALUE and IS_ERR abuses on 64bit systems.
From: Scott Wood @ 2016-08-02 19:57 UTC (permalink / raw)
  To: arvind Yadav, Arnd Bergmann, linuxppc-dev@lists.ozlabs.org
  Cc: qiang.zhao@freescale.com, viresh.kumar@linaro.org,
	zajec5@gmail.com, linux-wireless@vger.kernel.org,
	David.Laight@aculab.com, netdev@vger.kernel.org,
	scottwood@freescale.com, akpm@linux-foundation.org,
	davem@davemloft.net, linux@roeck-us.net, Li Yang
In-Reply-To: <57A0BD8E.9050305@gmail.com>

On 08/02/2016 10:34 AM, arvind Yadav wrote:
> 
> 
> On Tuesday 02 August 2016 01:15 PM, Arnd Bergmann wrote:
>> On Monday, August 1, 2016 4:55:43 PM CEST Scott Wood wrote:
>>> On 08/01/2016 02:02 AM, Arnd Bergmann wrote:
>>>>> diff --git a/include/linux/err.h b/include/linux/err.h
>>>>> index 1e35588..c2a2789 100644
>>>>> --- a/include/linux/err.h
>>>>> +++ b/include/linux/err.h
>>>>> @@ -18,7 +18,17 @@
>>>>>  
>>>>>  #ifndef __ASSEMBLY__
>>>>>  
>>>>> -#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
>>>>> +#define IS_ERR_VALUE(x) unlikely(is_error_check(x))
>>>>> +
>>>>> +static inline int is_error_check(unsigned long error)
>>>> Please leave the existing macro alone. I think you were looking for
>>>> something specific to the return code of qe_muram_alloc() function,
>>>> so please add a helper in that subsystem if you need it, not in
>>>> the generic header files.
>>> qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.  The
>>> problem is certain callers that store the return value in a u32.  Why
>>> not just fix those callers to store it in unsigned long (at least until
>>> error checking is done)?
>>>
>> Yes, that would also address another problem with code like
>>
>>          kfree((void *)ugeth->tx_bd_ring_offset[i]);
>>
>> which is not 64-bit safe when tx_bd_ring_offset is a 32-bit value
>> that also holds the return value of qe_muram_alloc.

Well, hopefully it doesn't hold a return of qe_muram_alloc() when it's
being passed to kfree()...

There's also the code that casts kmalloc()'s return to u32, etc.
ucc_geth is not 64-bit clean in general.

>>
>> 	Arnd
> Yes, we will fix caller. Caller api is not safe on 64bit.

The API is fine (or at least, I haven't seen a valid issue pointed out
yet).  The problem is the ucc_geth driver.

> Even qe_muram_addr(a.k.a. cpm_muram_addr )passing value unsigned int,
> but it should be unsigned long.

cpm_muram_addr takes unsigned long as a parameter, not that it matters
since you can't pass errors into it and a muram offset should never
exceed 32 bits.

-Scott


^ permalink raw reply

* [PATCH] Staging: rtl8723au: rtw_xmit: fixed coding style issues
From: Shiva Kerdel @ 2016-08-02 17:57 UTC (permalink / raw)
  To: Larry.Finger
  Cc: Jes.Sorensen, gregkh, geliangtang, linux-wireless, devel,
	linux-kernel, Shiva Kerdel

Fixed some coding style issues that were detected as errors.

Signed-off-by: Shiva Kerdel <shiva@exdev.nl>
---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c b/drivers/staging/rtl8723au/core/rtw_xmit.c
index 3de40cf..4ea7c5f 100644
--- a/drivers/staging/rtl8723au/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723au/core/rtw_xmit.c
@@ -174,7 +174,7 @@ int _rtw_init_xmit_priv23a(struct xmit_priv *pxmitpriv,
 	rtw_alloc_hwxmits23a(padapter);
 	rtw_init_hwxmits23a(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
 
-	for (i = 0; i < 4; i ++)
+	for (i = 0; i < 4; i++)
 		pxmitpriv->wmm_para_seq[i] = i;
 
 	sema_init(&pxmitpriv->tx_retevt, 0);
@@ -640,7 +640,7 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 	int curfragnum, length;
 	u8 *pframe, *payload, mic[8];
-	u8 priority[4]= {0x0, 0x0, 0x0, 0x0};
+	u8 priority[4] = {0x0, 0x0, 0x0, 0x0};
 	u8 hw_hdr_offset = 0;
 	int bmcst = is_multicast_ether_addr(pattrib->ra);
 
@@ -667,10 +667,10 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 	if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
 		/* encode mic code */
 		if (stainfo) {
-			u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
-					 0x0, 0x0, 0x0, 0x0,
-					 0x0, 0x0, 0x0, 0x0,
-					 0x0, 0x0, 0x0, 0x0};
+			u8 null_key[16] = {0x0, 0x0, 0x0, 0x0,
+					   0x0, 0x0, 0x0, 0x0,
+					   0x0, 0x0, 0x0, 0x0,
+					   0x0, 0x0, 0x0, 0x0};
 
 			pframe = pxmitframe->buf_addr + hw_hdr_offset;
 
@@ -902,8 +902,7 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
 
 			if (psta->qos_option)
 				qos_option = true;
-		}
-		else {
+		} else {
 			RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
 				 "fw_state:%x is not allowed to xmit frame\n",
 				 get_fwstate(pmlmepriv));
@@ -1190,7 +1189,7 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 		pdata += mem_sz;
 		data_len -= mem_sz;
 
-		if ((pattrib->icv_len >0) && (pattrib->bswenc)) {
+		if ((pattrib->icv_len > 0) && (pattrib->bswenc)) {
 			memcpy(pframe, pattrib->icv, pattrib->icv_len);
 			pframe += pattrib->icv_len;
 		}
@@ -1766,7 +1765,6 @@ void rtw_alloc_hwxmits23a(struct rtw_adapter *padapter)
 		hwxmits[4] .sta_queue = &pxmitpriv->be_pending;
 
 	} else if (pxmitpriv->hwxmit_entry == 4) {
-
 		/* pxmitpriv->vo_txqueue.head = 0; */
 		/* hwxmits[0] .phwtxqueue = &pxmitpriv->vo_txqueue; */
 		hwxmits[0] .sta_queue = &pxmitpriv->vo_pending;
@@ -1952,18 +1950,16 @@ int xmitframe_enqueue_for_sleeping_sta23a(struct rtw_adapter *padapter, struct x
 			/* spin_unlock_bh(&psta->sleep_q.lock); */
 
 			ret = true;
-
 		}
 
 		spin_unlock_bh(&psta->sleep_q.lock);
 
 		return ret;
-
 	}
 
 	spin_lock_bh(&psta->sleep_q.lock);
 
-	if (psta->state&WIFI_SLEEP_STATE) {
+	if (psta->state & WIFI_SLEEP_STATE) {
 		u8 wmmps_ac = 0;
 
 		if (pstapriv->sta_dz_bitmap & CHKBIT(psta->aid)) {
@@ -2017,9 +2013,7 @@ int xmitframe_enqueue_for_sleeping_sta23a(struct rtw_adapter *padapter, struct x
 			/*  */
 
 			ret = true;
-
 		}
-
 	}
 
 	spin_unlock_bh(&psta->sleep_q.lock);
@@ -2159,7 +2153,7 @@ void wakeup_sta_to_xmit23a(struct rtw_adapter *padapter, struct sta_info *psta)
 		/* update BCN for TIM IE */
 		update_mask = BIT(0);
 
-		if (psta->state&WIFI_SLEEP_STATE)
+		if (psta->state & WIFI_SLEEP_STATE)
 			psta->state ^= WIFI_SLEEP_STATE;
 
 		if (psta->state & WIFI_STA_ALIVE_CHK_STATE) {
-- 
2.9.2


^ permalink raw reply related

* [PATCH v3] nl80211: Receive correct value for NL80211_MESHCONF_HT_OPMODE command
From: Masashi Honma @ 2016-08-03  1:07 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, j, me, Masashi Honma
In-Reply-To: <1468927556-4703-1-git-send-email-masashi.honma@gmail.com>

Previously, NL80211_MESHCONF_HT_OPMODE rejected correct flag
combination, ex) IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED |
IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT.

This was caused by simple comparison with value 16. This causes setting
non-existent flag (like 0x08) and invalid flag combinations. So this
commit implements some checks based on IEEE 802.11 2012 8.4.2.59 HT
Operation element.

Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
 net/wireless/nl80211.c | 42 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 46417f9..7b7530d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5380,6 +5380,7 @@ static int nl80211_parse_mesh_config(struct genl_info *info,
 {
 	struct nlattr *tb[NL80211_MESHCONF_ATTR_MAX + 1];
 	u32 mask = 0;
+	u16 ht_opmode;
 
 #define FILL_IN_MESH_PARAM_IF_SET(tb, cfg, param, min, max, mask, attr, fn) \
 do {									    \
@@ -5471,9 +5472,44 @@ do {									    \
 	FILL_IN_MESH_PARAM_IF_SET(tb, cfg, rssi_threshold, -255, 0,
 				  mask, NL80211_MESHCONF_RSSI_THRESHOLD,
 				  nl80211_check_s32);
-	FILL_IN_MESH_PARAM_IF_SET(tb, cfg, ht_opmode, 0, 16,
-				  mask, NL80211_MESHCONF_HT_OPMODE,
-				  nl80211_check_u16);
+	/*
+	 * Check HT operation mode based on
+	 * IEEE 802.11 2012 8.4.2.59 HT Operation element.
+	 */
+	if (tb[NL80211_MESHCONF_HT_OPMODE]) {
+		ht_opmode = nla_get_u16(tb[NL80211_MESHCONF_HT_OPMODE]);
+
+		if (ht_opmode & (~(IEEE80211_HT_OP_MODE_PROTECTION |
+		    IEEE80211_HT_OP_MODE_NON_GF_STA_PRSNT |
+		    IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT)))
+			return -EINVAL;
+
+		if ((ht_opmode & IEEE80211_HT_OP_MODE_NON_GF_STA_PRSNT) &&
+		    (ht_opmode & IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT))
+			return -EINVAL;
+
+		switch (ht_opmode & IEEE80211_HT_OP_MODE_PROTECTION) {
+		case IEEE80211_HT_OP_MODE_PROTECTION_NONE:
+			if (ht_opmode & IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT)
+				return -EINVAL;
+			break;
+		case IEEE80211_HT_OP_MODE_PROTECTION_NONMEMBER:
+			if (!(ht_opmode &
+			    IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT))
+				return -EINVAL;
+			break;
+		case IEEE80211_HT_OP_MODE_PROTECTION_20MHZ:
+			if (ht_opmode & IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT)
+				return -EINVAL;
+			break;
+		case IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED:
+			if (!(ht_opmode &
+			    IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT))
+				return -EINVAL;
+			break;
+		}
+		cfg->ht_opmode = ht_opmode;
+	}
 	FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPactivePathToRootTimeout,
 				  1, 65535, mask,
 				  NL80211_MESHCONF_HWMP_PATH_TO_ROOT_TIMEOUT,
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v2] nl80211: Receive correct value for NL80211_MESHCONF_HT_OPMODE command
From: Masashi Honma @ 2016-08-03  1:07 UTC (permalink / raw)
  To: masashi.honma; +Cc: johannes, linux-wireless, j, me
In-Reply-To: <1470138089-6864-1-git-send-email-masashi.honma@gmail.com>

On 2016年08月02日 20:41, Masashi Honma wrote:
> -	FILL_IN_MESH_PARAM_IF_SET(tb, cfg, ht_opmode, 0, 16,
> +	FILL_IN_MESH_PARAM_IF_SET(tb, cfg, ht_opmode, 0,
> +				  IEEE80211_HT_OP_MODE_PROTECTION |
> +				  IEEE80211_HT_OP_MODE_NON_GF_STA_PRSNT |
> +				  IEEE80211_HT_OP_MODE_NON_HT_STA_PRSNT,
>  				  mask, NL80211_MESHCONF_HT_OPMODE,
>  				  nl80211_check_u16);

This patch could over write cfg->ht_opmode even though EINVAL.
I will modify this.

Masashi Honma.

^ permalink raw reply

* Re: [PATCH v3 3/3] mac80211: mesh: fixed HT ies in beacon template
From: Masashi Honma @ 2016-08-03  2:51 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Yaniv Machani, linux-kernel, Meirav Kama, David S. Miller,
	linux-wireless, netdev
In-Reply-To: <1470122837.2665.5.camel@sipsolutions.net>

On 2016年08月02日 16:27, Johannes Berg wrote:
> This explicitly configures *HT capability* though - that's even the
> name of the parameter. If you enable HT40 in the capability, the
> resulting BSS might still not actually *use* 40 MHz bandwidth, as
> required by overlapping BSS detection.

OK, I see.

HT Capabilities element = Defined by hardware and software spec of the 
node. So it does not be modified after boot.

HT Operation element = Defined by surrounding environment and 
configuration of the node. So it could be modified after boot.

So, if the node supports HT40, HT Capabilities shows HT40 is capable.
Now, I understand why you rejected this patch.

But now, when disable_ht=1, no HT Capabilities element in beacon even 
though the node supports HT.

My trailing patch could solve the issue.

Masashi Honma.

^ permalink raw reply

* [PATCH] mac80211: Include HT Capabilities element if capable
From: Masashi Honma @ 2016-08-03  2:54 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, j, me, Masashi Honma
In-Reply-To: <1470122837.2665.5.camel@sipsolutions.net>

Previously, "HT Capabilities element" was not included in beacon and
Mesh Peering Open/Close frames when wpa_supplicant config file includes
disable_ht=1 even though HT is capable. But "HT Capabilities element"
should not be modified because it is defined by hardware and software
spec of the node.

We do not change "HT Operation element" code, because it is defined by
surrounding environment and configuration of the node. So it could be
vanished by disable_ht=1.

Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
 net/mac80211/mesh.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index c66411d..ebd4159 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -409,10 +409,7 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
 	u8 *pos;
 
 	sband = local->hw.wiphy->bands[band];
-	if (!sband->ht_cap.ht_supported ||
-	    sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_20_NOHT ||
-	    sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_5 ||
-	    sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_10)
+	if (!sband->ht_cap.ht_supported)
 		return 0;
 
 	if (skb_tailroom(skb) < 2 + sizeof(struct ieee80211_ht_cap))
-- 
2.7.4


^ permalink raw reply related

* Re: PROBLEM: network data corruption (bisected to e5a4b0bb803b)
From: Alan Curry @ 2016-08-03  3:49 UTC (permalink / raw)
  To: Al Viro
  Cc: alexmcwhirter, David Miller, rlwinm, chunkeey, linux-wireless,
	netdev, linux-kernel
In-Reply-To: <20160728012253.GT2356@ZenIV.linux.org.uk>

Al Viro wrote:
> 
> Which just might mean that we have *three* issues here -
> 	(1) buggered __copy_to_user_inatomic() (and friends) on some sparcs
> 	(2) your ssl-only corruption
> 	(3) Alan's x86_64 corruption on plain TCP read - no ssl *or* sparc
> anywhere, and no multi-segment recvmsg().  Which would strongly argue in
> favour of some kind of copy_page_to_iter() breakage triggered when handling
> a fragmented skb, as in (1).  Except that I don't see anything similar in
> x86_64 uaccess primitives...
> 

I think I've solved (3) at least...

Using the twin weapons of printk and stubbornness, I have built a working
theory of the bug. I haven't traced it all the way through, so my explanation
may be partly wrong. I do have a patch that eliminates the symptom in all my
tests though. Here's what happens:

A corrupted packet somehow arrives in skb_copy_and_csum_datagram_msg().
During downloads at reasonably high speed, about 0.1% of my incoming
packets are bad. Probably because the access point is that suspicious
Comcast thing.

skb_copy_and_csum_datagram_msg() does this:

		if (skb_copy_and_csum_datagram(skb, hlen, &msg->msg_iter,
					       chunk, &csum))
			goto fault;
		if (csum_fold(csum))
			goto csum_error;

skb_copy_and_csum_datagram() copies the bad data, computes the checksum,
and *advances the iterator*. The checksum is bad, so it goes to
csum_error, which returns without indicating success to userspace, but the
bad data is in the userspace buffer, and since the iterator has advanced,
the proper data doesn't get written to the proper place when it arrives in a
retransmission. The same iterator is still used because we're still in the
same syscall (I guess - this is one of the parts I didn't check out).

My ugly patch fixes this in the most obvious way: make a local copy of
msg->msg_iter before the call to skb_copy_and_csum_datagram(), and copy it
back if the checksum is bad, just before "goto csum_error;". (I wonder if the
other failure exits from this function might need to do the same thing.)

You can probably reproduce this problem if you deliberately inject some
bad TCP checksums into a stream. Just make sure the receiving machine is
in a blocking read() on the socket when the bad packet arrives. You may
need to resend the offending packet afterward with the checksum corrected.

diff --git a/net/core/datagram.c b/net/core/datagram.c
index b7de71f..574d4bf 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -730,6 +730,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
 {
 	__wsum csum;
 	int chunk = skb->len - hlen;
+	struct iov_iter save_iter;
 
 	if (!chunk)
 		return 0;
@@ -741,11 +742,14 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
 			goto fault;
 	} else {
 		csum = csum_partial(skb->data, hlen, skb->csum);
+		memcpy(&save_iter, &msg->msg_iter, sizeof save_iter);
 		if (skb_copy_and_csum_datagram(skb, hlen, &msg->msg_iter,
 					       chunk, &csum))
 			goto fault;
-		if (csum_fold(csum))
+		if (csum_fold(csum)) {
+			memcpy(&msg->msg_iter, &save_iter, sizeof save_iter);
 			goto csum_error;
+		}
 		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE))
 			netdev_rx_csum_fault(skb->dev);
 	}

-- 
Alan Curry

^ permalink raw reply related

* Re: [PATCH v3 3/3] mac80211: mesh: fixed HT ies in beacon template
From: Johannes Berg @ 2016-08-03  6:50 UTC (permalink / raw)
  To: Masashi Honma
  Cc: Yaniv Machani, linux-kernel, Meirav Kama, David S. Miller,
	linux-wireless, netdev
In-Reply-To: <bb69b031-53ad-4649-bd1d-e95ca7a0cc70@gmail.com>

On Wed, 2016-08-03 at 11:51 +0900, Masashi Honma wrote:
> On 2016年08月02日 16:27, Johannes Berg wrote:
> > This explicitly configures *HT capability* though - that's even the
> > name of the parameter. If you enable HT40 in the capability, the
> > resulting BSS might still not actually *use* 40 MHz bandwidth, as
> > required by overlapping BSS detection.
> 
> OK, I see.
> 
> HT Capabilities element = Defined by hardware and software spec of
> the node. So it does not be modified after boot.

It shouldn't really need to be modified, but perhaps for
interoperability reasons one might want to, like for example we do in
assoc request (we restrict our own capabilities to what the AP
supports, because some APs are stupid.)

That said, I'm basically only objecting to calling this a bugfix. If
the behaviour of restricting the information is desired, I see no real
problem with that, I just don't see how it could possibly be a bugfix.

> HT Operation element = Defined by surrounding environment and 
> configuration of the node. So it could be modified after boot.
> 
> So, if the node supports HT40, HT Capabilities shows HT40 is capable.
> Now, I understand why you rejected this patch.
> 
> But now, when disable_ht=1, no HT Capabilities element in beacon even
> though the node supports HT.
> 
> My trailing patch could solve the issue.

Actually, *this* one I'm not sure is correct. If you want to disable HT
completely, then HT operation can't actually indicate that, and having
HT capabilities without HT operation would likely just confuse peers,
so I think in this case it's quite possibly necessary to remove HT
capabilities.

johannes

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox