[PATCH 0/7] mwifiex: cleanups

[PATCH 0/7] mwifiex: cleanups

[Andreas Fenkart]

Andreas Fenkart (7):
  mwifiex: scan: simplify dereference of bss_desc fields
  mwifiex: scan: factor out has_ieee_hdr/has_vendor_hdr
  mwifiex: scan: simplify ternary operators using gnu extension
  mwifiex: scan: factor out dbg_security_flags
  mwifiex: scan: replace pointer arithmetic with array access
  mwifiex: factor out mwifiex_cancel_pending_scan_cmd
  mwifiex: make mwifiex_insert_cmd_to_free_q local static

 drivers/net/wireless/marvell/mwifiex/cmdevt.c      |  49 +++---
 drivers/net/wireless/marvell/mwifiex/main.h        |   3 +-
 drivers/net/wireless/marvell/mwifiex/scan.c        | 184 +++++++--------------
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c |  13 +-
 4 files changed, 91 insertions(+), 158 deletions(-)

-- 
2.7.0

[PATCH 1/7] mwifiex: scan: simplify dereference of bss_desc fields

[Andreas Fenkart]

given this structure:
struct foo {
  struct bar {
     int baz;
  }
}

these accesses are equivalent:
(*(foo->bar)).baz
foo->bar->baz

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 96 ++++++++++++++---------------
 1 file changed, 45 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index c20017c..43d7a92 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -121,8 +121,8 @@ mwifiex_is_rsn_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
 	struct ie_body *iebody;
 	u8 ret = MWIFIEX_OUI_NOT_PRESENT;
 
