Hi Dmitry,

From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
Sent: Tuesday, October 25, 2016 5:44 AM
To: Amitkumar Karwar
Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
briannorris@google.com; Xinming Hu
Subject: Re: [PATCH 5/5] mwifiex: wait for firmware dump completion in
remove_card

On Mon, Oct 24, 2016 at 07:51:32PM +0530, Amitkumar Karwar wrote:

quoted

From: Xinming Hu <redacted>

This patch ensures to wait for firmware dump completion in
mwifiex_remove_card().

For sdio interface, reset_trigger variable is used to identify if
mwifiex_sdio_remove() is called by sdio_work during reset or the call
is from sdio subsystem.

This patch fixes a kernel crash observed during reboot when firmware
dump operation is in process.

Signed-off-by: Xinming Hu <redacted>
Signed-off-by: Amitkumar Karwar <redacted>
---
 drivers/net/wireless/marvell/mwifiex/pcie.c |  2 ++
drivers/net/wireless/marvell/mwifiex/sdio.c | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c

b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 986bf07..4512e86 100644

--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c

@@ -528,6 +528,8 @@ static void mwifiex_pcie_remove(struct pci_dev

*pdev)

quoted

 	if (!adapter || !adapter->priv_num)
 		return;

+	cancel_work_sync(&pcie_work);
+
 	if (user_rmmod && !adapter->mfg_mode) {  #ifdef CONFIG_PM_SLEEP
 		if (adapter->is_suspended)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c

b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 4cad1c2..f974260 100644

--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c

@@ -46,6 +46,15 @@
  */
 static u8 user_rmmod;

+/* reset_trigger variable is used to identify if
+mwifiex_sdio_remove()
+ * is called by sdio_work during reset or the call is from sdio

subsystem.

quoted

+ * We will cancel sdio_work only if the call is from sdio subsystem.
+ */
+static u8 reset_triggered;

It would be really great if the driver supported multiple devices. IOW
please avoid module-globals.

You are right. As Brian pointed out in other email, I will refactor the code and get rid of global variable.

quoted

+
+static void mwifiex_sdio_work(struct work_struct *work); static
+DECLARE_WORK(sdio_work, mwifiex_sdio_work);
+
 static struct mwifiex_if_ops sdio_ops;  static unsigned long
iface_work_flags;

@@ -289,6 +298,9 @@ mwifiex_sdio_remove(struct sdio_func *func)
 	if (!adapter || !adapter->priv_num)
 		return;

+	if (!reset_triggered)
+		cancel_work_sync(&sdio_work);
+
 	mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num);

 	if (user_rmmod && !adapter->mfg_mode) { @@ -2290,7 +2302,9 @@

static

quoted

void mwifiex_recreate_adapter(struct sdio_mmc_card *card)
 	 * discovered and initializes them from scratch.
 	 */

+	reset_triggered = 1;
 	mwifiex_sdio_remove(func);
+	reset_triggered = 0;

What exactly happens if I trigger mwifiex_sdio_remove() from sysfs at
the same time this code is running?

Yes. There would be a race. It will be avoided with Brian's code refactoring approach.

Regards,
Amitkumar