DriverDev-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 00/19] staging: wfx: various fixes
@ 2020-05-15  8:33 Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 01/19] staging: wfx: fix warning when unregister a frozen device Jerome Pouiller
                   ` (18 more replies)
  0 siblings, 19 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

Hello,

This series contains various changes. The most important patches are the
13 and 14 since they fix two functional defects. The other patches fix
runtime warnings (1, 17, 18, 19), improve robustness (3, 4, 5, 7, 10, 16)
and do some cosmetics improvements (2, 6, 8, 9, 11, 12, 15).

This series have to be applied on top of patch sent by Dan: "staging: wfx:
unlock on error path". Would I had include this patch in this PR?

Jérôme Pouiller (19):
  staging: wfx: fix warning when unregister a frozen device
  staging: wfx: apply 80-columns rule to strings
  staging: wfx: check pointers returned by allocations
  staging: wfx: fix value of scan timeout
  staging: wfx: fix coherency of hif_scan() prototype
  staging: wfx: fix indentation
  staging: wfx: fix status of dropped frames
  staging: wfx: split out wfx_tx_fill_rates() from wfx_tx_confirm_cb()
  staging: wfx: call wfx_tx_update_sta() before to destroy tx_priv
  staging: wfx: fix potential use-after-free
  staging: wfx: rename wfx_do_unjoin() into wfx_reset()
  staging: wfx: merge wfx_stop_ap() with wfx_reset()
  staging: wfx: fix potential dead lock between join and scan
  staging: wfx: fix PS parameters when multiple vif are in use
  staging: wfx: drop unnecessary filter configuration when disabling
    filter
  staging: wfx: fix error reporting in wfx_start_ap()
  staging: wfx: remove false-positive WARN()
  staging: wfx: trace acknowledges not linked to any stations
  staging: wfx: remove false positive warning

 drivers/staging/wfx/bus_sdio.c   |   3 +-
 drivers/staging/wfx/data_tx.c    | 110 +++++++++++++++++--------------
 drivers/staging/wfx/fwio.c       |   8 +--
 drivers/staging/wfx/hif_tx.c     |  57 ++++++++++++++--
 drivers/staging/wfx/hif_tx.h     |   2 +-
 drivers/staging/wfx/hif_tx_mib.c |   2 +
 drivers/staging/wfx/main.c       |  17 +++--
 drivers/staging/wfx/queue.c      |   7 --
 drivers/staging/wfx/scan.c       |  11 +++-
 drivers/staging/wfx/sta.c        |  69 ++++++++++---------
 drivers/staging/wfx/sta.h        |   1 +
 drivers/staging/wfx/wfx.h        |   2 +
 12 files changed, 182 insertions(+), 107 deletions(-)

-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 01/19] staging: wfx: fix warning when unregister a frozen device
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 02/19] staging: wfx: apply 80-columns rule to strings Jerome Pouiller
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

The device does not answer to the command hif_shutdown. Therefore,
hif_shutdown() is a bit special. It bypasses some of work normally made
by wfx_cmd_send(). In particularly, it unlock hif_cmd.lock and
hif_cmd.key_renew_lock.

However, if the driver notice that the device is frozen, wfx_cmd_send()
stops to send data and doesn't lock the mutexes. Then, it produced a
warning when hif_shutdown() tried to unlock these mutexes.

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

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 7f459719e7b4..3e5d9111e855 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -131,6 +131,8 @@ int hif_shutdown(struct wfx_dev *wdev)
 	int ret;
 	struct hif_msg *hif;
 
+	if (wdev->chip_frozen)
+		return 0;
 	wfx_alloc_hif(0, &hif);
 	wfx_fill_header(hif, -1, HIF_REQ_ID_SHUT_DOWN, 0);
 	ret = wfx_cmd_send(wdev, hif, NULL, 0, true);
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 02/19] staging: wfx: apply 80-columns rule to strings
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 01/19] staging: wfx: fix warning when unregister a frozen device Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 03/19] staging: wfx: check pointers returned by allocations Jerome Pouiller
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

Strings are allowed to exceed 80 columns but, in this case, the format
arguments should be placed on a new line. Apply this rule to the whole
code of the driver.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/bus_sdio.c |  3 ++-
 drivers/staging/wfx/data_tx.c  |  4 ++--
 drivers/staging/wfx/fwio.c     |  8 ++++----
 drivers/staging/wfx/hif_tx.c   |  4 ++--
 drivers/staging/wfx/main.c     | 17 ++++++++++-------
 5 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index 6464b2c508e4..496bfc8bbacc 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -180,7 +180,8 @@ static int wfx_sdio_probe(struct sdio_func *func,
 	int ret;
 
 	if (func->num != 1) {
-		dev_err(&func->dev, "SDIO function number is %d while it should always be 1 (unsupported chip?)\n", func->num);
+		dev_err(&func->dev, "SDIO function number is %d while it should always be 1 (unsupported chip?)\n",
+			func->num);
 		return -ENODEV;
 	}
 
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 4a2910897b6f..cac8c9ecbc34 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -550,8 +550,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 		}
 	}
 	if (tx_count)
-		dev_dbg(wvif->wdev->dev,
-			"%d more retries than expected\n", tx_count);
+		dev_dbg(wvif->wdev->dev, "%d more retries than expected\n",
+			tx_count);
 	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
diff --git a/drivers/staging/wfx/fwio.c b/drivers/staging/wfx/fwio.c
index 85b6a916a7d0..72bb3d2a9613 100644
--- a/drivers/staging/wfx/fwio.c
+++ b/drivers/staging/wfx/fwio.c
@@ -107,8 +107,8 @@ static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip,
 	const char *data;
 	int ret;
 
-	snprintf(filename, sizeof(filename), "%s_%02X.sec", wdev->pdata.file_fw,
-		 keyset_chip);
+	snprintf(filename, sizeof(filename), "%s_%02X.sec",
+		 wdev->pdata.file_fw, keyset_chip);
 	ret = firmware_request_nowarn(fw, filename, wdev->dev);
 	if (ret) {
 		dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n",
@@ -325,8 +325,8 @@ static int init_gpr(struct wfx_dev *wdev)
 				     gpr_init[i].value);
 		if (ret < 0)
 			return ret;
-		dev_dbg(wdev->dev, "  index %02x: %08x\n", gpr_init[i].index,
-			gpr_init[i].value);
+		dev_dbg(wdev->dev, "  index %02x: %08x\n",
+			gpr_init[i].index, gpr_init[i].value);
 	}
 	return 0;
 }
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 3e5d9111e855..58adfaf8066d 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -192,8 +192,8 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 	ret = wfx_cmd_send(wdev, hif, reply, buf_len, false);
 
 	if (!ret && mib_id != le16_to_cpu(reply->mib_id)) {
-		dev_warn(wdev->dev,
-			 "%s: confirmation mismatch request\n", __func__);
+		dev_warn(wdev->dev, "%s: confirmation mismatch request\n",
+			 __func__);
 		ret = -EIO;
 	}
 	if (ret == -ENOMEM)
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index d4e69c663f5a..ae23a56f50e0 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -192,12 +192,12 @@ struct gpio_desc *wfx_get_gpio(struct device *dev,
 		if (!ret || PTR_ERR(ret) == -ENOENT)
 			dev_warn(dev, "gpio %s is not defined\n", label);
 		else
-			dev_warn(dev,
-				 "error while requesting gpio %s\n", label);
+			dev_warn(dev, "error while requesting gpio %s\n",
+				 label);
 		ret = NULL;
 	} else {
-		dev_dbg(dev,
-			"using gpio %d for %s\n", desc_to_gpio(ret), label);
+		dev_dbg(dev, "using gpio %d for %s\n",
+			desc_to_gpio(ret), label);
 	}
 	return ret;
 }
@@ -230,15 +230,18 @@ int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len)
 			ret = hif_configuration(wdev, buf + start,
 						i - start + 1);
 			if (ret > 0) {
-				dev_err(wdev->dev, "PDS bytes %d to %d: invalid data (unsupported options?)\n", start, i);
+				dev_err(wdev->dev, "PDS bytes %d to %d: invalid data (unsupported options?)\n",
+					start, i);
 				return -EINVAL;
 			}
 			if (ret == -ETIMEDOUT) {
-				dev_err(wdev->dev, "PDS bytes %d to %d: chip didn't reply (corrupted file?)\n", start, i);
+				dev_err(wdev->dev, "PDS bytes %d to %d: chip didn't reply (corrupted file?)\n",
+					start, i);
 				return ret;
 			}
 			if (ret) {
-				dev_err(wdev->dev, "PDS bytes %d to %d: chip returned an unknown error\n", start, i);
+				dev_err(wdev->dev, "PDS bytes %d to %d: chip returned an unknown error\n",
+					start, i);
 				return -EIO;
 			}
 			buf[i] = ',';
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 03/19] staging: wfx: check pointers returned by allocations
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 01/19] staging: wfx: fix warning when unregister a frozen device Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 02/19] staging: wfx: apply 80-columns rule to strings Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 04/19] staging: wfx: fix value of scan timeout Jerome Pouiller
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Dan Carpenter, Greg Kroah-Hartman,
	David S . Miller, Kalle Valo

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

Until now, the driver did not always check if the allocations success.

The issue was discussed here:
   https://lore.kernel.org/netdev/2026476.QLiXXEGFCf@pc-42/

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_tx.c     | 43 ++++++++++++++++++++++++++++++++
 drivers/staging/wfx/hif_tx_mib.c |  2 ++
 2 files changed, 45 insertions(+)

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 58adfaf8066d..1cb71f0ad804 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -134,6 +134,8 @@ int hif_shutdown(struct wfx_dev *wdev)
 	if (wdev->chip_frozen)
 		return 0;
 	wfx_alloc_hif(0, &hif);
+	if (!hif)
+		return -ENOMEM;
 	wfx_fill_header(hif, -1, HIF_REQ_ID_SHUT_DOWN, 0);
 	ret = wfx_cmd_send(wdev, hif, NULL, 0, true);
 	// After this command, chip won't reply. Be sure to give enough time to
@@ -157,6 +159,8 @@ int hif_configuration(struct wfx_dev *wdev, const u8 *conf, size_t len)
 	struct hif_msg *hif;
 	struct hif_req_configuration *body = wfx_alloc_hif(buf_len, &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->length = cpu_to_le16(len);
 	memcpy(body->pds_data, conf, len);
 	wfx_fill_header(hif, -1, HIF_REQ_ID_CONFIGURATION, buf_len);
@@ -171,6 +175,8 @@ int hif_reset(struct wfx_vif *wvif, bool reset_stat)
 	struct hif_msg *hif;
 	struct hif_req_reset *body = wfx_alloc_hif(sizeof(*body), &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->reset_flags.reset_stat = reset_stat;
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_RESET, sizeof(*body));
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
@@ -187,6 +193,10 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 	struct hif_req_read_mib *body = wfx_alloc_hif(sizeof(*body), &hif);
 	struct hif_cnf_read_mib *reply = kmalloc(buf_len, GFP_KERNEL);
 
+	if (!body || !reply) {
+		ret = -ENOMEM;
+		goto out;
+	}
 	body->mib_id = cpu_to_le16(mib_id);
 	wfx_fill_header(hif, vif_id, HIF_REQ_ID_READ_MIB, sizeof(*body));
 	ret = wfx_cmd_send(wdev, hif, reply, buf_len, false);
@@ -204,6 +214,7 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 		memcpy(val, &reply->mib_data, le16_to_cpu(reply->length));
 	else
 		memset(val, 0xFF, val_len);
+out:
 	kfree(hif);
 	kfree(reply);
 	return ret;
@@ -217,6 +228,8 @@ int hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 	int buf_len = sizeof(struct hif_req_write_mib) + val_len;
 	struct hif_req_write_mib *body = wfx_alloc_hif(buf_len, &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->mib_id = cpu_to_le16(mib_id);
 	body->length = cpu_to_le16(val_len);
 	memcpy(&body->mib_data, val, val_len);
@@ -241,6 +254,8 @@ int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
 
 	compiletime_assert(IEEE80211_MAX_SSID_LEN == HIF_API_SSID_SIZE,
 			   "API inconsistency");
+	if (!hif)
+		return -ENOMEM;
 	for (i = 0; i < req->n_ssids; i++) {
 		memcpy(body->ssid_def[i].ssid, req->ssids[i].ssid,
 		       IEEE80211_MAX_SSID_LEN);
@@ -288,6 +303,8 @@ int hif_stop_scan(struct wfx_vif *wvif)
 	// body associated to HIF_REQ_ID_STOP_SCAN is empty
 	wfx_alloc_hif(0, &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_STOP_SCAN, 0);
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
 	kfree(hif);
@@ -305,6 +322,8 @@ int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 	WARN_ON(!conf->basic_rates);
 	WARN_ON(sizeof(body->ssid) < ssidlen);
 	WARN(!conf->ibss_joined && !ssidlen, "joining an unknown BSS");
+	if (!hif)
+		return -ENOMEM;
 	body->infrastructure_bss_mode = !conf->ibss_joined;
 	body->short_preamble = conf->use_short_preamble;
 	if (channel && channel->flags & IEEE80211_CHAN_NO_IR)
@@ -333,6 +352,8 @@ int hif_set_bss_params(struct wfx_vif *wvif, int aid, int beacon_lost_count)
 	struct hif_req_set_bss_params *body =
 		wfx_alloc_hif(sizeof(*body), &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->aid = cpu_to_le16(aid);
 	body->beacon_lost_count = beacon_lost_count;
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_SET_BSS_PARAMS,
@@ -349,6 +370,8 @@ int hif_add_key(struct wfx_dev *wdev, const struct hif_req_add_key *arg)
 	// FIXME: only send necessary bits
 	struct hif_req_add_key *body = wfx_alloc_hif(sizeof(*body), &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	// FIXME: swap bytes as necessary in body
 	memcpy(body, arg, sizeof(*body));
 	if (wfx_api_older_than(wdev, 1, 5))
@@ -369,6 +392,8 @@ int hif_remove_key(struct wfx_dev *wdev, int idx)
 	struct hif_msg *hif;
 	struct hif_req_remove_key *body = wfx_alloc_hif(sizeof(*body), &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->entry_index = idx;
 	wfx_fill_header(hif, -1, HIF_REQ_ID_REMOVE_KEY, sizeof(*body));
 	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
@@ -388,6 +413,8 @@ int hif_set_edca_queue_params(struct wfx_vif *wvif, u16 queue,
 		return -ENOMEM;
 
 	WARN_ON(arg->aifs > 255);
+	if (!hif)
+		return -ENOMEM;
 	body->aifsn = arg->aifs;
 	body->cw_min = cpu_to_le16(arg->cw_min);
 	body->cw_max = cpu_to_le16(arg->cw_max);
@@ -414,6 +441,8 @@ int hif_set_pm(struct wfx_vif *wvif, bool ps, int dynamic_ps_timeout)
 	if (!body)
 		return -ENOMEM;
 
+	if (!hif)
+		return -ENOMEM;
 	if (ps) {
 		body->pm_mode.enter_psm = 1;
 		// Firmware does not support more than 128ms
@@ -435,6 +464,8 @@ int hif_start(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 	struct hif_req_start *body = wfx_alloc_hif(sizeof(*body), &hif);
 
 	WARN_ON(!conf->beacon_int);
+	if (!hif)
+		return -ENOMEM;
 	body->dtim_period = conf->dtim_period;
 	body->short_preamble = conf->use_short_preamble;
 	body->channel_number = channel->hw_value;
@@ -456,6 +487,8 @@ int hif_beacon_transmit(struct wfx_vif *wvif, bool enable)
 	struct hif_req_beacon_transmit *body = wfx_alloc_hif(sizeof(*body),
 							     &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->enable_beaconing = enable ? 1 : 0;
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_BEACON_TRANSMIT,
 			sizeof(*body));
@@ -470,6 +503,8 @@ int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id)
 	struct hif_msg *hif;
 	struct hif_req_map_link *body = wfx_alloc_hif(sizeof(*body), &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	if (mac_addr)
 		ether_addr_copy(body->mac_addr, mac_addr);
 	body->map_link_flags = *(struct hif_map_link_flags *)&flags;
@@ -487,6 +522,8 @@ int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
 	int buf_len = sizeof(struct hif_req_update_ie) + ies_len;
 	struct hif_req_update_ie *body = wfx_alloc_hif(buf_len, &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->ie_flags.beacon = 1;
 	body->num_ies = cpu_to_le16(1);
 	memcpy(body->ie, ies, ies_len);
@@ -504,6 +541,8 @@ int hif_sl_send_pub_keys(struct wfx_dev *wdev,
 	struct hif_req_sl_exchange_pub_keys *body = wfx_alloc_hif(sizeof(*body),
 								  &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	body->algorithm = HIF_SL_CURVE25519;
 	memcpy(body->host_pub_key, pubkey, sizeof(body->host_pub_key));
 	memcpy(body->host_pub_key_mac, pubkey_hmac,
@@ -524,6 +563,8 @@ int hif_sl_config(struct wfx_dev *wdev, const unsigned long *bitmap)
 	struct hif_msg *hif;
 	struct hif_req_sl_configure *body = wfx_alloc_hif(sizeof(*body), &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	memcpy(body->encr_bmp, bitmap, sizeof(body->encr_bmp));
 	wfx_fill_header(hif, -1, HIF_REQ_ID_SL_CONFIGURE, sizeof(*body));
 	ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
@@ -538,6 +579,8 @@ int hif_sl_set_mac_key(struct wfx_dev *wdev, const u8 *slk_key, int destination)
 	struct hif_req_set_sl_mac_key *body = wfx_alloc_hif(sizeof(*body),
 							    &hif);
 
+	if (!hif)
+		return -ENOMEM;
 	memcpy(body->key_value, slk_key, sizeof(body->key_value));
 	body->otp_or_ram = destination;
 	wfx_fill_header(hif, -1, HIF_REQ_ID_SET_SL_MAC_KEY, sizeof(*body));
diff --git a/drivers/staging/wfx/hif_tx_mib.c b/drivers/staging/wfx/hif_tx_mib.c
index 567c61d1fe2e..1689cb42acc0 100644
--- a/drivers/staging/wfx/hif_tx_mib.c
+++ b/drivers/staging/wfx/hif_tx_mib.c
@@ -222,6 +222,8 @@ int hif_set_tx_rate_retry_policy(struct wfx_vif *wvif,
 	int ret;
 
 	arg = kzalloc(size, GFP_KERNEL);
+	if (!arg)
+		return -ENOMEM;
 	arg->num_tx_rate_policies = 1;
 	arg->tx_rate_retry_policy[0].policy_index = policy_index;
 	arg->tx_rate_retry_policy[0].short_retry_count = 255;
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 04/19] staging: wfx: fix value of scan timeout
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (2 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 03/19] staging: wfx: check pointers returned by allocations Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype Jerome Pouiller
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

Before to start the scan request, the firmware signals (with a null
frame) to the AP it won't be able to receive data. This frame can be
long to send: up to 512TU. The current calculus of the scan timeout does
not take into account this delay.

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

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 1cb71f0ad804..893b67f2f792 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -288,7 +288,7 @@ int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
 	tmo_chan_bg = le32_to_cpu(body->max_channel_time) * USEC_PER_TU;
 	tmo_chan_fg = 512 * USEC_PER_TU + body->probe_delay;
 	tmo_chan_fg *= body->num_of_probe_requests;
-	tmo = chan_num * max(tmo_chan_bg, tmo_chan_fg);
+	tmo = chan_num * max(tmo_chan_bg, tmo_chan_fg) + 512 * USEC_PER_TU;
 
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_START_SCAN, buf_len);
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (3 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 04/19] staging: wfx: fix value of scan timeout Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15 13:53   ` Greg Kroah-Hartman
  2020-05-15  8:33 ` [PATCH 06/19] staging: wfx: fix indentation Jerome Pouiller
                   ` (13 subsequent siblings)
  18 siblings, 1 reply; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

