LinuxLists.cc - [PATCH 0/9] wil6210 fixes

2014-12-01 13:33:42

Subject: [PATCH 0/9] wil6210 fixes

Assorted fixes

Vladimir Kondratiev (9):
wil6210: propagate disconnect reason
wil6210: add handling of RX HTRSH interrupt
wil6210: fix recovery after scan timeout
wil6210: remove wil_to_pcie_dev()
wil6210: configurable vring sizes
wil6210: fix warning in pointer arithmetic
wil6210: Rate limit "ring full" error message
wil6210: reset flow update
wil6210: remove TODO wrt buffer alignment

drivers/net/wireless/ath/wil6210/cfg80211.c | 2 +-
drivers/net/wireless/ath/wil6210/debug.c | 17 ++++++++
drivers/net/wireless/ath/wil6210/fw.c | 1 -
drivers/net/wireless/ath/wil6210/fw_inc.c | 4 +-
drivers/net/wireless/ath/wil6210/interrupt.c | 30 +++++++++++--
drivers/net/wireless/ath/wil6210/main.c | 63 +++++++++++++++++++++-------
drivers/net/wireless/ath/wil6210/txrx.c | 11 +++--
drivers/net/wireless/ath/wil6210/wil6210.h | 15 ++++---
drivers/net/wireless/ath/wil6210/wmi.c | 8 ++--
9 files changed, 114 insertions(+), 37 deletions(-)

--
2.1.0

2014-12-02 09:47:05

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 01/10] wil6210: propagate disconnect reason

Propagate reason for the disconnect through the relevant call chains:
- report to cfg80211 reason as reported by the firmware
- provide to the firmware reason as requested by cfg80211

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 2 +-
drivers/net/wireless/ath/wil6210/main.c | 25 ++++++++++++-------------
drivers/net/wireless/ath/wil6210/wil6210.h | 2 +-
drivers/net/wireless/ath/wil6210/wmi.c | 8 ++++----
4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 0fc0b9f..38332a6 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -798,7 +798,7 @@ static int wil_cfg80211_del_station(struct wiphy *wiphy,
struct wil6210_priv *wil = wiphy_to_wil(wiphy);

mutex_lock(&wil->mutex);
- wil6210_disconnect(wil, params->mac, false);
+ wil6210_disconnect(wil, params->mac, params->reason_code, false);
mutex_unlock(&wil->mutex);

return 0;
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 6212983..151ff6b 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -104,7 +104,7 @@ void wil_memcpy_toio_32(volatile void __iomem *dst, const void *src,
}

static void wil_disconnect_cid(struct wil6210_priv *wil, int cid,
- bool from_event)
+ u16 reason_code, bool from_event)
{
uint i;
struct net_device *ndev = wil_to_ndev(wil);
@@ -117,8 +117,7 @@ static void wil_disconnect_cid(struct wil6210_priv *wil, int cid,
sta->data_port_open = false;
if (sta->status != wil_sta_unused) {
if (!from_event)
- wmi_disconnect_sta(wil, sta->addr,
- WLAN_REASON_DEAUTH_LEAVING);
+ wmi_disconnect_sta(wil, sta->addr, reason_code);

switch (wdev->iftype) {
case NL80211_IFTYPE_AP:
@@ -152,7 +151,7 @@ static void wil_disconnect_cid(struct wil6210_priv *wil, int cid,
}

static void _wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
- bool from_event)
+ u16 reason_code, bool from_event)
{
int cid = -ENOENT;
struct net_device *ndev = wil_to_ndev(wil);
@@ -167,10 +166,10 @@ static void _wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
}

if (cid >= 0) /* disconnect 1 peer */
- wil_disconnect_cid(wil, cid, from_event);
+ wil_disconnect_cid(wil, cid, reason_code, from_event);
else /* disconnect all */
for (cid = 0; cid < WIL6210_MAX_CID; cid++)
- wil_disconnect_cid(wil, cid, from_event);
+ wil_disconnect_cid(wil, cid, reason_code, from_event);

/* link state */
switch (wdev->iftype) {
@@ -179,8 +178,7 @@ static void _wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
wil_link_off(wil);
if (test_bit(wil_status_fwconnected, &wil->status)) {
clear_bit(wil_status_fwconnected, &wil->status);
- cfg80211_disconnected(ndev,
- WLAN_STATUS_UNSPECIFIED_FAILURE,
+ cfg80211_disconnected(ndev, reason_code,
NULL, 0, GFP_KERNEL);
} else if (test_bit(wil_status_fwconnecting, &wil->status)) {
cfg80211_connect_result(ndev, bssid, NULL, 0, NULL, 0,
@@ -200,7 +198,7 @@ static void wil_disconnect_worker(struct work_struct *work)
struct wil6210_priv, disconnect_worker);

mutex_lock(&wil->mutex);
- _wil6210_disconnect(wil, NULL, false);
+ _wil6210_disconnect(wil, NULL, WLAN_REASON_UNSPECIFIED, false);
mutex_unlock(&wil->mutex);
}

@@ -392,18 +390,19 @@ int wil_priv_init(struct wil6210_priv *wil)
* wil6210_disconnect - disconnect one connection
* @wil: driver context
* @bssid: peer to disconnect, NULL to disconnect all
+ * @reason_code: Reason code for the Disassociation frame
* @from_event: whether is invoked from FW event handler
*
* Disconnect and release associated resources. If invoked not from the
* FW event handler, issue WMI command(s) to trigger MAC disconnect.
*/
void wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
- bool from_event)
+ u16 reason_code, bool from_event)
{
wil_dbg_misc(wil, "%s()\n", __func__);

del_timer_sync(&wil->connect_timer);
- _wil6210_disconnect(wil, bssid, from_event);
+ _wil6210_disconnect(wil, bssid, reason_code, from_event);
}

void wil_priv_deinit(struct wil6210_priv *wil)
@@ -415,7 +414,7 @@ void wil_priv_deinit(struct wil6210_priv *wil)
cancel_work_sync(&wil->disconnect_worker);
cancel_work_sync(&wil->fw_error_worker);
mutex_lock(&wil->mutex);
- wil6210_disconnect(wil, NULL, false);
+ wil6210_disconnect(wil, NULL, WLAN_REASON_DEAUTH_LEAVING, false);
mutex_unlock(&wil->mutex);
wmi_event_flush(wil);
destroy_workqueue(wil->wmi_wq_conn);
@@ -600,7 +599,7 @@ int wil_reset(struct wil6210_priv *wil)
WARN_ON(test_bit(wil_status_napi_en, &wil->status));

cancel_work_sync(&wil->disconnect_worker);
- wil6210_disconnect(wil, NULL, false);
+ wil6210_disconnect(wil, NULL, WLAN_REASON_DEAUTH_LEAVING, false);

wil->status = 0; /* prevent NAPI from being scheduled */

diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 95d3a06..18cff53 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -586,7 +586,7 @@ int wmi_set_mac_address(struct wil6210_priv *wil, void *addr);
int wmi_pcp_start(struct wil6210_priv *wil, int bi, u8 wmi_nettype, u8 chan);
int wmi_pcp_stop(struct wil6210_priv *wil);
void wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
- bool from_event);
+ u16 reason_code, bool from_event);

