[PATCH v2 1/5] mwifiex: remove redundant condition in main process

[PATCH v2 1/5] mwifiex: remove redundant condition in main process

[Amitkumar Karwar]

This condition while calling mwifiex_check_ps_cond() is redundant.
The function internally already takes care of it.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: Same as v1
---
 drivers/net/wireless/marvell/mwifiex/main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 2478ccd..3b31ea2 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -355,10 +355,8 @@ process_start:
 
 		/* Check if we need to confirm Sleep Request
 		   received previously */
-		if (adapter->ps_state == PS_STATE_PRE_SLEEP) {
-			if (!adapter->cmd_sent && !adapter->curr_cmd)
-				mwifiex_check_ps_cond(adapter);
-		}
+		if (adapter->ps_state == PS_STATE_PRE_SLEEP)
+			mwifiex_check_ps_cond(adapter);
 
 		/* * The ps_state may have been changed during processing of
 		 * Sleep Request event.
-- 
1.9.1

[PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Amitkumar Karwar]

This variable is guarded by spinlock at all other places. This patch
takes care of missing spinlock usage in mwifiex_shutdown_drv().

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: Same as v1
---
 drivers/net/wireless/marvell/mwifiex/init.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 82839d9..8e5e424 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -670,11 +670,14 @@ mwifiex_shutdown_drv(struct mwifiex_adapter *adapter)
 
 	adapter->hw_status = MWIFIEX_HW_STATUS_CLOSING;
 	/* wait for mwifiex_process to complete */
+	spin_lock_irqsave(&adapter->main_proc_lock, flags);
 	if (adapter->mwifiex_processing) {
+		spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
 		mwifiex_dbg(adapter, WARN,
 			    "main process is still running\n");
 		return ret;
 	}