The function hif_scan() return the timeout for the completion of the
scan request. It is the only function from hif_tx.c that return another
thing than just an error code. This behavior is not coherent with the
rest of file. Worse, if value returned is positive, the caller can't
make say if it is a timeout or the value returned by the hardware.

Uniformize API with other HIF functions, only return the error code and
pass timeout with parameters.

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

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 893b67f2f792..6db41587cc7a 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -240,7 +240,7 @@ int hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 }
 
 int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
-	     int chan_start_idx, int chan_num)
+	     int chan_start_idx, int chan_num, int *timeout)
 {
 	int ret, i;
 	struct hif_msg *hif;
@@ -289,11 +289,13 @@ int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
 	tmo_chan_fg = 512 * USEC_PER_TU + body->probe_delay;
 	tmo_chan_fg *= body->num_of_probe_requests;
 	tmo = chan_num * max(tmo_chan_bg, tmo_chan_fg) + 512 * USEC_PER_TU;
+	if (*timeout)
+		*timeout = usecs_to_jiffies(tmo);
 
 	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_START_SCAN, buf_len);
 	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
 	kfree(hif);
-	return ret ? ret : usecs_to_jiffies(tmo);
+	return ret;
 }
 
 int hif_stop_scan(struct wfx_vif *wvif)
