linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings
@ 2017-07-25  1:13 Brian Norris
  2017-07-25  1:13 ` [PATCH v2 01/20] mwifiex: reunite copy-and-pasted remove/reset code Brian Norris
                   ` (19 more replies)
  0 siblings, 20 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris, Johannes Berg

Hello,

I've previously sent a stack of similar patches (with no cover letter),
starting at this patch:

[PATCH 01/14] mwifiex: pcie: properly synchronize, disable interrupts in driver callbacks
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1405062.html
https://patchwork.kernel.org/patch/9747263/

They fixed several bugs related to the "reset" codepath in this driver,
in which the driver tries to recover from a dead firmware, as well as making
various code improvements (e.g., refactorings, removing redunancy, improving
safety).

There were some valid criticisms of the first patch in that series (and some
real issues I hit with it later on in testing), and in the end, I believe
the concern there was not actually appearing in practice, so in order to
make some forward progress, I've dropped that patch (and related changes)
from this series. I've kept most of the rest and added a few bugfixes along
the way.

Thanks to Johannes for some brainstorming/ideas that yielded the patch
"mwifiex: unregister wiphy before freeing resources".

Regards,
Brian

Brian Norris (20):
  mwifiex: reunite copy-and-pasted remove/reset code
  mwifiex: reset interrupt status across device reset
  mwifiex: pcie: don't allow cmd buffer reuse after reset
  mwifiex: re-register wiphy across reset
  mwifiex: unregister wiphy before freeing resources
  mwifiex: don't short-circuit netdev notifiers on interface deletion
  mwifiex: fixup init_channel_scan_gap error case
  mwifiex: ensure "disable auto DS" struct is initialized
  mwifiex: fix misnomers in mwifiex_free_lock_list()
  mwifiex: make mwifiex_free_cmd_buffer() return void
  mwifiex: utilize netif_tx_{wake,stop}_all_queues()
  mwifiex: don't open-code ARRAY_SIZE()
  mwifiex: drop 'add_tail' param from mwifiex_insert_cmd_to_pending_q()
  mwifiex: pcie: remove unnecessary masks
  mwifiex: pcie: unify MSI-X / non-MSI-X interrupt process
  mwifiex: debugfs: allow card_reset() to cancel things
  mwifiex: pcie: disable device DMA before unmapping/freeing buffers
  mwifiex: pcie: remove unnecessary 'pdev' check
  mwifiex: keep mwifiex_cancel_pending_ioctl() static
  mwifiex: drop num CPU notice

 drivers/net/wireless/marvell/mwifiex/cfg80211.c    |   4 -
 drivers/net/wireless/marvell/mwifiex/cfp.c         |   4 +-
 drivers/net/wireless/marvell/mwifiex/cmdevt.c      |  15 +--
 drivers/net/wireless/marvell/mwifiex/debugfs.c     |   2 -
 drivers/net/wireless/marvell/mwifiex/init.c        |  32 ++----
 drivers/net/wireless/marvell/mwifiex/main.c        | 124 +++++++--------------
 drivers/net/wireless/marvell/mwifiex/main.h        |   7 +-
 drivers/net/wireless/marvell/mwifiex/pcie.c        | 100 +++--------------
 drivers/net/wireless/marvell/mwifiex/scan.c        |   5 +-
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c     |   8 +-
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c |   5 +-
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c   |   6 +-
 12 files changed, 87 insertions(+), 225 deletions(-)

-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 01/20] mwifiex: reunite copy-and-pasted remove/reset code
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-28 14:49   ` [v2,01/20] " Kalle Valo
  2017-07-25  1:13 ` [PATCH v2 02/20] mwifiex: reset interrupt status across device reset Brian Norris
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

When PCIe FLR code was added, it explicitly copy-and-pasted much of
mwifiex_remove_card() into mwifiex_shutdown_sw(). This is unnecessary,
as almost all of the code should be reused.

Let's reunite what we can for now.

The only functional changes for now:

 * call netif_device_detach() in the remove() code path -- this wasn't
   done before, but it really should be a no-op, when the device is
   getting totally unregistered soon anyway

 * call the ->down_dev() driver callback only after we've finished all
   SW teardown -- this should have no significant effect, since the only
   user (pcie.c) does very minimal work there, and it doesn't matter
   that we reorder this

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/main.c | 104 ++++++++--------------------
 1 file changed, 28 insertions(+), 76 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index f2600b827e81..8615099468da 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1352,26 +1352,12 @@ static void mwifiex_main_work_queue(struct work_struct *work)
 	mwifiex_main_process(adapter);
 }
 
-/*
- * This function gets called during PCIe function level reset. Required
- * code is extracted from mwifiex_remove_card()
- */
-int
-mwifiex_shutdown_sw(struct mwifiex_adapter *adapter)
+/* Common teardown code used for both device removal and reset */
+static void mwifiex_uninit_sw(struct mwifiex_adapter *adapter)
 {
 	struct mwifiex_private *priv;
 	int i;
 
-	if (!adapter)
-		goto exit_return;
-
-	wait_for_completion(adapter->fw_done);
-	/* Caller should ensure we aren't suspending while this happens */
-	reinit_completion(adapter->fw_done);
-
-	priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
-	mwifiex_deauthenticate(priv, NULL);
-
 	/* We can no longer handle interrupts once we start doing the teardown
 	 * below.
 	 */
@@ -1393,12 +1379,9 @@ mwifiex_shutdown_sw(struct mwifiex_adapter *adapter)
 	}
 
 	mwifiex_dbg(adapter, CMD, "cmd: calling mwifiex_shutdown_drv...\n");
-
 	mwifiex_shutdown_drv(adapter);
-	if (adapter->if_ops.down_dev)
-		adapter->if_ops.down_dev(adapter);
-
 	mwifiex_dbg(adapter, CMD, "cmd: mwifiex_shutdown_drv done\n");
+
 	if (atomic_read(&adapter->rx_pending) ||
 	    atomic_read(&adapter->tx_pending) ||
 	    atomic_read(&adapter->cmd_pending)) {
@@ -1421,9 +1404,30 @@ mwifiex_shutdown_sw(struct mwifiex_adapter *adapter)
 		rtnl_unlock();
 	}
 	vfree(adapter->chan_stats);
