[PATCH 00/16] wil6210 patches

[PATCH 00/16] wil6210 patches

[Maya Erez]

The following set of patches include various wil6210 fixes.

Ahmad Masri (5):
  wil6210: refactor disconnect flow
  wil6210: notify cqm packet loss on disable_ap_sme
  wil6210: fix debugfs memory access alignment
  wil6210: remove unnecessary alignment code from rx flow
  wil6210: fix freeing of rx buffers in EDMA mode

Alexei Avshalom Lazar (2):
  wil6210: fix reset flow for Talyn-mb
  wil6210: add general initialization/size checks

Dedy Lansky (2):
  wil6210: make sure Rx ring sizes are correlated
  wil6210: add recovery for FW error while in AP mode

Hamad Kadmany (1):
  wil6210: remove fake support for RXHASH

Lior David (2):
  wil6210: fix memory leak in wil_find_tx_bcast_2
  wil6210: fix locking in wmi_call

Maya Erez (4):
  wil6210: increase RX rings and RX buff array size
  wil6210: fix L2 RX status handling
  wil6210: fix RGF_CAF_ICR address for Talyn-MB
  wil6210: ignore HALP ICR if already handled

 drivers/net/wireless/ath/wil6210/cfg80211.c  | 104 +++++++++++-
 drivers/net/wireless/ath/wil6210/debugfs.c   |  17 +-
 drivers/net/wireless/ath/wil6210/interrupt.c |  10 +-
 drivers/net/wireless/ath/wil6210/main.c      | 230 ++++++++++++++++++++-------
 drivers/net/wireless/ath/wil6210/netdev.c    |   5 +-
 drivers/net/wireless/ath/wil6210/txrx.c      |  14 +-
 drivers/net/wireless/ath/wil6210/txrx_edma.c |  92 +++++------
 drivers/net/wireless/ath/wil6210/txrx_edma.h |   4 +-
 drivers/net/wireless/ath/wil6210/wil6210.h   |  22 ++-
 drivers/net/wireless/ath/wil6210/wmi.c       |  56 ++++---
 10 files changed, 386 insertions(+), 168 deletions(-)

-- 
1.9.1

[PATCH 01/16] wil6210: remove fake support for RXHASH

[Maya Erez]

From: Hamad Kadmany <hkadmany@codeaurora.org>

Setting the same fake hash to all skbs prevents
distributing different flows to different CPU cores.

Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
Signed-off-by: Lior David <liord@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/netdev.c | 3 +--
 drivers/net/wireless/ath/wil6210/txrx.c   | 8 --------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/netdev.c b/drivers/net/wireless/ath/wil6210/netdev.c
index 7a78a06..64fa1a2 100644
--- a/drivers/net/wireless/ath/wil6210/netdev.c
+++ b/drivers/net/wireless/ath/wil6210/netdev.c
@@ -345,8 +345,7 @@ struct wil6210_vif *
 	ndev->ieee80211_ptr = wdev;
 	ndev->hw_features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
 			    NETIF_F_SG | NETIF_F_GRO |
-			    NETIF_F_TSO | NETIF_F_TSO6 |
-			    NETIF_F_RXHASH;
+			    NETIF_F_TSO | NETIF_F_TSO6;
 
 	ndev->features |= ndev->hw_features;
 	SET_NETDEV_DEV(ndev, wiphy_dev(wdev->wiphy));
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index cc5f263..70eceec 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -743,14 +743,6 @@ void wil_netif_rx_any(struct sk_buff *skb, struct net_device *ndev)
 
 	stats = &wil->sta[cid].stats;
 
-	if (ndev->features & NETIF_F_RXHASH)
-		/* fake L4 to ensure it won't be re-calculated later
-		 * set hash to any non-zero value to activate rps
-		 * mechanism, core will be chosen according
-		 * to user-level rps configuration.
-		 */
-		skb_set_hash(skb, 1, PKT_HASH_TYPE_L4);
-
 	skb_orphan(skb);
 
 	if (security && (wil->txrx_ops.rx_crypto_check(wil, skb) != 0)) {
-- 
1.9.1

Re: [PATCH 01/16] wil6210: remove fake support for RXHASH

[Kalle Valo]

Maya Erez <merez@codeaurora.org> wrote:

> Setting the same fake hash to all skbs prevents
> distributing different flows to different CPU cores.
> 
> Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
> Signed-off-by: Lior David <liord@codeaurora.org>
> Signed-off-by: Maya Erez <merez@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

15 patches applied to ath-next branch of ath.git, thanks.

a078c4cf0197 wil6210: remove fake support for RXHASH
d083b2e2b7db wil6210: fix reset flow for Talyn-mb
cbebe277beb1 wil6210: increase RX rings and RX buff array size
61e5ec044748 wil6210: make sure Rx ring sizes are correlated
e41ab937d47b wil6210: add recovery for FW error while in AP mode
664497400c89 wil6210: fix memory leak in wil_find_tx_bcast_2
e1b43407c034 wil6210: refactor disconnect flow
b571e71bcb98 wil6210: notify cqm packet loss on disable_ap_sme
ac0e541ab2f2 wil6210: add general initialization/size checks
84ec040d0fb2 wil6210: fix debugfs memory access alignment
04de15010aa4 wil6210: fix L2 RX status handling
7c69709f8ed2 wil6210: fix RGF_CAF_ICR address for Talyn-MB
a834df7497b4 wil6210: remove unnecessary alignment code from rx flow
6470f31927b4 wil6210: fix freeing of rx buffers in EDMA mode
dc57731dbd53 wil6210: fix locking in wmi_call

-- 
https://patchwork.kernel.org/patch/10662235/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 02/16] wil6210: fix reset flow for Talyn-mb

[Maya Erez]

From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>

With current reset flow, Talyn sometimes get stuck causing PCIe
enumeration to fail. Fix this by removing some reset flow operations
that are not relevant for Talyn.
Setting bit 15 in RGF_HP_CTRL is WBE specific and is not in use for
all wil6210 devices.
For Sparrow, BIT_HPAL_PERST_FROM_PAD and BIT_CAR_PERST_RST were set
as a WA an HW issue.

Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/main.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 398900a..c54b008 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -998,10 +998,13 @@ static int wil_target_reset(struct wil6210_priv *wil, int no_flash)
 
 	wil_dbg_misc(wil, "Resetting \"%s\"...\n", wil->hw_name);
 
-	/* Clear MAC link up */
-	wil_s(wil, RGF_HP_CTRL, BIT(15));
-	wil_s(wil, RGF_USER_CLKS_CTL_SW_RST_MASK_0, BIT_HPAL_PERST_FROM_PAD);
-	wil_s(wil, RGF_USER_CLKS_CTL_SW_RST_MASK_0, BIT_CAR_PERST_RST);
+	if (wil->hw_version < HW_VER_TALYN) {
+		/* Clear MAC link up */
+		wil_s(wil, RGF_HP_CTRL, BIT(15));
+		wil_s(wil, RGF_USER_CLKS_CTL_SW_RST_MASK_0,
+		      BIT_HPAL_PERST_FROM_PAD);
+		wil_s(wil, RGF_USER_CLKS_CTL_SW_RST_MASK_0, BIT_CAR_PERST_RST);
+	}
 
 	wil_halt_cpu(wil);
 
-- 
1.9.1

[PATCH 03/16] wil6210: increase RX rings and RX buff array size

[Maya Erez]

In Talyn-MB, the 11ad throughput is higher and performance drops
may occur in the current RX configuration due to unavailability
of Rx buffers.
Increase the RX descriptor ring, RX status ring and number of RX
buffers to stabilize the performance in high throughput.

Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/main.c      | 7 ++++++-
 drivers/net/wireless/ath/wil6210/txrx_edma.c | 4 ++--
 drivers/net/wireless/ath/wil6210/txrx_edma.h | 4 ++--
 drivers/net/wireless/ath/wil6210/wil6210.h   | 1 +
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index c54b008..078ad5cf 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -80,7 +80,7 @@ static int mtu_max_set(const char *val, const struct kernel_param *kp)
 module_param_cb(mtu_max, &mtu_max_ops, &mtu_max, 0444);
 MODULE_PARM_DESC(mtu_max, " Max MTU value.");
 
-static uint rx_ring_order = WIL_RX_RING_SIZE_ORDER_DEFAULT;
+static uint rx_ring_order;
 static uint tx_ring_order = WIL_TX_RING_SIZE_ORDER_DEFAULT;
 static uint bcast_ring_order = WIL_BCAST_RING_SIZE_ORDER_DEFAULT;
 
@@ -1684,6 +1684,11 @@ int __wil_up(struct wil6210_priv *wil)
 		return rc;
 
 	/* Rx RING. After MAC and beacon */
+	if (rx_ring_order == 0)
+		rx_ring_order = wil->hw_version < HW_VER_TALYN_MB ?
+			WIL_RX_RING_SIZE_ORDER_DEFAULT :
+			WIL_RX_RING_SIZE_ORDER_TALYN_DEFAULT;
+
 	rc = wil->txrx_ops.rx_init(wil, 1 << rx_ring_order);
 	if (rc)
 		return rc;
diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.c b/drivers/net/wireless/ath/wil6210/txrx_edma.c
index 2bbae75..ce71358 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.c
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.c
@@ -357,8 +357,8 @@ static int wil_init_rx_sring(struct wil6210_priv *wil,
 	struct wil_status_ring *sring = &wil->srings[ring_id];
 	int rc;
 
-	wil_dbg_misc(wil, "init RX sring: size=%u, ring_id=%u\n", sring->size,
-		     ring_id);
+	wil_dbg_misc(wil, "init RX sring: size=%u, ring_id=%u\n",
+		     status_ring_size, ring_id);
 
 	memset(&sring->rx_data, 0, sizeof(sring->rx_data));
 
diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.h b/drivers/net/wireless/ath/wil6210/txrx_edma.h
index a7fe929..343516a 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.h
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.h
@@ -23,9 +23,9 @@
 #define WIL_SRING_SIZE_ORDER_MIN	(WIL_RING_SIZE_ORDER_MIN)
 #define WIL_SRING_SIZE_ORDER_MAX	(WIL_RING_SIZE_ORDER_MAX)
 /* RX sring order should be bigger than RX ring order */
-#define WIL_RX_SRING_SIZE_ORDER_DEFAULT	(11)
+#define WIL_RX_SRING_SIZE_ORDER_DEFAULT	(12)
 #define WIL_TX_SRING_SIZE_ORDER_DEFAULT	(12)
-#define WIL_RX_BUFF_ARR_SIZE_DEFAULT (1536)
+#define WIL_RX_BUFF_ARR_SIZE_DEFAULT (2600)
 
 #define WIL_DEFAULT_RX_STATUS_RING_ID 0
 #define WIL_RX_DESC_RING_ID 0
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index abb8201..414de54 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -81,6 +81,7 @@ static inline u32 WIL_GET_BITS(u32 x, int b0, int b1)
 
 #define WIL_TX_Q_LEN_DEFAULT		(4000)
 #define WIL_RX_RING_SIZE_ORDER_DEFAULT	(10)
+#define WIL_RX_RING_SIZE_ORDER_TALYN_DEFAULT	(11)
 #define WIL_TX_RING_SIZE_ORDER_DEFAULT	(12)
 #define WIL_BCAST_RING_SIZE_ORDER_DEFAULT	(7)
 #define WIL_BCAST_MCS0_LIMIT		(1024) /* limit for MCS0 frame size */
-- 
1.9.1

[PATCH 04/16] wil6210: make sure Rx ring sizes are correlated

[Maya Erez]

From: Dedy Lansky <dlansky@codeaurora.org>

When enlarging rx_ring_order module param, wil6210 fails to load
because there are not enough Rx buffers.
Fix this by enlarging number of Rx buffers at startup, if needed based
on rx_ring_order.

Signed-off-by: Dedy Lansky <dlansky@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/main.c      |  2 +-
 drivers/net/wireless/ath/wil6210/txrx.c      |  4 ++--
 drivers/net/wireless/ath/wil6210/txrx_edma.c | 11 ++++++++---
 drivers/net/wireless/ath/wil6210/wil6210.h   |  2 +-
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 078ad5cf..55084770 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -1689,7 +1689,7 @@ int __wil_up(struct wil6210_priv *wil)
 			WIL_RX_RING_SIZE_ORDER_DEFAULT :
 			WIL_RX_RING_SIZE_ORDER_TALYN_DEFAULT;
 
-	rc = wil->txrx_ops.rx_init(wil, 1 << rx_ring_order);
+	rc = wil->txrx_ops.rx_init(wil, rx_ring_order);
 	if (rc)
 		return rc;
 
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index 70eceec..c592c8e 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -872,7 +872,7 @@ static void wil_rx_buf_len_init(struct wil6210_priv *wil)
 	}
 }
 
