[PATCH v2 01/12] staging: wfx: fix BA when device is AP and MFP is enabled

[PATCH v2 01/12] staging: wfx: fix BA when device is AP and MFP is enabled

[Jerome Pouiller]

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

The protection of the management frames is mainly done by mac80211.
However, frames for the management of the BlockAck sessions are directly
sent by the device. These frames have to be protected if MFP is in use.
So the driver has to pass the MFP configuration to the device.

Until now, the BlockAck management frames were completely unprotected
whatever the status of the MFP negotiation. So, some devices dropped
these frames.

The device has two knobs to control the MFP. One global and one per
station. Normally, the driver should always enable global MFP. Then it
should enable MFP on every station with which MFP was successfully
negotiated. Unfortunately, the older firmwares only provide the
global control.

So, this patch enable global MFP as it is exposed in the beacon. Then it
marks every station with which the MFP is effective.

Thus, the support for the old firmwares is not so bad. It may only
encounter some difficulties to negotiate BA sessions when the local
device (the AP) is MFP capable (ieee80211w=1) but the station is not.
The only solution for this case is to upgrade the firmware.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
v2:
  - Check that ptr does not point out of bounds of skb.

 drivers/staging/wfx/sta.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index ad63332f690c..c31e302d05c9 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -434,7 +434,7 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	wvif->link_id_map |= BIT(sta_priv->link_id);
 	WARN_ON(!sta_priv->link_id);
 	WARN_ON(sta_priv->link_id >= HIF_LINK_ID_MAX);
-	hif_map_link(wvif, sta->addr, 0, sta_priv->link_id);
+	hif_map_link(wvif, sta->addr, sta->mfp ? 2 : 0, sta_priv->link_id);
 
 	return 0;
 }
@@ -474,6 +474,31 @@ static int wfx_upload_ap_templates(struct wfx_vif *wvif)
 	return 0;
 }
 
+static void wfx_set_mfp_ap(struct wfx_vif *wvif)
+{
+	struct sk_buff *skb = ieee80211_beacon_get(wvif->wdev->hw, wvif->vif);
+	const int ieoffset = offsetof(struct ieee80211_mgmt, u.beacon.variable);
+	const u16 *ptr = (u16 *)cfg80211_find_ie(WLAN_EID_RSN,
+						 skb->data + ieoffset,
+						 skb->len - ieoffset);
+	const int pairwise_cipher_suite_count_offset = 8 / sizeof(u16);
+	const int pairwise_cipher_suite_size = 4 / sizeof(u16);
+	const int akm_suite_size = 4 / sizeof(u16);
+
+	if (ptr) {
+		ptr += pairwise_cipher_suite_count_offset;
+		if (WARN_ON(ptr > (u16 *)skb_tail_pointer(skb)))
+			return;
+		ptr += 1 + pairwise_cipher_suite_size * *ptr;
+		if (WARN_ON(ptr > (u16 *)skb_tail_pointer(skb)))
+			return;
+		ptr += 1 + akm_suite_size * *ptr;
+		if (WARN_ON(ptr > (u16 *)skb_tail_pointer(skb)))
+			return;
+		hif_set_mfp(wvif, *ptr & BIT(7), *ptr & BIT(6));
+	}
+}
+
 int wfx_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 {
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
@@ -488,6 +513,7 @@ int wfx_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	ret = hif_start(wvif, &vif->bss_conf, wvif->channel);
 	if (ret > 0)
 		return -EIO;
+	wfx_set_mfp_ap(wvif);
 	return ret;
 }
 
-- 
2.28.0

[PATCH v2 02/12] staging: wfx: improve usage of hif_map_link()

[Jerome Pouiller]

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

Until now, hif_map_link() get as argument the raw value for
map_link_flags when map_link_flags is defined as a bitfield. It was
error prone.

Now hif_map_link() takes explicit value for every flags of the
struct map_link_flags.

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

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 3b5f4dcc469c..6b89e55f03f4 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -499,7 +499,7 @@ int hif_beacon_transmit(struct wfx_vif *wvif, bool enable)
 	return ret;
 }
 
