linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/31] staging: wfx: fix last items of the TODO list
@ 2020-09-07 10:14 Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 01/31] staging: wfx: improve readability of association processing Jerome Pouiller
                   ` (30 more replies)
  0 siblings, 31 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Hello folks,

This PR fixes most of the items of the TODO list associated to the wfx driver.
Normally, my next PR will ask to move the wfx driver out of the staging
area.

Jérôme Pouiller (31):
  staging: wfx: improve readability of association processing
  staging: wfx: relocate wfx_join() beside wfx_join_finalize()
  staging: wfx: simplify hif_set_association_mode()
  staging: wfx: keep API error list up-to-date
  staging: wfx: drop 'secure link' feature
  staging: wfx: drop multicast filtering
  staging: wfx: drop useless function
  staging: wfx: drop useless enum hif_beacon
  staging: wfx: drop useless union hif_commands_ids
  staging: wfx: drop useless struct hif_reset_flags
  staging: wfx: drop useless struct hif_ie_flags
  staging: wfx: drop useless struct hif_join_flags
  staging: wfx: drop useless struct hif_bss_flags
  staging: wfx: drop useless struct hif_map_link_flags
  staging: wfx: drop useless struct hif_suspend_resume_flags
  staging: wfx: drop useless struct hif_pm_mode
  staging: wfx: drop useless struct hif_rx_flags
  staging: wfx: drop useless struct hif_tx_result_flags
  staging: wfx: drop useless structs only used in hif_req_tx
  staging: wfx: drop useless stricts only used in hif_req_start_scan_alt
  staging: wfx: drop useless structs only used in hif_ind_startup
  staging: wfx: drop useless union hif_privacy_key_data
  staging: wfx: drop useless union hif_event_data
  staging: wfx: drop useless union hif_indication_data
  staging: wfx: drop struct hif_ie_tlv
  staging: wfx: drop macro API_SSID_SIZE
  staging: wfx: fix naming of hif_tx_rate_retry_policy
  staging: wfx: fix spaces
  staging: wfx: uniformize naming rules in hif_tx_mib.c
  staging: wfx: drop async field from struct hif_cmd
  staging: wfx: update TODO list

 drivers/staging/wfx/TODO              |  19 --
 drivers/staging/wfx/bh.c              |  48 +----
 drivers/staging/wfx/data_rx.c         |   2 +-
 drivers/staging/wfx/data_tx.c         |  42 ++---
 drivers/staging/wfx/hif_api_cmd.h     | 254 ++++++++------------------
 drivers/staging/wfx/hif_api_general.h | 129 ++-----------
 drivers/staging/wfx/hif_api_mib.h     |  48 +----
 drivers/staging/wfx/hif_rx.c          |  45 ++---
 drivers/staging/wfx/hif_tx.c          | 109 ++---------
 drivers/staging/wfx/hif_tx.h          |   7 -
 drivers/staging/wfx/hif_tx_mib.c      | 122 +++----------
 drivers/staging/wfx/hif_tx_mib.h      |  11 +-
 drivers/staging/wfx/main.c            |  17 +-
 drivers/staging/wfx/secure_link.h     |  59 ------
 drivers/staging/wfx/sta.c             | 229 ++++++++++-------------
 drivers/staging/wfx/sta.h             |   2 -
 drivers/staging/wfx/wfx.h             |   5 -
 17 files changed, 280 insertions(+), 868 deletions(-)
 delete mode 100644 drivers/staging/wfx/secure_link.h

-- 
2.28.0


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 01/31] staging: wfx: improve readability of association processing
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 02/31] staging: wfx: relocate wfx_join() beside wfx_join_finalize() Jerome Pouiller
                   ` (29 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The statements in wfx_bss_info_changed() has no particular order.

For better readability, group and sort the statements relative to the
association processing.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 53 +++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index a890fe32161c..502967874373 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -547,19 +547,6 @@ void wfx_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	mutex_lock(&wdev->conf_mutex);
 
-	/* TODO: BSS_CHANGED_QOS */
-	if (changed & BSS_CHANGED_ARP_FILTER) {
-		for (i = 0; i < HIF_MAX_ARP_IP_ADDRTABLE_ENTRIES; i++) {
-			__be32 *arp_addr = &info->arp_addr_list[i];
-
-			if (info->arp_addr_cnt > HIF_MAX_ARP_IP_ADDRTABLE_ENTRIES)
-				arp_addr = NULL;
-			if (i >= info->arp_addr_cnt)
-				arp_addr = NULL;
-			hif_set_arp_ipv4_filter(wvif, i, arp_addr);
-		}
-	}
-
 	if (changed & BSS_CHANGED_BASIC_RATES ||
 	    changed & BSS_CHANGED_BEACON_INT ||
 	    changed & BSS_CHANGED_BSSID) {
@@ -567,12 +554,15 @@ void wfx_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			wfx_do_join(wvif);
 	}
 
-	if (changed & BSS_CHANGED_AP_PROBE_RESP ||
-	    changed & BSS_CHANGED_BEACON)
-		wfx_upload_ap_templates(wvif);
-
-	if (changed & BSS_CHANGED_BEACON_ENABLED)
-		wfx_enable_beacon(wvif, info->enable_beacon);
+	if (changed & BSS_CHANGED_ASSOC) {
+		if (info->assoc || info->ibss_joined)
+			wfx_join_finalize(wvif, info);
+		else if (!info->assoc && vif->type == NL80211_IFTYPE_STATION)
+			wfx_reset(wvif);
+		else
+			dev_warn(wdev->dev, "%s: misunderstood change: ASSOC\n",
+				 __func__);
+	}
 
 	if (changed & BSS_CHANGED_BEACON_INFO) {
 		if (vif->type != NL80211_IFTYPE_STATION)
@@ -585,16 +575,25 @@ void wfx_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		wfx_filter_beacon(wvif, true);
 	}
 
-	if (changed & BSS_CHANGED_ASSOC) {
-		if (info->assoc || info->ibss_joined)
-			wfx_join_finalize(wvif, info);
-		else if (!info->assoc && vif->type == NL80211_IFTYPE_STATION)
-			wfx_reset(wvif);
-		else
-			dev_warn(wdev->dev, "%s: misunderstood change: ASSOC\n",
-				 __func__);
+	if (changed & BSS_CHANGED_ARP_FILTER) {
+		for (i = 0; i < HIF_MAX_ARP_IP_ADDRTABLE_ENTRIES; i++) {
+			__be32 *arp_addr = &info->arp_addr_list[i];
+
+			if (info->arp_addr_cnt > HIF_MAX_ARP_IP_ADDRTABLE_ENTRIES)
+				arp_addr = NULL;
+			if (i >= info->arp_addr_cnt)
+				arp_addr = NULL;
+			hif_set_arp_ipv4_filter(wvif, i, arp_addr);
+		}
 	}
 
+	if (changed & BSS_CHANGED_AP_PROBE_RESP ||
+	    changed & BSS_CHANGED_BEACON)
+		wfx_upload_ap_templates(wvif);
+
+	if (changed & BSS_CHANGED_BEACON_ENABLED)
+		wfx_enable_beacon(wvif, info->enable_beacon);
+
 	if (changed & BSS_CHANGED_KEEP_ALIVE)
 		hif_keep_alive_period(wvif, info->max_idle_period *
 					    USEC_PER_TU / USEC_PER_MSEC);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 02/31] staging: wfx: relocate wfx_join() beside wfx_join_finalize()
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 01/31] staging: wfx: improve readability of association processing Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 03/31] staging: wfx: simplify hif_set_association_mode() Jerome Pouiller
                   ` (28 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

wfx_join() and wfx_join_finalize() are the two halves of the association
process. Group them.

In addition, for better uniformity of the code, rename wfx_do_join() in
wfx_join().

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 100 +++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 502967874373..b2a29b2ac20c 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -340,54 +340,6 @@ void wfx_reset(struct wfx_vif *wvif)
 		wfx_update_pm(wvif);
 }
 
-static void wfx_do_join(struct wfx_vif *wvif)
-{
-	int ret;
-	struct ieee80211_bss_conf *conf = &wvif->vif->bss_conf;
-	struct cfg80211_bss *bss = NULL;
-	u8 ssid[IEEE80211_MAX_SSID_LEN];
-	const u8 *ssidie = NULL;
-	int ssidlen = 0;
-
-	wfx_tx_lock_flush(wvif->wdev);
-
-	bss = cfg80211_get_bss(wvif->wdev->hw->wiphy, wvif->channel,
-			       conf->bssid, NULL, 0,
-			       IEEE80211_BSS_TYPE_ANY, IEEE80211_PRIVACY_ANY);
-	if (!bss && !conf->ibss_joined) {
-		wfx_tx_unlock(wvif->wdev);
-		return;
-	}
-
-	rcu_read_lock(); // protect ssidie
-	if (bss)
-		ssidie = ieee80211_bss_get_ie(bss, WLAN_EID_SSID);
-	if (ssidie) {
-		ssidlen = ssidie[1];
-		if (ssidlen > IEEE80211_MAX_SSID_LEN)
-			ssidlen = IEEE80211_MAX_SSID_LEN;
-		memcpy(ssid, &ssidie[2], ssidlen);
-	}
-	rcu_read_unlock();
-
-	cfg80211_put_bss(wvif->wdev->hw->wiphy, bss);
-
-	wvif->join_in_progress = true;
-	ret = hif_join(wvif, conf, wvif->channel, ssid, ssidlen);
-	if (ret) {
-		ieee80211_connection_loss(wvif->vif);
-		wfx_reset(wvif);
-	} else {
-		/* Due to beacon filtering it is possible that the
-		 * AP's beacon is not known for the mac80211 stack.
-		 * Disable filtering temporary to make sure the stack
-		 * receives at least one
-		 */
-		wfx_filter_beacon(wvif, false);
-	}
-	wfx_tx_unlock(wvif->wdev);
-}
-
 int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		struct ieee80211_sta *sta)
 {
@@ -496,6 +448,54 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	wfx_reset(wvif);
 }
 
+static void wfx_join(struct wfx_vif *wvif)
+{
+	int ret;
+	struct ieee80211_bss_conf *conf = &wvif->vif->bss_conf;
+	struct cfg80211_bss *bss = NULL;
+	u8 ssid[IEEE80211_MAX_SSID_LEN];
+	const u8 *ssidie = NULL;
+	int ssidlen = 0;
+
+	wfx_tx_lock_flush(wvif->wdev);
+
+	bss = cfg80211_get_bss(wvif->wdev->hw->wiphy, wvif->channel,
+			       conf->bssid, NULL, 0,
+			       IEEE80211_BSS_TYPE_ANY, IEEE80211_PRIVACY_ANY);
+	if (!bss && !conf->ibss_joined) {
+		wfx_tx_unlock(wvif->wdev);
+		return;
+	}
+
+	rcu_read_lock(); // protect ssidie
+	if (bss)
+		ssidie = ieee80211_bss_get_ie(bss, WLAN_EID_SSID);
+	if (ssidie) {
+		ssidlen = ssidie[1];
+		if (ssidlen > IEEE80211_MAX_SSID_LEN)
+			ssidlen = IEEE80211_MAX_SSID_LEN;
+		memcpy(ssid, &ssidie[2], ssidlen);
+	}
+	rcu_read_unlock();
+
+	cfg80211_put_bss(wvif->wdev->hw->wiphy, bss);
+
+	wvif->join_in_progress = true;
+	ret = hif_join(wvif, conf, wvif->channel, ssid, ssidlen);
+	if (ret) {
+		ieee80211_connection_loss(wvif->vif);
+		wfx_reset(wvif);
+	} else {
+		/* Due to beacon filtering it is possible that the
+		 * AP's beacon is not known for the mac80211 stack.
+		 * Disable filtering temporary to make sure the stack
+		 * receives at least one
+		 */
+		wfx_filter_beacon(wvif, false);
+	}
+	wfx_tx_unlock(wvif->wdev);
+}
+
 static void wfx_join_finalize(struct wfx_vif *wvif,
 			      struct ieee80211_bss_conf *info)
 {
@@ -514,7 +514,7 @@ int wfx_join_ibss(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 
 	wfx_upload_ap_templates(wvif);
-	wfx_do_join(wvif);
+	wfx_join(wvif);
 	return 0;
 }
 
@@ -551,7 +551,7 @@ void wfx_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	    changed & BSS_CHANGED_BEACON_INT ||
 	    changed & BSS_CHANGED_BSSID) {
 		if (vif->type == NL80211_IFTYPE_STATION)
-			wfx_do_join(wvif);
+			wfx_join(wvif);
 	}
 
 	if (changed & BSS_CHANGED_ASSOC) {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 03/31] staging: wfx: simplify hif_set_association_mode()
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 01/31] staging: wfx: improve readability of association processing Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 02/31] staging: wfx: relocate wfx_join() beside wfx_join_finalize() Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 04/31] staging: wfx: keep API error list up-to-date Jerome Pouiller
                   ` (27 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The file hif_tx_mib.c expects to contain functions that format messages
for the hardware. It is unexpected to find function that manipulate
RCU and structures from mac80211.

Keep hif_set_association_mode() with the code necessary for message
formatting and relocate the smart part in wfx_join_finalize().

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_tx_mib.c | 21 +++++----------------
 drivers/staging/wfx/hif_tx_mib.h |  4 ++--
 drivers/staging/wfx/sta.c        | 17 ++++++++++++++++-
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/wfx/hif_tx_mib.c b/drivers/staging/wfx/hif_tx_mib.c
index 05f1e1e98af9..3b20b7486f08 100644
--- a/drivers/staging/wfx/hif_tx_mib.c
+++ b/drivers/staging/wfx/hif_tx_mib.c
@@ -187,29 +187,18 @@ int hif_set_block_ack_policy(struct wfx_vif *wvif,
 			     &val, sizeof(val));
 }
 