int wil_rx_init(struct wil6210_priv *wil);
void wil_rx_fini(struct wil6210_priv *wil);
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index bb1e066..63476c8 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -478,15 +478,15 @@ static void wmi_evt_disconnect(struct wil6210_priv *wil, int id,
void *d, int len)
{
struct wmi_disconnect_event *evt = d;
+ u16 reason_code = le16_to_cpu(evt->protocol_reason_status);

- wil_dbg_wmi(wil, "Disconnect %pM reason %d proto %d wmi\n",
- evt->bssid,
- evt->protocol_reason_status, evt->disconnect_reason);
+ wil_dbg_wmi(wil, "Disconnect %pM reason [proto %d wmi %d]\n",
+ evt->bssid, reason_code, evt->disconnect_reason);

wil->sinfo_gen++;

mutex_lock(&wil->mutex);
- wil6210_disconnect(wil, evt->bssid, true);
+ wil6210_disconnect(wil, evt->bssid, reason_code, true);
mutex_unlock(&wil->mutex);
}

--
2.1.0

2014-12-02 09:47:00

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 00/10] wil6210 fixes, reroll

Assorted fixes

V2: split single commit into 2, accordingly to input by
Joe Perches.

re-sending whole series to avoid mis-interpretation.

Vladimir Kondratiev (10):
wil6210: propagate disconnect reason
wil6210: introduce wil_err_ratelimited()
wil6210: add handling of RX HTRSH interrupt
wil6210: fix recovery after scan timeout
wil6210: remove wil_to_pcie_dev()
wil6210: configurable vring sizes
wil6210: fix warning in pointer arithmetic
wil6210: Rate limit "ring full" error message
wil6210: reset flow update
wil6210: remove TODO wrt buffer alignment

drivers/net/wireless/ath/wil6210/cfg80211.c | 2 +-
drivers/net/wireless/ath/wil6210/debug.c | 17 ++++++++
drivers/net/wireless/ath/wil6210/fw.c | 1 -
drivers/net/wireless/ath/wil6210/fw_inc.c | 4 +-
drivers/net/wireless/ath/wil6210/interrupt.c | 30 +++++++++++--
drivers/net/wireless/ath/wil6210/main.c | 63 +++++++++++++++++++++-------
drivers/net/wireless/ath/wil6210/txrx.c | 11 +++--
drivers/net/wireless/ath/wil6210/wil6210.h | 15 ++++---
drivers/net/wireless/ath/wil6210/wmi.c | 8 ++--
9 files changed, 114 insertions(+), 37 deletions(-)

--
2.1.0

2014-12-02 09:47:27

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 04/10] wil6210: fix recovery after scan timeout

Scan timeout treated as indication for firmware error;
and should be handled in the same way.

Recovery state machine does not perform as designed because
its state is not updated in case of scan timeout.

Fix is to set recovery state machine into the proper state.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/main.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 151ff6b..3bef153 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -220,6 +220,7 @@ static void wil_scan_timer_fn(ulong x)

clear_bit(wil_status_fwready, &wil->status);
wil_err(wil, "Scan timeout detected, start fw error recovery\n");
+ wil->recovery_state = fw_recovery_pending;
schedule_work(&wil->fw_error_worker);
}

--
2.1.0

2014-12-02 09:52:19

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 08/10] wil6210: Rate limit "ring full" error message

In the wil_tx_ring, error message printed when tx attempted
while vring has no space to accommodate all fragments of frame.
Normally, such situation handled by stopping tx queue.
But, if tx queue is by-passed (like pktgen does), this error
will be triggered at high rate and dmesg will be flooded with
this message. Whole system may become unstable and hang with
no possible recover except power cycle.

Rate-limit it to prevent dmesg flooding.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/txrx.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index c3a5489..405ede6 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -928,8 +928,9 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
wil_dbg_txrx(wil, "%s()\n", __func__);

if (avail < 1 + nr_frags) {
- wil_err(wil, "Tx ring full. No space for %d fragments\n",
- 1 + nr_frags);
+ wil_err_ratelimited(wil,
+ "Tx ring full. No space for %d fragments\n",
+ 1 + nr_frags);
return -ENOMEM;
}
_d = &vring->va[i].tx;
--
2.1.0