+}
+
+/*
+ * This function gets called during PCIe function level reset.
+ */
+int mwifiex_shutdown_sw(struct mwifiex_adapter *adapter)
+{
+	struct mwifiex_private *priv;
+
+	if (!adapter)
+		return 0;
+
+	wait_for_completion(adapter->fw_done);
+	/* Caller should ensure we aren't suspending while this happens */
+	reinit_completion(adapter->fw_done);
+
+	priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
+	mwifiex_deauthenticate(priv, NULL);
+
+	mwifiex_uninit_sw(adapter);
+
+	if (adapter->if_ops.down_dev)
+		adapter->if_ops.down_dev(adapter);
 
-	mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
-exit_return:
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mwifiex_shutdown_sw);
@@ -1676,61 +1680,10 @@ EXPORT_SYMBOL_GPL(mwifiex_add_card);
  */
 int mwifiex_remove_card(struct mwifiex_adapter *adapter)
 {
-	struct mwifiex_private *priv = NULL;
-	int i;
-
 	if (!adapter)
-		goto exit_remove;
-
-	/* We can no longer handle interrupts once we start doing the teardown
-	 * below. */
-	if (adapter->if_ops.disable_int)
-		adapter->if_ops.disable_int(adapter);
-
-	adapter->surprise_removed = true;
-
-	mwifiex_terminate_workqueue(adapter);
-
-	/* Stop data */
-	for (i = 0; i < adapter->priv_num; i++) {
-		priv = adapter->priv[i];
-		if (priv && priv->netdev) {
-			mwifiex_stop_net_dev_queue(priv->netdev, adapter);
-			if (netif_carrier_ok(priv->netdev))
-				netif_carrier_off(priv->netdev);
-		}
-	}
-
-	mwifiex_dbg(adapter, CMD,
-		    "cmd: calling mwifiex_shutdown_drv...\n");
-
-	mwifiex_shutdown_drv(adapter);
-	mwifiex_dbg(adapter, CMD,
-		    "cmd: mwifiex_shutdown_drv done\n");
-	if (atomic_read(&adapter->rx_pending) ||
-	    atomic_read(&adapter->tx_pending) ||
-	    atomic_read(&adapter->cmd_pending)) {
-		mwifiex_dbg(adapter, ERROR,
-			    "rx_pending=%d, tx_pending=%d,\t"
-			    "cmd_pending=%d\n",
-			    atomic_read(&adapter->rx_pending),
-			    atomic_read(&adapter->tx_pending),
-			    atomic_read(&adapter->cmd_pending));
-	}
-
-	for (i = 0; i < adapter->priv_num; i++) {
-		priv = adapter->priv[i];
-
-		if (!priv)
-			continue;
+		return 0;
 
-		rtnl_lock();
-		if (priv->netdev &&
-		    priv->wdev.iftype != NL80211_IFTYPE_UNSPECIFIED)
-			mwifiex_del_virtual_intf(adapter->wiphy, &priv->wdev);
-		rtnl_unlock();
-	}
-	vfree(adapter->chan_stats);
+	mwifiex_uninit_sw(adapter);
 
 	wiphy_unregister(adapter->wiphy);
 	wiphy_free(adapter->wiphy);
@@ -1748,7 +1701,6 @@ int mwifiex_remove_card(struct mwifiex_adapter *adapter)
 		    "info: free adapter\n");
 	mwifiex_free_adapter(adapter);
 