-int hif_set_association_mode(struct wfx_vif *wvif,
-			     struct ieee80211_bss_conf *info)
+int hif_set_association_mode(struct wfx_vif *wvif, int ampdu_density,
+			     bool greenfield, bool short_preamble)
 {
-	struct ieee80211_sta *sta = NULL;
 	struct hif_mib_set_association_mode val = {
 		.preambtype_use = 1,
 		.mode = 1,
 		.spacing = 1,
-		.short_preamble = info->use_short_preamble,
+		.short_preamble = short_preamble,
+		.greenfield = greenfield,
+		.mpdu_start_spacing = ampdu_density,
 	};
 
-	rcu_read_lock(); // protect sta
-	if (info->bssid && !info->ibss_joined)
-		sta = ieee80211_find_sta(wvif->vif, info->bssid);
-
-	// FIXME: it is strange to not retrieve all information from bss_info
-	if (sta && sta->ht_cap.ht_supported) {
-		val.mpdu_start_spacing = sta->ht_cap.ampdu_density;
-		if (!(info->ht_operation_mode & IEEE80211_HT_OP_MODE_NON_GF_STA_PRSNT))
-			val.greenfield = !!(sta->ht_cap.cap & IEEE80211_HT_CAP_GRN_FLD);
-	}
-	rcu_read_unlock();
-
 	return hif_write_mib(wvif->wdev, wvif->id,
 			     HIF_MIB_ID_SET_ASSOCIATION_MODE, &val, sizeof(val));
 }
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index 86683de7de7c..1a6f4221bdeb 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -33,8 +33,8 @@ int hif_set_template_frame(struct wfx_vif *wvif, struct sk_buff *skb,
 int hif_set_mfp(struct wfx_vif *wvif, bool capable, bool required);
 int hif_set_block_ack_policy(struct wfx_vif *wvif,
 			     u8 tx_tid_policy, u8 rx_tid_policy);
-int hif_set_association_mode(struct wfx_vif *wvif,
-			     struct ieee80211_bss_conf *info);
+int hif_set_association_mode(struct wfx_vif *wvif, int ampdu_density,
+			     bool greenfield, bool short_preamble);
 int hif_set_tx_rate_retry_policy(struct wfx_vif *wvif,
 				 int policy_index, u8 *rates);
 int hif_set_mac_addr_condition(struct wfx_vif *wvif,
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index b2a29b2ac20c..feebb7c3bdfe 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -499,8 +499,23 @@ static void wfx_join(struct wfx_vif *wvif)
 static void wfx_join_finalize(struct wfx_vif *wvif,
 			      struct ieee80211_bss_conf *info)
 {
+	struct ieee80211_sta *sta = NULL;
+	int ampdu_density = 0;
+	bool greenfield = false;
+
+	rcu_read_lock(); // protect sta
+	if (info->bssid && !info->ibss_joined)
+		sta = ieee80211_find_sta(wvif->vif, info->bssid);
+	if (sta && sta->ht_cap.ht_supported)
+		ampdu_density = sta->ht_cap.ampdu_density;
+	if (sta && sta->ht_cap.ht_supported &&
+	    !(info->ht_operation_mode & IEEE80211_HT_OP_MODE_NON_GF_STA_PRSNT))
+		greenfield = !!(sta->ht_cap.cap & IEEE80211_HT_CAP_GRN_FLD);
+	rcu_read_unlock();
+
 	wvif->join_in_progress = false;
-	hif_set_association_mode(wvif, info);
+	hif_set_association_mode(wvif, ampdu_density, greenfield,
+				 info->use_short_preamble);
 	hif_keep_alive_period(wvif, 0);
 	// beacon_loss_count is defined to 7 in net/mac80211/mlme.c. Let's use
 	// the same value.
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 04/31] staging: wfx: keep API error list up-to-date
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (2 preceding siblings ...)
  2020-09-07 10:14 ` [PATCH 03/31] staging: wfx: simplify hif_set_association_mode() Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 05/31] staging: wfx: drop 'secure link' feature Jerome Pouiller
                   ` (26 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

A new kind of error has appeared in API 3.4.

The Linux driver is not concerned by this new error, but let's keep the
API in sync with the firmware.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_general.h | 1 +
 drivers/staging/wfx/hif_rx.c          | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index dba18a7ae919..791d7375bd7f 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -262,6 +262,7 @@ enum hif_error {
 	HIF_ERROR_HIF_TX_QUEUE_FULL           = 0x0d,
 	HIF_ERROR_HIF_BUS                     = 0x0f,
 	HIF_ERROR_PDS_TESTFEATURE             = 0x10,
+	HIF_ERROR_SLK_UNCONFIGURED            = 0x11,
 };
 
 struct hif_ind_error {
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 1d32973d8ec1..36b393b92936 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -301,6 +301,8 @@ static const struct {
 		"secure link overflow" },
 	{ HIF_ERROR_SLK_WRONG_ENCRYPTION_STATE,
 		"secure link messages list does not match message encryption" },
+	{ HIF_ERROR_SLK_UNCONFIGURED,
+		"secure link not yet configured" },
 	{ HIF_ERROR_HIF_BUS_FREQUENCY_TOO_LOW,
 		"bus clock is too slow (<1kHz)" },
 	{ HIF_ERROR_HIF_RX_DATA_TOO_LARGE,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 05/31] staging: wfx: drop 'secure link' feature
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (3 preceding siblings ...)
  2020-09-07 10:14 ` [PATCH 04/31] staging: wfx: keep API error list up-to-date Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 06/31] staging: wfx: drop multicast filtering Jerome Pouiller
                   ` (25 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The Secure Link (slk) feature allows to encrypt (and authenticate) the
traffic between the host and the device. The official implementation of
this feature relies on mbedTLS. For that reason, this implementation is
not included in the current driver. To be included, the implementation
has to rely on kernel crypto API instead of mbedTLS.

So, for now, the driver contained stub functions waiting for the new
implementation based on kernel crypto API.

This new implementation represent a bunch of work and it is unlikely to
be done in near future.  Moreover, we strongly dislike to maintain
useless code. So, drop all the code related to secure link.

Whoever wants to implement secure link should revert this patch before
starting to work on it.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/bh.c              | 48 ++--------------
 drivers/staging/wfx/hif_api_general.h | 80 ---------------------------
 drivers/staging/wfx/hif_rx.c          | 19 -------
 drivers/staging/wfx/hif_tx.c          | 66 ----------------------
 drivers/staging/wfx/hif_tx.h          |  6 --
 drivers/staging/wfx/main.c            |  9 +--
 drivers/staging/wfx/secure_link.h     | 59 --------------------
 drivers/staging/wfx/wfx.h             |  2 -
 8 files changed, 8 insertions(+), 281 deletions(-)
 delete mode 100644 drivers/staging/wfx/secure_link.h

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index f07bcee50e3f..72da2f4af49f 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -12,7 +12,6 @@
 #include "wfx.h"
 #include "hwio.h"
 #include "traces.h"
-#include "secure_link.h"
 #include "hif_rx.h"
 #include "hif_api_cmd.h"
 
@@ -88,20 +87,11 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	_trace_piggyback(piggyback, false);
 
 	hif = (struct hif_msg *)skb->data;
-	WARN(hif->encrypted & 0x1, "unsupported encryption type");
-	if (hif->encrypted == 0x2) {
-		if (WARN(read_len < sizeof(struct hif_sl_msg), "corrupted read"))
-			goto err;
-		computed_len = le16_to_cpu(((struct hif_sl_msg *)hif)->len);
-		computed_len = round_up(computed_len - sizeof(u16), 16);
-		computed_len += sizeof(struct hif_sl_msg);
-		computed_len += sizeof(struct hif_sl_tag);
-	} else {
-		if (WARN(read_len < sizeof(struct hif_msg), "corrupted read"))
-			goto err;
-		computed_len = le16_to_cpu(hif->len);
-		computed_len = round_up(computed_len, 2);
-	}
+	WARN(hif->encrypted & 0x3, "encryption is unsupported");
+	if (WARN(read_len < sizeof(struct hif_msg), "corrupted read"))
+		goto err;
+	computed_len = le16_to_cpu(hif->len);
+	computed_len = round_up(computed_len, 2);
 	if (computed_len != read_len) {
 		dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
 			computed_len, read_len);
@@ -109,16 +99,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 			       hif, read_len, true);
 		goto err;
 	}
-	if (hif->encrypted == 0x2) {
-		if (wfx_sl_decode(wdev, (struct hif_sl_msg *)hif)) {
-			dev_kfree_skb(skb);
-			// If frame was a confirmation, expect trouble in next
-			// exchange. However, it is harmless to fail to decode
-			// an indication frame, so try to continue. Anyway,
-			// piggyback is probably correct.
-			return piggyback;
-		}
-	}
 
 	if (!(hif->id & HIF_ID_IS_INDICATION)) {
 		(*is_cnf)++;
@@ -199,23 +179,7 @@ static void tx_helper(struct wfx_dev *wdev, struct hif_msg *hif)
 	hif->seqnum = wdev->hif.tx_seqnum;
 	wdev->hif.tx_seqnum = (wdev->hif.tx_seqnum + 1) % (HIF_COUNTER_MAX + 1);
 
-	if (wfx_is_secure_command(wdev, hif->id)) {
-		len = round_up(len - sizeof(hif->len), 16) + sizeof(hif->len) +
-			sizeof(struct hif_sl_msg_hdr) +
-			sizeof(struct hif_sl_tag);
-		// AES support encryption in-place. However, mac80211 access to
-		// 802.11 header after frame was sent (to get MAC addresses).
-		// So, keep origin buffer clear.
-		data = kmalloc(len, GFP_KERNEL);
-		if (!data)
-			goto end;
-		is_encrypted = true;
-		ret = wfx_sl_encode(wdev, hif, data);
-		if (ret)
-			goto end;
-	} else {
-		data = hif;
-	}
+	data = hif;
 	WARN(len > wdev->hw_caps.size_inp_ch_buf,
 	     "%s: request exceed WFx capability: %zu > %d\n", __func__,
 	     len, wdev->hw_caps.size_inp_ch_buf);
diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index 791d7375bd7f..0dc13176a05e 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -282,84 +282,4 @@ enum hif_secure_link_state {
 	SEC_LINK_ENFORCED    = 0x3
 };
 
-enum hif_sl_encryption_type {
-	NO_ENCRYPTION = 0,
-	TX_ENCRYPTION = 1,
-	RX_ENCRYPTION = 2,
-	HP_ENCRYPTION = 3
-};
-
-struct hif_sl_msg_hdr {
-	u32    seqnum:30;
-	u32    encrypted:2;
-} __packed;
-
-struct hif_sl_msg {
-	struct hif_sl_msg_hdr hdr;
-	__le16 len;
-	u8     payload[];
-} __packed;
-
-#define AES_CCM_TAG_SIZE          16
-
-struct hif_sl_tag {
-	u8     tag[16];
-} __packed;
-
-enum hif_sl_mac_key_dest {
-	SL_MAC_KEY_DEST_OTP = 0x78,
-	SL_MAC_KEY_DEST_RAM = 0x87
-};
-
-#define API_KEY_VALUE_SIZE        32
-
-struct hif_req_set_sl_mac_key {
-	u8     otp_or_ram;
-	u8     key_value[API_KEY_VALUE_SIZE];
-} __packed;
-
-struct hif_cnf_set_sl_mac_key {
-	__le32 status;
-} __packed;
-
-enum hif_sl_session_key_alg {
-	HIF_SL_CURVE25519 = 0x01,
-	HIF_SL_KDF        = 0x02
-};
-
-#define API_HOST_PUB_KEY_SIZE     32
-#define API_HOST_PUB_KEY_MAC_SIZE 64
-
-struct hif_req_sl_exchange_pub_keys {
-	u8     algorithm:2;
-	u8     reserved1:6;
-	u8     reserved2[3];
-	u8     host_pub_key[API_HOST_PUB_KEY_SIZE];
-	u8     host_pub_key_mac[API_HOST_PUB_KEY_MAC_SIZE];
-} __packed;
-
-struct hif_cnf_sl_exchange_pub_keys {
-	__le32 status;
-} __packed;
-
-#define API_NCP_PUB_KEY_SIZE      32
-#define API_NCP_PUB_KEY_MAC_SIZE  64
-
-struct hif_ind_sl_exchange_pub_keys {
-	__le32 status;
-	u8     ncp_pub_key[API_NCP_PUB_KEY_SIZE];
-	u8     ncp_pub_key_mac[API_NCP_PUB_KEY_MAC_SIZE];
-} __packed;
-
-struct hif_req_sl_configure {
-	u8     encr_bmp[32];
-	u8     disable_session_key_protection:1;
-	u8     reserved1:7;
-	u8     reserved2[3];
-} __packed;
-
-struct hif_cnf_sl_configure {
-	__le32 status;
-} __packed;
-
 #endif
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 36b393b92936..cf7a956ef00a 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -15,7 +15,6 @@
 #include "bh.h"
 #include "sta.h"
 #include "data_rx.h"
-#include "secure_link.h"
 #include "hif_api_cmd.h"
 
 static int hif_generic_confirm(struct wfx_dev *wdev,
@@ -53,8 +52,6 @@ static int hif_generic_confirm(struct wfx_dev *wdev,
 	} else {
 		wdev->hif_cmd.buf_send = NULL;
 		mutex_unlock(&wdev->hif_cmd.lock);
-		if (cmd != HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS)
-			mutex_unlock(&wdev->hif_cmd.key_renew_lock);
 	}
 	return status;
 }
@@ -110,21 +107,6 @@ static int hif_wakeup_indication(struct wfx_dev *wdev,
 	return 0;
 }
 
-static int hif_keys_indication(struct wfx_dev *wdev,
-			       const struct hif_msg *hif, const void *buf)
-{
-	const struct hif_ind_sl_exchange_pub_keys *body = buf;
-	u8 pubkey[API_NCP_PUB_KEY_SIZE];
-
-	// SL_PUB_KEY_EXCHANGE_STATUS_SUCCESS is used by legacy secure link
-	if (body->status && body->status != HIF_STATUS_SLK_NEGO_SUCCESS)
-		dev_warn(wdev->dev, "secure link negotiation error\n");
-	memcpy(pubkey, body->ncp_pub_key, sizeof(pubkey));
-	memreverse(pubkey, sizeof(pubkey));
-	wfx_sl_check_pubkey(wdev, pubkey, body->ncp_pub_key_mac);
-	return 0;
-}
-
 static int hif_receive_indication(struct wfx_dev *wdev,
 				  const struct hif_msg *hif,
 				  const void *buf, struct sk_buff *skb)
@@ -380,7 +362,6 @@ static const struct {
 	{ HIF_IND_ID_SET_PM_MODE_CMPL,     hif_pm_mode_complete_indication },
 	{ HIF_IND_ID_SCAN_CMPL,            hif_scan_complete_indication },
 	{ HIF_IND_ID_SUSPEND_RESUME_TX,    hif_suspend_resume_indication },
-	{ HIF_IND_ID_SL_EXCHANGE_PUB_KEYS, hif_keys_indication },
 	{ HIF_IND_ID_EVENT,                hif_event_indication },
 	{ HIF_IND_ID_GENERIC,              hif_generic_indication },
 	{ HIF_IND_ID_ERROR,                hif_error_indication },
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 6b89e55f03f4..f91b19ddf8e3 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -20,7 +20,6 @@ void wfx_init_hif_cmd(struct wfx_hif_cmd *hif_cmd)
 	init_completion(&hif_cmd->ready);
 	init_completion(&hif_cmd->done);
 	mutex_init(&hif_cmd->lock);
-	mutex_init(&hif_cmd->key_renew_lock);
 }
 
 static void wfx_fill_header(struct hif_msg *hif, int if_id,
@@ -62,9 +61,6 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
 	if (wdev->chip_frozen)
 		return -ETIMEDOUT;
 
-	if (cmd != HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS)
-		mutex_lock(&wdev->hif_cmd.key_renew_lock);
-
 	mutex_lock(&wdev->hif_cmd.lock);
 	WARN(wdev->hif_cmd.buf_send, "data locking error");
 
@@ -118,8 +114,6 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
 			 "WSM request %s%s%s (%#.2x) on vif %d returned status %d\n",
 			 get_hif_name(cmd), mib_sep, mib_name, cmd, vif, ret);
 
-	if (cmd != HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS)
-		mutex_unlock(&wdev->hif_cmd.key_renew_lock);
 	return ret;
 }
 
@@ -147,7 +141,6 @@ int hif_shutdown(struct wfx_dev *wdev)
 	else
 		control_reg_write(wdev, 0);
 	mutex_unlock(&wdev->hif_cmd.lock);
-	mutex_unlock(&wdev->hif_cmd.key_renew_lock);
 	kfree(hif);
 	return ret;
 }
@@ -535,62 +528,3 @@ int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
 	kfree(hif);
 	return ret;
 }
-
-int hif_sl_send_pub_keys(struct wfx_dev *wdev,
-			 const u8 *pubkey, const u8 *pubkey_hmac)
-{
-	int ret;
-	struct hif_msg *hif;
-	struct hif_req_sl_exchange_pub_keys *body = wfx_alloc_hif(sizeof(*body),
-								  &hif);
-
-	if (!hif)
-		return -ENOMEM;
-	body->algorithm = HIF_SL_CURVE25519;
-	memcpy(body->host_pub_key, pubkey, sizeof(body->host_pub_key));
-	memcpy(body->host_pub_key_mac, pubkey_hmac,
-	       sizeof(body->host_pub_key_mac));
-	wfx_fill_header(hif, -1, HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS,
-			sizeof(*body));
-	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-	kfree(hif);
-	// Compatibility with legacy secure link
-	if (ret == le32_to_cpu(HIF_STATUS_SLK_NEGO_SUCCESS))
-		ret = 0;
-	return ret;
-}
-
-int hif_sl_config(struct wfx_dev *wdev, const unsigned long *bitmap)
-{
-	int ret;
-	struct hif_msg *hif;
-	struct hif_req_sl_configure *body = wfx_alloc_hif(sizeof(*body), &hif);
-
-	if (!hif)
-		return -ENOMEM;
-	memcpy(body->encr_bmp, bitmap, sizeof(body->encr_bmp));
-	wfx_fill_header(hif, -1, HIF_REQ_ID_SL_CONFIGURE, sizeof(*body));
-	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-	kfree(hif);
-	return ret;
-}
-
-int hif_sl_set_mac_key(struct wfx_dev *wdev, const u8 *slk_key, int destination)
-{
-	int ret;
-	struct hif_msg *hif;
-	struct hif_req_set_sl_mac_key *body = wfx_alloc_hif(sizeof(*body),
-							    &hif);
-
-	if (!hif)
-		return -ENOMEM;
-	memcpy(body->key_value, slk_key, sizeof(body->key_value));
-	body->otp_or_ram = destination;
-	wfx_fill_header(hif, -1, HIF_REQ_ID_SET_SL_MAC_KEY, sizeof(*body));
-	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-	kfree(hif);
-	// Compatibility with legacy secure link
-	if (ret == le32_to_cpu(HIF_STATUS_SLK_SET_KEY_SUCCESS))
-		ret = 0;
-	return ret;
-}
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index b371b3777a31..e8aca39e7497 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -20,7 +20,6 @@ struct wfx_vif;
 
 struct wfx_hif_cmd {
 	struct mutex      lock;
-	struct mutex      key_renew_lock;
 	struct completion ready;
 	struct completion done;
 	bool              async;
@@ -58,10 +57,5 @@ int hif_beacon_transmit(struct wfx_vif *wvif, bool enable);
 int hif_map_link(struct wfx_vif *wvif,
 		 bool unmap, u8 *mac_addr, int sta_id, bool mfp);
 int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len);
-int hif_sl_set_mac_key(struct wfx_dev *wdev,
-		       const u8 *slk_key, int destination);
-int hif_sl_config(struct wfx_dev *wdev, const unsigned long *bitmap);
-int hif_sl_send_pub_keys(struct wfx_dev *wdev,
-			 const u8 *pubkey, const u8 *pubkey_hmac);
 
 #endif
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 5e2b82499004..2af4914e905c 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -30,7 +30,6 @@
 #include "scan.h"
 #include "debug.h"
 #include "data_tx.h"
-#include "secure_link.h"
 #include "hif_tx_mib.h"
 #include "hif_api_cmd.h"
 
@@ -271,8 +270,7 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 	hw->queues = 4;
 	hw->max_rates = 8;
 	hw->max_rate_tries = 8;
-	hw->extra_tx_headroom = sizeof(struct hif_sl_msg_hdr) +
-				sizeof(struct hif_msg)
+	hw->extra_tx_headroom = sizeof(struct hif_msg)
 				+ sizeof(struct hif_req_tx)
 				+ 4 /* alignment */ + 8 /* TKIP IV */;
 	hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
@@ -309,7 +307,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 		return ERR_CAST(wdev->pdata.gpio_wakeup);
 	if (wdev->pdata.gpio_wakeup)
 		gpiod_set_consumer_name(wdev->pdata.gpio_wakeup, "wfx wakeup");
-	wfx_sl_fill_pdata(dev, &wdev->pdata);
 
 	mutex_init(&wdev->conf_mutex);
 	mutex_init(&wdev->rx_stats_lock);
@@ -381,8 +378,7 @@ int wfx_probe(struct wfx_dev *wdev)
 		goto err0;
 	}
 