-static int wil_rx_init(struct wil6210_priv *wil, u16 size)
+static int wil_rx_init(struct wil6210_priv *wil, uint order)
 {
 	struct wil_ring *vring = &wil->ring_rx;
 	int rc;
@@ -886,7 +886,7 @@ static int wil_rx_init(struct wil6210_priv *wil, u16 size)
 
 	wil_rx_buf_len_init(wil);
 
-	vring->size = size;
+	vring->size = 1 << order;
 	vring->is_rx = true;
 	rc = wil_vring_alloc(wil, vring);
 	if (rc)
diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.c b/drivers/net/wireless/ath/wil6210/txrx_edma.c
index ce71358..fe758f2 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.c
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.c
@@ -606,9 +606,9 @@ static void wil_rx_buf_len_init_edma(struct wil6210_priv *wil)
 		WIL_MAX_ETH_MTU : WIL_EDMA_RX_BUF_LEN_DEFAULT;
 }
 
-static int wil_rx_init_edma(struct wil6210_priv *wil, u16 desc_ring_size)
+static int wil_rx_init_edma(struct wil6210_priv *wil, uint desc_ring_order)
 {
-	u16 status_ring_size;
+	u16 status_ring_size, desc_ring_size = 1 << desc_ring_order;
 	struct wil_ring *ring = &wil->ring_rx;
 	int rc;
 	size_t elem_size = wil->use_compressed_rx_status ?
@@ -623,7 +623,12 @@ static int wil_rx_init_edma(struct wil6210_priv *wil, u16 desc_ring_size)
 			"compressed RX status cannot be used with SW reorder\n");
 		return -EINVAL;
 	}
-
+	if (wil->rx_status_ring_order <= desc_ring_order)
+		/* make sure sring is larger than desc ring */
+		wil->rx_status_ring_order = desc_ring_order + 1;
+	if (wil->rx_buff_id_count <= desc_ring_size)
+		/* make sure we will not run out of buff_ids */
+		wil->rx_buff_id_count = desc_ring_size + 512;
 	if (wil->rx_status_ring_order < WIL_SRING_SIZE_ORDER_MIN ||
 	    wil->rx_status_ring_order > WIL_SRING_SIZE_ORDER_MAX)
 		wil->rx_status_ring_order = WIL_RX_SRING_SIZE_ORDER_DEFAULT;
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 414de54..31753f94 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -614,7 +614,7 @@ struct wil_txrx_ops {
 			      int cid, int tid);
 	irqreturn_t (*irq_tx)(int irq, void *cookie);
 	/* RX ops */
-	int (*rx_init)(struct wil6210_priv *wil, u16 ring_size);
+	int (*rx_init)(struct wil6210_priv *wil, uint ring_order);
 	void (*rx_fini)(struct wil6210_priv *wil);
 	int (*wmi_addba_rx_resp)(struct wil6210_priv *wil, u8 mid, u8 cid,
 				 u8 tid, u8 token, u16 status, bool amsdu,
-- 
1.9.1

[PATCH 05/16] wil6210: add recovery for FW error while in AP mode

[Maya Erez]

From: Dedy Lansky <dlansky@codeaurora.org>

AP configuration is stored by the driver. Upon FW error, disconnect
notification is sent to user space for any associated stations. AP is
then internally restarted with the stored configuration.

Signed-off-by: Dedy Lansky <dlansky@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/cfg80211.c | 102 ++++++++++++++++++++++++++--
 drivers/net/wireless/ath/wil6210/main.c     |  17 ++++-
 drivers/net/wireless/ath/wil6210/wil6210.h  |   9 +++
 3 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index d18e81f..e9135d6 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -51,6 +51,19 @@
 	CHAN60G(4, 0),
 };
 