-exit_remove:
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mwifiex_remove_card);
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 02/20] mwifiex: reset interrupt status across device reset
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
  2017-07-25  1:13 ` [PATCH v2 01/20] mwifiex: reunite copy-and-pasted remove/reset code Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 03/20] mwifiex: pcie: don't allow cmd buffer reuse after reset Brian Norris
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

When resetting the device, we might have queued up interrupts that
didn't get a chance to finish processing. We really don't need to handle
them at this point; we just want to make sure they don't cause us to try
to process old commands from before the device was reset.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 8615099468da..275cf8dc4f2a 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1366,6 +1366,7 @@ static void mwifiex_uninit_sw(struct mwifiex_adapter *adapter)
 
 	adapter->surprise_removed = true;
 	mwifiex_terminate_workqueue(adapter);
+	adapter->int_status = 0;
 
 	/* Stop data */
 	for (i = 0; i < adapter->priv_num; i++) {
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 03/20] mwifiex: pcie: don't allow cmd buffer reuse after reset
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
  2017-07-25  1:13 ` [PATCH v2 01/20] mwifiex: reunite copy-and-pasted remove/reset code Brian Norris
  2017-07-25  1:13 ` [PATCH v2 02/20] mwifiex: reset interrupt status across device reset Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 04/20] mwifiex: re-register wiphy across reset Brian Norris
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

In rogue cases (due to other bugs) it's possible we try to process an
old command response *after* resetting the device. This could trigger a
double-free (or the SKB can get reallocated elsewhere...causing other
memory corruptions) in mwifiex_pcie_process_cmd_complete().

For safety (and symmetry) let's always NULL out the command buffer as we
free it up. We're already doing this for the command response buffer.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index b53ecf1eddda..5c07edd4e094 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -1030,12 +1030,14 @@ static int mwifiex_pcie_delete_cmdrsp_buf(struct mwifiex_adapter *adapter)
 		mwifiex_unmap_pci_memory(adapter, card->cmdrsp_buf,
 					 PCI_DMA_FROMDEVICE);
 		dev_kfree_skb_any(card->cmdrsp_buf);
+		card->cmdrsp_buf = NULL;
 	}
 
 	if (card && card->cmd_buf) {
 		mwifiex_unmap_pci_memory(adapter, card->cmd_buf,
 					 PCI_DMA_TODEVICE);
 		dev_kfree_skb_any(card->cmd_buf);
+		card->cmd_buf = NULL;
 	}
 	return 0;
 }
@@ -2921,7 +2923,6 @@ static void mwifiex_pcie_free_buffers(struct mwifiex_adapter *adapter)
 	mwifiex_pcie_delete_evtbd_ring(adapter);
 	mwifiex_pcie_delete_rxbd_ring(adapter);
 	mwifiex_pcie_delete_txbd_ring(adapter);
-	card->cmdrsp_buf = NULL;
 }
 
 /*
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 04/20] mwifiex: re-register wiphy across reset
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (2 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 03/20] mwifiex: pcie: don't allow cmd buffer reuse after reset Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 05/20] mwifiex: unregister wiphy before freeing resources Brian Norris
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

In general, it's helpful to use the same code for device removal as for
device reset, as this tends to have fewer bugs. Let's move the wiphy
unregistration code into the common reset and removal code.

In particular, it's very hard to properly handle the reset sequence when
something fails. Currently, if mwifiex_reinit_sw() fails, we've failed
to unregister the associated wiphy, and so running something as simple
as "iw phy" can trigger an OOPS, as the wiphy still has hooks back into
freed mwifiex data structures. For example, KASAN complained:

[... see reset fail for other reasons ...]
[ 1184.821158] mwifiex_pcie 0000:01:00.0: info: dnld wifi firmware from 174948 bytes
[ 1186.870914] mwifiex_pcie 0000:01:00.0: info: FW download over, size 608396 bytes
[ 1187.685990] mwifiex_pcie 0000:01:00.0: WLAN FW is active
[ 1187.692673] mwifiex_pcie 0000:01:00.0: cmd_wait_q terminated: -512
[ 1187.699075] mwifiex_pcie 0000:01:00.0: info: _mwifiex_fw_dpc: unregister device
[ 1187.713476] mwifiex: Failed to bring up adapter: -5
[ 1187.718644] mwifiex_pcie 0000:01:00.0: reinit failed: -5

[... run `iw phy` ...]
[ 1212.902419] ==================================================================
[ 1212.909806] BUG: KASAN: use-after-free in mwifiex_cfg80211_get_antenna+0x54/0xfc [mwifiex] at addr ffffffc0ad1a8028
[ 1212.920246] Read of size 1 by task iw/3127
[...]
[ 1212.934946] page dumped because: kasan: bad access detected
[...]
[ 1212.950665] Call trace:
[ 1212.953148] [<ffffffc00020a69c>] dump_backtrace+0x0/0x190
[ 1212.958572] [<ffffffc00020a96c>] show_stack+0x20/0x28
[ 1212.963648] [<ffffffc0005ce18c>] dump_stack+0xa4/0xcc
[ 1212.968723] [<ffffffc0003c4430>] kasan_report+0x378/0x500
[ 1212.974140] [<ffffffc0003c3358>] __asan_load1+0x44/0x4c
[ 1212.979462] [<ffffffbffc2e8360>] mwifiex_cfg80211_get_antenna+0x54/0xfc [mwifiex]
[ 1212.987131] [<ffffffbffc084fc4>] nl80211_send_wiphy+0x75c/0x2de0 [cfg80211]
[ 1212.994246] [<ffffffbffc094f60>] nl80211_dump_wiphy+0x32c/0x438 [cfg80211]
[ 1213.001149] [<ffffffc000ab6404>] genl_lock_dumpit+0x48/0x64
[ 1213.006746] [<ffffffc000ab3474>] netlink_dump+0x178/0x398
[ 1213.012171] [<ffffffc000ab3d18>] __netlink_dump_start+0x1bc/0x260
[...]

This all goes away if we just tear down the wiphy on the way down, and
set it back up if/when we bring the device back up.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 275cf8dc4f2a..9c8f7bcfef8b 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1405,6 +1405,10 @@ static void mwifiex_uninit_sw(struct mwifiex_adapter *adapter)
 		rtnl_unlock();
 	}
 	vfree(adapter->chan_stats);
+
+	wiphy_unregister(adapter->wiphy);
+	wiphy_free(adapter->wiphy);
+	adapter->wiphy = NULL;
 }
 
 /*
@@ -1686,9 +1690,6 @@ int mwifiex_remove_card(struct mwifiex_adapter *adapter)
 
 	mwifiex_uninit_sw(adapter);
 
-	wiphy_unregister(adapter->wiphy);
-	wiphy_free(adapter->wiphy);
-
 	if (adapter->irq_wakeup >= 0)
 		device_init_wakeup(adapter->dev, false);
 
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 05/20] mwifiex: unregister wiphy before freeing resources
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (3 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 04/20] mwifiex: re-register wiphy across reset Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 06/20] mwifiex: don't short-circuit netdev notifiers on interface deletion Brian Norris
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris, Johannes Berg

It's possible for some control interfaces (e.g., scans, set freq) to be
active after we've stopped our main work queue and the netif TX queues.
These don't get completely shut out until we've unregistered the wdevs
and wiphy.

So let's only free command buffers and poison our lists after
wiphy_unregister().

This resolves various use-after-free issues seen when resetting the
device.

Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/init.c | 3 +++
 drivers/net/wireless/marvell/mwifiex/main.c | 7 ++++++-
 drivers/net/wireless/marvell/mwifiex/main.h | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 3ecb59f7405b..de96675e43d5 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -418,7 +418,10 @@ mwifiex_adapter_cleanup(struct mwifiex_adapter *adapter)
 	mwifiex_cancel_all_pending_cmd(adapter);
 	wake_up_interruptible(&adapter->cmd_wait_q.wait);
 	wake_up_interruptible(&adapter->hs_activate_wait_q);
+}
 
+void mwifiex_free_cmd_buffers(struct mwifiex_adapter *adapter)
+{
 	/* Free lock variables */
 	mwifiex_free_lock_list(adapter);
 
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 9c8f7bcfef8b..77e491720664 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -653,6 +653,7 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
 	if (adapter->hw_status == MWIFIEX_HW_STATUS_READY) {
 		pr_debug("info: %s: shutdown mwifiex\n", __func__);
 		mwifiex_shutdown_drv(adapter);
+		mwifiex_free_cmd_buffers(adapter);
 	}
 
 	init_failed = true;
@@ -1404,11 +1405,13 @@ static void mwifiex_uninit_sw(struct mwifiex_adapter *adapter)
 			mwifiex_del_virtual_intf(adapter->wiphy, &priv->wdev);
 		rtnl_unlock();
 	}
-	vfree(adapter->chan_stats);
 
 	wiphy_unregister(adapter->wiphy);
 	wiphy_free(adapter->wiphy);
 	adapter->wiphy = NULL;
+
+	vfree(adapter->chan_stats);
+	mwifiex_free_cmd_buffers(adapter);
 }
 
 /*
@@ -1515,6 +1518,7 @@ mwifiex_reinit_sw(struct mwifiex_adapter *adapter)
 		mwifiex_dbg(adapter, ERROR,
 			    "info: %s: shutdown mwifiex\n", __func__);
 		mwifiex_shutdown_drv(adapter);
+		mwifiex_free_cmd_buffers(adapter);
 	}
 
 	complete_all(adapter->fw_done);
@@ -1662,6 +1666,7 @@ mwifiex_add_card(void *card, struct completion *fw_done,
 	if (adapter->hw_status == MWIFIEX_HW_STATUS_READY) {
 		pr_debug("info: %s: shutdown mwifiex\n", __func__);
 		mwifiex_shutdown_drv(adapter);
+		mwifiex_free_cmd_buffers(adapter);
 	}
 err_kmalloc:
 	mwifiex_free_adapter(adapter);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index f8cf3079ac7d..62ce4e81f695 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1078,6 +1078,7 @@ int mwifiex_get_debug_info(struct mwifiex_private *,
 
 int mwifiex_alloc_cmd_buffer(struct mwifiex_adapter *adapter);
 int mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter);
+void mwifiex_free_cmd_buffers(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter);
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 06/20] mwifiex: don't short-circuit netdev notifiers on interface deletion
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (4 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 05/20] mwifiex: unregister wiphy before freeing resources Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 07/20] mwifiex: fixup init_channel_scan_gap error case Brian Norris
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

When we leave the delete interface function, there are still netdev
hooks that might try to process the device. We're short-circuiting some
of that by changing the interface type and clearing ieee80211_ptr. This
means we skip NETDEV_UNREGISTER_FINAL in cfg80211. Fortunately, that is
currently a no-op.

We don't need most of the cleanup here anyway:

 * the connection state will get (un)set as part of the disconnect
   process (which cfg80211 already initiates for us)
 * the interface type doesn't actually need to be cleared at all (it'll
   trigger a WARN_ON() in cfg80211 if we do)
 * the iee80211_ptr isn't really "ours" to clear anyway

So stop resetting those 3 things.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 06ad2d50f9b0..f86a8a69d060 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3123,11 +3123,7 @@ int mwifiex_del_virtual_intf(struct wiphy *wiphy, struct wireless_dev *wdev)
 		priv->dfs_chan_sw_workqueue = NULL;
 	}
 	/* Clear the priv in adapter */
-	priv->netdev->ieee80211_ptr = NULL;
 	priv->netdev = NULL;
-	priv->wdev.iftype = NL80211_IFTYPE_UNSPECIFIED;
-
-	priv->media_connected = false;
 
 	switch (priv->bss_mode) {
 	case NL80211_IFTYPE_UNSPECIFIED:
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 07/20] mwifiex: fixup init_channel_scan_gap error case
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (5 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 06/20] mwifiex: don't short-circuit netdev notifiers on interface deletion Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 08/20] mwifiex: ensure "disable auto DS" struct is initialized Brian Norris
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

In reading through _mwifiex_fw_dpc(), I noticed that after we've
registered our wiphy, we still have error paths that don't free it back
up. Let's do that.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 77e491720664..0448dcc07139 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -588,7 +588,7 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
 	if (mwifiex_init_channel_scan_gap(adapter)) {
 		mwifiex_dbg(adapter, ERROR,
 			    "could not init channel stats table\n");
-		goto err_init_fw;
+		goto err_init_chan_scan;
 	}
 
 	if (driver_mode) {
@@ -636,6 +636,7 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
 
 err_add_intf:
 	vfree(adapter->chan_stats);
+err_init_chan_scan:
 	wiphy_unregister(adapter->wiphy);
 	wiphy_free(adapter->wiphy);
 err_init_fw:
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 08/20] mwifiex: ensure "disable auto DS" struct is initialized
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (6 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 07/20] mwifiex: fixup init_channel_scan_gap error case Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 09/20] mwifiex: fix misnomers in mwifiex_free_lock_list() Brian Norris
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

The .idle_time field *should* be unused, but technically, we're allowing
unitialized stack garbage to pass all the way through to the firmware
host command. Let's zero it out instead.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index 42997e05d90f..43ecd621d1ef 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -654,9 +654,9 @@ int mwifiex_get_bss_info(struct mwifiex_private *priv,
  */
 int mwifiex_disable_auto_ds(struct mwifiex_private *priv)
 {
-	struct mwifiex_ds_auto_ds auto_ds;
-
-	auto_ds.auto_ds = DEEP_SLEEP_OFF;
+	struct mwifiex_ds_auto_ds auto_ds = {
+		.auto_ds = DEEP_SLEEP_OFF,
+	};
 
 	return mwifiex_send_cmd(priv, HostCmd_CMD_802_11_PS_MODE_ENH,
 				DIS_AUTO_PS, BITMAP_AUTO_DS, &auto_ds, true);
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 09/20] mwifiex: fix misnomers in mwifiex_free_lock_list()
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (7 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 08/20] mwifiex: ensure "disable auto DS" struct is initialized Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 10/20] mwifiex: make mwifiex_free_cmd_buffer() return void Brian Norris
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

Despite the name (and meticulous comments), this function frees no
memory and does not touch any locks. All it does is "delete" the list
heads -- which just means they'll be dangling, and we'll need to re-init
them if we use them again.

It seems like this code would work OK as a sort of canary for using the
list after we've torn everything down, so it's fine to keep the code;
let's just get the name and comments to match what's actually happening.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2; noticed when bugfixing/reworking other parts of this series
---
 drivers/net/wireless/marvell/mwifiex/init.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index de96675e43d5..de974e8bb9c6 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -373,15 +373,13 @@ void mwifiex_stop_net_dev_queue(struct net_device *netdev,
 }
 
 /*
- *  This function releases the lock variables and frees the locks and
- *  associated locks.
+ * This function invalidates the list heads.
  */
-static void mwifiex_free_lock_list(struct mwifiex_adapter *adapter)
+static void mwifiex_invalidate_lists(struct mwifiex_adapter *adapter)
 {
 	struct mwifiex_private *priv;
 	s32 i, j;
 
-	/* Free lists */
 	list_del(&adapter->cmd_free_q);
 	list_del(&adapter->cmd_pending_q);
 	list_del(&adapter->scan_pending_q);
@@ -422,8 +420,7 @@ mwifiex_adapter_cleanup(struct mwifiex_adapter *adapter)
 
 void mwifiex_free_cmd_buffers(struct mwifiex_adapter *adapter)
 {
-	/* Free lock variables */
-	mwifiex_free_lock_list(adapter);
+	mwifiex_invalidate_lists(adapter);
 
 	/* Free command buffer */
 	mwifiex_dbg(adapter, INFO, "info: free cmd buffer\n");
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 10/20] mwifiex: make mwifiex_free_cmd_buffer() return void
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (8 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 09/20] mwifiex: fix misnomers in mwifiex_free_lock_list() Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 11/20] mwifiex: utilize netif_tx_{wake,stop}_all_queues() Brian Norris
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

It doesn't fail.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2; noticed when reworking driver
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c | 6 ++----
 drivers/net/wireless/marvell/mwifiex/main.h   | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 8dad52886034..6ff8e84b01e0 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -427,7 +427,7 @@ int mwifiex_alloc_cmd_buffer(struct mwifiex_adapter *adapter)
  * The function calls the completion callback for all the command
  * buffers that still have response buffers associated with them.
  */
-int mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter)
+void mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter)
 {
 	struct cmd_ctrl_node *cmd_array;
 	u32 i;
@@ -436,7 +436,7 @@ int mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter)
 	if (!adapter->cmd_pool) {
 		mwifiex_dbg(adapter, FATAL,
 			    "info: FREE_CMD_BUF: cmd_pool is null\n");
-		return 0;
+		return;
 	}
 
 	cmd_array = adapter->cmd_pool;
@@ -464,8 +464,6 @@ int mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter)
 		kfree(adapter->cmd_pool);
 		adapter->cmd_pool = NULL;
 	}
-
-	return 0;
 }
 
 /*
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 62ce4e81f695..2bee5cdf1fc8 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1077,7 +1077,7 @@ int mwifiex_get_debug_info(struct mwifiex_private *,
 			   struct mwifiex_debug_info *);
 
 int mwifiex_alloc_cmd_buffer(struct mwifiex_adapter *adapter);
-int mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter);
+void mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter);
 void mwifiex_free_cmd_buffers(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 11/20] mwifiex: utilize netif_tx_{wake,stop}_all_queues()
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (9 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 10/20] mwifiex: make mwifiex_free_cmd_buffer() return void Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 12/20] mwifiex: don't open-code ARRAY_SIZE() Brian Norris
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

We're open-coding these. Just use the helpers.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/init.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index de974e8bb9c6..e11919db7818 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -337,17 +337,9 @@ void mwifiex_wake_up_net_dev_queue(struct net_device *netdev,
 					struct mwifiex_adapter *adapter)
 {
 	unsigned long dev_queue_flags;
-	unsigned int i;
 
 	spin_lock_irqsave(&adapter->queue_lock, dev_queue_flags);
-
-	for (i = 0; i < netdev->num_tx_queues; i++) {
-		struct netdev_queue *txq = netdev_get_tx_queue(netdev, i);
-
-		if (netif_tx_queue_stopped(txq))
-			netif_tx_wake_queue(txq);
-	}
-
+	netif_tx_wake_all_queues(netdev);
 	spin_unlock_irqrestore(&adapter->queue_lock, dev_queue_flags);
 }
 
@@ -358,17 +350,9 @@ void mwifiex_stop_net_dev_queue(struct net_device *netdev,
 					struct mwifiex_adapter *adapter)
 {
 	unsigned long dev_queue_flags;
-	unsigned int i;
 
 	spin_lock_irqsave(&adapter->queue_lock, dev_queue_flags);
-
-	for (i = 0; i < netdev->num_tx_queues; i++) {
-		struct netdev_queue *txq = netdev_get_tx_queue(netdev, i);
-
-		if (!netif_tx_queue_stopped(txq))
-			netif_tx_stop_queue(txq);
-	}
-
+	netif_tx_stop_all_queues(netdev);
 	spin_unlock_irqrestore(&adapter->queue_lock, dev_queue_flags);
 }
 
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 12/20] mwifiex: don't open-code ARRAY_SIZE()
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (10 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 11/20] mwifiex: utilize netif_tx_{wake,stop}_all_queues() Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 13/20] mwifiex: drop 'add_tail' param from mwifiex_insert_cmd_to_pending_q() Brian Norris
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/cfp.c         | 4 +---
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c     | 8 ++------
 drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 5 ++---
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfp.c b/drivers/net/wireless/marvell/mwifiex/cfp.c
index 6e2994308526..bfe84e55df77 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfp.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfp.c
@@ -180,11 +180,9 @@ static struct region_code_mapping region_code_mapping_t[] = {
 u8 *mwifiex_11d_code_2_region(u8 code)
 {
 	u8 i;
-	u8 size = sizeof(region_code_mapping_t)/
-				sizeof(struct region_code_mapping);
 
 	/* Look for code in mapping table */
-	for (i = 0; i < size; i++)
+	for (i = 0; i < ARRAY_SIZE(region_code_mapping_t); i++)
 		if (region_code_mapping_t[i].code == code)
 			return region_code_mapping_t[i].region;
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index 534d94a206a5..b71ad4de5e54 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -189,9 +189,7 @@ static int mwifiex_cmd_tx_rate_cfg(struct mwifiex_private *priv,
 	if (pbitmap_rates != NULL) {
 		rate_scope->hr_dsss_rate_bitmap = cpu_to_le16(pbitmap_rates[0]);
 		rate_scope->ofdm_rate_bitmap = cpu_to_le16(pbitmap_rates[1]);
-		for (i = 0;
-		     i < sizeof(rate_scope->ht_mcs_rate_bitmap) / sizeof(u16);
-		     i++)
+		for (i = 0; i < ARRAY_SIZE(rate_scope->ht_mcs_rate_bitmap); i++)
 			rate_scope->ht_mcs_rate_bitmap[i] =
 				cpu_to_le16(pbitmap_rates[2 + i]);
 		if (priv->adapter->fw_api_ver == MWIFIEX_FW_V15) {
@@ -206,9 +204,7 @@ static int mwifiex_cmd_tx_rate_cfg(struct mwifiex_private *priv,
 			cpu_to_le16(priv->bitmap_rates[0]);
 		rate_scope->ofdm_rate_bitmap =
 			cpu_to_le16(priv->bitmap_rates[1]);
-		for (i = 0;
-		     i < sizeof(rate_scope->ht_mcs_rate_bitmap) / sizeof(u16);
-		     i++)
+		for (i = 0; i < ARRAY_SIZE(rate_scope->ht_mcs_rate_bitmap); i++)
 			rate_scope->ht_mcs_rate_bitmap[i] =
 				cpu_to_le16(priv->bitmap_rates[2 + i]);
 		if (priv->adapter->fw_api_ver == MWIFIEX_FW_V15) {
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
index 2945775e83c5..0fba5b10ef2d 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c
@@ -298,9 +298,8 @@ static int mwifiex_ret_tx_rate_cfg(struct mwifiex_private *priv,
 			priv->bitmap_rates[1] =
 				le16_to_cpu(rate_scope->ofdm_rate_bitmap);
 			for (i = 0;
-			     i <
-			     sizeof(rate_scope->ht_mcs_rate_bitmap) /
-			     sizeof(u16); i++)
+			     i < ARRAY_SIZE(rate_scope->ht_mcs_rate_bitmap);
+			     i++)
 				priv->bitmap_rates[2 + i] =
 					le16_to_cpu(rate_scope->
 						    ht_mcs_rate_bitmap[i]);
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 13/20] mwifiex: drop 'add_tail' param from mwifiex_insert_cmd_to_pending_q()
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (11 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 12/20] mwifiex: don't open-code ARRAY_SIZE() Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 14/20] mwifiex: pcie: remove unnecessary masks Brian Norris
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

It's always called with 'true' -- we only determine it 'false' locally
within this function. So drop the parameter.

Also, this should be 'bool' (since we use true/false), not 'u32'.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: no change
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c | 5 +++--
 drivers/net/wireless/marvell/mwifiex/main.h   | 3 +--
 drivers/net/wireless/marvell/mwifiex/scan.c   | 5 ++---
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 6ff8e84b01e0..3f5e822673bf 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -664,7 +664,7 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
 	    cmd_no == HostCmd_CMD_802_11_SCAN_EXT) {
 		mwifiex_queue_scan_cmd(priv, cmd_node);
 	} else {
-		mwifiex_insert_cmd_to_pending_q(adapter, cmd_node, true);
+		mwifiex_insert_cmd_to_pending_q(adapter, cmd_node);
 		queue_work(adapter->workqueue, &adapter->main_work);
 		if (cmd_node->wait_q_enabled)
 			ret = mwifiex_wait_queue_complete(adapter, cmd_node);
@@ -682,11 +682,12 @@ int mwifiex_send_cmd(struct mwifiex_private *priv, u16 cmd_no,
  */
 void
 mwifiex_insert_cmd_to_pending_q(struct mwifiex_adapter *adapter,
-				struct cmd_ctrl_node *cmd_node, u32 add_tail)
+				struct cmd_ctrl_node *cmd_node)
 {
 	struct host_cmd_ds_command *host_cmd = NULL;
 	u16 command;
 	unsigned long flags;
+	bool add_tail = true;
 
 	host_cmd = (struct host_cmd_ds_command *) (cmd_node->cmd_skb->data);
 	if (!host_cmd) {
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 2bee5cdf1fc8..909bd1ad3838 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1088,8 +1088,7 @@ void mwifiex_recycle_cmd_node(struct mwifiex_adapter *adapter,
 			      struct cmd_ctrl_node *cmd_node);
 
 void mwifiex_insert_cmd_to_pending_q(struct mwifiex_adapter *adapter,
-				     struct cmd_ctrl_node *cmd_node,
-				     u32 addtail);
+				     struct cmd_ctrl_node *cmd_node);
 
 int mwifiex_exec_next_cmd(struct mwifiex_adapter *adapter);
 int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter);
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index ae9630b49342..16eaeae74979 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1534,8 +1534,7 @@ int mwifiex_scan_networks(struct mwifiex_private *priv,
 			list_del(&cmd_node->list);
 			spin_unlock_irqrestore(&adapter->scan_pending_q_lock,
 					       flags);
-			mwifiex_insert_cmd_to_pending_q(adapter, cmd_node,
-							true);
+			mwifiex_insert_cmd_to_pending_q(adapter, cmd_node);
 			queue_work(adapter->workqueue, &adapter->main_work);
 
 			/* Perform internal scan synchronously */
@@ -2033,7 +2032,7 @@ static void mwifiex_check_next_scan_command(struct mwifiex_private *priv)
 					    struct cmd_ctrl_node, list);
 		list_del(&cmd_node->list);
 		spin_unlock_irqrestore(&adapter->scan_pending_q_lock, flags);
-		mwifiex_insert_cmd_to_pending_q(adapter, cmd_node, true);
+		mwifiex_insert_cmd_to_pending_q(adapter, cmd_node);
 	}
 
 	return;
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 14/20] mwifiex: pcie: remove unnecessary masks
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (12 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 13/20] mwifiex: drop 'add_tail' param from mwifiex_insert_cmd_to_pending_q() Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 15/20] mwifiex: pcie: unify MSI-X / non-MSI-X interrupt process Brian Norris
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

After removing the interrupt loop in commit 5d5ddb5e0d9b ("mwifiex:
pcie: don't loop/retry interrupt status checks"), we don't need to keep
track of the cleared interrupts (actually, we didn't need to do that
before, but we *really* don't need to now).

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 5c07edd4e094..2f4da08f127c 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2460,28 +2460,24 @@ static int mwifiex_process_pcie_int(struct mwifiex_adapter *adapter)
 	}
 
 	if (pcie_ireg & HOST_INTR_DNLD_DONE) {
-		pcie_ireg &= ~HOST_INTR_DNLD_DONE;
 		mwifiex_dbg(adapter, INTR, "info: TX DNLD Done\n");
 		ret = mwifiex_pcie_send_data_complete(adapter);
 		if (ret)
 			return ret;
 	}
 	if (pcie_ireg & HOST_INTR_UPLD_RDY) {
-		pcie_ireg &= ~HOST_INTR_UPLD_RDY;
 		mwifiex_dbg(adapter, INTR, "info: Rx DATA\n");
 		ret = mwifiex_pcie_process_recv_data(adapter);
 		if (ret)
 			return ret;
 	}
 	if (pcie_ireg & HOST_INTR_EVENT_RDY) {
-		pcie_ireg &= ~HOST_INTR_EVENT_RDY;
 		mwifiex_dbg(adapter, INTR, "info: Rx EVENT\n");
 		ret = mwifiex_pcie_process_event_ready(adapter);
 		if (ret)
 			return ret;
 	}
 	if (pcie_ireg & HOST_INTR_CMD_DONE) {
-		pcie_ireg &= ~HOST_INTR_CMD_DONE;
 		if (adapter->cmd_sent) {
 			mwifiex_dbg(adapter, INTR,
 				    "info: CMD sent Interrupt\n");
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 15/20] mwifiex: pcie: unify MSI-X / non-MSI-X interrupt process
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (13 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 14/20] mwifiex: pcie: remove unnecessary masks Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 16/20] mwifiex: debugfs: allow card_reset() to cancel things Brian Norris
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

After removing the interrupt loop in commit 5d5ddb5e0d9b ("mwifiex:
pcie: don't loop/retry interrupt status checks"), there is practically
zero difference between mwifiex_process_pcie_int() (which handled legacy
PCI interrupts and MSI interrupts) and mwifiex_process_msix_int() (which
handled MSI-X interrupts). Let's add the one relevant line to
mwifiex_process_pcie_int() and kill the copy-and-paste.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 68 ++---------------------------
 1 file changed, 3 insertions(+), 65 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 2f4da08f127c..c08ebb55a7e8 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2417,7 +2417,7 @@ static irqreturn_t mwifiex_pcie_interrupt(int irq, void *context)
  * In case of Rx packets received, the packets are uploaded from card to
  * host and processed accordingly.
  */
-static int mwifiex_process_pcie_int(struct mwifiex_adapter *adapter)
+static int mwifiex_process_int_status(struct mwifiex_adapter *adapter)
 {
 	int ret;
 	u32 pcie_ireg = 0;
@@ -2492,75 +2492,13 @@ static int mwifiex_process_pcie_int(struct mwifiex_adapter *adapter)
 	mwifiex_dbg(adapter, INTR,
 		    "info: cmd_sent=%d data_sent=%d\n",
 		    adapter->cmd_sent, adapter->data_sent);
-	if (!card->msi_enable && adapter->ps_state != PS_STATE_SLEEP)
+	if (!card->msi_enable && !card->msix_enable &&
+				 adapter->ps_state != PS_STATE_SLEEP)
 		mwifiex_pcie_enable_host_int(adapter);
 
 	return 0;
 }
 
-static int mwifiex_process_msix_int(struct mwifiex_adapter *adapter)
-{
-	int ret;
-	u32 pcie_ireg;
-	unsigned long flags;
-
-	spin_lock_irqsave(&adapter->int_lock, flags);
-	/* Clear out unused interrupts */
-	pcie_ireg = adapter->int_status;
-	adapter->int_status = 0;
-	spin_unlock_irqrestore(&adapter->int_lock, flags);
-
-	if (pcie_ireg & HOST_INTR_DNLD_DONE) {
-		mwifiex_dbg(adapter, INTR,
-			    "info: TX DNLD Done\n");
-		ret = mwifiex_pcie_send_data_complete(adapter);
-		if (ret)
-			return ret;
-	}
-	if (pcie_ireg & HOST_INTR_UPLD_RDY) {
-		mwifiex_dbg(adapter, INTR,
-			    "info: Rx DATA\n");
-		ret = mwifiex_pcie_process_recv_data(adapter);
-		if (ret)
-			return ret;
-	}
-	if (pcie_ireg & HOST_INTR_EVENT_RDY) {
-		mwifiex_dbg(adapter, INTR,
-			    "info: Rx EVENT\n");
-		ret = mwifiex_pcie_process_event_ready(adapter);
-		if (ret)
-			return ret;
-	}
-
-	if (pcie_ireg & HOST_INTR_CMD_DONE) {
-		if (adapter->cmd_sent) {
-			mwifiex_dbg(adapter, INTR,
-				    "info: CMD sent Interrupt\n");
-			adapter->cmd_sent = false;
-		}
-		/* Handle command response */
-		ret = mwifiex_pcie_process_cmd_complete(adapter);
-		if (ret)
-			return ret;
-	}
-
-	mwifiex_dbg(adapter, INTR,
-		    "info: cmd_sent=%d data_sent=%d\n",
-		    adapter->cmd_sent, adapter->data_sent);
-
-	return 0;
-}
-
-static int mwifiex_process_int_status(struct mwifiex_adapter *adapter)
-{
-	struct pcie_service_card *card = adapter->card;
-
-	if (card->msix_enable)
-		return mwifiex_process_msix_int(adapter);
-	else
-		return mwifiex_process_pcie_int(adapter);
-}
-
 /*
  * This function downloads data from driver to card.
  *
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 16/20] mwifiex: debugfs: allow card_reset() to cancel things
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (14 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 15/20] mwifiex: pcie: unify MSI-X / non-MSI-X interrupt process Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 17/20] mwifiex: pcie: disable device DMA before unmapping/freeing buffers Brian Norris
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

The card_reset() implementation should be setting our state flags and
cancelling commands for us (i.e., in mwifiex_shutdown_drv()), so let's
not do it here.

Also, this debugfs file is useful for testing and debugging the reset
feature, so we shouldn't do extra preparatory steps here, as that might
cause different reset behavior, which could either cause new bugs or
paper over existing ones that this debug feature should otherwise help
us catch.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/debugfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/debugfs.c b/drivers/net/wireless/marvell/mwifiex/debugfs.c
index f6f105a7d3ff..6f4239be609d 100644
--- a/drivers/net/wireless/marvell/mwifiex/debugfs.c
+++ b/drivers/net/wireless/marvell/mwifiex/debugfs.c
@@ -940,8 +940,6 @@ mwifiex_reset_write(struct file *file,
 
 	if (adapter->if_ops.card_reset) {
 		dev_info(adapter->dev, "Resetting per request\n");
-		adapter->hw_status = MWIFIEX_HW_STATUS_RESET;
-		mwifiex_cancel_all_pending_cmd(adapter);
 		adapter->if_ops.card_reset(adapter);
 	}
 
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 17/20] mwifiex: pcie: disable device DMA before unmapping/freeing buffers
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (15 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 16/20] mwifiex: debugfs: allow card_reset() to cancel things Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 18/20] mwifiex: pcie: remove unnecessary 'pdev' check Brian Norris
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

In testing the mwifiex reset code path, I've noticed KASAN complaining
about some "overwritten poison values" in our RX buffer descriptors.
Because KASAN didn't notice this at the time of a CPU write, this seems
to suggest that the device is writing to this memory.

This makes a little sense, because when resetting, we don't necessarily
expect the device to be responsive, so we don't have a chance to disable
everything cleanly.

We can at least take the precaution of disabling DMA for the device
though, and in my testing that seems to clear up this particular issue.

This patch reorders the removal path so that we disable the device
*before* releasing our last PCIe buffers, and it clears/sets the bus
master feature from the PCI device when resetting.

Along the way, remove the insufficient (and confusing) error path in
mwifiex_pcie_up_dev() (it doesn't unwind things well enough, and it
doesn't propagate its errors upward anyway).

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index c08ebb55a7e8..a1907e8e620f 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2958,15 +2958,17 @@ static void mwifiex_cleanup_pcie(struct mwifiex_adapter *adapter)
 				    "Failed to write driver not-ready signature\n");
 	}
 
-	mwifiex_pcie_free_buffers(adapter);
-
 	if (pdev) {
+		pci_disable_device(pdev);
+
 		pci_iounmap(pdev, card->pci_mmap);
 		pci_iounmap(pdev, card->pci_mmap1);
 		pci_disable_device(pdev);
 		pci_release_region(pdev, 2);
 		pci_release_region(pdev, 0);
 	}
+
+	mwifiex_pcie_free_buffers(adapter);
 }
 
 static int mwifiex_pcie_request_irq(struct mwifiex_adapter *adapter)
@@ -3142,7 +3144,6 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
 static void mwifiex_pcie_up_dev(struct mwifiex_adapter *adapter)
 {
 	struct pcie_service_card *card = adapter->card;
-	int ret;
 	struct pci_dev *pdev = card->dev;
 
 	/* tx_buf_size might be changed to 3584 by firmware during
@@ -3150,11 +3151,9 @@ static void mwifiex_pcie_up_dev(struct mwifiex_adapter *adapter)
 	 */
 	adapter->tx_buf_size = card->pcie.tx_buf_size;
 
-	ret = mwifiex_pcie_alloc_buffers(adapter);
-	if (!ret)
-		return;
+	mwifiex_pcie_alloc_buffers(adapter);
 
-	pci_iounmap(pdev, card->pci_mmap1);
+	pci_set_master(pdev);
 }
 
 /* This function cleans up the PCI-E host memory space. */
@@ -3162,10 +3161,13 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
 {
 	struct pcie_service_card *card = adapter->card;
 	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
+	struct pci_dev *pdev = card->dev;
 
 	if (mwifiex_write_reg(adapter, reg->drv_rdy, 0x00000000))
 		mwifiex_dbg(adapter, ERROR, "Failed to write driver not-ready signature\n");
 
+	pci_clear_master(pdev);
+
 	adapter->seq_num = 0;
 
 	mwifiex_pcie_free_buffers(adapter);
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 18/20] mwifiex: pcie: remove unnecessary 'pdev' check
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (16 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 17/20] mwifiex: pcie: disable device DMA before unmapping/freeing buffers Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 19/20] mwifiex: keep mwifiex_cancel_pending_ioctl() static Brian Norris
  2017-07-25  1:13 ` [PATCH v2 20/20] mwifiex: drop num CPU notice Brian Norris
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