-	err = wfx_sl_init(wdev);
-	if (err && wdev->hw_caps.capabilities.link_mode == SEC_LINK_ENFORCED) {
+	if (wdev->hw_caps.capabilities.link_mode == SEC_LINK_ENFORCED) {
 		dev_err(wdev->dev,
 			"chip require secure_link, but can't negotiate it\n");
 		goto err0;
@@ -466,7 +462,6 @@ void wfx_release(struct wfx_dev *wdev)
 	hif_shutdown(wdev);
 	wdev->hwbus_ops->irq_unsubscribe(wdev->hwbus_priv);
 	wfx_bh_unregister(wdev);
-	wfx_sl_deinit(wdev);
 }
 
 static int __init wfx_core_init(void)
diff --git a/drivers/staging/wfx/secure_link.h b/drivers/staging/wfx/secure_link.h
deleted file mode 100644
index c3d055b2f8b1..000000000000
--- a/drivers/staging/wfx/secure_link.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (c) 2019, Silicon Laboratories, Inc.
- */
-#ifndef WFX_SECURE_LINK_H
-#define WFX_SECURE_LINK_H
-
-#include <linux/of.h>
-
-#include "hif_api_general.h"
-
-struct wfx_dev;
-
-
-struct sl_context {
-};
-
-static inline bool wfx_is_secure_command(struct wfx_dev *wdev, int cmd_id)
-{
-	return false;
-}
-
-static inline int wfx_sl_decode(struct wfx_dev *wdev, struct hif_sl_msg *m)
-{
-	return -EIO;
-}
-
-static inline int wfx_sl_encode(struct wfx_dev *wdev,
-				const struct hif_msg *input,
-				struct hif_sl_msg *output)
-{
-	return -EIO;
-}
-
-static inline int wfx_sl_check_pubkey(struct wfx_dev *wdev,
-				      const u8 *ncp_pubkey,
-				      const u8 *ncp_pubmac)
-{
-	return -EIO;
-}
-
-static inline void wfx_sl_fill_pdata(struct device *dev,
-				     struct wfx_platform_data *pdata)
-{
-	if (of_find_property(dev->of_node, "slk_key", NULL))
-		dev_err(dev, "secure link is not supported by this driver, ignoring provided key\n");
-}
-
-static inline int wfx_sl_init(struct wfx_dev *wdev)
-{
-	return -EIO;
-}
-
-static inline void wfx_sl_deinit(struct wfx_dev *wdev)
-{
-}
-
-
-#endif
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 38e24d7f72f2..241eaaf71f5e 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -20,7 +20,6 @@
 #include "data_tx.h"
 #include "main.h"
 #include "queue.h"
-#include "secure_link.h"
 #include "hif_tx.h"
 
 #define USEC_PER_TXOP 32 // see struct ieee80211_tx_queue_params
@@ -41,7 +40,6 @@ struct wfx_dev {
 	struct completion	firmware_ready;
 	struct hif_ind_startup	hw_caps;
 	struct wfx_hif		hif;
-	struct sl_context	sl;
 	struct delayed_work	cooling_timeout_work;
 	bool			poll_irq;
 	bool			chip_frozen;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 06/31] staging: wfx: drop multicast filtering
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (4 preceding siblings ...)
  2020-09-07 10:14 ` [PATCH 05/31] staging: wfx: drop 'secure link' feature Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 07/31] staging: wfx: drop useless function Jerome Pouiller
                   ` (24 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The device allows to filter multicast frames. The driver has the
necessary code to take advantage of this feature. However, some bugs
has been reported on this feature. So, it was temporary disabled.

Finally, the things work well as-is for more than one year now. So there
is no plan to enable this feature in near future.

Since we dislike to maintain dead code, drop it.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_mib.h | 36 -------------------
 drivers/staging/wfx/hif_tx_mib.c  | 40 ---------------------
 drivers/staging/wfx/hif_tx_mib.h  |  6 ----
 drivers/staging/wfx/main.c        |  1 -
 drivers/staging/wfx/sta.c         | 59 ++-----------------------------
 drivers/staging/wfx/sta.h         |  2 --
 drivers/staging/wfx/wfx.h         |  3 --
 7 files changed, 2 insertions(+), 145 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_mib.h b/drivers/staging/wfx/hif_api_mib.h
index 6f1434795fa8..d0e0a9e29afa 100644
--- a/drivers/staging/wfx/hif_api_mib.h
+++ b/drivers/staging/wfx/hif_api_mib.h
@@ -82,42 +82,6 @@ struct hif_mib_gl_set_multi_msg {
 	u8     reserved2[3];
 } __packed;
 
-enum hif_mac_addr_type {
-	HIF_MAC_ADDR_A1 = 0x0,
-	HIF_MAC_ADDR_A2 = 0x1,
-	HIF_MAC_ADDR_A3 = 0x2
-};
-
-struct hif_mib_mac_addr_data_frame_condition {
-	u8     condition_idx;
-	u8     address_type;
-	u8     mac_address[ETH_ALEN];
-} __packed;
-
-#define HIF_FILTER_UNICAST   0x1
-#define HIF_FILTER_MULTICAST 0x2
-#define HIF_FILTER_BROADCAST 0x4
-
-struct hif_mib_uc_mc_bc_data_frame_condition {
-	u8     condition_idx;
-	u8     allowed_frames;
-	u8     reserved[2];
-} __packed;
-
-struct hif_mib_config_data_filter {
-	u8     filter_idx;
-	u8     enable;
-	u8     reserved1[2];
-	u8     eth_type_cond;
-	u8     port_cond;
-	u8     magic_cond;
-	u8     mac_cond;
-	u8     ipv4_cond;
-	u8     ipv6_cond;
-	u8     uc_mc_bc_cond;
-	u8     reserved2;
-} __packed;
-
 struct hif_mib_set_data_filtering {
 	u8     invert_matching:1;
 	u8     reserved1:7;
diff --git a/drivers/staging/wfx/hif_tx_mib.c b/drivers/staging/wfx/hif_tx_mib.c
index 3b20b7486f08..7f24e9f77c22 100644
--- a/drivers/staging/wfx/hif_tx_mib.c
+++ b/drivers/staging/wfx/hif_tx_mib.c
@@ -228,46 +228,6 @@ int hif_set_tx_rate_retry_policy(struct wfx_vif *wvif,
 	return ret;
 }
 
-int hif_set_mac_addr_condition(struct wfx_vif *wvif,
-			       int idx, const u8 *mac_addr)
-{
-	struct hif_mib_mac_addr_data_frame_condition val = {
-		.condition_idx = idx,
-		.address_type = HIF_MAC_ADDR_A1,
-	};
-
-	ether_addr_copy(val.mac_address, mac_addr);
-	return hif_write_mib(wvif->wdev, wvif->id,
-			     HIF_MIB_ID_MAC_ADDR_DATAFRAME_CONDITION,
-			     &val, sizeof(val));
-}
-
-int hif_set_uc_mc_bc_condition(struct wfx_vif *wvif, int idx, u8 allowed_frames)
-{
-	struct hif_mib_uc_mc_bc_data_frame_condition val = {
-		.condition_idx = idx,
-		.allowed_frames = allowed_frames,
-	};
-
-	return hif_write_mib(wvif->wdev, wvif->id,
-			     HIF_MIB_ID_UC_MC_BC_DATAFRAME_CONDITION,
-			     &val, sizeof(val));
-}
-
-int hif_set_config_data_filter(struct wfx_vif *wvif, bool enable, int idx,
-			       int mac_filters, int frames_types_filters)
-{
-	struct hif_mib_config_data_filter val = {
-		.enable = enable,
-		.filter_idx = idx,
-		.mac_cond = mac_filters,
-		.uc_mc_bc_cond = frames_types_filters,
-	};
-
-	return hif_write_mib(wvif->wdev, wvif->id,
-			     HIF_MIB_ID_CONFIG_DATA_FILTER, &val, sizeof(val));
-}
-
 int hif_set_data_filtering(struct wfx_vif *wvif, bool enable, bool invert)
 {
 	struct hif_mib_set_data_filtering val = {
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index 1a6f4221bdeb..d4cac63164ba 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -37,12 +37,6 @@ int hif_set_association_mode(struct wfx_vif *wvif, int ampdu_density,
 			     bool greenfield, bool short_preamble);
 int hif_set_tx_rate_retry_policy(struct wfx_vif *wvif,
 				 int policy_index, u8 *rates);
-int hif_set_mac_addr_condition(struct wfx_vif *wvif,
-			       int idx, const u8 *mac_addr);
-int hif_set_uc_mc_bc_condition(struct wfx_vif *wvif,
-			       int idx, u8 allowed_frames);
-int hif_set_config_data_filter(struct wfx_vif *wvif, bool enable, int idx,
-			       int mac_filters, int frames_types_filters);
 int hif_set_data_filtering(struct wfx_vif *wvif, bool enable, bool invert);
 int hif_keep_alive_period(struct wfx_vif *wvif, int period);
 int hif_set_arp_ipv4_filter(struct wfx_vif *wvif, int idx, __be32 *addr);
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 2af4914e905c..1017a2290f08 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -142,7 +142,6 @@ static const struct ieee80211_ops wfx_ops = {
 	.set_rts_threshold	= wfx_set_rts_threshold,
 	.set_default_unicast_key = wfx_set_default_unicast_key,
 	.bss_info_changed	= wfx_bss_info_changed,
-	.prepare_multicast	= wfx_prepare_multicast,
 	.configure_filter	= wfx_configure_filter,
 	.ampdu_action		= wfx_ampdu_action,
 	.flush			= wfx_flush,
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index feebb7c3bdfe..8700d2fc6a77 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -91,59 +91,12 @@ static void wfx_filter_beacon(struct wfx_vif *wvif, bool filter_beacon)
 	}
 }
 
-static void wfx_filter_mcast(struct wfx_vif *wvif, bool filter_mcast)
-{
-	int i;
-
-	// Temporary workaround for filters
-	hif_set_data_filtering(wvif, false, true);
-	return;
-
-	if (!filter_mcast) {
-		hif_set_data_filtering(wvif, false, true);
-		return;
-	}
-	for (i = 0; i < wvif->filter_mcast_count; i++)
-		hif_set_mac_addr_condition(wvif, i, wvif->filter_mcast_addr[i]);
-	hif_set_uc_mc_bc_condition(wvif, 0,
-				   HIF_FILTER_UNICAST | HIF_FILTER_BROADCAST);
-	hif_set_config_data_filter(wvif, true, 0, BIT(1),
-				   BIT(wvif->filter_mcast_count) - 1);
-	hif_set_data_filtering(wvif, true, true);
-}
-
-u64 wfx_prepare_multicast(struct ieee80211_hw *hw,
-			  struct netdev_hw_addr_list *mc_list)
-{
-	int i;
-	struct netdev_hw_addr *ha;
-	struct wfx_vif *wvif = NULL;
-	struct wfx_dev *wdev = hw->priv;
-	int count = netdev_hw_addr_list_count(mc_list);
-
-	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
-		if (count > ARRAY_SIZE(wvif->filter_mcast_addr)) {
-			wvif->filter_mcast_count = 0;
-			continue;
-		}
-		wvif->filter_mcast_count = count;
-
-		i = 0;
-		netdev_hw_addr_list_for_each(ha, mc_list) {
-			ether_addr_copy(wvif->filter_mcast_addr[i], ha->addr);
-			i++;
-		}
-	}
-
-	return 0;
-}
-
 void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 			  unsigned int *total_flags, u64 unused)
 {
 	struct wfx_vif *wvif = NULL;
 	struct wfx_dev *wdev = hw->priv;
-	bool filter_bssid, filter_prbreq, filter_beacon, filter_mcast;
+	bool filter_bssid, filter_prbreq, filter_beacon;
 
 	// Notes:
 	//   - Probe responses (FIF_BCN_PRBRESP_PROMISC) are never filtered
@@ -167,15 +120,7 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 			filter_beacon = true;
 		wfx_filter_beacon(wvif, filter_beacon);
 
-		if (*total_flags & FIF_ALLMULTI) {
-			filter_mcast = false;
-		} else if (!wvif->filter_mcast_count) {
-			dev_dbg(wdev->dev, "disabling unconfigured multicast filter");
-			filter_mcast = false;
-		} else {
-			filter_mcast = true;
-		}
-		wfx_filter_mcast(wvif, filter_mcast);
+		hif_set_data_filtering(wvif, false, true);
 
 		if (*total_flags & FIF_OTHER_BSS)
 			filter_bssid = false;
diff --git a/drivers/staging/wfx/sta.h b/drivers/staging/wfx/sta.h
index 6b15a64ac9e2..610cfb0fcd02 100644
--- a/drivers/staging/wfx/sta.h
+++ b/drivers/staging/wfx/sta.h
@@ -25,8 +25,6 @@ int wfx_config(struct ieee80211_hw *hw, u32 changed);
 int wfx_set_rts_threshold(struct ieee80211_hw *hw, u32 value);
 void wfx_set_default_unicast_key(struct ieee80211_hw *hw,
 				 struct ieee80211_vif *vif, int idx);
-u64 wfx_prepare_multicast(struct ieee80211_hw *hw,
-			  struct netdev_hw_addr_list *mc_list);
 void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 			  unsigned int *total_flags, u64 unused);
 
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 241eaaf71f5e..56fbfab05651 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -79,9 +79,6 @@ struct wfx_vif {
 
 	struct work_struct	update_tim_work;
 
-	int			filter_mcast_count;
-	u8			filter_mcast_addr[8][ETH_ALEN];
-
 	unsigned long		uapsd_mask;
 
 	/* avoid some operations in parallel with scan */
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 07/31] staging: wfx: drop useless function
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (5 preceding siblings ...)
  2020-09-07 10:14 ` [PATCH 06/31] staging: wfx: drop multicast filtering Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 08/31] staging: wfx: drop useless enum hif_beacon Jerome Pouiller
                   ` (23 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Since the code for multicast filtering has been dropped, the function
hif_set_data_filtering() is only called to disable multicast filtering.
In fact, the multicast filtering is already disabled by default. So,
this function is useless and can be dropped.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_mib.h |  8 --------
 drivers/staging/wfx/hif_tx_mib.c  | 11 -----------
 drivers/staging/wfx/hif_tx_mib.h  |  1 -
 drivers/staging/wfx/sta.c         |  2 --
 4 files changed, 22 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_mib.h b/drivers/staging/wfx/hif_api_mib.h
index d0e0a9e29afa..73873d29456d 100644
--- a/drivers/staging/wfx/hif_api_mib.h
+++ b/drivers/staging/wfx/hif_api_mib.h
@@ -82,14 +82,6 @@ struct hif_mib_gl_set_multi_msg {
 	u8     reserved2[3];
 } __packed;
 
-struct hif_mib_set_data_filtering {
-	u8     invert_matching:1;
-	u8     reserved1:7;
-	u8     enable:1;
-	u8     reserved2:7;
-	u8     reserved3[2];
-} __packed;
-
 enum hif_arp_ns_frame_treatment {
 	HIF_ARP_NS_FILTERING_DISABLE = 0x0,
 	HIF_ARP_NS_FILTERING_ENABLE  = 0x1,
diff --git a/drivers/staging/wfx/hif_tx_mib.c b/drivers/staging/wfx/hif_tx_mib.c
index 7f24e9f77c22..2eb2a20890c7 100644
--- a/drivers/staging/wfx/hif_tx_mib.c
+++ b/drivers/staging/wfx/hif_tx_mib.c
@@ -228,17 +228,6 @@ int hif_set_tx_rate_retry_policy(struct wfx_vif *wvif,
 	return ret;
 }
 
-int hif_set_data_filtering(struct wfx_vif *wvif, bool enable, bool invert)
-{
-	struct hif_mib_set_data_filtering val = {
-		.enable = enable,
-		.invert_matching = invert,
-	};
-
-	return hif_write_mib(wvif->wdev, wvif->id,
-			     HIF_MIB_ID_SET_DATA_FILTERING, &val, sizeof(val));
-}
-
 int hif_keep_alive_period(struct wfx_vif *wvif, int period)
 {
 	struct hif_mib_keep_alive_period arg = {
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index d4cac63164ba..6c25015173cd 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -37,7 +37,6 @@ int hif_set_association_mode(struct wfx_vif *wvif, int ampdu_density,
 			     bool greenfield, bool short_preamble);
 int hif_set_tx_rate_retry_policy(struct wfx_vif *wvif,
 				 int policy_index, u8 *rates);
-int hif_set_data_filtering(struct wfx_vif *wvif, bool enable, bool invert);
 int hif_keep_alive_period(struct wfx_vif *wvif, int period);
 int hif_set_arp_ipv4_filter(struct wfx_vif *wvif, int idx, __be32 *addr);
 int hif_use_multi_tx_conf(struct wfx_dev *wdev, bool enable);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 8700d2fc6a77..0d27ca27e48c 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -120,8 +120,6 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 			filter_beacon = true;
 		wfx_filter_beacon(wvif, filter_beacon);
 
-		hif_set_data_filtering(wvif, false, true);
-
 		if (*total_flags & FIF_OTHER_BSS)
 			filter_bssid = false;
 		else
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 08/31] staging: wfx: drop useless enum hif_beacon
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (6 preceding siblings ...)
  2020-09-07 10:14 ` [PATCH 07/31] staging: wfx: drop useless function Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:14 ` [PATCH 09/31] staging: wfx: drop useless union hif_commands_ids Jerome Pouiller
                   ` (22 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Enum hif_beacon is not used. Moreover, it is just another definition of
a boolean. Absolutely useless.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 21cde19cff75..75e8c2a7fdf9 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -440,11 +440,6 @@ struct hif_cnf_start {
 	__le32 status;
 } __packed;
 
-enum hif_beacon {
-	HIF_BEACON_STOP                       = 0x0,
-	HIF_BEACON_START                      = 0x1
-};
-
 struct hif_req_beacon_transmit {
 	u8     enable_beaconing;
 	u8     reserved[3];
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 09/31] staging: wfx: drop useless union hif_commands_ids
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (7 preceding siblings ...)
  2020-09-07 10:14 ` [PATCH 08/31] staging: wfx: drop useless enum hif_beacon Jerome Pouiller
@ 2020-09-07 10:14 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 10/31] staging: wfx: drop useless struct hif_reset_flags Jerome Pouiller
                   ` (21 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:14 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Union hif_commands_ids is unused.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 75e8c2a7fdf9..c132d8e43b50 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -60,12 +60,6 @@ enum hif_indications_ids {
 	HIF_IND_ID_EVENT                = 0x85
 };
 
-union hif_commands_ids {
-	enum hif_requests_ids request;
-	enum hif_confirmations_ids confirmation;
-	enum hif_indications_ids indication;
-};
-
 struct hif_reset_flags {
 	u8     reset_stat:1;
 	u8     reset_all_int:1;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 10/31] staging: wfx: drop useless struct hif_reset_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (8 preceding siblings ...)
  2020-09-07 10:14 ` [PATCH 09/31] staging: wfx: drop useless union hif_commands_ids Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 11/31] staging: wfx: drop useless struct hif_ie_flags Jerome Pouiller
                   ` (20 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_reset_flags has no reason to exist. Drop it and simplify
access to struct hif_req_reset.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 6 +++---
 drivers/staging/wfx/hif_tx.c      | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index c132d8e43b50..3da736dbf52c 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -60,15 +60,15 @@ enum hif_indications_ids {
 	HIF_IND_ID_EVENT                = 0x85
 };
 
-struct hif_reset_flags {
+struct hif_req_reset {
 	u8     reset_stat:1;
 	u8     reset_all_int:1;
 	u8     reserved1:6;
 	u8     reserved2[3];
 } __packed;
 
-struct hif_req_reset {
-	struct hif_reset_flags reset_flags;
+struct hif_cnf_reset {
+	__le32 status;
 } __packed;
 
 struct hif_req_read_mib {
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index f91b19ddf8e3..8736eb4d5f15 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -170,7 +170,7 @@ int hif_reset(struct wfx_vif *wvif, bool reset_stat)
 
 	if (!hif)
 		return -ENOMEM;
-	body->reset_flags.reset_stat = reset_stat;
+	body->reset_stat = reset_stat;
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_RESET, sizeof(*body));
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
 	kfree(hif);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 11/31] staging: wfx: drop useless struct hif_ie_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (9 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 10/31] staging: wfx: drop useless struct hif_reset_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 12/31] staging: wfx: drop useless struct hif_join_flags Jerome Pouiller
                   ` (19 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_ie_flags has no reason to exist. Drop it and simplify
access to struct hif_req_update_ie.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 14 +++++---------
 drivers/staging/wfx/hif_tx.c      |  2 +-
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 3da736dbf52c..b104abbc5b25 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -93,14 +93,6 @@ struct hif_cnf_write_mib {
 	__le32 status;
 } __packed;
 
-struct hif_ie_flags {
-	u8     beacon:1;
-	u8     probe_resp:1;
-	u8     probe_req:1;
-	u8     reserved1:5;
-	u8     reserved2;
-} __packed;
-
 struct hif_ie_tlv {
 	u8     type;
 	u8     length;
@@ -108,7 +100,11 @@ struct hif_ie_tlv {
 } __packed;
 
 struct hif_req_update_ie {
-	struct hif_ie_flags ie_flags;
+	u8     beacon:1;
+	u8     probe_resp:1;
+	u8     probe_req:1;
+	u8     reserved1:5;
+	u8     reserved2;
 	__le16 num_ies;
 	struct hif_ie_tlv ie[];
 } __packed;
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 8736eb4d5f15..49523e70af6c 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -520,7 +520,7 @@ int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
 
 	if (!hif)
 		return -ENOMEM;
-	body->ie_flags.beacon = 1;
+	body->beacon = 1;
 	body->num_ies = cpu_to_le16(1);
 	memcpy(body->ie, ies, ies_len);
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_UPDATE_IE, buf_len);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 12/31] staging: wfx: drop useless struct hif_join_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (10 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 11/31] staging: wfx: drop useless struct hif_ie_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 13/31] staging: wfx: drop useless struct hif_bss_flags Jerome Pouiller
                   ` (18 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_join_flags has no reason to exist. Drop it and simplify
access to struct hif_req_join.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index b104abbc5b25..1c99431eb90f 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -336,26 +336,22 @@ struct hif_cnf_edca_queue_params {
 	__le32 status;
 } __packed;
 
-struct hif_join_flags {
-	u8     reserved1:2;
-	u8     force_no_beacon:1;
-	u8     force_with_ind:1;
-	u8     reserved2:4;
-} __packed;
-
 struct hif_req_join {
 	u8     infrastructure_bss_mode:1;
 	u8     reserved1:7;
 	u8     band;
 	u8     channel_number;
-	u8     reserved;
+	u8     reserved2;
 	u8     bssid[ETH_ALEN];
 	__le16 atim_window;
 	u8     short_preamble:1;
-	u8     reserved2:7;
+	u8     reserved3:7;
 	u8     probe_for_join;
-	u8     reserved3;
-	struct hif_join_flags join_flags;
+	u8     reserved4;
+	u8     reserved5:2;
+	u8     force_no_beacon:1;
+	u8     force_with_ind:1;
+	u8     reserved6:4;
 	__le32 ssid_length;
 	u8     ssid[HIF_API_SSID_SIZE];
 	__le32 beacon_interval;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 13/31] staging: wfx: drop useless struct hif_bss_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (11 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 12/31] staging: wfx: drop useless struct hif_join_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 14/31] staging: wfx: drop useless struct hif_map_link_flags Jerome Pouiller
                   ` (17 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_bss_flags has no reason to exist. In add, it is never used.
Drop it.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 1c99431eb90f..895f26d9f1a2 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -366,13 +366,9 @@ struct hif_ind_join_complete {
 	__le32 status;
 } __packed;
 
-struct hif_bss_flags {
+struct hif_req_set_bss_params {
 	u8     lost_count_only:1;
 	u8     reserved:7;
-} __packed;
-
-struct hif_req_set_bss_params {
-	struct hif_bss_flags bss_flags;
 	u8     beacon_lost_count;
 	__le16 aid;
 	__le32 operational_rate_set;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 14/31] staging: wfx: drop useless struct hif_map_link_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (12 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 13/31] staging: wfx: drop useless struct hif_bss_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 15/31] staging: wfx: drop useless struct hif_suspend_resume_flags Jerome Pouiller
                   ` (16 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_map_link_flags has no reason to exist. Drop it and simplify
access to struct hif_req_map_link.

Also rename the field 'map_direction' in 'unmap'. It is more
meaningful and allows to drop enum hif_sta_map_direction.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 15 +++------------
 drivers/staging/wfx/hif_tx.c      |  4 ++--
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 895f26d9f1a2..f86f6d491fb2 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -434,20 +434,11 @@ struct hif_cnf_beacon_transmit {
 #define HIF_LINK_ID_MAX            14
 #define HIF_LINK_ID_NOT_ASSOCIATED (HIF_LINK_ID_MAX + 1)
 
-enum hif_sta_map_direction {
-	HIF_STA_MAP                       = 0x0,
-	HIF_STA_UNMAP                     = 0x1
-};
-
-struct hif_map_link_flags {
-	u8     map_direction:1;
-	u8     mfpc:1;
-	u8     reserved:6;
-} __packed;
-
 struct hif_req_map_link {
 	u8     mac_addr[ETH_ALEN];
-	struct hif_map_link_flags map_link_flags;
+	u8     unmap:1;
+	u8     mfpc:1;
+	u8     reserved:6;
 	u8     peer_sta_id;
 } __packed;
 
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 49523e70af6c..eddb60dec069 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -502,8 +502,8 @@ int hif_map_link(struct wfx_vif *wvif, bool unmap, u8 *mac_addr, int sta_id, boo
 		return -ENOMEM;
 	if (mac_addr)
 		ether_addr_copy(body->mac_addr, mac_addr);
-	body->map_link_flags.mfpc = mfp ? 1 : 0;
-	body->map_link_flags.map_direction = unmap ? 1 : 0;
+	body->mfpc = mfp ? 1 : 0;
+	body->unmap = unmap ? 1 : 0;
 	body->peer_sta_id = sta_id;
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_MAP_LINK, sizeof(*body));
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 15/31] staging: wfx: drop useless struct hif_suspend_resume_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (13 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 14/31] staging: wfx: drop useless struct hif_map_link_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 16/31] staging: wfx: drop useless struct hif_pm_mode Jerome Pouiller
                   ` (15 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_suspend_resume_flags has no reason to exist. Drop it and
simplify access to struct hif_ind_suspend_resume_tx.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 6 +-----
 drivers/staging/wfx/hif_rx.c      | 6 +++---
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index f86f6d491fb2..4b01be677e08 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -446,16 +446,12 @@ struct hif_cnf_map_link {
 	__le32 status;
 } __packed;
 
-struct hif_suspend_resume_flags {
+struct hif_ind_suspend_resume_tx {
 	u8     resume:1;
 	u8     reserved1:2;
 	u8     bc_mc_only:1;
 	u8     reserved2:4;
 	u8     reserved3;
-} __packed;
-
-struct hif_ind_suspend_resume_tx {
-	struct hif_suspend_resume_flags suspend_resume_flags;
 	__le16 peer_sta_set;
 } __packed;
 
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index cf7a956ef00a..798167aa4c7f 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -203,16 +203,16 @@ static int hif_suspend_resume_indication(struct wfx_dev *wdev,
 	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
 	const struct hif_ind_suspend_resume_tx *body = buf;
 
-	if (body->suspend_resume_flags.bc_mc_only) {
+	if (body->bc_mc_only) {
 		WARN_ON(!wvif);
-		if (body->suspend_resume_flags.resume)
+		if (body->resume)
 			wfx_suspend_resume_mc(wvif, STA_NOTIFY_AWAKE);
 		else
 			wfx_suspend_resume_mc(wvif, STA_NOTIFY_SLEEP);
 	} else {
 		WARN(body->peer_sta_set, "misunderstood indication");
 		WARN(hif->interface != 2, "misunderstood indication");
-		if (body->suspend_resume_flags.resume)
+		if (body->resume)
 			wfx_suspend_hot_dev(wdev, STA_NOTIFY_AWAKE);
 		else
 			wfx_suspend_hot_dev(wdev, STA_NOTIFY_SLEEP);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 16/31] staging: wfx: drop useless struct hif_pm_mode
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (14 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 15/31] staging: wfx: drop useless struct hif_suspend_resume_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 17/31] staging: wfx: drop useless struct hif_rx_flags Jerome Pouiller
                   ` (14 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_pm_mode has no reason to exist. Drop it and simplify access
to struct hif_req_set_pm_mode.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 6 +-----
 drivers/staging/wfx/hif_tx.c      | 4 ++--
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 4b01be677e08..6ecb23ceaf8c 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -378,14 +378,10 @@ struct hif_cnf_set_bss_params {
 	__le32 status;
 } __packed;
 
-struct hif_pm_mode {
+struct hif_req_set_pm_mode {
 	u8     enter_psm:1;
 	u8     reserved:6;
 	u8     fast_psm:1;
-} __packed;
-
-struct hif_req_set_pm_mode {
-	struct hif_pm_mode pm_mode;
 	u8     fast_psm_idle_period;
 	u8     ap_psm_change_period;
 	u8     min_auto_ps_poll_period;
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index eddb60dec069..134af4daee96 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -439,11 +439,11 @@ int hif_set_pm(struct wfx_vif *wvif, bool ps, int dynamic_ps_timeout)
 	if (!hif)
 		return -ENOMEM;
 	if (ps) {
-		body->pm_mode.enter_psm = 1;
+		body->enter_psm = 1;
 		// Firmware does not support more than 128ms
 		body->fast_psm_idle_period = min(dynamic_ps_timeout * 2, 255);
 		if (body->fast_psm_idle_period)
-			body->pm_mode.fast_psm = 1;
+			body->fast_psm = 1;
 	}
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_SET_PM_MODE, sizeof(*body));
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 17/31] staging: wfx: drop useless struct hif_rx_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (15 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 16/31] staging: wfx: drop useless struct hif_pm_mode Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 18/31] staging: wfx: drop useless struct hif_tx_result_flags Jerome Pouiller
                   ` (13 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_rx_flags has no reason to exist. Drop it and simplify access
to struct hif_ind_rx.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_rx.c     |  2 +-
 drivers/staging/wfx/hif_api_cmd.h | 25 ++++++++++---------------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 7fcbbfc53416..fe111d0aab63 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -70,7 +70,7 @@ void wfx_rx_cb(struct wfx_vif *wvif,
 	hdr->signal = arg->rcpi_rssi / 2 - 110;
 	hdr->antenna = 0;
 
-	if (arg->rx_flags.encryp)
+	if (arg->encryp)
 		hdr->flag |= RX_FLAG_DECRYPTED;
 
 	// Block ack negotiation is offloaded by the firmware. However,
diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 6ecb23ceaf8c..3a60bdf286f3 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -283,7 +283,12 @@ enum hif_ri_flags_encrypt {
 	HIF_RI_FLAGS_WAPI_ENCRYPTED                = 0x4
 };
 
-struct hif_rx_flags {
+struct hif_ind_rx {
+	__le32 status;
+	u8     channel_number;
+	u8     reserved1;
+	u8     rxed_rate;
+	u8     rcpi_rssi;
 	u8     encryp:3;
 	u8     in_aggr:1;
 	u8     first_aggr:1;
@@ -295,7 +300,7 @@ struct hif_rx_flags {
 	u8     match_ssid:1;
 	u8     match_bssid:1;
 	u8     more:1;
-	u8     reserved1:1;
+	u8     reserved2:1;
 	u8     ht:1;
 	u8     stbc:1;
 	u8     match_uc_addr:1;
@@ -303,23 +308,13 @@ struct hif_rx_flags {
 	u8     match_bc_addr:1;
 	u8     key_type:1;
 	u8     key_index:4;
-	u8     reserved2:1;
+	u8     reserved3:1;
 	u8     peer_sta_id:4;
-	u8     reserved3:2;
-	u8     reserved4:1;
-} __packed;
-
-struct hif_ind_rx {
-	__le32 status;
-	u8     channel_number;
-	u8     reserved;
-	u8     rxed_rate;
-	u8     rcpi_rssi;
-	struct hif_rx_flags rx_flags;
+	u8     reserved4:2;
+	u8     reserved5:1;
 	u8     frame[];
 } __packed;
 
-
 struct hif_req_edca_queue_params {
 	u8     queue_id;
 	u8     reserved1;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 18/31] staging: wfx: drop useless struct hif_tx_result_flags
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (16 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 17/31] staging: wfx: drop useless struct hif_rx_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 19/31] staging: wfx: drop useless structs only used in hif_req_tx Jerome Pouiller
                   ` (12 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Struct hif_tx_result_flags has no reason to exist. Drop it and simplify
access to struct hif_cnf_tx.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c     |  3 +--
 drivers/staging/wfx/hif_api_cmd.h | 16 ++++++----------
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 485907b0faa2..1f2158d6eaa9 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -518,8 +518,7 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg)
 		else
 			tx_info->flags |= IEEE80211_TX_STAT_ACK;
 	} else if (arg->status == HIF_STATUS_TX_FAIL_REQUEUE) {
-		WARN(!arg->tx_result_flags.requeue,
-		     "incoherent status and result_flags");
+		WARN(!arg->requeue, "incoherent status and result_flags");
 		if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) {
 			wvif->after_dtim_tx_allowed = false; // DTIM period elapsed
 			schedule_work(&wvif->update_tim_work);
diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 3a60bdf286f3..b86ec39f2615 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -248,15 +248,6 @@ enum hif_qos_ackplcy {
 	HIF_QOS_ACKPLCY_BLCKACK                        = 0x3
 };
 
-struct hif_tx_result_flags {
-	u8     aggr:1;
-	u8     requeue:1;
-	u8     ack_policy:2;
-	u8     txop_limit:1;
-	u8     reserved1:3;
-	u8     reserved2;
-} __packed;
-
 struct hif_cnf_tx {
 	__le32 status;
 	// packet_id is copied from struct hif_req_tx without been interpreted
@@ -264,7 +255,12 @@ struct hif_cnf_tx {
 	u32    packet_id;
 	u8     txed_rate;
 	u8     ack_failures;
-	struct hif_tx_result_flags tx_result_flags;
+	u8     aggr:1;
+	u8     requeue:1;
+	u8     ack_policy:2;
+	u8     txop_limit:1;
+	u8     reserved1:3;
+	u8     reserved2;
 	__le32 media_delay;
 	__le32 tx_queue_delay;
 } __packed;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 19/31] staging: wfx: drop useless structs only used in hif_req_tx
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (17 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 18/31] staging: wfx: drop useless struct hif_tx_result_flags Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 20/31] staging: wfx: drop useless stricts only used in hif_req_start_scan_alt Jerome Pouiller
                   ` (11 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The structs hif_queue, hif_data_flags, hif_tx_flags and
hif_ht_tx_parameters have no real reasons to exist. Drop them and
simplify access to fields of struct hif_req_tx.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c     | 39 ++++++++++--------------
 drivers/staging/wfx/hif_api_cmd.h | 49 +++++++++----------------------
 2 files changed, 30 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 1f2158d6eaa9..e2fb770e98fc 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -300,23 +300,14 @@ static u8 wfx_tx_get_rate_id(struct wfx_vif *wvif,
 	return rate_id;
 }
 
-static struct hif_ht_tx_parameters wfx_tx_get_tx_parms(struct wfx_dev *wdev,
-						       struct ieee80211_tx_info *tx_info)
+static int wfx_tx_get_frame_format(struct ieee80211_tx_info *tx_info)
 {
-	struct ieee80211_tx_rate *rate = &tx_info->driver_rates[0];
-	struct hif_ht_tx_parameters ret = { };
-
-	if (!(rate->flags & IEEE80211_TX_RC_MCS))
-		ret.frame_format = HIF_FRAME_FORMAT_NON_HT;
-	else if (!(rate->flags & IEEE80211_TX_RC_GREEN_FIELD))
-		ret.frame_format = HIF_FRAME_FORMAT_MIXED_FORMAT_HT;
+	if (!(tx_info->driver_rates[0].flags & IEEE80211_TX_RC_MCS))
+		return HIF_FRAME_FORMAT_NON_HT;
+	else if (!(tx_info->driver_rates[0].flags & IEEE80211_TX_RC_GREEN_FIELD))
+		return HIF_FRAME_FORMAT_MIXED_FORMAT_HT;
 	else
-		ret.frame_format = HIF_FRAME_FORMAT_GF_HT_11N;
-	if (rate->flags & IEEE80211_TX_RC_SHORT_GI)
-		ret.short_gi = 1;
-	if (tx_info->flags & IEEE80211_TX_CTL_STBC)
-		ret.stbc = 0; // FIXME: Not yet supported by firmware?
-	return ret;
+		return HIF_FRAME_FORMAT_GF_HT_11N;
 }
 
 static int wfx_tx_get_icv_len(struct ieee80211_key_conf *hw_key)
@@ -377,14 +368,16 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	req->packet_id |= IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl)) << 16;
 	req->packet_id |= queue_id << 28;
 
-	req->data_flags.fc_offset = offset;
+	req->fc_offset = offset;
 	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
-		req->data_flags.after_dtim = 1;
-	req->queue_id.peer_sta_id = wfx_tx_get_link_id(wvif, sta, hdr);
+		req->after_dtim = 1;
+	req->peer_sta_id = wfx_tx_get_link_id(wvif, sta, hdr);
 	// Queue index are inverted between firmware and Linux
-	req->queue_id.queue_id = 3 - queue_id;
-	req->ht_tx_parameters = wfx_tx_get_tx_parms(wvif->wdev, tx_info);
-	req->tx_flags.retry_policy_index = wfx_tx_get_rate_id(wvif, tx_info);
+	req->queue_id = 3 - queue_id;
+	req->retry_policy_index = wfx_tx_get_rate_id(wvif, tx_info);
+	req->frame_format = wfx_tx_get_frame_format(tx_info);
+	if (tx_info->driver_rates[0].flags & IEEE80211_TX_RC_SHORT_GI)
+		req->short_gi = 1;
 
 	// Auxiliary operations
 	wfx_tx_queues_put(wvif, skb);
@@ -436,10 +429,10 @@ static void wfx_skb_dtor(struct wfx_vif *wvif, struct sk_buff *skb)
 	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
 	unsigned int offset = sizeof(struct hif_msg) +
 			      sizeof(struct hif_req_tx) +
-			      req->data_flags.fc_offset;
+			      req->fc_offset;
 
 	WARN_ON(!wvif);
-	wfx_tx_policy_put(wvif, req->tx_flags.retry_policy_index);
+	wfx_tx_policy_put(wvif, req->retry_policy_index);
 	skb_pull(skb, offset);
 	ieee80211_tx_status_irqsafe(wvif->wdev->hw, skb);
 }
diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index b86ec39f2615..d5ef1118b87c 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -191,53 +191,32 @@ enum hif_frame_format {
 	HIF_FRAME_FORMAT_GF_HT_11N                 = 0x2
 };
 
-enum hif_stbc {
-	HIF_STBC_NOT_ALLOWED                       = 0x0,
-	HIF_STBC_ALLOWED                           = 0x1
-};
-
-struct hif_queue {
+struct hif_req_tx {
+	// packet_id is not interpreted by the device, so it is not necessary to
+	// declare it little endian
+	u32    packet_id;
+	u8     max_tx_rate;
 	u8     queue_id:2;
 	u8     peer_sta_id:4;
-	u8     reserved:2;
-} __packed;
-
-struct hif_data_flags {
+	u8     reserved1:2;
 	u8     more:1;
 	u8     fc_offset:3;
 	u8     after_dtim:1;
-	u8     reserved:3;
-} __packed;
-
-struct hif_tx_flags {
+	u8     reserved2:3;
 	u8     start_exp:1;
-	u8     reserved:3;
+	u8     reserved3:3;
 	u8     retry_policy_index:4;
-} __packed;
-
-struct hif_ht_tx_parameters {
+	__le32 reserved4;
+	__le32 expire_time;
 	u8     frame_format:4;
 	u8     fec_coding:1;
 	u8     short_gi:1;
-	u8     reserved1:1;
+	u8     reserved5:1;
 	u8     stbc:1;
-	u8     reserved2;
+	u8     reserved6;
 	u8     aggregation:1;
-	u8     reserved3:7;
-	u8     reserved4;
-} __packed;
-
-struct hif_req_tx {
-	// packet_id is not interpreted by the device, so it is not necessary to
-	// declare it little endian
-	u32    packet_id;
-	u8     max_tx_rate;
-	struct hif_queue queue_id;
-	struct hif_data_flags data_flags;
-	struct hif_tx_flags tx_flags;
-	__le32 reserved;
-	__le32 expire_time;
-	struct hif_ht_tx_parameters ht_tx_parameters;
+	u8     reserved7:7;
+	u8     reserved8;
 	u8     frame[];
 } __packed;
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 20/31] staging: wfx: drop useless stricts only used in hif_req_start_scan_alt
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (18 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 19/31] staging: wfx: drop useless structs only used in hif_req_tx Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 21/31] staging: wfx: drop useless structs only used in hif_ind_startup Jerome Pouiller
                   ` (10 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The structs hif_scan_type, hif_scan_flags and hif_auto_scan_param have
no real reasons to exist (apart maybe defining namespaces). Moreover,
the names of the fields within these structs are not all meaningful.

Drop the structs and rename the fields.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 32 ++++++++++---------------------
 drivers/staging/wfx/hif_tx.c      |  5 ++---
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index d5ef1118b87c..c7e6fdf183b1 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -113,25 +113,6 @@ struct hif_cnf_update_ie {
 	__le32 status;
 } __packed;
 
-struct hif_scan_type {
-	u8     type:1;
-	u8     mode:1;
-	u8     reserved:6;
-} __packed;
-
-struct hif_scan_flags {
-	u8     fbg:1;
-	u8     reserved1:1;
-	u8     pre:1;
-	u8     reserved2:5;
-} __packed;
-
-struct hif_auto_scan_param {
-	__le16 interval;
-	u8     reserved;
-	s8     rssi_thr;
-} __packed;
-
 struct hif_ssid_def {
 	__le32 ssid_length;
 	u8     ssid[HIF_API_SSID_SIZE];
@@ -142,10 +123,17 @@ struct hif_ssid_def {
 
 struct hif_req_start_scan_alt {
 	u8     band;
-	struct hif_scan_type scan_type;
-	struct hif_scan_flags scan_flags;
+	u8     maintain_current_bss:1;
+	u8     periodic:1;
+	u8     reserved1:6;
+	u8     disallow_ps:1;
+	u8     reserved2:1;
+	u8     short_preamble:1;
+	u8     reserved3:5;
 	u8     max_transmit_rate;
-	struct hif_auto_scan_param auto_scan_param;
+	__le16 periodic_interval;
+	u8     reserved4;
+	s8     periodic_rssi_thr;
 	u8     num_of_probe_requests;
 	u8     probe_delay;
 	u8     num_of_ssids;
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 134af4daee96..0553e79595a6 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -256,9 +256,8 @@ int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
 			cpu_to_le32(req->ssids[i].ssid_len);
 	}
 	body->num_of_ssids = HIF_API_MAX_NB_SSIDS;
-	// Background scan is always a good idea
-	body->scan_type.type = 1;
-	body->scan_flags.fbg = 1;
+	body->maintain_current_bss = 1;
+	body->disallow_ps = 1;
 	body->tx_power_level =
 		cpu_to_le32(req->channels[chan_start_idx]->max_power);
 	body->num_of_channels = chan_num;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 21/31] staging: wfx: drop useless structs only used in hif_ind_startup
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (19 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 20/31] staging: wfx: drop useless stricts only used in hif_req_start_scan_alt Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 22/31] staging: wfx: drop useless union hif_privacy_key_data Jerome Pouiller
                   ` (9 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The structs hif_capabilities, hif_otp_regul_sel_mode_info and
hif_otp_phy_info have no real reasons to exist. Drop them and simplify
access to fields of struct hif_ind_startup.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_general.h | 32 +++++++++------------------
 drivers/staging/wfx/main.c            |  9 ++++----
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index 0dc13176a05e..4058016ec664 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -122,25 +122,6 @@ enum hif_fw_type {
 	HIF_FW_TYPE_WSM  = 0x2
 };
 
-struct hif_capabilities {
-	u8     link_mode:2;
-	u8     reserved1:6;
-	u8     reserved2;
-	u8     reserved3;
-	u8     reserved4;
-} __packed;
-
-struct hif_otp_regul_sel_mode_info {
-	u8     region_sel_mode:4;
-	u8     reserved:4;
-} __packed;
-
-struct hif_otp_phy_info {
-	u8     phy1_region:3;
-	u8     phy0_region:3;
-	u8     otp_phy_ver:2;
-} __packed;
-
 struct hif_ind_startup {
 	// As the others, this struct is interpreted as little endian by the
 	// device. However, this struct is also used by the driver. We prefer to
@@ -156,14 +137,21 @@ struct hif_ind_startup {
 	u8     mac_addr[2][ETH_ALEN];
 	u8     api_version_minor;
 	u8     api_version_major;
-	struct hif_capabilities capabilities;
+	u8     link_mode:2;
+	u8     reserved1:6;
+	u8     reserved2;
+	u8     reserved3;
+	u8     reserved4;
 	u8     firmware_build;
 	u8     firmware_minor;
 	u8     firmware_major;
 	u8     firmware_type;
 	u8     disabled_channel_list[2];
-	struct hif_otp_regul_sel_mode_info regul_sel_mode_info;
-	struct hif_otp_phy_info otp_phy_info;
+	u8     region_sel_mode:4;
+	u8     reserved5:4;
+	u8     phy1_region:3;
+	u8     phy0_region:3;
+	u8     otp_phy_ver:2;
 	u32    supported_rate_mask;
 	u8     firmware_label[128];
 } __packed;
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 1017a2290f08..2a9098bad1f5 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -359,9 +359,8 @@ int wfx_probe(struct wfx_dev *wdev)
 	dev_info(wdev->dev, "started firmware %d.%d.%d \"%s\" (API: %d.%d, keyset: %02X, caps: 0x%.8X)\n",
 		 wdev->hw_caps.firmware_major, wdev->hw_caps.firmware_minor,
 		 wdev->hw_caps.firmware_build, wdev->hw_caps.firmware_label,
-		 wdev->hw_caps.api_version_major,
-		 wdev->hw_caps.api_version_minor,
-		 wdev->keyset, *((u32 *)&wdev->hw_caps.capabilities));
+		 wdev->hw_caps.api_version_major, wdev->hw_caps.api_version_minor,
+		 wdev->keyset, wdev->hw_caps.link_mode);
 	snprintf(wdev->hw->wiphy->fw_version,
 		 sizeof(wdev->hw->wiphy->fw_version),
 		 "%d.%d.%d",
@@ -377,13 +376,13 @@ int wfx_probe(struct wfx_dev *wdev)
 		goto err0;
 	}
 
-	if (wdev->hw_caps.capabilities.link_mode == SEC_LINK_ENFORCED) {
+	if (wdev->hw_caps.link_mode == SEC_LINK_ENFORCED) {
 		dev_err(wdev->dev,
 			"chip require secure_link, but can't negotiate it\n");
 		goto err0;
 	}
 
-	if (wdev->hw_caps.regul_sel_mode_info.region_sel_mode) {
+	if (wdev->hw_caps.region_sel_mode) {
 		wdev->hw->wiphy->bands[NL80211_BAND_2GHZ]->channels[11].flags |= IEEE80211_CHAN_NO_IR;
 		wdev->hw->wiphy->bands[NL80211_BAND_2GHZ]->channels[12].flags |= IEEE80211_CHAN_NO_IR;
 		wdev->hw->wiphy->bands[NL80211_BAND_2GHZ]->channels[13].flags |= IEEE80211_CHAN_DISABLED;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 22/31] staging: wfx: drop useless union hif_privacy_key_data
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (20 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 21/31] staging: wfx: drop useless structs only used in hif_ind_startup Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 23/31] staging: wfx: drop useless union hif_event_data Jerome Pouiller
                   ` (8 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The union hif_privacy_key_data is never used in the driver. So, it is
not necessary to declare it separately from hif_req_add_key.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index c7e6fdf183b1..17cd317de824 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -500,25 +500,23 @@ struct hif_igtk_group_key {
 	u8     ipn[HIF_API_IPN_SIZE];
 } __packed;
 
-union hif_privacy_key_data {
-	struct hif_wep_pairwise_key  wep_pairwise_key;
-	struct hif_wep_group_key     wep_group_key;
-	struct hif_tkip_pairwise_key tkip_pairwise_key;
-	struct hif_tkip_group_key    tkip_group_key;
-	struct hif_aes_pairwise_key  aes_pairwise_key;
-	struct hif_aes_group_key     aes_group_key;
-	struct hif_wapi_pairwise_key wapi_pairwise_key;
-	struct hif_wapi_group_key    wapi_group_key;
-	struct hif_igtk_group_key    igtk_group_key;
-};
-
 struct hif_req_add_key {
 	u8     type;
 	u8     entry_index;
 	u8     int_id:2;
 	u8     reserved1:6;
 	u8     reserved2;
-	union hif_privacy_key_data key;
+	union {
+		struct hif_wep_pairwise_key  wep_pairwise_key;
+		struct hif_wep_group_key     wep_group_key;
+		struct hif_tkip_pairwise_key tkip_pairwise_key;
+		struct hif_tkip_group_key    tkip_group_key;
+		struct hif_aes_pairwise_key  aes_pairwise_key;
+		struct hif_aes_group_key     aes_group_key;
+		struct hif_wapi_pairwise_key wapi_pairwise_key;
+		struct hif_wapi_group_key    wapi_group_key;
+		struct hif_igtk_group_key    igtk_group_key;
+	} key;
 } __packed;
 
 struct hif_cnf_add_key {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 23/31] staging: wfx: drop useless union hif_event_data
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (21 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 22/31] staging: wfx: drop useless union hif_privacy_key_data Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 24/31] staging: wfx: drop useless union hif_indication_data Jerome Pouiller
                   ` (7 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The union hif_event_data is never used in the driver. So, it is
not necessary to declare it separately from hif_ind_event.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 17cd317de824..c18e762485a9 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -548,15 +548,13 @@ enum hif_ps_mode_error {
 	HIF_PS_ERROR_AP_NO_DATA_AFTER_TIM          = 4
 };
 
-union hif_event_data {
-	u8     rcpi_rssi;
-	__le32 ps_mode_error;
-	__le32 peer_sta_set;
-};
-
 struct hif_ind_event {
 	__le32 event_id;
-	union hif_event_data event_data;
+	union {
+		u8     rcpi_rssi;
+		__le32 ps_mode_error;
+		__le32 peer_sta_set;
+	} event_data;
 } __packed;
 
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 24/31] staging: wfx: drop useless union hif_indication_data
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (22 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 23/31] staging: wfx: drop useless union hif_event_data Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 25/31] staging: wfx: drop struct hif_ie_tlv Jerome Pouiller
                   ` (6 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The union hif_indication_data is never used in the driver. So, it is not
necessary to declare it separately from hif_ind_generic.

In add, drop prefix 'indication_' from the names 'indication_type' and
'indication_data' since it is redundant with the name of the struct.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_general.h | 13 +++++--------
 drivers/staging/wfx/hif_rx.c          | 11 +++++------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index 4058016ec664..da63ba6f5148 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -221,15 +221,12 @@ struct hif_tx_power_loop_info {
 	u8     reserved;
 } __packed;
 
-union hif_indication_data {
-	struct hif_rx_stats rx_stats;
-	struct hif_tx_power_loop_info tx_power_loop_info;
-	u8     raw_data[1];
-};
-
 struct hif_ind_generic {
-	__le32 indication_type;
-	union hif_indication_data indication_data;
+	__le32 type;
+	union {
+		struct hif_rx_stats rx_stats;
+		struct hif_tx_power_loop_info tx_power_loop_info;
+	} data;
 } __packed;
 
 enum hif_error {
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 798167aa4c7f..6950b3e9d7cf 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -225,29 +225,28 @@ static int hif_generic_indication(struct wfx_dev *wdev,
 				  const struct hif_msg *hif, const void *buf)
 {
 	const struct hif_ind_generic *body = buf;
-	int type = le32_to_cpu(body->indication_type);
+	int type = le32_to_cpu(body->type);
 
 	switch (type) {
 	case HIF_GENERIC_INDICATION_TYPE_RAW:
 		return 0;
 	case HIF_GENERIC_INDICATION_TYPE_STRING:
-		dev_info(wdev->dev, "firmware says: %s\n",
-			 (char *)body->indication_data.raw_data);
+		dev_info(wdev->dev, "firmware says: %s\n", (char *)&body->data);
 		return 0;
 	case HIF_GENERIC_INDICATION_TYPE_RX_STATS:
 		mutex_lock(&wdev->rx_stats_lock);
 		// Older firmware send a generic indication beside RxStats
 		if (!wfx_api_older_than(wdev, 1, 4))
 			dev_info(wdev->dev, "Rx test ongoing. Temperature: %d°C\n",
-				 body->indication_data.rx_stats.current_temp);
-		memcpy(&wdev->rx_stats, &body->indication_data.rx_stats,
+				 body->data.rx_stats.current_temp);
+		memcpy(&wdev->rx_stats, &body->data.rx_stats,
 		       sizeof(wdev->rx_stats));
 		mutex_unlock(&wdev->rx_stats_lock);
 		return 0;
 	case HIF_GENERIC_INDICATION_TYPE_TX_POWER_LOOP_INFO:
 		mutex_lock(&wdev->tx_power_loop_info_lock);
 		memcpy(&wdev->tx_power_loop_info,
-		       &body->indication_data.tx_power_loop_info,
+		       &body->data.tx_power_loop_info,
 		       sizeof(wdev->tx_power_loop_info));
 		mutex_unlock(&wdev->tx_power_loop_info_lock);
 		return 0;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 25/31] staging: wfx: drop struct hif_ie_tlv
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (23 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 24/31] staging: wfx: drop useless union hif_indication_data Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 26/31] staging: wfx: drop macro API_SSID_SIZE Jerome Pouiller
                   ` (5 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

This struct hif_ie_tlv is definitively an Information Element (IE). This
struct is defined by 802.11 specification and already exists in
mac80211. Reuse this definition instead of struct hif_ie_tlv.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index c18e762485a9..ef5483609dcb 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -8,6 +8,8 @@
 #ifndef WFX_HIF_API_CMD_H
 #define WFX_HIF_API_CMD_H
 
+#include <linux/ieee80211.h>
+
 #include "hif_api_general.h"
 
 #define HIF_API_SSID_SIZE                      API_SSID_SIZE
@@ -93,12 +95,6 @@ struct hif_cnf_write_mib {
 	__le32 status;
 } __packed;
 
-struct hif_ie_tlv {
-	u8     type;
-	u8     length;
-	u8     data[];
-} __packed;
-
 struct hif_req_update_ie {
 	u8     beacon:1;
 	u8     probe_resp:1;
@@ -106,7 +102,7 @@ struct hif_req_update_ie {
 	u8     reserved1:5;
 	u8     reserved2;
 	__le16 num_ies;
-	struct hif_ie_tlv ie[];
+	struct element ie[];
 } __packed;
 
 struct hif_cnf_update_ie {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 26/31] staging: wfx: drop macro API_SSID_SIZE
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (24 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 25/31] staging: wfx: drop struct hif_ie_tlv Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 27/31] staging: wfx: fix naming of hif_tx_rate_retry_policy Jerome Pouiller
                   ` (4 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The maximum length of a SSID is defined by 802.11 specification. It is
already defined in mac80211: IEEE80211_MAX_SSID_LEN. Therefore, use this
generic definition.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h     | 8 +++-----
 drivers/staging/wfx/hif_api_general.h | 2 --
 drivers/staging/wfx/hif_tx.c          | 2 --
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index ef5483609dcb..7b365bd01b81 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -12,8 +12,6 @@
 
 #include "hif_api_general.h"
 
-#define HIF_API_SSID_SIZE                      API_SSID_SIZE
-
 enum hif_requests_ids {
 	HIF_REQ_ID_RESET                = 0x0a,
 	HIF_REQ_ID_READ_MIB             = 0x05,
@@ -111,7 +109,7 @@ struct hif_cnf_update_ie {
 
 struct hif_ssid_def {
 	__le32 ssid_length;
-	u8     ssid[HIF_API_SSID_SIZE];
+	u8     ssid[IEEE80211_MAX_SSID_LEN];
 } __packed;
 
 #define HIF_API_MAX_NB_SSIDS                           2
@@ -307,7 +305,7 @@ struct hif_req_join {
 	u8     force_with_ind:1;
 	u8     reserved6:4;
 	__le32 ssid_length;
-	u8     ssid[HIF_API_SSID_SIZE];
+	u8     ssid[IEEE80211_MAX_SSID_LEN];
 	__le32 beacon_interval;
 	__le32 basic_rate_set;
 } __packed;
@@ -364,7 +362,7 @@ struct hif_req_start {
 	u8     reserved3:7;
 	u8     reserved4;
 	u8     ssid_length;
-	u8     ssid[HIF_API_SSID_SIZE];
+	u8     ssid[IEEE80211_MAX_SSID_LEN];
 	__le32 basic_rate_set;
 } __packed;
 
diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index da63ba6f5148..c9e3c0f758c8 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -17,8 +17,6 @@
 #define __packed __attribute__((__packed__))
 #endif
 
-#define API_SSID_SIZE             32
-
 #define HIF_ID_IS_INDICATION      0x80
 #define HIF_COUNTER_MAX           7
 
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 0553e79595a6..a75c6b9804ba 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -245,8 +245,6 @@ int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
 	WARN(chan_num > HIF_API_MAX_NB_CHANNELS, "invalid params");
 	WARN(req->n_ssids > HIF_API_MAX_NB_SSIDS, "invalid params");
 
-	compiletime_assert(IEEE80211_MAX_SSID_LEN == HIF_API_SSID_SIZE,
-			   "API inconsistency");
 	if (!hif)
 		return -ENOMEM;
 	for (i = 0; i < req->n_ssids; i++) {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 27/31] staging: wfx: fix naming of hif_tx_rate_retry_policy
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (25 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 26/31] staging: wfx: drop macro API_SSID_SIZE Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 28/31] staging: wfx: fix spaces Jerome Pouiller
                   ` (3 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

In the wfx driver, the prefix 'hif_mib_' is normally used for structures
that represent a hardware message. hif_mib_tx_rate_retry_policy does not
fall in this category. So, rename it.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_mib.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_mib.h b/drivers/staging/wfx/hif_api_mib.h
index 73873d29456d..55bd399ccdfb 100644
--- a/drivers/staging/wfx/hif_api_mib.h
+++ b/drivers/staging/wfx/hif_api_mib.h
@@ -305,7 +305,7 @@ struct hif_mib_set_uapsd_information {
 	__le16 auto_trigger_step;
 } __packed;
 
-struct hif_mib_tx_rate_retry_policy {
+struct hif_tx_rate_retry_policy {
 	u8     policy_index;
 	u8     short_retry_count;
 	u8     long_retry_count;
@@ -324,7 +324,7 @@ struct hif_mib_tx_rate_retry_policy {
 struct hif_mib_set_tx_rate_retry_policy {
 	u8     num_tx_rate_policies;
 	u8     reserved[3];
-	struct hif_mib_tx_rate_retry_policy tx_rate_retry_policy[];
+	struct hif_tx_rate_retry_policy tx_rate_retry_policy[];
 } __packed;
 
 struct hif_mib_protected_mgmt_policy {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 28/31] staging: wfx: fix spaces
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (26 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 27/31] staging: wfx: fix naming of hif_tx_rate_retry_policy Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 29/31] staging: wfx: uniformize naming rules in hif_tx_mib.c Jerome Pouiller
                   ` (2 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

There is no reason to place two spaces between the field tx_conf_payload
and its type.

In the same vein, remove duplicate empty lines between declarations.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_api_cmd.h     | 4 +---
 drivers/staging/wfx/hif_api_general.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/wfx/hif_api_cmd.h b/drivers/staging/wfx/hif_api_cmd.h
index 7b365bd01b81..bdd468800189 100644
--- a/drivers/staging/wfx/hif_api_cmd.h
+++ b/drivers/staging/wfx/hif_api_cmd.h
@@ -229,7 +229,7 @@ struct hif_cnf_tx {
 struct hif_cnf_multi_transmit {
 	u8     num_tx_confs;
 	u8     reserved[3];
-	struct hif_cnf_tx   tx_conf_payload[];
+	struct hif_cnf_tx tx_conf_payload[];
 } __packed;
 
 enum hif_ri_flags_encrypt {
@@ -349,7 +349,6 @@ struct hif_ind_set_pm_mode_cmpl {
 	u8     reserved[3];
 } __packed;
 
-
 struct hif_req_start {
 	u8     mode;
 	u8     band;
@@ -551,5 +550,4 @@ struct hif_ind_event {
 	} event_data;
 } __packed;
 
-
 #endif
diff --git a/drivers/staging/wfx/hif_api_general.h b/drivers/staging/wfx/hif_api_general.h
index c9e3c0f758c8..9d522bc1aa69 100644
--- a/drivers/staging/wfx/hif_api_general.h
+++ b/drivers/staging/wfx/hif_api_general.h
@@ -113,7 +113,6 @@ enum hif_api_rate_index {
 	API_RATE_NUM_ENTRIES       = 22
 };
 
-
 enum hif_fw_type {
 	HIF_FW_TYPE_ETF  = 0x0,
 	HIF_FW_TYPE_WFM  = 0x1,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 29/31] staging: wfx: uniformize naming rules in hif_tx_mib.c
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (27 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 28/31] staging: wfx: fix spaces Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 30/31] staging: wfx: drop async field from struct hif_cmd Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 31/31] staging: wfx: update TODO list Jerome Pouiller
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

hif_tx_mib.c contains functions that format data to be sent to the
hardware. In this file, sometime the struct to be sent is named 'arg',
sometime 'val'. In some other function 'val' is used for the argument of
the function.

This patch uniformize the things and choose to call all the data in
destination to the hardware 'arg' (note this choice is only dictated by
the number of lines to change in the code)

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_tx_mib.c | 50 ++++++++++++++++----------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/wfx/hif_tx_mib.c b/drivers/staging/wfx/hif_tx_mib.c
index 2eb2a20890c7..c375b9052a07 100644
--- a/drivers/staging/wfx/hif_tx_mib.c
+++ b/drivers/staging/wfx/hif_tx_mib.c
@@ -29,7 +29,7 @@ int hif_set_beacon_wakeup_period(struct wfx_vif *wvif,
 				 unsigned int dtim_interval,
 				 unsigned int listen_interval)
 {
-	struct hif_mib_beacon_wake_up_period val = {
+	struct hif_mib_beacon_wake_up_period arg = {
 		.wakeup_period_min = dtim_interval,
 		.receive_dtim = 0,
 		.wakeup_period_max = listen_interval,
@@ -39,7 +39,7 @@ int hif_set_beacon_wakeup_period(struct wfx_vif *wvif,
 		return -EINVAL;
 	return hif_write_mib(wvif->wdev, wvif->id,
 			     HIF_MIB_ID_BEACON_WAKEUP_PERIOD,
-			     &val, sizeof(val));
+			     &arg, sizeof(arg));
 }
 
 int hif_set_rcpi_rssi_threshold(struct wfx_vif *wvif,
@@ -92,31 +92,31 @@ int hif_set_macaddr(struct wfx_vif *wvif, u8 *mac)
 int hif_set_rx_filter(struct wfx_vif *wvif,
 		      bool filter_bssid, bool filter_prbreq)
 {
-	struct hif_mib_rx_filter val = { };
+	struct hif_mib_rx_filter arg = { };
 
 	if (filter_bssid)
-		val.bssid_filter = 1;
+		arg.bssid_filter = 1;
 	if (!filter_prbreq)
-		val.fwd_probe_req = 1;
+		arg.fwd_probe_req = 1;
 	return hif_write_mib(wvif->wdev, wvif->id, HIF_MIB_ID_RX_FILTER,
-			     &val, sizeof(val));
+			     &arg, sizeof(arg));
 }
 
 int hif_set_beacon_filter_table(struct wfx_vif *wvif, int tbl_len,
 				const struct hif_ie_table_entry *tbl)
 {
 	int ret;
-	struct hif_mib_bcn_filter_table *val;
-	int buf_len = struct_size(val, ie_table, tbl_len);
+	struct hif_mib_bcn_filter_table *arg;
+	int buf_len = struct_size(arg, ie_table, tbl_len);
 
-	val = kzalloc(buf_len, GFP_KERNEL);
-	if (!val)
+	arg = kzalloc(buf_len, GFP_KERNEL);
+	if (!arg)
 		return -ENOMEM;
-	val->num_of_info_elmts = cpu_to_le32(tbl_len);
-	memcpy(val->ie_table, tbl, flex_array_size(val, ie_table, tbl_len));
+	arg->num_of_info_elmts = cpu_to_le32(tbl_len);
+	memcpy(arg->ie_table, tbl, flex_array_size(arg, ie_table, tbl_len));
 	ret = hif_write_mib(wvif->wdev, wvif->id,
-			    HIF_MIB_ID_BEACON_FILTER_TABLE, val, buf_len);
-	kfree(val);
+			    HIF_MIB_ID_BEACON_FILTER_TABLE, arg, buf_len);
+	kfree(arg);
 	return ret;
 }
 
@@ -134,13 +134,13 @@ int hif_beacon_filter_control(struct wfx_vif *wvif,
 
 int hif_set_operational_mode(struct wfx_dev *wdev, enum hif_op_power_mode mode)
 {
-	struct hif_mib_gl_operational_power_mode val = {
+	struct hif_mib_gl_operational_power_mode arg = {
 		.power_mode = mode,
 		.wup_ind_activation = 1,
 	};
 
 	return hif_write_mib(wdev, -1, HIF_MIB_ID_GL_OPERATIONAL_POWER_MODE,
-			     &val, sizeof(val));
+			     &arg, sizeof(arg));
 }
 
 int hif_set_template_frame(struct wfx_vif *wvif, struct sk_buff *skb,
@@ -161,36 +161,36 @@ int hif_set_template_frame(struct wfx_vif *wvif, struct sk_buff *skb,
 
 int hif_set_mfp(struct wfx_vif *wvif, bool capable, bool required)
 {
-	struct hif_mib_protected_mgmt_policy val = { };
+	struct hif_mib_protected_mgmt_policy arg = { };
 
 	WARN(required && !capable, "incoherent arguments");
 	if (capable) {
-		val.pmf_enable = 1;
-		val.host_enc_auth_frames = 1;
+		arg.pmf_enable = 1;
+		arg.host_enc_auth_frames = 1;
 	}
 	if (!required)
-		val.unpmf_allowed = 1;
+		arg.unpmf_allowed = 1;
 	return hif_write_mib(wvif->wdev, wvif->id,
 			     HIF_MIB_ID_PROTECTED_MGMT_POLICY,
-			     &val, sizeof(val));
+			     &arg, sizeof(arg));
 }
 
 int hif_set_block_ack_policy(struct wfx_vif *wvif,
 			     u8 tx_tid_policy, u8 rx_tid_policy)
 {
-	struct hif_mib_block_ack_policy val = {
+	struct hif_mib_block_ack_policy arg = {
 		.block_ack_tx_tid_policy = tx_tid_policy,
 		.block_ack_rx_tid_policy = rx_tid_policy,
 	};
 
 	return hif_write_mib(wvif->wdev, wvif->id, HIF_MIB_ID_BLOCK_ACK_POLICY,
-			     &val, sizeof(val));
+			     &arg, sizeof(arg));
 }
 
 int hif_set_association_mode(struct wfx_vif *wvif, int ampdu_density,
 			     bool greenfield, bool short_preamble)
 {
-	struct hif_mib_set_association_mode val = {
+	struct hif_mib_set_association_mode arg = {
 		.preambtype_use = 1,
 		.mode = 1,
 		.spacing = 1,
@@ -200,7 +200,7 @@ int hif_set_association_mode(struct wfx_vif *wvif, int ampdu_density,
 	};
 
 	return hif_write_mib(wvif->wdev, wvif->id,
-			     HIF_MIB_ID_SET_ASSOCIATION_MODE, &val, sizeof(val));
+			     HIF_MIB_ID_SET_ASSOCIATION_MODE, &arg, sizeof(arg));
 }
 
 int hif_set_tx_rate_retry_policy(struct wfx_vif *wvif,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 30/31] staging: wfx: drop async field from struct hif_cmd
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (28 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 29/31] staging: wfx: uniformize naming rules in hif_tx_mib.c Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  2020-09-07 10:15 ` [PATCH 31/31] staging: wfx: update TODO list Jerome Pouiller
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller, kbuild test robot,
	Julia Lawall

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The parameter "async" in wfx_cmd_send() allows to send command without
waiting for the reply. In this case, the mutex hif_cmd.lock is released
asynchronously in the context of the receiver workqueue.

However, "kbuild test robot" complains about this architecture[1] since
it is not able to follow the lock duration of hif_cmd.lock (and indeed,
the state of the driver if the hardware wouldn't reply is not well
defined).

Besides, this feature is not really necessary. It is only used by
hif_shutdown(). This function hijack the 'async' flag to run a command
that won't answer.

So, this patch removes the 'async' flag and introduces a 'no_reply' flag.
Thus, the mutex hif_cmd.lock is only acquired/released from
hif_cmd_send(). Therefore:
  - hif_shutdown() does not have to touch the private data of the struct
    hif_cmd
  - Kbuild test robot should be happy
  - the resulting code is simpler

[1] https://lore.kernel.org/driverdev-devel/alpine.DEB.2.21.1910041317381.2992@hadrien/

Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_rx.c |  7 +------
 drivers/staging/wfx/hif_tx.c | 24 +++++++++---------------
 drivers/staging/wfx/hif_tx.h |  1 -
 3 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index 6950b3e9d7cf..b40af86356f1 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -47,12 +47,7 @@ static int hif_generic_confirm(struct wfx_dev *wdev,
 	}
 	wdev->hif_cmd.ret = status;
 
-	if (!wdev->hif_cmd.async) {
-		complete(&wdev->hif_cmd.done);
-	} else {
-		wdev->hif_cmd.buf_send = NULL;
-		mutex_unlock(&wdev->hif_cmd.lock);
-	}
+	complete(&wdev->hif_cmd.done);
 	return status;
 }
 
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index a75c6b9804ba..1bd7f773209c 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -47,7 +47,7 @@ static void *wfx_alloc_hif(size_t body_len, struct hif_msg **hif)
 }
 
 int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
-		 void *reply, size_t reply_len, bool async)
+		 void *reply, size_t reply_len, bool no_reply)
 {
 	const char *mib_name = "";
 	const char *mib_sep = "";
@@ -55,8 +55,6 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
 	int vif = request->interface;
 	int ret;
 
-	WARN(wdev->hif_cmd.buf_recv && wdev->hif_cmd.async, "API usage error");
-
 	// Do not wait for any reply if chip is frozen
 	if (wdev->chip_frozen)
 		return -ETIMEDOUT;
@@ -69,14 +67,18 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
 	wdev->hif_cmd.buf_send = request;
 	wdev->hif_cmd.buf_recv = reply;
 	wdev->hif_cmd.len_recv = reply_len;
-	wdev->hif_cmd.async = async;
 	complete(&wdev->hif_cmd.ready);
 
 	wfx_bh_request_tx(wdev);
 
-	// NOTE: no timeout is caught async is enabled
-	if (async)
+	if (no_reply) {
+		// Chip won't reply. Give enough time to the wq to send the
+		// buffer.
+		msleep(100);
+		wdev->hif_cmd.buf_send = NULL;
+		mutex_unlock(&wdev->hif_cmd.lock);
 		return 0;
+	}
 
 	if (wdev->poll_irq)
 		wfx_bh_poll_irq(wdev);
@@ -118,29 +120,21 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
 }
 
 // This function is special. After HIF_REQ_ID_SHUT_DOWN, chip won't reply to any
-// request anymore. We need to slightly hack struct wfx_hif_cmd for that job. Be
-// careful to only call this function during device unregister.
+// request anymore. Obviously, only call this function during device unregister.
 int hif_shutdown(struct wfx_dev *wdev)
 {
 	int ret;
 	struct hif_msg *hif;
 
-	if (wdev->chip_frozen)
-		return 0;
 	wfx_alloc_hif(0, &hif);
 	if (!hif)
 		return -ENOMEM;
 	wfx_fill_header(hif, -1, HIF_REQ_ID_SHUT_DOWN, 0);
 	ret = wfx_cmd_send(wdev, hif, NULL, 0, true);
-	// After this command, chip won't reply. Be sure to give enough time to
-	// bh to send buffer:
-	msleep(100);
-	wdev->hif_cmd.buf_send = NULL;
 	if (wdev->pdata.gpio_wakeup)
 		gpiod_set_value(wdev->pdata.gpio_wakeup, 0);
 	else
 		control_reg_write(wdev, 0);
-	mutex_unlock(&wdev->hif_cmd.lock);
 	kfree(hif);
 	return ret;
 }
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index e8aca39e7497..960d5f2fa41c 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -22,7 +22,6 @@ struct wfx_hif_cmd {
 	struct mutex      lock;
 	struct completion ready;
 	struct completion done;
-	bool              async;
 	struct hif_msg    *buf_send;
 	void              *buf_recv;
 	size_t            len_recv;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 31/31] staging: wfx: update TODO list
  2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
                   ` (29 preceding siblings ...)
  2020-09-07 10:15 ` [PATCH 30/31] staging: wfx: drop async field from struct hif_cmd Jerome Pouiller
@ 2020-09-07 10:15 ` Jerome Pouiller
  30 siblings, 0 replies; 32+ messages in thread
From: Jerome Pouiller @ 2020-09-07 10:15 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

The driver is now close to leave the staging directory. Update the TODO
list to reflect the work done.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/TODO | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/staging/wfx/TODO b/drivers/staging/wfx/TODO
index 42bf36d43970..1b4bc2af94b6 100644
--- a/drivers/staging/wfx/TODO
+++ b/drivers/staging/wfx/TODO
@@ -1,25 +1,6 @@
 This is a list of things that need to be done to get this driver out of the
 staging directory.
 
-  - The HIF API is not yet clean enough.
-
-  - The code that check the corectness of received message (in rx_helper()) can
-    be improved. See:
-       https://lore.kernel.org/driverdev-devel/2302785.6C7ODC2LYm@pc-42/
-
   - As suggested by Felix, rate control could be improved following this idea:
         https://lore.kernel.org/lkml/3099559.gv3Q75KnN1@pc-42/
 
-  - Feature called "secure link" should be either developed (using kernel
-    crypto API) or dropped.
-
-  - The device allows to filter multicast traffic. The code to support these
-    filters exists in the driver but it is disabled because it has never been
-    tested.
-
-  - In wfx_cmd_send(), "async" allow to send command without waiting the reply.
-    It may help in some situation, but it is not yet used. In add, it may cause
-    some trouble:
-      https://lore.kernel.org/driverdev-devel/alpine.DEB.2.21.1910041317381.2992@hadrien/
-    So, fix it (by replacing the mutex with a semaphore) or drop it.
-
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2020-09-07 10:36 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-07 10:14 [PATCH 00/31] staging: wfx: fix last items of the TODO list Jerome Pouiller
2020-09-07 10:14 ` [PATCH 01/31] staging: wfx: improve readability of association processing Jerome Pouiller
2020-09-07 10:14 ` [PATCH 02/31] staging: wfx: relocate wfx_join() beside wfx_join_finalize() Jerome Pouiller
2020-09-07 10:14 ` [PATCH 03/31] staging: wfx: simplify hif_set_association_mode() Jerome Pouiller
2020-09-07 10:14 ` [PATCH 04/31] staging: wfx: keep API error list up-to-date Jerome Pouiller
2020-09-07 10:14 ` [PATCH 05/31] staging: wfx: drop 'secure link' feature Jerome Pouiller
2020-09-07 10:14 ` [PATCH 06/31] staging: wfx: drop multicast filtering Jerome Pouiller
2020-09-07 10:14 ` [PATCH 07/31] staging: wfx: drop useless function Jerome Pouiller
2020-09-07 10:14 ` [PATCH 08/31] staging: wfx: drop useless enum hif_beacon Jerome Pouiller
2020-09-07 10:14 ` [PATCH 09/31] staging: wfx: drop useless union hif_commands_ids Jerome Pouiller
2020-09-07 10:15 ` [PATCH 10/31] staging: wfx: drop useless struct hif_reset_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 11/31] staging: wfx: drop useless struct hif_ie_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 12/31] staging: wfx: drop useless struct hif_join_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 13/31] staging: wfx: drop useless struct hif_bss_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 14/31] staging: wfx: drop useless struct hif_map_link_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 15/31] staging: wfx: drop useless struct hif_suspend_resume_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 16/31] staging: wfx: drop useless struct hif_pm_mode Jerome Pouiller
2020-09-07 10:15 ` [PATCH 17/31] staging: wfx: drop useless struct hif_rx_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 18/31] staging: wfx: drop useless struct hif_tx_result_flags Jerome Pouiller
2020-09-07 10:15 ` [PATCH 19/31] staging: wfx: drop useless structs only used in hif_req_tx Jerome Pouiller
2020-09-07 10:15 ` [PATCH 20/31] staging: wfx: drop useless stricts only used in hif_req_start_scan_alt Jerome Pouiller
2020-09-07 10:15 ` [PATCH 21/31] staging: wfx: drop useless structs only used in hif_ind_startup Jerome Pouiller
2020-09-07 10:15 ` [PATCH 22/31] staging: wfx: drop useless union hif_privacy_key_data Jerome Pouiller
2020-09-07 10:15 ` [PATCH 23/31] staging: wfx: drop useless union hif_event_data Jerome Pouiller
2020-09-07 10:15 ` [PATCH 24/31] staging: wfx: drop useless union hif_indication_data Jerome Pouiller
2020-09-07 10:15 ` [PATCH 25/31] staging: wfx: drop struct hif_ie_tlv Jerome Pouiller
2020-09-07 10:15 ` [PATCH 26/31] staging: wfx: drop macro API_SSID_SIZE Jerome Pouiller
2020-09-07 10:15 ` [PATCH 27/31] staging: wfx: fix naming of hif_tx_rate_retry_policy Jerome Pouiller
2020-09-07 10:15 ` [PATCH 28/31] staging: wfx: fix spaces Jerome Pouiller
2020-09-07 10:15 ` [PATCH 29/31] staging: wfx: uniformize naming rules in hif_tx_mib.c Jerome Pouiller
2020-09-07 10:15 ` [PATCH 30/31] staging: wfx: drop async field from struct hif_cmd Jerome Pouiller
2020-09-07 10:15 ` [PATCH 31/31] staging: wfx: update TODO list Jerome Pouiller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).