2015-12-12 13:22:49

by Maya Erez

[permalink] [raw]
Subject: [PATCH 0/2] wil6210 patches

Those patches fix wil6210 issue and add support for platform specific recovery after FW crash

Hamad Kadmany (1):
wil6210: fix kernel OOPS when stopping interface during Rx traffic

Maya Erez (1):
wil6210: support for platform specific crash recovery

drivers/net/wireless/ath/wil6210/interrupt.c | 8 +++--
drivers/net/wireless/ath/wil6210/pcie_bus.c | 30 ++++++++++++++++--
drivers/net/wireless/ath/wil6210/rx_reorder.c | 12 ++++++-
drivers/net/wireless/ath/wil6210/wil6210.h | 1 +
drivers/net/wireless/ath/wil6210/wil_crash_dump.c | 3 +-
drivers/net/wireless/ath/wil6210/wil_platform.c | 3 +-
drivers/net/wireless/ath/wil6210/wil_platform.h | 38 +++++++++++++++++++++--
7 files changed, 84 insertions(+), 11 deletions(-)

--
1.8.5.2



2015-12-12 13:22:53

by Maya Erez

[permalink] [raw]
Subject: [PATCH 2/2] wil6210: support for platform specific crash recovery

Added a simple interface for platform to perform crash
recovery.
When firmware crashes, wil driver can notify the platform
which can trigger a crash recovery process. During
the process the platform can request a ram dump
from the wil driver as well as control when firmware
recovery will start. This interface allows the platform
to implement a more advanced crash recovery, for
example to reset dependent subsystems in proper order, or
to provide its own notifications during the recovery process.

Signed-off-by: Lior David <[email protected]>
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/interrupt.c | 8 +++--
drivers/net/wireless/ath/wil6210/pcie_bus.c | 30 ++++++++++++++++--
drivers/net/wireless/ath/wil6210/wil6210.h | 1 +
drivers/net/wireless/ath/wil6210/wil_crash_dump.c | 3 +-
drivers/net/wireless/ath/wil6210/wil_platform.c | 3 +-
drivers/net/wireless/ath/wil6210/wil_platform.h | 38 +++++++++++++++++++++--
6 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/interrupt.c b/drivers/net/wireless/ath/wil6210/interrupt.c
index 50c136e..4f2ffa5 100644
--- a/drivers/net/wireless/ath/wil6210/interrupt.c
+++ b/drivers/net/wireless/ath/wil6210/interrupt.c
@@ -394,9 +394,13 @@ static irqreturn_t wil6210_irq_misc_thread(int irq, void *cookie)
wil_fw_core_dump(wil);
wil_notify_fw_error(wil);
isr &= ~ISR_MISC_FW_ERROR;
- wil_fw_error_recovery(wil);
+ if (wil->platform_ops.notify_crash) {
+ wil_err(wil, "notify platform driver about FW crash");
+ wil->platform_ops.notify_crash(wil->platform_handle);
+ } else {
+ wil_fw_error_recovery(wil);
+ }
}
-
if (isr & ISR_MISC_MBOX_EVT) {
wil_dbg_irq(wil, "MBOX event\n");
wmi_recv_cmd(wil);
diff --git a/drivers/net/wireless/ath/wil6210/pcie_bus.c b/drivers/net/wireless/ath/wil6210/pcie_bus.c
index 1a3142c3..e36f2a0 100644
--- a/drivers/net/wireless/ath/wil6210/pcie_bus.c
+++ b/drivers/net/wireless/ath/wil6210/pcie_bus.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2014 Qualcomm Atheros, Inc.
+ * Copyright (c) 2012-2015 Qualcomm Atheros, Inc.
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -125,11 +125,37 @@ static int wil_if_pcie_disable(struct wil6210_priv *wil)
return 0;
}

