2014-10-01 12:05:49

by Vladimir Kondratiev

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

2 new features:
- IOCTL API for the card memory access,
- manual FW recovery, to allow FW dump and other debug

Vladimir Kondratiev (2):
wil6210: manual FW error recovery mode
wil6210: atomic I/O for the card memory

MAINTAINERS | 1 +
drivers/net/wireless/ath/wil6210/Makefile | 1 +
drivers/net/wireless/ath/wil6210/cfg80211.c | 4 +
drivers/net/wireless/ath/wil6210/debugfs.c | 67 +++++++++++
drivers/net/wireless/ath/wil6210/ioctl.c | 173 ++++++++++++++++++++++++++++
drivers/net/wireless/ath/wil6210/main.c | 46 ++++++--
drivers/net/wireless/ath/wil6210/netdev.c | 12 ++
drivers/net/wireless/ath/wil6210/wil6210.h | 14 ++-
drivers/net/wireless/ath/wil6210/wmi.c | 1 +
include/uapi/linux/wil6210_uapi.h | 88 ++++++++++++++
10 files changed, 398 insertions(+), 9 deletions(-)
create mode 100644 drivers/net/wireless/ath/wil6210/ioctl.c
create mode 100644 include/uapi/linux/wil6210_uapi.h

--
1.9.1



2014-10-01 12:05:55

by Vladimir Kondratiev

[permalink] [raw]
Subject: [PATCH 2/2] wil6210: atomic I/O for the card memory

Introduce netdev IOCTLs, to be used by the debug tools.

Allows to read/write single dword value or
memory block, aligned to dword
Different address modes supported:
- BAR offset
- Firmware "linker" address
- target's AHB bus

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
MAINTAINERS | 1 +
drivers/net/wireless/ath/wil6210/Makefile | 1 +
drivers/net/wireless/ath/wil6210/ioctl.c | 173 +++++++++++++++++++++++++++++
drivers/net/wireless/ath/wil6210/netdev.c | 12 ++
drivers/net/wireless/ath/wil6210/wil6210.h | 2 +
include/uapi/linux/wil6210_uapi.h | 88 +++++++++++++++
6 files changed, 277 insertions(+)
create mode 100644 drivers/net/wireless/ath/wil6210/ioctl.c
create mode 100644 include/uapi/linux/wil6210_uapi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 133d2fd..5f2b1dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1617,6 +1617,7 @@ L: [email protected]
S: Supported
W: http://wireless.kernel.org/en/users/Drivers/wil6210
F: drivers/net/wireless/ath/wil6210/
+F: include/uapi/linux/wil6210_uapi.h