2014-12-02 09:51:15

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 06/10] wil6210: configurable vring sizes

Allow to configure VRING size for both Rx and Tx via module parameters:
rx_ring_order and tx_ring_order. Parameters are ring size orders, i.e.
ring size calculated as 1 << order.
Defaults for both Tx and Rx are order 9, i.e. size 512

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/main.c | 34 ++++++++++++++++++++++++++++--
drivers/net/wireless/ath/wil6210/txrx.c | 4 ++--
drivers/net/wireless/ath/wil6210/wil6210.h | 9 +++++---
3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 3bef153..8f257a4 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -67,6 +67,36 @@ static struct kernel_param_ops mtu_max_ops = {
module_param_cb(mtu_max, &mtu_max_ops, &mtu_max, S_IRUGO);
MODULE_PARM_DESC(mtu_max, " Max MTU value.");

+static uint rx_ring_order = WIL_RX_RING_SIZE_ORDER_DEFAULT;
+static uint tx_ring_order = WIL_TX_RING_SIZE_ORDER_DEFAULT;
+
+static int ring_order_set(const char *val, const struct kernel_param *kp)
+{
+ int ret;
+ uint x;
+
+ ret = kstrtouint(val, 0, &x);
+ if (ret)
+ return ret;
+
+ if ((x < WIL_RING_SIZE_ORDER_MIN) || (x > WIL_RING_SIZE_ORDER_MAX))
+ return -EINVAL;
+
+ *((uint *)kp->arg) = x;
+
+ return 0;
+}
+
+static struct kernel_param_ops ring_order_ops = {
+ .set = ring_order_set,
+ .get = param_get_uint,
+};
+
+module_param_cb(rx_ring_order, &ring_order_ops, &rx_ring_order, S_IRUGO);
+MODULE_PARM_DESC(rx_ring_order, " Rx ring order; size = 1 << order");
+module_param_cb(tx_ring_order, &ring_order_ops, &tx_ring_order, S_IRUGO);
+MODULE_PARM_DESC(tx_ring_order, " Tx ring order; size = 1 << order");
+
#define RST_DELAY (20) /* msec, for loop in @wil_target_reset */
#define RST_COUNT (1 + 1000/RST_DELAY) /* round up to be above 1 sec total */

@@ -332,7 +362,7 @@ static void wil_connect_worker(struct work_struct *work)

wil_dbg_wmi(wil, "Configure for connection CID %d\n", cid);

- rc = wil_vring_init_tx(wil, ringid, WIL6210_TX_RING_SIZE, cid, 0);
+ rc = wil_vring_init_tx(wil, ringid, 1 << tx_ring_order, cid, 0);
wil->pending_connect_cid = -1;
if (rc == 0) {
wil->sta[cid].status = wil_sta_connected;
@@ -705,7 +735,7 @@ int __wil_up(struct wil6210_priv *wil)
return rc;

/* Rx VRING. After MAC and beacon */
- rc = wil_rx_init(wil);
+ rc = wil_rx_init(wil, 1 << rx_ring_order);
if (rc)
return rc;

diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index c680906..c3a5489 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -596,7 +596,7 @@ void wil_rx_handle(struct wil6210_priv *wil, int *quota)
wil_rx_refill(wil, v->size);
}

-int wil_rx_init(struct wil6210_priv *wil)
+int wil_rx_init(struct wil6210_priv *wil, u16 size)
{
struct vring *vring = &wil->vring_rx;
int rc;
@@ -608,7 +608,7 @@ int wil_rx_init(struct wil6210_priv *wil)
return -EINVAL;
}

- vring->size = WIL6210_RX_RING_SIZE;
+ vring->size = size;
rc = wil_vring_alloc(wil, vring);
if (rc)
return rc;
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 8c09700..c6ec5b9 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -49,8 +49,11 @@ static inline u32 WIL_GET_BITS(u32 x, int b0, int b1)

#define WIL6210_MEM_SIZE (2*1024*1024UL)

-#define WIL6210_RX_RING_SIZE (128)
-#define WIL6210_TX_RING_SIZE (512)
+#define WIL_RX_RING_SIZE_ORDER_DEFAULT (9)
+#define WIL_TX_RING_SIZE_ORDER_DEFAULT (9)
+/* limit ring size in range [32..32k] */
+#define WIL_RING_SIZE_ORDER_MIN (5)
+#define WIL_RING_SIZE_ORDER_MAX (15)
#define WIL6210_MAX_TX_RINGS (24) /* HW limit */
#define WIL6210_MAX_CID (8) /* HW limit */
#define WIL6210_NAPI_BUDGET (16) /* arbitrary */
@@ -590,7 +593,7 @@ int wmi_pcp_stop(struct wil6210_priv *wil);
void wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
u16 reason_code, bool from_event);

-int wil_rx_init(struct wil6210_priv *wil);
+int wil_rx_init(struct wil6210_priv *wil, u16 size);
void wil_rx_fini(struct wil6210_priv *wil);

/* TX API */
--
2.1.0

2014-12-02 09:50:44

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 05/10] wil6210: remove wil_to_pcie_dev()

There is no need to obtain physical device through
wil->pdev->dev path, as it is done by this macro.
The same device already stored as wiphy's device, thus
wil_to_dev() returns the same device as wil_to_pcie_dev()