-int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id)
+int hif_map_link(struct wfx_vif *wvif, bool unmap, u8 *mac_addr, int sta_id, bool mfp)
 {
 	int ret;
 	struct hif_msg *hif;
@@ -509,7 +509,8 @@ int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id)
 		return -ENOMEM;
 	if (mac_addr)
 		ether_addr_copy(body->mac_addr, mac_addr);
-	body->map_link_flags = *(struct hif_map_link_flags *)&flags;
+	body->map_link_flags.mfpc = mfp ? 1 : 0;
+	body->map_link_flags.map_direction = 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);
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index e1da28aef706..b371b3777a31 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -55,7 +55,8 @@ int hif_set_edca_queue_params(struct wfx_vif *wvif, u16 queue,
 int hif_start(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 	      const struct ieee80211_channel *channel);
 int hif_beacon_transmit(struct wfx_vif *wvif, bool enable);
-int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id);
+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);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index c31e302d05c9..b6ccb04f308a 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -434,7 +434,7 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	wvif->link_id_map |= BIT(sta_priv->link_id);
 	WARN_ON(!sta_priv->link_id);
 	WARN_ON(sta_priv->link_id >= HIF_LINK_ID_MAX);
-	hif_map_link(wvif, sta->addr, sta->mfp ? 2 : 0, sta_priv->link_id);
+	hif_map_link(wvif, false, sta->addr, sta_priv->link_id, sta->mfp);
 
 	return 0;
 }
@@ -449,7 +449,7 @@ int wfx_sta_remove(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (!sta_priv->link_id)
 		return 0;
 	// FIXME add a mutex?
-	hif_map_link(wvif, sta->addr, 1, sta_priv->link_id);
+	hif_map_link(wvif, true, sta->addr, sta_priv->link_id, false);
 	wvif->link_id_map &= ~BIT(sta_priv->link_id);
 	return 0;
 }
-- 
2.28.0

[PATCH v2 03/12] staging: wfx: fix BA when MFP is disabled but BSS is MFP capable

[Jerome Pouiller]

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

The protection of the management frames is mainly done by mac80211.
However, frames for the management of the BlockAck sessions are directly
sent by the device. These frames have to be protected if MFP is in use.
So the driver has to pass the MFP configuration to the device.

Until now, the driver directly read the RSN IE of the BSS. However, it
didn't work when the BSS was MFP capable (ieee80211w=1) and the local
device has disabled MFP (ieee80211w=0).

This patch read the MFP information directly from the struct
ieee80211_sta. This information take into account the MFP negotiated
during the association. In addition, the code is far simpler.

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

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index b6ccb04f308a..2b848b8985df 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -323,36 +323,6 @@ void wfx_set_default_unicast_key(struct ieee80211_hw *hw,
 	hif_wep_default_key_id(wvif, idx);
 }
 
-static void wfx_set_mfp(struct wfx_vif *wvif,
-			struct cfg80211_bss *bss)
-{
-	const int pairwise_cipher_suite_count_offset = 8 / sizeof(u16);
-	const int pairwise_cipher_suite_size = 4 / sizeof(u16);
-	const int akm_suite_size = 4 / sizeof(u16);
-	const u16 *ptr = NULL;
-	bool mfpc = false;
-	bool mfpr = false;
-
-	/* 802.11w protected mgmt frames */
-
-	/* retrieve MFPC and MFPR flags from beacon or PBRSP */
-
-	rcu_read_lock();
-	if (bss)
-		ptr = (const u16 *)ieee80211_bss_get_ie(bss, WLAN_EID_RSN);
-
-	if (ptr) {
-		ptr += pairwise_cipher_suite_count_offset;
-		ptr += 1 + pairwise_cipher_suite_size * *ptr;
-		ptr += 1 + akm_suite_size * *ptr;
-		mfpr = *ptr & BIT(6);
-		mfpc = *ptr & BIT(7);
-	}
-	rcu_read_unlock();
-
-	hif_set_mfp(wvif, mfpc, mfpr);
-}
-
 void wfx_reset(struct wfx_vif *wvif)
 {
 	struct wfx_dev *wdev = wvif->wdev;
@@ -400,7 +370,6 @@ static void wfx_do_join(struct wfx_vif *wvif)
 	}
 	rcu_read_unlock();
 