diff --git a/drivers/staging/wfx/hif_tx.h b/drivers/staging/wfx/hif_tx.h
index e9eca9330178..e1da28aef706 100644
--- a/drivers/staging/wfx/hif_tx.h
+++ b/drivers/staging/wfx/hif_tx.h
@@ -42,7 +42,7 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 int hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
 		  void *buf, size_t buf_size);
 int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req80211,
-	     int chan_start, int chan_num);
+	     int chan_start, int chan_num, int *timeout);
 int hif_stop_scan(struct wfx_vif *wvif);
 int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
 	     struct ieee80211_channel *channel, const u8 *ssid, int ssidlen);
diff --git a/drivers/staging/wfx/scan.c b/drivers/staging/wfx/scan.c
index eff1be9fb28f..bf7ddc75c7db 100644
--- a/drivers/staging/wfx/scan.c
+++ b/drivers/staging/wfx/scan.c
@@ -56,10 +56,10 @@ static int send_scan_req(struct wfx_vif *wvif,
 	wfx_tx_lock_flush(wvif->wdev);
 	wvif->scan_abort = false;
 	reinit_completion(&wvif->scan_complete);
-	timeout = hif_scan(wvif, req, start_idx, i - start_idx);
-	if (timeout < 0) {
+	ret = hif_scan(wvif, req, start_idx, i - start_idx, &timeout);
+	if (ret) {
 		wfx_tx_unlock(wvif->wdev);
-		return timeout;
+		return -EIO;
 	}
 	ret = wait_for_completion_timeout(&wvif->scan_complete, timeout);
 	if (req->channels[start_idx]->max_power != wvif->vif->bss_conf.txpower)
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 06/19] staging: wfx: fix indentation
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (4 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 07/19] staging: wfx: fix status of dropped frames Jerome Pouiller
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

Fix indention of wfx_skb_dtor().

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index cac8c9ecbc34..a12590214a5d 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -484,9 +484,9 @@ static void wfx_skb_dtor(struct wfx_dev *wdev,
 	struct hif_msg *hif = (struct hif_msg *)skb->data;
 	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
 	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
-	unsigned int offset = sizeof(struct hif_req_tx) +
-				sizeof(struct hif_msg) +
-				req->data_flags.fc_offset;
+	unsigned int offset = sizeof(struct hif_msg) +
+			      sizeof(struct hif_req_tx) +
+			      req->data_flags.fc_offset;
 
 	WARN_ON(!wvif);
 	skb_pull(skb, offset);
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 07/19] staging: wfx: fix status of dropped frames
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (5 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 06/19] staging: wfx: fix indentation Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 08/19] staging: wfx: split out wfx_tx_fill_rates() from wfx_tx_confirm_cb() Jerome Pouiller
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

