| Message ID | 20170510162546.15439-1-adrian@freebsd.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
Hi Adrian, On Wed, May 10, 2017 at 9:25 AM, Adrian Chadd <adrian@freebsd.org> wrote: > diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h > index 257d10985c6e..7bd461927029 100644 > --- a/drivers/net/wireless/ath/ath10k/debug.h > +++ b/drivers/net/wireless/ath/ath10k/debug.h > @@ -200,27 +200,43 @@ void ath10k_sta_update_rx_duration(struct ath10k *ar, > #endif /* CONFIG_MAC80211_DEBUGFS */ > > #ifdef CONFIG_ATH10K_DEBUG > -__printf(3, 4) void ath10k_dbg(struct ath10k *ar, > +static inline int > +_ath10k_do_dbg(struct ath10k *ar, enum ath10k_debug_mask mask) > +{ > + if (ar->trace_debug_mask & mask) > + return (1); > + if (ar->debug_mask & mask) > + return (1); > + return (0); > +} > + > +void _ath10k_dbg(struct ath10k *ar, > enum ath10k_debug_mask mask, > const char *fmt, ...); > -void ath10k_dbg_dump(struct ath10k *ar, > + > +void _ath10k_dbg_dump(struct ath10k *ar, > enum ath10k_debug_mask mask, > const char *msg, const char *prefix, > const void *buf, size_t len); > + > +#define ath10k_dbg(ar, mask, ...) \ > + do { \ > + if (_ath10k_do_dbg(ar, mask)) { \ > + _ath10k_dbg((ar), (mask), __VA_ARGS__); \ > + }; \ > + } while (0) > + Looks to me you dropped the "__printf(3, 4)" safety check. Was that intentional? Thanks, - Steve
grr, no. lemme go re-add that and resubmit. thanks! -a On 10 May 2017 at 09:44, Steve deRosier <derosier@gmail.com> wrote: > Hi Adrian, > > On Wed, May 10, 2017 at 9:25 AM, Adrian Chadd <adrian@freebsd.org> wrote: > >> diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h >> index 257d10985c6e..7bd461927029 100644 >> --- a/drivers/net/wireless/ath/ath10k/debug.h >> +++ b/drivers/net/wireless/ath/ath10k/debug.h >> @@ -200,27 +200,43 @@ void ath10k_sta_update_rx_duration(struct ath10k *ar, >> #endif /* CONFIG_MAC80211_DEBUGFS */ >> >> #ifdef CONFIG_ATH10K_DEBUG >> -__printf(3, 4) void ath10k_dbg(struct ath10k *ar, >> +static inline int >> +_ath10k_do_dbg(struct ath10k *ar, enum ath10k_debug_mask mask) >> +{ >> + if (ar->trace_debug_mask & mask) >> + return (1); >> + if (ar->debug_mask & mask) >> + return (1); >> + return (0); >> +} >> + >> +void _ath10k_dbg(struct ath10k *ar, >> enum ath10k_debug_mask mask, >> const char *fmt, ...); >> -void ath10k_dbg_dump(struct ath10k *ar, >> + >> +void _ath10k_dbg_dump(struct ath10k *ar, >> enum ath10k_debug_mask mask, >> const char *msg, const char *prefix, >> const void *buf, size_t len); >> + >> +#define ath10k_dbg(ar, mask, ...) \ >> + do { \ >> + if (_ath10k_do_dbg(ar, mask)) { \ >> + _ath10k_dbg((ar), (mask), __VA_ARGS__); \ >> + }; \ >> + } while (0) >> + > > Looks to me you dropped the "__printf(3, 4)" safety check. Was that intentional? > > Thanks, > - Steve
Adrian Chadd <adrian@freebsd.org> writes: > This adds a few configurable debugging options: > > * driver debugging and tracing is now configurable per device > * driver debugging and tracing is now configurable at runtime > * the debugging / tracing is not run at all (besides a mask check) > unless the specific debugging bitmap field is configured. > > Signed-off-by: Adrian Chadd <adrian@FreeBSD.org> ath10k-check found some trivial whitespace problems: drivers/net/wireless/ath/ath10k/debug.h:207: return is not a function, parentheses are not required drivers/net/wireless/ath/ath10k/debug.h:209: return is not a function, parentheses are not required drivers/net/wireless/ath/ath10k/debug.h:210: return is not a function, parentheses are not required drivers/net/wireless/ath/ath10k/debug.h:214: Alignment should match open parenthesis drivers/net/wireless/ath/ath10k/debug.h:218: Alignment should match open parenthesis drivers/net/wireless/ath/ath10k/debug.c:2430: code indent should use tabs where possible drivers/net/wireless/ath/ath10k/debug.c:2430: please, no spaces at the start of a line drivers/net/wireless/ath/ath10k/debug.c:2431: code indent should use tabs where possible drivers/net/wireless/ath/ath10k/debug.c:2431: please, no spaces at the start of a line drivers/net/wireless/ath/ath10k/debug.c:2464: code indent should use tabs where possible drivers/net/wireless/ath/ath10k/debug.c:2464: please, no spaces at the start of a line drivers/net/wireless/ath/ath10k/debug.c:2465: code indent should use tabs where possible drivers/net/wireless/ath/ath10k/debug.c:2465: please, no spaces at the start of a line drivers/net/wireless/ath/ath10k/debug.c:2493: Please don't use multiple blank lines drivers/net/wireless/ath/ath10k/debug.c:2525: Symbolic permissions 'S_IRUSR' are not preferred. Consider using octal permissions '0400'. drivers/net/wireless/ath/ath10k/debug.c:2527: Symbolic permissions 'S_IRUSR' are not preferred. Consider using octal permissions '0400'. drivers/net/wireless/ath/ath10k/debug.c:2620: Alignment should match open parenthesis drivers/net/wireless/ath/ath10k/debug.c:2640: Alignment should match open parenthesis I fixed those in the pending branch: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=bd8c3bdce70adc201037b2eb7eda0a83911ef375 I'll look at this more closely later.
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index eea111d704c5..fcb068cb0248 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -2444,6 +2444,8 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev, ar->hw_rev = hw_rev; ar->hif.ops = hif_ops; ar->hif.bus = bus; + ar->debug_mask = ath10k_debug_mask; + ar->trace_debug_mask = ath10k_debug_mask; switch (hw_rev) { case ATH10K_HW_QCA988X: diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index 8fc08a5043db..07e392a377d0 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -762,6 +762,8 @@ struct ath10k { struct device *dev; u8 mac_addr[ETH_ALEN]; + u32 debug_mask; + u32 trace_debug_mask; enum ath10k_hw_rev hw_rev; u16 dev_id; u32 chip_id; diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index 389fcb7a9fd0..017360a26b40 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -2418,6 +2418,79 @@ int ath10k_debug_create(struct ath10k *ar) return 0; } +#ifdef CONFIG_ATH10K_DEBUGFS +static ssize_t ath10k_write_debug_mask(struct file *file, + const char __user *ubuf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + int ret; + u32 val; + + if (kstrtou32_from_user(ubuf, count, 0, &val)) + return -EINVAL; + + ar->debug_mask = val; + ret = count; + + return ret; +} + +static ssize_t ath10k_read_debug_mask(struct file *file, char __user *ubuf, + size_t count, loff_t *ppos) +{ + char buf[32]; + struct ath10k *ar = file->private_data; + int len = 0; + + len = scnprintf(buf, sizeof(buf) - len, "0x%x\n", ar->debug_mask); + return simple_read_from_buffer(ubuf, count, ppos, buf, len); +} + +static const struct file_operations fops_debug_mask = { + .read = ath10k_read_debug_mask, + .write = ath10k_write_debug_mask, + .open = simple_open +}; + +static ssize_t ath10k_write_trace_debug_mask(struct file *file, + const char __user *ubuf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + int ret; + u32 val; + + if (kstrtou32_from_user(ubuf, count, 0, &val)) + return -EINVAL; + + ar->trace_debug_mask = val; + ret = count; + + return ret; +} + +static ssize_t ath10k_read_trace_debug_mask(struct file *file, + char __user *ubuf, + size_t count, loff_t *ppos) +{ + char buf[32]; + struct ath10k *ar = file->private_data; + int len = 0; + + len = scnprintf(buf, sizeof(buf) - len, "0x%x\n", + ar->trace_debug_mask); + return simple_read_from_buffer(ubuf, count, ppos, buf, len); +} + +static const struct file_operations fops_trace_debug_mask = { + .read = ath10k_read_trace_debug_mask, + .write = ath10k_write_trace_debug_mask, + .open = simple_open +}; +#endif /* CONFIG_ATH10K_DEBUGFS */ + + void ath10k_debug_destroy(struct ath10k *ar) { vfree(ar->debug.fw_crash_data); @@ -2448,6 +2521,13 @@ int ath10k_debug_register(struct ath10k *ar) init_completion(&ar->debug.tpc_complete); init_completion(&ar->debug.fw_stats_complete); +#ifdef CONFIG_ATH10K_DEBUG + debugfs_create_file("debug", S_IRUSR, ar->debug.debugfs_phy, ar, + &fops_debug_mask); + debugfs_create_file("trace_debug", S_IRUSR, ar->debug.debugfs_phy, ar, + &fops_trace_debug_mask); +#endif + debugfs_create_file("fw_stats", 0400, ar->debug.debugfs_phy, ar, &fops_fw_stats); @@ -2536,7 +2616,7 @@ void ath10k_debug_unregister(struct ath10k *ar) #endif /* CONFIG_ATH10K_DEBUGFS */ #ifdef CONFIG_ATH10K_DEBUG -void ath10k_dbg(struct ath10k *ar, enum ath10k_debug_mask mask, +void _ath10k_dbg(struct ath10k *ar, enum ath10k_debug_mask mask, const char *fmt, ...) { struct va_format vaf; @@ -2547,16 +2627,16 @@ void ath10k_dbg(struct ath10k *ar, enum ath10k_debug_mask mask, vaf.fmt = fmt; vaf.va = &args; - if (ath10k_debug_mask & mask) + if (ar->debug_mask & mask) dev_printk(KERN_DEBUG, ar->dev, "%pV", &vaf); - - trace_ath10k_log_dbg(ar, mask, &vaf); + if (ar->trace_debug_mask & mask) + trace_ath10k_log_dbg(ar, mask, &vaf); va_end(args); } -EXPORT_SYMBOL(ath10k_dbg); +EXPORT_SYMBOL(_ath10k_dbg); -void ath10k_dbg_dump(struct ath10k *ar, +void _ath10k_dbg_dump(struct ath10k *ar, enum ath10k_debug_mask mask, const char *msg, const char *prefix, const void *buf, size_t len) @@ -2565,7 +2645,7 @@ void ath10k_dbg_dump(struct ath10k *ar, size_t linebuflen; const void *ptr; - if (ath10k_debug_mask & mask) { + if (ar->debug_mask & mask) { if (msg) ath10k_dbg(ar, mask, "%s\n", msg); @@ -2584,9 +2664,10 @@ void ath10k_dbg_dump(struct ath10k *ar, } /* tracing code doesn't like null strings :/ */ - trace_ath10k_log_dbg_dump(ar, msg ? msg : "", prefix ? prefix : "", - buf, len); + if (ar->trace_debug_mask & mask) + trace_ath10k_log_dbg_dump(ar, msg ? msg : "", prefix ? prefix : "", + buf, len); } -EXPORT_SYMBOL(ath10k_dbg_dump); +EXPORT_SYMBOL(_ath10k_dbg_dump); #endif /* CONFIG_ATH10K_DEBUG */ diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h index 257d10985c6e..7bd461927029 100644 --- a/drivers/net/wireless/ath/ath10k/debug.h +++ b/drivers/net/wireless/ath/ath10k/debug.h @@ -200,27 +200,43 @@ void ath10k_sta_update_rx_duration(struct ath10k *ar, #endif /* CONFIG_MAC80211_DEBUGFS */ #ifdef CONFIG_ATH10K_DEBUG -__printf(3, 4) void ath10k_dbg(struct ath10k *ar, +static inline int +_ath10k_do_dbg(struct ath10k *ar, enum ath10k_debug_mask mask) +{ + if (ar->trace_debug_mask & mask) + return (1); + if (ar->debug_mask & mask) + return (1); + return (0); +} + +void _ath10k_dbg(struct ath10k *ar, enum ath10k_debug_mask mask, const char *fmt, ...); -void ath10k_dbg_dump(struct ath10k *ar, + +void _ath10k_dbg_dump(struct ath10k *ar, enum ath10k_debug_mask mask, const char *msg, const char *prefix, const void *buf, size_t len); + +#define ath10k_dbg(ar, mask, ...) \ + do { \ + if (_ath10k_do_dbg(ar, mask)) { \ + _ath10k_dbg((ar), (mask), __VA_ARGS__); \ + }; \ + } while (0) + +#define ath10k_dbg_dump(ar, mask, msg, pfx, buf, len) \ + do { \ + if (_ath10k_do_dbg(ar, mask)) { \ + _ath10k_dbg_dump((ar), (mask), (msg), (pfx), (buf), (len)); \ + }; \ + } while (0) + #else /* CONFIG_ATH10K_DEBUG */ -static inline int ath10k_dbg(struct ath10k *ar, - enum ath10k_debug_mask dbg_mask, - const char *fmt, ...) -{ - return 0; -} +#define ath10k_dbg(ar, mask, fmt, ...) +#define ath10k_dbg_dump(ar, mask, msg, pfx, buf, len) -static inline void ath10k_dbg_dump(struct ath10k *ar, - enum ath10k_debug_mask mask, - const char *msg, const char *prefix, - const void *buf, size_t len) -{ -} #endif /* CONFIG_ATH10K_DEBUG */ #endif /* _DEBUG_H_ */
This adds a few configurable debugging options: * driver debugging and tracing is now configurable per device * driver debugging and tracing is now configurable at runtime * the debugging / tracing is not run at all (besides a mask check) unless the specific debugging bitmap field is configured. Signed-off-by: Adrian Chadd <adrian@FreeBSD.org> --- drivers/net/wireless/ath/ath10k/core.c | 2 + drivers/net/wireless/ath/ath10k/core.h | 2 + drivers/net/wireless/ath/ath10k/debug.c | 101 ++++++++++++++++++++++++++++---- drivers/net/wireless/ath/ath10k/debug.h | 44 +++++++++----- 4 files changed, 125 insertions(+), 24 deletions(-)