-	wfx_set_mfp(wvif, bss);
 	cfg80211_put_bss(wvif->wdev->hw->wiphy, bss);
 
 	wvif->join_in_progress = true;
@@ -427,6 +396,9 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	sta_priv->vif_id = wvif->id;
 
+	if (vif->type == NL80211_IFTYPE_STATION)
+		hif_set_mfp(wvif, sta->mfp, sta->mfp);
+
 	// In station mode, the firmware interprets new link-id as a TDLS peer.
 	if (vif->type == NL80211_IFTYPE_STATION && !sta->tdls)
 		return 0;
-- 
2.28.0

[PATCH v2 04/12] staging: wfx: fix spaces around binary operators

[Jerome Pouiller]

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

A binary operator should be followed by exactly one space.

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

diff --git a/drivers/staging/wfx/key.c b/drivers/staging/wfx/key.c
index 5ee2ffc5f935..6165df59ecf9 100644
--- a/drivers/staging/wfx/key.c
+++ b/drivers/staging/wfx/key.c
@@ -171,7 +171,7 @@ static int wfx_add_key(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	k.int_id = wvif->id;
 	k.entry_index = idx;
 	if (key->cipher == WLAN_CIPHER_SUITE_WEP40 ||
-	    key->cipher ==  WLAN_CIPHER_SUITE_WEP104) {
+	    key->cipher == WLAN_CIPHER_SUITE_WEP104) {
 		if (pairwise)
 			k.type = fill_wep_pair(&k.key.wep_pairwise_key, key,
 					       sta->addr);
@@ -191,13 +191,13 @@ static int wfx_add_key(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 		else
 			k.type = fill_ccmp_group(&k.key.aes_group_key, key,
 						 &seq);
-	} else if (key->cipher ==  WLAN_CIPHER_SUITE_SMS4) {
+	} else if (key->cipher == WLAN_CIPHER_SUITE_SMS4) {
 		if (pairwise)
 			k.type = fill_sms4_pair(&k.key.wapi_pairwise_key, key,
 						sta->addr);
 		else
 			k.type = fill_sms4_group(&k.key.wapi_group_key, key);
-	} else if (key->cipher ==  WLAN_CIPHER_SUITE_AES_CMAC) {
+	} else if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC) {
 		k.type = fill_aes_cmac_group(&k.key.igtk_group_key, key,
 					     &seq);
 	} else {
-- 
2.28.0

[PATCH v2 05/12] staging: wfx: fix support for cipher AES_CMAC (multicast PMF)

[Jerome Pouiller]

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

When MFP is enabled, the multicast management frames are not protected,
in fact. Instead, but they should include an IE containing the MMIC of
the frames (i.e. a cryptographic signature).

Until now, the driver didn't correctly detect this kind of frames (they
are not marked protected but they are associated to a key) and didn't
ask to the device to encrypt them.

In add, the device is not able to generate the IE itself. Mac80211 has
to generate the IE and let the device compute the MMIC.

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 41f9afd41e14..d16b516ad7cf 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -325,6 +325,8 @@ static int wfx_tx_get_icv_len(struct ieee80211_key_conf *hw_key)
 
 	if (!hw_key)
 		return 0;
+	if (hw_key->cipher == WLAN_CIPHER_SUITE_AES_CMAC)
+		return 0;
 	mic_space = (hw_key->cipher == WLAN_CIPHER_SUITE_TKIP) ? 8 : 0;
 	return hw_key->icv_len + mic_space;
 }