+static int wil_platform_rop_ramdump(void *wil_handle, void *buf, uint32_t size)
+{
+ struct wil6210_priv *wil = wil_handle;
+
+ if (!wil)
+ return -EINVAL;
+
+ return wil_fw_copy_crash_dump(wil, buf, size);
+}
+
+static int wil_platform_rop_fw_recovery(void *wil_handle)
+{
+ struct wil6210_priv *wil = wil_handle;
+
+ if (!wil)
+ return -EINVAL;
+
+ wil_fw_error_recovery(wil);
+
+ return 0;
+}
+
static int wil_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct wil6210_priv *wil;
struct device *dev = &pdev->dev;
int rc;
+ const struct wil_platform_rops rops = {
+ .ramdump = wil_platform_rop_ramdump,
+ .fw_recovery = wil_platform_rop_fw_recovery,
+ };

/* check HW */
dev_info(&pdev->dev, WIL_NAME
@@ -154,7 +180,7 @@ static int wil_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
/* rollback to if_free */

wil->platform_handle =
- wil_platform_init(&pdev->dev, &wil->platform_ops);
+ wil_platform_init(&pdev->dev, &wil->platform_ops, &rops, wil);
if (!wil->platform_handle) {
rc = -ENODEV;
wil_err(wil, "wil_platform_init failed\n");
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index ade5f3b8..235e205 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -828,6 +828,7 @@ int wil_can_suspend(struct wil6210_priv *wil, bool is_runtime);
int wil_suspend(struct wil6210_priv *wil, bool is_runtime);
int wil_resume(struct wil6210_priv *wil, bool is_runtime);

+int wil_fw_copy_crash_dump(struct wil6210_priv *wil, void *dest, u32 size);
void wil_fw_core_dump(struct wil6210_priv *wil);

#endif /* __WIL6210_H__ */
diff --git a/drivers/net/wireless/ath/wil6210/wil_crash_dump.c b/drivers/net/wireless/ath/wil6210/wil_crash_dump.c
index 7e70934..b57d280 100644
--- a/drivers/net/wireless/ath/wil6210/wil_crash_dump.c
+++ b/drivers/net/wireless/ath/wil6210/wil_crash_dump.c
@@ -51,8 +51,7 @@ static int wil_fw_get_crash_dump_bounds(struct wil6210_priv *wil,
return 0;
}

-static int wil_fw_copy_crash_dump(struct wil6210_priv *wil, void *dest,
- u32 size)
+int wil_fw_copy_crash_dump(struct wil6210_priv *wil, void *dest, u32 size)
{
int i;
const struct fw_map *map;
diff --git a/drivers/net/wireless/ath/wil6210/wil_platform.c b/drivers/net/wireless/ath/wil6210/wil_platform.c
index 2e831bf..4eed05bd 100644
--- a/drivers/net/wireless/ath/wil6210/wil_platform.c
+++ b/drivers/net/wireless/ath/wil6210/wil_platform.c
@@ -33,7 +33,8 @@ void wil_platform_modexit(void)
* It returns a handle which is used with the rest of the API
*
*/
-void *wil_platform_init(struct device *dev, struct wil_platform_ops *ops)
+void *wil_platform_init(struct device *dev, struct wil_platform_ops *ops,
+ const struct wil_platform_rops *rops, void *wil_handle)
{
void *handle = ops; /* to return some non-NULL for 'void' impl. */

diff --git a/drivers/net/wireless/ath/wil6210/wil_platform.h b/drivers/net/wireless/ath/wil6210/wil_platform.h
index d7fa19b..9a949d9 100644
--- a/drivers/net/wireless/ath/wil6210/wil_platform.h
+++ b/drivers/net/wireless/ath/wil6210/wil_platform.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2014 Qualcomm Atheros, Inc.
+ * Copyright (c) 2014-2015 Qualcomm Atheros, Inc.
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -20,16 +20,48 @@
struct device;

/**
- * struct wil_platform_ops - wil platform module callbacks
+ * struct wil_platform_ops - wil platform module calls from this
+ * driver to platform driver
*/
struct wil_platform_ops {
int (*bus_request)(void *handle, uint32_t kbps /* KBytes/Sec */);
int (*suspend)(void *handle);
int (*resume)(void *handle);
void (*uninit)(void *handle);
+ int (*notify_crash)(void *handle);
};

-void *wil_platform_init(struct device *dev, struct wil_platform_ops *ops);
+/**
+ * struct wil_platform_rops - wil platform module callbacks from
+ * platform driver to this driver
+ * @ramdump: store a ramdump from the wil firmware. The platform
+ * driver may add additional data to the ramdump to
+ * generate the final crash dump.
+ * @fw_recovery: start a firmware recovery process. Called as
+ * part of a crash recovery process which may include other
+ * related platform subsystems.
+ */
+struct wil_platform_rops {
+ int (*ramdump)(void *wil_handle, void *buf, uint32_t size);
+ int (*fw_recovery)(void *wil_handle);
+};
+
+/**
+ * wil_platform_init - initialize the platform driver
+ *
+ * @dev - pointer to the wil6210 device
+ * @ops - structure with platform driver operations. Platform
+ * driver will fill this structure with function pointers.
+ * @rops - structure with callbacks from platform driver to
+ * this driver. The platform driver copies the structure to
+ * its own storage. Can be NULL if this driver does not
+ * support crash recovery.
+ * @wil_handle - context for this driver that will be passed
+ * when platform driver invokes one of the callbacks in
+ * rops. May be NULL if rops is NULL.
+ */
+void *wil_platform_init(struct device *dev, struct wil_platform_ops *ops,
+ const struct wil_platform_rops *rops, void *wil_handle);

int __init wil_platform_modinit(void);
void wil_platform_modexit(void);
--
1.8.5.2


2015-12-16 14:39:36

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] wil6210: fix kernel OOPS when stopping interface during Rx traffic

Maya Erez <[email protected]> writes:

> From: Hamad Kadmany <[email protected]>
>
> When network interface is stopping, some resources may
> be already released by the network stack, and Rx frames
> cause kernel OOPS (observed one is in netfilter code)
>
> Proper solution is to drop packets pending in reorder buffer.
>
> Signed-off-by: Vladimir Kondratiev <[email protected]>
> Signed-off-by: Maya Erez <[email protected]>

This is missing Hamad's Signed-off-by line. It should be the first
before Vladimir's and yours.

--
Kalle Valo

2015-12-12 13:22:51

by Maya Erez

[permalink] [raw]
Subject: [PATCH 1/2] wil6210: fix kernel OOPS when stopping interface during Rx traffic

From: Hamad Kadmany <[email protected]>

When network interface is stopping, some resources may
be already released by the network stack, and Rx frames
cause kernel OOPS (observed one is in netfilter code)

Proper solution is to drop packets pending in reorder buffer.

Signed-off-by: Vladimir Kondratiev <[email protected]>
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/rx_reorder.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wil6210/rx_reorder.c b/drivers/net/wireless/ath/wil6210/rx_reorder.c
index e3d1be8..32031e7 100644
--- a/drivers/net/wireless/ath/wil6210/rx_reorder.c
+++ b/drivers/net/wireless/ath/wil6210/rx_reorder.c
@@ -261,9 +261,19 @@ struct wil_tid_ampdu_rx *wil_tid_ampdu_rx_alloc(struct wil6210_priv *wil,
void wil_tid_ampdu_rx_free(struct wil6210_priv *wil,
struct wil_tid_ampdu_rx *r)
{
+ int i;
+
if (!r)
return;
- wil_release_reorder_frames(wil, r, r->head_seq_num + r->buf_size);
+
+ /* Do not pass remaining frames to the network stack - it may be
+ * not expecting to get any more Rx. Rx from here may lead to
+ * kernel OOPS since some per-socket accounting info was already
+ * released.
+ */
+ for (i = 0; i < r->buf_size; i++)
+ kfree_skb(r->reorder_buf[i]);
+
kfree(r->reorder_buf);
kfree(r->reorder_time);
kfree(r);
--
1.8.5.2