* [PATCH v4 02/11] mwifiex: complete blocked power save handshake in main process
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 03/11] mwifiex: resolve races between async FW init (failure) and device removal Amitkumar Karwar
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Shengzhen Li, Amitkumar Karwar
From: Shengzhen Li <szli@marvell.com>
Power save handshake with firmware might be blocked by on-going
data transfer.
this patch check the PS status in main process and complete
previous blocked PS handshake.
this patch also remove redudant check before call
mwifiex_check_ps_cond function.
Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Shengzhen Li <szli@marvell.com>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v2: 1. remove redudant check(Brain)
2. reorgnized to follow tx_hw_pending patch
v4: Same as v2, v3
---
drivers/net/wireless/marvell/mwifiex/main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 948f5c2..9e7acb4 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -308,6 +308,9 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
/* We have tried to wakeup the card already */
if (adapter->pm_wakeup_fw_try)
break;
+ if (adapter->ps_state == PS_STATE_PRE_SLEEP)
+ mwifiex_check_ps_cond(adapter);
+
if (adapter->ps_state != PS_STATE_AWAKE)
break;
if (adapter->tx_lock_flag) {
@@ -355,10 +358,8 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
/* 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 03/11] mwifiex: resolve races between async FW init (failure) and device removal
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 02/11] mwifiex: complete blocked power save handshake in main process Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 04/11] mwifiex: remove redundant pdev check in suspend/resume handlers Amitkumar Karwar
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris
From: Brian Norris <briannorris@chromium.org>
It's possible for the FW init sequence to fail, which will trigger a
device cleanup sequence in mwifiex_fw_dpc(). This sequence can race with
device suspend() or remove() (e.g., reboot or unbind), and can trigger
use-after-free issues. Currently, this driver attempts (poorly) to
synchronize remove() using a semaphore, but it doesn't protect some of
the critical sections properly. Particularly, we grab a pointer to the
adapter struct (card->adapter) without checking if it's being freed or
not. We later do a NULL check on the adapter, but that doesn't work if
the adapter was freed.
Also note that the PCIe interface driver doesn't ever set card->adapter
to NULL, so even if we get the synchronization right, we still might try
to redo the cleanup in ->remove(), even if the FW init failure sequence
already did it.
This patch replaces the static semaphore with a per-device completion
struct, and uses that completion to synchronize the remove() thread with
the mwifiex_fw_dpc(). A future patch will utilize this completion to
synchronize the suspend() thread as well.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
v3: Included Brian's suggested change which fixes new use-after-free
introduced by this patch.
v4: Resolved minor conflict while rebasing v3 on top of
"[v4,1/3] mwifiex: Allow mwifiex early access to device structure"
---
drivers/net/wireless/marvell/mwifiex/main.c | 47 +++++++++++------------------
drivers/net/wireless/marvell/mwifiex/main.h | 11 +++++--
drivers/net/wireless/marvell/mwifiex/pcie.c | 18 +++++------
drivers/net/wireless/marvell/mwifiex/pcie.h | 2 ++
drivers/net/wireless/marvell/mwifiex/sdio.c | 18 +++++------
drivers/net/wireless/marvell/mwifiex/sdio.h | 2 ++
drivers/net/wireless/marvell/mwifiex/usb.c | 23 ++++++--------
drivers/net/wireless/marvell/mwifiex/usb.h | 2 ++
8 files changed, 55 insertions(+), 68 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 9e7acb4..eac44fe 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -521,9 +521,9 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
struct mwifiex_private *priv;
struct mwifiex_adapter *adapter = context;
struct mwifiex_fw_image fw;
- struct semaphore *sem = adapter->card_sem;
bool init_failed = false;
struct wireless_dev *wdev;
+ struct completion *fw_done = adapter->fw_done;
if (!firmware) {
mwifiex_dbg(adapter, ERROR,
@@ -670,7 +670,8 @@ static void mwifiex_fw_dpc(const struct firmware *firmware, void *context)
}
if (init_failed)
mwifiex_free_adapter(adapter);
- up(sem);
+ /* Tell all current and future waiters we're finished */
+ complete_all(fw_done);
return;
}
@@ -1365,7 +1366,7 @@ static void mwifiex_main_work_queue(struct work_struct *work)
* code is extracted from mwifiex_remove_card()
*/
static int
-mwifiex_shutdown_sw(struct mwifiex_adapter *adapter, struct semaphore *sem)
+mwifiex_shutdown_sw(struct mwifiex_adapter *adapter)
{
struct mwifiex_private *priv;
int i;
@@ -1373,8 +1374,9 @@ static void mwifiex_main_work_queue(struct work_struct *work)
if (!adapter)
goto exit_return;
- if (down_interruptible(sem))
- goto exit_sem_err;
+ 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);
@@ -1431,8 +1433,6 @@ static void mwifiex_main_work_queue(struct work_struct *work)
rtnl_unlock();
}
- up(sem);
-exit_sem_err:
mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
exit_return:
return 0;
@@ -1442,21 +1442,18 @@ static void mwifiex_main_work_queue(struct work_struct *work)
* code is extracted from mwifiex_add_card()
*/
static int
-mwifiex_reinit_sw(struct mwifiex_adapter *adapter, struct semaphore *sem,
+mwifiex_reinit_sw(struct mwifiex_adapter *adapter, struct completion *fw_done,
struct mwifiex_if_ops *if_ops, u8 iface_type)
{
char fw_name[32];
struct pcie_service_card *card = adapter->card;
- if (down_interruptible(sem))
- goto exit_sem_err;
-
mwifiex_init_lock_list(adapter);
if (adapter->if_ops.up_dev)
adapter->if_ops.up_dev(adapter);
adapter->iface_type = iface_type;
- adapter->card_sem = sem;
+ adapter->fw_done = fw_done;
adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING;
adapter->surprise_removed = false;
@@ -1507,7 +1504,8 @@ static void mwifiex_main_work_queue(struct work_struct *work)
}
strcpy(adapter->fw_name, fw_name);
mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
- up(sem);
+
+ complete_all(adapter->fw_done);
return 0;
err_init_fw:
@@ -1527,8 +1525,7 @@ static void mwifiex_main_work_queue(struct work_struct *work)
err_kmalloc:
mwifiex_terminate_workqueue(adapter);
adapter->surprise_removed = true;
- up(sem);
-exit_sem_err:
+ complete_all(adapter->fw_done);
mwifiex_dbg(adapter, INFO, "%s, error\n", __func__);
return -1;
@@ -1543,12 +1540,12 @@ void mwifiex_do_flr(struct mwifiex_adapter *adapter, bool prepare)
struct mwifiex_if_ops if_ops;
if (!prepare) {
- mwifiex_reinit_sw(adapter, adapter->card_sem, &if_ops,
+ mwifiex_reinit_sw(adapter, adapter->fw_done, &if_ops,
adapter->iface_type);
} else {
memcpy(&if_ops, &adapter->if_ops,
sizeof(struct mwifiex_if_ops));
- mwifiex_shutdown_sw(adapter, adapter->card_sem);
+ mwifiex_shutdown_sw(adapter);
}
}
EXPORT_SYMBOL_GPL(mwifiex_do_flr);
@@ -1618,15 +1615,12 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
* - Add logical interfaces
*/
int
-mwifiex_add_card(void *card, struct semaphore *sem,
+mwifiex_add_card(void *card, struct completion *fw_done,
struct mwifiex_if_ops *if_ops, u8 iface_type,
struct device *dev)
{
struct mwifiex_adapter *adapter;
- if (down_interruptible(sem))
- goto exit_sem_err;
-
if (mwifiex_register(card, if_ops, (void **)&adapter)) {
pr_err("%s: software init failed\n", __func__);
goto err_init_sw;
@@ -1636,7 +1630,7 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
mwifiex_probe_of(adapter);
adapter->iface_type = iface_type;
- adapter->card_sem = sem;
+ adapter->fw_done = fw_done;
adapter->hw_status = MWIFIEX_HW_STATUS_INITIALIZING;
adapter->surprise_removed = false;
@@ -1705,9 +1699,7 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
mwifiex_free_adapter(adapter);
err_init_sw:
- up(sem);
-exit_sem_err:
return -1;
}
EXPORT_SYMBOL_GPL(mwifiex_add_card);
@@ -1723,14 +1715,11 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
* - Unregister the device
* - Free the adapter structure
*/
-int mwifiex_remove_card(struct mwifiex_adapter *adapter, struct semaphore *sem)
+int mwifiex_remove_card(struct mwifiex_adapter *adapter)
{
struct mwifiex_private *priv = NULL;
int i;
- if (down_trylock(sem))
- goto exit_sem_err;
-
if (!adapter)
goto exit_remove;
@@ -1800,8 +1789,6 @@ int mwifiex_remove_card(struct mwifiex_adapter *adapter, struct semaphore *sem)
mwifiex_free_adapter(adapter);
exit_remove:
- up(sem);
-exit_sem_err:
return 0;
}
EXPORT_SYMBOL_GPL(mwifiex_remove_card);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 904a2ed..cf4c780 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -20,6 +20,7 @@
#ifndef _MWIFIEX_MAIN_H_
#define _MWIFIEX_MAIN_H_
+#include <linux/completion.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/sched.h>
@@ -985,7 +986,10 @@ struct mwifiex_adapter {
u32 usr_dot_11ac_mcs_support;
atomic_t pending_bridged_pkts;
- struct semaphore *card_sem;
+
+ /* For synchronizing FW initialization with device lifecycle. */
+ struct completion *fw_done;
+
bool ext_scan;
u8 fw_api_ver;
u8 key_api_major_ver, key_api_minor_ver;
@@ -1438,10 +1442,11 @@ static inline void mwifiex_enable_wake(struct mwifiex_adapter *adapter)
int mwifiex_init_shutdown_fw(struct mwifiex_private *priv,
u32 func_init_shutdown);
-int mwifiex_add_card(void *card, struct semaphore *sem,
+
+int mwifiex_add_card(void *card, struct completion *fw_done,
struct mwifiex_if_ops *if_ops, u8 iface_type,
struct device *dev);
-int mwifiex_remove_card(struct mwifiex_adapter *, struct semaphore *);
+int mwifiex_remove_card(struct mwifiex_adapter *adapter);
void mwifiex_get_version(struct mwifiex_adapter *adapter, char *version,
int maxlen);
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index cfb45ef..547806f 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -35,8 +35,6 @@
static struct mwifiex_if_ops pcie_ops;
-static struct semaphore add_remove_card_sem;
-
static const struct of_device_id mwifiex_pcie_of_match_table[] = {
{ .compatible = "pci11ab,2b42" },
{ .compatible = "pci1b4b,2b42" },
@@ -212,6 +210,8 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
if (!card)
return -ENOMEM;
+ init_completion(&card->fw_done);
+
card->dev = pdev;
if (ent->driver_data) {
@@ -232,7 +232,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
return ret;
}
- if (mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
+ if (mwifiex_add_card(card, &card->fw_done, &pcie_ops,
MWIFIEX_PCIE, &pdev->dev)) {
pr_err("%s failed\n", __func__);
return -1;
@@ -254,6 +254,8 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
if (!card)
return;
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
if (!adapter || !adapter->priv_num)
return;
@@ -273,7 +275,7 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
mwifiex_init_shutdown_fw(priv, MWIFIEX_FUNC_SHUTDOWN);
}
- mwifiex_remove_card(card->adapter, &add_remove_card_sem);
+ mwifiex_remove_card(adapter);
}
static void mwifiex_pcie_shutdown(struct pci_dev *pdev)
@@ -3175,8 +3177,7 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
/*
* This function initializes the PCIE driver module.
*
- * This initiates the semaphore and registers the device with
- * PCIE bus.
+ * This registers the device with PCIE bus.
*/
static int mwifiex_pcie_init_module(void)
{
@@ -3184,8 +3185,6 @@ static int mwifiex_pcie_init_module(void)
pr_debug("Marvell PCIe Driver\n");
- sema_init(&add_remove_card_sem, 1);
-
/* Clear the flag in case user removes the card. */
user_rmmod = 0;
@@ -3209,9 +3208,6 @@ static int mwifiex_pcie_init_module(void)
*/
static void mwifiex_pcie_cleanup_module(void)
{
- if (!down_interruptible(&add_remove_card_sem))
- up(&add_remove_card_sem);
-
/* Set the flag as user is removing this module. */
user_rmmod = 1;
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h b/drivers/net/wireless/marvell/mwifiex/pcie.h
index 46f99ca..ae3365d 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
@@ -22,6 +22,7 @@
#ifndef _MWIFIEX_PCIE_H
#define _MWIFIEX_PCIE_H
+#include <linux/completion.h>
#include <linux/pci.h>
#include <linux/interrupt.h>
@@ -345,6 +346,7 @@ struct pcie_service_card {
struct pci_dev *dev;
struct mwifiex_adapter *adapter;
struct mwifiex_pcie_device pcie;
+ struct completion fw_done;
u8 txbd_flush;
u32 txbd_wrptr;
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index f410cf5..5312ffb 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -49,8 +49,6 @@
static struct mwifiex_if_ops sdio_ops;
static unsigned long iface_work_flags;
-static struct semaphore add_remove_card_sem;
-
static struct memory_type_mapping generic_mem_type_map[] = {
{"DUMP", NULL, 0, 0xDD},
};
@@ -115,6 +113,8 @@ static int mwifiex_sdio_probe_of(struct device *dev)
if (!card)
return -ENOMEM;
+ init_completion(&card->fw_done);
+
card->func = func;
card->device_id = id;
@@ -154,7 +154,7 @@ static int mwifiex_sdio_probe_of(struct device *dev)
goto err_disable;
}
- ret = mwifiex_add_card(card, &add_remove_card_sem, &sdio_ops,
+ ret = mwifiex_add_card(card, &card->fw_done, &sdio_ops,
MWIFIEX_SDIO, &func->dev);
if (ret) {
dev_err(&func->dev, "add card failed\n");
@@ -235,6 +235,8 @@ static int mwifiex_sdio_resume(struct device *dev)
if (!card)
return;
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
if (!adapter || !adapter->priv_num)
return;
@@ -252,7 +254,7 @@ static int mwifiex_sdio_resume(struct device *dev)
mwifiex_init_shutdown_fw(priv, MWIFIEX_FUNC_SHUTDOWN);
}
- mwifiex_remove_card(card->adapter, &add_remove_card_sem);
+ mwifiex_remove_card(adapter);
}
/*
@@ -2714,14 +2716,11 @@ static void mwifiex_sdio_device_dump(struct mwifiex_adapter *adapter)
/*
* This function initializes the SDIO driver.
*
- * This initiates the semaphore and registers the device with
- * SDIO bus.
+ * This registers the device with SDIO bus.
*/
static int
mwifiex_sdio_init_module(void)
{
- sema_init(&add_remove_card_sem, 1);
-
/* Clear the flag in case user removes the card. */
user_rmmod = 0;
@@ -2740,9 +2739,6 @@ static void mwifiex_sdio_device_dump(struct mwifiex_adapter *adapter)
static void
mwifiex_sdio_cleanup_module(void)
{
- if (!down_interruptible(&add_remove_card_sem))
- up(&add_remove_card_sem);
-
/* Set the flag as user is removing this module. */
user_rmmod = 1;
cancel_work_sync(&sdio_work);
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h b/drivers/net/wireless/marvell/mwifiex/sdio.h
index b9fbc5c..cdbf3a3a 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -21,6 +21,7 @@
#define _MWIFIEX_SDIO_H
+#include <linux/completion.h>
#include <linux/mmc/sdio.h>
#include <linux/mmc/sdio_ids.h>
#include <linux/mmc/sdio_func.h>
@@ -238,6 +239,7 @@ struct sdio_mmc_card {
struct sdio_func *func;
struct mwifiex_adapter *adapter;
+ struct completion fw_done;
const char *firmware;
const struct mwifiex_sdio_card_reg *reg;
u8 max_ports;
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 9cc2a0c0..63a755c 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -24,7 +24,6 @@
static u8 user_rmmod;
static struct mwifiex_if_ops usb_ops;
-static struct semaphore add_remove_card_sem;
static struct usb_device_id mwifiex_usb_table[] = {
/* 8766 */
@@ -386,6 +385,8 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
if (!card)
return -ENOMEM;
+ init_completion(&card->fw_done);
+
id_vendor = le16_to_cpu(udev->descriptor.idVendor);
id_product = le16_to_cpu(udev->descriptor.idProduct);
bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
@@ -475,7 +476,7 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
usb_set_intfdata(intf, card);
- ret = mwifiex_add_card(card, &add_remove_card_sem, &usb_ops,
+ ret = mwifiex_add_card(card, &card->fw_done, &usb_ops,
MWIFIEX_USB, &card->udev->dev);
if (ret) {
pr_err("%s: mwifiex_add_card failed: %d\n", __func__, ret);
@@ -601,13 +602,15 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
struct usb_card_rec *card = usb_get_intfdata(intf);
struct mwifiex_adapter *adapter;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card) {
+ dev_err(&intf->dev, "%s: card is NULL\n", __func__);
return;
}
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
- if (!adapter->priv_num)
+ if (!adapter || !adapter->priv_num)
return;
if (user_rmmod && !adapter->mfg_mode) {
@@ -627,7 +630,7 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
mwifiex_dbg(adapter, FATAL,
"%s: removing card\n", __func__);
- mwifiex_remove_card(adapter, &add_remove_card_sem);
+ mwifiex_remove_card(adapter);
usb_put_dev(interface_to_usbdev(intf));
}
@@ -1200,8 +1203,7 @@ static void mwifiex_usb_submit_rem_rx_urbs(struct mwifiex_adapter *adapter)
/* This function initializes the USB driver module.
*
- * This initiates the semaphore and registers the device with
- * USB bus.
+ * This registers the device with USB bus.
*/
static int mwifiex_usb_init_module(void)
{
@@ -1209,8 +1211,6 @@ static int mwifiex_usb_init_module(void)
pr_debug("Marvell USB8797 Driver\n");
- sema_init(&add_remove_card_sem, 1);
-
ret = usb_register(&mwifiex_usb_driver);
if (ret)
pr_err("Driver register failed!\n");
@@ -1230,9 +1230,6 @@ static int mwifiex_usb_init_module(void)
*/
static void mwifiex_usb_cleanup_module(void)
{
- if (!down_interruptible(&add_remove_card_sem))
- up(&add_remove_card_sem);
-
/* set the flag as user is removing this module */
user_rmmod = 1;
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h b/drivers/net/wireless/marvell/mwifiex/usb.h
index 30e8eb8..e5f204e 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.h
+++ b/drivers/net/wireless/marvell/mwifiex/usb.h
@@ -20,6 +20,7 @@
#ifndef _MWIFIEX_USB_H
#define _MWIFIEX_USB_H
+#include <linux/completion.h>
#include <linux/usb.h>
#define USB8XXX_VID 0x1286
@@ -75,6 +76,7 @@ struct usb_card_rec {
struct mwifiex_adapter *adapter;
struct usb_device *udev;
struct usb_interface *intf;
+ struct completion fw_done;
u8 rx_cmd_ep;
struct urb_context rx_cmd;
atomic_t rx_cmd_urb_pending;
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 04/11] mwifiex: remove redundant pdev check in suspend/resume handlers
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 02/11] mwifiex: complete blocked power save handshake in main process Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 03/11] mwifiex: resolve races between async FW init (failure) and device removal Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 05/11] mwifiex: don't pretend to resume while remove()'ing Amitkumar Karwar
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Amitkumar Karwar, Brian Norris
to_pci_dev() would just do struct offset arithmetic on struct
device to get 'pdev' pointer. We never get NULL pdev pointer.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v4: Same v1, v2, v3
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 547806f..86ae084 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -117,14 +117,9 @@ static int mwifiex_pcie_suspend(struct device *dev)
struct pcie_service_card *card;
struct pci_dev *pdev = to_pci_dev(dev);
- if (pdev) {
- card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- pr_err("Card or adapter structure is not valid\n");
- return 0;
- }
- } else {
- pr_err("PCIE device is not specified\n");
+ card = pci_get_drvdata(pdev);
+ if (!card || !card->adapter) {
+ pr_err("Card or adapter structure is not valid\n");
return 0;
}
@@ -162,14 +157,9 @@ static int mwifiex_pcie_resume(struct device *dev)
struct pcie_service_card *card;
struct pci_dev *pdev = to_pci_dev(dev);
- if (pdev) {
- card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- pr_err("Card or adapter structure is not valid\n");
- return 0;
- }
- } else {
- pr_err("PCIE device is not specified\n");
+ card = pci_get_drvdata(pdev);
+ if (!card || !card->adapter) {
+ dev_err(dev, "Card or adapter structure is not valid\n");
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 05/11] mwifiex: don't pretend to resume while remove()'ing
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (2 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 04/11] mwifiex: remove redundant pdev check in suspend/resume handlers Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 06/11] mwifiex: resolve suspend() race with async FW init failure Amitkumar Karwar
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris, Amitkumar Karwar
From: Brian Norris <briannorris@chromium.org>
The device core will not allow suspend() to race with remove().
Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: Same as v1, v2, v3
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 5 -----
drivers/net/wireless/marvell/mwifiex/sdio.c | 3 ---
drivers/net/wireless/marvell/mwifiex/usb.c | 5 -----
3 files changed, 13 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 86ae084..cecf1a0 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -251,11 +251,6 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
return;
if (user_rmmod && !adapter->mfg_mode) {
-#ifdef CONFIG_PM_SLEEP
- if (adapter->is_suspended)
- mwifiex_pcie_resume(&pdev->dev);
-#endif
-
mwifiex_deauthenticate_all(adapter);
priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 5312ffb..2f5b01f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -244,9 +244,6 @@ static int mwifiex_sdio_resume(struct device *dev)
mwifiex_dbg(adapter, INFO, "info: SDIO func num=%d\n", func->num);
if (user_rmmod && !adapter->mfg_mode) {
- if (adapter->is_suspended)
- mwifiex_sdio_resume(adapter->dev);
-
mwifiex_deauthenticate_all(adapter);
priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 63a755c..45c9430a 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -614,11 +614,6 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
return;
if (user_rmmod && !adapter->mfg_mode) {
-#ifdef CONFIG_PM
- if (adapter->is_suspended)
- mwifiex_usb_resume(intf);
-#endif
-
mwifiex_deauthenticate_all(adapter);
mwifiex_init_shutdown_fw(mwifiex_get_priv(adapter,
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 06/11] mwifiex: resolve suspend() race with async FW init failure
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (3 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 05/11] mwifiex: don't pretend to resume while remove()'ing Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 07/11] mwifiex: reset card->adapter during device unregister Amitkumar Karwar
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris, Amitkumar Karwar
From: Brian Norris <briannorris@chromium.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: Same as v1, v2, v3
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 12 ++++++++++--
drivers/net/wireless/marvell/mwifiex/sdio.c | 12 ++++++++++--
drivers/net/wireless/marvell/mwifiex/usb.c | 12 ++++++++++--
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index cecf1a0..6176cb78 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -118,12 +118,20 @@ static int mwifiex_pcie_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- pr_err("Card or adapter structure is not valid\n");
+ if (!card) {
+ dev_err(dev, "card structure is not valid\n");
return 0;
}
+ /* Might still be loading firmware */
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
+ if (!adapter) {
+ dev_err(dev, "adapter is not valid\n");
+ return 0;
+ }
+
mwifiex_enable_wake(adapter);
/* Enable the Host Sleep */
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 2f5b01f..5077d56 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -283,8 +283,8 @@ static int mwifiex_sdio_suspend(struct device *dev)
}
card = sdio_get_drvdata(func);
- if (!card || !card->adapter) {
- pr_err("suspend: invalid card or adapter\n");
+ if (!card) {
+ dev_err(dev, "suspend: invalid card\n");
return 0;
}
} else {
@@ -292,7 +292,15 @@ static int mwifiex_sdio_suspend(struct device *dev)
return 0;
}
+ /* Might still be loading firmware */
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
+ if (!adapter) {
+ dev_err(dev, "adapter is not valid\n");
+ return 0;
+ }
+
mwifiex_enable_wake(adapter);
/* Enable the Host Sleep */
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 45c9430a..64a4020 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -503,11 +503,19 @@ static int mwifiex_usb_suspend(struct usb_interface *intf, pm_message_t message)
struct usb_tx_data_port *port;
int i, j;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card) {
+ dev_err(&intf->dev, "%s: card is NULL\n", __func__);
return 0;
}
+
+ /* Might still be loading firmware */
+ wait_for_completion(&card->fw_done);
+
adapter = card->adapter;
+ if (!adapter) {
+ dev_err(&intf->dev, "card is not valid\n");
+ return 0;
+ }
if (unlikely(adapter->is_suspended))
mwifiex_dbg(adapter, WARN,
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 07/11] mwifiex: reset card->adapter during device unregister
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (4 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 06/11] mwifiex: resolve suspend() race with async FW init failure Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 08/11] mwifiex: usb: handle HS failures Amitkumar Karwar
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Xinming Hu, Amitkumar Karwar, Brian Norris
From: Xinming Hu <huxm@marvell.com>
card->adapter gets initialized in mwifiex_register_dev(). As it's not
cleared in mwifiex_unregister_dev(), we may end up accessing the memory
which is already free in below scenario.
Scenario: Driver initialization is failed due to incorrect firmware or
some other reason. Meanwhile device reboot/unload occurs.
This is safe, now that we've properly synchronized suspend() and
remove() with the FW initialization thread; now that code can simply
check for 'card->adapter == NULL' and exit safely.
Signed-off-by: Xinming Hu <huxm@marvell.com>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v4: Same as v1, v2, v3
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 1 +
drivers/net/wireless/marvell/mwifiex/sdio.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 6176cb78..f6d28d9 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -3042,6 +3042,7 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
if (card->msi_enable)
pci_disable_msi(pdev);
}
+ card->adapter = NULL;
}
}
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 5077d56..69542f8 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -2017,6 +2017,7 @@ static int mwifiex_alloc_sdio_mpa_buffers(struct mwifiex_adapter *adapter,
struct sdio_mmc_card *card = adapter->card;
if (adapter->card) {
+ card->adapter = NULL;
sdio_claim_host(card->func);
sdio_disable_func(card->func);
sdio_release_host(card->func);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 08/11] mwifiex: usb: handle HS failures
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (5 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 07/11] mwifiex: reset card->adapter during device unregister Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 09/11] mwifiex: sdio: don't check for NULL sdio_func Amitkumar Karwar
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris, Amitkumar Karwar
From: Brian Norris <briannorris@chromium.org>
SDIO and PCIe drivers handle this. Let's imitate it.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: Same as v1, v2, v3
---
drivers/net/wireless/marvell/mwifiex/usb.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 64a4020..55e3a01 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -521,7 +521,14 @@ static int mwifiex_usb_suspend(struct usb_interface *intf, pm_message_t message)
mwifiex_dbg(adapter, WARN,
"Device already suspended\n");
- mwifiex_enable_hs(adapter);
+ /* Enable the Host Sleep */
+ if (!mwifiex_enable_hs(adapter)) {
+ mwifiex_dbg(adapter, ERROR,
+ "cmd: failed to suspend\n");
+ adapter->hs_enabling = false;
+ return -EFAULT;
+ }
+
/* 'is_suspended' flag indicates device is suspended.
* It must be set here before the usb_kill_urb() calls. Reason
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 09/11] mwifiex: sdio: don't check for NULL sdio_func
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (6 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 08/11] mwifiex: usb: handle HS failures Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 10/11] mwifiex: stop checking for NULL drvata/intfdata Amitkumar Karwar
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris, Amitkumar Karwar
From: Brian Norris <briannorris@chromium.org>
sdio_func is retrieved via container_of() and should never be NULL.
Checking for NULL just makes the logic more confusing than necessary.
Stop doing that.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: Same as v1, v2, v3
---
drivers/net/wireless/marvell/mwifiex/sdio.c | 40 +++++++++++------------------
1 file changed, 15 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 69542f8..0d00db5 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -188,15 +188,10 @@ static int mwifiex_sdio_resume(struct device *dev)
struct mwifiex_adapter *adapter;
mmc_pm_flag_t pm_flag = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- card = sdio_get_drvdata(func);
- if (!card || !card->adapter) {
- pr_err("resume: invalid card or adapter\n");
- return 0;
- }
- } else {
- pr_err("resume: sdio_func is not specified\n");
+ pm_flag = sdio_get_host_pm_caps(func);
+ card = sdio_get_drvdata(func);
+ if (!card || !card->adapter) {
+ dev_err(dev, "resume: invalid card or adapter\n");
return 0;
}
@@ -272,23 +267,18 @@ static int mwifiex_sdio_suspend(struct device *dev)
mmc_pm_flag_t pm_flag = 0;
int ret = 0;
- if (func) {
- pm_flag = sdio_get_host_pm_caps(func);
- pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
- sdio_func_id(func), pm_flag);
- if (!(pm_flag & MMC_PM_KEEP_POWER)) {
- pr_err("%s: cannot remain alive while host is"
- " suspended\n", sdio_func_id(func));
- return -ENOSYS;
- }
+ pm_flag = sdio_get_host_pm_caps(func);
+ pr_debug("cmd: %s: suspend: PM flag = 0x%x\n",
+ sdio_func_id(func), pm_flag);
+ if (!(pm_flag & MMC_PM_KEEP_POWER)) {
+ dev_err(dev, "%s: cannot remain alive while host is"
+ " suspended\n", sdio_func_id(func));
+ return -ENOSYS;
+ }
- card = sdio_get_drvdata(func);
- if (!card) {
- dev_err(dev, "suspend: invalid card\n");
- return 0;
- }
- } else {
- pr_err("suspend: sdio_func is not specified\n");
+ card = sdio_get_drvdata(func);
+ if (!card) {
+ dev_err(dev, "suspend: invalid card\n");
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 10/11] mwifiex: stop checking for NULL drvata/intfdata
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (7 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 09/11] mwifiex: sdio: don't check for NULL sdio_func Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-18 14:00 ` [PATCH v4 11/11] mwifiex: pcie: stop checking for NULL adapter->card Amitkumar Karwar
2016-11-19 7:19 ` [v4, 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Kalle Valo
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris, Amitkumar Karwar
From: Brian Norris <briannorris@chromium.org>
These are never NULL, so stop making people think they might be.
I don't change this for SDIO because SDIO has a racy card-reset handler
that reallocates this struct. I'd rather not touch that mess right now.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Xinming Hu <huxm@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
v4: Same as v1, v2, v3
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 14 +++++---------
drivers/net/wireless/marvell/mwifiex/usb.c | 15 +++------------
2 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index f6d28d9..98299a1 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -118,10 +118,6 @@ static int mwifiex_pcie_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card) {
- dev_err(dev, "card structure is not valid\n");
- return 0;
- }
/* Might still be loading firmware */
wait_for_completion(&card->fw_done);
@@ -166,8 +162,9 @@ static int mwifiex_pcie_resume(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
- dev_err(dev, "Card or adapter structure is not valid\n");
+
+ if (!card->adapter) {
+ dev_err(dev, "adapter structure is not valid\n");
return 0;
}
@@ -249,8 +246,6 @@ static void mwifiex_pcie_remove(struct pci_dev *pdev)
struct mwifiex_private *priv;
card = pci_get_drvdata(pdev);
- if (!card)
- return;
wait_for_completion(&card->fw_done);
@@ -2243,7 +2238,8 @@ static irqreturn_t mwifiex_pcie_interrupt(int irq, void *context)
}
card = pci_get_drvdata(pdev);
- if (!card || !card->adapter) {
+
+ if (!card->adapter) {
pr_err("info: %s: card=%p adapter=%p\n", __func__, card,
card ? card->adapter : NULL);
goto exit;
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 55e3a01..c3f696a 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -503,11 +503,6 @@ static int mwifiex_usb_suspend(struct usb_interface *intf, pm_message_t message)
struct usb_tx_data_port *port;
int i, j;
- if (!card) {
- dev_err(&intf->dev, "%s: card is NULL\n", __func__);
- return 0;
- }
-
/* Might still be loading firmware */
wait_for_completion(&card->fw_done);
@@ -574,8 +569,9 @@ static int mwifiex_usb_resume(struct usb_interface *intf)
struct mwifiex_adapter *adapter;
int i;
- if (!card || !card->adapter) {
- pr_err("%s: card or card->adapter is NULL\n", __func__);
+ if (!card->adapter) {
+ dev_err(&intf->dev, "%s: card->adapter is NULL\n",
+ __func__);
return 0;
}
adapter = card->adapter;
@@ -617,11 +613,6 @@ static void mwifiex_usb_disconnect(struct usb_interface *intf)
struct usb_card_rec *card = usb_get_intfdata(intf);
struct mwifiex_adapter *adapter;
- if (!card) {
- dev_err(&intf->dev, "%s: card is NULL\n", __func__);
- return;
- }
-
wait_for_completion(&card->fw_done);
adapter = card->adapter;
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 11/11] mwifiex: pcie: stop checking for NULL adapter->card
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (8 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 10/11] mwifiex: stop checking for NULL drvata/intfdata Amitkumar Karwar
@ 2016-11-18 14:00 ` Amitkumar Karwar
2016-11-19 7:19 ` [v4, 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Kalle Valo
10 siblings, 0 replies; 12+ messages in thread
From: Amitkumar Karwar @ 2016-11-18 14:00 UTC (permalink / raw)
To: linux-wireless
Cc: Cathy Luo, Nishant Sarmukadam, rajatja, briannorris,
dmitry.torokhov, Brian Norris
From: Brian Norris <briannorris@chromium.org>
It should never be NULL here, and to think otherwise makes things
confusing.
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v2: Same as v1
v3: Below checkpatch warnings are resolved
WARNING: please, no spaces at the start of a line
#50: FILE: drivers/net/wireless/marvell/mwifiex/pcie.c:3037:
+ } else {$
WARNING: please, no spaces at the start of a line
#62: FILE: drivers/net/wireless/marvell/mwifiex/pcie.c:3044:
+ }$
v4: Same as v3
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 53 +++++++++++++----------------
1 file changed, 23 insertions(+), 30 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 98299a1..4b89f55 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -3015,31 +3015,28 @@ static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
{
struct pcie_service_card *card = adapter->card;
- struct pci_dev *pdev;
+ struct pci_dev *pdev = card->dev;
int i;
- if (card) {
- pdev = card->dev;
- if (card->msix_enable) {
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- synchronize_irq(card->msix_entries[i].vector);
+ if (card->msix_enable) {
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ synchronize_irq(card->msix_entries[i].vector);
- for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
- free_irq(card->msix_entries[i].vector,
- &card->msix_ctx[i]);
+ for (i = 0; i < MWIFIEX_NUM_MSIX_VECTORS; i++)
+ free_irq(card->msix_entries[i].vector,
+ &card->msix_ctx[i]);
- card->msix_enable = 0;
- pci_disable_msix(pdev);
- } else {
- mwifiex_dbg(adapter, INFO,
- "%s(): calling free_irq()\n", __func__);
- free_irq(card->dev->irq, &card->share_irq_ctx);
+ card->msix_enable = 0;
+ pci_disable_msix(pdev);
+ } else {
+ mwifiex_dbg(adapter, INFO,
+ "%s(): calling free_irq()\n", __func__);
+ free_irq(card->dev->irq, &card->share_irq_ctx);
- if (card->msi_enable)
- pci_disable_msi(pdev);
- }
- card->adapter = NULL;
+ if (card->msi_enable)
+ pci_disable_msi(pdev);
}
+ card->adapter = NULL;
}
/* This function initializes the PCI-E host memory space, WCB rings, etc.
@@ -3122,18 +3119,14 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
adapter->seq_num = 0;
adapter->tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_4K;
- if (card) {
- if (reg->sleep_cookie)
- mwifiex_pcie_delete_sleep_cookie_buf(adapter);
-
- mwifiex_pcie_delete_cmdrsp_buf(adapter);
- mwifiex_pcie_delete_evtbd_ring(adapter);
- mwifiex_pcie_delete_rxbd_ring(adapter);
- mwifiex_pcie_delete_txbd_ring(adapter);
- card->cmdrsp_buf = NULL;
- }
+ if (reg->sleep_cookie)
+ mwifiex_pcie_delete_sleep_cookie_buf(adapter);
- return;
+ mwifiex_pcie_delete_cmdrsp_buf(adapter);
+ mwifiex_pcie_delete_evtbd_ring(adapter);
+ mwifiex_pcie_delete_rxbd_ring(adapter);
+ mwifiex_pcie_delete_txbd_ring(adapter);
+ card->cmdrsp_buf = NULL;
}
static struct mwifiex_if_ops pcie_ops = {
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [v4, 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm
2016-11-18 14:00 [PATCH v4 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm Amitkumar Karwar
` (9 preceding siblings ...)
2016-11-18 14:00 ` [PATCH v4 11/11] mwifiex: pcie: stop checking for NULL adapter->card Amitkumar Karwar
@ 2016-11-19 7:19 ` Kalle Valo
10 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2016-11-19 7:19 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
briannorris, dmitry.torokhov, Shengzhen Li, Amitkumar Karwar
Amitkumar Karwar <akarwar@marvell.com> wrote:
> From: Shengzhen Li <szli@marvell.com>
>
> We may get SLEEP event from firmware even if TXDone interrupt
> for last Tx packet is still pending. In this case, we may
> end up accessing PCIe memory for handling TXDone after power
> save handshake is completed. This causes kernel crash with
> external abort.
>
> This patch will only allow downloading sleep confirm
> when no tx done interrupt is pending in the hardware.
>
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Shengzhen Li <szli@marvell.com>
> Tested-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
11 patches applied to wireless-drivers-next.git, thanks.
eb2428fb1a21 mwifiex: check tx_hw_pending before downloading sleep confirm
6712076883ca mwifiex: complete blocked power save handshake in main process
4a79aa17d53e mwifiex: resolve races between async FW init (failure) and device removal
a1beec4b2c6f mwifiex: remove redundant pdev check in suspend/resume handlers
7ccdf72f9128 mwifiex: don't pretend to resume while remove()'ing
b42dbb27e326 mwifiex: resolve suspend() race with async FW init failure
bcf28a2f2846 mwifiex: reset card->adapter during device unregister
58b7033551cd mwifiex: usb: handle HS failures
6caf34cb3a92 mwifiex: sdio: don't check for NULL sdio_func
e98fb11ffa0a mwifiex: stop checking for NULL drvata/intfdata
97489c284d9f mwifiex: pcie: stop checking for NULL adapter->card
--
https://patchwork.kernel.org/patch/9436513/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 12+ messages in thread