@@ -350,8 +352,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	memset(tx_info->rate_driver_data, 0, sizeof(struct wfx_tx_priv));
 	// Fill tx_priv
 	tx_priv = (struct wfx_tx_priv *)tx_info->rate_driver_data;
-	if (ieee80211_has_protected(hdr->frame_control))
-		tx_priv->hw_key = hw_key;
+	tx_priv->hw_key = hw_key;
 
 	// Fill hif_msg
 	WARN(skb_headroom(skb) < wmsg_len, "not enough space in skb");
diff --git a/drivers/staging/wfx/key.c b/drivers/staging/wfx/key.c
index 6165df59ecf9..728e5f8d3b7c 100644
--- a/drivers/staging/wfx/key.c
+++ b/drivers/staging/wfx/key.c
@@ -198,8 +198,8 @@ static int wfx_add_key(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 		else
 			k.type = fill_sms4_group(&k.key.wapi_group_key, key);
 	} else if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC) {
-		k.type = fill_aes_cmac_group(&k.key.igtk_group_key, key,
-					     &seq);
+		k.type = fill_aes_cmac_group(&k.key.igtk_group_key, key, &seq);
+		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIE;
 	} else {
 		dev_warn(wdev->dev, "unsupported key type %d\n", key->cipher);
 		wfx_free_key(wdev, idx);
-- 
2.28.0

[PATCH v2 06/12] staging: wfx: drop useless field from struct wfx_tx_priv

[Jerome Pouiller]

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

The device need to receive a skb with necessary space for the ICV. So,
the driver adds this space before to send the frame.

Currently, once the frame is sent, the driver restore the original
content of the skb. However, this step is useless. Mac80211 don't do it
when software encryption is enabled.

Once we have removed this step, it appears that it is no more necessary
to keep hw_key in tx_priv. Then, it is possible to simplify a bunch of
code in the Tx path.

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index d16b516ad7cf..485907b0faa2 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -336,7 +336,6 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 {
 	struct hif_msg *hif_msg;
 	struct hif_req_tx *req;
-	struct wfx_tx_priv *tx_priv;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_key_conf *hw_key = tx_info->control.hw_key;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
@@ -350,14 +349,11 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 
 	// From now tx_info->control is unusable
 	memset(tx_info->rate_driver_data, 0, sizeof(struct wfx_tx_priv));
-	// Fill tx_priv
-	tx_priv = (struct wfx_tx_priv *)tx_info->rate_driver_data;
-	tx_priv->hw_key = hw_key;
 
 	// Fill hif_msg
 	WARN(skb_headroom(skb) < wmsg_len, "not enough space in skb");
 	WARN(offset & 1, "attempt to transmit an unaligned frame");
-	skb_put(skb, wfx_tx_get_icv_len(tx_priv->hw_key));
+	skb_put(skb, wfx_tx_get_icv_len(hw_key));
 	skb_push(skb, wmsg_len);
 	memset(skb->data, 0, wmsg_len);
 	hif_msg = (struct hif_msg *)skb->data;
@@ -489,7 +485,6 @@ static void wfx_tx_fill_rates(struct wfx_dev *wdev,
 void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg)
 {
 	struct ieee80211_tx_info *tx_info;
-	const struct wfx_tx_priv *tx_priv;
 	struct wfx_vif *wvif;
 	struct sk_buff *skb;
 
@@ -499,18 +494,15 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg)
 			 arg->packet_id);
 		return;
 	}
+	tx_info = IEEE80211_SKB_CB(skb);
 	wvif = wdev_to_wvif(wdev, ((struct hif_msg *)skb->data)->interface);
 	WARN_ON(!wvif);
 	if (!wvif)
 		return;
-	tx_info = IEEE80211_SKB_CB(skb);
-	tx_priv = wfx_skb_tx_priv(skb);
+
+	// Note that wfx_pending_get_pkt_us_delay() get data from tx_info
 	_trace_tx_stats(arg, skb, wfx_pending_get_pkt_us_delay(wdev, skb));
-
-	// You can touch to tx_priv, but don't touch to tx_info->status.
 	wfx_tx_fill_rates(wdev, tx_info, arg);