When wfx_flush() is called, the status of pending frames are reported to
mac80211 with random status. mac80211 probably won't interpret this
status in this case, but it is cleaner to return a correctly initialized
status.

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index a12590214a5d..5d029b0718e9 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -613,6 +613,7 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		wfx_pending_drop(wdev, &dropped);
 	while ((skb = skb_dequeue(&dropped)) != NULL) {
 		tx_priv = wfx_skb_tx_priv(skb);
+		ieee80211_tx_info_clear_status(IEEE80211_SKB_CB(skb));
 		wfx_skb_dtor(wdev, skb, tx_priv->has_sta);
 	}
 }
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 08/19] staging: wfx: split out wfx_tx_fill_rates() from wfx_tx_confirm_cb()
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (6 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 07/19] staging: wfx: fix status of dropped frames Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 09/19] staging: wfx: call wfx_tx_update_sta() before to destroy tx_priv Jerome Pouiller
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

wfx_tx_confirm_cb() is a big function. A big part of its body aims to
fill the rates list. So, create a new function wfx_tx_fill_rates() and
make wfx_tx_confirm_cb() smaller.

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 5d029b0718e9..2ba3b5c3d1a7 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -496,30 +496,14 @@ static void wfx_skb_dtor(struct wfx_dev *wdev,
 	ieee80211_tx_status_irqsafe(wdev->hw, skb);
 }
 
-void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
+static void wfx_tx_fill_rates(struct wfx_dev *wdev,
+			      struct ieee80211_tx_info *tx_info,
+			      const struct hif_cnf_tx *arg)
 {
-	int i;
-	int tx_count;
-	struct sk_buff *skb;
 	struct ieee80211_tx_rate *rate;
-	struct ieee80211_tx_info *tx_info;
-	const struct wfx_tx_priv *tx_priv;
-	bool has_sta;
+	int tx_count;
+	int i;
 
-	skb = wfx_pending_get(wvif->wdev, arg->packet_id);
-	if (!skb) {
-		dev_warn(wvif->wdev->dev,
-			 "received unknown packet_id (%#.8x) from chip\n",
-			 arg->packet_id);
-		return;
-	}
-	tx_info = IEEE80211_SKB_CB(skb);
-	tx_priv = wfx_skb_tx_priv(skb);
-	has_sta = tx_priv->has_sta;
-	_trace_tx_stats(arg, skb,
-			wfx_pending_get_pkt_us_delay(wvif->wdev, skb));
-
-	// You can touch to tx_priv, but don't touch to tx_info->status.
 	tx_count = arg->ack_failures;
 	if (!arg->status || arg->ack_failures)
 		tx_count += 1; // Also report success
@@ -530,15 +514,12 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 		if (tx_count < rate->count &&
 		    arg->status == HIF_STATUS_TX_FAIL_RETRIES &&
 		    arg->ack_failures)
-			dev_dbg(wvif->wdev->dev,
-				"all retries were not consumed: %d != %d\n",
+			dev_dbg(wdev->dev, "all retries were not consumed: %d != %d\n",
 				rate->count, tx_count);
 		if (tx_count <= rate->count && tx_count &&
-		    arg->txed_rate != wfx_get_hw_rate(wvif->wdev, rate))
-			dev_dbg(wvif->wdev->dev,
-				"inconsistent tx_info rates: %d != %d\n",
-				arg->txed_rate,
-				wfx_get_hw_rate(wvif->wdev, rate));
+		    arg->txed_rate != wfx_get_hw_rate(wdev, rate))
+			dev_dbg(wdev->dev, "inconsistent tx_info rates: %d != %d\n",
+				arg->txed_rate, wfx_get_hw_rate(wdev, rate));
 		if (tx_count > rate->count) {
 			tx_count -= rate->count;
 		} else if (!tx_count) {
@@ -550,8 +531,30 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 		}
 	}
 	if (tx_count)
-		dev_dbg(wvif->wdev->dev, "%d more retries than expected\n",
-			tx_count);
+		dev_dbg(wdev->dev, "%d more retries than expected\n", tx_count);
+}
+
+void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
+{
+	struct ieee80211_tx_info *tx_info;
+	const struct wfx_tx_priv *tx_priv;
+	struct sk_buff *skb;
+	bool has_sta;
+
+	skb = wfx_pending_get(wvif->wdev, arg->packet_id);
+	if (!skb) {
+		dev_warn(wvif->wdev->dev, "received unknown packet_id (%#.8x) from chip\n",
+			 arg->packet_id);
+		return;
+	}
+	tx_info = IEEE80211_SKB_CB(skb);
+	tx_priv = wfx_skb_tx_priv(skb);
+	has_sta = tx_priv->has_sta;
+	_trace_tx_stats(arg, skb,
+			wfx_pending_get_pkt_us_delay(wvif->wdev, skb));
+
+	// You can touch to tx_priv, but don't touch to tx_info->status.
+	wfx_tx_fill_rates(wvif->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
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 09/19] staging: wfx: call wfx_tx_update_sta() before to destroy tx_priv
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (7 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 08/19] staging: wfx: split out wfx_tx_fill_rates() from wfx_tx_confirm_cb() Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 10/19] staging: wfx: fix potential use-after-free Jerome Pouiller
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

The function wfx_notify_buffered_tx() need to know if the frame was
associated to a station. This information is available in the Control
Buffer (CB) of the skb. However, when wfx_notify_buffered_tx() is
called, the CB is no more available. Thus, the caller has to take care
of this information.

wfx_notify_buffered_tx() is a specific case. All the other function are
called before the destruction of the CB. So, this patch align the API of
wfx_notify_buffered_tx() with the other functions. Call it before the CB
was destroyed and drop the extra argument 'has_sta'.

It is also the right time to rename it into wfx_tx_update_sta() (which
is closer to the behavior of the function).

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 2ba3b5c3d1a7..314cc2743a2b 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -457,12 +457,19 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	ieee80211_tx_status_irqsafe(wdev->hw, skb);
 }
 