+	spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
 
 	/* cancel current command */
 	if (adapter->curr_cmd) {
-- 
1.9.1

[PATCH v2 3/5] mwifiex: do not free firmware dump memory in shutdown_drv

[Amitkumar Karwar]

From: Xinming Hu <huxm@marvell.com>

mwifiex_upload_device_dump() already takes care of freeing firmware dump
memory. Doing the same thing in mwifiex_shutdown_drv() is redundant.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: Same as v1. 
drv_info_dump and drv_info_size need not be in adapter structure. Saparate
patch is created for this (Dmitry Torokhov)
---
 drivers/net/wireless/marvell/mwifiex/init.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 8e5e424..365efb8 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -407,8 +407,6 @@ static void mwifiex_free_lock_list(struct mwifiex_adapter *adapter)
 static void
 mwifiex_adapter_cleanup(struct mwifiex_adapter *adapter)
 {
-	int idx;
-
 	if (!adapter) {
 		pr_err("%s: adapter is NULL\n", __func__);
 		return;
@@ -426,23 +424,6 @@ mwifiex_adapter_cleanup(struct mwifiex_adapter *adapter)
 	mwifiex_dbg(adapter, INFO, "info: free cmd buffer\n");
 	mwifiex_free_cmd_buffer(adapter);
 
-	for (idx = 0; idx < adapter->num_mem_types; idx++) {
-		struct memory_type_mapping *entry =
-				&adapter->mem_type_mapping_tbl[idx];
-
-		if (entry->mem_ptr) {
-			vfree(entry->mem_ptr);
-			entry->mem_ptr = NULL;
-		}
-		entry->mem_size = 0;
-	}
-
-	if (adapter->drv_info_dump) {
-		vfree(adapter->drv_info_dump);
-		adapter->drv_info_dump = NULL;
-		adapter->drv_info_size = 0;
-	}
-
 	if (adapter->sleep_cfm)
 		dev_kfree_skb_any(adapter->sleep_cfm);
 }
-- 
1.9.1

[PATCH v2 4/5] mwifiex: get rid of drv_info* adapter variables

[Amitkumar Karwar]

From: Xinming Hu <huxm@marvell.com>

We can avoid drv_info_dump and drv_info_size adapter variables.
This info can be passed to mwifiex_upload_device_dump() as parameters

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: This patch is introduced in v2. 
Also, I have dropped "v1 4/5 mwifiex: firmware dump code rearrangement
in pcie.c" patch in this series to avoid rebase efforts for other patches
from Rajat/Brian in queue. I will submit this later.
---
 drivers/net/wireless/marvell/mwifiex/main.c | 42 ++++++++++++-----------------
 drivers/net/wireless/marvell/mwifiex/main.h |  7 +++--
 drivers/net/wireless/marvell/mwifiex/pcie.c |  7 +++--
 drivers/net/wireless/marvell/mwifiex/sdio.c |  6 +++--
 4 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 3b31ea2..158305f 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1031,7 +1031,7 @@ void mwifiex_multi_chan_resync(struct mwifiex_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(mwifiex_multi_chan_resync);
 
-void mwifiex_drv_info_dump(struct mwifiex_adapter *adapter)
+int mwifiex_drv_info_dump(struct mwifiex_adapter *adapter, void **drv_info)
 {
 	void *p;
 	char drv_version[64];
@@ -1041,21 +1041,17 @@ void mwifiex_drv_info_dump(struct mwifiex_adapter *adapter)
 	int i, idx;
 	struct netdev_queue *txq;
 	struct mwifiex_debug_info *debug_info;
-
-	if (adapter->drv_info_dump) {
-		vfree(adapter->drv_info_dump);
-		adapter->drv_info_dump = NULL;
-		adapter->drv_info_size = 0;
-	}
+	void *drv_info_dump;
 
 	mwifiex_dbg(adapter, MSG, "===mwifiex driverinfo dump start===\n");
 
-	adapter->drv_info_dump = vzalloc(MWIFIEX_DRV_INFO_SIZE_MAX);
+	/* memory allocate here should be free in mwifiex_upload_device_dump*/
+	drv_info_dump = vzalloc(MWIFIEX_DRV_INFO_SIZE_MAX);
 
-	if (!adapter->drv_info_dump)
-		return;
+	if (!drv_info_dump)
+		return 0;
 
-	p = (char *)(adapter->drv_info_dump);
+	p = (char *)(drv_info_dump);
 	p += sprintf(p, "driver_name = " "\"mwifiex\"\n");
 
 	mwifiex_drv_get_driver_version(adapter, drv_version,
@@ -1139,18 +1135,20 @@ void mwifiex_drv_info_dump(struct mwifiex_adapter *adapter)
 		kfree(debug_info);
 	}
 
-	adapter->drv_info_size = p - adapter->drv_info_dump;
 	mwifiex_dbg(adapter, MSG, "===mwifiex driverinfo dump end===\n");
+	*drv_info = drv_info_dump;
+	return p - drv_info_dump;
 }
 EXPORT_SYMBOL_GPL(mwifiex_drv_info_dump);
 
-void mwifiex_upload_device_dump(struct mwifiex_adapter *adapter)
+void mwifiex_upload_device_dump(struct mwifiex_adapter *adapter, void *drv_info,
+				int drv_info_size)
 {
 	u8 idx, *dump_data, *fw_dump_ptr;
 	u32 dump_len;
 
 	dump_len = (strlen("========Start dump driverinfo========\n") +
-		       adapter->drv_info_size +
+		       drv_info_size +
 		       strlen("\n========End dump========\n"));
 
 	for (idx = 0; idx < adapter->num_mem_types; idx++) {
@@ -1180,8 +1178,8 @@ void mwifiex_upload_device_dump(struct mwifiex_adapter *adapter)
 
 	strcpy(fw_dump_ptr, "========Start dump driverinfo========\n");
 	fw_dump_ptr += strlen("========Start dump driverinfo========\n");
-	memcpy(fw_dump_ptr, adapter->drv_info_dump, adapter->drv_info_size);
-	fw_dump_ptr += adapter->drv_info_size;
+	memcpy(fw_dump_ptr, drv_info, drv_info_size);
+	fw_dump_ptr += drv_info_size;
 	strcpy(fw_dump_ptr, "\n========End dump========\n");
 	fw_dump_ptr += strlen("\n========End dump========\n");
 
@@ -1219,18 +1217,12 @@ done:
 		struct memory_type_mapping *entry =
 			&adapter->mem_type_mapping_tbl[idx];
 
-		if (entry->mem_ptr) {
-			vfree(entry->mem_ptr);
-			entry->mem_ptr = NULL;
-		}
+		vfree(entry->mem_ptr);
+		entry->mem_ptr = NULL;
 		entry->mem_size = 0;
 	}
 
-	if (adapter->drv_info_dump) {
-		vfree(adapter->drv_info_dump);
-		adapter->drv_info_dump = NULL;
-		adapter->drv_info_size = 0;
-	}
+	vfree(drv_info);
 }
 EXPORT_SYMBOL_GPL(mwifiex_upload_device_dump);
 
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index d61fe3a..a4aca45 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -990,8 +990,6 @@ struct mwifiex_adapter {
 	u8 key_api_major_ver, key_api_minor_ver;
 	struct memory_type_mapping *mem_type_mapping_tbl;
 	u8 num_mem_types;
-	void *drv_info_dump;
-	u32 drv_info_size;
 	bool scan_chan_gap_enabled;
 	struct sk_buff_head rx_data_q;
 	bool mfg_mode;
@@ -1606,8 +1604,9 @@ void mwifiex_hist_data_add(struct mwifiex_private *priv,
 u8 mwifiex_adjust_data_rate(struct mwifiex_private *priv,
 			    u8 rx_rate, u8 ht_info);
 
-void mwifiex_drv_info_dump(struct mwifiex_adapter *adapter);
-void mwifiex_upload_device_dump(struct mwifiex_adapter *adapter);
+int mwifiex_drv_info_dump(struct mwifiex_adapter *adapter, void **drv_info);
+void mwifiex_upload_device_dump(struct mwifiex_adapter *adapter, void *drv_info,
+				int drv_info_size);
 void *mwifiex_alloc_dma_align_buf(int rx_len, gfp_t flags);
 void mwifiex_queue_main_work(struct mwifiex_adapter *adapter);
 int mwifiex_get_wakeup_reason(struct mwifiex_private *priv, u16 action,
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 9147e6a..9025af7 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2699,9 +2699,12 @@ static void mwifiex_pcie_fw_dump(struct mwifiex_adapter *adapter)
 
 static void mwifiex_pcie_device_dump_work(struct mwifiex_adapter *adapter)
 {
-	mwifiex_drv_info_dump(adapter);
+	int drv_info_size;
+	void *drv_info;
+
+	drv_info_size = mwifiex_drv_info_dump(adapter, &drv_info);
 	mwifiex_pcie_fw_dump(adapter);
-	mwifiex_upload_device_dump(adapter);
+	mwifiex_upload_device_dump(adapter, drv_info, drv_info_size);
 }
 
 static unsigned long iface_work_flags;
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 4cad1c2..241d2b3 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -2602,13 +2602,15 @@ done:
 static void mwifiex_sdio_device_dump_work(struct mwifiex_adapter *adapter)
 {
 	struct sdio_mmc_card *card = adapter->card;
+	int drv_info_size;
+	void *drv_info;
 
-	mwifiex_drv_info_dump(adapter);
+	drv_info_size = mwifiex_drv_info_dump(adapter, &drv_info);
 	if (card->fw_dump_enh)
 		mwifiex_sdio_generic_fw_dump(adapter);
 	else
 		mwifiex_sdio_fw_dump(adapter);
-	mwifiex_upload_device_dump(adapter);
+	mwifiex_upload_device_dump(adapter, drv_info, drv_info_size);
 }
 
 static void mwifiex_sdio_work(struct work_struct *work)
-- 
1.9.1

[PATCH v2 5/5] mwifiex: wait firmware dump complete during card remove process

[Amitkumar Karwar]

From: Xinming Hu <huxm@marvell.com>

Wait for firmware dump complete in card remove function.
For sdio interface, there are two diffenrent cases,
card reset trigger sdio_work and firmware dump trigger sdio_work.
Do code rearrangement for distinguish between these two cases.

Signed-off-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: 1. Get rid of reset_triggered flag. Instead split the code and use
    __mwifiex_sdio_remove() (Brian Norris/Dmitry Torokhov)
    2. "v1 4/5 mwifiex: firmware dump code rearrangement.." is dropped. So
    rebased accordingly.
---
 drivers/net/wireless/marvell/mwifiex/pcie.c |  6 +++++-
 drivers/net/wireless/marvell/mwifiex/sdio.c | 15 ++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 9025af7..6c421ad 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -37,6 +37,9 @@ static struct mwifiex_if_ops pcie_ops;
 
 static struct semaphore add_remove_card_sem;
 
+static void mwifiex_pcie_work(struct work_struct *work);
+static DECLARE_WORK(pcie_work, mwifiex_pcie_work);
+
 static int
 mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
 		       size_t size, int flags)
@@ -249,6 +252,8 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
 	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)
@@ -2716,7 +2721,6 @@ static void mwifiex_pcie_work(struct work_struct *work)
 		mwifiex_pcie_device_dump_work(save_adapter);
 }
 
-static DECLARE_WORK(pcie_work, mwifiex_pcie_work);
 /* This function dumps FW information */
 static void mwifiex_pcie_device_dump(struct mwifiex_adapter *adapter)
 {
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 241d2b3..5d84c563 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -46,6 +46,9 @@
  */
 static u8 user_rmmod;
 
+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;
 
@@ -275,7 +278,7 @@ static int mwifiex_sdio_resume(struct device *dev)
  * This function removes the interface and frees up the card structure.
  */
 static void
-mwifiex_sdio_remove(struct sdio_func *func)
+__mwifiex_sdio_remove(struct sdio_func *func)
 {
 	struct sdio_mmc_card *card;
 	struct mwifiex_adapter *adapter;
@@ -305,6 +308,13 @@ mwifiex_sdio_remove(struct sdio_func *func)
 	mwifiex_remove_card(card->adapter, &add_remove_card_sem);
 }
 
+static void
+mwifiex_sdio_remove(struct sdio_func *func)
+{
+	cancel_work_sync(&sdio_work);
+	__mwifiex_sdio_remove(func);
+}
+
 /*
  * SDIO suspend.
  *
@@ -2290,7 +2300,7 @@ static void mwifiex_recreate_adapter(struct sdio_mmc_card *card)
 	 * discovered and initializes them from scratch.
 	 */
 
-	mwifiex_sdio_remove(func);
+	__mwifiex_sdio_remove(func);
 
 	/* power cycle the adapter */
 	sdio_claim_host(func);
@@ -2623,7 +2633,6 @@ static void mwifiex_sdio_work(struct work_struct *work)
 		mwifiex_sdio_card_reset_work(save_adapter);
 }
 
-static DECLARE_WORK(sdio_work, mwifiex_sdio_work);
 /* This function resets the card */
 static void mwifiex_sdio_card_reset(struct mwifiex_adapter *adapter)
 {
-- 
1.9.1

Re: [PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Dmitry Torokhov]

Hi Amit,

On Thu, Oct 27, 2016 at 02:42:40PM +0530, Amitkumar Karwar wrote:
> This variable is guarded by spinlock at all other places. This patch
> takes care of missing spinlock usage in mwifiex_shutdown_drv().

Since in the previous discussion you stated that we inhibit interrupts
and flush the workqueue so that mwifiex_shutdown_drv() can't run
simultaneously with the main processing routine, why do we need this?

Instead please remove call to mwifiex_shutdown_drv() in the main routine
and "if (adapter->mwifiex_processing)" check here.

Thanks.

> 
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v2: Same as v1
> ---
>  drivers/net/wireless/marvell/mwifiex/init.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
> index 82839d9..8e5e424 100644
> --- a/drivers/net/wireless/marvell/mwifiex/init.c
> +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> @@ -670,11 +670,14 @@ mwifiex_shutdown_drv(struct mwifiex_adapter *adapter)
>  
>  	adapter->hw_status = MWIFIEX_HW_STATUS_CLOSING;
>  	/* wait for mwifiex_process to complete */
> +	spin_lock_irqsave(&adapter->main_proc_lock, flags);
>  	if (adapter->mwifiex_processing) {
> +		spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>  		mwifiex_dbg(adapter, WARN,
>  			    "main process is still running\n");
>  		return ret;
>  	}
> +	spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>  
>  	/* cancel current command */
>  	if (adapter->curr_cmd) {
> -- 
> 1.9.1
> 

-- 
Dmitry

Re: [PATCH v2 1/5] mwifiex: remove redundant condition in main process

[Brian Norris]

On Thu, Oct 27, 2016 at 02:42:39PM +0530, Amitkumar Karwar wrote:
> This condition while calling mwifiex_check_ps_cond() is redundant.
> The function internally already takes care of it.
> 
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v2: Same as v1

You've omitted the Reviewed-by tags from v1:

Reviewed-by: Brian Norris <briannorris@chromium.org>

Re: [PATCH v2 5/5] mwifiex: wait firmware dump complete during card remove process

[Brian Norris]

Hi,

On Thu, Oct 27, 2016 at 02:42:43PM +0530, Amitkumar Karwar wrote:
> From: Xinming Hu <huxm@marvell.com>
> 
> Wait for firmware dump complete in card remove function.
> For sdio interface, there are two diffenrent cases,
> card reset trigger sdio_work and firmware dump trigger sdio_work.
> Do code rearrangement for distinguish between these two cases.
> 
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v2: 1. Get rid of reset_triggered flag. Instead split the code and use
>     __mwifiex_sdio_remove() (Brian Norris/Dmitry Torokhov)
>     2. "v1 4/5 mwifiex: firmware dump code rearrangement.." is dropped. So
>     rebased accordingly.
> ---
>  drivers/net/wireless/marvell/mwifiex/pcie.c |  6 +++++-
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 15 ++++++++++++---
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 9025af7..6c421ad 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -37,6 +37,9 @@ static struct mwifiex_if_ops pcie_ops;
>  
>  static struct semaphore add_remove_card_sem;
>  
> +static void mwifiex_pcie_work(struct work_struct *work);
> +static DECLARE_WORK(pcie_work, mwifiex_pcie_work);

This work_struct didn't need to be static in the first place; it should
be embedded in the 'card' and initialized during device init. Then once
you cancel the work in remove() (like this patch is doing) you can also
kill the cancel_work_sync() from the module_exit() path -- you really
shouldn't be doing work like this in the module_init()/module_exit(). It
all belongs in device init/teardown.

> +
>  static int
>  mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
>  		       size_t size, int flags)
> @@ -249,6 +252,8 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
>  	if (!adapter || !adapter->priv_num)
>  		return;
>  
> +	cancel_work_sync(&pcie_work);

Hmm, actually what happens if we have !adapter? Then that means we've
already torn down the device (e.g., unregister_dev() -- except we
haven't quite fixed that bug, see the other patch series you sent out),
and so we'll never reach this. But that also means we haven't
synchronized any outstanding work.

So this really belongs in one of the earlier mwifiex callbacks
(unregister_dev()?), and not in the device remove() callback.

Same applies to sdio.c I think.

Brian

> +
>  	if (user_rmmod && !adapter->mfg_mode) {
>  #ifdef CONFIG_PM_SLEEP
>  		if (adapter->is_suspended)
> @@ -2716,7 +2721,6 @@ static void mwifiex_pcie_work(struct work_struct *work)
>  		mwifiex_pcie_device_dump_work(save_adapter);
>  }
>  
> -static DECLARE_WORK(pcie_work, mwifiex_pcie_work);
>  /* This function dumps FW information */
>  static void mwifiex_pcie_device_dump(struct mwifiex_adapter *adapter)
>  {
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 241d2b3..5d84c563 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -46,6 +46,9 @@
>   */
>  static u8 user_rmmod;
>  
> +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;
>  
> @@ -275,7 +278,7 @@ static int mwifiex_sdio_resume(struct device *dev)
>   * This function removes the interface and frees up the card structure.
>   */
>  static void
> -mwifiex_sdio_remove(struct sdio_func *func)
> +__mwifiex_sdio_remove(struct sdio_func *func)
>  {
>  	struct sdio_mmc_card *card;
>  	struct mwifiex_adapter *adapter;
> @@ -305,6 +308,13 @@ mwifiex_sdio_remove(struct sdio_func *func)
>  	mwifiex_remove_card(card->adapter, &add_remove_card_sem);
>  }
>  
> +static void
> +mwifiex_sdio_remove(struct sdio_func *func)
> +{
> +	cancel_work_sync(&sdio_work);
> +	__mwifiex_sdio_remove(func);
> +}
> +
>  /*
>   * SDIO suspend.
>   *
> @@ -2290,7 +2300,7 @@ static void mwifiex_recreate_adapter(struct sdio_mmc_card *card)
>  	 * discovered and initializes them from scratch.
>  	 */
>  
> -	mwifiex_sdio_remove(func);
> +	__mwifiex_sdio_remove(func);
>  
>  	/* power cycle the adapter */
>  	sdio_claim_host(func);
> @@ -2623,7 +2633,6 @@ static void mwifiex_sdio_work(struct work_struct *work)
>  		mwifiex_sdio_card_reset_work(save_adapter);
>  }
>  
> -static DECLARE_WORK(sdio_work, mwifiex_sdio_work);
>  /* This function resets the card */
>  static void mwifiex_sdio_card_reset(struct mwifiex_adapter *adapter)
>  {
> -- 
> 1.9.1
> 

RE: [PATCH v2 1/5] mwifiex: remove redundant condition in main process

[Xinming Hu]

SGksDQoNCldlIGhhdmUgaW5jbHVkZSBiZWxvdyBjaGFuZ2UgaW4gbGF0ZXN0IHN1Ym1pdCBodHRw
czovL3BhdGNod29yay5rZXJuZWwub3JnL3BhdGNoLzk0MDcyODMvDQpQbGVhc2UgZHJvcCB0aGlz
IHBhdGNoLg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IGxpbnV4LXdp
cmVsZXNzLW93bmVyQHZnZXIua2VybmVsLm9yZw0KPiBbbWFpbHRvOmxpbnV4LXdpcmVsZXNzLW93
bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIEJyaWFuIE5vcnJpcw0KPiBTZW50OiAy
MDE2xOoxMNTCMjjI1SAyOjM2DQo+IFRvOiBBbWl0a3VtYXIgS2Fyd2FyDQo+IENjOiBsaW51eC13
aXJlbGVzc0B2Z2VyLmtlcm5lbC5vcmc7IENhdGh5IEx1bzsgTmlzaGFudCBTYXJtdWthZGFtOw0K
PiByYWphdGphQGdvb2dsZS5jb207IGJyaWFubm9ycmlzQGdvb2dsZS5jb207IGRtaXRyeS50b3Jv
a2hvdkBnbWFpbC5jb20NCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2MiAxLzVdIG13aWZpZXg6IHJl
bW92ZSByZWR1bmRhbnQgY29uZGl0aW9uIGluIG1haW4NCj4gcHJvY2Vzcw0KPiANCj4gT24gVGh1
LCBPY3QgMjcsIDIwMTYgYXQgMDI6NDI6MzlQTSArMDUzMCwgQW1pdGt1bWFyIEthcndhciB3cm90
ZToNCj4gPiBUaGlzIGNvbmRpdGlvbiB3aGlsZSBjYWxsaW5nIG13aWZpZXhfY2hlY2tfcHNfY29u
ZCgpIGlzIHJlZHVuZGFudC4NCj4gPiBUaGUgZnVuY3Rpb24gaW50ZXJuYWxseSBhbHJlYWR5IHRh
a2VzIGNhcmUgb2YgaXQuDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBBbWl0a3VtYXIgS2Fyd2Fy
IDxha2Fyd2FyQG1hcnZlbGwuY29tPg0KPiA+IC0tLQ0KPiA+IHYyOiBTYW1lIGFzIHYxDQo+IA0K
PiBZb3UndmUgb21pdHRlZCB0aGUgUmV2aWV3ZWQtYnkgdGFncyBmcm9tIHYxOg0KPiANCj4gUmV2
aWV3ZWQtYnk6IEJyaWFuIE5vcnJpcyA8YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnPg0K

RE: [PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Xinming Hu]

SGkgRG1pdHJ5LA0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IGxpbnV4
LXdpcmVsZXNzLW93bmVyQHZnZXIua2VybmVsLm9yZw0KPiBbbWFpbHRvOmxpbnV4LXdpcmVsZXNz
LW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIERtaXRyeSBUb3Jva2hvdg0KPiBT
ZW50OiAyMDE2xOoxMNTCMjjI1SAxOjQ0DQo+IFRvOiBBbWl0a3VtYXIgS2Fyd2FyDQo+IENjOiBs
aW51eC13aXJlbGVzc0B2Z2VyLmtlcm5lbC5vcmc7IENhdGh5IEx1bzsgTmlzaGFudCBTYXJtdWth
ZGFtOw0KPiByYWphdGphQGdvb2dsZS5jb207IGJyaWFubm9ycmlzQGdvb2dsZS5jb20NCj4gU3Vi
amVjdDogUmU6IFtQQVRDSCB2MiAyLzVdIG13aWZpZXg6IHVzZSBzcGlubG9jayBmb3IgJ213aWZp
ZXhfcHJvY2Vzc2luZycgaW4NCj4gc2h1dGRvd25fZHJ2DQo+IA0KPiBIaSBBbWl0LA0KPiANCj4g
T24gVGh1LCBPY3QgMjcsIDIwMTYgYXQgMDI6NDI6NDBQTSArMDUzMCwgQW1pdGt1bWFyIEthcndh
ciB3cm90ZToNCj4gPiBUaGlzIHZhcmlhYmxlIGlzIGd1YXJkZWQgYnkgc3BpbmxvY2sgYXQgYWxs
IG90aGVyIHBsYWNlcy4gVGhpcyBwYXRjaA0KPiA+IHRha2VzIGNhcmUgb2YgbWlzc2luZyBzcGlu
bG9jayB1c2FnZSBpbiBtd2lmaWV4X3NodXRkb3duX2RydigpLg0KPiANCj4gU2luY2UgaW4gdGhl
IHByZXZpb3VzIGRpc2N1c3Npb24geW91IHN0YXRlZCB0aGF0IHdlIGluaGliaXQgaW50ZXJydXB0
cyBhbmQgZmx1c2gNCj4gdGhlIHdvcmtxdWV1ZSBzbyB0aGF0IG13aWZpZXhfc2h1dGRvd25fZHJ2
KCkgY2FuJ3QgcnVuIHNpbXVsdGFuZW91c2x5IHdpdGgNCj4gdGhlIG1haW4gcHJvY2Vzc2luZyBy
b3V0aW5lLCB3aHkgZG8gd2UgbmVlZCB0aGlzPw0KPiANCj4gSW5zdGVhZCBwbGVhc2UgcmVtb3Zl
IGNhbGwgdG8gbXdpZmlleF9zaHV0ZG93bl9kcnYoKSBpbiB0aGUgbWFpbiByb3V0aW5lDQo+IGFu
ZCAiaWYgKGFkYXB0ZXItPm13aWZpZXhfcHJvY2Vzc2luZykiIGNoZWNrIGhlcmUuDQo+IA0KDQpt
d2lmaWV4X21haW5fcHJvY2VzcyB3aWxsIGJlIHVzZWQgZnJvbSBpbnRlcnJ1cHQgb3Igd29ya3F1
ZXVlLg0KTm93IHdlIGhhdmUgZGlzYWJsZWQgaW50ZXJydXB0IGFuZCBmbHVzaCB3b3JrcXVldWUs
IHNvIG13aWZpZXhfbWFpbl9wcm9jZXNzIHdvbid0IGJlIHNjaGVkdWxlZCBpbiB0aGUgZnV0dXJl
Lg0KQnV0IG13aWZpZXhfbWFpbl9wcm9jZXNzIG1pZ2h0IGp1c3QgcnVubmluZyBpbiBjb250ZXh0
IG9mIGxhc3QgaW50ZXJydXB0LCBzbyB3ZSBuZWVkIHdhaXQgY3VycmVudCBtYWluX3Byb2Nlc3Mg
Y29tcGxldGUgaW4gbXdpZmlleF9zaHV0ZG93bl9kcnYuDQoNCg0KPiBUaGFua3MuDQo+IA0KPiA+
DQo+ID4gU2lnbmVkLW9mZi1ieTogQW1pdGt1bWFyIEthcndhciA8YWthcndhckBtYXJ2ZWxsLmNv
bT4NCj4gPiAtLS0NCj4gPiB2MjogU2FtZSBhcyB2MQ0KPiA+IC0tLQ0KPiA+ICBkcml2ZXJzL25l
dC93aXJlbGVzcy9tYXJ2ZWxsL213aWZpZXgvaW5pdC5jIHwgMyArKysNCj4gPiAgMSBmaWxlIGNo
YW5nZWQsIDMgaW5zZXJ0aW9ucygrKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0
L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9pbml0LmMNCj4gPiBiL2RyaXZlcnMvbmV0L3dpcmVs
ZXNzL21hcnZlbGwvbXdpZmlleC9pbml0LmMNCj4gPiBpbmRleCA4MjgzOWQ5Li44ZTVlNDI0IDEw
MDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL21hcnZlbGwvbXdpZmlleC9pbml0
LmMNCj4gPiArKysgYi9kcml2ZXJzL25ldC93aXJlbGVzcy9tYXJ2ZWxsL213aWZpZXgvaW5pdC5j
DQo+ID4gQEAgLTY3MCwxMSArNjcwLDE0IEBAIG13aWZpZXhfc2h1dGRvd25fZHJ2KHN0cnVjdCBt
d2lmaWV4X2FkYXB0ZXINCj4gPiAqYWRhcHRlcikNCj4gPg0KPiA+ICAJYWRhcHRlci0+aHdfc3Rh
dHVzID0gTVdJRklFWF9IV19TVEFUVVNfQ0xPU0lORzsNCj4gPiAgCS8qIHdhaXQgZm9yIG13aWZp
ZXhfcHJvY2VzcyB0byBjb21wbGV0ZSAqLw0KPiA+ICsJc3Bpbl9sb2NrX2lycXNhdmUoJmFkYXB0
ZXItPm1haW5fcHJvY19sb2NrLCBmbGFncyk7DQo+ID4gIAlpZiAoYWRhcHRlci0+bXdpZmlleF9w
cm9jZXNzaW5nKSB7DQo+ID4gKwkJc3Bpbl91bmxvY2tfaXJxcmVzdG9yZSgmYWRhcHRlci0+bWFp
bl9wcm9jX2xvY2ssIGZsYWdzKTsNCj4gPiAgCQltd2lmaWV4X2RiZyhhZGFwdGVyLCBXQVJOLA0K
PiA+ICAJCQkgICAgIm1haW4gcHJvY2VzcyBpcyBzdGlsbCBydW5uaW5nXG4iKTsNCj4gPiAgCQly
ZXR1cm4gcmV0Ow0KPiA+ICAJfQ0KPiA+ICsJc3Bpbl91bmxvY2tfaXJxcmVzdG9yZSgmYWRhcHRl
ci0+bWFpbl9wcm9jX2xvY2ssIGZsYWdzKTsNCj4gPg0KPiA+ICAJLyogY2FuY2VsIGN1cnJlbnQg
Y29tbWFuZCAqLw0KPiA+ICAJaWYgKGFkYXB0ZXItPmN1cnJfY21kKSB7DQo+ID4gLS0NCj4gPiAx
LjkuMQ0KPiA+DQo+IA0KPiAtLQ0KPiBEbWl0cnkNCg==

Re: [PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Dmitry Torokhov]

On Thu, Nov 03, 2016 at 08:34:06AM +0000, Xinming Hu wrote:
> Hi Dmitry,
> 
> > -----Original Message-----
> > From: linux-wireless-owner@vger.kernel.org
> > [mailto:linux-wireless-owner@vger.kernel.org] On Behalf Of Dmitry Torokhov
> > Sent: 2016年10月28日 1:44
> > To: Amitkumar Karwar
> > Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> > rajatja@google.com; briannorris@google.com
> > Subject: Re: [PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in
> > shutdown_drv
> > 
> > Hi Amit,
> > 
> > On Thu, Oct 27, 2016 at 02:42:40PM +0530, Amitkumar Karwar wrote:
> > > This variable is guarded by spinlock at all other places. This patch
> > > takes care of missing spinlock usage in mwifiex_shutdown_drv().
> > 
> > Since in the previous discussion you stated that we inhibit interrupts and flush
> > the workqueue so that mwifiex_shutdown_drv() can't run simultaneously with
> > the main processing routine, why do we need this?
> > 
> > Instead please remove call to mwifiex_shutdown_drv() in the main routine
> > and "if (adapter->mwifiex_processing)" check here.
> > 
> 
> mwifiex_main_process will be used from interrupt or workqueue.
> Now we have disabled interrupt and flush workqueue, so mwifiex_main_process won't be scheduled in the future.
> But mwifiex_main_process might just running in context of last interrupt, so we need wait current main_process complete in mwifiex_shutdown_drv.

synchronize_irq() is your friend then.

Thanks.

-- 
Dmitry

Re: [PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Brian Norris]

On Thu, Nov 03, 2016 at 09:15:04AM -0700, Dmitry Torokhov wrote:
> On Thu, Nov 03, 2016 at 08:34:06AM +0000, Xinming Hu wrote:
> > > -----Original Message-----
> > > From: linux-wireless-owner@vger.kernel.org
> > > [mailto:linux-wireless-owner@vger.kernel.org] On Behalf Of Dmitry Torokhov
> > > 
> > > Instead please remove call to mwifiex_shutdown_drv() in the main routine
> > > and "if (adapter->mwifiex_processing)" check here.
> > > 
> > 
> > mwifiex_main_process will be used from interrupt or workqueue.
> > Now we have disabled interrupt and flush workqueue, so
> > mwifiex_main_process won't be scheduled in the future.
> > But mwifiex_main_process might just running in context of last
> > interrupt, so we need wait current main_process complete in
> > mwifiex_shutdown_drv.
> 
> synchronize_irq() is your friend then.

Hmm, that sounds right, but IIUC, the "interrupt context" is actually
only used for SDIO, and for SDIO, the driver doesn't actually have
access to the IRQ number. The MMC/SDIO layer has some extra abstraction
around the IRQ. So this may be more difficult than it appears.

Do we need a sdio_synchronize_irq() API?

Brian

Re: [PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Dmitry Torokhov]

On Thu, Nov 3, 2016 at 12:27 PM, Brian Norris <briannorris@chromium.org> wrote:
> On Thu, Nov 03, 2016 at 09:15:04AM -0700, Dmitry Torokhov wrote:
>> On Thu, Nov 03, 2016 at 08:34:06AM +0000, Xinming Hu wrote:
>> > > -----Original Message-----
>> > > From: linux-wireless-owner@vger.kernel.org
>> > > [mailto:linux-wireless-owner@vger.kernel.org] On Behalf Of Dmitry Torokhov
>> > >
>> > > Instead please remove call to mwifiex_shutdown_drv() in the main routine
>> > > and "if (adapter->mwifiex_processing)" check here.
>> > >
>> >
>> > mwifiex_main_process will be used from interrupt or workqueue.
>> > Now we have disabled interrupt and flush workqueue, so
>> > mwifiex_main_process won't be scheduled in the future.
>> > But mwifiex_main_process might just running in context of last
>> > interrupt, so we need wait current main_process complete in
>> > mwifiex_shutdown_drv.
>>
>> synchronize_irq() is your friend then.
>
> Hmm, that sounds right, but IIUC, the "interrupt context" is actually
> only used for SDIO, and for SDIO, the driver doesn't actually have
> access to the IRQ number. The MMC/SDIO layer has some extra abstraction
> around the IRQ. So this may be more difficult than it appears.
>
> Do we need a sdio_synchronize_irq() API?

Actually the ->disable_irq() method should be responsible for making
sure it does not complete while interrupt handler is running. As far
as I can see, for SDIO case, we end up calling sdio_card_irq_put()
which stops kernel thread and won't return while the thread is
running. For other interfaces we need to check. IIRC USB lacks
->disable_irq() altogether and this is something that shoudl be fixed
(by doing usb_kill_urb() at the minimum).

Thanks.

-- 
Dmitry

RE: [PATCH v2 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Xinming Hu]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogRG1pdHJ5IFRvcm9raG92
IFttYWlsdG86ZG1pdHJ5LnRvcm9raG92QGdtYWlsLmNvbV0NCj4gU2VudDogMjAxNuW5tDEx5pyI
NOaXpSAyOjQ5DQo+IFRvOiBCcmlhbiBOb3JyaXMNCj4gQ2M6IFhpbm1pbmcgSHU7IEFtaXRrdW1h
ciBLYXJ3YXI7IGxpbnV4LXdpcmVsZXNzQHZnZXIua2VybmVsLm9yZzsgQ2F0aHkgTHVvOw0KPiBO
aXNoYW50IFNhcm11a2FkYW07IHJhamF0amFAZ29vZ2xlLmNvbQ0KPiBTdWJqZWN0OiBSZTogW1BB
VENIIHYyIDIvNV0gbXdpZmlleDogdXNlIHNwaW5sb2NrIGZvciAnbXdpZmlleF9wcm9jZXNzaW5n
JyBpbg0KPiBzaHV0ZG93bl9kcnYNCj4gDQo+IE9uIFRodSwgTm92IDMsIDIwMTYgYXQgMTI6Mjcg
UE0sIEJyaWFuIE5vcnJpcyA8YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnPg0KPiB3cm90ZToNCj4g
PiBPbiBUaHUsIE5vdiAwMywgMjAxNiBhdCAwOToxNTowNEFNIC0wNzAwLCBEbWl0cnkgVG9yb2to
b3Ygd3JvdGU6DQo+ID4+IE9uIFRodSwgTm92IDAzLCAyMDE2IGF0IDA4OjM0OjA2QU0gKzAwMDAs
IFhpbm1pbmcgSHUgd3JvdGU6DQo+ID4+ID4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0K
PiA+PiA+ID4gRnJvbTogbGludXgtd2lyZWxlc3Mtb3duZXJAdmdlci5rZXJuZWwub3JnDQo+ID4+
ID4gPiBbbWFpbHRvOmxpbnV4LXdpcmVsZXNzLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVo
YWxmIE9mIERtaXRyeQ0KPiA+PiA+ID4gVG9yb2tob3YNCj4gPj4gPiA+DQo+ID4+ID4gPiBJbnN0
ZWFkIHBsZWFzZSByZW1vdmUgY2FsbCB0byBtd2lmaWV4X3NodXRkb3duX2RydigpIGluIHRoZSBt
YWluDQo+ID4+ID4gPiByb3V0aW5lIGFuZCAiaWYgKGFkYXB0ZXItPm13aWZpZXhfcHJvY2Vzc2lu
ZykiIGNoZWNrIGhlcmUuDQo+ID4+ID4gPg0KPiA+PiA+DQo+ID4+ID4gbXdpZmlleF9tYWluX3By
b2Nlc3Mgd2lsbCBiZSB1c2VkIGZyb20gaW50ZXJydXB0IG9yIHdvcmtxdWV1ZS4NCj4gPj4gPiBO
b3cgd2UgaGF2ZSBkaXNhYmxlZCBpbnRlcnJ1cHQgYW5kIGZsdXNoIHdvcmtxdWV1ZSwgc28NCj4g
Pj4gPiBtd2lmaWV4X21haW5fcHJvY2VzcyB3b24ndCBiZSBzY2hlZHVsZWQgaW4gdGhlIGZ1dHVy
ZS4NCj4gPj4gPiBCdXQgbXdpZmlleF9tYWluX3Byb2Nlc3MgbWlnaHQganVzdCBydW5uaW5nIGlu
IGNvbnRleHQgb2YgbGFzdA0KPiA+PiA+IGludGVycnVwdCwgc28gd2UgbmVlZCB3YWl0IGN1cnJl
bnQgbWFpbl9wcm9jZXNzIGNvbXBsZXRlIGluDQo+ID4+ID4gbXdpZmlleF9zaHV0ZG93bl9kcnYu
DQo+ID4+DQo+ID4+IHN5bmNocm9uaXplX2lycSgpIGlzIHlvdXIgZnJpZW5kIHRoZW4uDQo+ID4N
Cj4gPiBIbW0sIHRoYXQgc291bmRzIHJpZ2h0LCBidXQgSUlVQywgdGhlICJpbnRlcnJ1cHQgY29u
dGV4dCIgaXMgYWN0dWFsbHkNCj4gPiBvbmx5IHVzZWQgZm9yIFNESU8sIGFuZCBmb3IgU0RJTywg
dGhlIGRyaXZlciBkb2Vzbid0IGFjdHVhbGx5IGhhdmUNCj4gPiBhY2Nlc3MgdG8gdGhlIElSUSBu
dW1iZXIuIFRoZSBNTUMvU0RJTyBsYXllciBoYXMgc29tZSBleHRyYQ0KPiA+IGFic3RyYWN0aW9u
IGFyb3VuZCB0aGUgSVJRLiBTbyB0aGlzIG1heSBiZSBtb3JlIGRpZmZpY3VsdCB0aGFuIGl0IGFw
cGVhcnMuDQo+ID4NCj4gPiBEbyB3ZSBuZWVkIGEgc2Rpb19zeW5jaHJvbml6ZV9pcnEoKSBBUEk/
DQo+IA0KPiBBY3R1YWxseSB0aGUgLT5kaXNhYmxlX2lycSgpIG1ldGhvZCBzaG91bGQgYmUgcmVz
cG9uc2libGUgZm9yIG1ha2luZyBzdXJlIGl0DQo+IGRvZXMgbm90IGNvbXBsZXRlIHdoaWxlIGlu
dGVycnVwdCBoYW5kbGVyIGlzIHJ1bm5pbmcuIEFzIGZhciBhcyBJIGNhbiBzZWUsIGZvcg0KPiBT
RElPIGNhc2UsIHdlIGVuZCB1cCBjYWxsaW5nIHNkaW9fY2FyZF9pcnFfcHV0KCkgd2hpY2ggc3Rv
cHMga2VybmVsIHRocmVhZA0KPiBhbmQgd29uJ3QgcmV0dXJuIHdoaWxlIHRoZSB0aHJlYWQgaXMg
cnVubmluZy4gRm9yIG90aGVyIGludGVyZmFjZXMgd2UgbmVlZCB0bw0KPiBjaGVjay4gSUlSQyBV
U0IgbGFja3MNCj4gLT5kaXNhYmxlX2lycSgpIGFsdG9nZXRoZXIgYW5kIHRoaXMgaXMgc29tZXRo
aW5nIHRoYXQgc2hvdWRsIGJlIGZpeGVkDQo+IChieSBkb2luZyB1c2Jfa2lsbF91cmIoKSBhdCB0
aGUgbWluaW11bSkuDQo+IA0KDQpUaGFua3MgZm9yIHRoZSBleHBsYWluLCB3aWxsIGtlZXAgdGhp
cyBjbGVhbnVwIHdvcmsuDQoNCj4gVGhhbmtzLg0KPiANCj4gLS0NCj4gRG1pdHJ5DQoNClJlZ2Fy
ZHMsDQpYaW5taW5nDQo=

Re: [PATCH v2 1/5] mwifiex: remove redundant condition in main process

[Kalle Valo]

Xinming Hu <huxm@marvell.com> writes:

> We have include below change in latest submit https://patchwork.kernel.org/patch/9407283/
> Please drop this patch.

When making changes please resend the whole patchset. I do not want to
individually pick patches from different places, that will eventually go
wrong anyway. So I just apply full patchsets.

-- 
Kalle Valo

Re: [PATCH v2 1/5] mwifiex: remove redundant condition in main process

[Brian Norris]

On Mon, Nov 07, 2016 at 08:46:41PM +0200, Kalle Valo wrote:
> Xinming Hu <huxm@marvell.com> writes:
> 
> > We have include below change in latest submit https://patchwork.kernel.org/patch/9407283/
> > Please drop this patch.
> 
> When making changes please resend the whole patchset. I do not want to
> individually pick patches from different places, that will eventually go
> wrong anyway. So I just apply full patchsets.

I'm going to assume that Xinming intended that the entire series be
resent, possibly without this patch. There were enough other issues in
the series anyway.

Brian

RE: [PATCH v2 1/5] mwifiex: remove redundant condition in main process

[Amitkumar Karwar]

Hi Brian,

> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Friday, November 11, 2016 1:17 AM
> To: Kalle Valo
> Cc: Xinming Hu; Amitkumar Karwar; linux-wireless@vger.kernel.org; Cathy
> Luo; Nishant Sarmukadam; rajatja@google.com; dmitry.torokhov@gmail.com
> Subject: Re: [PATCH v2 1/5] mwifiex: remove redundant condition in main
> process
> 
> On Mon, Nov 07, 2016 at 08:46:41PM +0200, Kalle Valo wrote:
> > Xinming Hu <huxm@marvell.com> writes:
> >
> > > We have include below change in latest submit
> > > https://patchwork.kernel.org/patch/9407283/
> > > Please drop this patch.
> >
> > When making changes please resend the whole patchset. I do not want
> to
> > individually pick patches from different places, that will eventually
> > go wrong anyway. So I just apply full patchsets.
> 
> I'm going to assume that Xinming intended that the entire series be
> resent, possibly without this patch. There were enough other issues in
> the series anyway.

I will submit v3 series shortly.

Regards,
Amitkumar

RE: [PATCH v2 5/5] mwifiex: wait firmware dump complete during card remove process

[Amitkumar Karwar]

Hi Brian,

> > +
> >  static int
> >  mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct
> sk_buff *skb,
> >  		       size_t size, int flags)
> > @@ -249,6 +252,8 @@ static void mwifiex_pcie_remove(struct pci_dev
> *pdev)
> >  	if (!adapter || !adapter->priv_num)
> >  		return;
> >
> > +	cancel_work_sync(&pcie_work);
> 
> Hmm, actually what happens if we have !adapter? Then that means we've
> already torn down the device (e.g., unregister_dev() -- except we
> haven't quite fixed that bug, see the other patch series you sent out),
> and so we'll never reach this. But that also means we haven't
> synchronized any outstanding work.

There won't be any outstanding work in that case where init failure thread has already cleared "adapter"

Pcie/sdio Work(firmware dump + card reset) is scheduled in below two scenarios
1) Command timeout -- We have a check to skip triggering work when hw_status shows it's INITIALIZING. 
2) Tx data timeout -- Tx data is applicable only when wifi connection is alive. In above mentioned case, device itself has failed to initialize.

> 
> So this really belongs in one of the earlier mwifiex callbacks
> (unregister_dev()?), and not in the device remove() callback.
> 

Regards,
Amitkumar