-	skb_trim(skb, skb->len - wfx_tx_get_icv_len(tx_priv->hw_key));
-
 	// From now, you can touch to tx_info->status, but do not touch to
 	// tx_priv anymore
 	// FIXME: use ieee80211_tx_info_clear_status()
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index cff7b9ff99a9..87e1b9b62dbb 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -35,8 +35,7 @@ struct tx_policy_cache {
 
 struct wfx_tx_priv {
 	ktime_t xmit_timestamp;
-	struct ieee80211_key_conf *hw_key;
-} __packed;
+};
 
 void wfx_tx_policy_init(struct wfx_vif *wvif);
 void wfx_tx_policy_upload_work(struct work_struct *work);
-- 
2.28.0

[PATCH v2 07/12] staging: wfx: fix frame reordering

[Jerome Pouiller]

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

When mac80211 debug is enabled, the trace below appears:

    [60744.340037] wlan0: Rx A-MPDU request on aa:bb:cc:97:60:24 tid 0 result -524

This imply that ___ieee80211_start_rx_ba_session will prematurely exit
and frame reordering won't be enabled.

Fixes: e5da5fbd77411 ("staging: wfx: fix CCMP/TKIP replay protection")
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 2b848b8985df..7a4c9f63c4a2 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -680,15 +680,16 @@ int wfx_ampdu_action(struct ieee80211_hw *hw,
 		     struct ieee80211_vif *vif,
 		     struct ieee80211_ampdu_params *params)
 {
-	/* Aggregation is implemented fully in firmware,
-	 * including block ack negotiation. Do not allow
-	 * mac80211 stack to do anything: it interferes with
-	 * the firmware.
-	 */
-
-	/* Note that we still need this function stubbed. */
-
-	return -ENOTSUPP;
+	// Aggregation is implemented fully in firmware
+	switch (params->action) {
+	case IEEE80211_AMPDU_RX_START:
+	case IEEE80211_AMPDU_RX_STOP:
+		// Just acknowledge it to enable frame re-ordering
+		return 0;
+	default:
+		// Leave the firmware doing its business for tx aggregation
+		return -ENOTSUPP;
+	}
 }
 
 int wfx_add_chanctx(struct ieee80211_hw *hw,
-- 
2.28.0

[PATCH v2 08/12] staging: wfx: fix potential use before init

[Jerome Pouiller]

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

The trace below can appear:

    [83613.832200] INFO: trying to register non-static key.
    [83613.837248] the code is fine but needs lockdep annotation.
    [83613.842808] turning off the locking correctness validator.
    [83613.848375] CPU: 3 PID: 141 Comm: kworker/3:2H Tainted: G           O      5.6.13-silabs15 #2
    [83613.857019] Hardware name: BCM2835
    [83613.860605] Workqueue: events_highpri bh_work [wfx]
    [83613.865552] Backtrace:
    [83613.868041] [<c010f2cc>] (dump_backtrace) from [<c010f7b8>] (show_stack+0x20/0x24)
    [83613.881463] [<c010f798>] (show_stack) from [<c0d82138>] (dump_stack+0xe8/0x114)
    [83613.888882] [<c0d82050>] (dump_stack) from [<c01a02ec>] (register_lock_class+0x748/0x768)
    [83613.905035] [<c019fba4>] (register_lock_class) from [<c019da04>] (__lock_acquire+0x88/0x13dc)
    [83613.924192] [<c019d97c>] (__lock_acquire) from [<c019f6a4>] (lock_acquire+0xe8/0x274)
    [83613.942644] [<c019f5bc>] (lock_acquire) from [<c0daa5dc>] (_raw_spin_lock_irqsave+0x58/0x6c)
    [83613.961714] [<c0daa584>] (_raw_spin_lock_irqsave) from [<c0ab3248>] (skb_dequeue+0x24/0x78)
    [83613.974967] [<c0ab3224>] (skb_dequeue) from [<bf330db0>] (wfx_tx_queues_get+0x96c/0x1294 [wfx])
    [83613.989728] [<bf330444>] (wfx_tx_queues_get [wfx]) from [<bf320454>] (bh_work+0x454/0x26d8 [wfx])
    [83614.009337] [<bf320000>] (bh_work [wfx]) from [<c014c920>] (process_one_work+0x23c/0x7ec)
    [83614.028141] [<c014c6e4>] (process_one_work) from [<c014cf1c>] (worker_thread+0x4c/0x55c)
    [83614.046861] [<c014ced0>] (worker_thread) from [<c0154c04>] (kthread+0x138/0x168)
    [83614.064876] [<c0154acc>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)
    [83614.072200] Exception stack(0xecad3fb0 to 0xecad3ff8)
    [83614.077323] 3fa0:                                     00000000 00000000 00000000 00000000
    [83614.085620] 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
    [83614.093914] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000

Indeed, the code of wfx_add_interface() shows that the interface is
enabled to early. So, the spinlock associated with some skb_queue may
not yet initialized when wfx_tx_queues_get() is called.

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

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 7a4c9f63c4a2..a890fe32161c 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -759,17 +759,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 		return -EOPNOTSUPP;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(wdev->vif); i++) {
-		if (!wdev->vif[i]) {
-			wdev->vif[i] = vif;
-			wvif->id = i;
-			break;
-		}
-	}
-	if (i == ARRAY_SIZE(wdev->vif)) {
-		mutex_unlock(&wdev->conf_mutex);
-		return -EOPNOTSUPP;
-	}
 	// FIXME: prefer use of container_of() to get vif
 	wvif->vif = vif;
 	wvif->wdev = wdev;