-static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb)
+static struct ieee80211_hdr *wfx_skb_hdr80211(struct sk_buff *skb)
 {
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	struct hif_msg *hif = (struct hif_msg *)skb->data;
+	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
+
+	return (struct ieee80211_hdr *)(req->frame + req->data_flags.fc_offset);
+}
+
+static void wfx_tx_update_sta(struct wfx_vif *wvif, struct ieee80211_hdr *hdr)
+{
+	int tid = ieee80211_get_tid(hdr);
+	struct wfx_sta_priv *sta_priv;
 	struct ieee80211_sta *sta;
-	struct wfx_sta_priv *sta_priv;
-	int tid = ieee80211_get_tid(hdr);
 
 	rcu_read_lock(); // protect sta
 	sta = ieee80211_find_sta(wvif->vif, hdr->addr1);
@@ -478,22 +485,18 @@ static void wfx_notify_buffered_tx(struct wfx_vif *wvif, struct sk_buff *skb)
 	rcu_read_unlock();
 }
 
-static void wfx_skb_dtor(struct wfx_dev *wdev,
-			 struct sk_buff *skb, bool has_sta)
+static void wfx_skb_dtor(struct wfx_vif *wvif, struct sk_buff *skb)
 {
 	struct hif_msg *hif = (struct hif_msg *)skb->data;
 	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
-	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
 	unsigned int offset = sizeof(struct hif_msg) +
 			      sizeof(struct hif_req_tx) +
 			      req->data_flags.fc_offset;
 
 	WARN_ON(!wvif);
 	skb_pull(skb, offset);
-	if (has_sta)
-		wfx_notify_buffered_tx(wvif, skb);
 	wfx_tx_policy_put(wvif, req->tx_flags.retry_policy_index);
-	ieee80211_tx_status_irqsafe(wdev->hw, skb);
+	ieee80211_tx_status_irqsafe(wvif->wdev->hw, skb);
 }
 
 static void wfx_tx_fill_rates(struct wfx_dev *wdev,
@@ -539,7 +542,6 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 	struct ieee80211_tx_info *tx_info;
 	const struct wfx_tx_priv *tx_priv;
 	struct sk_buff *skb;
-	bool has_sta;
 
 	skb = wfx_pending_get(wvif->wdev, arg->packet_id);
 	if (!skb) {
@@ -549,12 +551,13 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 	}
 	tx_info = IEEE80211_SKB_CB(skb);
 	tx_priv = wfx_skb_tx_priv(skb);
-	has_sta = tx_priv->has_sta;
 	_trace_tx_stats(arg, skb,
 			wfx_pending_get_pkt_us_delay(wvif->wdev, skb));
 
 	// You can touch to tx_priv, but don't touch to tx_info->status.
 	wfx_tx_fill_rates(wvif->wdev, tx_info, arg);
+	if (tx_priv->has_sta)
+		wfx_tx_update_sta(wvif, wfx_skb_hdr80211(skb));
 	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
@@ -580,16 +583,17 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 		}
 		tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
 	}
-	wfx_skb_dtor(wvif->wdev, skb, has_sta);
+	wfx_skb_dtor(wvif, skb);
 }
 
 void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	       u32 queues, bool drop)
 {
-	const struct wfx_tx_priv *tx_priv;
 	struct wfx_dev *wdev = hw->priv;
 	struct sk_buff_head dropped;
 	struct wfx_queue *queue;
+	struct wfx_vif *wvif;
+	struct hif_msg *hif;
 	struct sk_buff *skb;
 	int vif_id = -1;
 	int i;
@@ -615,9 +619,12 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (wdev->chip_frozen)
 		wfx_pending_drop(wdev, &dropped);
 	while ((skb = skb_dequeue(&dropped)) != NULL) {
-		tx_priv = wfx_skb_tx_priv(skb);
+		hif = (struct hif_msg *)skb->data;
+		wvif = wdev_to_wvif(wdev, hif->interface);
+		if (wfx_skb_tx_priv(skb)->has_sta)
+			wfx_tx_update_sta(wvif, wfx_skb_hdr80211(skb));
 		ieee80211_tx_info_clear_status(IEEE80211_SKB_CB(skb));
-		wfx_skb_dtor(wdev, skb, tx_priv->has_sta);
+		wfx_skb_dtor(wvif, skb);
 	}
 }
 
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 10/19] staging: wfx: fix potential use-after-free
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (8 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 09/19] staging: wfx: call wfx_tx_update_sta() before to destroy tx_priv Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 11/19] staging: wfx: rename wfx_do_unjoin() into wfx_reset() Jerome Pouiller
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

wfx_tx_policy_put() use data from the skb. However, the call to
skb_pull() has just discarded them (even if the memory is in fact not
really discarded).

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 314cc2743a2b..d01e679b0880 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -494,8 +494,8 @@ static void wfx_skb_dtor(struct wfx_vif *wvif, struct sk_buff *skb)
 			      req->data_flags.fc_offset;
 
 	WARN_ON(!wvif);
-	skb_pull(skb, offset);
 	wfx_tx_policy_put(wvif, req->tx_flags.retry_policy_index);
+	skb_pull(skb, offset);
 	ieee80211_tx_status_irqsafe(wvif->wdev->hw, skb);
 }
 
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 11/19] staging: wfx: rename wfx_do_unjoin() into wfx_reset()
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (9 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 10/19] staging: wfx: fix potential use-after-free Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 12/19] staging: wfx: merge wfx_stop_ap() with wfx_reset() Jerome Pouiller
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

In fact, wfx_do_unjoin() resets the interface. This mechanism can be
used in more cases than just disassociating from a BSS. So, rename it to
reflect that fact.

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

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 1a876a0faaf5..e077f42b62dc 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -315,20 +315,6 @@ void wfx_set_default_unicast_key(struct ieee80211_hw *hw,
 	hif_wep_default_key_id(wvif, idx);
 }
 
-// Call it with wdev->conf_mutex locked
-static void wfx_do_unjoin(struct wfx_vif *wvif)
-{
-	/* Unjoin is a reset. */
-	wfx_tx_lock_flush(wvif->wdev);
-	hif_reset(wvif, false);
-	wfx_tx_policy_init(wvif);
-	if (wvif_count(wvif->wdev) <= 1)
-		hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
-	wfx_tx_unlock(wvif->wdev);
-	wvif->bss_not_support_ps_poll = false;
-	cancel_delayed_work_sync(&wvif->beacon_loss_work);
-}
-
 static void wfx_set_mfp(struct wfx_vif *wvif,
 			struct cfg80211_bss *bss)
 {
@@ -359,6 +345,18 @@ static void wfx_set_mfp(struct wfx_vif *wvif,
 	hif_set_mfp(wvif, mfpc, mfpr);
 }
 
+void wfx_reset(struct wfx_vif *wvif)
+{
+	wfx_tx_lock_flush(wvif->wdev);
+	hif_reset(wvif, false);
+	wfx_tx_policy_init(wvif);
+	if (wvif_count(wvif->wdev) <= 1)
+		hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
+	wfx_tx_unlock(wvif->wdev);
+	wvif->bss_not_support_ps_poll = false;
+	cancel_delayed_work_sync(&wvif->beacon_loss_work);
+}
+
 static void wfx_do_join(struct wfx_vif *wvif)
 {
 	int ret;
@@ -395,7 +393,7 @@ static void wfx_do_join(struct wfx_vif *wvif)
 	ret = hif_join(wvif, conf, wvif->channel, ssid, ssidlen);
 	if (ret) {
 		ieee80211_connection_loss(wvif->vif);
-		wfx_do_unjoin(wvif);
+		wfx_reset(wvif);
 	} else {
 		/* Due to beacon filtering it is possible that the
 		 * AP's beacon is not known for the mac80211 stack.
@@ -513,7 +511,7 @@ void wfx_leave_ibss(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 {
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 
-	wfx_do_unjoin(wvif);
+	wfx_reset(wvif);
 }
 
 static void wfx_enable_beacon(struct wfx_vif *wvif, bool enable)
@@ -580,7 +578,7 @@ void wfx_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		if (info->assoc || info->ibss_joined)
 			wfx_join_finalize(wvif, info);
 		else if (!info->assoc && vif->type == NL80211_IFTYPE_STATION)
-			wfx_do_unjoin(wvif);
+			wfx_reset(wvif);
 		else
 			dev_warn(wdev->dev, "%s: misunderstood change: ASSOC\n",
 				 __func__);
diff --git a/drivers/staging/wfx/sta.h b/drivers/staging/wfx/sta.h
index c84c3749ec4f..8a20ad9ae017 100644
--- a/drivers/staging/wfx/sta.h
+++ b/drivers/staging/wfx/sta.h
@@ -71,6 +71,7 @@ void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd cmd);
 void wfx_event_report_rssi(struct wfx_vif *wvif, u8 raw_rcpi_rssi);
 
 // Other Helpers
+void wfx_reset(struct wfx_vif *wvif);
 u32 wfx_rate_mask_to_hw(struct wfx_dev *wdev, u32 rates);
 
 #endif /* WFX_STA_H */
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 12/19] staging: wfx: merge wfx_stop_ap() with wfx_reset()
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (10 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 11/19] staging: wfx: rename wfx_do_unjoin() into wfx_reset() Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 13/19] staging: wfx: fix potential dead lock between join and scan Jerome Pouiller
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

wfx_stop_ap() and wfx_reset() do the same thing. Merge them.

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

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index e077f42b62dc..7d9f680ca53a 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -479,11 +479,7 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 {
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 
-	hif_reset(wvif, false);
-	wfx_tx_policy_init(wvif);
-	if (wvif_count(wvif->wdev) <= 1)
-		hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
-	wvif->bss_not_support_ps_poll = false;
+	wfx_reset(wvif);
 }
 
 static void wfx_join_finalize(struct wfx_vif *wvif,
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 13/19] staging: wfx: fix potential dead lock between join and scan
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (11 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 12/19] staging: wfx: merge wfx_stop_ap() with wfx_reset() Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 14/19] staging: wfx: fix PS parameters when multiple vif are in use Jerome Pouiller
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