Remove unnecessary macros, this allows to drop dependency
by pci.h in the firmware download code.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/fw.c | 1 -
drivers/net/wireless/ath/wil6210/fw_inc.c | 2 +-
drivers/net/wireless/ath/wil6210/wil6210.h | 1 -
3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/fw.c b/drivers/net/wireless/ath/wil6210/fw.c
index 8c6f3b0..93c5cc1 100644
--- a/drivers/net/wireless/ath/wil6210/fw.c
+++ b/drivers/net/wireless/ath/wil6210/fw.c
@@ -15,7 +15,6 @@
*/
#include <linux/firmware.h>
#include <linux/module.h>
-#include <linux/pci.h>
#include <linux/crc32.h>
#include "wil6210.h"
#include "fw.h"
diff --git a/drivers/net/wireless/ath/wil6210/fw_inc.c b/drivers/net/wireless/ath/wil6210/fw_inc.c
index 44cb71f..2658455 100644
--- a/drivers/net/wireless/ath/wil6210/fw_inc.c
+++ b/drivers/net/wireless/ath/wil6210/fw_inc.c
@@ -471,7 +471,7 @@ int wil_request_firmware(struct wil6210_priv *wil, const char *name)
size_t sz;
const void *d;

- rc = request_firmware(&fw, name, wil_to_pcie_dev(wil));
+ rc = request_firmware(&fw, name, wil_to_dev(wil));
if (rc) {
wil_err_fw(wil, "Failed to load firmware %s\n", name);
return rc;
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index f2f89e0..8c09700 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -469,7 +469,6 @@ struct wil6210_priv {
#define wdev_to_wil(w) (struct wil6210_priv *)(wdev_priv(w))
#define wil_to_ndev(i) (wil_to_wdev(i)->netdev)
#define ndev_to_wil(n) (wdev_to_wil(n->ieee80211_ptr))
-#define wil_to_pcie_dev(i) (&i->pdev->dev)

__printf(2, 3)
void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...);
--
2.1.0

2014-12-01 13:33:53

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH 3/9] wil6210: fix recovery after scan timeout

2014-12-02 09:52:55

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 09/10] wil6210: reset flow update

If card reset with firmware download executed, followed by reset
with use of firmware from build in flash, firmware download indication
remains in the hardware register.
When running firmware download flow,
the SW download indication is written by the driver to bit 0 in usage_6:
wil_fw_load(), "S(RGF_USER_USAGE_6, 1);"
This register, like all USER RGF, wasn't reset in SW reset flow.
Therefore the driver must clear it on SW reset flow.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/main.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 8f257a4..8ff3fe3 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -493,6 +493,9 @@ static int wil_target_reset(struct wil6210_priv *wil)

wil_halt_cpu(wil);

+ /* Clear Fw Download notification */
+ C(RGF_USER_USAGE_6, BIT(0));
+
if (is_sparrow) {
S(RGF_CAF_OSC_CONTROL, BIT_CAF_OSC_XTAL_EN);
/* XTAL stabilization should take about 3ms */
--
2.1.0

2014-12-02 15:07:51

by Joe Perches

[permalink] [raw]

Subject: Re: [PATCH v2 02/10] wil6210: introduce wil_err_ratelimited()

On Tue, 2014-12-02 at 11:46 +0200, Vladimir Kondratiev wrote:
> Add rate-limited flavor of wil_err(); in some cases,
> specially in IRQ and data path, if something went wrong,
> error message may be printed at very high rate, rendering
> whole system unresponsive or unstable.
> Need to do real function and not a macro to have arguments
> always evaluated.

net_<foo>_ratelimited uses the same form.

include/linux/net.h:#define net_ratelimited_function(function, ...)
include/linux/net.h-do {
include/linux/net.h- if (net_ratelimit())
include/linux/net.h- function(__VA_ARGS__);
include/linux/net.h-} while (0)

include/linux/net.h-#define net_err_ratelimited(fmt, ...)
include/linux/net.h: net_ratelimited_function(pr_err, fmt, ##__VA_ARGS__)

2014-12-02 09:53:41

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 10/10] wil6210: remove TODO wrt buffer alignment

Hardware doesn't place any restrictions on the buffer alignment,
consider this TODO resolved.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/txrx.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index 405ede6..e3f8bdc 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -210,8 +210,6 @@ static int wil_vring_alloc_skb(struct wil6210_priv *wil, struct vring *vring,
struct vring_rx_desc dd, *d = &dd;
volatile struct vring_rx_desc *_d = &vring->va[i].rx;
dma_addr_t pa;
-
- /* TODO align */
struct sk_buff *skb = dev_alloc_skb(sz + headroom);

if (unlikely(!skb))
--
2.1.0

2014-12-01 13:33:55

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH 4/9] wil6210: remove wil_to_pcie_dev()

2014-12-02 16:07:51

by Vladimir Kondratiev

[permalink] [raw]

Subject: Re: [PATCH v2 02/10] wil6210: introduce wil_err_ratelimited()

On Tuesday, December 02, 2014 07:07:47 AM Joe Perches wrote:
> net_<foo>_ratelimited uses the same form.
>
> include/linux/net.h:#define net_ratelimited_function(function, ...)
> include/linux/net.h-do {
> include/linux/net.h- if (net_ratelimit())
> include/linux/net.h- function(__VA_ARGS__);
> include/linux/net.h-} while (0)
>
Yes, I see. There are representatives of both "schools". As counter example,
netdev_<foo> uses "long" way:

#define define_netdev_printk_level(func, level) \
void func(const struct net_device *dev, const char *fmt, ...) \
{ \
struct va_format vaf; \
va_list args; \
\
va_start(args, fmt); \
\
vaf.fmt = fmt; \
vaf.va = &args; \
\
__netdev_printk(level, dev, &vaf); \
\
va_end(args); \
} \
EXPORT_SYMBOL(func);

define_netdev_printk_level(netdev_emerg, KERN_EMERG);

Problem with "short" way, it is hard to detect usage with side effect.
Example from netfilter:
if (dst_mtu(skb_dst(skb)) <= minlen) {
net_err_ratelimited("unknown or invalid path-MTU (%u)\n",
dst_mtu(skb_dst(skb)));

Now, one need to remember that dst_mtu() can't have side effects.
I agree it is bad practice to have non-trivial arguments in printk-like functions,
but it is hard to detect, specifically if one just automatically replace function with its
"ratelimited" version.

2014-12-02 03:44:13

by Joe Perches

[permalink] [raw]

Subject: Re: [PATCH 2/9] wil6210: add handling of RX HTRSH interrupt

On Mon, 2014-12-01 at 15:33 +0200, Vladimir Kondratiev wrote:
> In addition there's a rate limitted warning message.

limited.

I think this _ratelimited bit should be a separate patch.
(and a suggestion / comment below too)

> diff --git a/drivers/net/wireless/ath/wil6210/debug.c b/drivers/net/wireless/ath/wil6210/debug.c
[]
> +void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
> +{
> + if (net_ratelimit()) {
> + struct net_device *ndev = wil_to_ndev(wil);
> + struct va_format vaf = {
> + .fmt = fmt,
> + };
> + va_list args;
> +
> + va_start(args, fmt);
> + vaf.va = &args;
> + netdev_err(ndev, "%pV", &vaf);
> + trace_wil6210_log_err(&vaf);
> + va_end(args);
> + }
> +}
[]
> diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
[]
> @@ -475,6 +476,8 @@ void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...);
> __printf(2, 3)
> void wil_err(struct wil6210_priv *wil, const char *fmt, ...);
> __printf(2, 3)
> +void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...);
> +__printf(2, 3)
> void wil_info(struct wil6210_priv *wil, const char *fmt, ...);
> #define wil_dbg(wil, fmt, arg...) do { \
> netdev_dbg(wil_to_ndev(wil), fmt, ##arg); \

I think it'd be simpler and smaller to use a macro:

#define wil_err_ratelimited(wil, fmt, ...) \
do { \
if (net_ratelimit()) \
wil_err(wil, fmt, ##__VA_ARGS__); \
} while (0)

2014-12-01 13:33:48

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH 1/9] wil6210: propagate disconnect reason

2014-12-02 09:47:21

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 03/10] wil6210: add handling of RX HTRSH interrupt