@@ -786,12 +775,22 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	init_completion(&wvif->scan_complete);
 	INIT_WORK(&wvif->scan_work, wfx_hw_scan_work);
 
-	mutex_unlock(&wdev->conf_mutex);
-
-	hif_set_macaddr(wvif, vif->addr);
-
 	wfx_tx_queues_init(wvif);
 	wfx_tx_policy_init(wvif);
+
+	for (i = 0; i < ARRAY_SIZE(wdev->vif); i++) {
+		if (!wdev->vif[i]) {
+			wdev->vif[i] = vif;
+			wvif->id = i;
+			break;
+		}
+	}
+	WARN(i == ARRAY_SIZE(wdev->vif), "try to instantiate more vif than supported");
+
+	hif_set_macaddr(wvif, vif->addr);
+
+	mutex_unlock(&wdev->conf_mutex);
+
 	wvif = NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
 		// Combo mode does not support Block Acks. We can re-enable them
@@ -823,6 +822,7 @@ void wfx_remove_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	wvif->vif = NULL;
 
 	mutex_unlock(&wdev->conf_mutex);
+
 	wvif = NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
 		// Combo mode does not support Block Acks. We can re-enable them
-- 
2.28.0

[PATCH v2 09/12] staging: wfx: scan while AP is supported

[Jerome Pouiller]

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

The device is able to scan while running an Access Point. Just declare
it.

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

diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 4263f912760b..5a3018e14445 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -282,6 +282,7 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 					NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WPS2 |
 					NL80211_PROBE_RESP_OFFLOAD_SUPPORT_P2P |
 					NL80211_PROBE_RESP_OFFLOAD_SUPPORT_80211U;
+	hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
 	hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
diff --git a/drivers/staging/wfx/scan.c b/drivers/staging/wfx/scan.c
index e9de19784865..02d4e653d594 100644
--- a/drivers/staging/wfx/scan.c
+++ b/drivers/staging/wfx/scan.c
@@ -113,10 +113,6 @@ int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 
 	WARN_ON(hw_req->req.n_channels > HIF_API_MAX_NB_CHANNELS);
-
-	if (vif->type == NL80211_IFTYPE_AP)
-		return -EOPNOTSUPP;
-
 	wvif->scan_req = hw_req;
 	schedule_work(&wvif->scan_work);
 	return 0;