The device disallows to start a scan request between hif_join() and
hif_set_bss_params(). The driver is not protected against that. The
worst case happens when association is aborted and hif_set_bss_params()
never happens.

mac80211 would never ask for scan during the association process. So,
this patch just aborts the association in progress when scan is
requested.

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

diff --git a/drivers/staging/wfx/scan.c b/drivers/staging/wfx/scan.c
index bf7ddc75c7db..e9de19784865 100644
--- a/drivers/staging/wfx/scan.c
+++ b/drivers/staging/wfx/scan.c
@@ -90,6 +90,11 @@ void wfx_hw_scan_work(struct work_struct *work)
 
 	mutex_lock(&wvif->wdev->conf_mutex);
 	mutex_lock(&wvif->scan_lock);
+	if (wvif->join_in_progress) {
+		dev_info(wvif->wdev->dev, "%s: abort in-progress REQ_JOIN",
+			 __func__);
+		wfx_reset(wvif);
+	}
 	update_probe_tmpl(wvif, &hw_req->req);
 	chan_cur = 0;
 	do {
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 7d9f680ca53a..6e9f38d051ab 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -353,6 +353,7 @@ void wfx_reset(struct wfx_vif *wvif)
 	if (wvif_count(wvif->wdev) <= 1)
 		hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
 	wfx_tx_unlock(wvif->wdev);
+	wvif->join_in_progress = false;
 	wvif->bss_not_support_ps_poll = false;
 	cancel_delayed_work_sync(&wvif->beacon_loss_work);
 }
@@ -390,6 +391,7 @@ static void wfx_do_join(struct wfx_vif *wvif)
 	wfx_set_mfp(wvif, bss);
 	cfg80211_put_bss(wvif->wdev->hw->wiphy, bss);
 
+	wvif->join_in_progress = true;
 	ret = hif_join(wvif, conf, wvif->channel, ssid, ssidlen);
 	if (ret) {
 		ieee80211_connection_loss(wvif->vif);
@@ -485,6 +487,7 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 static void wfx_join_finalize(struct wfx_vif *wvif,
 			      struct ieee80211_bss_conf *info)
 {
+	wvif->join_in_progress = false;
 	hif_set_association_mode(wvif, info);
 	hif_keep_alive_period(wvif, 0);
 	// beacon_loss_count is defined to 7 in net/mac80211/mlme.c. Let's use
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 09a24561f092..cc9f7d16ee8b 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -69,6 +69,8 @@ struct wfx_vif {
 	u32			link_id_map;
 
 	bool			after_dtim_tx_allowed;
+	bool			join_in_progress;
+
 	struct delayed_work	beacon_loss_work;
 
 	struct tx_policy_cache	tx_policy_cache;
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 14/19] staging: wfx: fix PS parameters when multiple vif are in use
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (12 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 13/19] staging: wfx: fix potential dead lock between join and scan Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 15/19] staging: wfx: drop unnecessary filter configuration when disabling filter Jerome Pouiller
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

When multiple vif are in use (= one access point and one station), and
when the channels are different, it is necessary to enable power save on
station.

The firmware check that steps are done in the correct order:
  - AP can't start if PS is not enable on the station
  - PS can't set on the station before the association has finished
    (= before the call set_bss_params)

Obviously, in add, when one of the interface disappears, it is necessary to
restore the power save status.

wfx_update_pm() is able to set the correct PS configuration. But it has
to be called at the right time:
   1. before hif_start(), but after the channel configuration is known
   2. after hif_set_bss_params()
   3. after hif_reset()

Therefore, the call to wfx_update_pm() from wfx_add_interface() is too
early to address 1.

The call after hif_set_bss_params() already exists.

For the symmetry, the call from wfx_remove_interface() (that handle 3.)
is also relocated.

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

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 6e9f38d051ab..0cb7315bb050 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -347,15 +347,20 @@ static void wfx_set_mfp(struct wfx_vif *wvif,
 
 void wfx_reset(struct wfx_vif *wvif)
 {
-	wfx_tx_lock_flush(wvif->wdev);
+	struct wfx_dev *wdev = wvif->wdev;
+
+	wfx_tx_lock_flush(wdev);
 	hif_reset(wvif, false);
 	wfx_tx_policy_init(wvif);
-	if (wvif_count(wvif->wdev) <= 1)
+	if (wvif_count(wdev) <= 1)
 		hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
-	wfx_tx_unlock(wvif->wdev);
+	wfx_tx_unlock(wdev);
 	wvif->join_in_progress = false;
 	wvif->bss_not_support_ps_poll = false;
 	cancel_delayed_work_sync(&wvif->beacon_loss_work);
+	wvif =  NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
+		wfx_update_pm(wvif);
 }
 
 static void wfx_do_join(struct wfx_vif *wvif)
@@ -471,7 +476,12 @@ static int wfx_upload_ap_templates(struct wfx_vif *wvif)
 int wfx_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 {
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
+	struct wfx_dev *wdev = wvif->wdev;
 
+	wvif =  NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
+		wfx_update_pm(wvif);
+	wvif = (struct wfx_vif *)vif->drv_priv;
 	wfx_upload_ap_templates(wvif);
 	hif_start(wvif, &vif->bss_conf, wvif->channel);
 	return 0;
@@ -786,8 +796,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 			hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
 		else
 			hif_set_block_ack_policy(wvif, 0x00, 0x00);
-		// Combo force powersave mode. We can re-enable it now
-		ret = wfx_update_pm(wvif);
 	}
 	return ret;
 }
@@ -818,8 +826,6 @@ void wfx_remove_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 			hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
 		else
 			hif_set_block_ack_policy(wvif, 0x00, 0x00);
-		// Combo force powersave mode. We can re-enable it now
-		wfx_update_pm(wvif);
 	}
 }
 
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 15/19] staging: wfx: drop unnecessary filter configuration when disabling filter
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (13 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 14/19] staging: wfx: fix PS parameters when multiple vif are in use Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 16/19] staging: wfx: fix error reporting in wfx_start_ap() Jerome Pouiller
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