CARL9170 LINUX COMMUNITY WIRELESS DRIVER
M: Christian Lamparter <[email protected]>
diff --git a/drivers/net/wireless/ath/wil6210/Makefile b/drivers/net/wireless/ath/wil6210/Makefile
index 05f29a6..8ad4b5f 100644
--- a/drivers/net/wireless/ath/wil6210/Makefile
+++ b/drivers/net/wireless/ath/wil6210/Makefile
@@ -10,6 +10,7 @@ wil6210-y += interrupt.o
wil6210-y += txrx.o
wil6210-y += debug.o
wil6210-y += rx_reorder.o
+wil6210-y += ioctl.o
wil6210-y += fw.o
wil6210-$(CONFIG_WIL6210_TRACING) += trace.o
wil6210-y += wil_platform.o
diff --git a/drivers/net/wireless/ath/wil6210/ioctl.c b/drivers/net/wireless/ath/wil6210/ioctl.c
new file mode 100644
index 0000000..e9c0673
--- /dev/null
+++ b/drivers/net/wireless/ath/wil6210/ioctl.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright (c) 2014 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
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <linux/uaccess.h>
+
+#include "wil6210.h"
+#include <uapi/linux/wil6210_uapi.h>
+
+#define wil_hex_dump_ioctl(prefix_str, buf, len) \
+ print_hex_dump_debug("DBG[IOC ]" prefix_str, \
+ DUMP_PREFIX_OFFSET, 16, 1, buf, len, true)
+#define wil_dbg_ioctl(wil, fmt, arg...) wil_dbg(wil, "DBG[IOC ]" fmt, ##arg)
+
+static void __iomem *wil_ioc_addr(struct wil6210_priv *wil, uint32_t addr,
+ uint32_t size, enum wil_memio_op op)
+{
+ void __iomem *a;
+ u32 off;
+
+ switch (op & wil_mmio_addr_mask) {
+ case wil_mmio_addr_linker:
+ a = wmi_buffer(wil, cpu_to_le32(addr));
+ break;
+ case wil_mmio_addr_ahb:
+ a = wmi_addr(wil, addr);
+ break;
+ case wil_mmio_addr_bar:
+ a = wmi_addr(wil, addr + WIL6210_FW_HOST_OFF);
+ break;
+ default:
+ wil_err(wil, "Unsupported address mode, op = 0x%08x\n", op);
+ return NULL;
+ }
+
+ off = a - wil->csr;
+ if (size >= WIL6210_MEM_SIZE - off) {
+ wil_err(wil, "Requested block does not fit into memory: "
+ "off = 0x%08x size = 0x%08x\n", off, size);
+ return NULL;
+ }
+
+ return a;
+}
+
+static int wil_ioc_memio_dword(struct wil6210_priv *wil, void __user *data)
+{
+ struct wil_memio io;
+ void __iomem *a;
+ bool need_copy = false;
+
+ if (copy_from_user(&io, data, sizeof(io)))
+ return -EFAULT;
+
+ wil_dbg_ioctl(wil, "IO: addr = 0x%08x val = 0x%08x op = 0x%08x\n",
+ io.addr, io.val, io.op);
+
+ a = wil_ioc_addr(wil, io.addr, sizeof(u32), io.op);
+ if (!a) {
+ wil_err(wil, "invalid address 0x%08x, op = 0x%08x\n", io.addr,
+ io.op);
+ return -EINVAL;
+ }
+ /* operation */
+ switch (io.op & wil_mmio_op_mask) {
+ case wil_mmio_read:
+ io.val = ioread32(a);
+ need_copy = true;
+ break;
+ case wil_mmio_write:
+ iowrite32(io.val, a);
+ wmb(); /* make sure write propagated to HW */
+ break;
+ default:
+ wil_err(wil, "Unsupported operation, op = 0x%08x\n", io.op);
+ return -EINVAL;
+ }
+
+ if (need_copy) {
+ wil_dbg_ioctl(wil, "IO done: addr = 0x%08x"
+ " val = 0x%08x op = 0x%08x\n",
+ io.addr, io.val, io.op);
+ if (copy_to_user(data, &io, sizeof(io)))
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+static int wil_ioc_memio_block(struct wil6210_priv *wil, void __user *data)
+{
+ struct wil_memio_block io;
+ void *block;
+ void __iomem *a;
+ int rc = 0;
+
+ if (copy_from_user(&io, data, sizeof(io)))
+ return -EFAULT;
+
+ wil_dbg_ioctl(wil, "IO: addr = 0x%08x size = 0x%08x op = 0x%08x\n",
+ io.addr, io.size, io.op);
+
+ /* size */
+ if (io.size % 4) {
+ wil_err(wil, "size is not multiple of 4: 0x%08x\n", io.size);
+ return -EINVAL;
+ }
+
+ a = wil_ioc_addr(wil, io.addr, io.size, io.op);
+ if (!a) {
+ wil_err(wil, "invalid address 0x%08x, op = 0x%08x\n", io.addr,
+ io.op);
+ return -EINVAL;
+ }
+
+ block = kmalloc(io.size, GFP_USER);
+ if (!block)
+ return -ENOMEM;
+
+ /* operation */
+ switch (io.op & wil_mmio_op_mask) {
+ case wil_mmio_read:
+ wil_memcpy_fromio_32(block, a, io.size);
+ wil_hex_dump_ioctl("Read ", block, io.size);
+ if (copy_to_user(io.block, block, io.size)) {
+ rc = -EFAULT;
+ goto out_free;
+ }
+ break;
+ case wil_mmio_write:
+ if (copy_from_user(block, io.block, io.size)) {
+ rc = -EFAULT;
+ goto out_free;
+ }
+ wil_memcpy_toio_32(a, block, io.size);
+ wmb(); /* make sure write propagated to HW */
+ wil_hex_dump_ioctl("Write ", block, io.size);
+ break;
+ default:
+ wil_err(wil, "Unsupported operation, op = 0x%08x\n", io.op);
+ rc = -EINVAL;
+ break;
+ }
+
+out_free:
+ kfree(block);
+ return rc;
+}
+
+int wil_ioctl(struct wil6210_priv *wil, void __user *data, int cmd)
+{
+ switch (cmd) {
+ case WIL_IOCTL_MEMIO:
+ return wil_ioc_memio_dword(wil, data);
+ case WIL_IOCTL_MEMIO_BLOCK:
+ return wil_ioc_memio_block(wil, data);
+ default:
+ wil_dbg_ioctl(wil, "Unsupported IOCTL 0x%04x\n", cmd);
+ return -ENOIOCTLCMD;
+ }
+}
diff --git a/drivers/net/wireless/ath/wil6210/netdev.c b/drivers/net/wireless/ath/wil6210/netdev.c
index c3f0ddf..2399651 100644
--- a/drivers/net/wireless/ath/wil6210/netdev.c
+++ b/drivers/net/wireless/ath/wil6210/netdev.c
@@ -52,6 +52,17 @@ static int wil_change_mtu(struct net_device *ndev, int new_mtu)
return 0;
}

+static int wil_do_ioctl(struct net_device *ndev, struct ifreq *ifr, int cmd)
+{
+ struct wil6210_priv *wil = ndev_to_wil(ndev);
+
+ int ret = wil_ioctl(wil, ifr->ifr_data, cmd);
+
+ wil_dbg_misc(wil, "ioctl(0x%04x) -> %d\n", cmd, ret);
+
+ return ret;
+}
+
static const struct net_device_ops wil_netdev_ops = {
.ndo_open = wil_open,
.ndo_stop = wil_stop,
@@ -59,6 +70,7 @@ static const struct net_device_ops wil_netdev_ops = {
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
.ndo_change_mtu = wil_change_mtu,
+ .ndo_do_ioctl = wil_do_ioctl,
};

static int wil6210_netdev_poll_rx(struct napi_struct *napi, int budget)
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 2991609..61cceca 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -595,5 +595,7 @@ void wil6210_unmask_irq_rx(struct wil6210_priv *wil);

int wil_iftype_nl2wmi(enum nl80211_iftype type);

+int wil_ioctl(struct wil6210_priv *wil, void __user *data, int cmd);
int wil_request_firmware(struct wil6210_priv *wil, const char *name);
+
#endif /* __WIL6210_H__ */
diff --git a/include/uapi/linux/wil6210_uapi.h b/include/uapi/linux/wil6210_uapi.h
new file mode 100644
index 0000000..d9ec1d0
--- /dev/null
+++ b/include/uapi/linux/wil6210_uapi.h
@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2014 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
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef __WIL6210_UAPI_H__
+#define __WIL6210_UAPI_H__
+
+#if !defined(__KERNEL__)
+#define __user
+#endif
+
+#include <linux/sockios.h>
+
+/* Numbers SIOCDEVPRIVATE and SIOCDEVPRIVATE + 1
+ * are used by Android devices to implement PNO (preferred network offload).
+ * Albeit it is temporary solution, use different numbers to avoid conflicts
+ */
+
+/**
+ * Perform 32-bit I/O operation to the card memory
+ *
+ * User code should arrange data in memory like this:
+ *
+ * struct wil_memio io;
+ * struct ifreq ifr = {
+ * .ifr_data = &io,
+ * };
+ */
+#define WIL_IOCTL_MEMIO (SIOCDEVPRIVATE + 2)
+
+/**
+ * Perform block I/O operation to the card memory
+ *
+ * User code should arrange data in memory like this:
+ *
+ * void *buf;
+ * struct wil_memio_block io = {
+ * .block = buf,
+ * };
+ * struct ifreq ifr = {
+ * .ifr_data = &io,
+ * };
+ */
+#define WIL_IOCTL_MEMIO_BLOCK (SIOCDEVPRIVATE + 3)
+
+/**
+ * operation to perform
+ *
+ * @wil_mmio_op_mask - bits defining operation,
+ * @wil_mmio_addr_mask - bits defining addressing mode
+ */
+enum wil_memio_op {
+ wil_mmio_read = 0,
+ wil_mmio_write = 1,
+ wil_mmio_op_mask = 0xff,
+ wil_mmio_addr_linker = 0 << 8,
+ wil_mmio_addr_ahb = 1 << 8,
+ wil_mmio_addr_bar = 2 << 8,
+ wil_mmio_addr_mask = 0xff00,
+};
+
+struct wil_memio {
+ uint32_t op; /* enum wil_memio_op */
+ uint32_t addr; /* should be 32-bit aligned */
+ uint32_t val;
+};
+
+struct wil_memio_block {
+ uint32_t op; /* enum wil_memio_op */
+ uint32_t addr; /* should be 32-bit aligned */
+ uint32_t size; /* should be multiple of 4 */
+ void __user *block; /* block address */
+};
+
+#endif /* __WIL6210_UAPI_H__ */
+
--
1.9.1


2014-10-01 12:05:52

by Vladimir Kondratiev

[permalink] [raw]
Subject: [PATCH 1/2] wil6210: manual FW error recovery mode

Introduce manual FW recovery mode. It is activated if module parameter
@no_fw_recovery set to true. May be changed at runtime.

Recovery information provided by new "recovery" debugfs file. It prints:

mode = [auto|manual]
state = [idle|pending|running]

In manual mode, after FW error, recovery won't start automatically. Instead,
after notification to user space, recovery waits in "pending" state, as indicated by the
"recovery" debugfs file. User space tools may perform data collection and allow to
continue recovery by writing "run" to the "recovery" debugfs file.
Alternatively, recovery pending may be canceled by stopping network interface
i.e. 'ifconfig wlan0 down'

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 4 ++
drivers/net/wireless/ath/wil6210/debugfs.c | 67 +++++++++++++++++++++++++++++
drivers/net/wireless/ath/wil6210/main.c | 46 ++++++++++++++++----
drivers/net/wireless/ath/wil6210/wil6210.h | 12 +++++-
drivers/net/wireless/ath/wil6210/wmi.c | 1 +
5 files changed, 121 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index f3a31e8..d9f4b30 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -728,6 +728,8 @@ static int wil_cfg80211_start_ap(struct wiphy *wiphy,
wil_print_bcon_data(bcon);
}

+ wil_set_recovery_state(wil, fw_recovery_idle);
+
mutex_lock(&wil->mutex);

__wil_down(wil);
@@ -775,6 +777,8 @@ static int wil_cfg80211_stop_ap(struct wiphy *wiphy,

wil_dbg_misc(wil, "%s()\n", __func__);

+ wil_set_recovery_state(wil, fw_recovery_idle);
+
mutex_lock(&wil->mutex);

rc = wmi_pcp_stop(wil);
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index eb2204e..54a6ddc 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1041,6 +1041,71 @@ static const struct file_operations fops_info = {
.llseek = seq_lseek,
};

+/*---------recovery------------*/
+/* mode = [manual|auto]
+ * state = [idle|pending|running]
+ */
+static ssize_t wil_read_file_recovery(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct wil6210_priv *wil = file->private_data;
+ char buf[80];
+ int n;
+ static const char * const sstate[] = {"idle", "pending", "running"};
+
+ n = snprintf(buf, sizeof(buf), "mode = %s\nstate = %s\n",
+ no_fw_recovery ? "manual" : "auto",
+ sstate[wil->recovery_state]);
+
+ n = min_t(int, n, sizeof(buf));
+
+ return simple_read_from_buffer(user_buf, count, ppos,
+ buf, n);
+}
+
+static ssize_t wil_write_file_recovery(struct file *file,
+ const char __user *buf_,
+ size_t count, loff_t *ppos)
+{
+ struct wil6210_priv *wil = file->private_data;
+ static const char run_command[] = "run";
+ char buf[sizeof(run_command) + 1]; /* to detect "runx" */
+ ssize_t rc;
+
+ if (wil->recovery_state != fw_recovery_pending) {
+ wil_err(wil, "No recovery pending\n");
+ return -EINVAL;
+ }
+
+ if (*ppos != 0) {
+ wil_err(wil, "Offset [%d]\n", (int)*ppos);
+ return -EINVAL;
+ }
+
+ if (count > sizeof(buf)) {
+ wil_err(wil, "Input too long, len = %d\n", (int)count);
+ return -EINVAL;
+ }
+
+ rc = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, buf_, count);
+ if (rc < 0)
+ return rc;
+
+ buf[rc] = '\0';
+ if (0 == strcmp(buf, run_command))
+ wil_set_recovery_state(wil, fw_recovery_running);
+ else
+ wil_err(wil, "Bad recovery command \"%s\"\n", buf);
+
+ return rc;
+}
+
+static const struct file_operations fops_recovery = {
+ .read = wil_read_file_recovery,
+ .write = wil_write_file_recovery,
+ .open = simple_open,
+};
+
/*---------Station matrix------------*/
static void wil_print_rxtid(struct seq_file *s, struct wil_tid_ampdu_rx *r)
{
@@ -1152,6 +1217,7 @@ static const struct {
{"freq", S_IRUGO, &fops_freq},
{"link", S_IRUGO, &fops_link},
{"info", S_IRUGO, &fops_info},
+ {"recovery", S_IRUGO | S_IWUSR, &fops_recovery},
};

static void wil6210_debugfs_init_files(struct wil6210_priv *wil,
@@ -1194,6 +1260,7 @@ static const struct dbg_off dbg_wil_off[] = {
WIL_FIELD(status, S_IRUGO | S_IWUSR, doff_ulong),
WIL_FIELD(fw_version, S_IRUGO, doff_u32),
WIL_FIELD(hw_version, S_IRUGO, doff_x32),
+ WIL_FIELD(recovery_count, S_IRUGO, doff_u32),
{},
};

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 0424763..6500caf 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -25,9 +25,9 @@
#define WAIT_FOR_DISCONNECT_TIMEOUT_MS 2000
#define WAIT_FOR_DISCONNECT_INTERVAL_MS 10

-static bool no_fw_recovery;
+bool no_fw_recovery;
module_param(no_fw_recovery, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(no_fw_recovery, " disable FW error recovery");
+MODULE_PARM_DESC(no_fw_recovery, " disable automatic FW error recovery");

static bool no_fw_load = true;
module_param(no_fw_load, bool, S_IRUGO | S_IWUSR);
@@ -191,17 +191,38 @@ static void wil_scan_timer_fn(ulong x)
schedule_work(&wil->fw_error_worker);
}

+static int wil_wait_for_recovery(struct wil6210_priv *wil)
+{
+ if (wait_event_interruptible(wil->wq, wil->recovery_state !=
+ fw_recovery_pending)) {
+ wil_err(wil, "Interrupt, canceling recovery\n");
+ return -ERESTARTSYS;
+ }
+ if (wil->recovery_state != fw_recovery_running) {
+ wil_info(wil, "Recovery cancelled\n");
+ return -EINTR;
+ }
+ wil_info(wil, "Proceed with recovery\n");
+ return 0;
+}
+
+void wil_set_recovery_state(struct wil6210_priv *wil, int state)
+{
+ wil_dbg_misc(wil, "%s(%d -> %d)\n", __func__,
+ wil->recovery_state, state);
+
+ wil->recovery_state = state;
+ wake_up_interruptible(&wil->wq);
+}
+
static void wil_fw_error_worker(struct work_struct *work)
{
- struct wil6210_priv *wil = container_of(work,
- struct wil6210_priv, fw_error_worker);
+ struct wil6210_priv *wil = container_of(work, struct wil6210_priv,
+ fw_error_worker);
struct wireless_dev *wdev = wil->wdev;

wil_dbg_misc(wil, "fw error worker\n");

- if (no_fw_recovery)
- return;
-
/* increment @recovery_count if less then WIL6210_FW_RECOVERY_TO
* passed since last recovery attempt
*/
@@ -224,8 +245,13 @@ static void wil_fw_error_worker(struct work_struct *work)
case NL80211_IFTYPE_STATION:
case NL80211_IFTYPE_P2P_CLIENT:
case NL80211_IFTYPE_MONITOR:
- wil_info(wil, "fw error recovery started (try %d)...\n",
+ wil_info(wil, "fw error recovery requested (try %d)...\n",
wil->recovery_count);
+ if (!no_fw_recovery)
+ wil->recovery_state = fw_recovery_running;
+ if (0 != wil_wait_for_recovery(wil))
+ break;
+
__wil_down(wil);
__wil_up(wil);
break;
@@ -302,6 +328,7 @@ int wil_priv_init(struct wil6210_priv *wil)

INIT_LIST_HEAD(&wil->pending_wmi_ev);
spin_lock_init(&wil->wmi_ev_lock);
+ init_waitqueue_head(&wil->wq);

wil->wmi_wq = create_singlethread_workqueue(WIL_NAME"_wmi");
if (!wil->wmi_wq)
@@ -331,6 +358,7 @@ void wil_priv_deinit(struct wil6210_priv *wil)
{
wil_dbg_misc(wil, "%s()\n", __func__);

+ wil_set_recovery_state(wil, fw_recovery_idle);
del_timer_sync(&wil->scan_timer);
cancel_work_sync(&wil->disconnect_worker);
cancel_work_sync(&wil->fw_error_worker);
@@ -573,6 +601,7 @@ int wil_reset(struct wil6210_priv *wil)
void wil_fw_error_recovery(struct wil6210_priv *wil)
{
wil_dbg_misc(wil, "starting fw error recovery\n");
+ wil->recovery_state = fw_recovery_pending;
schedule_work(&wil->fw_error_worker);
}

@@ -724,6 +753,7 @@ int wil_down(struct wil6210_priv *wil)

wil_dbg_misc(wil, "%s()\n", __func__);

+ wil_set_recovery_state(wil, fw_recovery_idle);
mutex_lock(&wil->mutex);
rc = __wil_down(wil);
mutex_unlock(&wil->mutex);
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 00c4f0a..2991609 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -23,6 +23,7 @@
#include <linux/timex.h>
#include "wil_platform.h"

+extern bool no_fw_recovery;

#define WIL_NAME "wil6210"
#define WIL_FW_NAME "wil6210.fw"
@@ -379,6 +380,12 @@ struct wil_sta_info {
unsigned long tid_rx_stop_requested[BITS_TO_LONGS(WIL_STA_TID_NUM)];
};

+enum {
+ fw_recovery_idle = 0,
+ fw_recovery_pending = 1,
+ fw_recovery_running = 2,
+};
+
struct wil6210_priv {
struct pci_dev *pdev;
int n_msi;
@@ -389,8 +396,10 @@ struct wil6210_priv {
u32 hw_version;
struct wil_board *board;
u8 n_mids; /* number of additional MIDs as reported by FW */
- int recovery_count; /* num of FW recovery attempts in a short time */
+ u32 recovery_count; /* num of FW recovery attempts in a short time */
+ u32 recovery_state; /* FW recovery state machine */
unsigned long last_fw_recovery; /* jiffies of last fw recovery */
+ wait_queue_head_t wq; /* for all wait_event() use */
/* profile */
u32 monitor_flags;
u32 secure_pcp; /* create secure PCP? */
@@ -507,6 +516,7 @@ void wil_priv_deinit(struct wil6210_priv *wil);
int wil_reset(struct wil6210_priv *wil);
void wil_set_itr_trsh(struct wil6210_priv *wil);
void wil_fw_error_recovery(struct wil6210_priv *wil);
+void wil_set_recovery_state(struct wil6210_priv *wil, int state);
void wil_link_on(struct wil6210_priv *wil);
void wil_link_off(struct wil6210_priv *wil);
int wil_up(struct wil6210_priv *wil);
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index bd781c7..4311df9 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -299,6 +299,7 @@ static void wmi_evt_fw_ready(struct wil6210_priv *wil, int id, void *d,
{
wil_dbg_wmi(wil, "WMI: got FW ready event\n");

+ wil_set_recovery_state(wil, fw_recovery_idle);
set_bit(wil_status_fwready, &wil->status);
/* let the reset sequence continue */
complete(&wil->wmi_ready);
--
1.9.1


2014-11-14 14:31:57

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 2/2] wil6210: atomic I/O for the card memory

On 11/14/14 14:29, Kalle Valo wrote:
> Vladimir Kondratiev<[email protected]> writes:
>
>> Introduce netdev IOCTLs, to be used by the debug tools.
>>
>> Allows to read/write single dword value or
>> memory block, aligned to dword
>> Different address modes supported:
>> - BAR offset
>> - Firmware "linker" address
>> - target's AHB bus
>>
>> Signed-off-by: Vladimir Kondratiev<[email protected]>
>
> An ioctl interface for a wireless driver? IMHO that would have been ok
> in 2004, but not in 2014. Isn't there really better way to implement
> this?

If this is really driver/device specific debug tool stuff, you should
probably consider the vendor commands through nl80211 API.

Regards,
Arend

>> +/* Numbers SIOCDEVPRIVATE and SIOCDEVPRIVATE + 1
>> + * are used by Android devices to implement PNO (preferred network offload).
>> + * Albeit it is temporary solution, use different numbers to avoid conflicts
>> + */
>
> Comments like this make me even more worried that that this is just yet
> another way to implement wext iwpriv interface.
>


2014-11-16 08:55:27

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: [PATCH 2/2] wil6210: atomic I/O for the card memory

On Friday, November 14, 2014 03:29:52 PM Kalle Valo wrote:
> Vladimir Kondratiev <[email protected]> writes:
>
> > Introduce netdev IOCTLs, to be used by the debug tools.
> >
> > Allows to read/write single dword value or
> > memory block, aligned to dword
> > Different address modes supported:
> > - BAR offset
> > - Firmware "linker" address
> > - target's AHB bus
> >
> > Signed-off-by: Vladimir Kondratiev <[email protected]>
>
> An ioctl interface for a wireless driver? IMHO that would have been ok
> in 2004, but not in 2014. Isn't there really better way to implement
> this?

Functionality implemented through ioctl is hardware access used for
debug/monitoring purposes. I did not found any alternative way to do this.

Any ideas? I need to implement read/write that takes address,
value (for write) and few options like address interpretation flavor
(hardware has several types of addresses)

>
> > +/* Numbers SIOCDEVPRIVATE and SIOCDEVPRIVATE + 1
> > + * are used by Android devices to implement PNO (preferred network offload).
> > + * Albeit it is temporary solution, use different numbers to avoid conflicts
> > + */
>
> Comments like this make me even more worried that that this is just yet
> another way to implement wext iwpriv interface.
It just happens wifi drivers used for android, already use these 2 ioctls
for PNO. And, this (PNO) is clearly something that should be done
with nl80211 instead of ioctl.


2014-11-14 13:29:59

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wil6210: atomic I/O for the card memory

Vladimir Kondratiev <[email protected]> writes:

> Introduce netdev IOCTLs, to be used by the debug tools.
>
> Allows to read/write single dword value or
> memory block, aligned to dword
> Different address modes supported:
> - BAR offset
> - Firmware "linker" address
> - target's AHB bus
>
> Signed-off-by: Vladimir Kondratiev <[email protected]>

An ioctl interface for a wireless driver? IMHO that would have been ok
in 2004, but not in 2014. Isn't there really better way to implement
this?

> +/* Numbers SIOCDEVPRIVATE and SIOCDEVPRIVATE + 1
> + * are used by Android devices to implement PNO (preferred network offload).
> + * Albeit it is temporary solution, use different numbers to avoid conflicts
> + */

Comments like this make me even more worried that that this is just yet
another way to implement wext iwpriv interface.

--
Kalle Valo

2014-11-14 14:54:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] wil6210: atomic I/O for the card memory

Arend van Spriel <[email protected]> writes:

> On 11/14/14 14:29, Kalle Valo wrote:
>> Vladimir Kondratiev<[email protected]> writes:
>>
>>> Introduce netdev IOCTLs, to be used by the debug tools.
>>>
>>> Allows to read/write single dword value or
>>> memory block, aligned to dword
>>> Different address modes supported:
>>> - BAR offset
>>> - Firmware "linker" address
>>> - target's AHB bus
>>>
>>> Signed-off-by: Vladimir Kondratiev<[email protected]>
>>
>> An ioctl interface for a wireless driver? IMHO that would have been ok
>> in 2004, but not in 2014. Isn't there really better way to implement
>> this?
>
> If this is really driver/device specific debug tool stuff, you should
> probably consider the vendor commands through nl80211 API.

Yeah, that's one way to do it. In ath6kl we used debugfs for this, but
no idea which one is better.

--
Kalle Valo