Various of wil6210 fixes
Lior David (1):
wil6210: support regular scan on P2P_DEVICE interface
Maya Erez (6):
wil6210: add function name to wil log macros
wil6210: change RX_HTRSH interrupt print level to debug
wil6210: print debug message when transmitting while disconnected
wil6210: unmask RX_HTRSH interrupt only when connected
wil6210: prevent deep sleep of 60G device in critical paths
wil6210: add support for device led configuration
drivers/net/wireless/ath/wil6210/cfg80211.c | 5 +-
drivers/net/wireless/ath/wil6210/debug.c | 23 ++++-
drivers/net/wireless/ath/wil6210/debugfs.c | 137 +++++++++++++++++++++++++--
drivers/net/wireless/ath/wil6210/interrupt.c | 93 ++++++++++++++----
drivers/net/wireless/ath/wil6210/main.c | 95 ++++++++++++++++++-
drivers/net/wireless/ath/wil6210/p2p.c | 6 ++
drivers/net/wireless/ath/wil6210/txrx.c | 2 +-
drivers/net/wireless/ath/wil6210/wil6210.h | 84 ++++++++++++++--
drivers/net/wireless/ath/wil6210/wmi.c | 97 ++++++++++++++++++-
drivers/net/wireless/ath/wil6210/wmi.h | 61 ++++++++++++
10 files changed, 551 insertions(+), 52 deletions(-)
--
1.8.5.2
From: Lior David <[email protected]>
P2P search can only run on the social channel (channel 2).
When issuing a scan request on the P2P_DEVICE interface,
driver ignored the channels argument and always performed a P2P
search.
Fix this by checking the channels argument, if it is
not specified (meaning full scan) or if a non-social channel
was specified, perform a regular scan and not a P2P search.
Signed-off-by: Lior David <[email protected]>
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 5 +++--
drivers/net/wireless/ath/wil6210/p2p.c | 6 ++++++
drivers/net/wireless/ath/wil6210/wil6210.h | 1 +
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 12cae3c..49eb2e2 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -375,8 +375,9 @@ static int wil_cfg80211_scan(struct wiphy *wiphy,
return -EBUSY;
}
- /* scan on P2P_DEVICE is handled as p2p search */
- if (wdev->iftype == NL80211_IFTYPE_P2P_DEVICE) {
+ /* social scan on P2P_DEVICE is handled as p2p search */
+ if (wdev->iftype == NL80211_IFTYPE_P2P_DEVICE &&
+ wil_p2p_is_social_scan(request)) {
wil->scan_request = request;
wil->radio_wdev = wdev;
rc = wil_p2p_search(wil, request);
diff --git a/drivers/net/wireless/ath/wil6210/p2p.c b/drivers/net/wireless/ath/wil6210/p2p.c
index 2c1b895..1c91538 100644
--- a/drivers/net/wireless/ath/wil6210/p2p.c
+++ b/drivers/net/wireless/ath/wil6210/p2p.c
@@ -22,6 +22,12 @@
#define P2P_SEARCH_DURATION_MS 500
#define P2P_DEFAULT_BI 100
+bool wil_p2p_is_social_scan(struct cfg80211_scan_request *request)
+{
+ return (request->n_channels == 1) &&
+ (request->channels[0]->hw_value == P2P_DMG_SOCIAL_CHANNEL);
+}
+
void wil_p2p_discovery_timer_fn(ulong x)
{
struct wil6210_priv *wil = (void *)x;
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index d005370..7fcfd6f 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -781,6 +781,7 @@ void wil_disable_irq(struct wil6210_priv *wil);
void wil_enable_irq(struct wil6210_priv *wil);
/* P2P */
+bool wil_p2p_is_social_scan(struct cfg80211_scan_request *request);
void wil_p2p_discovery_timer_fn(ulong x);
int wil_p2p_search(struct wil6210_priv *wil,
struct cfg80211_scan_request *request);
--
1.8.5.2
"Haim, Maya" <[email protected]> writes:
> On 4/6/2016 10:19 AM, Joe Perches wrote:
>> On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
>>> Add __func__ to all wil log macros for easier debugging.
>> I think this is unnecessary and merely bloats code size.
>> For all the _dbg calls, dynamic debug can add function names if
>> desired.
>>
>> If really desired, I suggest changing the logging functions to use
>> "%ps and __builtin_return_address(0)
>
> I implemented it with __builtin_return_address(0) at first but found
> its format less readable (e.g. wil_start_xmit+0x58/0x7e8).
Will that work with inline functions and with functions which the
compiler has optimised out?
--
Kalle Valo
On 2016-04-11 05:30, Joe Perches wrote:
> On Sun, 2016-04-10 at 19:17 +0000, qca_merez wrote:
>> On 4/6/2016 10:22 AM, Joe Perches wrote:
>> >
>> > On Tue, 2016-04-05 at 14:24 +0300, Maya Erez w
>> > >
>> > > +void __wil_dbg_ratelimited(struct wil6210_priv *wil, const char
> *fmt,
>> > > +...) {
>> > > + if (net_ratelimit()) {
>> >
>> > Inverting the test would reduce indentation.
>> I preferred to have the same implementation as in wil_err_ratelimited.
>
> That's easy enough.
>
> Maybe:
> ---
> drivers/net/wireless/ath/wil6210/debug.c | 55
> ++++++++++++++----------------
> drivers/net/wireless/ath/wil6210/wil6210.h | 8 ++---
> 2 files changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wil6210/debug.c
> b/drivers/net/wireless/ath/wil6210/debug.c
> index 3249562..de1e932 100644
> --- a/drivers/net/wireless/ath/wil6210/debug.c
> +++ b/drivers/net/wireless/ath/wil6210/debug.c
> @@ -17,61 +17,58 @@
> #include "wil6210.h"
> #include "trace.h"
>
> -void wil_err(struct wil6210_priv *wil, const char *fmt, ...)
> +void wil_err(const struct wil6210_priv *wil, const char *fmt, ...)
> {
> - struct net_device *ndev = wil_to_ndev(wil);
> - struct va_format vaf = {
> - .fmt = fmt,
> - };
> + struct va_format vaf;
> va_list args;
>
> va_start(args, fmt);
> + vaf.fmt = fmt;
> vaf.va = &args;
> - netdev_err(ndev, "%pV", &vaf);
> + netdev_err(wil_to_ndev(wil), "%pV", &vaf);
> trace_wil6210_log_err(&vaf);
> va_end(args);
> }
>
> -void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt,
> ...)
> +void wil_err_ratelimited(const 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;
> + struct va_format vaf;
> + va_list args;
> +
> + if (!net_ratelimit())
> + return;
>
> - va_start(args, fmt);
> - vaf.va = &args;
> - netdev_err(ndev, "%pV", &vaf);
> - trace_wil6210_log_err(&vaf);
> - va_end(args);
> - }
> + va_start(args, fmt);
> + vaf.fmt = fmt;
> + vaf.va = &args;
> + netdev_err(wil_to_ndev(wil), "%pV", &vaf);
> + trace_wil6210_log_err(&vaf);
> + va_end(args);
> }
>
> -void wil_info(struct wil6210_priv *wil, const char *fmt, ...)
> +void wil_info(const struct wil6210_priv *wil, const char *fmt, ...)
> {
> - struct net_device *ndev = wil_to_ndev(wil);
> - struct va_format vaf = {
> - .fmt = fmt,
> - };
> + struct va_format vaf;
> va_list args;
>
> + if (!net_ratelimit())
> + return;
> +
> va_start(args, fmt);
> + vaf.fmt = fmt;
> vaf.va = &args;
> - netdev_info(ndev, "%pV", &vaf);
> + netdev_info(wil_to_ndev(wil), "%pV", &vaf);
> trace_wil6210_log_info(&vaf);
> va_end(args);
> }
>
> -void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...)
> +void wil_dbg_trace(const struct wil6210_priv *wil, const char *fmt,
> ...)
> {
> - struct va_format vaf = {
> - .fmt = fmt,
> - };
> + struct va_format vaf;
> va_list args;
>
> va_start(args, fmt);
> + vaf.fmt = fmt;
> vaf.va = &args;
> trace_wil6210_log_dbg(&vaf);
> va_end(args);
> diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h
> b/drivers/net/wireless/ath/wil6210/wil6210.h
> index 4d699ea4..e2b62b1 100644
> --- a/drivers/net/wireless/ath/wil6210/wil6210.h
> +++ b/drivers/net/wireless/ath/wil6210/wil6210.h
> @@ -633,13 +633,13 @@ struct wil6210_priv {
> #define ndev_to_wil(n) (wdev_to_wil(n->ieee80211_ptr))
>
> __printf(2, 3)
> -void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...);
> +void wil_dbg_trace(const struct wil6210_priv *wil, const char *fmt,
> ...);
> __printf(2, 3)
> -void wil_err(struct wil6210_priv *wil, const char *fmt, ...);
> +void wil_err(const struct wil6210_priv *wil, const char *fmt, ...);
> __printf(2, 3)
> -void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt,
> ...);
> +void wil_err_ratelimited(const struct wil6210_priv *wil, const char
> *fmt,
> ...);
> __printf(2, 3)
> -void wil_info(struct wil6210_priv *wil, const char *fmt, ...);
> +void wil_info(const struct wil6210_priv *wil, const char *fmt, ...);
> #define wil_dbg(wil, fmt, arg...) do { \
> netdev_dbg(wil_to_ndev(wil), fmt, ##arg); \
> wil_dbg_trace(wil, fmt, ##arg); \
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-wireless"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for your suggestion, I currently changed wil_dbg_ratelimited only
and will
change the other functions in a separate patch.
--
Maya Erez
Qualcomm Israel, Inc. on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
"Haim, Maya" <[email protected]> writes:
> On 4/7/2016 6:41 PM, Kalle Valo wrote:
>> "Haim, Maya" <[email protected]> writes:
>>
>>> On 4/6/2016 10:19 AM, Joe Perches wrote:
>>>> On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
>>>>> Add __func__ to all wil log macros for easier debugging.
>>>> I think this is unnecessary and merely bloats code size.
>>>> For all the _dbg calls, dynamic debug can add function names if
>>>> desired.
>>>>
>>>> If really desired, I suggest changing the logging functions to use
>>>> "%ps and __builtin_return_address(0)
>>> I implemented it with __builtin_return_address(0) at first but found
>>> its format less readable (e.g. wil_start_xmit+0x58/0x7e8).
>> Will that work with inline functions and with functions which the
>> compiler has optimised out?
>
> That's a good point. I did a quick check and it doesn't work for
> inline or static functions - for such functions, the name of the
> calling function is printed.
Thanks for checking.
> We can either (1) use my initial implementation (2) add __func__ only
> to the wil_dbg_... macros or (3) revert this patch completely. I find
> the addition of the function names very useful and since most of the
> code doesn't include it, it makes the analysis of issues less
> efficient. Kalle - what is your say on that?
I don't have any preference, it's up to you what you like most.
One more possibility: in ath10k we have a kconfig option
CONFIG_ATH10K_DEBUG to make it possible to disable all overhead from
debug functionality, that would at least solve Joe's concern of extra
memory usage.
--
Kalle Valo
RX_HTRSH interrupt sometimes triggered during device reset
procedure.
To prevent handling this interrupt when not required, unmask
this interrupt only if we are connected and mask it when
disconnected.
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/interrupt.c | 6 +++++-
drivers/net/wireless/ath/wil6210/main.c | 17 +++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/wil6210/interrupt.c b/drivers/net/wireless/ath/wil6210/interrupt.c
index 6897754..22592f3 100644
--- a/drivers/net/wireless/ath/wil6210/interrupt.c
+++ b/drivers/net/wireless/ath/wil6210/interrupt.c
@@ -38,6 +38,8 @@
#define WIL6210_IRQ_DISABLE (0xFFFFFFFFUL)
#define WIL6210_IMC_RX (BIT_DMA_EP_RX_ICR_RX_DONE | \
BIT_DMA_EP_RX_ICR_RX_HTRSH)
+#define WIL6210_IMC_RX_NO_RX_HTRSH (WIL6210_IMC_RX & \
+ (~(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 | \
@@ -109,8 +111,10 @@ void wil6210_unmask_irq_tx(struct wil6210_priv *wil)
void wil6210_unmask_irq_rx(struct wil6210_priv *wil)
{
+ bool unmask_rx_htrsh = test_bit(wil_status_fwconnected, wil->status);
+
wil_w(wil, RGF_DMA_EP_RX_ICR + offsetof(struct RGF_ICR, IMC),
- WIL6210_IMC_RX);
+ unmask_rx_htrsh ? WIL6210_IMC_RX : WIL6210_IMC_RX_NO_RX_HTRSH);
}
static void wil6210_unmask_irq_misc(struct wil6210_priv *wil)
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 8d4e884..261bdab 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -194,6 +194,18 @@ __acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
memset(&sta->stats, 0, sizeof(sta->stats));
}
+static bool wil_ap_is_connected(struct wil6210_priv *wil)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(wil->sta); i++) {
+ if (wil->sta[i].status == wil_sta_connected)
+ return true;
+ }
+
+ return false;
+}
+
static void _wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
u16 reason_code, bool from_event)
{
@@ -247,6 +259,11 @@ static void _wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
}
clear_bit(wil_status_fwconnecting, wil->status);
break;
+ case NL80211_IFTYPE_AP:
+ case NL80211_IFTYPE_P2P_GO:
+ if (!wil_ap_is_connected(wil))
+ clear_bit(wil_status_fwconnected, wil->status);
+ break;
default:
break;
}
--
1.8.5.2
Add __func__ to all wil log macros for easier debugging.
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/debug.c | 6 +++---
drivers/net/wireless/ath/wil6210/wil6210.h | 25 +++++++++++++++++--------
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debug.c b/drivers/net/wireless/ath/wil6210/debug.c
index 3249562..0a30127 100644
--- a/drivers/net/wireless/ath/wil6210/debug.c
+++ b/drivers/net/wireless/ath/wil6210/debug.c
@@ -17,7 +17,7 @@
#include "wil6210.h"
#include "trace.h"
-void wil_err(struct wil6210_priv *wil, const char *fmt, ...)
+void __wil_err(struct wil6210_priv *wil, const char *fmt, ...)
{
struct net_device *ndev = wil_to_ndev(wil);
struct va_format vaf = {
@@ -32,7 +32,7 @@ void wil_err(struct wil6210_priv *wil, const char *fmt, ...)
va_end(args);
}
-void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
+void __wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
{
if (net_ratelimit()) {
struct net_device *ndev = wil_to_ndev(wil);
@@ -49,7 +49,7 @@ void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
}
}
-void wil_info(struct wil6210_priv *wil, const char *fmt, ...)
+void __wil_info(struct wil6210_priv *wil, const char *fmt, ...)
{
struct net_device *ndev = wil_to_ndev(wil);
struct va_format vaf = {
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 4d699ea4..d005370 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -635,21 +635,30 @@ struct wil6210_priv {
__printf(2, 3)
void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...);
__printf(2, 3)
-void wil_err(struct wil6210_priv *wil, const char *fmt, ...);
+void __wil_err(struct wil6210_priv *wil, const char *fmt, ...);
__printf(2, 3)
-void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...);
+void __wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...);
__printf(2, 3)
-void wil_info(struct wil6210_priv *wil, const char *fmt, ...);
+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); \
wil_dbg_trace(wil, fmt, ##arg); \
} while (0)
-#define wil_dbg_irq(wil, fmt, arg...) wil_dbg(wil, "DBG[ IRQ]" fmt, ##arg)
-#define wil_dbg_txrx(wil, fmt, arg...) wil_dbg(wil, "DBG[TXRX]" fmt, ##arg)
-#define wil_dbg_wmi(wil, fmt, arg...) wil_dbg(wil, "DBG[ WMI]" fmt, ##arg)
-#define wil_dbg_misc(wil, fmt, arg...) wil_dbg(wil, "DBG[MISC]" fmt, ##arg)
-#define wil_dbg_pm(wil, fmt, arg...) wil_dbg(wil, "DBG[ PM ]" fmt, ##arg)
+#define wil_dbg_irq(wil, fmt, arg...) \
+ wil_dbg(wil, "DBG[ IRQ]%s: " fmt, __func__, ##arg)
+#define wil_dbg_txrx(wil, fmt, arg...) \
+ wil_dbg(wil, "DBG[TXRX]%s: " fmt, __func__, ##arg)
+#define wil_dbg_wmi(wil, fmt, arg...) \
+ wil_dbg(wil, "DBG[ WMI]%s: " fmt, __func__, ##arg)
+#define wil_dbg_misc(wil, fmt, arg...) \
+ wil_dbg(wil, "DBG[MISC]%s: " fmt, __func__, ##arg)
+#define wil_dbg_pm(wil, fmt, arg...) \
+ wil_dbg(wil, "DBG[ PM ]%s: " fmt, __func__, ##arg)
+#define wil_err(wil, fmt, arg...) __wil_err(wil, "%s: " fmt, __func__, ##arg)
+#define wil_info(wil, fmt, arg...) __wil_info(wil, "%s: " fmt, __func__, ##arg)
+#define wil_err_ratelimited(wil, fmt, arg...) \
+ __wil_err_ratelimited(wil, "%s: " fmt, __func__, ##arg)
/* target operations */
/* register read */
--
1.8.5.2
Add the ability to configure the device led to be used for notifying
the AP activity (60G device supports leds 0-2).
The host can also configure the blinking frequency of the led in
three states.
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 115 +++++++++++++++++++++++++++++
drivers/net/wireless/ath/wil6210/main.c | 3 +
drivers/net/wireless/ath/wil6210/wil6210.h | 25 +++++++
drivers/net/wireless/ath/wil6210/wmi.c | 77 +++++++++++++++++++
drivers/net/wireless/ath/wil6210/wmi.h | 61 +++++++++++++++
5 files changed, 281 insertions(+)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 5d8823d..a8098b4 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1441,6 +1441,118 @@ static const struct file_operations fops_sta = {
.llseek = seq_lseek,
};
+static ssize_t wil_read_file_led_cfg(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ char buf[80];
+ int n;
+
+ n = snprintf(buf, sizeof(buf),
+ "led_id is set to %d, echo 1 to enable, 0 to disable\n",
+ led_id);
+
+ n = min_t(int, n, sizeof(buf));
+
+ return simple_read_from_buffer(user_buf, count, ppos,
+ buf, n);
+}
+
+static ssize_t wil_write_file_led_cfg(struct file *file,
+ const char __user *buf_,
+ size_t count, loff_t *ppos)
+{
+ struct wil6210_priv *wil = file->private_data;
+ int val;
+ int rc;
+
+ rc = kstrtoint_from_user(buf_, count, 0, &val);
+ if (rc) {
+ wil_err(wil, "Invalid argument\n");
+ return rc;
+ }
+
+ wil_info(wil, "%s led %d\n", val ? "Enabling" : "Disabling", led_id);
+ rc = wmi_led_cfg(wil, val);
+ if (rc) {
+ wil_info(wil, "%s led %d failed\n",
+ val ? "Enabling" : "Disabling", led_id);
+ return rc;
+ }
+
+ return count;
+}
+
+static const struct file_operations fops_led_cfg = {
+ .read = wil_read_file_led_cfg,
+ .write = wil_write_file_led_cfg,
+ .open = simple_open,
+};
+
+/* led_blink_time, write:
+ * "<blink_on_slow> <blink_off_slow> <blink_on_med> <blink_off_med> <blink_on_fast> <blink_off_fast>
+ */
+static ssize_t wil_write_led_blink_time(struct file *file,
+ const char __user *buf,
+ size_t len, loff_t *ppos)
+{
+ int rc;
+ char *kbuf = kmalloc(len + 1, GFP_KERNEL);
+
+ if (!kbuf)
+ return -ENOMEM;
+
+ rc = simple_write_to_buffer(kbuf, len, ppos, buf, len);
+ if (rc != len) {
+ kfree(kbuf);
+ return rc >= 0 ? -EIO : rc;
+ }
+
+ kbuf[len] = '\0';
+ rc = sscanf(kbuf, "%d %d %d %d %d %d",
+ &led_blink_time[WIL_LED_TIME_SLOW].on_ms,
+ &led_blink_time[WIL_LED_TIME_SLOW].off_ms,
+ &led_blink_time[WIL_LED_TIME_MED].on_ms,
+ &led_blink_time[WIL_LED_TIME_MED].off_ms,
+ &led_blink_time[WIL_LED_TIME_FAST].on_ms,
+ &led_blink_time[WIL_LED_TIME_FAST].off_ms);
+ kfree(kbuf);
+
+ if (rc < 0)
+ return rc;
+ if (rc < 6)
+ return -EINVAL;
+
+ return len;
+}
+
+static ssize_t wil_read_led_blink_time(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ static char text[400];
+
+ snprintf(text, sizeof(text),
+ "To set led blink on/off time variables write:\n"
+ "<blink_on_slow> <blink_off_slow> <blink_on_med> "
+ "<blink_off_med> <blink_on_fast> <blink_off_fast>\n"
+ "The current values are:\n"
+ "%d %d %d %d %d %d\n",
+ led_blink_time[WIL_LED_TIME_SLOW].on_ms,
+ led_blink_time[WIL_LED_TIME_SLOW].off_ms,
+ led_blink_time[WIL_LED_TIME_MED].on_ms,
+ led_blink_time[WIL_LED_TIME_MED].off_ms,
+ led_blink_time[WIL_LED_TIME_FAST].on_ms,
+ led_blink_time[WIL_LED_TIME_FAST].off_ms);
+
+ return simple_read_from_buffer(user_buf, count, ppos, text,
+ sizeof(text));
+}
+
+static const struct file_operations fops_led_blink_time = {
+ .read = wil_read_led_blink_time,
+ .write = wil_write_led_blink_time,
+ .open = simple_open,
+};
+
/*----------------*/
static void wil6210_debugfs_init_blobs(struct wil6210_priv *wil,
struct dentry *dbg)
@@ -1489,6 +1601,8 @@ static const struct {
{"link", S_IRUGO, &fops_link},
{"info", S_IRUGO, &fops_info},
{"recovery", S_IRUGO | S_IWUSR, &fops_recovery},
+ {"led_cfg", S_IRUGO | S_IWUSR, &fops_led_cfg},
+ {"led_blink_time", S_IRUGO | S_IWUSR, &fops_led_blink_time},
};
static void wil6210_debugfs_init_files(struct wil6210_priv *wil,
@@ -1551,6 +1665,7 @@ static const struct dbg_off dbg_statics[] = {
{"mem_addr", S_IRUGO | S_IWUSR, (ulong)&mem_addr, doff_u32},
{"vring_idle_trsh", S_IRUGO | S_IWUSR, (ulong)&vring_idle_trsh,
doff_u32},
+ {"led_polarity", S_IRUGO | S_IWUSR, (ulong)&led_polarity, doff_u8},
{},
};
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index f7ed651..8e31d75 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -841,6 +841,9 @@ int wil_reset(struct wil6210_priv *wil, bool load_fw)
wil6210_disconnect(wil, NULL, WLAN_REASON_DEAUTH_LEAVING, false);
wil_bcast_fini(wil);
+ /* Disable device led before reset*/
+ wmi_led_cfg(wil, false);
+
/* prevent NAPI from being scheduled and prevent wmi commands */
mutex_lock(&wil->wmi_mutex);
bitmap_zero(wil->status, wil_status_last);
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 32e7348..5f8411f 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -546,6 +546,30 @@ struct wil_blob_wrapper {
struct debugfs_blob_wrapper blob;
};
+#define WIL_LED_MAX_ID (2)
+#define WIL_LED_INVALID_ID (0xF)
+#define WIL_LED_BLINK_ON_SLOW_MS (300)
+#define WIL_LED_BLINK_OFF_SLOW_MS (300)
+#define WIL_LED_BLINK_ON_MED_MS (200)
+#define WIL_LED_BLINK_OFF_MED_MS (200)
+#define WIL_LED_BLINK_ON_FAST_MS (100)
+#define WIL_LED_BLINK_OFF_FAST_MS (100)
+enum {
+ WIL_LED_TIME_SLOW = 0,
+ WIL_LED_TIME_MED,
+ WIL_LED_TIME_FAST,
+ WIL_LED_TIME_LAST,
+};
+
+struct blink_on_off_time {
+ u32 on_ms;
+ u32 off_ms;
+};
+
+extern struct blink_on_off_time led_blink_time[WIL_LED_TIME_LAST];
+extern u8 led_id;
+extern u8 led_polarity;
+
struct wil6210_priv {
struct pci_dev *pdev;
struct wireless_dev *wdev;
@@ -841,6 +865,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, u8 hidden_ssid, u8 is_go);
int wmi_pcp_stop(struct wil6210_priv *wil);
+int wmi_led_cfg(struct wil6210_priv *wil, bool enable);
void wil6210_disconnect(struct wil6210_priv *wil, const u8 *bssid,
u16 reason_code, bool from_event);
void wil_probe_client_flush(struct wil6210_priv *wil);
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index f83bde1..c9fef36 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -32,6 +32,11 @@ module_param(agg_wsize, int, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(agg_wsize, " Window size for Tx Block Ack after connect;"
" 0 - use default; < 0 - don't auto-establish");
+u8 led_id = WIL_LED_INVALID_ID;
+module_param(led_id, byte, S_IRUGO);
+MODULE_PARM_DESC(led_id,
+ " 60G device led enablement. Set the led ID (0-2) to enable");
+
/**
* WMI event receiving - theory of operations
*
@@ -94,6 +99,14 @@ const struct fw_map fw_mapping[] = {
*/
};
+struct blink_on_off_time led_blink_time[] = {
+ {WIL_LED_BLINK_ON_SLOW_MS, WIL_LED_BLINK_OFF_SLOW_MS},
+ {WIL_LED_BLINK_ON_MED_MS, WIL_LED_BLINK_OFF_MED_MS},
+ {WIL_LED_BLINK_ON_FAST_MS, WIL_LED_BLINK_OFF_FAST_MS},
+};
+
+u8 led_polarity = LED_POLARITY_LOW_ACTIVE;
+
/**
* return AHB address for given firmware/ucode internal (linker) address
* @x - internal address
@@ -971,6 +984,60 @@ int wmi_set_mac_address(struct wil6210_priv *wil, void *addr)
return wmi_send(wil, WMI_SET_MAC_ADDRESS_CMDID, &cmd, sizeof(cmd));
}
+int wmi_led_cfg(struct wil6210_priv *wil, bool enable)
+{
+ int rc = 0;
+ struct wmi_led_cfg_cmd cmd = {
+ .led_mode = enable,
+ .id = led_id,
+ .slow_blink_cfg.blink_on =
+ cpu_to_le32(led_blink_time[WIL_LED_TIME_SLOW].on_ms),
+ .slow_blink_cfg.blink_off =
+ cpu_to_le32(led_blink_time[WIL_LED_TIME_SLOW].off_ms),
+ .medium_blink_cfg.blink_on =
+ cpu_to_le32(led_blink_time[WIL_LED_TIME_MED].on_ms),
+ .medium_blink_cfg.blink_off =
+ cpu_to_le32(led_blink_time[WIL_LED_TIME_MED].off_ms),
+ .fast_blink_cfg.blink_on =
+ cpu_to_le32(led_blink_time[WIL_LED_TIME_FAST].on_ms),
+ .fast_blink_cfg.blink_off =
+ cpu_to_le32(led_blink_time[WIL_LED_TIME_FAST].off_ms),
+ .led_polarity = led_polarity,
+ };
+ struct {
+ struct wmi_cmd_hdr wmi;
+ struct wmi_led_cfg_done_event evt;
+ } __packed reply;
+
+ if (led_id == WIL_LED_INVALID_ID)
+ goto out;
+
+ if (led_id > WIL_LED_MAX_ID) {
+ wil_err(wil, "Invalid led id %d\n", led_id);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ wil_dbg_wmi(wil,
+ "%s led %d\n",
+ enable ? "enabling" : "disabling", led_id);
+
+ rc = wmi_call(wil, WMI_LED_CFG_CMDID, &cmd, sizeof(cmd),
+ WMI_LED_CFG_DONE_EVENTID, &reply, sizeof(reply),
+ 100);
+ if (rc)
+ goto out;
+
+ if (reply.evt.status) {
+ wil_err(wil, "led %d cfg failed with status %d\n",
+ led_id, le32_to_cpu(reply.evt.status));
+ rc = -EINVAL;
+ }
+
+out:
+ return rc;
+}
+
int wmi_pcp_start(struct wil6210_priv *wil, int bi, u8 wmi_nettype,
u8 chan, u8 hidden_ssid, u8 is_go)
{
@@ -1013,11 +1080,21 @@ int wmi_pcp_start(struct wil6210_priv *wil, int bi, u8 wmi_nettype,
if (reply.evt.status != WMI_FW_STATUS_SUCCESS)
rc = -EINVAL;
+ if (wmi_nettype != WMI_NETTYPE_P2P)
+ /* Don't fail due to error in the led configuration */
+ wmi_led_cfg(wil, true);
+
return rc;
}
int wmi_pcp_stop(struct wil6210_priv *wil)
{
+ int rc;
+
+ rc = wmi_led_cfg(wil, false);
+ if (rc)
+ return rc;
+
return wmi_call(wil, WMI_PCP_STOP_CMDID, NULL, 0,
WMI_PCP_STOPPED_EVENTID, NULL, 0, 20);
}
diff --git a/drivers/net/wireless/ath/wil6210/wmi.h b/drivers/net/wireless/ath/wil6210/wmi.h
index 29865e0..685fe0d 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.h
+++ b/drivers/net/wireless/ath/wil6210/wmi.h
@@ -129,6 +129,7 @@ enum wmi_command_id {
WMI_THERMAL_THROTTLING_GET_STATUS_CMDID = 0x855,
WMI_OTP_READ_CMDID = 0x856,
WMI_OTP_WRITE_CMDID = 0x857,
+ WMI_LED_CFG_CMDID = 0x858,
/* Performance monitoring commands */
WMI_BF_CTRL_CMDID = 0x862,
WMI_NOTIFY_REQ_CMDID = 0x863,
@@ -868,6 +869,7 @@ enum wmi_event_id {
WMI_RX_MGMT_PACKET_EVENTID = 0x1840,
WMI_TX_MGMT_PACKET_EVENTID = 0x1841,
WMI_OTP_READ_RESULT_EVENTID = 0x1856,
+ WMI_LED_CFG_DONE_EVENTID = 0x1858,
/* Performance monitoring events */
WMI_DATA_PORT_OPEN_EVENTID = 0x1860,
WMI_WBE_LINK_DOWN_EVENTID = 0x1861,
@@ -1349,4 +1351,63 @@ enum wmi_hidden_ssid {
WMI_HIDDEN_SSID_CLEAR = 0xFE,
};
+/* WMI_LED_CFG_CMDID
+ *
+ * Configure LED On\Off\Blinking operation
+ *
+ * Returned events:
+ * - WMI_LED_CFG_DONE_EVENTID
+ */
+enum led_mode {
+ LED_DISABLE = 0x00,
+ LED_ENABLE = 0x01,
+};
+
+/* The names of the led as
+ * described on HW schemes.
+ */
+enum wmi_led_id {
+ WMI_LED_WLAN = 0x00,
+ WMI_LED_WPAN = 0x01,
+ WMI_LED_WWAN = 0x02,
+};
+
+/* Led polarity mode. */
+enum wmi_led_polarity {
+ LED_POLARITY_HIGH_ACTIVE = 0x00,
+ LED_POLARITY_LOW_ACTIVE = 0x01,
+};
+
+/* Combination of on and off
+ * creates the blinking period
+ */
+struct wmi_led_blink_mode {
+ __le32 blink_on;
+ __le32 blink_off;
+} __packed;
+
+/* WMI_LED_CFG_CMDID */
+struct wmi_led_cfg_cmd {
+ /* enum led_mode_e */
+ u8 led_mode;
+ /* enum wmi_led_id_e */
+ u8 id;
+ /* slow speed blinking combination */
+ struct wmi_led_blink_mode slow_blink_cfg;
+ /* medium speed blinking combination */
+ struct wmi_led_blink_mode medium_blink_cfg;
+ /* high speed blinking combination */
+ struct wmi_led_blink_mode fast_blink_cfg;
+ /* polarity of the led */
+ u8 led_polarity;
+ /* reserved */
+ u8 reserved;
+} __packed;
+
+/* WMI_LED_CFG_DONE_EVENTID */
+struct wmi_led_cfg_done_event {
+ /* led config status */
+ __le32 status;
+} __packed;
+
#endif /* __WILOCITY_WMI_H__ */
--
1.8.5.2
Network stack can try to transmit data while AP / STA is
disconnected.
Change this print-out to debug level as this should not be
handled as error.
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/debug.c | 17 +++++++++++++++++
drivers/net/wireless/ath/wil6210/txrx.c | 2 +-
drivers/net/wireless/ath/wil6210/wil6210.h | 4 ++++
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/wil6210/debug.c b/drivers/net/wireless/ath/wil6210/debug.c
index 0a30127..e5395fd 100644
--- a/drivers/net/wireless/ath/wil6210/debug.c
+++ b/drivers/net/wireless/ath/wil6210/debug.c
@@ -49,6 +49,23 @@ void __wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
}
}
+void __wil_dbg_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_dbg(ndev, "%pV", &vaf);
+ trace_wil6210_log_dbg(&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/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index f260b232..a4e4379 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -1759,7 +1759,7 @@ netdev_tx_t wil_start_xmit(struct sk_buff *skb, struct net_device *ndev)
goto drop;
}
if (unlikely(!test_bit(wil_status_fwconnected, wil->status))) {
- wil_err_ratelimited(wil, "FW not connected\n");
+ wil_dbg_ratelimited(wil, "FW not connected, packet dropped\n");
goto drop;
}
if (unlikely(wil->wdev->iftype == NL80211_IFTYPE_MONITOR)) {
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 7fcfd6f..84b3fa6 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -640,6 +640,8 @@ __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, ...);
+__printf(2, 3)
+void __wil_dbg_ratelimited(struct wil6210_priv *wil, const char *fmt, ...);
#define wil_dbg(wil, fmt, arg...) do { \
netdev_dbg(wil_to_ndev(wil), fmt, ##arg); \
wil_dbg_trace(wil, fmt, ##arg); \
@@ -659,6 +661,8 @@ void __wil_info(struct wil6210_priv *wil, const char *fmt, ...);
#define wil_info(wil, fmt, arg...) __wil_info(wil, "%s: " fmt, __func__, ##arg)
#define wil_err_ratelimited(wil, fmt, arg...) \
__wil_err_ratelimited(wil, "%s: " fmt, __func__, ##arg)
+#define wil_dbg_ratelimited(wil, fmt, arg...) \
+ __wil_dbg_ratelimited(wil, "%s: " fmt, __func__, ##arg)
/* target operations */
/* register read */
--
1.8.5.2
On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
> Add __func__ to all wil log macros for easier debugging.
I think this is unnecessary and merely bloats code size.
For all the _dbg calls, dynamic debug can add function
names if desired.
If really desired, I suggest changing the logging
functions to use "%ps and __builtin_return_address(0)
On 4/6/2016 10:19 AM, Joe Perches wrote:
> On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
>> Add __func__ to all wil log macros for easier debugging.
> I think this is unnecessary and merely bloats code size.
> For all the _dbg calls, dynamic debug can add function names if
> desired.
>
> If really desired, I suggest changing the logging functions to use
> "%ps and __builtin_return_address(0)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless"
> in
> the body of a message to [email protected] More majordomo info
> at http://vger.kernel.org/majordomo-info.html
I implemented it with __builtin_return_address(0) at first but found its format less readable (e.g. wil_start_xmit+0x58/0x7e8).
I agree that using __builtin_return_address(0) simplifies the code and I'm OK with your suggestion to use it.
On 4/6/2016 10:22 AM, Joe Perches wrote:
> On Tue, 2016-04-05 at 14:24 +0300, Maya Erez w
>> +void __wil_dbg_ratelimited(struct wil6210_priv *wil, const char *fmt,
>> +...) {
>> + if (net_ratelimit()) {
> Inverting the test would reduce indentation.
I preferred to have the same implementation as in wil_err_ratelimited.
On Thu, 2016-04-07 at 15:04 +0000, Haim, Maya wrote:
> On 4/6/2016 10:19 AM, Joe Perches wrote:
> > On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
> > > Add __func__ to all wil log macros for easier debugging.
> > I think this is unnecessary and merely bloats code size.
> > For all the _dbg calls, dynamic debug can add function names if?
> > desired.
> >
> > If really desired, I suggest changing the logging functions to use?
> > "%ps and __builtin_return_address(0)
[]
> implemented it with __builtin_return_address(0) at first but found its format less readable (e.g. wil_start_xmit+0x58/0x7e8).
> I agree that using __builtin_return_address(0) simplifies the code and I'm OK with your suggestion to use it.
I believe you must have used %pS and not %ps
See: Documentation/printk-formats.txt
Symbols/Function Pointers:
%pS versatile_init+0x0/0x110
%ps versatile_init
In idle times 60G device can enter deep sleep and turn off
its XTAL clock.
Host access triggers the device power-up flow which will hold
the AHB during XTAL stabilization until device switches from
slow-clock to XTAL clock.
This behavior can stall the PCIe bus for some arbitrary period
of time.
In order to prevent this stall, host can vote for High Latency
Access Policy (HALP) before reading from PCIe bus.
This vote will wakeup the device from deep sleep and prevent
deep sleep until unvote is done.
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 22 +++++---
drivers/net/wireless/ath/wil6210/interrupt.c | 80 ++++++++++++++++++++++------
drivers/net/wireless/ath/wil6210/main.c | 75 ++++++++++++++++++++++++--
drivers/net/wireless/ath/wil6210/wil6210.h | 29 +++++++++-
drivers/net/wireless/ath/wil6210/wmi.c | 20 +++++--
5 files changed, 194 insertions(+), 32 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index b338a09..5d8823d 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -171,6 +171,8 @@ static void wil_print_ring(struct seq_file *s, const char *prefix,
int rsize;
uint i;
+ wil_halp_vote(wil);
+
wil_memcpy_fromio_32(&r, off, sizeof(r));
wil_mbox_ring_le2cpus(&r);
/*
@@ -236,6 +238,7 @@ static void wil_print_ring(struct seq_file *s, const char *prefix,
}
out:
seq_puts(s, "}\n");
+ wil_halp_unvote(wil);
}
static int wil_mbox_debugfs_show(struct seq_file *s, void *data)
@@ -500,9 +503,9 @@ static ssize_t wil_read_file_ioblob(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
enum { max_count = 4096 };
- struct debugfs_blob_wrapper *blob = file->private_data;
+ struct wil_blob_wrapper *wil_blob = file->private_data;
loff_t pos = *ppos;
- size_t available = blob->size;
+ size_t available = wil_blob->blob.size;
void *buf;
size_t ret;
@@ -521,8 +524,9 @@ static ssize_t wil_read_file_ioblob(struct file *file, char __user *user_buf,
if (!buf)
return -ENOMEM;
- wil_memcpy_fromio_32(buf, (const volatile void __iomem *)blob->data +
- pos, count);
+ wil_memcpy_fromio_halp_vote(wil_blob->wil, buf,
+ (const volatile void __iomem *)
+ wil_blob->blob.data + pos, count);
ret = copy_to_user(user_buf, buf, count);
kfree(buf);
@@ -545,9 +549,9 @@ static
struct dentry *wil_debugfs_create_ioblob(const char *name,
umode_t mode,
struct dentry *parent,
- struct debugfs_blob_wrapper *blob)
+ struct wil_blob_wrapper *wil_blob)
{
- return debugfs_create_file(name, mode, parent, blob, &fops_ioblob);
+ return debugfs_create_file(name, mode, parent, wil_blob, &fops_ioblob);
}
/*---reset---*/
@@ -1445,16 +1449,18 @@ static void wil6210_debugfs_init_blobs(struct wil6210_priv *wil,
char name[32];
for (i = 0; i < ARRAY_SIZE(fw_mapping); i++) {
- struct debugfs_blob_wrapper *blob = &wil->blobs[i];
+ struct wil_blob_wrapper *wil_blob = &wil->blobs[i];
+ struct debugfs_blob_wrapper *blob = &wil_blob->blob;
const struct fw_map *map = &fw_mapping[i];
if (!map->name)
continue;
+ wil_blob->wil = wil;
blob->data = (void * __force)wil->csr + HOSTADDR(map->host);
blob->size = map->to - map->from;
snprintf(name, sizeof(name), "blob_%s", map->name);
- wil_debugfs_create_ioblob(name, S_IRUGO, dbg, blob);
+ wil_debugfs_create_ioblob(name, S_IRUGO, dbg, wil_blob);
}
}
diff --git a/drivers/net/wireless/ath/wil6210/interrupt.c b/drivers/net/wireless/ath/wil6210/interrupt.c
index 22592f3..011e741 100644
--- a/drivers/net/wireless/ath/wil6210/interrupt.c
+++ b/drivers/net/wireless/ath/wil6210/interrupt.c
@@ -35,17 +35,19 @@
*
*/
-#define WIL6210_IRQ_DISABLE (0xFFFFFFFFUL)
+#define WIL6210_IRQ_DISABLE (0xFFFFFFFFUL)
+#define WIL6210_IRQ_DISABLE_NO_HALP (0xF7FFFFFFUL)
#define WIL6210_IMC_RX (BIT_DMA_EP_RX_ICR_RX_DONE | \
BIT_DMA_EP_RX_ICR_RX_HTRSH)
#define WIL6210_IMC_RX_NO_RX_HTRSH (WIL6210_IMC_RX & \
(~(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 | \
- ISR_MISC_MBOX_EVT | \
- ISR_MISC_FW_ERROR)
-
+#define WIL6210_IMC_MISC_NO_HALP (ISR_MISC_FW_READY | \
+ ISR_MISC_MBOX_EVT | \
+ ISR_MISC_FW_ERROR)
+#define WIL6210_IMC_MISC (WIL6210_IMC_MISC_NO_HALP | \
+ BIT_DMA_EP_MISC_ICR_HALP)
#define WIL6210_IRQ_PSEUDO_MASK (u32)(~(BIT_DMA_PSEUDO_CAUSE_RX | \
BIT_DMA_PSEUDO_CAUSE_TX | \
BIT_DMA_PSEUDO_CAUSE_MISC))
@@ -53,6 +55,7 @@
#if defined(CONFIG_WIL6210_ISR_COR)
/* configure to Clear-On-Read mode */
#define WIL_ICR_ICC_VALUE (0xFFFFFFFFUL)
+#define WIL_ICR_ICC_MISC_VALUE (0xF7FFFFFFUL)
static inline void wil_icr_clear(u32 x, void __iomem *addr)
{
@@ -60,6 +63,7 @@ static inline void wil_icr_clear(u32 x, void __iomem *addr)
#else /* defined(CONFIG_WIL6210_ISR_COR) */
/* configure to Write-1-to-Clear mode */
#define WIL_ICR_ICC_VALUE (0UL)
+#define WIL_ICR_ICC_MISC_VALUE (0UL)
static inline void wil_icr_clear(u32 x, void __iomem *addr)
{
@@ -88,10 +92,21 @@ static void wil6210_mask_irq_rx(struct wil6210_priv *wil)
WIL6210_IRQ_DISABLE);
}
-static void wil6210_mask_irq_misc(struct wil6210_priv *wil)
+static void wil6210_mask_irq_misc(struct wil6210_priv *wil, bool mask_halp)
{
+ wil_dbg_irq(wil, "%s: mask_halp(%s)\n", __func__,
+ mask_halp ? "true" : "false");
+
wil_w(wil, RGF_DMA_EP_MISC_ICR + offsetof(struct RGF_ICR, IMS),
- WIL6210_IRQ_DISABLE);
+ mask_halp ? WIL6210_IRQ_DISABLE : WIL6210_IRQ_DISABLE_NO_HALP);
+}
+
+static void wil6210_mask_halp(struct wil6210_priv *wil)
+{
+ wil_dbg_irq(wil, "%s()\n", __func__);
+
+ wil_w(wil, RGF_DMA_EP_MISC_ICR + offsetof(struct RGF_ICR, IMS),
+ BIT_DMA_EP_MISC_ICR_HALP);
}
static void wil6210_mask_irq_pseudo(struct wil6210_priv *wil)
@@ -117,10 +132,21 @@ void wil6210_unmask_irq_rx(struct wil6210_priv *wil)
unmask_rx_htrsh ? WIL6210_IMC_RX : WIL6210_IMC_RX_NO_RX_HTRSH);
}
-static void wil6210_unmask_irq_misc(struct wil6210_priv *wil)
+static void wil6210_unmask_irq_misc(struct wil6210_priv *wil, bool unmask_halp)
{
+ wil_dbg_irq(wil, "%s: unmask_halp(%s)\n", __func__,
+ unmask_halp ? "true" : "false");
+
wil_w(wil, RGF_DMA_EP_MISC_ICR + offsetof(struct RGF_ICR, IMC),
- WIL6210_IMC_MISC);
+ unmask_halp ? WIL6210_IMC_MISC : WIL6210_IMC_MISC_NO_HALP);
+}
+
+static void wil6210_unmask_halp(struct wil6210_priv *wil)
+{
+ wil_dbg_irq(wil, "%s()\n", __func__);
+
+ wil_w(wil, RGF_DMA_EP_MISC_ICR + offsetof(struct RGF_ICR, IMC),
+ BIT_DMA_EP_MISC_ICR_HALP);
}
static void wil6210_unmask_irq_pseudo(struct wil6210_priv *wil)
@@ -138,7 +164,7 @@ void wil_mask_irq(struct wil6210_priv *wil)
wil6210_mask_irq_tx(wil);
wil6210_mask_irq_rx(wil);
- wil6210_mask_irq_misc(wil);
+ wil6210_mask_irq_misc(wil, true);
wil6210_mask_irq_pseudo(wil);
}
@@ -151,12 +177,12 @@ void wil_unmask_irq(struct wil6210_priv *wil)
wil_w(wil, RGF_DMA_EP_TX_ICR + offsetof(struct RGF_ICR, ICC),
WIL_ICR_ICC_VALUE);
wil_w(wil, RGF_DMA_EP_MISC_ICR + offsetof(struct RGF_ICR, ICC),
- WIL_ICR_ICC_VALUE);
+ WIL_ICR_ICC_MISC_VALUE);
wil6210_unmask_irq_pseudo(wil);
wil6210_unmask_irq_tx(wil);
wil6210_unmask_irq_rx(wil);
- wil6210_unmask_irq_misc(wil);
+ wil6210_unmask_irq_misc(wil, true);
}
void wil_configure_interrupt_moderation(struct wil6210_priv *wil)
@@ -345,7 +371,7 @@ static irqreturn_t wil6210_irq_misc(int irq, void *cookie)
return IRQ_NONE;
}
- wil6210_mask_irq_misc(wil);
+ wil6210_mask_irq_misc(wil, false);
if (isr & ISR_MISC_FW_ERROR) {
u32 fw_assert_code = wil_r(wil, RGF_FW_ASSERT_CODE);
@@ -373,12 +399,19 @@ static irqreturn_t wil6210_irq_misc(int irq, void *cookie)
isr &= ~ISR_MISC_FW_READY;
}
+ if (isr & BIT_DMA_EP_MISC_ICR_HALP) {
+ wil_dbg_irq(wil, "%s: HALP IRQ invoked\n", __func__);
+ wil6210_mask_halp(wil);
+ isr &= ~BIT_DMA_EP_MISC_ICR_HALP;
+ complete(&wil->halp.comp);
+ }
+
wil->isr_misc = isr;
if (isr) {
return IRQ_WAKE_THREAD;
} else {
- wil6210_unmask_irq_misc(wil);
+ wil6210_unmask_irq_misc(wil, false);
return IRQ_HANDLED;
}
}
@@ -415,7 +448,7 @@ static irqreturn_t wil6210_irq_misc_thread(int irq, void *cookie)
wil->isr_misc = 0;
- wil6210_unmask_irq_misc(wil);
+ wil6210_unmask_irq_misc(wil, false);
return IRQ_HANDLED;
}
@@ -557,6 +590,23 @@ void wil6210_clear_irq(struct wil6210_priv *wil)
wmb(); /* make sure write completed */
}
+void wil6210_set_halp(struct wil6210_priv *wil)
+{
+ wil_dbg_misc(wil, "%s()\n", __func__);
+
+ wil_w(wil, RGF_DMA_EP_MISC_ICR + offsetof(struct RGF_ICR, ICS),
+ BIT_DMA_EP_MISC_ICR_HALP);
+}
+
+void wil6210_clear_halp(struct wil6210_priv *wil)
+{
+ wil_dbg_misc(wil, "%s()\n", __func__);
+
+ wil_w(wil, RGF_DMA_EP_MISC_ICR + offsetof(struct RGF_ICR, ICR),
+ BIT_DMA_EP_MISC_ICR_HALP);
+ wil6210_unmask_halp(wil);
+}
+
int wil6210_init_irq(struct wil6210_priv *wil, int irq, bool use_msi)
{
int rc;
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 261bdab..f7ed651 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -23,6 +23,8 @@
#include "wmi.h"
#include "boot_loader.h"
+#define WAIT_FOR_HALP_VOTE_MS 100
+
bool debug_fw; /* = false; */
module_param(debug_fw, bool, S_IRUGO);
MODULE_PARM_DESC(debug_fw, " do not perform card reset. For FW debug");
@@ -132,6 +134,14 @@ void wil_memcpy_fromio_32(void *dst, const volatile void __iomem *src,
*d++ = __raw_readl(s++);
}
+void wil_memcpy_fromio_halp_vote(struct wil6210_priv *wil, void *dst,
+ const volatile void __iomem *src, size_t count)
+{
+ wil_halp_vote(wil);
+ wil_memcpy_fromio_32(dst, src, count);
+ wil_halp_unvote(wil);
+}
+
void wil_memcpy_toio_32(volatile void __iomem *dst, const void *src,
size_t count)
{
@@ -142,6 +152,15 @@ void wil_memcpy_toio_32(volatile void __iomem *dst, const void *src,
__raw_writel(*s++, d++);
}
+void wil_memcpy_toio_halp_vote(struct wil6210_priv *wil,
+ volatile void __iomem *dst,
+ const void *src, size_t count)
+{
+ wil_halp_vote(wil);
+ wil_memcpy_toio_32(dst, src, count);
+ wil_halp_unvote(wil);
+}
+
static void wil_disconnect_cid(struct wil6210_priv *wil, int cid,
u16 reason_code, bool from_event)
__acquires(&sta->tid_rx_lock) __releases(&sta->tid_rx_lock)
@@ -474,9 +493,11 @@ int wil_priv_init(struct wil6210_priv *wil)
mutex_init(&wil->wmi_mutex);
mutex_init(&wil->probe_client_mutex);
mutex_init(&wil->p2p_wdev_mutex);
+ mutex_init(&wil->halp.lock);
init_completion(&wil->wmi_ready);
init_completion(&wil->wmi_call);
+ init_completion(&wil->halp.comp);
wil->bcast_vring = -1;
setup_timer(&wil->connect_timer, wil_connect_timer_fn, (ulong)wil);
@@ -572,11 +593,10 @@ static inline void wil_release_cpu(struct wil6210_priv *wil)
static void wil_set_oob_mode(struct wil6210_priv *wil, bool enable)
{
wil_info(wil, "%s: enable=%d\n", __func__, enable);
- if (enable) {
+ if (enable)
wil_s(wil, RGF_USER_USAGE_6, BIT_USER_OOB_MODE);
- } else {
+ else
wil_c(wil, RGF_USER_USAGE_6, BIT_USER_OOB_MODE);
- }
}
static int wil_target_reset(struct wil6210_priv *wil)
@@ -888,6 +908,7 @@ int wil_reset(struct wil6210_priv *wil, bool load_fw)
wil->ap_isolate = 0;
reinit_completion(&wil->wmi_ready);
reinit_completion(&wil->wmi_call);
+ reinit_completion(&wil->halp.comp);
if (load_fw) {
wil_configure_interrupt_moderation(wil);
@@ -1078,3 +1099,51 @@ int wil_find_cid(struct wil6210_priv *wil, const u8 *mac)
return rc;
}
+
+void wil_halp_vote(struct wil6210_priv *wil)
+{
+ unsigned long rc;
+ unsigned long to_jiffies = msecs_to_jiffies(WAIT_FOR_HALP_VOTE_MS);
+
+ mutex_lock(&wil->halp.lock);
+
+ wil_dbg_misc(wil, "%s: start, HALP ref_cnt (%d)\n", __func__,
+ wil->halp.ref_cnt);
+
+ if (++wil->halp.ref_cnt == 1) {
+ wil6210_set_halp(wil);
+ rc = wait_for_completion_timeout(&wil->halp.comp, to_jiffies);
+ if (!rc)
+ wil_err(wil, "%s: HALP vote timed out\n", __func__);
+ else
+ wil_dbg_misc(wil,
+ "%s: HALP vote completed after %d ms\n",
+ __func__,
+ jiffies_to_msecs(to_jiffies - rc));
+ }
+
+ wil_dbg_misc(wil, "%s: end, HALP ref_cnt (%d)\n", __func__,
+ wil->halp.ref_cnt);
+
+ mutex_unlock(&wil->halp.lock);
+}
+
+void wil_halp_unvote(struct wil6210_priv *wil)
+{
+ WARN_ON(wil->halp.ref_cnt == 0);
+
+ mutex_lock(&wil->halp.lock);
+
+ wil_dbg_misc(wil, "%s: start, HALP ref_cnt (%d)\n", __func__,
+ wil->halp.ref_cnt);
+
+ if (--wil->halp.ref_cnt == 0) {
+ wil6210_clear_halp(wil);
+ wil_dbg_misc(wil, "%s: HALP unvote\n", __func__);
+ }
+
+ wil_dbg_misc(wil, "%s: end, HALP ref_cnt (%d)\n", __func__,
+ wil->halp.ref_cnt);
+
+ mutex_unlock(&wil->halp.lock);
+}
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 84b3fa6..32e7348 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -168,6 +168,7 @@ struct RGF_ICR {
#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)
+ #define BIT_DMA_EP_MISC_ICR_HALP BIT(27)
#define BIT_DMA_EP_MISC_ICR_FW_INT(n) BIT(28+n) /* n = [0..3] */
/* Legacy interrupt moderation control (before Sparrow v2)*/
@@ -534,6 +535,17 @@ struct pmc_ctx {
int descriptor_size;
};
+struct wil_halp {
+ struct mutex lock; /* protect halp ref_cnt */
+ unsigned int ref_cnt;
+ struct completion comp;
+};
+
+struct wil_blob_wrapper {
+ struct wil6210_priv *wil;
+ struct debugfs_blob_wrapper blob;
+};
+
struct wil6210_priv {
struct pci_dev *pdev;
struct wireless_dev *wdev;
@@ -606,7 +618,7 @@ struct wil6210_priv {
atomic_t isr_count_rx, isr_count_tx;
/* debugfs */
struct dentry *debug;
- struct debugfs_blob_wrapper blobs[ARRAY_SIZE(fw_mapping)];
+ struct wil_blob_wrapper blobs[ARRAY_SIZE(fw_mapping)];
u8 discovery_mode;
void *platform_handle;
@@ -622,6 +634,10 @@ struct wil6210_priv {
struct wireless_dev *p2p_wdev;
struct mutex p2p_wdev_mutex; /* protect @p2p_wdev */
struct wireless_dev *radio_wdev;
+
+ /* High Access Latency Policy voting */
+ struct wil_halp halp;
+
};
#define wil_to_wiphy(i) (i->wdev->wiphy)
@@ -720,6 +736,12 @@ void wil_memcpy_fromio_32(void *dst, const volatile void __iomem *src,
size_t count);
void wil_memcpy_toio_32(volatile void __iomem *dst, const void *src,
size_t count);
+void wil_memcpy_fromio_halp_vote(struct wil6210_priv *wil, void *dst,
+ const volatile void __iomem *src,
+ size_t count);
+void wil_memcpy_toio_halp_vote(struct wil6210_priv *wil,
+ volatile void __iomem *dst,
+ const void *src, size_t count);
void *wil_if_alloc(struct device *dev);
void wil_if_free(struct wil6210_priv *wil);
@@ -856,4 +878,9 @@ 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);
+void wil_halp_vote(struct wil6210_priv *wil);
+void wil_halp_unvote(struct wil6210_priv *wil);
+void wil6210_set_halp(struct wil6210_priv *wil);
+void wil6210_clear_halp(struct wil6210_priv *wil);
+
#endif /* __WIL6210_H__ */
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index 3cc4462..f83bde1 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -194,6 +194,7 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
void __iomem *dst;
void __iomem *head = wmi_addr(wil, r->head);
uint retry;
+ int rc = 0;
if (sizeof(cmd) + len > r->entry_size) {
wil_err(wil, "WMI size too large: %d bytes, max is %d\n",
@@ -212,6 +213,9 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
wil_err(wil, "WMI head is garbage: 0x%08x\n", r->head);
return -EINVAL;
}
+
+ wil_halp_vote(wil);
+
/* read Tx head till it is not busy */
for (retry = 5; retry > 0; retry--) {
wil_memcpy_fromio_32(&d_head, head, sizeof(d_head));
@@ -221,7 +225,8 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
}
if (d_head.sync != 0) {
wil_err(wil, "WMI head busy\n");
- return -EBUSY;
+ rc = -EBUSY;
+ goto out;
}
/* next head */
next_head = r->base + ((r->head - r->base + sizeof(d_head)) % r->size);
@@ -230,7 +235,8 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
for (retry = 5; retry > 0; retry--) {
if (!test_bit(wil_status_fwready, wil->status)) {
wil_err(wil, "WMI: cannot send command while FW not ready\n");
- return -EAGAIN;
+ rc = -EAGAIN;
+ goto out;
}
r->tail = wil_r(wil, RGF_MBOX +
offsetof(struct wil6210_mbox_ctl, tx.tail));
@@ -240,13 +246,15 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
}
if (next_head == r->tail) {
wil_err(wil, "WMI ring full\n");
- return -EBUSY;
+ rc = -EBUSY;
+ goto out;
}
dst = wmi_buffer(wil, d_head.addr);
if (!dst) {
wil_err(wil, "invalid WMI buffer: 0x%08x\n",
le32_to_cpu(d_head.addr));
- return -EINVAL;
+ rc = -EAGAIN;
+ goto out;
}
cmd.hdr.seq = cpu_to_le16(++wil->wmi_seq);
/* set command */
@@ -269,7 +277,9 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
wil_w(wil, RGF_USER_USER_ICR + offsetof(struct RGF_ICR, ICS),
SW_INT_MBOX);
- return 0;
+out:
+ wil_halp_unvote(wil);
+ return rc;
}
int wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
--
1.8.5.2
On 4/7/2016 6:41 PM, Kalle Valo wrote:
> "Haim, Maya" <[email protected]> writes:
>
>> On 4/6/2016 10:19 AM, Joe Perches wrote:
>>> On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
>>>> Add __func__ to all wil log macros for easier debugging.
>>> I think this is unnecessary and merely bloats code size.
>>> For all the _dbg calls, dynamic debug can add function names if
>>> desired.
>>>
>>> If really desired, I suggest changing the logging functions to use
>>> "%ps and __builtin_return_address(0)
>> I implemented it with __builtin_return_address(0) at first but found
>> its format less readable (e.g. wil_start_xmit+0x58/0x7e8).
> Will that work with inline functions and with functions which the
> compiler has optimised out?
>
That's a good point. I did a quick check and it doesn't work for inline or static functions - for such functions, the name of the calling function is printed.
We can either (1) use my initial implementation (2) add __func__ only to the wil_dbg_... macros or (3) revert this patch completely.
I find the addition of the function names very useful and since most of the code doesn't include it, it makes the analysis of issues less efficient.
Kalle - what is your say on that?
:כתב Kalle Valo, 2016-04-15 15:28 בתאריך
> "Haim, Maya" <[email protected]> writes:
>
>> On 4/7/2016 6:41 PM, Kalle Valo wrote:
>>> "Haim, Maya" <[email protected]> writes:
>>>
>>>> On 4/6/2016 10:19 AM, Joe Perches wrote:
>>>>> On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
>>>>>> Add __func__ to all wil log macros for easier debugging.
>>>>> I think this is unnecessary and merely bloats code size.
>>>>> For all the _dbg calls, dynamic debug can add function names if
>>>>> desired.
>>>>>
>>>>> If really desired, I suggest changing the logging functions to use
>>>>> "%ps and __builtin_return_address(0)
>>>> I implemented it with __builtin_return_address(0) at first but found
>>>> its format less readable (e.g. wil_start_xmit+0x58/0x7e8).
>>> Will that work with inline functions and with functions which the
>>> compiler has optimised out?
>>
>> That's a good point. I did a quick check and it doesn't work for
>> inline or static functions - for such functions, the name of the
>> calling function is printed.
>
> Thanks for checking.
>
>> We can either (1) use my initial implementation (2) add __func__ only
>> to the wil_dbg_... macros or (3) revert this patch completely. I find
>> the addition of the function names very useful and since most of the
>> code doesn't include it, it makes the analysis of issues less
>> efficient. Kalle - what is your say on that?
>
> I don't have any preference, it's up to you what you like most.
>
> One more possibility: in ath10k we have a kconfig option
> CONFIG_ATH10K_DEBUG to make it possible to disable all overhead from
> debug functionality, that would at least solve Joe's concern of extra
> memory usage.
I looked a bit more on the different options.
For the dbg functions, the function name can be added as part of the
dynamic debug options (as mentioned by Joe). Therefore I will remove
the addition of __func__ from those macros.
The dynamic debug also guarantees zero overhead so I don't see the need
for an additional debug config flag (such as CONFIG_ATH10K_DEBUG).
As for wil_err and wil_info, I believe we can add the __func__ to them,
as wil_info is not commonly used in our code.
When using interrupt moderation RX_HTRSH interrupt can occur
frequently during high throughput and should not be considered
as error.
Such print-outs can degrade the performance hence should be printed
in debug print level.
Signed-off-by: Maya Erez <[email protected]>
---
drivers/net/wireless/ath/wil6210/interrupt.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/interrupt.c b/drivers/net/wireless/ath/wil6210/interrupt.c
index fe66b2b..6897754 100644
--- a/drivers/net/wireless/ath/wil6210/interrupt.c
+++ b/drivers/net/wireless/ath/wil6210/interrupt.c
@@ -228,11 +228,8 @@ static irqreturn_t wil6210_irq_rx(int irq, void *cookie)
*/
if (likely(isr & (BIT_DMA_EP_RX_ICR_RX_DONE |
BIT_DMA_EP_RX_ICR_RX_HTRSH))) {
- wil_dbg_irq(wil, "RX done\n");
-
- if (unlikely(isr & BIT_DMA_EP_RX_ICR_RX_HTRSH))
- wil_err_ratelimited(wil,
- "Received \"Rx buffer is in risk of overflow\" interrupt\n");
+ wil_dbg_irq(wil, "RX done / RX_HTRSH received, ISR (0x%x)\n",
+ isr);
isr &= ~(BIT_DMA_EP_RX_ICR_RX_DONE |
BIT_DMA_EP_RX_ICR_RX_HTRSH);
--
1.8.5.2
On Sun, 2016-04-10 at 19:17 +0000, qca_merez wrote:
> On 4/6/2016 10:22 AM, Joe Perches wrote:
> >
> > On Tue, 2016-04-05 at 14:24 +0300, Maya Erez w
> > >
> > > +void __wil_dbg_ratelimited(struct wil6210_priv *wil, const char *fmt,?
> > > +...) {
> > > + if (net_ratelimit()) {
> >
> > Inverting the test would reduce indentation.
> I preferred to have the same implementation as in wil_err_ratelimited.
That's easy enough.
Maybe:
---
?drivers/net/wireless/ath/wil6210/debug.c???| 55 ++++++++++++++----------------
?drivers/net/wireless/ath/wil6210/wil6210.h |??8 ++---
?2 files changed, 30 insertions(+), 33 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debug.c b/drivers/net/wireless/ath/wil6210/debug.c
index 3249562..de1e932 100644
--- a/drivers/net/wireless/ath/wil6210/debug.c
+++ b/drivers/net/wireless/ath/wil6210/debug.c
@@ -17,61 +17,58 @@
?#include "wil6210.h"
?#include "trace.h"
?
-void wil_err(struct wil6210_priv *wil, const char *fmt, ...)
+void wil_err(const struct wil6210_priv *wil, const char *fmt, ...)
?{
- struct net_device *ndev = wil_to_ndev(wil);
- struct va_format vaf = {
- .fmt = fmt,
- };
+ struct va_format vaf;
? va_list args;
?
? va_start(args, fmt);
+ vaf.fmt = fmt;
? vaf.va = &args;
- netdev_err(ndev, "%pV", &vaf);
+ netdev_err(wil_to_ndev(wil), "%pV", &vaf);
? trace_wil6210_log_err(&vaf);
? va_end(args);
?}
?
-void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
+void wil_err_ratelimited(const 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;
+ struct va_format vaf;
+ va_list args;
+
+ if (!net_ratelimit())
+ return;
?
- va_start(args, fmt);
- vaf.va = &args;
- netdev_err(ndev, "%pV", &vaf);
- trace_wil6210_log_err(&vaf);
- va_end(args);
- }
+ va_start(args, fmt);
+ vaf.fmt = fmt;
+ vaf.va = &args;
+ netdev_err(wil_to_ndev(wil), "%pV", &vaf);
+ trace_wil6210_log_err(&vaf);
+ va_end(args);
?}
?
-void wil_info(struct wil6210_priv *wil, const char *fmt, ...)
+void wil_info(const struct wil6210_priv *wil, const char *fmt, ...)
?{
- struct net_device *ndev = wil_to_ndev(wil);
- struct va_format vaf = {
- .fmt = fmt,
- };
+ struct va_format vaf;
? va_list args;
?
+ if (!net_ratelimit())
+ return;
+
? va_start(args, fmt);
+ vaf.fmt = fmt;
? vaf.va = &args;
- netdev_info(ndev, "%pV", &vaf);
+ netdev_info(wil_to_ndev(wil), "%pV", &vaf);
? trace_wil6210_log_info(&vaf);
? va_end(args);
?}
?
-void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...)
+void wil_dbg_trace(const struct wil6210_priv *wil, const char *fmt, ...)
?{
- struct va_format vaf = {
- .fmt = fmt,
- };
+ struct va_format vaf;
? va_list args;
?
? va_start(args, fmt);
+ vaf.fmt = fmt;
? vaf.va = &args;
? trace_wil6210_log_dbg(&vaf);
? va_end(args);
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 4d699ea4..e2b62b1 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -633,13 +633,13 @@ struct wil6210_priv {
?#define ndev_to_wil(n) (wdev_to_wil(n->ieee80211_ptr))
?
?__printf(2, 3)
-void wil_dbg_trace(struct wil6210_priv *wil, const char *fmt, ...);
+void wil_dbg_trace(const struct wil6210_priv *wil, const char *fmt, ...);
?__printf(2, 3)
-void wil_err(struct wil6210_priv *wil, const char *fmt, ...);
+void wil_err(const struct wil6210_priv *wil, const char *fmt, ...);
?__printf(2, 3)
-void wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...);
+void wil_err_ratelimited(const struct wil6210_priv *wil, const char *fmt, ...);
?__printf(2, 3)
-void wil_info(struct wil6210_priv *wil, const char *fmt, ...);
+void wil_info(const struct wil6210_priv *wil, const char *fmt, ...);
?#define wil_dbg(wil, fmt, arg...) do { \
? netdev_dbg(wil_to_ndev(wil), fmt, ##arg); \
? wil_dbg_trace(wil, fmt, ##arg); \
On Tue, 2016-04-05 at 14:24 +0300, Maya Erez w
> Network stack can try to transmit data while AP / STA is
> disconnected.
> Change this print-out to debug level as this should not be
> handled as error.
Should probably say something about adding ratelimited
logging functions
> diff --git a/drivers/net/wireless/ath/wil6210/debug.c b/drivers/net/wireless/ath/wil6210/debug.c
[]
> @@ -49,6 +49,23 @@ void __wil_err_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
> ? }
> ?}
> ?
> +void __wil_dbg_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
> +{
> + if (net_ratelimit()) {
Inverting the test would reduce indentation.
> + 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_dbg(ndev, "%pV", &vaf);
> + trace_wil6210_log_dbg(&vaf);
> + va_end(args);
> + }
> +}