Currently, when mac80211 want to disable beacon filtering, the driver
reset the filter table and disable the beacon filtering. Only the latter
action is required.

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

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 0cb7315bb050..57304ed42e79 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -84,7 +84,6 @@ static void wfx_filter_beacon(struct wfx_vif *wvif, bool filter_beacon)
 	};
 
 	if (!filter_beacon) {
-		hif_set_beacon_filter_table(wvif, 0, NULL);
 		hif_beacon_filter_control(wvif, 0, 1);
 	} else {
 		hif_set_beacon_filter_table(wvif, 3, filter_ies);
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 16/19] staging: wfx: fix error reporting in wfx_start_ap()
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (14 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 15/19] staging: wfx: drop unnecessary filter configuration when disabling filter Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 17/19] staging: wfx: remove false-positive WARN() Jerome Pouiller
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

If AP did not start, the error was not reported to mac80211.

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

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 57304ed42e79..f448957c1a92 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -476,14 +476,17 @@ int wfx_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 {
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 	struct wfx_dev *wdev = wvif->wdev;
+	int ret;
 
 	wvif =  NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
 		wfx_update_pm(wvif);
 	wvif = (struct wfx_vif *)vif->drv_priv;
 	wfx_upload_ap_templates(wvif);
-	hif_start(wvif, &vif->bss_conf, wvif->channel);
-	return 0;
+	ret = hif_start(wvif, &vif->bss_conf, wvif->channel);
+	if (ret > 0)
+		return -EIO;
+	return ret;
 }
 
 void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 17/19] staging: wfx: remove false-positive WARN()
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (15 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 16/19] staging: wfx: fix error reporting in wfx_start_ap() Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 18/19] staging: wfx: trace acknowledges not linked to any stations Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 19/19] staging: wfx: remove false positive warning Jerome Pouiller
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

The function wfx_tx_flush() wait for there is no more queued frames in
hardware queue. Then, for the sanity, it checks that there is no more
pending frame on any AC queue.

However, there is a race here. It may happens that hardware queues are
empty, but the counters of the AC queues are not yet updated. So, it may
produce false-positive warning.