'card->dev' is initialized once and is never cleared. Drop the
unnecessary "safety" check, as it simply obscures things, and we don't
do this check everywhere (and therefore it's not really "safe").

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index a1907e8e620f..1993b9b339df 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2958,15 +2958,12 @@ static void mwifiex_cleanup_pcie(struct mwifiex_adapter *adapter)
 				    "Failed to write driver not-ready signature\n");
 	}
 
-	if (pdev) {
-		pci_disable_device(pdev);
+	pci_disable_device(pdev);
 
-		pci_iounmap(pdev, card->pci_mmap);
-		pci_iounmap(pdev, card->pci_mmap1);
-		pci_disable_device(pdev);
-		pci_release_region(pdev, 2);
-		pci_release_region(pdev, 0);
-	}
+	pci_iounmap(pdev, card->pci_mmap);
+	pci_iounmap(pdev, card->pci_mmap1);
+	pci_release_region(pdev, 2);
+	pci_release_region(pdev, 0);
 
 	mwifiex_pcie_free_buffers(adapter);
 }
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 19/20] mwifiex: keep mwifiex_cancel_pending_ioctl() static
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (17 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 18/20] mwifiex: pcie: remove unnecessary 'pdev' check Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  2017-07-25  1:13 ` [PATCH v2 20/20] mwifiex: drop num CPU notice Brian Norris
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

It has some scary comments about "only being called" from the timeout
handler, so let's help keep it that way.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c | 4 +++-
 drivers/net/wireless/marvell/mwifiex/main.h   | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 3f5e822673bf..0edc5d621304 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -26,6 +26,8 @@
 #include "11n.h"
 #include "11ac.h"
 
+static void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
+
 /*
  * This function initializes a command node.
  *
@@ -1074,7 +1076,7 @@ mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter)
  * In case of scan commands, all pending commands in scan pending queue
  * are cancelled.
  */
-void
+static void
 mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter)
 {
 	struct cmd_ctrl_node *cmd_node = NULL;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 909bd1ad3838..537a0ad795ff 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1080,7 +1080,6 @@ int mwifiex_alloc_cmd_buffer(struct mwifiex_adapter *adapter);
 void mwifiex_free_cmd_buffer(struct mwifiex_adapter *adapter);
 void mwifiex_free_cmd_buffers(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_all_pending_cmd(struct mwifiex_adapter *adapter);
-void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_pending_scan_cmd(struct mwifiex_adapter *adapter);
 void mwifiex_cancel_scan(struct mwifiex_adapter *adapter);
 
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* [PATCH v2 20/20] mwifiex: drop num CPU notice
  2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
                   ` (18 preceding siblings ...)
  2017-07-25  1:13 ` [PATCH v2 19/20] mwifiex: keep mwifiex_cancel_pending_ioctl() static Brian Norris
@ 2017-07-25  1:13 ` Brian Norris
  19 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-07-25  1:13 UTC (permalink / raw)
  To: Ganapathi Bhat, Nishant Sarmukadam
  Cc: linux-kernel, Dmitry Torokhov, Amitkumar Karwar, Kalle Valo,
	linux-wireless, Brian Norris