-	if (((bss_desc->bcn_rsn_ie) && ((*(bss_desc->bcn_rsn_ie)).
-					ieee_hdr.element_id == WLAN_EID_RSN))) {
+	if (bss_desc->bcn_rsn_ie &&
+	    (bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN)) {
 		iebody = (struct ie_body *)
 			 (((u8 *) bss_desc->bcn_rsn_ie->data) +
 			  RSN_GTK_OUI_OFFSET);
@@ -148,9 +148,9 @@ mwifiex_is_wpa_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
 	struct ie_body *iebody;
 	u8 ret = MWIFIEX_OUI_NOT_PRESENT;
 
-	if (((bss_desc->bcn_wpa_ie) &&
-	     ((*(bss_desc->bcn_wpa_ie)).vend_hdr.element_id ==
-	      WLAN_EID_VENDOR_SPECIFIC))) {
+	if (bss_desc->bcn_wpa_ie &&
+	    (bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
+	     WLAN_EID_VENDOR_SPECIFIC)) {
 		iebody = (struct ie_body *) bss_desc->bcn_wpa_ie->data;
 		oui = &mwifiex_wpa_oui[cipher][0];
 		ret = mwifiex_search_oui_in_ie(iebody, oui);
@@ -181,8 +181,8 @@ mwifiex_is_bss_wapi(struct mwifiex_private *priv,
 {
 	if (priv->sec_info.wapi_enabled &&
 	    (bss_desc->bcn_wapi_ie &&
-	     ((*(bss_desc->bcn_wapi_ie)).ieee_hdr.element_id ==
-			WLAN_EID_BSS_AC_ACCESS_DELAY))) {
+	     (bss_desc->bcn_wapi_ie->ieee_hdr.element_id ==
+	      WLAN_EID_BSS_AC_ACCESS_DELAY))) {
 		return true;
 	}
 	return false;
@@ -197,12 +197,12 @@ mwifiex_is_bss_no_sec(struct mwifiex_private *priv,
 		      struct mwifiex_bssdescriptor *bss_desc)
 {
 	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
-	    !priv->sec_info.wpa2_enabled && ((!bss_desc->bcn_wpa_ie) ||
-		((*(bss_desc->bcn_wpa_ie)).vend_hdr.element_id !=
+	    !priv->sec_info.wpa2_enabled &&
+	    (!bss_desc->bcn_wpa_ie ||
+		(bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
 		 WLAN_EID_VENDOR_SPECIFIC)) &&
-	    ((!bss_desc->bcn_rsn_ie) ||
-		((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id !=
-		 WLAN_EID_RSN)) &&
+	    (!bss_desc->bcn_rsn_ie ||
+		(bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
 	    !priv->sec_info.encryption_mode && !bss_desc->privacy) {
 		return true;
 	}
@@ -233,9 +233,10 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
 		   struct mwifiex_bssdescriptor *bss_desc)
 {
 	if (!priv->sec_info.wep_enabled && priv->sec_info.wpa_enabled &&
-	    !priv->sec_info.wpa2_enabled && ((bss_desc->bcn_wpa_ie) &&
-	    ((*(bss_desc->bcn_wpa_ie)).
-	     vend_hdr.element_id == WLAN_EID_VENDOR_SPECIFIC))
+	    !priv->sec_info.wpa2_enabled &&
+	    (bss_desc->bcn_wpa_ie &&
+	     (bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
+	      WLAN_EID_VENDOR_SPECIFIC))
 	   /*
 	    * Privacy bit may NOT be set in some APs like
 	    * LinkSys WRT54G && bss_desc->privacy
@@ -245,12 +246,10 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
 			    "info: %s: WPA:\t"
 			    "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
 			    "EncMode=%#x privacy=%#x\n", __func__,
-			    (bss_desc->bcn_wpa_ie) ?
-			    (*bss_desc->bcn_wpa_ie).
-			    vend_hdr.element_id : 0,
-			    (bss_desc->bcn_rsn_ie) ?
-			    (*bss_desc->bcn_rsn_ie).
-			    ieee_hdr.element_id : 0,
+			    bss_desc->bcn_wpa_ie ?
+			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+			    bss_desc->bcn_rsn_ie ?
+			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
 			    (priv->sec_info.wep_enabled) ? "e" : "d",
 			    (priv->sec_info.wpa_enabled) ? "e" : "d",
 			    (priv->sec_info.wpa2_enabled) ? "e" : "d",
@@ -269,11 +268,10 @@ static bool
 mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
 		    struct mwifiex_bssdescriptor *bss_desc)
 {
-	if (!priv->sec_info.wep_enabled &&
-	    !priv->sec_info.wpa_enabled &&
+	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
 	    priv->sec_info.wpa2_enabled &&
-	    ((bss_desc->bcn_rsn_ie) &&
-	     ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id == WLAN_EID_RSN))) {
+	    (bss_desc->bcn_rsn_ie &&
+	     (bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN))) {
 		/*
 		 * Privacy bit may NOT be set in some APs like
 		 * LinkSys WRT54G && bss_desc->privacy
@@ -282,12 +280,10 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
 			    "info: %s: WPA2:\t"
 			    "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
 			    "EncMode=%#x privacy=%#x\n", __func__,
-			    (bss_desc->bcn_wpa_ie) ?
-			    (*bss_desc->bcn_wpa_ie).
-			    vend_hdr.element_id : 0,
-			    (bss_desc->bcn_rsn_ie) ?
-			    (*bss_desc->bcn_rsn_ie).
-			    ieee_hdr.element_id : 0,
+			    bss_desc->bcn_wpa_ie ?
+			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+			    bss_desc->bcn_rsn_ie ?
+			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
 			    (priv->sec_info.wep_enabled) ? "e" : "d",
 			    (priv->sec_info.wpa_enabled) ? "e" : "d",
 			    (priv->sec_info.wpa2_enabled) ? "e" : "d",
@@ -308,11 +304,11 @@ mwifiex_is_bss_adhoc_aes(struct mwifiex_private *priv,
 {
 	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
 	    !priv->sec_info.wpa2_enabled &&
-	    ((!bss_desc->bcn_wpa_ie) ||
-	     ((*(bss_desc->bcn_wpa_ie)).
-	      vend_hdr.element_id != WLAN_EID_VENDOR_SPECIFIC)) &&
-	    ((!bss_desc->bcn_rsn_ie) ||
-	     ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id != WLAN_EID_RSN)) &&
+	    (!bss_desc->bcn_wpa_ie ||
+	     (bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
+	      WLAN_EID_VENDOR_SPECIFIC)) &&
+	    (!bss_desc->bcn_rsn_ie ||
+	     (bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
 	    !priv->sec_info.encryption_mode && bss_desc->privacy) {
 		return true;
 	}
@@ -329,23 +325,21 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
 {
 	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
 	    !priv->sec_info.wpa2_enabled &&
-	    ((!bss_desc->bcn_wpa_ie) ||
-	     ((*(bss_desc->bcn_wpa_ie)).
-	      vend_hdr.element_id != WLAN_EID_VENDOR_SPECIFIC)) &&
-	    ((!bss_desc->bcn_rsn_ie) ||
-	     ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id != WLAN_EID_RSN)) &&
+	    (!bss_desc->bcn_wpa_ie ||
+	     (bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
+	      WLAN_EID_VENDOR_SPECIFIC)) &&
+	    (!bss_desc->bcn_rsn_ie ||
+	     (bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
 	    priv->sec_info.encryption_mode && bss_desc->privacy) {
 		mwifiex_dbg(priv->adapter, INFO,
 			    "info: %s: dynamic\t"
 			    "WEP: wpa_ie=%#x wpa2_ie=%#x\t"
 			    "EncMode=%#x privacy=%#x\n",
 			    __func__,
-			    (bss_desc->bcn_wpa_ie) ?
-			    (*bss_desc->bcn_wpa_ie).
-			    vend_hdr.element_id : 0,
-			    (bss_desc->bcn_rsn_ie) ?
-			    (*bss_desc->bcn_rsn_ie).
-			    ieee_hdr.element_id : 0,
+			    bss_desc->bcn_wpa_ie ?
+			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+			    bss_desc->bcn_rsn_ie ?
+			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
 			    priv->sec_info.encryption_mode,
 			    bss_desc->privacy);
 		return true;
@@ -464,10 +458,10 @@ mwifiex_is_network_compatible(struct mwifiex_private *priv,
 			    "info: %s: failed: wpa_ie=%#x wpa2_ie=%#x WEP=%s\t"
 			    "WPA=%s WPA2=%s EncMode=%#x privacy=%#x\n",
 			    __func__,
-			    (bss_desc->bcn_wpa_ie) ?
-			    (*bss_desc->bcn_wpa_ie).vend_hdr.element_id : 0,
-			    (bss_desc->bcn_rsn_ie) ?
-			    (*bss_desc->bcn_rsn_ie).ieee_hdr.element_id : 0,
+			    bss_desc->bcn_wpa_ie ?
+			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+			    bss_desc->bcn_rsn_ie ?
+			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
 			    (priv->sec_info.wep_enabled) ? "e" : "d",
 			    (priv->sec_info.wpa_enabled) ? "e" : "d",
 			    (priv->sec_info.wpa2_enabled) ? "e" : "d",
-- 
2.7.0

[PATCH 2/7] mwifiex: scan: factor out has_ieee_hdr/has_vendor_hdr

[Andreas Fenkart]

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 52 +++++++++++++----------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 43d7a92..276382e 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -76,6 +76,18 @@ static u8 mwifiex_rsn_oui[CIPHER_SUITE_MAX][4] = {
 	{ 0x00, 0x0f, 0xac, 0x04 },	/* AES  */
 };
 
+static bool
+has_ieee_hdr(struct ieee_types_generic *ie, u8 key)
+{
+	return (ie && (ie->ieee_hdr.element_id == key));
+}
+
+static bool
+has_vendor_hdr(struct ieee_types_vendor_specific *ie, u8 key)
+{
+	return (ie && (ie->vend_hdr.element_id == key));
+}
+
 /*
  * This function parses a given IE for a given OUI.
  *
@@ -121,8 +133,7 @@ mwifiex_is_rsn_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
 	struct ie_body *iebody;
 	u8 ret = MWIFIEX_OUI_NOT_PRESENT;
 
-	if (bss_desc->bcn_rsn_ie &&
-	    (bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN)) {
+	if (has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN)) {
 		iebody = (struct ie_body *)
 			 (((u8 *) bss_desc->bcn_rsn_ie->data) +
 			  RSN_GTK_OUI_OFFSET);
@@ -148,9 +159,7 @@ mwifiex_is_wpa_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
 	struct ie_body *iebody;
 	u8 ret = MWIFIEX_OUI_NOT_PRESENT;
 
-	if (bss_desc->bcn_wpa_ie &&
-	    (bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
-	     WLAN_EID_VENDOR_SPECIFIC)) {
+	if (has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC)) {
 		iebody = (struct ie_body *) bss_desc->bcn_wpa_ie->data;
 		oui = &mwifiex_wpa_oui[cipher][0];
 		ret = mwifiex_search_oui_in_ie(iebody, oui);
@@ -180,11 +189,8 @@ mwifiex_is_bss_wapi(struct mwifiex_private *priv,
 		    struct mwifiex_bssdescriptor *bss_desc)
 {
 	if (priv->sec_info.wapi_enabled &&
-	    (bss_desc->bcn_wapi_ie &&
-	     (bss_desc->bcn_wapi_ie->ieee_hdr.element_id ==
-	      WLAN_EID_BSS_AC_ACCESS_DELAY))) {
+	    has_ieee_hdr(bss_desc->bcn_wapi_ie, WLAN_EID_BSS_AC_ACCESS_DELAY))
 		return true;
-	}
 	return false;
 }
 
@@ -198,11 +204,8 @@ mwifiex_is_bss_no_sec(struct mwifiex_private *priv,
 {
 	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
 	    !priv->sec_info.wpa2_enabled &&
-	    (!bss_desc->bcn_wpa_ie ||
-		(bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
-		 WLAN_EID_VENDOR_SPECIFIC)) &&
-	    (!bss_desc->bcn_rsn_ie ||
-		(bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
+	    !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
+	    !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
 	    !priv->sec_info.encryption_mode && !bss_desc->privacy) {
 		return true;
 	}
@@ -234,9 +237,7 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
 {
 	if (!priv->sec_info.wep_enabled && priv->sec_info.wpa_enabled &&
 	    !priv->sec_info.wpa2_enabled &&
-	    (bss_desc->bcn_wpa_ie &&
-	     (bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
-	      WLAN_EID_VENDOR_SPECIFIC))
+	    has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC)
 	   /*
 	    * Privacy bit may NOT be set in some APs like
 	    * LinkSys WRT54G && bss_desc->privacy
@@ -270,8 +271,7 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
 {
 	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
 	    priv->sec_info.wpa2_enabled &&
-	    (bss_desc->bcn_rsn_ie &&
-	     (bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN))) {
+	    has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN)) {
 		/*
 		 * Privacy bit may NOT be set in some APs like
 		 * LinkSys WRT54G && bss_desc->privacy
@@ -304,11 +304,8 @@ mwifiex_is_bss_adhoc_aes(struct mwifiex_private *priv,
 {
 	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
 	    !priv->sec_info.wpa2_enabled &&
-	    (!bss_desc->bcn_wpa_ie ||
-	     (bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
-	      WLAN_EID_VENDOR_SPECIFIC)) &&
-	    (!bss_desc->bcn_rsn_ie ||
-	     (bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
+	    !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
+	    !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
 	    !priv->sec_info.encryption_mode && bss_desc->privacy) {
 		return true;
 	}
@@ -325,11 +322,8 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
 {
 	if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
 	    !priv->sec_info.wpa2_enabled &&
-	    (!bss_desc->bcn_wpa_ie ||
-	     (bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
-	      WLAN_EID_VENDOR_SPECIFIC)) &&
-	    (!bss_desc->bcn_rsn_ie ||
-	     (bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
+	    !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
+	    !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
 	    priv->sec_info.encryption_mode && bss_desc->privacy) {
 		mwifiex_dbg(priv->adapter, INFO,
 			    "info: %s: dynamic\t"
-- 
2.7.0

[PATCH 3/7] mwifiex: scan: simplify ternary operators using gnu extension

[Andreas Fenkart]

"x ? x : y" can be simplified as "x ? : y"
https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html#Conditionals

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 276382e..f612c1b 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -845,14 +845,11 @@ mwifiex_config_scan(struct mwifiex_private *priv,
 		/* Set the BSS type scan filter, use Adapter setting if
 		   unset */
 		scan_cfg_out->bss_mode =
-			(user_scan_in->bss_mode ? (u8) user_scan_in->
-			 bss_mode : (u8) adapter->scan_mode);
+			(u8)(user_scan_in->bss_mode ?: adapter->scan_mode);
 
 		/* Set the number of probes to send, use Adapter setting
 		   if unset */
-		num_probes =
-			(user_scan_in->num_probes ? user_scan_in->
-			 num_probes : adapter->scan_probes);
+		num_probes = user_scan_in->num_probes ?: adapter->scan_probes;
 
 		/*
 		 * Set the BSSID filter to the incoming configuration,
-- 
2.7.0

[PATCH 4/7] mwifiex: scan: factor out dbg_security_flags

[Andreas Fenkart]

merge copy/paste code

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 73 ++++++++++-------------------
 1 file changed, 24 insertions(+), 49 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index f612c1b..5588750 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -76,6 +76,26 @@ static u8 mwifiex_rsn_oui[CIPHER_SUITE_MAX][4] = {
 	{ 0x00, 0x0f, 0xac, 0x04 },	/* AES  */
 };
 
+static void
+_dbg_security_flags(int log_level, struct mwifiex_private *priv,
+		    struct mwifiex_bssdescriptor *bss_desc)
+{
+	_mwifiex_dbg(priv->adapter, log_level,
+		     "info: %s: WPA:\twpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\tEncMode=%#x privacy=%#x\n",
+		     __func__,
+		     bss_desc->bcn_wpa_ie ?
+		     bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
+		     bss_desc->bcn_rsn_ie ?
+		     bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
+		     priv->sec_info.wep_enabled ? "e" : "d",
+		     priv->sec_info.wpa_enabled ? "e" : "d",
+		     priv->sec_info.wpa2_enabled ? "e" : "d",
+		     priv->sec_info.encryption_mode,
+		     bss_desc->privacy);
+}
+#define dbg_security_flags(mask, priv, bss_desc) \
+	_dbg_security_flags(MWIFIEX_DBG_##mask, priv, bss_desc)
+
 static bool
 has_ieee_hdr(struct ieee_types_generic *ie, u8 key)
 {
@@ -243,19 +263,7 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
 	    * LinkSys WRT54G && bss_desc->privacy
 	    */
 	 ) {
-		mwifiex_dbg(priv->adapter, INFO,
-			    "info: %s: WPA:\t"
-			    "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
-			    "EncMode=%#x privacy=%#x\n", __func__,
-			    bss_desc->bcn_wpa_ie ?
-			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
-			    bss_desc->bcn_rsn_ie ?
-			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
-			    (priv->sec_info.wep_enabled) ? "e" : "d",
-			    (priv->sec_info.wpa_enabled) ? "e" : "d",
-			    (priv->sec_info.wpa2_enabled) ? "e" : "d",
-			    priv->sec_info.encryption_mode,
-			    bss_desc->privacy);
+		dbg_security_flags(INFO, priv, bss_desc);
 		return true;
 	}
 	return false;
@@ -276,19 +284,7 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
 		 * Privacy bit may NOT be set in some APs like
 		 * LinkSys WRT54G && bss_desc->privacy
 		 */
-		mwifiex_dbg(priv->adapter, INFO,
-			    "info: %s: WPA2:\t"
-			    "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
-			    "EncMode=%#x privacy=%#x\n", __func__,
-			    bss_desc->bcn_wpa_ie ?
-			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
-			    bss_desc->bcn_rsn_ie ?
-			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
-			    (priv->sec_info.wep_enabled) ? "e" : "d",
-			    (priv->sec_info.wpa_enabled) ? "e" : "d",
-			    (priv->sec_info.wpa2_enabled) ? "e" : "d",
-			    priv->sec_info.encryption_mode,
-			    bss_desc->privacy);
+		dbg_security_flags(INFO, priv, bss_desc);
 		return true;
 	}
 	return false;
@@ -325,17 +321,7 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
 	    !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
 	    !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
 	    priv->sec_info.encryption_mode && bss_desc->privacy) {
-		mwifiex_dbg(priv->adapter, INFO,
-			    "info: %s: dynamic\t"
-			    "WEP: wpa_ie=%#x wpa2_ie=%#x\t"
-			    "EncMode=%#x privacy=%#x\n",
-			    __func__,
-			    bss_desc->bcn_wpa_ie ?
-			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
-			    bss_desc->bcn_rsn_ie ?
-			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
-			    priv->sec_info.encryption_mode,
-			    bss_desc->privacy);
+		dbg_security_flags(INFO, priv, bss_desc);
 		return true;
 	}
 	return false;
@@ -448,18 +434,7 @@ mwifiex_is_network_compatible(struct mwifiex_private *priv,
 		}
 
 		/* Security doesn't match */
-		mwifiex_dbg(adapter, ERROR,
-			    "info: %s: failed: wpa_ie=%#x wpa2_ie=%#x WEP=%s\t"
-			    "WPA=%s WPA2=%s EncMode=%#x privacy=%#x\n",
-			    __func__,
-			    bss_desc->bcn_wpa_ie ?
-			    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
-			    bss_desc->bcn_rsn_ie ?
-			    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
-			    (priv->sec_info.wep_enabled) ? "e" : "d",
-			    (priv->sec_info.wpa_enabled) ? "e" : "d",
-			    (priv->sec_info.wpa2_enabled) ? "e" : "d",
-			    priv->sec_info.encryption_mode, bss_desc->privacy);
+		dbg_security_flags(ERROR, priv, bss_desc);
 		return -1;
 	}
 
-- 
2.7.0

[PATCH 5/7] mwifiex: scan: replace pointer arithmetic with array access

[Andreas Fenkart]

improves readability

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 5588750..6ddc98b 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -999,27 +999,24 @@ mwifiex_config_scan(struct mwifiex_private *priv,
 		     chan_idx++) {
 
 			channel = user_scan_in->chan_list[chan_idx].chan_number;
-			(scan_chan_list + chan_idx)->chan_number = channel;
+			scan_chan_list[chan_idx].chan_number = channel;
 
 			radio_type =
 				user_scan_in->chan_list[chan_idx].radio_type;
-			(scan_chan_list + chan_idx)->radio_type = radio_type;
+			scan_chan_list[chan_idx].radio_type = radio_type;
 
 			scan_type = user_scan_in->chan_list[chan_idx].scan_type;
 
 			if (scan_type == MWIFIEX_SCAN_TYPE_PASSIVE)
-				(scan_chan_list +
-				 chan_idx)->chan_scan_mode_bitmap
+				scan_chan_list[chan_idx].chan_scan_mode_bitmap
 					|= (MWIFIEX_PASSIVE_SCAN |
 					    MWIFIEX_HIDDEN_SSID_REPORT);
 			else
-				(scan_chan_list +
-				 chan_idx)->chan_scan_mode_bitmap
+				scan_chan_list[chan_idx].chan_scan_mode_bitmap
 					&= ~MWIFIEX_PASSIVE_SCAN;
 
 			if (*filtered_scan)
-				(scan_chan_list +
-				 chan_idx)->chan_scan_mode_bitmap
+				scan_chan_list[chan_idx].chan_scan_mode_bitmap
 					|= MWIFIEX_DISABLE_CHAN_FILT;
 
 			if (user_scan_in->chan_list[chan_idx].scan_time) {
@@ -1034,9 +1031,9 @@ mwifiex_config_scan(struct mwifiex_private *priv,
 					scan_dur = adapter->active_scan_time;
 			}
 
-			(scan_chan_list + chan_idx)->min_scan_time =
+			scan_chan_list[chan_idx].min_scan_time =
 				cpu_to_le16(scan_dur);
-			(scan_chan_list + chan_idx)->max_scan_time =
+			scan_chan_list[chan_idx].max_scan_time =
 				cpu_to_le16(scan_dur);
 		}
 
-- 
2.7.0

[PATCH 6/7] mwifiex: factor out mwifiex_cancel_pending_scan_cmd

[Andreas Fenkart]

releasing the scan_pending lock in mwifiex_check_next_scan_command
is valid, since the lock is taken again, and all nodes removed
from the scan_pending queue.

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c      | 43 ++++++++++------------
 drivers/net/wireless/marvell/mwifiex/main.h        |  1 +
 drivers/net/wireless/marvell/mwifiex/scan.c        | 23 +++---------
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 13 +------
 4 files changed, 27 insertions(+), 53 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index cb25aa7..61426b3 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -991,6 +991,23 @@ mwifiex_cmd_timeout_func(unsigned long function_context)
 		adapter->if_ops.card_reset(adapter);
 }
 
+void
+mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter)
+{
+	struct cmd_ctrl_node *cmd_node = NULL, *tmp_node;
+	unsigned long flags;
+
+	/* Cancel all pending scan command */
+	spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
+	list_for_each_entry_safe(cmd_node, tmp_node,
+				 &adapter->scan_pending_q, list) {
+		list_del(&cmd_node->list);
+		cmd_node->wait_q_enabled = false;
+		mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
+	}
+	spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+}
+
 /*
  * This function cancels all the pending commands.
  *
@@ -1029,16 +1046,7 @@ mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter)
 	spin_unlock_irqrestore(&adapter->cmd_pending_q_lock, flags);
 	spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, cmd_flags);
 
-	/* Cancel all pending scan command */
-	spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
-	list_for_each_entry_safe(cmd_node, tmp_node,
-				 &adapter->scan_pending_q, list) {
-		list_del(&cmd_node->list);
-
-		cmd_node->wait_q_enabled = false;
-		mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
-	}
-	spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+	mwifiex_cancel_pending_scan_cmd(adapter);
 
 	if (adapter->scan_processing) {
 		spin_lock_irqsave(&adapter->mwifiex_cmd_lock, cmd_flags);
@@ -1070,9 +1078,8 @@ mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter)
 void
 mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
 {
-	struct cmd_ctrl_node *cmd_node = NULL, *tmp_node = NULL;
+	struct cmd_ctrl_node *cmd_node = NULL;
 	unsigned long cmd_flags;
-	unsigned long scan_pending_q_flags;
 	struct mwifiex_private *priv;
 	int i;
 
@@ -1094,17 +1101,7 @@ mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
 		mwifiex_recycle_cmd_node(adapter, cmd_node);
 	}
 
-	/* Cancel all pending scan command */
-	spin_lock_irqsave(&adapter->scan_pending_q_lock,
-			  scan_pending_q_flags);
-	list_for_each_entry_safe(cmd_node, tmp_node,
-				 &adapter->scan_pending_q, list) {
-		list_del(&cmd_node->list);
-		cmd_node->wait_q_enabled = false;
-		mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
-	}
-	spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
-			       scan_pending_q_flags);
+	mwifiex_cancel_pending_scan_cmd(adapter);
 
 	if (adapter->scan_processing) {
 		spin_lock_irqsave(&adapter->mwifiex_cmd_lock, cmd_flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 2f7f478..f71f894 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1043,6 +1043,7 @@ int mwifiex_alloc_cmd_buffer(struct mwifiex_adapter *adapter);
 int mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
+void mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter);
 
 void mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
 				  struct cmd_ctrl_node *cmd_node);
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 6ddc98b..490d0d1 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -563,8 +563,6 @@ mwifiex_scan_channel_list(struct mwifiex_private *priv,
 	int ret = 0;
 	struct mwifiex_chan_scan_param_set *tmp_chan_list;
 	struct mwifiex_chan_scan_param_set *start_chan;
-	struct cmd_ctrl_node *cmd_node, *tmp_node;
-	unsigned long flags;
 	u32 tlv_idx, rates_size, cmd_no;
 	u32 total_scan_time;
 	u32 done_early;
@@ -721,16 +719,7 @@ mwifiex_scan_channel_list(struct mwifiex_private *priv,
 			    sizeof(struct mwifiex_ie_types_header) + rates_size;
 
 		if (ret) {
-			spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
-			list_for_each_entry_safe(cmd_node, tmp_node,
-						 &adapter->scan_pending_q,
-						 list) {
-				list_del(&cmd_node->list);
-				cmd_node->wait_q_enabled = false;
-				mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
-			}
-			spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
-					       flags);
+			mwifiex_cancel_pending_scan_cmd(adapter);
 			break;
 		}
 	}
@@ -1893,12 +1882,13 @@ mwifiex_active_scan_req_for_passive_chan(struct mwifiex_private *priv)
 static void mwifiex_check_next_scan_command(struct mwifiex_private *priv)
 {
 	struct mwifiex_adapter *adapter = priv->adapter;
-	struct cmd_ctrl_node *cmd_node, *tmp_node;
+	struct cmd_ctrl_node *cmd_node;
 	unsigned long flags;
 
 	spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
 	if (list_empty(&adapter->scan_pending_q)) {
 		spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+
 		spin_lock_irqsave(&adapter->mwifiex_cmd_lock, flags);
 		adapter->scan_processing = false;
 		spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, flags);
@@ -1920,13 +1910,10 @@ static void mwifiex_check_next_scan_command(struct mwifiex_private *priv)
 		}
 	} else if ((priv->scan_aborting && !priv->scan_request) ||
 		   priv->scan_block) {
-		list_for_each_entry_safe(cmd_node, tmp_node,
-					 &adapter->scan_pending_q, list) {
-			list_del(&cmd_node->list);
-			mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
-		}
 		spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
 
+		mwifiex_cancel_pending_scan_cmd(adapter);
+
 		spin_lock_irqsave(&adapter->mwifiex_cmd_lock, flags);
 		adapter->scan_processing = false;
 		spin_unlock_irqrestore(&adapter->mwifiex_cmd_lock, flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 9ac7aa2..81e23d8 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -44,7 +44,6 @@ static void
 mwifiex_process_cmdresp_error(struct mwifiex_private *priv,
 			      struct host_cmd_ds_command *resp)
 {
-	struct cmd_ctrl_node *cmd_node = NULL, *tmp_node;
 	struct mwifiex_adapter *adapter = priv->adapter;
 	struct host_cmd_ds_802_11_ps_mode_enh *pm;
 	unsigned long flags;
@@ -71,17 +70,7 @@ mwifiex_process_cmdresp_error(struct mwifiex_private *priv,
 		break;
 	case HostCmd_CMD_802_11_SCAN:
 	case HostCmd_CMD_802_11_SCAN_EXT:
-		/* Cancel all pending scan command */
-		spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
-		list_for_each_entry_safe(cmd_node, tmp_node,
-					 &adapter->scan_pending_q, list) {
-			list_del(&cmd_node->list);
-			spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
-					       flags);
-			mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
-			spin_lock_irqsave(&adapter->scan_pending_q_lock, flags);
-		}
-		spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
+		mwifiex_cancel_pending_scan_cmd(adapter);
 
 		spin_lock_irqsave(&adapter->mwifiex_cmd_lock, flags);
 		adapter->scan_processing = false;
-- 
2.7.0

[PATCH 7/7] mwifiex: make mwifiex_insert_cmd_to_free_q local static

[Andreas Fenkart]

after factoring out mwifiex_cancel_pending_scan_cmd
the function is noc called outside of cmdevt file

Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c | 6 +++++-
 drivers/net/wireless/marvell/mwifiex/main.h   | 2 --
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 61426b3..4b0f44d 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -26,6 +26,10 @@
 #include "11n.h"
 #include "11ac.h"
 
+static void
+mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
+			     struct cmd_ctrl_node *cmd_node);
+
 /*
  * This function initializes a command node.
  *
@@ -619,7 +623,7 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
  * The function also calls the completion callback if required, before
  * cleaning the command node and re-inserting it into the free queue.
  */
-void
+static void
 mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
 			     struct cmd_ctrl_node *cmd_node)
 {
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index f71f894..840e5d4 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1045,8 +1045,6 @@ void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter);
 
-void mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
-				  struct cmd_ctrl_node *cmd_node);
 void mwifiex_recycle_cmd_node(struct mwifiex_adapter *adapter,
 			      struct cmd_ctrl_node *cmd_node);
 
-- 
2.7.0

Re: [PATCH 1/7] mwifiex: scan: simplify dereference of bss_desc fields

[Julian Calaby]

Hi All,

On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
> given this structure:
> struct foo {
>   struct bar {
>      int baz;
>   }
> }
>
> these accesses are equivalent:
> (*(foo->bar)).baz
> foo->bar->baz
>
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>

You leave a lot of unnecessary parenthesis in the conditions, i.e.

if (ie && (ie->iee_hdr.element_id == WLAN_EID_RSN))

is equivalent to

if (ie && ie->iee_hdr.element_id == WLAN_EID_RSN)

however I think you remove most of these in subsequent patches.

Other than that, it's a good cleanup.

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 96 ++++++++++++++---------------
>  1 file changed, 45 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index c20017c..43d7a92 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -121,8 +121,8 @@ mwifiex_is_rsn_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
>         struct ie_body *iebody;
>         u8 ret = MWIFIEX_OUI_NOT_PRESENT;
>
> -       if (((bss_desc->bcn_rsn_ie) && ((*(bss_desc->bcn_rsn_ie)).
> -                                       ieee_hdr.element_id == WLAN_EID_RSN))) {
> +       if (bss_desc->bcn_rsn_ie &&
> +           (bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN)) {
>                 iebody = (struct ie_body *)
>                          (((u8 *) bss_desc->bcn_rsn_ie->data) +
>                           RSN_GTK_OUI_OFFSET);
> @@ -148,9 +148,9 @@ mwifiex_is_wpa_oui_present(struct mwifiex_bssdescriptor *bss_desc, u32 cipher)
>         struct ie_body *iebody;
>         u8 ret = MWIFIEX_OUI_NOT_PRESENT;
>
> -       if (((bss_desc->bcn_wpa_ie) &&
> -            ((*(bss_desc->bcn_wpa_ie)).vend_hdr.element_id ==
> -             WLAN_EID_VENDOR_SPECIFIC))) {
> +       if (bss_desc->bcn_wpa_ie &&
> +           (bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
> +            WLAN_EID_VENDOR_SPECIFIC)) {
>                 iebody = (struct ie_body *) bss_desc->bcn_wpa_ie->data;
>                 oui = &mwifiex_wpa_oui[cipher][0];
>                 ret = mwifiex_search_oui_in_ie(iebody, oui);
> @@ -181,8 +181,8 @@ mwifiex_is_bss_wapi(struct mwifiex_private *priv,
>  {
>         if (priv->sec_info.wapi_enabled &&
>             (bss_desc->bcn_wapi_ie &&
> -            ((*(bss_desc->bcn_wapi_ie)).ieee_hdr.element_id ==
> -                       WLAN_EID_BSS_AC_ACCESS_DELAY))) {
> +            (bss_desc->bcn_wapi_ie->ieee_hdr.element_id ==
> +             WLAN_EID_BSS_AC_ACCESS_DELAY))) {
>                 return true;
>         }
>         return false;
> @@ -197,12 +197,12 @@ mwifiex_is_bss_no_sec(struct mwifiex_private *priv,
>                       struct mwifiex_bssdescriptor *bss_desc)
>  {
>         if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
> -           !priv->sec_info.wpa2_enabled && ((!bss_desc->bcn_wpa_ie) ||
> -               ((*(bss_desc->bcn_wpa_ie)).vend_hdr.element_id !=
> +           !priv->sec_info.wpa2_enabled &&
> +           (!bss_desc->bcn_wpa_ie ||
> +               (bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
>                  WLAN_EID_VENDOR_SPECIFIC)) &&
> -           ((!bss_desc->bcn_rsn_ie) ||
> -               ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id !=
> -                WLAN_EID_RSN)) &&
> +           (!bss_desc->bcn_rsn_ie ||
> +               (bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
>             !priv->sec_info.encryption_mode && !bss_desc->privacy) {
>                 return true;
>         }
> @@ -233,9 +233,10 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
>                    struct mwifiex_bssdescriptor *bss_desc)
>  {
>         if (!priv->sec_info.wep_enabled && priv->sec_info.wpa_enabled &&
> -           !priv->sec_info.wpa2_enabled && ((bss_desc->bcn_wpa_ie) &&
> -           ((*(bss_desc->bcn_wpa_ie)).
> -            vend_hdr.element_id == WLAN_EID_VENDOR_SPECIFIC))
> +           !priv->sec_info.wpa2_enabled &&
> +           (bss_desc->bcn_wpa_ie &&
> +            (bss_desc->bcn_wpa_ie->vend_hdr.element_id ==
> +             WLAN_EID_VENDOR_SPECIFIC))
>            /*
>             * Privacy bit may NOT be set in some APs like
>             * LinkSys WRT54G && bss_desc->privacy
> @@ -245,12 +246,10 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
>                             "info: %s: WPA:\t"
>                             "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
>                             "EncMode=%#x privacy=%#x\n", __func__,
> -                           (bss_desc->bcn_wpa_ie) ?
> -                           (*bss_desc->bcn_wpa_ie).
> -                           vend_hdr.element_id : 0,
> -                           (bss_desc->bcn_rsn_ie) ?
> -                           (*bss_desc->bcn_rsn_ie).
> -                           ieee_hdr.element_id : 0,
> +                           bss_desc->bcn_wpa_ie ?
> +                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> +                           bss_desc->bcn_rsn_ie ?
> +                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
>                             (priv->sec_info.wep_enabled) ? "e" : "d",
>                             (priv->sec_info.wpa_enabled) ? "e" : "d",
>                             (priv->sec_info.wpa2_enabled) ? "e" : "d",
> @@ -269,11 +268,10 @@ static bool
>  mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
>                     struct mwifiex_bssdescriptor *bss_desc)
>  {
> -       if (!priv->sec_info.wep_enabled &&
> -           !priv->sec_info.wpa_enabled &&
> +       if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
>             priv->sec_info.wpa2_enabled &&
> -           ((bss_desc->bcn_rsn_ie) &&
> -            ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id == WLAN_EID_RSN))) {
> +           (bss_desc->bcn_rsn_ie &&
> +            (bss_desc->bcn_rsn_ie->ieee_hdr.element_id == WLAN_EID_RSN))) {
>                 /*
>                  * Privacy bit may NOT be set in some APs like
>                  * LinkSys WRT54G && bss_desc->privacy
> @@ -282,12 +280,10 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
>                             "info: %s: WPA2:\t"
>                             "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
>                             "EncMode=%#x privacy=%#x\n", __func__,
> -                           (bss_desc->bcn_wpa_ie) ?
> -                           (*bss_desc->bcn_wpa_ie).
> -                           vend_hdr.element_id : 0,
> -                           (bss_desc->bcn_rsn_ie) ?
> -                           (*bss_desc->bcn_rsn_ie).
> -                           ieee_hdr.element_id : 0,
> +                           bss_desc->bcn_wpa_ie ?
> +                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> +                           bss_desc->bcn_rsn_ie ?
> +                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
>                             (priv->sec_info.wep_enabled) ? "e" : "d",
>                             (priv->sec_info.wpa_enabled) ? "e" : "d",
>                             (priv->sec_info.wpa2_enabled) ? "e" : "d",
> @@ -308,11 +304,11 @@ mwifiex_is_bss_adhoc_aes(struct mwifiex_private *priv,
>  {
>         if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
>             !priv->sec_info.wpa2_enabled &&
> -           ((!bss_desc->bcn_wpa_ie) ||
> -            ((*(bss_desc->bcn_wpa_ie)).
> -             vend_hdr.element_id != WLAN_EID_VENDOR_SPECIFIC)) &&
> -           ((!bss_desc->bcn_rsn_ie) ||
> -            ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id != WLAN_EID_RSN)) &&
> +           (!bss_desc->bcn_wpa_ie ||
> +            (bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
> +             WLAN_EID_VENDOR_SPECIFIC)) &&
> +           (!bss_desc->bcn_rsn_ie ||
> +            (bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
>             !priv->sec_info.encryption_mode && bss_desc->privacy) {
>                 return true;
>         }
> @@ -329,23 +325,21 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
>  {
>         if (!priv->sec_info.wep_enabled && !priv->sec_info.wpa_enabled &&
>             !priv->sec_info.wpa2_enabled &&
> -           ((!bss_desc->bcn_wpa_ie) ||
> -            ((*(bss_desc->bcn_wpa_ie)).
> -             vend_hdr.element_id != WLAN_EID_VENDOR_SPECIFIC)) &&
> -           ((!bss_desc->bcn_rsn_ie) ||
> -            ((*(bss_desc->bcn_rsn_ie)).ieee_hdr.element_id != WLAN_EID_RSN)) &&
> +           (!bss_desc->bcn_wpa_ie ||
> +            (bss_desc->bcn_wpa_ie->vend_hdr.element_id !=
> +             WLAN_EID_VENDOR_SPECIFIC)) &&
> +           (!bss_desc->bcn_rsn_ie ||
> +            (bss_desc->bcn_rsn_ie->ieee_hdr.element_id != WLAN_EID_RSN)) &&
>             priv->sec_info.encryption_mode && bss_desc->privacy) {
>                 mwifiex_dbg(priv->adapter, INFO,
>                             "info: %s: dynamic\t"
>                             "WEP: wpa_ie=%#x wpa2_ie=%#x\t"
>                             "EncMode=%#x privacy=%#x\n",
>                             __func__,
> -                           (bss_desc->bcn_wpa_ie) ?
> -                           (*bss_desc->bcn_wpa_ie).
> -                           vend_hdr.element_id : 0,
> -                           (bss_desc->bcn_rsn_ie) ?
> -                           (*bss_desc->bcn_rsn_ie).
> -                           ieee_hdr.element_id : 0,
> +                           bss_desc->bcn_wpa_ie ?
> +                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> +                           bss_desc->bcn_rsn_ie ?
> +                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
>                             priv->sec_info.encryption_mode,
>                             bss_desc->privacy);
>                 return true;
> @@ -464,10 +458,10 @@ mwifiex_is_network_compatible(struct mwifiex_private *priv,
>                             "info: %s: failed: wpa_ie=%#x wpa2_ie=%#x WEP=%s\t"
>                             "WPA=%s WPA2=%s EncMode=%#x privacy=%#x\n",
>                             __func__,
> -                           (bss_desc->bcn_wpa_ie) ?
> -                           (*bss_desc->bcn_wpa_ie).vend_hdr.element_id : 0,
> -                           (bss_desc->bcn_rsn_ie) ?
> -                           (*bss_desc->bcn_rsn_ie).ieee_hdr.element_id : 0,
> +                           bss_desc->bcn_wpa_ie ?
> +                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> +                           bss_desc->bcn_rsn_ie ?
> +                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
>                             (priv->sec_info.wep_enabled) ? "e" : "d",
>                             (priv->sec_info.wpa_enabled) ? "e" : "d",
>                             (priv->sec_info.wpa2_enabled) ? "e" : "d",
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 2/7] mwifiex: scan: factor out has_ieee_hdr/has_vendor_hdr

[Julian Calaby]

Hi Andreas,

On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 52 +++++++++++++----------------
>  1 file changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 43d7a92..276382e 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -76,6 +76,18 @@ static u8 mwifiex_rsn_oui[CIPHER_SUITE_MAX][4] = {
>         { 0x00, 0x0f, 0xac, 0x04 },     /* AES  */
>  };
>
> +static bool
> +has_ieee_hdr(struct ieee_types_generic *ie, u8 key)
> +{
> +       return (ie && (ie->ieee_hdr.element_id == key));

Please remove all the parentheses in this line as they're all unnecessary.

> +}
> +
> +static bool
> +has_vendor_hdr(struct ieee_types_vendor_specific *ie, u8 key)
> +{
> +       return (ie && (ie->vend_hdr.element_id == key));

Here too.

> +}
> +
>  /*
>   * This function parses a given IE for a given OUI.
>   *

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 3/7] mwifiex: scan: simplify ternary operators using gnu extension

[Julian Calaby]

Hi Andreas,

On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
> "x ? x : y" can be simplified as "x ? : y"
> https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html#Conditionals
>
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>

I'm not sure people are 100% happy with changes like these, however
they are correct, so

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 276382e..f612c1b 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -845,14 +845,11 @@ mwifiex_config_scan(struct mwifiex_private *priv,
>                 /* Set the BSS type scan filter, use Adapter setting if
>                    unset */
>                 scan_cfg_out->bss_mode =
> -                       (user_scan_in->bss_mode ? (u8) user_scan_in->
> -                        bss_mode : (u8) adapter->scan_mode);
> +                       (u8)(user_scan_in->bss_mode ?: adapter->scan_mode);
>
>                 /* Set the number of probes to send, use Adapter setting
>                    if unset */
> -               num_probes =
> -                       (user_scan_in->num_probes ? user_scan_in->
> -                        num_probes : adapter->scan_probes);
> +               num_probes = user_scan_in->num_probes ?: adapter->scan_probes;
>
>                 /*
>                  * Set the BSSID filter to the incoming configuration,
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 4/7] mwifiex: scan: factor out dbg_security_flags

[Julian Calaby]

Hi Andreas,

On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
> merge copy/paste code

The code you delete isn't all equivalent. I'll explain below.

> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 73 ++++++++++-------------------
>  1 file changed, 24 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index f612c1b..5588750 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -76,6 +76,26 @@ static u8 mwifiex_rsn_oui[CIPHER_SUITE_MAX][4] = {
>         { 0x00, 0x0f, 0xac, 0x04 },     /* AES  */
>  };
>
> +static void
> +_dbg_security_flags(int log_level, struct mwifiex_private *priv,
> +                   struct mwifiex_bssdescriptor *bss_desc)
> +{
> +       _mwifiex_dbg(priv->adapter, log_level,
> +                    "info: %s: WPA:\twpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\tEncMode=%#x privacy=%#x\n",
> +                    __func__,

After the __func__, "WPA:" is printed in your new common function

> +                    bss_desc->bcn_wpa_ie ?
> +                    bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> +                    bss_desc->bcn_rsn_ie ?
> +                    bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
> +                    priv->sec_info.wep_enabled ? "e" : "d",
> +                    priv->sec_info.wpa_enabled ? "e" : "d",
> +                    priv->sec_info.wpa2_enabled ? "e" : "d",
> +                    priv->sec_info.encryption_mode,
> +                    bss_desc->privacy);
> +}
> +#define dbg_security_flags(mask, priv, bss_desc) \
> +       _dbg_security_flags(MWIFIEX_DBG_##mask, priv, bss_desc)
> +
>  static bool
>  has_ieee_hdr(struct ieee_types_generic *ie, u8 key)
>  {
> @@ -243,19 +263,7 @@ mwifiex_is_bss_wpa(struct mwifiex_private *priv,
>             * LinkSys WRT54G && bss_desc->privacy
>             */
>          ) {
> -               mwifiex_dbg(priv->adapter, INFO,
> -                           "info: %s: WPA:\t"

which is the same as here

> -                           "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
> -                           "EncMode=%#x privacy=%#x\n", __func__,
> -                           bss_desc->bcn_wpa_ie ?
> -                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> -                           bss_desc->bcn_rsn_ie ?
> -                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
> -                           (priv->sec_info.wep_enabled) ? "e" : "d",
> -                           (priv->sec_info.wpa_enabled) ? "e" : "d",
> -                           (priv->sec_info.wpa2_enabled) ? "e" : "d",
> -                           priv->sec_info.encryption_mode,
> -                           bss_desc->privacy);
> +               dbg_security_flags(INFO, priv, bss_desc);
>                 return true;
>         }
>         return false;
> @@ -276,19 +284,7 @@ mwifiex_is_bss_wpa2(struct mwifiex_private *priv,
>                  * Privacy bit may NOT be set in some APs like
>                  * LinkSys WRT54G && bss_desc->privacy
>                  */
> -               mwifiex_dbg(priv->adapter, INFO,
> -                           "info: %s: WPA2:\t"

but we print "WPA2:" here,

> -                           "wpa_ie=%#x wpa2_ie=%#x WEP=%s WPA=%s WPA2=%s\t"
> -                           "EncMode=%#x privacy=%#x\n", __func__,
> -                           bss_desc->bcn_wpa_ie ?
> -                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> -                           bss_desc->bcn_rsn_ie ?
> -                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
> -                           (priv->sec_info.wep_enabled) ? "e" : "d",
> -                           (priv->sec_info.wpa_enabled) ? "e" : "d",
> -                           (priv->sec_info.wpa2_enabled) ? "e" : "d",
> -                           priv->sec_info.encryption_mode,
> -                           bss_desc->privacy);
> +               dbg_security_flags(INFO, priv, bss_desc);
>                 return true;
>         }
>         return false;
> @@ -325,17 +321,7 @@ mwifiex_is_bss_dynamic_wep(struct mwifiex_private *priv,
>             !has_vendor_hdr(bss_desc->bcn_wpa_ie, WLAN_EID_VENDOR_SPECIFIC) &&
>             !has_ieee_hdr(bss_desc->bcn_rsn_ie, WLAN_EID_RSN) &&
>             priv->sec_info.encryption_mode && bss_desc->privacy) {
> -               mwifiex_dbg(priv->adapter, INFO,
> -                           "info: %s: dynamic\t"

"dynamic" here,

> -                           "WEP: wpa_ie=%#x wpa2_ie=%#x\t"
> -                           "EncMode=%#x privacy=%#x\n",
> -                           __func__,
> -                           bss_desc->bcn_wpa_ie ?
> -                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> -                           bss_desc->bcn_rsn_ie ?
> -                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
> -                           priv->sec_info.encryption_mode,
> -                           bss_desc->privacy);
> +               dbg_security_flags(INFO, priv, bss_desc);
>                 return true;
>         }
>         return false;
> @@ -448,18 +434,7 @@ mwifiex_is_network_compatible(struct mwifiex_private *priv,
>                 }
>
>                 /* Security doesn't match */
> -               mwifiex_dbg(adapter, ERROR,
> -                           "info: %s: failed: wpa_ie=%#x wpa2_ie=%#x WEP=%s\t"

and "failed" here.

> -                           "WPA=%s WPA2=%s EncMode=%#x privacy=%#x\n",
> -                           __func__,
> -                           bss_desc->bcn_wpa_ie ?
> -                           bss_desc->bcn_wpa_ie->vend_hdr.element_id : 0,
> -                           bss_desc->bcn_rsn_ie ?
> -                           bss_desc->bcn_rsn_ie->ieee_hdr.element_id : 0,
> -                           (priv->sec_info.wep_enabled) ? "e" : "d",
> -                           (priv->sec_info.wpa_enabled) ? "e" : "d",
> -                           (priv->sec_info.wpa2_enabled) ? "e" : "d",
> -                           priv->sec_info.encryption_mode, bss_desc->privacy);
> +               dbg_security_flags(ERROR, priv, bss_desc);
>                 return -1;
>         }

Significantly less info is printed in the second to last occurrence,
however that's probably not an issue. You should add an extra
parameter to your dbg_security_flags() function to capture this
difference.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 5/7] mwifiex: scan: replace pointer arithmetic with array access

[Julian Calaby]

Hi,

On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
> improves readability
>
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>

Looks correct to me.

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 5588750..6ddc98b 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -999,27 +999,24 @@ mwifiex_config_scan(struct mwifiex_private *priv,
>                      chan_idx++) {
>
>                         channel = user_scan_in->chan_list[chan_idx].chan_number;
> -                       (scan_chan_list + chan_idx)->chan_number = channel;
> +                       scan_chan_list[chan_idx].chan_number = channel;
>
>                         radio_type =
>                                 user_scan_in->chan_list[chan_idx].radio_type;
> -                       (scan_chan_list + chan_idx)->radio_type = radio_type;
> +                       scan_chan_list[chan_idx].radio_type = radio_type;
>
>                         scan_type = user_scan_in->chan_list[chan_idx].scan_type;
>
>                         if (scan_type == MWIFIEX_SCAN_TYPE_PASSIVE)
> -                               (scan_chan_list +
> -                                chan_idx)->chan_scan_mode_bitmap
> +                               scan_chan_list[chan_idx].chan_scan_mode_bitmap
>                                         |= (MWIFIEX_PASSIVE_SCAN |
>                                             MWIFIEX_HIDDEN_SSID_REPORT);
>                         else
> -                               (scan_chan_list +
> -                                chan_idx)->chan_scan_mode_bitmap
> +                               scan_chan_list[chan_idx].chan_scan_mode_bitmap
>                                         &= ~MWIFIEX_PASSIVE_SCAN;
>
>                         if (*filtered_scan)
> -                               (scan_chan_list +
> -                                chan_idx)->chan_scan_mode_bitmap
> +                               scan_chan_list[chan_idx].chan_scan_mode_bitmap
>                                         |= MWIFIEX_DISABLE_CHAN_FILT;
>
>                         if (user_scan_in->chan_list[chan_idx].scan_time) {
> @@ -1034,9 +1031,9 @@ mwifiex_config_scan(struct mwifiex_private *priv,
>                                         scan_dur = adapter->active_scan_time;
>                         }
>
> -                       (scan_chan_list + chan_idx)->min_scan_time =
> +                       scan_chan_list[chan_idx].min_scan_time =
>                                 cpu_to_le16(scan_dur);
> -                       (scan_chan_list + chan_idx)->max_scan_time =
> +                       scan_chan_list[chan_idx].max_scan_time =
>                                 cpu_to_le16(scan_dur);
>                 }
>
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 6/7] mwifiex: factor out mwifiex_cancel_pending_scan_cmd

[Julian Calaby]

Hi Andreas,

On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
> releasing the scan_pending lock in mwifiex_check_next_scan_command
> is valid, since the lock is taken again, and all nodes removed
> from the scan_pending queue.
>
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/cmdevt.c      | 43 ++++++++++------------
>  drivers/net/wireless/marvell/mwifiex/main.h        |  1 +
>  drivers/net/wireless/marvell/mwifiex/scan.c        | 23 +++---------
>  drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 13 +------
>  4 files changed, 27 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 6ddc98b..490d0d1 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1920,13 +1910,10 @@ static void mwifiex_check_next_scan_command(struct mwifiex_private *priv)
>                 }
>         } else if ((priv->scan_aborting && !priv->scan_request) ||
>                    priv->scan_block) {
> -               list_for_each_entry_safe(cmd_node, tmp_node,
> -                                        &adapter->scan_pending_q, list) {
> -                       list_del(&cmd_node->list);
> -                       mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
> -               }
>                 spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
>
> +               mwifiex_cancel_pending_scan_cmd(adapter);
> +

This is creating a "short" window where &adapter->scan_pending_q_lock
is unlocked here. Is that safe?

You might want to write mwifiex_cancel_pending_scan_cmd() as two
functions, one which takes the spinlock and calls the other and one
which does all the work so you can call the latter here without that
window where ..._q_lock is unlocked.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 7/7] mwifiex: make mwifiex_insert_cmd_to_free_q local static

[Julian Calaby]

Hi Andreas,

On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
> after factoring out mwifiex_cancel_pending_scan_cmd
> the function is noc called outside of cmdevt file
>
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/cmdevt.c | 6 +++++-
>  drivers/net/wireless/marvell/mwifiex/main.h   | 2 --
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 61426b3..4b0f44d 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -26,6 +26,10 @@
>  #include "11n.h"
>  #include "11ac.h"
>
> +static void
> +mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
> +                            struct cmd_ctrl_node *cmd_node);
> +

Why not just move the function definition above where it's needed?

>  /*
>   * This function initializes a command node.
>   *
> @@ -619,7 +623,7 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
>   * The function also calls the completion callback if required, before
>   * cleaning the command node and re-inserting it into the free queue.
>   */
> -void
> +static void
>  mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
>                              struct cmd_ctrl_node *cmd_node)
>  {
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> index f71f894..840e5d4 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -1045,8 +1045,6 @@ void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
>  void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
>  void mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter);
>
> -void mwifiex_insert_cmd_to_free_q(struct mwifiex_adapter *adapter,
> -                                 struct cmd_ctrl_node *cmd_node);
>  void mwifiex_recycle_cmd_node(struct mwifiex_adapter *adapter,
>                               struct cmd_ctrl_node *cmd_node);
>
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 6/7] mwifiex: factor out mwifiex_cancel_pending_scan_cmd

[Andreas Fenkart]

Hi Julian,

thanks for your time!

2016-02-25 2:14 GMT+01:00 Julian Calaby <julian.calaby@gmail.com>:
> Hi Andreas,
>
> On Thu, Feb 25, 2016 at 11:08 AM, Andreas Fenkart <afenkart@gmail.com> wrote:
>> releasing the scan_pending lock in mwifiex_check_next_scan_command
>> is valid, since the lock is taken again, and all nodes removed
>> from the scan_pending queue.
>>
>> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
>> ---
>>  drivers/net/wireless/marvell/mwifiex/cmdevt.c      | 43 ++++++++++------------
>>  drivers/net/wireless/marvell/mwifiex/main.h        |  1 +
>>  drivers/net/wireless/marvell/mwifiex/scan.c        | 23 +++---------
>>  drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 13 +------
>>  4 files changed, 27 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
>> index 6ddc98b..490d0d1 100644
>> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
>> @@ -1920,13 +1910,10 @@ static void mwifiex_check_next_scan_command(struct mwifiex_private *priv)
>>                 }
>>         } else if ((priv->scan_aborting && !priv->scan_request) ||
>>                    priv->scan_block) {
>> -               list_for_each_entry_safe(cmd_node, tmp_node,
>> -                                        &adapter->scan_pending_q, list) {
>> -                       list_del(&cmd_node->list);
>> -                       mwifiex_insert_cmd_to_free_q(adapter, cmd_node);
>> -               }
>>                 spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
>>
>> +               mwifiex_cancel_pending_scan_cmd(adapter);
>> +
>
> This is creating a "short" window where &adapter->scan_pending_q_lock
> is unlocked here. Is that safe?
>
> You might want to write mwifiex_cancel_pending_scan_cmd() as two
> functions, one which takes the spinlock and calls the other and one
> which does all the work so you can call the latter here without that
> window where ..._q_lock is unlocked.

I added this comment to the description of the updated patch, that I
will send out shortly:

Releasing the scan_pending lock in mwifiex_check_next_scan_command
introduces a short window where pending scan commands can be removed
or added before removing them all in mwifiex_cancel_pending_scan_cmd.
I think this is safe, since the worst thing to happen is that a
pending scan cmd is removed by the command handler. Adding new scan
commands is not possible while one is pending, see scan_processing.
Since all commands are removed from the queue anyway, we don't care if
some commands are removed by a different code path earlier, the final
state remains the same.
I assume, that the critical section needed for the check has been
extended over clearing the pending scan queue, out of convenience. The
lock was already held and releasing it first just to grab it again was
more work. It doesn't seem to be necessary because of concurrency.

Thanks,
Andi