The easiest way to solve the problem is just to remove the sanity check.

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

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 0c799cedd101..26b141cbd303 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -30,7 +30,6 @@ void wfx_tx_unlock(struct wfx_dev *wdev)
 void wfx_tx_flush(struct wfx_dev *wdev)
 {
 	int ret;
-	int i;
 
 	// Do not wait for any reply if chip is frozen
 	if (wdev->chip_frozen)
@@ -41,12 +40,6 @@ void wfx_tx_flush(struct wfx_dev *wdev)
 	ret = wait_event_timeout(wdev->hif.tx_buffers_empty,
 				 !wdev->hif.tx_buffers_used,
 				 msecs_to_jiffies(3000));
-	if (ret) {
-		for (i = 0; i < IEEE80211_NUM_ACS; i++)
-			WARN(atomic_read(&wdev->tx_queue[i].pending_frames),
-			     "there are still %d pending frames on queue %d",
-			     atomic_read(&wdev->tx_queue[i].pending_frames), i);
-	}
 	if (!ret) {
 		dev_warn(wdev->dev, "cannot flush tx buffers (%d still busy)\n",
 			 wdev->hif.tx_buffers_used);
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 18/19] staging: wfx: trace acknowledges not linked to any stations
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (16 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 17/19] staging: wfx: remove false-positive WARN() Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  2020-05-15  8:33 ` [PATCH 19/19] staging: wfx: remove false positive warning Jerome Pouiller
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

Some resources are associated to the outgoing of the stations. To avoid
any resource leaks. It is important to understand why an acknowledge is
not associated to any station.

Add a trace for that purpose.

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

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index d01e679b0880..a82f00f8f17b 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -481,6 +481,9 @@ static void wfx_tx_update_sta(struct wfx_vif *wvif, struct ieee80211_hdr *hdr)
 		if (!sta_priv->buffered[tid])
 			ieee80211_sta_set_buffered(sta, tid, false);
 		spin_unlock_bh(&sta_priv->lock);
+	} else {
+		dev_dbg(wvif->wdev->dev, "%s: sta does not exist anymore\n",
+			__func__);
 	}
 	rcu_read_unlock();
 }
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 19/19] staging: wfx: remove false positive warning
  2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
                   ` (17 preceding siblings ...)
  2020-05-15  8:33 ` [PATCH 18/19] staging: wfx: trace acknowledges not linked to any stations Jerome Pouiller
@ 2020-05-15  8:33 ` Jerome Pouiller
  18 siblings, 0 replies; 24+ messages in thread
From: Jerome Pouiller @ 2020-05-15  8:33 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, David S . Miller, Kalle Valo

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

When a station is removed, the driver check that all the Tx frames were
correctly sent. However, the station can be removed before all the Tx
frames were acknowledged and a false positive warning can be emitted.

The previous commit has added a trace when driver received an
acknowledge for a non-existent station. It appear that these events
are perfectly correlated and there is no leak.

Now, the subject is perfectly understood. Remove the warning. Just keep
a debug trace in case we have any doubt in the future.

In the past, the subject has already been discussed here:
   https://lore.kernel.org/driverdev-devel/6287924.ghGFUMk3OD@pc-42/

Fixes: 4bbc6a3e7ad0 ("staging: wfx: make warning about pending frame less scary")
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/sta.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index f448957c1a92..6015cd2c4d8a 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -441,8 +441,10 @@ int wfx_sta_remove(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	for (i = 0; i < ARRAY_SIZE(sta_priv->buffered); i++)
 		if (sta_priv->buffered[i])
-			dev_warn(wvif->wdev->dev, "release station while %d pending frame on queue %d",
-				 sta_priv->buffered[i], i);
+			// Not an error if paired with trace in
+			// wfx_tx_update_sta()
+			dev_dbg(wvif->wdev->dev, "release station while %d pending frame on queue %d",
+				sta_priv->buffered[i], i);
 	// See note in wfx_sta_add()
 	if (!sta_priv->link_id)
 		return 0;
-- 
2.26.2

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype
  2020-05-15  8:33 ` [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype Jerome Pouiller
@ 2020-05-15 13:53   ` Greg Kroah-Hartman
  2020-05-15 14:01     ` Greg Kroah-Hartman
  2020-05-15 15:03     ` Jérôme Pouiller
  0 siblings, 2 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-15 13:53 UTC (permalink / raw)
  To: Jerome Pouiller
  Cc: devel, netdev, linux-wireless, linux-kernel, David S . Miller,
	Kalle Valo

On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> The function hif_scan() return the timeout for the completion of the
> scan request. It is the only function from hif_tx.c that return another
> thing than just an error code. This behavior is not coherent with the
> rest of file. Worse, if value returned is positive, the caller can't
> make say if it is a timeout or the value returned by the hardware.
> 
> Uniformize API with other HIF functions, only return the error code and
> pass timeout with parameters.
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/staging/wfx/hif_tx.c | 6 ++++--
>  drivers/staging/wfx/hif_tx.h | 2 +-
>  drivers/staging/wfx/scan.c   | 6 +++---
>  3 files changed, 8 insertions(+), 6 deletions(-)

This patch fails to apply to my branch, so I've stopped here in the
patch series.

Can you rebase and resend the remaining ones?

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype
  2020-05-15 13:53   ` Greg Kroah-Hartman
@ 2020-05-15 14:01     ` Greg Kroah-Hartman
  2020-05-15 15:03     ` Jérôme Pouiller
  1 sibling, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-15 14:01 UTC (permalink / raw)
  To: Jerome Pouiller
  Cc: devel, netdev, linux-wireless, linux-kernel, David S . Miller,
	Kalle Valo

On Fri, May 15, 2020 at 03:53:59PM +0200, Greg Kroah-Hartman wrote:
> On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > 
> > The function hif_scan() return the timeout for the completion of the
> > scan request. It is the only function from hif_tx.c that return another
> > thing than just an error code. This behavior is not coherent with the
> > rest of file. Worse, if value returned is positive, the caller can't
> > make say if it is a timeout or the value returned by the hardware.
> > 
> > Uniformize API with other HIF functions, only return the error code and
> > pass timeout with parameters.
> > 
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/staging/wfx/hif_tx.c | 6 ++++--
> >  drivers/staging/wfx/hif_tx.h | 2 +-
> >  drivers/staging/wfx/scan.c   | 6 +++---
> >  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> This patch fails to apply to my branch, so I've stopped here in the
> patch series.
> 
> Can you rebase and resend the remaining ones?

Ok, all others worked, just this one failed, so I've applied them.

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype
  2020-05-15 13:53   ` Greg Kroah-Hartman
  2020-05-15 14:01     ` Greg Kroah-Hartman
@ 2020-05-15 15:03     ` Jérôme Pouiller
  2020-05-15 15:09       ` Greg Kroah-Hartman
  1 sibling, 1 reply; 24+ messages in thread
From: Jérôme Pouiller @ 2020-05-15 15:03 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, netdev, linux-wireless, linux-kernel, David S . Miller,
	Kalle Valo

On Friday 15 May 2020 15:53:59 CEST Greg Kroah-Hartman wrote:
> On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> >
> > The function hif_scan() return the timeout for the completion of the
> > scan request. It is the only function from hif_tx.c that return another
> > thing than just an error code. This behavior is not coherent with the
> > rest of file. Worse, if value returned is positive, the caller can't
> > make say if it is a timeout or the value returned by the hardware.
> >
> > Uniformize API with other HIF functions, only return the error code and
> > pass timeout with parameters.
> >
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/staging/wfx/hif_tx.c | 6 ++++--
> >  drivers/staging/wfx/hif_tx.h | 2 +-
> >  drivers/staging/wfx/scan.c   | 6 +++---
> >  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> This patch fails to apply to my branch, so I've stopped here in the
> patch series.

Hello Greg,

Did you applied the patch called "staging: wfx: unlock on error path" from
Dan?

(I wrote that information in the introduction letter, but maybe I would
had include the Dan's patch in my PR?)


-- 
Jérôme Pouiller


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype
  2020-05-15 15:03     ` Jérôme Pouiller
@ 2020-05-15 15:09       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-15 15:09 UTC (permalink / raw)
  To: Jérôme Pouiller
  Cc: devel, netdev, linux-wireless, linux-kernel, David S . Miller,
	Kalle Valo

On Fri, May 15, 2020 at 05:03:40PM +0200, Jérôme Pouiller wrote:
> On Friday 15 May 2020 15:53:59 CEST Greg Kroah-Hartman wrote:
> > On Fri, May 15, 2020 at 10:33:11AM +0200, Jerome Pouiller wrote:
> > > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > >
> > > The function hif_scan() return the timeout for the completion of the
> > > scan request. It is the only function from hif_tx.c that return another
> > > thing than just an error code. This behavior is not coherent with the
> > > rest of file. Worse, if value returned is positive, the caller can't
> > > make say if it is a timeout or the value returned by the hardware.
> > >
> > > Uniformize API with other HIF functions, only return the error code and
> > > pass timeout with parameters.
> > >
> > > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > > ---
> > >  drivers/staging/wfx/hif_tx.c | 6 ++++--
> > >  drivers/staging/wfx/hif_tx.h | 2 +-
> > >  drivers/staging/wfx/scan.c   | 6 +++---
> > >  3 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > This patch fails to apply to my branch, so I've stopped here in the
> > patch series.
> 
> Hello Greg,
> 
> Did you applied the patch called "staging: wfx: unlock on error path" from
> Dan?

I have no idea :)

> (I wrote that information in the introduction letter, but maybe I would
> had include the Dan's patch in my PR?)

I think you should have, as my queue is empty now.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15  8:33 [PATCH 00/19] staging: wfx: various fixes Jerome Pouiller
2020-05-15  8:33 ` [PATCH 01/19] staging: wfx: fix warning when unregister a frozen device Jerome Pouiller
2020-05-15  8:33 ` [PATCH 02/19] staging: wfx: apply 80-columns rule to strings Jerome Pouiller
2020-05-15  8:33 ` [PATCH 03/19] staging: wfx: check pointers returned by allocations Jerome Pouiller
2020-05-15  8:33 ` [PATCH 04/19] staging: wfx: fix value of scan timeout Jerome Pouiller
2020-05-15  8:33 ` [PATCH 05/19] staging: wfx: fix coherency of hif_scan() prototype Jerome Pouiller
2020-05-15 13:53   ` Greg Kroah-Hartman
2020-05-15 14:01     ` Greg Kroah-Hartman
2020-05-15 15:03     ` Jérôme Pouiller
2020-05-15 15:09       ` Greg Kroah-Hartman
2020-05-15  8:33 ` [PATCH 06/19] staging: wfx: fix indentation Jerome Pouiller
2020-05-15  8:33 ` [PATCH 07/19] staging: wfx: fix status of dropped frames Jerome Pouiller
2020-05-15  8:33 ` [PATCH 08/19] staging: wfx: split out wfx_tx_fill_rates() from wfx_tx_confirm_cb() Jerome Pouiller
2020-05-15  8:33 ` [PATCH 09/19] staging: wfx: call wfx_tx_update_sta() before to destroy tx_priv Jerome Pouiller
2020-05-15  8:33 ` [PATCH 10/19] staging: wfx: fix potential use-after-free Jerome Pouiller
2020-05-15  8:33 ` [PATCH 11/19] staging: wfx: rename wfx_do_unjoin() into wfx_reset() Jerome Pouiller
2020-05-15  8:33 ` [PATCH 12/19] staging: wfx: merge wfx_stop_ap() with wfx_reset() Jerome Pouiller
2020-05-15  8:33 ` [PATCH 13/19] staging: wfx: fix potential dead lock between join and scan Jerome Pouiller
2020-05-15  8:33 ` [PATCH 14/19] staging: wfx: fix PS parameters when multiple vif are in use Jerome Pouiller
2020-05-15  8:33 ` [PATCH 15/19] staging: wfx: drop unnecessary filter configuration when disabling filter Jerome Pouiller
2020-05-15  8:33 ` [PATCH 16/19] staging: wfx: fix error reporting in wfx_start_ap() Jerome Pouiller
2020-05-15  8:33 ` [PATCH 17/19] staging: wfx: remove false-positive WARN() Jerome Pouiller
2020-05-15  8:33 ` [PATCH 18/19] staging: wfx: trace acknowledges not linked to any stations Jerome Pouiller
2020-05-15  8:33 ` [PATCH 19/19] staging: wfx: remove false positive warning Jerome Pouiller

DriverDev-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/driverdev-devel/0 driverdev-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 driverdev-devel driverdev-devel/ https://lore.kernel.org/driverdev-devel \
		driverdev-devel@linuxdriverproject.org devel@driverdev.osuosl.org
	public-inbox-index driverdev-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxdriverproject.driverdev-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git