This print isn't very useful. It's also different between
mwifiex_add_card() and mwifiex_reinit_sw(), and I'd like to consolidate
them eventually.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
new in v2
---
 drivers/net/wireless/marvell/mwifiex/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 0448dcc07139..13fc7b6ed11d 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1619,10 +1619,8 @@ mwifiex_add_card(void *card, struct completion *fw_done,
 	adapter->cmd_wait_q.status = 0;
 	adapter->scan_wait_q_woken = false;
 
-	if ((num_possible_cpus() > 1) || adapter->iface_type == MWIFIEX_USB) {
+	if ((num_possible_cpus() > 1) || adapter->iface_type == MWIFIEX_USB)
 		adapter->rx_work_enabled = true;
-		pr_notice("rx work enabled, cpus %d\n", num_possible_cpus());
-	}
 
 	adapter->workqueue =
 		alloc_workqueue("MWIFIEX_WORK_QUEUE",
-- 
2.14.0.rc0.284.gd933b75aa4-goog

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

* Re: [v2,01/20] mwifiex: reunite copy-and-pasted remove/reset code
  2017-07-25  1:13 ` [PATCH v2 01/20] mwifiex: reunite copy-and-pasted remove/reset code Brian Norris
@ 2017-07-28 14:49   ` Kalle Valo
  0 siblings, 0 replies; 22+ messages in thread
From: Kalle Valo @ 2017-07-28 14:49 UTC (permalink / raw)
  To: Brian Norris
  Cc: Ganapathi Bhat, Nishant Sarmukadam, linux-kernel,
	Dmitry Torokhov, Amitkumar Karwar, linux-wireless, Brian Norris

Brian Norris <briannorris@chromium.org> wrote:

> When PCIe FLR code was added, it explicitly copy-and-pasted much of
> mwifiex_remove_card() into mwifiex_shutdown_sw(). This is unnecessary,
> as almost all of the code should be reused.
> 
> Let's reunite what we can for now.
> 
> The only functional changes for now:
> 
>  * call netif_device_detach() in the remove() code path -- this wasn't
>    done before, but it really should be a no-op, when the device is
>    getting totally unregistered soon anyway
> 
>  * call the ->down_dev() driver callback only after we've finished all
>    SW teardown -- this should have no significant effect, since the only
>    user (pcie.c) does very minimal work there, and it doesn't matter
>    that we reorder this
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

20 patches applied to wireless-drivers-next.git, thanks.

b6658b66d8a6 mwifiex: reunite copy-and-pasted remove/reset code
4b1f5a0d2eeb mwifiex: reset interrupt status across device reset
7dc4a6b5ca94 mwifiex: pcie: don't allow cmd buffer reuse after reset
643acea6297f mwifiex: re-register wiphy across reset
ce32d1d83702 mwifiex: unregister wiphy before freeing resources
6417dba33538 mwifiex: don't short-circuit netdev notifiers on interface deletion
c253a62da9b4 mwifiex: fixup init_channel_scan_gap error case
9557d9f2e62b mwifiex: ensure "disable auto DS" struct is initialized
5e6588b9d4ab mwifiex: fix misnomers in mwifiex_free_lock_list()
f7d7e4b689ca mwifiex: make mwifiex_free_cmd_buffer() return void
fe8d730adaee mwifiex: utilize netif_tx_{wake,stop}_all_queues()
8395fd9b194c mwifiex: don't open-code ARRAY_SIZE()
463df4719084 mwifiex: drop 'add_tail' param from mwifiex_insert_cmd_to_pending_q()
605db27f7405 mwifiex: pcie: remove unnecessary masks
87a602126aaf mwifiex: pcie: unify MSI-X / non-MSI-X interrupt process
37680819c6e1 mwifiex: debugfs: allow card_reset() to cancel things
2f47150ab3ef mwifiex: pcie: disable device DMA before unmapping/freeing buffers
43a0c9aea64d mwifiex: pcie: remove unnecessary 'pdev' check
2d98cfd17e92 mwifiex: keep mwifiex_cancel_pending_ioctl() static
0bc03cfd8247 mwifiex: drop num CPU notice

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

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

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

end of thread, other threads:[~2017-07-28 14:49 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-25  1:13 [PATCH v2 00/20] mwifiex: "reset" bugfixes and other refactorings Brian Norris
2017-07-25  1:13 ` [PATCH v2 01/20] mwifiex: reunite copy-and-pasted remove/reset code Brian Norris
2017-07-28 14:49   ` [v2,01/20] " Kalle Valo
2017-07-25  1:13 ` [PATCH v2 02/20] mwifiex: reset interrupt status across device reset Brian Norris
2017-07-25  1:13 ` [PATCH v2 03/20] mwifiex: pcie: don't allow cmd buffer reuse after reset Brian Norris
2017-07-25  1:13 ` [PATCH v2 04/20] mwifiex: re-register wiphy across reset Brian Norris
2017-07-25  1:13 ` [PATCH v2 05/20] mwifiex: unregister wiphy before freeing resources Brian Norris
2017-07-25  1:13 ` [PATCH v2 06/20] mwifiex: don't short-circuit netdev notifiers on interface deletion Brian Norris
2017-07-25  1:13 ` [PATCH v2 07/20] mwifiex: fixup init_channel_scan_gap error case Brian Norris
2017-07-25  1:13 ` [PATCH v2 08/20] mwifiex: ensure "disable auto DS" struct is initialized Brian Norris
2017-07-25  1:13 ` [PATCH v2 09/20] mwifiex: fix misnomers in mwifiex_free_lock_list() Brian Norris
2017-07-25  1:13 ` [PATCH v2 10/20] mwifiex: make mwifiex_free_cmd_buffer() return void Brian Norris
2017-07-25  1:13 ` [PATCH v2 11/20] mwifiex: utilize netif_tx_{wake,stop}_all_queues() Brian Norris
2017-07-25  1:13 ` [PATCH v2 12/20] mwifiex: don't open-code ARRAY_SIZE() Brian Norris
2017-07-25  1:13 ` [PATCH v2 13/20] mwifiex: drop 'add_tail' param from mwifiex_insert_cmd_to_pending_q() Brian Norris
2017-07-25  1:13 ` [PATCH v2 14/20] mwifiex: pcie: remove unnecessary masks Brian Norris
2017-07-25  1:13 ` [PATCH v2 15/20] mwifiex: pcie: unify MSI-X / non-MSI-X interrupt process Brian Norris
2017-07-25  1:13 ` [PATCH v2 16/20] mwifiex: debugfs: allow card_reset() to cancel things Brian Norris
2017-07-25  1:13 ` [PATCH v2 17/20] mwifiex: pcie: disable device DMA before unmapping/freeing buffers Brian Norris
2017-07-25  1:13 ` [PATCH v2 18/20] mwifiex: pcie: remove unnecessary 'pdev' check Brian Norris
2017-07-25  1:13 ` [PATCH v2 19/20] mwifiex: keep mwifiex_cancel_pending_ioctl() static Brian Norris
2017-07-25  1:13 ` [PATCH v2 20/20] mwifiex: drop num CPU notice Brian Norris

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