+static void
+wil_memdup_ie(u8 **pdst, size_t *pdst_len, const u8 *src, size_t src_len)
+{
+	kfree(*pdst);
+	*pdst = NULL;
+	*pdst_len = 0;
+	if (src_len > 0) {
+		*pdst = kmemdup(src, src_len, GFP_KERNEL);
+		if (*pdst)
+			*pdst_len = src_len;
+	}
+}
+
 static int wil_num_supported_channels(struct wil6210_priv *wil)
 {
 	int num_channels = ARRAY_SIZE(wil_60ghz_channels);
@@ -1441,11 +1454,19 @@ static int wil_cfg80211_add_key(struct wiphy *wiphy,
 
 	rc = wmi_add_cipher_key(vif, key_index, mac_addr, params->key_len,
 				params->key, key_usage);
-	if (!rc && !IS_ERR(cs))
+	if (!rc && !IS_ERR(cs)) {
+		/* update local storage used for AP recovery */
+		if (key_usage == WMI_KEY_USE_TX_GROUP && params->key &&
+		    params->key_len <= WMI_MAX_KEY_LEN) {
+			vif->gtk_index = key_index;
+			memcpy(vif->gtk, params->key, params->key_len);
+			vif->gtk_len = params->key_len;
+		}
 		/* in FT set crypto will take place upon receiving
 		 * WMI_RING_EN_EVENTID event
 		 */
 		wil_set_crypto_rx(key_index, key_usage, cs, params);
+	}
 
 	return rc;
 }
@@ -1634,6 +1655,14 @@ static int _wil_cfg80211_set_ies(struct wil6210_vif *vif,
 	u16 len = 0, proberesp_len = 0;
 	u8 *ies = NULL, *proberesp;
 
+	/* update local storage used for AP recovery */
+	wil_memdup_ie(&vif->proberesp, &vif->proberesp_len, bcon->probe_resp,
+		      bcon->probe_resp_len);
+	wil_memdup_ie(&vif->proberesp_ies, &vif->proberesp_ies_len,
+		      bcon->proberesp_ies, bcon->proberesp_ies_len);
+	wil_memdup_ie(&vif->assocresp_ies, &vif->assocresp_ies_len,
+		      bcon->assocresp_ies, bcon->assocresp_ies_len);
+
 	proberesp = _wil_cfg80211_get_proberesp_ies(bcon->probe_resp,
 						    bcon->probe_resp_len,
 						    &proberesp_len);
@@ -1735,6 +1764,9 @@ static int _wil_cfg80211_start_ap(struct wiphy *wiphy,
 	vif->channel = chan;
 	vif->hidden_ssid = hidden_ssid;
 	vif->pbss = pbss;
+	vif->bi = bi;
+	memcpy(vif->ssid, ssid, ssid_len);
+	vif->ssid_len = ssid_len;
 
 	netif_carrier_on(ndev);
 	if (!wil_has_other_active_ifaces(wil, ndev, false, true))
@@ -1761,11 +1793,64 @@ static int _wil_cfg80211_start_ap(struct wiphy *wiphy,
 	return rc;
 }
 
+void wil_cfg80211_ap_recovery(struct wil6210_priv *wil)
+{
+	int rc, i;
+	struct wiphy *wiphy = wil_to_wiphy(wil);
+
+	for (i = 0; i < wil->max_vifs; i++) {
+		struct wil6210_vif *vif = wil->vifs[i];
+		struct net_device *ndev;
+		struct cfg80211_beacon_data bcon = {};
+		struct key_params key_params = {};
+
+		if (!vif || vif->ssid_len == 0)
+			continue;
+
+		ndev = vif_to_ndev(vif);
+		bcon.proberesp_ies = vif->proberesp_ies;
+		bcon.assocresp_ies = vif->assocresp_ies;
+		bcon.probe_resp = vif->proberesp;
+		bcon.proberesp_ies_len = vif->proberesp_ies_len;
+		bcon.assocresp_ies_len = vif->assocresp_ies_len;
+		bcon.probe_resp_len = vif->proberesp_len;
+
+		wil_info(wil,
+			 "AP (vif %d) recovery: privacy %d, bi %d, channel %d, hidden %d, pbss %d\n",
+			 i, vif->privacy, vif->bi, vif->channel,
+			 vif->hidden_ssid, vif->pbss);
+		wil_hex_dump_misc("SSID ", DUMP_PREFIX_OFFSET, 16, 1,
+				  vif->ssid, vif->ssid_len, true);
+		rc = _wil_cfg80211_start_ap(wiphy, ndev,
+					    vif->ssid, vif->ssid_len,
+					    vif->privacy, vif->bi,
+					    vif->channel, &bcon,
+					    vif->hidden_ssid, vif->pbss);
+		if (rc) {
+			wil_err(wil, "vif %d recovery failed (%d)\n", i, rc);
+			continue;
+		}
+
+		if (!vif->privacy || vif->gtk_len == 0)
+			continue;
+
+		key_params.key = vif->gtk;
+		key_params.key_len = vif->gtk_len;
+		key_params.seq_len = IEEE80211_GCMP_PN_LEN;
+		rc = wil_cfg80211_add_key(wiphy, ndev, vif->gtk_index, false,
+					  NULL, &key_params);
+		if (rc)
+			wil_err(wil, "vif %d recovery add key failed (%d)\n",
+				i, rc);
+	}
+}
+
 static int wil_cfg80211_change_beacon(struct wiphy *wiphy,
 				      struct net_device *ndev,
 				      struct cfg80211_beacon_data *bcon)
 {
 	struct wil6210_priv *wil = wiphy_to_wil(wiphy);
+	struct wireless_dev *wdev = ndev->ieee80211_ptr;
 	struct wil6210_vif *vif = ndev_to_vif(ndev);
 	int rc;
 	u32 privacy = 0;
@@ -1778,15 +1863,16 @@ static int wil_cfg80211_change_beacon(struct wiphy *wiphy,
 			     bcon->tail_len))
 		privacy = 1;
 
+	memcpy(vif->ssid, wdev->ssid, wdev->ssid_len);
+	vif->ssid_len = wdev->ssid_len;
+
 	/* in case privacy has changed, need to restart the AP */
 	if (vif->privacy != privacy) {
-		struct wireless_dev *wdev = ndev->ieee80211_ptr;
-
 		wil_dbg_misc(wil, "privacy changed %d=>%d. Restarting AP\n",
 			     vif->privacy, privacy);
 
-		rc = _wil_cfg80211_start_ap(wiphy, ndev, wdev->ssid,
-					    wdev->ssid_len, privacy,
+		rc = _wil_cfg80211_start_ap(wiphy, ndev, vif->ssid,
+					    vif->ssid_len, privacy,
 					    wdev->beacon_interval,
 					    vif->channel, bcon,
 					    vif->hidden_ssid,
@@ -1876,6 +1962,12 @@ static int wil_cfg80211_stop_ap(struct wiphy *wiphy,
 
 	wmi_pcp_stop(vif);
 	clear_bit(wil_vif_ft_roam, vif->status);
+	vif->ssid_len = 0;
+	wil_memdup_ie(&vif->proberesp, &vif->proberesp_len, NULL, 0);
+	wil_memdup_ie(&vif->proberesp_ies, &vif->proberesp_ies_len, NULL, 0);
+	wil_memdup_ie(&vif->assocresp_ies, &vif->assocresp_ies_len, NULL, 0);
+	memset(vif->gtk, 0, WMI_MAX_KEY_LEN);
+	vif->gtk_len = 0;
 
 	if (last)
 		__wil_down(wil);
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 55084770..2b328c1 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -18,6 +18,7 @@
 #include <linux/moduleparam.h>
 #include <linux/if_arp.h>
 #include <linux/etherdevice.h>
+#include <linux/rtnetlink.h>
 
 #include "wil6210.h"
 #include "txrx.h"
@@ -485,10 +486,11 @@ static void wil_fw_error_worker(struct work_struct *work)
 	if (wil_wait_for_recovery(wil) != 0)
 		return;
 
+	rtnl_lock();
 	mutex_lock(&wil->mutex);
 	/* Needs adaptation for multiple VIFs
 	 * need to go over all VIFs and consider the appropriate
-	 * recovery.
+	 * recovery because each one can have different iftype.
 	 */
 	switch (wdev->iftype) {
 	case NL80211_IFTYPE_STATION:
@@ -500,15 +502,24 @@ static void wil_fw_error_worker(struct work_struct *work)
 		break;
 	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_P2P_GO:
-		wil_info(wil, "No recovery for AP-like interface\n");
-		/* recovery in these modes is done by upper layers */
+		if (no_fw_recovery) /* upper layers do recovery */
+			break;
+		/* silent recovery, upper layers will see disconnect */
+		__wil_down(wil);
+		__wil_up(wil);
+		mutex_unlock(&wil->mutex);
+		wil_cfg80211_ap_recovery(wil);
+		mutex_lock(&wil->mutex);
+		wil_info(wil, "... completed\n");
 		break;
 	default:
 		wil_err(wil, "No recovery - unknown interface type %d\n",
 			wdev->iftype);
 		break;
 	}
+
 	mutex_unlock(&wil->mutex);
+	rtnl_unlock();
 }
 
 static int wil_find_free_ring(struct wil6210_priv *wil)
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 31753f94..8050c4b 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -849,6 +849,14 @@ struct wil6210_vif {
 	u8 hidden_ssid; /* relevant in AP mode */
 	u32 ap_isolate; /* no intra-BSS communication */
 	bool pbss;
+	int bi;
+	u8 *proberesp, *proberesp_ies, *assocresp_ies;
+	size_t proberesp_len, proberesp_ies_len, assocresp_ies_len;
+	u8 ssid[IEEE80211_MAX_SSID_LEN];
+	size_t ssid_len;
+	u8 gtk_index;
+	u8 gtk[WMI_MAX_KEY_LEN];
+	size_t gtk_len;
 	int bcast_ring;
 	struct cfg80211_bss *bss; /* connected bss, relevant in STA mode */
 	int locally_generated_disc; /* relevant in STA mode */
@@ -1277,6 +1285,7 @@ int wil_p2p_listen(struct wil6210_priv *wil, struct wireless_dev *wdev,
 int wil_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 			 struct cfg80211_mgmt_tx_params *params,
 			 u64 *cookie);
+void wil_cfg80211_ap_recovery(struct wil6210_priv *wil);
 int wil_cfg80211_iface_combinations_from_fw(
 	struct wil6210_priv *wil,
 	const struct wil_fw_record_concurrency *conc);
-- 
1.9.1

[PATCH 06/16] wil6210: fix memory leak in wil_find_tx_bcast_2

[Maya Erez]

From: Lior David <liord@codeaurora.org>

A successful call to wil_tx_ring takes skb reference so
it will only be freed in wil_tx_complete. Consume the skb
in wil_find_tx_bcast_2 to prevent memory leak.

Signed-off-by: Lior David <liord@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/txrx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index c592c8e..3e1c831 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -1395,6 +1395,8 @@ static struct wil_ring *wil_find_tx_bcast_2(struct wil6210_priv *wil,
 			wil_dbg_txrx(wil, "BCAST DUP -> ring %d\n", i);
 			wil_set_da_for_vring(wil, skb2, i);
 			wil_tx_ring(wil, vif, v2, skb2);
+			/* successful call to wil_tx_ring takes skb2 ref */
+			dev_kfree_skb_any(skb2);
 		} else {
 			wil_err(wil, "skb_copy failed\n");
 		}
-- 
1.9.1

[PATCH 07/16] wil6210: refactor disconnect flow

[Maya Erez]

From: Ahmad Masri <amasri@codeaurora.org>

Separate sending command to the fw from the event handling function to
simplify the disconnect flow and track the from_event flag correctly.

Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/cfg80211.c |   2 +-
 drivers/net/wireless/ath/wil6210/main.c     | 180 +++++++++++++++++++++-------
 drivers/net/wireless/ath/wil6210/netdev.c   |   2 +-
 drivers/net/wireless/ath/wil6210/wil6210.h  |   8 +-
 drivers/net/wireless/ath/wil6210/wmi.c      |  28 ++---
 5 files changed, 148 insertions(+), 72 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index e9135d6..9b2f9f5 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -2015,7 +2015,7 @@ static int wil_cfg80211_del_station(struct wiphy *wiphy,
 		     params->mac, params->reason_code, vif->mid);
 
 	mutex_lock(&wil->mutex);
-	wil6210_disconnect(vif, params->mac, params->reason_code, false);
+	wil6210_disconnect(vif, params->mac, params->reason_code);
 	mutex_unlock(&wil->mutex);
 
 	return 0;
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 2b328c1..9dd068d 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -215,8 +215,21 @@ static void wil_ring_fini_tx(struct wil6210_priv *wil, int id)
 	wil->txrx_ops.ring_fini_tx(wil, ring);
 }
 
-static void wil_disconnect_cid(struct wil6210_vif *vif, int cid,
-			       u16 reason_code, bool from_event)
+static bool wil_vif_is_connected(struct wil6210_priv *wil, u8 mid)
+{
+	int i;
+
+	for (i = 0; i < WIL6210_MAX_CID; i++) {
+		if (wil->sta[i].mid == mid &&
+		    wil->sta[i].status == wil_sta_connected)
+			return true;
+	}
+
+	return false;
+}
+
+static void wil_disconnect_cid_complete(struct wil6210_vif *vif, int cid,
+					u16 reason_code)
 __acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
 {
 	uint i;
@@ -227,24 +240,14 @@ static void wil_disconnect_cid(struct wil6210_vif *vif, int cid,
 	int min_ring_id = wil_get_min_tx_ring_id(wil);
 
 	might_sleep();
-	wil_dbg_misc(wil, "disconnect_cid: CID %d, MID %d, status %d\n",
+	wil_dbg_misc(wil,
+		     "disconnect_cid_complete: CID %d, MID %d, status %d\n",
 		     cid, sta->mid, sta->status);
-	/* inform upper/lower layers */
+	/* inform upper layers */
 	if (sta->status != wil_sta_unused) {
 		if (vif->mid != sta->mid) {
 			wil_err(wil, "STA MID mismatch with VIF MID(%d)\n",
 				vif->mid);
-			/* let FW override sta->mid but be more strict with
-			 * user space requests
-			 */
-			if (!from_event)
-				return;
-		}
-		if (!from_event) {
-			bool del_sta = (wdev->iftype == NL80211_IFTYPE_AP) ?
-						disable_ap_sme : false;
-			wmi_disconnect_sta(vif, sta->addr, reason_code,
-					   true, del_sta);
 		}
 
 		switch (wdev->iftype) {
@@ -284,36 +287,20 @@ static void wil_disconnect_cid(struct wil6210_vif *vif, int cid,
 	sta->stats.tx_latency_min_us = U32_MAX;
 }
 
-static bool wil_vif_is_connected(struct wil6210_priv *wil, u8 mid)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(wil->sta); i++) {
-		if (wil->sta[i].mid == mid &&
-		    wil->sta[i].status == wil_sta_connected)
-			return true;
-	}
-
-	return false;
-}
-
-static void _wil6210_disconnect(struct wil6210_vif *vif, const u8 *bssid,
-				u16 reason_code, bool from_event)
+static void _wil6210_disconnect_complete(struct wil6210_vif *vif,
+					 const u8 *bssid, u16 reason_code)
 {
 	struct wil6210_priv *wil = vif_to_wil(vif);
 	int cid = -ENOENT;
 	struct net_device *ndev;
 	struct wireless_dev *wdev;
 
-	if (unlikely(!vif))
-		return;
-
 	ndev = vif_to_ndev(vif);
 	wdev = vif_to_wdev(vif);
 
 	might_sleep();
-	wil_info(wil, "bssid=%pM, reason=%d, ev%s\n", bssid,
-		 reason_code, from_event ? "+" : "-");
+	wil_info(wil, "disconnect_complete: bssid=%pM, reason=%d\n",
+		 bssid, reason_code);
 
 	/* Cases are:
 	 * - disconnect single STA, still connected
@@ -328,14 +315,15 @@ static void _wil6210_disconnect(struct wil6210_vif *vif, const u8 *bssid,
 	if (bssid && !is_broadcast_ether_addr(bssid) &&
 	    !ether_addr_equal_unaligned(ndev->dev_addr, bssid)) {
 		cid = wil_find_cid(wil, vif->mid, bssid);
-		wil_dbg_misc(wil, "Disconnect %pM, CID=%d, reason=%d\n",
+		wil_dbg_misc(wil,
+			     "Disconnect complete %pM, CID=%d, reason=%d\n",
 			     bssid, cid, reason_code);
 		if (cid >= 0) /* disconnect 1 peer */
-			wil_disconnect_cid(vif, cid, reason_code, from_event);
+			wil_disconnect_cid_complete(vif, cid, reason_code);
 	} else { /* all */
-		wil_dbg_misc(wil, "Disconnect all\n");
+		wil_dbg_misc(wil, "Disconnect complete all\n");
 		for (cid = 0; cid < WIL6210_MAX_CID; cid++)
-			wil_disconnect_cid(vif, cid, reason_code, from_event);
+			wil_disconnect_cid_complete(vif, cid, reason_code);
 	}
 
 	/* link state */
@@ -381,6 +369,85 @@ static void _wil6210_disconnect(struct wil6210_vif *vif, const u8 *bssid,
 	}
 }
 
+static int wil_disconnect_cid(struct wil6210_vif *vif, int cid,
+			      u16 reason_code)
+__acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
+{
+	struct wil6210_priv *wil = vif_to_wil(vif);
+	struct wireless_dev *wdev = vif_to_wdev(vif);
+	struct wil_sta_info *sta = &wil->sta[cid];
+	bool del_sta = false;
+
+	might_sleep();
+	wil_dbg_misc(wil, "disconnect_cid: CID %d, MID %d, status %d\n",
+		     cid, sta->mid, sta->status);
+
+	if (sta->status == wil_sta_unused)
+		return 0;
+
+	if (vif->mid != sta->mid) {
+		wil_err(wil, "STA MID mismatch with VIF MID(%d)\n", vif->mid);
+		return -EINVAL;
+	}
+
+	/* inform lower layers */
+	if (wdev->iftype == NL80211_IFTYPE_AP && disable_ap_sme)
+		del_sta = true;
+
+	/* disconnect by sending command disconnect/del_sta and wait
+	 * synchronously for WMI_DISCONNECT_EVENTID event.
+	 */
+	return wmi_disconnect_sta(vif, sta->addr, reason_code, del_sta);
+}
+
+static void _wil6210_disconnect(struct wil6210_vif *vif, const u8 *bssid,
+				u16 reason_code)
+{
+	struct wil6210_priv *wil;
+	struct net_device *ndev;
+	struct wireless_dev *wdev;
+	int cid = -ENOENT;
+
+	if (unlikely(!vif))
+		return;
+
+	wil = vif_to_wil(vif);
+	ndev = vif_to_ndev(vif);
+	wdev = vif_to_wdev(vif);
+
+	might_sleep();
+	wil_info(wil, "disconnect bssid=%pM, reason=%d\n", bssid, reason_code);
+
+	/* Cases are:
+	 * - disconnect single STA, still connected
+	 * - disconnect single STA, already disconnected
+	 * - disconnect all
+	 *
+	 * For "disconnect all", there are 3 options:
+	 * - bssid == NULL
+	 * - bssid is broadcast address (ff:ff:ff:ff:ff:ff)
+	 * - bssid is our MAC address
+	 */
+	if (bssid && !is_broadcast_ether_addr(bssid) &&
+	    !ether_addr_equal_unaligned(ndev->dev_addr, bssid)) {
+		cid = wil_find_cid(wil, vif->mid, bssid);
+		wil_dbg_misc(wil, "Disconnect %pM, CID=%d, reason=%d\n",
+			     bssid, cid, reason_code);
+		if (cid >= 0) /* disconnect 1 peer */
+			wil_disconnect_cid(vif, cid, reason_code);
+	} else { /* all */
+		wil_dbg_misc(wil, "Disconnect all\n");
+		for (cid = 0; cid < WIL6210_MAX_CID; cid++)
+			wil_disconnect_cid(vif, cid, reason_code);
+	}
+
+	/* call event handler manually after processing wmi_call,
+	 * to avoid deadlock - disconnect event handler acquires
+	 * wil->mutex while it is already held here
+	 */
+	_wil6210_disconnect_complete(vif, bssid, reason_code);
+}
+
 void wil_disconnect_worker(struct work_struct *work)
 {
 	struct wil6210_vif *vif = container_of(work,
@@ -705,20 +772,41 @@ void wil6210_bus_request(struct wil6210_priv *wil, u32 kbps)
  * @vif: virtual interface context
  * @bssid: peer to disconnect, NULL to disconnect all
  * @reason_code: Reason code for the Disassociation frame
- * @from_event: whether is invoked from FW event handler
  *
- * Disconnect and release associated resources. If invoked not from the
- * FW event handler, issue WMI command(s) to trigger MAC disconnect.
+ * Disconnect and release associated resources. Issue WMI
+ * command(s) to trigger MAC disconnect. When command was issued
+ * successfully, call the wil6210_disconnect_complete function
+ * to handle the event synchronously
  */
 void wil6210_disconnect(struct wil6210_vif *vif, const u8 *bssid,
-			u16 reason_code, bool from_event)
+			u16 reason_code)
+{
+	struct wil6210_priv *wil = vif_to_wil(vif);
+
+	wil_dbg_misc(wil, "disconnecting\n");
+
+	del_timer_sync(&vif->connect_timer);
+	_wil6210_disconnect(vif, bssid, reason_code);
+}
+
+/**
+ * wil6210_disconnect_complete - handle disconnect event
+ * @vif: virtual interface context
+ * @bssid: peer to disconnect, NULL to disconnect all
+ * @reason_code: Reason code for the Disassociation frame
+ *
+ * Release associated resources and indicate upper layers the
+ * connection is terminated.
+ */
+void wil6210_disconnect_complete(struct wil6210_vif *vif, const u8 *bssid,
+				 u16 reason_code)
 {
 	struct wil6210_priv *wil = vif_to_wil(vif);
 
-	wil_dbg_misc(wil, "disconnect\n");
+	wil_dbg_misc(wil, "got disconnect\n");
 
 	del_timer_sync(&vif->connect_timer);
-	_wil6210_disconnect(vif, bssid, reason_code, from_event);
+	_wil6210_disconnect_complete(vif, bssid, reason_code);
 }
 
 void wil_priv_deinit(struct wil6210_priv *wil)
@@ -1525,7 +1613,7 @@ int wil_reset(struct wil6210_priv *wil, bool load_fw)
 		if (vif) {
 			cancel_work_sync(&vif->disconnect_worker);
 			wil6210_disconnect(vif, NULL,
-					   WLAN_REASON_DEAUTH_LEAVING, false);
+					   WLAN_REASON_DEAUTH_LEAVING);
 		}
 	}
 	wil_bcast_fini_all(wil);
diff --git a/drivers/net/wireless/ath/wil6210/netdev.c b/drivers/net/wireless/ath/wil6210/netdev.c
index 64fa1a2..b4e0eb1 100644
--- a/drivers/net/wireless/ath/wil6210/netdev.c
+++ b/drivers/net/wireless/ath/wil6210/netdev.c
@@ -512,7 +512,7 @@ void wil_vif_remove(struct wil6210_priv *wil, u8 mid)
 	}
 
 	mutex_lock(&wil->mutex);
-	wil6210_disconnect(vif, NULL, WLAN_REASON_DEAUTH_LEAVING, false);
+	wil6210_disconnect(vif, NULL, WLAN_REASON_DEAUTH_LEAVING);
 	mutex_unlock(&wil->mutex);
 
 	ndev = vif_to_ndev(vif);
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 8050c4b..ad7003f 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -1229,8 +1229,8 @@ int wmi_add_cipher_key(struct wil6210_vif *vif, u8 key_index,
 int wmi_update_ft_ies(struct wil6210_vif *vif, u16 ie_len, const void *ie);
 int wmi_rxon(struct wil6210_priv *wil, bool on);
 int wmi_get_temperature(struct wil6210_priv *wil, u32 *t_m, u32 *t_r);
-int wmi_disconnect_sta(struct wil6210_vif *vif, const u8 *mac,
-		       u16 reason, bool full_disconnect, bool del_sta);
+int wmi_disconnect_sta(struct wil6210_vif *vif, const u8 *mac, u16 reason,
+		       bool del_sta);
 int wmi_addba(struct wil6210_priv *wil, u8 mid,
 	      u8 ringid, u8 size, u16 timeout);
 int wmi_delba_tx(struct wil6210_priv *wil, u8 mid, u8 ringid, u16 reason);
@@ -1316,7 +1316,9 @@ int wmi_pcp_start(struct wil6210_vif *vif, int bi, u8 wmi_nettype, u8 chan,
 void wil_abort_scan_all_vifs(struct wil6210_priv *wil, bool sync);
 void wil6210_bus_request(struct wil6210_priv *wil, u32 kbps);
 void wil6210_disconnect(struct wil6210_vif *vif, const u8 *bssid,
-			u16 reason_code, bool from_event);
+			u16 reason_code);
+void wil6210_disconnect_complete(struct wil6210_vif *vif, const u8 *bssid,
+				 u16 reason_code);
 void wil_probe_client_flush(struct wil6210_vif *vif);
 void wil_probe_client_worker(struct work_struct *work);
 void wil_disconnect_worker(struct work_struct *work);
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index 4859f0e..5ff1862 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -1018,7 +1018,7 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int id, void *d, int len)
 		wil_err(wil, "config tx vring failed for CID %d, rc (%d)\n",
 			evt->cid, rc);
 		wmi_disconnect_sta(vif, wil->sta[evt->cid].addr,
-				   WLAN_REASON_UNSPECIFIED, false, false);
+				   WLAN_REASON_UNSPECIFIED, false);
 	} else {
 		wil_info(wil, "successful connection to CID %d\n", evt->cid);
 	}
@@ -1112,7 +1112,7 @@ static void wmi_evt_disconnect(struct wil6210_vif *vif, int id,
 	}
 
 	mutex_lock(&wil->mutex);
-	wil6210_disconnect(vif, evt->bssid, reason_code, true);
+	wil6210_disconnect_complete(vif, evt->bssid, reason_code);
 	mutex_unlock(&wil->mutex);
 }
 
@@ -1637,7 +1637,7 @@ static int wil_find_cid_ringid_sta(struct wil6210_priv *wil,
 	return;
 
 fail:
-	wil6210_disconnect(vif, NULL, WLAN_REASON_PREV_AUTH_NOT_VALID, false);
+	wil6210_disconnect(vif, NULL, WLAN_REASON_PREV_AUTH_NOT_VALID);
 }
 
 static void
@@ -1766,7 +1766,7 @@ static int wil_find_cid_ringid_sta(struct wil6210_priv *wil,
 	return;
 
 fail:
-	wil6210_disconnect(vif, NULL, WLAN_REASON_PREV_AUTH_NOT_VALID, false);
+	wil6210_disconnect(vif, NULL, WLAN_REASON_PREV_AUTH_NOT_VALID);
 }
 
 /**
@@ -2560,12 +2560,11 @@ int wmi_get_temperature(struct wil6210_priv *wil, u32 *t_bb, u32 *t_rf)
 	return 0;
 }
 
-int wmi_disconnect_sta(struct wil6210_vif *vif, const u8 *mac,
-		       u16 reason, bool full_disconnect, bool del_sta)
+int wmi_disconnect_sta(struct wil6210_vif *vif, const u8 *mac, u16 reason,
+		       bool del_sta)
 {
 	struct wil6210_priv *wil = vif_to_wil(vif);
 	int rc;
-	u16 reason_code;
 	struct wmi_disconnect_sta_cmd disc_sta_cmd = {
 		.disconnect_reason = cpu_to_le16(reason),
 	};
@@ -2598,21 +2597,8 @@ int wmi_disconnect_sta(struct wil6210_vif *vif, const u8 *mac,
 		wil_fw_error_recovery(wil);
 		return rc;
 	}
+	wil->sinfo_gen++;
 
-	if (full_disconnect) {
-		/* call event handler manually after processing wmi_call,
-		 * to avoid deadlock - disconnect event handler acquires
-		 * wil->mutex while it is already held here
-		 */
-		reason_code = le16_to_cpu(reply.evt.protocol_reason_status);
-
-		wil_dbg_wmi(wil, "Disconnect %pM reason [proto %d wmi %d]\n",
-			    reply.evt.bssid, reason_code,
-			    reply.evt.disconnect_reason);
-
-		wil->sinfo_gen++;
-		wil6210_disconnect(vif, reply.evt.bssid, reason_code, true);
-	}
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH 07/16] wil6210: refactor disconnect flow

[Kalle Valo]

Maya Erez <merez@codeaurora.org> writes:

> From: Ahmad Masri <amasri@codeaurora.org>
>
> Separate sending command to the fw from the event handling function to
> simplify the disconnect flow and track the from_event flag correctly.
>
> Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
> Signed-off-by: Maya Erez <merez@codeaurora.org>

[...]

> +static int wil_disconnect_cid(struct wil6210_vif *vif, int cid,
> +			      u16 reason_code)
> +__acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
> +{
> +	struct wil6210_priv *wil = vif_to_wil(vif);
> +	struct wireless_dev *wdev = vif_to_wdev(vif);
> +	struct wil_sta_info *sta = &wil->sta[cid];
> +	bool del_sta = false;
> +
> +	might_sleep();
> +	wil_dbg_misc(wil, "disconnect_cid: CID %d, MID %d, status %d\n",
> +		     cid, sta->mid, sta->status);
> +
> +	if (sta->status == wil_sta_unused)
> +		return 0;
> +
> +	if (vif->mid != sta->mid) {
> +		wil_err(wil, "STA MID mismatch with VIF MID(%d)\n", vif->mid);
> +		return -EINVAL;
> +	}
> +
> +	/* inform lower layers */
> +	if (wdev->iftype == NL80211_IFTYPE_AP && disable_ap_sme)
> +		del_sta = true;
> +
> +	/* disconnect by sending command disconnect/del_sta and wait
> +	 * synchronously for WMI_DISCONNECT_EVENTID event.
> +	 */
> +	return wmi_disconnect_sta(vif, sta->addr, reason_code, del_sta);
> +}

I don't get use of __acquires() and __releases() in this function. I see
similar pattern already in wil6210 but care to explain why this is
needed? I don't see the function even accessing tid_rx_lock so I'm very
confused.

-- 
Kalle Valo

Re: [PATCH 07/16] wil6210: refactor disconnect flow

[merez]

On 2018-11-06 12:30, Kalle Valo wrote:
> Maya Erez <merez@codeaurora.org> writes:
> 
>> From: Ahmad Masri <amasri@codeaurora.org>
>> 
>> Separate sending command to the fw from the event handling function to
>> simplify the disconnect flow and track the from_event flag correctly.
>> 
>> Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
>> Signed-off-by: Maya Erez <merez@codeaurora.org>
> 
> [...]
> 
>> +static int wil_disconnect_cid(struct wil6210_vif *vif, int cid,
>> +			      u16 reason_code)
>> +__acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
>> +{
>> +	struct wil6210_priv *wil = vif_to_wil(vif);
>> +	struct wireless_dev *wdev = vif_to_wdev(vif);
>> +	struct wil_sta_info *sta = &wil->sta[cid];
>> +	bool del_sta = false;
>> +
>> +	might_sleep();
>> +	wil_dbg_misc(wil, "disconnect_cid: CID %d, MID %d, status %d\n",
>> +		     cid, sta->mid, sta->status);
>> +
>> +	if (sta->status == wil_sta_unused)
>> +		return 0;
>> +
>> +	if (vif->mid != sta->mid) {
>> +		wil_err(wil, "STA MID mismatch with VIF MID(%d)\n", vif->mid);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* inform lower layers */
>> +	if (wdev->iftype == NL80211_IFTYPE_AP && disable_ap_sme)
>> +		del_sta = true;
>> +
>> +	/* disconnect by sending command disconnect/del_sta and wait
>> +	 * synchronously for WMI_DISCONNECT_EVENTID event.
>> +	 */
>> +	return wmi_disconnect_sta(vif, sta->addr, reason_code, del_sta);
>> +}
> 
> I don't get use of __acquires() and __releases() in this function. I 
> see
> similar pattern already in wil6210 but care to explain why this is
> needed? I don't see the function even accessing tid_rx_lock so I'm very
> confused.

I assume it is a copy / paste leftover that we missed in the code 
review.
We will remove it.

-- 
Maya Erez
Qualcomm Israel, Inc. on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 07/16] wil6210: refactor disconnect flow

[Kalle Valo]

merez@codeaurora.org writes:

> On 2018-11-06 12:30, Kalle Valo wrote:
>> Maya Erez <merez@codeaurora.org> writes:
>>
>>> From: Ahmad Masri <amasri@codeaurora.org>
>>>
>>> Separate sending command to the fw from the event handling function to
>>> simplify the disconnect flow and track the from_event flag correctly.
>>>
>>> Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
>>> Signed-off-by: Maya Erez <merez@codeaurora.org>
>>
>> [...]
>>
>>> +static int wil_disconnect_cid(struct wil6210_vif *vif, int cid,
>>> +			      u16 reason_code)
>>> +__acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
>>> +{
>>> +	struct wil6210_priv *wil = vif_to_wil(vif);
>>> +	struct wireless_dev *wdev = vif_to_wdev(vif);
>>> +	struct wil_sta_info *sta = &wil->sta[cid];
>>> +	bool del_sta = false;
>>> +
>>> +	might_sleep();
>>> +	wil_dbg_misc(wil, "disconnect_cid: CID %d, MID %d, status %d\n",
>>> +		     cid, sta->mid, sta->status);
>>> +
>>> +	if (sta->status == wil_sta_unused)
>>> +		return 0;
>>> +
>>> +	if (vif->mid != sta->mid) {
>>> +		wil_err(wil, "STA MID mismatch with VIF MID(%d)\n", vif->mid);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	/* inform lower layers */
>>> +	if (wdev->iftype == NL80211_IFTYPE_AP && disable_ap_sme)
>>> +		del_sta = true;
>>> +
>>> +	/* disconnect by sending command disconnect/del_sta and wait
>>> +	 * synchronously for WMI_DISCONNECT_EVENTID event.
>>> +	 */
>>> +	return wmi_disconnect_sta(vif, sta->addr, reason_code, del_sta);
>>> +}
>>
>> I don't get use of __acquires() and __releases() in this function. I
>> see
>> similar pattern already in wil6210 but care to explain why this is
>> needed? I don't see the function even accessing tid_rx_lock so I'm very
>> confused.
>
> I assume it is a copy / paste leftover that we missed in the code
> review. We will remove it.

Actually I already removed the annotations from the pending branch and
no need to resend, it's faster that way. Please double check if you can,
unfortunately I cannot provide a direct link cgit doesn't show the new
commit yet.

-- 
Kalle Valo

Re: [PATCH 07/16] wil6210: refactor disconnect flow

[merez]

On 2018-11-06 14:28, Kalle Valo wrote:
> merez@codeaurora.org writes:
> 
>> On 2018-11-06 12:30, Kalle Valo wrote:
>>> Maya Erez <merez@codeaurora.org> writes:
>>> 
>>>> From: Ahmad Masri <amasri@codeaurora.org>
>>>> 
>>>> Separate sending command to the fw from the event handling function 
>>>> to
>>>> simplify the disconnect flow and track the from_event flag 
>>>> correctly.
>>>> 
>>>> Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
>>>> Signed-off-by: Maya Erez <merez@codeaurora.org>
>>> 
>>> [...]
>>> 
>>>> +static int wil_disconnect_cid(struct wil6210_vif *vif, int cid,
>>>> +			      u16 reason_code)
>>>> +__acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
>>>> +{
>>>> +	struct wil6210_priv *wil = vif_to_wil(vif);
>>>> +	struct wireless_dev *wdev = vif_to_wdev(vif);
>>>> +	struct wil_sta_info *sta = &wil->sta[cid];
>>>> +	bool del_sta = false;
>>>> +
>>>> +	might_sleep();
>>>> +	wil_dbg_misc(wil, "disconnect_cid: CID %d, MID %d, status %d\n",
>>>> +		     cid, sta->mid, sta->status);
>>>> +
>>>> +	if (sta->status == wil_sta_unused)
>>>> +		return 0;
>>>> +
>>>> +	if (vif->mid != sta->mid) {
>>>> +		wil_err(wil, "STA MID mismatch with VIF MID(%d)\n", vif->mid);
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	/* inform lower layers */
>>>> +	if (wdev->iftype == NL80211_IFTYPE_AP && disable_ap_sme)
>>>> +		del_sta = true;
>>>> +
>>>> +	/* disconnect by sending command disconnect/del_sta and wait
>>>> +	 * synchronously for WMI_DISCONNECT_EVENTID event.
>>>> +	 */
>>>> +	return wmi_disconnect_sta(vif, sta->addr, reason_code, del_sta);
>>>> +}
>>> 
>>> I don't get use of __acquires() and __releases() in this function. I
>>> see
>>> similar pattern already in wil6210 but care to explain why this is
>>> needed? I don't see the function even accessing tid_rx_lock so I'm 
>>> very
>>> confused.
>> 
>> I assume it is a copy / paste leftover that we missed in the code
>> review. We will remove it.
> 
> Actually I already removed the annotations from the pending branch and
> no need to resend, it's faster that way. Please double check if you 
> can,
> unfortunately I cannot provide a direct link cgit doesn't show the new
> commit yet.

In such a case you can go ahead and apply the patches without
"wil6210: ignore HALP ICR if already handled". I'll upstream its fixed 
version
in the next set of wil6210 patches.

-- 
Maya Erez
Qualcomm Israel, Inc. on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 07/16] wil6210: refactor disconnect flow

[Kalle Valo]

merez@codeaurora.org writes:

> On 2018-11-06 14:28, Kalle Valo wrote:
>> merez@codeaurora.org writes:
>>
>>> On 2018-11-06 12:30, Kalle Valo wrote:
>>>> Maya Erez <merez@codeaurora.org> writes:
>>>>
>>>>> From: Ahmad Masri <amasri@codeaurora.org>
>>>>>
>>>>> Separate sending command to the fw from the event handling
>>>>> function to
>>>>> simplify the disconnect flow and track the from_event flag
>>>>> correctly.
>>>>>
>>>>> Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
>>>>> Signed-off-by: Maya Erez <merez@codeaurora.org>
>>>>
>>>> [...]
>>>>
>>>>> +static int wil_disconnect_cid(struct wil6210_vif *vif, int cid,
>>>>> +			      u16 reason_code)
>>>>> +__acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
>>>>> +{
>>>>> +	struct wil6210_priv *wil = vif_to_wil(vif);
>>>>> +	struct wireless_dev *wdev = vif_to_wdev(vif);
>>>>> +	struct wil_sta_info *sta = &wil->sta[cid];
>>>>> +	bool del_sta = false;
>>>>> +
>>>>> +	might_sleep();
>>>>> +	wil_dbg_misc(wil, "disconnect_cid: CID %d, MID %d, status %d\n",
>>>>> +		     cid, sta->mid, sta->status);
>>>>> +
>>>>> +	if (sta->status == wil_sta_unused)
>>>>> +		return 0;
>>>>> +
>>>>> +	if (vif->mid != sta->mid) {
>>>>> +		wil_err(wil, "STA MID mismatch with VIF MID(%d)\n", vif->mid);
>>>>> +		return -EINVAL;
>>>>> +	}
>>>>> +
>>>>> +	/* inform lower layers */
>>>>> +	if (wdev->iftype == NL80211_IFTYPE_AP && disable_ap_sme)
>>>>> +		del_sta = true;
>>>>> +
>>>>> +	/* disconnect by sending command disconnect/del_sta and wait
>>>>> +	 * synchronously for WMI_DISCONNECT_EVENTID event.
>>>>> +	 */
>>>>> +	return wmi_disconnect_sta(vif, sta->addr, reason_code, del_sta);
>>>>> +}
>>>>
>>>> I don't get use of __acquires() and __releases() in this function. I
>>>> see
>>>> similar pattern already in wil6210 but care to explain why this is
>>>> needed? I don't see the function even accessing tid_rx_lock so I'm
>>>> very
>>>> confused.
>>>
>>> I assume it is a copy / paste leftover that we missed in the code
>>> review. We will remove it.
>>
>> Actually I already removed the annotations from the pending branch and
>> no need to resend, it's faster that way. Please double check if you
>> can,
>> unfortunately I cannot provide a direct link cgit doesn't show the new
>> commit yet.
>
> In such a case you can go ahead and apply the patches without
> "wil6210: ignore HALP ICR if already handled". I'll upstream its fixed
> version
> in the next set of wil6210 patches.

Ok, I dropped "wil6210: ignore HALP ICR if already handled" and I'm
planning to apply the rest.

-- 
Kalle Valo

[PATCH 08/16] wil6210: notify cqm packet loss on disable_ap_sme

[Maya Erez]

From: Ahmad Masri <amasri@codeaurora.org>

wil6210 used to notify cfg80211_del_sta on every fw disconnect event.
In disable_ap_sme mode the userspace manages the protocol SME and
FW sends disconnect event only due to link loss.

In disable_ap_sme mode, indicate CQM packet loss to let the host
control the connection and disconnect the link if needed.

Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/wmi.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index 5ff1862..fa08a96 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -1113,6 +1113,23 @@ static void wmi_evt_disconnect(struct wil6210_vif *vif, int id,
 
 	mutex_lock(&wil->mutex);
 	wil6210_disconnect_complete(vif, evt->bssid, reason_code);
+	if (disable_ap_sme) {
+		struct wireless_dev *wdev = vif_to_wdev(vif);
+		struct net_device *ndev = vif_to_ndev(vif);
+
+		/* disconnect event in disable_ap_sme mode means link loss */
+		switch (wdev->iftype) {
+		/* AP-like interface */
+		case NL80211_IFTYPE_AP:
+		case NL80211_IFTYPE_P2P_GO:
+			/* notify hostapd about link loss */
+			cfg80211_cqm_pktloss_notify(ndev, evt->bssid, 0,
+						    GFP_KERNEL);
+			break;
+		default:
+			break;
+		}
+	}
 	mutex_unlock(&wil->mutex);
 }
 
-- 
1.9.1

[PATCH 09/16] wil6210: add general initialization/size checks

[Maya Erez]

From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>

Initialize unset variable, and verify that mid is valid.

Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/debugfs.c | 2 ++
 drivers/net/wireless/ath/wil6210/wmi.c     | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index aa50813..099a04b 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -962,6 +962,8 @@ static ssize_t wil_write_file_txmgmt(struct file *file, const char __user *buf,
 	int rc;
 	void *frame;
 
+	memset(&params, 0, sizeof(params));
+
 	if (!len)
 		return -EINVAL;
 
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index fa08a96..f46c703 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -3148,7 +3148,7 @@ static void wmi_event_handle(struct wil6210_priv *wil,
 
 		if (mid == MID_BROADCAST)
 			mid = 0;
-		if (mid >= wil->max_vifs) {
+		if (mid >= ARRAY_SIZE(wil->vifs) || mid >= wil->max_vifs) {
 			wil_dbg_wmi(wil, "invalid mid %d, event skipped\n",
 				    mid);
 			return;
-- 
1.9.1

[PATCH 10/16] wil6210: fix debugfs memory access alignment

[Maya Erez]

From: Ahmad Masri <amasri@codeaurora.org>

All wil6210 device memory access should be 4 bytes aligned. In io
blob wil6210 did not force alignment for read function, this caused
alignment fault on some platforms.
Fixing that by accessing all 4 lower bytes and return to host the
requested data.

Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/debugfs.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 099a04b..20dd4d0 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -664,10 +664,10 @@ static ssize_t wil_read_file_ioblob(struct file *file, char __user *user_buf,
 	enum { max_count = 4096 };
 	struct wil_blob_wrapper *wil_blob = file->private_data;
 	struct wil6210_priv *wil = wil_blob->wil;
-	loff_t pos = *ppos;
+	loff_t aligned_pos, pos = *ppos;
 	size_t available = wil_blob->blob.size;
 	void *buf;
-	size_t ret;
+	size_t unaligned_bytes, aligned_count, ret;
 	int rc;
 
 	if (test_bit(wil_status_suspending, wil_blob->wil->status) ||
@@ -685,7 +685,12 @@ static ssize_t wil_read_file_ioblob(struct file *file, char __user *user_buf,
 	if (count > max_count)
 		count = max_count;
 
-	buf = kmalloc(count, GFP_KERNEL);
+	/* set pos to 4 bytes aligned */
+	unaligned_bytes = pos % 4;
+	aligned_pos = pos - unaligned_bytes;
+	aligned_count = count + unaligned_bytes;
+
+	buf = kmalloc(aligned_count, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -696,9 +701,9 @@ static ssize_t wil_read_file_ioblob(struct file *file, char __user *user_buf,
 	}
 
 	wil_memcpy_fromio_32(buf, (const void __iomem *)
-			     wil_blob->blob.data + pos, count);
+			     wil_blob->blob.data + aligned_pos, aligned_count);
 
-	ret = copy_to_user(user_buf, buf, count);
+	ret = copy_to_user(user_buf, buf + unaligned_bytes, count);
 
 	wil_pm_runtime_put(wil);
 
-- 
1.9.1

[PATCH 11/16] wil6210: fix L2 RX status handling

[Maya Erez]

L2 RX status errors should not be treated as a bitmap and the actual
error values should be checked.
Print L2 errors as wil_err_ratelimited for easier debugging
when such errors occurs.

Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/txrx_edma.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.c b/drivers/net/wireless/ath/wil6210/txrx_edma.c
index fe758f2..c8490e7 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.c
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.c
@@ -839,23 +839,24 @@ static int wil_rx_error_check_edma(struct wil6210_priv *wil,
 		wil_dbg_txrx(wil, "L2 RX error, l2_rx_status=0x%x\n",
 			     l2_rx_status);
 		/* Due to HW issue, KEY error will trigger a MIC error */
-		if (l2_rx_status & WIL_RX_EDMA_ERROR_MIC) {
-			wil_dbg_txrx(wil,
-				     "L2 MIC/KEY error, dropping packet\n");
+		if (l2_rx_status == WIL_RX_EDMA_ERROR_MIC) {
+			wil_err_ratelimited(wil,
+					    "L2 MIC/KEY error, dropping packet\n");
 			stats->rx_mic_error++;
 		}
-		if (l2_rx_status & WIL_RX_EDMA_ERROR_KEY) {
-			wil_dbg_txrx(wil, "L2 KEY error, dropping packet\n");
+		if (l2_rx_status == WIL_RX_EDMA_ERROR_KEY) {
+			wil_err_ratelimited(wil,
+					    "L2 KEY error, dropping packet\n");
 			stats->rx_key_error++;
 		}
-		if (l2_rx_status & WIL_RX_EDMA_ERROR_REPLAY) {
-			wil_dbg_txrx(wil,
-				     "L2 REPLAY error, dropping packet\n");
+		if (l2_rx_status == WIL_RX_EDMA_ERROR_REPLAY) {
+			wil_err_ratelimited(wil,
+					    "L2 REPLAY error, dropping packet\n");
 			stats->rx_replay++;
 		}
-		if (l2_rx_status & WIL_RX_EDMA_ERROR_AMSDU) {
-			wil_dbg_txrx(wil,
-				     "L2 AMSDU error, dropping packet\n");
+		if (l2_rx_status == WIL_RX_EDMA_ERROR_AMSDU) {
+			wil_err_ratelimited(wil,
+					    "L2 AMSDU error, dropping packet\n");
 			stats->rx_amsdu_error++;
 		}
 		return -EFAULT;
-- 
1.9.1

[PATCH 12/16] wil6210: fix RGF_CAF_ICR address for Talyn-MB

[Maya Erez]

RGF_CAF_ICR register location has changed in Talyn-MB.
Add RGF_CAF_ICR_TALYN_MB to support the new address.

Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/main.c    | 11 +++++++++--
 drivers/net/wireless/ath/wil6210/wil6210.h |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 9dd068d..5303867 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -1500,8 +1500,15 @@ static void wil_pre_fw_config(struct wil6210_priv *wil)
 	wil6210_clear_irq(wil);
 	/* CAF_ICR - clear and mask */
 	/* it is W1C, clear by writing back same value */
-	wil_s(wil, RGF_CAF_ICR + offsetof(struct RGF_ICR, ICR), 0);
-	wil_w(wil, RGF_CAF_ICR + offsetof(struct RGF_ICR, IMV), ~0);
+	if (wil->hw_version < HW_VER_TALYN_MB) {
+		wil_s(wil, RGF_CAF_ICR + offsetof(struct RGF_ICR, ICR), 0);
+		wil_w(wil, RGF_CAF_ICR + offsetof(struct RGF_ICR, IMV), ~0);
+	} else {
+		wil_s(wil,
+		      RGF_CAF_ICR_TALYN_MB + offsetof(struct RGF_ICR, ICR), 0);
+		wil_w(wil, RGF_CAF_ICR_TALYN_MB +
+		      offsetof(struct RGF_ICR, IMV), ~0);
+	}
 	/* clear PAL_UNIT_ICR (potential D0->D3 leftover)
 	 * In Talyn-MB host cannot access this register due to
 	 * access control, hence PAL_UNIT_ICR is cleared by the FW
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index ad7003f..0f3be3ff 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -320,6 +320,7 @@ struct RGF_ICR {
 /* MAC timer, usec, for packet lifetime */
 #define RGF_MAC_MTRL_COUNTER_0		(0x886aa8)
 
+#define RGF_CAF_ICR_TALYN_MB		(0x8893d4) /* struct RGF_ICR */
 #define RGF_CAF_ICR			(0x88946c) /* struct RGF_ICR */
 #define RGF_CAF_OSC_CONTROL		(0x88afa4)
 	#define BIT_CAF_OSC_XTAL_EN		BIT(0)
-- 
1.9.1

[PATCH 13/16] wil6210: ignore HALP ICR if already handled

[Maya Erez]

HALP ICR is set as long as the FW should stay awake.
To prevent its multiple handling the driver masks this IRQ bit.
However, if there is a different MISC ICR before the driver clears
this bit, there is a risk of race condition between HALP mask and
unmask. This race leads to HALP timeout, in case it is mistakenly
masked.
Add an atomic flag to indicate if HALP ICR should be handled.

Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/interrupt.c | 10 +++++++---
 drivers/net/wireless/ath/wil6210/main.c      |  2 ++
 drivers/net/wireless/ath/wil6210/wil6210.h   |  1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/interrupt.c b/drivers/net/wireless/ath/wil6210/interrupt.c
index 5d287a8..a58fccb 100644
--- a/drivers/net/wireless/ath/wil6210/interrupt.c
+++ b/drivers/net/wireless/ath/wil6210/interrupt.c
@@ -575,10 +575,14 @@ static irqreturn_t wil6210_irq_misc(int irq, void *cookie)
 	}
 
 	if (isr & BIT_DMA_EP_MISC_ICR_HALP) {
-		wil_dbg_irq(wil, "irq_misc: HALP IRQ invoked\n");
-		wil6210_mask_halp(wil);
 		isr &= ~BIT_DMA_EP_MISC_ICR_HALP;
-		complete(&wil->halp.comp);
+		if (atomic_read(&wil->halp.handle_icr)) {
+			/* no need to handle HALP ICRs until next vote */
+			atomic_set(&wil->halp.handle_icr, 0);
+			wil_dbg_irq(wil, "irq_misc: HALP IRQ invoked\n");
+			wil6210_mask_halp(wil);
+			complete(&wil->halp.comp);
+		}
 	}
 
 	wil->isr_misc = isr;
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 5303867..5c89c01 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -1922,6 +1922,8 @@ void wil_halp_vote(struct wil6210_priv *wil)
 
 	if (++wil->halp.ref_cnt == 1) {
 		reinit_completion(&wil->halp.comp);
+		/* mark to IRQ context to handle HALP ICR */
+		atomic_set(&wil->halp.handle_icr, 1);
 		wil6210_set_halp(wil);
 		rc = wait_for_completion_timeout(&wil->halp.comp, to_jiffies);
 		if (!rc) {
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 0f3be3ff..1897d8c 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -791,6 +791,7 @@ struct wil_halp {
 	struct mutex		lock; /* protect halp ref_cnt */
 	unsigned int		ref_cnt;
 	struct completion	comp;
+	atomic_t		handle_icr;
 };
 
 struct wil_blob_wrapper {
-- 
1.9.1

Re: [PATCH 13/16] wil6210: ignore HALP ICR if already handled

[Kalle Valo]

Maya Erez <merez@codeaurora.org> writes:

> HALP ICR is set as long as the FW should stay awake.
> To prevent its multiple handling the driver masks this IRQ bit.
> However, if there is a different MISC ICR before the driver clears
> this bit, there is a risk of race condition between HALP mask and
> unmask. This race leads to HALP timeout, in case it is mistakenly
> masked.
> Add an atomic flag to indicate if HALP ICR should be handled.
>
> Signed-off-by: Maya Erez <merez@codeaurora.org>

[...]

> --- a/drivers/net/wireless/ath/wil6210/interrupt.c
> +++ b/drivers/net/wireless/ath/wil6210/interrupt.c
> @@ -575,10 +575,14 @@ static irqreturn_t wil6210_irq_misc(int irq, void *cookie)
>  	}
>  
>  	if (isr & BIT_DMA_EP_MISC_ICR_HALP) {
> -		wil_dbg_irq(wil, "irq_misc: HALP IRQ invoked\n");
> -		wil6210_mask_halp(wil);
>  		isr &= ~BIT_DMA_EP_MISC_ICR_HALP;
> -		complete(&wil->halp.comp);
> +		if (atomic_read(&wil->halp.handle_icr)) {
> +			/* no need to handle HALP ICRs until next vote */
> +			atomic_set(&wil->halp.handle_icr, 0);

atomic_read() followed by atomic_set() is IMHO not really atomic :) I
would assume there's a function to reset the variable really in atomic
way.

-- 
Kalle Valo

Re: [PATCH 13/16] wil6210: ignore HALP ICR if already handled

[merez]

On 2018-11-06 12:04, Kalle Valo wrote:
> Maya Erez <merez@codeaurora.org> writes:
> 
>> HALP ICR is set as long as the FW should stay awake.
>> To prevent its multiple handling the driver masks this IRQ bit.
>> However, if there is a different MISC ICR before the driver clears
>> this bit, there is a risk of race condition between HALP mask and
>> unmask. This race leads to HALP timeout, in case it is mistakenly
>> masked.
>> Add an atomic flag to indicate if HALP ICR should be handled.
>> 
>> Signed-off-by: Maya Erez <merez@codeaurora.org>
> 
> [...]
> 
>> --- a/drivers/net/wireless/ath/wil6210/interrupt.c
>> +++ b/drivers/net/wireless/ath/wil6210/interrupt.c
>> @@ -575,10 +575,14 @@ static irqreturn_t wil6210_irq_misc(int irq, 
>> void *cookie)
>>  	}
>> 
>>  	if (isr & BIT_DMA_EP_MISC_ICR_HALP) {
>> -		wil_dbg_irq(wil, "irq_misc: HALP IRQ invoked\n");
>> -		wil6210_mask_halp(wil);
>>  		isr &= ~BIT_DMA_EP_MISC_ICR_HALP;
>> -		complete(&wil->halp.comp);
>> +		if (atomic_read(&wil->halp.handle_icr)) {
>> +			/* no need to handle HALP ICRs until next vote */
>> +			atomic_set(&wil->halp.handle_icr, 0);
> 
> atomic_read() followed by atomic_set() is IMHO not really atomic :) I
> would assume there's a function to reset the variable really in atomic
> way.

Actually it shouldn't be atomic :-)
I'll remove it.

-- 
Maya Erez
Qualcomm Israel, Inc. on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

[PATCH 14/16] wil6210: remove unnecessary alignment code from rx flow

[Maya Erez]

From: Ahmad Masri <amasri@codeaurora.org>

Rx buffers in EDMA mode are initialized to 4 bytes aligned size.
Remove the unnecessary alignment code applied on rx buffer size.

Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/txrx_edma.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.c b/drivers/net/wireless/ath/wil6210/txrx_edma.c
index c8490e7..686ba34 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.c
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.c
@@ -160,7 +160,7 @@ static int wil_ring_alloc_skb_edma(struct wil6210_priv *wil,
 				   struct wil_ring *ring, u32 i)
 {
 	struct device *dev = wil_to_dev(wil);
-	unsigned int sz = ALIGN(wil->rx_buf_len, 4);
+	unsigned int sz = wil->rx_buf_len;
 	dma_addr_t pa;
 	u16 buff_id;
 	struct list_head *active = &wil->rx_buff_mgmt.active;
@@ -602,6 +602,7 @@ static bool wil_is_rx_idle_edma(struct wil6210_priv *wil)
 
 static void wil_rx_buf_len_init_edma(struct wil6210_priv *wil)
 {
+	/* RX buffer size must be aligned to 4 bytes */
 	wil->rx_buf_len = rx_large_buf ?
 		WIL_MAX_ETH_MTU : WIL_EDMA_RX_BUF_LEN_DEFAULT;
 }
@@ -615,7 +616,6 @@ static int wil_rx_init_edma(struct wil6210_priv *wil, uint desc_ring_order)
 		sizeof(struct wil_rx_status_compressed) :
 		sizeof(struct wil_rx_status_extended);
 	int i;
-	u16 max_rx_pl_per_desc;
 
 	/* In SW reorder one must use extended status messages */
 	if (wil->use_compressed_rx_status && !wil->use_rx_hw_reordering) {
@@ -641,8 +641,6 @@ static int wil_rx_init_edma(struct wil6210_priv *wil, uint desc_ring_order)
 
 	wil_rx_buf_len_init_edma(wil);
 
-	max_rx_pl_per_desc = ALIGN(wil->rx_buf_len, 4);
-
 	/* Use debugfs dbg_num_rx_srings if set, reserve one sring for TX */
 	if (wil->num_rx_status_rings > WIL6210_MAX_STATUS_RINGS - 1)
 		wil->num_rx_status_rings = WIL6210_MAX_STATUS_RINGS - 1;
@@ -650,7 +648,7 @@ static int wil_rx_init_edma(struct wil6210_priv *wil, uint desc_ring_order)
 	wil_dbg_misc(wil, "rx_init: allocate %d status rings\n",
 		     wil->num_rx_status_rings);
 
-	rc = wil_wmi_cfg_def_rx_offload(wil, max_rx_pl_per_desc);
+	rc = wil_wmi_cfg_def_rx_offload(wil, wil->rx_buf_len);
 	if (rc)
 		return rc;
 
@@ -887,7 +885,7 @@ static struct sk_buff *wil_sring_reap_rx_edma(struct wil6210_priv *wil,
 	struct sk_buff *skb;
 	dma_addr_t pa;
 	struct wil_ring_rx_data *rxdata = &sring->rx_data;
-	unsigned int sz = ALIGN(wil->rx_buf_len, 4);
+	unsigned int sz = wil->rx_buf_len;
 	struct wil_net_stats *stats = NULL;
 	u16 dmalen;
 	int cid;
-- 
1.9.1

[PATCH 15/16] wil6210: fix freeing of rx buffers in EDMA mode

[Maya Erez]

From: Ahmad Masri <amasri@codeaurora.org>

After being associated with some EDMA rx traffic, upon "down" driver
doesn't free all skbs in the rx ring.
Modify wil_move_all_rx_buff_to_free_list to loop on active list of rx
buffers, unmap the physical memory and free the skb.

Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/txrx_edma.c | 44 +++++++++-------------------
 1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.c b/drivers/net/wireless/ath/wil6210/txrx_edma.c
index 686ba34..05a8348 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.c
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.c
@@ -234,9 +234,10 @@ static int wil_rx_refill_edma(struct wil6210_priv *wil)
 	struct wil_ring *ring = &wil->ring_rx;
 	u32 next_head;
 	int rc = 0;
-	u32 swtail = *ring->edma_rx_swtail.va;
+	ring->swtail = *ring->edma_rx_swtail.va;
 
-	for (; next_head = wil_ring_next_head(ring), (next_head != swtail);
+	for (; next_head = wil_ring_next_head(ring),
+	     (next_head != ring->swtail);
 	     ring->swhead = next_head) {
 		rc = wil_ring_alloc_skb_edma(wil, ring, ring->swhead);
 		if (unlikely(rc)) {
@@ -264,43 +265,26 @@ static void wil_move_all_rx_buff_to_free_list(struct wil6210_priv *wil,
 					      struct wil_ring *ring)
 {
 	struct device *dev = wil_to_dev(wil);
-	u32 next_tail;
-	u32 swhead = (ring->swhead + 1) % ring->size;
+	struct list_head *active = &wil->rx_buff_mgmt.active;
 	dma_addr_t pa;
-	u16 dmalen;
 
-	for (; next_tail = wil_ring_next_tail(ring), (next_tail != swhead);
-	     ring->swtail = next_tail) {
-		struct wil_rx_enhanced_desc dd, *d = &dd;
-		struct wil_rx_enhanced_desc *_d =
-			(struct wil_rx_enhanced_desc *)
-			&ring->va[ring->swtail].rx.enhanced;
-		struct sk_buff *skb;
-		u16 buff_id;
+	while (!list_empty(active)) {
+		struct wil_rx_buff *rx_buff =
+			list_first_entry(active, struct wil_rx_buff, list);
+		struct sk_buff *skb = rx_buff->skb;
 
-		*d = *_d;
-
-		/* Extract the SKB from the rx_buff management array */
-		buff_id = __le16_to_cpu(d->mac.buff_id);
-		if (buff_id >= wil->rx_buff_mgmt.size) {
-			wil_err(wil, "invalid buff_id %d\n", buff_id);
-			continue;
-		}
-		skb = wil->rx_buff_mgmt.buff_arr[buff_id].skb;
-		wil->rx_buff_mgmt.buff_arr[buff_id].skb = NULL;
 		if (unlikely(!skb)) {
-			wil_err(wil, "No Rx skb at buff_id %d\n", buff_id);
+			wil_err(wil, "No Rx skb at buff_id %d\n", rx_buff->id);
 		} else {
-			pa = wil_rx_desc_get_addr_edma(&d->dma);
-			dmalen = le16_to_cpu(d->dma.length);
-			dma_unmap_single(dev, pa, dmalen, DMA_FROM_DEVICE);
-
+			rx_buff->skb = NULL;
+			memcpy(&pa, skb->cb, sizeof(pa));
+			dma_unmap_single(dev, pa, wil->rx_buf_len,
+					 DMA_FROM_DEVICE);
 			kfree_skb(skb);
 		}
 
 		/* Move the buffer from the active to the free list */
-		list_move(&wil->rx_buff_mgmt.buff_arr[buff_id].list,
-			  &wil->rx_buff_mgmt.free);
+		list_move(&rx_buff->list, &wil->rx_buff_mgmt.free);
 	}
 }
 
-- 
1.9.1

[PATCH 16/16] wil6210: fix locking in wmi_call

[Maya Erez]

From: Lior David <liord@codeaurora.org>

Switch from spin_lock to spin_lock_irqsave, because
wmi_ev_lock is used inside interrupt handler.

Signed-off-by: Lior David <liord@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
 drivers/net/wireless/ath/wil6210/wmi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index f46c703..345f059 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -1966,16 +1966,17 @@ int wmi_call(struct wil6210_priv *wil, u16 cmdid, u8 mid, void *buf, u16 len,
 {
 	int rc;
 	unsigned long remain;
+	ulong flags;
 
 	mutex_lock(&wil->wmi_mutex);
 
-	spin_lock(&wil->wmi_ev_lock);
+	spin_lock_irqsave(&wil->wmi_ev_lock, flags);
 	wil->reply_id = reply_id;
 	wil->reply_mid = mid;
 	wil->reply_buf = reply;
 	wil->reply_size = reply_size;
 	reinit_completion(&wil->wmi_call);
-	spin_unlock(&wil->wmi_ev_lock);
+	spin_unlock_irqrestore(&wil->wmi_ev_lock, flags);
 
 	rc = __wmi_send(wil, cmdid, mid, buf, len);
 	if (rc)
@@ -1995,12 +1996,12 @@ int wmi_call(struct wil6210_priv *wil, u16 cmdid, u8 mid, void *buf, u16 len,
 	}
 
 out:
-	spin_lock(&wil->wmi_ev_lock);
+	spin_lock_irqsave(&wil->wmi_ev_lock, flags);
 	wil->reply_id = 0;
 	wil->reply_mid = U8_MAX;
 	wil->reply_buf = NULL;
 	wil->reply_size = 0;
-	spin_unlock(&wil->wmi_ev_lock);
+	spin_unlock_irqrestore(&wil->wmi_ev_lock, flags);
 
 	mutex_unlock(&wil->wmi_mutex);
 
-- 
1.9.1