-- 
2.28.0

[PATCH v2 10/12] staging: wfx: enable powersave on probe

[Jerome Pouiller]

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

In the old days, ieee80211 powersave has some impact on the Rx speed.
These problems are solved for a long time now. There is no more reason
to not enabling it.

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

diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 5a3018e14445..5e2b82499004 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -285,7 +285,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 	hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
-	hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
 	hw->wiphy->max_ap_assoc_sta = HIF_LINK_ID_MAX;
 	hw->wiphy->max_scan_ssids = 2;
 	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
-- 
2.28.0

[PATCH v2 11/12] staging: wfx: remove useless extra jiffy

[Jerome Pouiller]

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

The initial developer has feared msecs_to_jiffies() could round down the
result. However, the documentation of msecs_to_jiffies() says that the
result is rounded upward. So the increment of the result of
msecs_to_jiffies() is not necessary.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/bh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 53ae0b5abcdd..07304a80c29b 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -33,7 +33,7 @@ static void device_wakeup(struct wfx_dev *wdev)
 		// wait_for_completion_done_timeout()). So we have to emulate
 		// it.
 		if (wait_for_completion_timeout(&wdev->hif.ctrl_ready,
-						msecs_to_jiffies(2) + 1))
+						msecs_to_jiffies(2)))
 			complete(&wdev->hif.ctrl_ready);
 		else
 			dev_err(wdev->dev, "timeout while wake up chip\n");
-- 
2.28.0

[PATCH v2 12/12] staging: wfx: add workaround for 'timeout while wake up chip'

[Jerome Pouiller]

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

The host and the device can be connected with a called Wake-Up GPIO.
When the host fall down this GPIO, it allows the device to enter in deep
sleep and no communication with the device is no more possible (the
device wakes up automatically on DTIM and fetch data if necessary).

So, before to communicate with the device, the driver have to raise the
Wake-up GPIO and then wait for an IRQ from the device.

Unfortunately, old firmwares have a race in sleep/wake-up process and
the device may never wake up. In this case, the IRQ is not sent and
driver complains with "timeout while wake up chip". Then, the driver
tries anyway to access the bus and an other error is raised by the bus.

Fortunately, when the bug occurs, it is possible to fall down the IRQ
and the device will eventually finish the sleep process. Then the driver
can wake it up normally.

The patch implements that workaround and add a retry limit in case
something goes very wrong.

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

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 07304a80c29b..f07bcee50e3f 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -18,25 +18,40 @@
 
 static void device_wakeup(struct wfx_dev *wdev)
 {
+	int max_retry = 3;
+
 	if (!wdev->pdata.gpio_wakeup)
 		return;
 	if (gpiod_get_value_cansleep(wdev->pdata.gpio_wakeup))
 		return;
 
-	gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 1);
 	if (wfx_api_older_than(wdev, 1, 4)) {
+		gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 1);
 		if (!completion_done(&wdev->hif.ctrl_ready))
 			usleep_range(2000, 2500);
-	} else {
+		return;
+	}
+	for (;;) {
+		gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 1);
 		// completion.h does not provide any function to wait
 		// completion without consume it (a kind of
 		// wait_for_completion_done_timeout()). So we have to emulate
 		// it.
 		if (wait_for_completion_timeout(&wdev->hif.ctrl_ready,
-						msecs_to_jiffies(2)))
+						msecs_to_jiffies(2))) {
 			complete(&wdev->hif.ctrl_ready);
-		else
+			return;
+		} else if (max_retry-- > 0) {
+			// Older firmwares have a race in sleep/wake-up process.
+			// Redo the process is sufficient to unfreeze the
+			// chip.
 			dev_err(wdev->dev, "timeout while wake up chip\n");
+			gpiod_set_value_cansleep(wdev->pdata.gpio_wakeup, 0);
+			usleep_range(2000, 2500);
+		} else {
+			dev_err(wdev->dev, "max wake-up retries reached\n");
+			return;
+		}
 	}
 }
 
-- 
2.28.0