RX_HTRSH interrupt indicates Rx ring threshold of not-filled buffers
reached. Rx ring is about to overflow. It is handled in exactly
the same manner as RX_DONE interrupt - fetching accumulated
packets from RX. In addition there's a rate limited error message.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/interrupt.c | 30 +++++++++++++++++++++++++---
drivers/net/wireless/ath/wil6210/wil6210.h | 1 +
2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/interrupt.c b/drivers/net/wireless/ath/wil6210/interrupt.c
index 90f416f..4bcbd62 100644
--- a/drivers/net/wireless/ath/wil6210/interrupt.c
+++ b/drivers/net/wireless/ath/wil6210/interrupt.c
@@ -36,7 +36,8 @@
*/

#define WIL6210_IRQ_DISABLE (0xFFFFFFFFUL)
-#define WIL6210_IMC_RX BIT_DMA_EP_RX_ICR_RX_DONE
+#define WIL6210_IMC_RX (BIT_DMA_EP_RX_ICR_RX_DONE | \
+ BIT_DMA_EP_RX_ICR_RX_HTRSH)
#define WIL6210_IMC_TX (BIT_DMA_EP_TX_ICR_TX_DONE | \
BIT_DMA_EP_TX_ICR_TX_DONE_N(0))
#define WIL6210_IMC_MISC (ISR_MISC_FW_READY | \
@@ -171,6 +172,7 @@ static irqreturn_t wil6210_irq_rx(int irq, void *cookie)
u32 isr = wil_ioread32_and_clear(wil->csr +
HOSTADDR(RGF_DMA_EP_RX_ICR) +
offsetof(struct RGF_ICR, ICR));
+ bool need_unmask = true;

trace_wil6210_irq_rx(isr);
wil_dbg_irq(wil, "ISR RX 0x%08x\n", isr);
@@ -182,12 +184,24 @@ static irqreturn_t wil6210_irq_rx(int irq, void *cookie)

wil6210_mask_irq_rx(wil);

- if (isr & BIT_DMA_EP_RX_ICR_RX_DONE) {
+ /* RX_DONE and RX_HTRSH interrupts are the same if interrupt
+ * moderation is not used. Interrupt moderation may cause RX
+ * buffer overflow while RX_DONE is delayed. The required
+ * action is always the same - should empty the accumulated
+ * packets from the RX ring.
+ */
+ if (isr & (BIT_DMA_EP_RX_ICR_RX_DONE | BIT_DMA_EP_RX_ICR_RX_HTRSH)) {
wil_dbg_irq(wil, "RX done\n");
- isr &= ~BIT_DMA_EP_RX_ICR_RX_DONE;
+
+ if (isr & BIT_DMA_EP_RX_ICR_RX_HTRSH)
+ wil_err_ratelimited(wil, "Received \"Rx buffer is in risk "
+ "of overflow\" interrupt\n");
+
+ isr &= ~(BIT_DMA_EP_RX_ICR_RX_DONE | BIT_DMA_EP_RX_ICR_RX_HTRSH);
if (test_bit(wil_status_reset_done, &wil->status)) {
if (test_bit(wil_status_napi_en, &wil->status)) {
wil_dbg_txrx(wil, "NAPI(Rx) schedule\n");
+ need_unmask = false;
napi_schedule(&wil->napi_rx);
} else {
wil_err(wil, "Got Rx interrupt while "
@@ -204,6 +218,10 @@ static irqreturn_t wil6210_irq_rx(int irq, void *cookie)
/* Rx IRQ will be enabled when NAPI processing finished */

atomic_inc(&wil->isr_count_rx);
+
+ if (unlikely(need_unmask))
+ wil6210_unmask_irq_rx(wil);
+
return IRQ_HANDLED;
}

@@ -213,6 +231,7 @@ static irqreturn_t wil6210_irq_tx(int irq, void *cookie)
u32 isr = wil_ioread32_and_clear(wil->csr +
HOSTADDR(RGF_DMA_EP_TX_ICR) +
offsetof(struct RGF_ICR, ICR));
+ bool need_unmask = true;

trace_wil6210_irq_tx(isr);
wil_dbg_irq(wil, "ISR TX 0x%08x\n", isr);
@@ -231,6 +250,7 @@ static irqreturn_t wil6210_irq_tx(int irq, void *cookie)
isr &= ~(BIT(25) - 1UL);
if (test_bit(wil_status_reset_done, &wil->status)) {
wil_dbg_txrx(wil, "NAPI(Tx) schedule\n");
+ need_unmask = false;
napi_schedule(&wil->napi_tx);
} else {
wil_err(wil, "Got Tx interrupt while in reset\n");
@@ -243,6 +263,10 @@ static irqreturn_t wil6210_irq_tx(int irq, void *cookie)
/* Tx IRQ will be enabled when NAPI processing finished */

atomic_inc(&wil->isr_count_tx);
+
+ if (unlikely(need_unmask))
+ wil6210_unmask_irq_tx(wil);
+
return IRQ_HANDLED;
}

diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 450b688..f2f89e0 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -126,6 +126,7 @@ struct RGF_ICR {
#define BIT_DMA_EP_TX_ICR_TX_DONE_N(n) BIT(n+1) /* n = [0..23] */
#define RGF_DMA_EP_RX_ICR (0x881bd0) /* struct RGF_ICR */
#define BIT_DMA_EP_RX_ICR_RX_DONE BIT(0)
+ #define BIT_DMA_EP_RX_ICR_RX_HTRSH BIT(1)
#define RGF_DMA_EP_MISC_ICR (0x881bec) /* struct RGF_ICR */
#define BIT_DMA_EP_MISC_ICR_RX_HTRSH BIT(0)
#define BIT_DMA_EP_MISC_ICR_TX_NO_ACT BIT(1)
--
2.1.0

2014-12-02 16:20:07

by Joe Perches

[permalink] [raw]

Subject: Re: [PATCH v2 02/10] wil6210: introduce wil_err_ratelimited()

On Tue, 2014-12-02 at 18:07 +0200, Vladimir Kondratiev wrote:
> On Tuesday, December 02, 2014 07:07:47 AM Joe Perches wrote:
> > net_<foo>_ratelimited uses the same form.
> >
> > include/linux/net.h:#define net_ratelimited_function(function, ...)
> > include/linux/net.h-do {
> > include/linux/net.h- if (net_ratelimit())
> > include/linux/net.h- function(__VA_ARGS__);
> > include/linux/net.h-} while (0)
> >
> Yes, I see. There are representatives of both "schools". As counter example,
> netdev_<foo> uses "long" way:

These used to be macros.
They were converted to reduce overall code "size".

commit 256df2f3879efdb2e9808bdb1b54b16fbb11fa38
Date: Sun Jun 27 01:02:35 2010 +0000

netdevice.h net/core/dev.c: Convert netdev_<level> logging macros to functions

Reduces an x86 defconfig text and data ~2k.
text is smaller, data is larger.

$ size vmlinux*
text data bss dec hex filename
7198862 720112 1366288 9285262 8dae8e vmlinux
7205273 716016 1366288 9287577 8db799 vmlinux.device_h

Uses %pV and struct va_format
Format arguments are verified before printk

> I agree it is bad practice to have non-trivial arguments in printk-like functions,

true.

It limits the ability to make side-effect
free smaller code when printk is a no-op.

> but it is hard to detect, specifically if one just automatically replace function with its
> "ratelimited" version.

Also true.

It is and remains just a suggestion.

Your code, your choices...

2014-12-02 09:51:48

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 07/10] wil6210: fix warning in pointer arithmetic

In some compilation environments, result of pointer arithmetic interpreted as int
while in others it is long int. Force conversion to long.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/fw_inc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wil6210/fw_inc.c b/drivers/net/wireless/ath/wil6210/fw_inc.c
index 2658455..d4acf93 100644
--- a/drivers/net/wireless/ath/wil6210/fw_inc.c
+++ b/drivers/net/wireless/ath/wil6210/fw_inc.c
@@ -446,7 +446,7 @@ static int wil_fw_load(struct wil6210_priv *wil, const void *data, size_t size)
if (size >= sizeof(*hdr)) {
wil_err_fw(wil, "Stop at offset %ld"
" record type %d [%zd bytes]\n",
- (const void *)hdr - data,
+ (long)((const void *)hdr - data),
le16_to_cpu(hdr->type), hdr_sz);
}
return -EINVAL;
--
2.1.0

2014-12-02 10:55:13

by Vladimir Kondratiev

[permalink] [raw]

Subject: Re: [PATCH 2/9] wil6210: add handling of RX HTRSH interrupt

On Monday, December 01, 2014 07:44:10 PM Joe Perches wrote:
> > In addition there's a rate limitted warning message.
>
> limited.
Yes, thanks.

> I think this _ratelimited bit should be a separate patch.
> (and a suggestion / comment below too)
Agree, sending v2

> #define wil_err_ratelimited(wil, fmt, ...) \
> do { \
> if (net_ratelimit()) \
> wil_err(wil, fmt, ##__VA_ARGS__); \
> } while (0)
Oh no, this way arguments won't be evaluated for suppressed messages.
The way it is done is less compact but do predictable job with arguments.

Thanks, Vladimir

2014-12-02 20:00:13

by John W. Linville

[permalink] [raw]

Subject: Re: [PATCH v2 00/10] wil6210 fixes, reroll

I already merged the earlier version. Feel free to send any changes
on top, ASAP!

On Tue, Dec 02, 2014 at 11:46:28AM +0200, Vladimir Kondratiev wrote:
> Assorted fixes
>
> V2: split single commit into 2, accordingly to input by
> Joe Perches.
>
> re-sending whole series to avoid mis-interpretation.
>
> Vladimir Kondratiev (10):
> wil6210: propagate disconnect reason
> wil6210: introduce wil_err_ratelimited()
> wil6210: add handling of RX HTRSH interrupt
> wil6210: fix recovery after scan timeout
> wil6210: remove wil_to_pcie_dev()
> wil6210: configurable vring sizes
> wil6210: fix warning in pointer arithmetic
> wil6210: Rate limit "ring full" error message
> wil6210: reset flow update
> wil6210: remove TODO wrt buffer alignment
>
> drivers/net/wireless/ath/wil6210/cfg80211.c | 2 +-
> drivers/net/wireless/ath/wil6210/debug.c | 17 ++++++++
> drivers/net/wireless/ath/wil6210/fw.c | 1 -
> drivers/net/wireless/ath/wil6210/fw_inc.c | 4 +-
> drivers/net/wireless/ath/wil6210/interrupt.c | 30 +++++++++++--
> drivers/net/wireless/ath/wil6210/main.c | 63 +++++++++++++++++++++-------
> drivers/net/wireless/ath/wil6210/txrx.c | 11 +++--
> drivers/net/wireless/ath/wil6210/wil6210.h | 15 ++++---
> drivers/net/wireless/ath/wil6210/wmi.c | 8 ++--
> 9 files changed, 114 insertions(+), 37 deletions(-)
>
> --
> 2.1.0
>
>

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2014-12-01 13:33:47

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH 2/9] wil6210: add handling of RX HTRSH interrupt

RX_HTRSH interrupt is handled in exactly the same manner
as RX_DONE interrupt - fetching accumulated packets from RX
ring. In addition there's a rate limitted warning message.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/debug.c | 17 ++++++++++++++++
drivers/net/wireless/ath/wil6210/interrupt.c | 30 +++++++++++++++++++++++++---
drivers/net/wireless/ath/wil6210/wil6210.h | 3 +++
3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/debug.c b/drivers/net/wireless/ath/wil6210/debug.c
index 8d99021..3249562 100644
--- a/drivers/net/wireless/ath/wil6210/debug.c
+++ b/drivers/net/wireless/ath/wil6210/debug.c
@@ -32,6 +32,23 @@ void wil_err(struct wil6210_priv *wil, const char *fmt, ...)
va_end(args);
}

+void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
+{
+ if (net_ratelimit()) {
+ struct net_device *ndev = wil_to_ndev(wil);
+ struct va_format vaf = {
+ .fmt = fmt,
+ };
+ va_list args;
+
+ va_start(args, fmt);
+ vaf.va = &args;
+ netdev_err(ndev, "%pV", &vaf);
+ trace_wil6210_log_err(&vaf);
+ va_end(args);
+ }
+}
+
void wil_info(struct wil6210_priv *wil, const char *fmt, ...)
{
struct net_device *ndev = wil_to_ndev(wil);
diff --git a/drivers/net/wireless/ath/wil6210/interrupt.c b/drivers/net/wireless/ath/wil6210/interrupt.c
index 90f416f..4bcbd62 100644
--- a/drivers/net/wireless/ath/wil6210/interrupt.c
+++ b/drivers/net/wireless/ath/wil6210/interrupt.c
@@ -36,7 +36,8 @@
*/

#define WIL6210_IRQ_DISABLE (0xFFFFFFFFUL)
-#define WIL6210_IMC_RX BIT_DMA_EP_RX_ICR_RX_DONE
+#define WIL6210_IMC_RX (BIT_DMA_EP_RX_ICR_RX_DONE | \
+ BIT_DMA_EP_RX_ICR_RX_HTRSH)
#define WIL6210_IMC_TX (BIT_DMA_EP_TX_ICR_TX_DONE | \
BIT_DMA_EP_TX_ICR_TX_DONE_N(0))
#define WIL6210_IMC_MISC (ISR_MISC_FW_READY | \
@@ -171,6 +172,7 @@ static irqreturn_t wil6210_irq_rx(int irq, void *cookie)
u32 isr = wil_ioread32_and_clear(wil->csr +
HOSTADDR(RGF_DMA_EP_RX_ICR) +
offsetof(struct RGF_ICR, ICR));
+ bool need_unmask = true;

trace_wil6210_irq_rx(isr);
wil_dbg_irq(wil, "ISR RX 0x%08x\n", isr);
@@ -182,12 +184,24 @@ static irqreturn_t wil6210_irq_rx(int irq, void *cookie)

wil6210_mask_irq_rx(wil);

- if (isr & BIT_DMA_EP_RX_ICR_RX_DONE) {
+ /* RX_DONE and RX_HTRSH interrupts are the same if interrupt
+ * moderation is not used. Interrupt moderation may cause RX
+ * buffer overflow while RX_DONE is delayed. The required
+ * action is always the same - should empty the accumulated
+ * packets from the RX ring.
+ */
+ if (isr & (BIT_DMA_EP_RX_ICR_RX_DONE | BIT_DMA_EP_RX_ICR_RX_HTRSH)) {
wil_dbg_irq(wil, "RX done\n");
- isr &= ~BIT_DMA_EP_RX_ICR_RX_DONE;
+
+ if (isr & BIT_DMA_EP_RX_ICR_RX_HTRSH)
+ wil_err_ratelimited(wil, "Received \"Rx buffer is in risk "
+ "of overflow\" interrupt\n");
+
+ isr &= ~(BIT_DMA_EP_RX_ICR_RX_DONE | BIT_DMA_EP_RX_ICR_RX_HTRSH);
if (test_bit(wil_status_reset_done, &wil->status)) {
if (test_bit(wil_status_napi_en, &wil->status)) {
wil_dbg_txrx(wil, "NAPI(Rx) schedule\n");
+ need_unmask = false;
napi_schedule(&wil->napi_rx);
} else {
wil_err(wil, "Got Rx interrupt while "
@@ -204,6 +218,10 @@ static irqreturn_t wil6210_irq_rx(int irq, void *cookie)
/* Rx IRQ will be enabled when NAPI processing finished */

atomic_inc(&wil->isr_count_rx);
+
+ if (unlikely(need_unmask))
+ wil6210_unmask_irq_rx(wil);
+
return IRQ_HANDLED;
}

@@ -213,6 +231,7 @@ static irqreturn_t wil6210_irq_tx(int irq, void *cookie)
u32 isr = wil_ioread32_and_clear(wil->csr +
HOSTADDR(RGF_DMA_EP_TX_ICR) +
offsetof(struct RGF_ICR, ICR));
+ bool need_unmask = true;

trace_wil6210_irq_tx(isr);
wil_dbg_irq(wil, "ISR TX 0x%08x\n", isr);
@@ -231,6 +250,7 @@ static irqreturn_t wil6210_irq_tx(int irq, void *cookie)
isr &= ~(BIT(25) - 1UL);
if (test_bit(wil_status_reset_done, &wil->status)) {
wil_dbg_txrx(wil, "NAPI(Tx) schedule\n");
+ need_unmask = false;
napi_schedule(&wil->napi_tx);
} else {
wil_err(wil, "Got Tx interrupt while in reset\n");
@@ -243,6 +263,10 @@ static irqreturn_t wil6210_irq_tx(int irq, void *cookie)
/* Tx IRQ will be enabled when NAPI processing finished */

atomic_inc(&wil->isr_count_tx);
+
+ if (unlikely(need_unmask))
+ wil6210_unmask_irq_tx(wil);
+
return IRQ_HANDLED;
}

diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 18cff53..f2f89e0 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -126,6 +126,7 @@ struct RGF_ICR {
#define BIT_DMA_EP_TX_ICR_TX_DONE_N(n) BIT(n+1) /* n = [0..23] */
#define RGF_DMA_EP_RX_ICR (0x881bd0) /* struct RGF_ICR */
#define BIT_DMA_EP_RX_ICR_RX_DONE BIT(0)
+ #define BIT_DMA_EP_RX_ICR_RX_HTRSH BIT(1)
#define RGF_DMA_EP_MISC_ICR (0x881bec) /* struct RGF_ICR */
#define BIT_DMA_EP_MISC_ICR_RX_HTRSH BIT(0)
#define BIT_DMA_EP_MISC_ICR_TX_NO_ACT BIT(1)
@@ -475,6 +476,8 @@ void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...);
__printf(2, 3)
void wil_err(struct wil6210_priv *wil, const char *fmt, ...);
__printf(2, 3)
+void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...);
+__printf(2, 3)
void wil_info(struct wil6210_priv *wil, const char *fmt, ...);
#define wil_dbg(wil, fmt, arg...) do { \
netdev_dbg(wil_to_ndev(wil), fmt, ##arg); \
--
2.1.0

2014-12-02 09:47:13

by Vladimir Kondratiev

[permalink] [raw]

Subject: [PATCH v2 02/10] wil6210: introduce wil_err_ratelimited()

Add rate-limited flavor of wil_err(); in some cases,
specially in IRQ and data path, if something went wrong,
error message may be printed at very high rate, rendering
whole system unresponsive or unstable.
Need to do real function and not a macro to have arguments
always evaluated.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/debug.c | 17 +++++++++++++++++
drivers/net/wireless/ath/wil6210/wil6210.h | 2 ++
2 files changed, 19 insertions(+)

diff --git a/drivers/net/wireless/ath/wil6210/debug.c b/drivers/net/wireless/ath/wil6210/debug.c
index 8d99021..3249562 100644
--- a/drivers/net/wireless/ath/wil6210/debug.c
+++ b/drivers/net/wireless/ath/wil6210/debug.c
@@ -32,6 +32,23 @@ void wil_err(struct wil6210_priv *wil, const char *fmt, ...)
va_end(args);
}

+void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
+{
+ if (net_ratelimit()) {
+ struct net_device *ndev = wil_to_ndev(wil);
+ struct va_format vaf = {
+ .fmt = fmt,
+ };
+ va_list args;
+
+ va_start(args, fmt);
+ vaf.va = &args;
+ netdev_err(ndev, "%pV", &vaf);
+ trace_wil6210_log_err(&vaf);
+ va_end(args);
+ }
+}
+
void wil_info(struct wil6210_priv *wil, const char *fmt, ...)
{
struct net_device *ndev = wil_to_ndev(wil);
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 18cff53..450b688 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -475,6 +475,8 @@ void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...);
__printf(2, 3)
void wil_err(struct wil6210_priv *wil, const char *fmt, ...);
__printf(2, 3)
+void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...);
+__printf(2, 3)
void wil_info(struct wil6210_priv *wil, const char *fmt, ...);
#define wil_dbg(wil, fmt, arg...) do { \
netdev_dbg(wil_to_ndev(wil), fmt, ##arg); \
--
2.1.0