Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755482Ab0LARRK (ORCPT ); Wed, 1 Dec 2010 12:17:10 -0500 Received: from mail.perches.com ([173.55.12.10]:2048 "EHLO mail.perches.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754972Ab0LARRH (ORCPT ); Wed, 1 Dec 2010 12:17:07 -0500 Subject: Re: [ath9k-devel] [PATCH wireless-next] ath: Rename ath_print to ath_debug From: Joe Perches To: Felix Fietkau Cc: "Luis R. Rodriguez" , linux-wireless , ath9k-devel@venema.h4ckr.net, peter@stuge.se, linux-kernel@vger.kernel.org In-Reply-To: <4CF65DB9.3050007@openwrt.org> References: <5febb0e1fba0ec2bb77f6ade8b251ba0edf4614c.1290988277.git.joe@perches.com> <20101129060732.5130.qmail@stuge.se> <4CF42C17.2070500@openwrt.org> <1291081185.16349.133.camel@Joe-Laptop> <4CF464D6.2020507@openwrt.org> <1291148375.18026.342.camel@Joe-Laptop> <1291213675.1845.12.camel@Joe-Laptop> <4CF65DB9.3050007@openwrt.org> Content-Type: text/plain; charset="UTF-8" Date: Wed, 01 Dec 2010 09:17:05 -0800 Message-ID: <1291223825.1845.170.camel@Joe-Laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10078 Lines: 317 On Wed, 2010-12-01 at 15:37 +0100, Felix Fietkau wrote: > On 2010-12-01 3:27 PM, Joe Perches wrote: > > On Tue, 2010-11-30 at 23:56 -0800, Luis R. Rodriguez wrote: > >> On Tue, Nov 30, 2010 at 12:19 PM, Joe Perches wrote: > >> > Poor function naming is just that. > >> > It reduces readability and the uses are counter expectation. > >> The name is perfect, we use it to print anything, even non-debugging stuff. > > > > 'fraid not. > > > > ath/debug.h > > > > #ifdef CONFIG_ATH_DEBUG > > void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) > > __attribute__ ((format (printf, 3, 4))); > > #else > > static inline void __attribute__ ((format (printf, 3, 4))) > > ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) > > { > > } > > #endif /* CONFIG_ATH_DEBUG */ > Now we're getting closer to something worth fixing. IMHO we should > change the code so that ath_print(common, ATH_DBG_FATAL, ...) prints > something even with CONFIG_ATH_DEBUG unset. To get this done, some > renaming would make sense here. Perhaps the function name is bad after all if Luis believed it be be always active. If there are going to be other non-debug uses, I suggest adding the more standard styles of ath_printk and ath_ similar to dev_printk and dev_ from device.h Something like this for a start, then a more gradual rename of ath_print to ath_dbg (or ath_debug) as the original patch suggested. This basically removes debug.h leaving only an #define ath_printk ath_dbg there and moving all the ATH_DBG_ enums to ath.h I do not think there's much purpose for struct ath_common * being passed to the ath_printk functions, but perhaps there might be. Signed-off-by: Joe Perches --- drivers/net/wireless/ath/ath.h | 103 ++++++++++++++++++++++++++++++++++++++ drivers/net/wireless/ath/debug.c | 20 ------- drivers/net/wireless/ath/debug.h | 72 +-------------------------- drivers/net/wireless/ath/main.c | 20 +++++++ 4 files changed, 124 insertions(+), 91 deletions(-) diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h index 26bdbee..5a598b9 100644 --- a/drivers/net/wireless/ath/ath.h +++ b/drivers/net/wireless/ath/ath.h @@ -186,4 +186,107 @@ bool ath_hw_keyreset(struct ath_common *common, u16 entry); void ath_hw_cycle_counters_update(struct ath_common *common); int32_t ath_hw_get_listen_time(struct ath_common *common); +extern __attribute__ ((format (printf, 3, 4))) int +ath_printk(const char *level, struct ath_common *common, const char *fmt, ...); + +#define ath_emerg(common, fmt, ...) \ + ath_printk(KERN_EMERG, common, fmt, ##__VA_ARGS) +#define ath_alert(common, fmt, ...) \ + ath_printk(KERN_ALERT, common, fmt, ##__VA_ARGS) +#define ath_crit(common, fmt, ...) \ + ath_printk(KERN_CRIT, common, fmt, ##__VA_ARGS) +#define ath_err(common, fmt, ...) \ + ath_printk(KERN_ERR, common, fmt, ##__VA_ARGS) +#define ath_warn(common, fmt, ...) \ + ath_printk(KERN_WARNING, common, fmt, ##__VA_ARGS) +#define ath_notice(common, fmt, ...) \ + ath_printk(KERN_NOTICE, common, fmt, ##__VA_ARGS) +#define ath_info(common, fmt, ...) \ + ath_printk(KERN_INFO, common, fmt, ##__VA_ARGS) + +/** + * enum ath_debug_level - atheros wireless debug level + * + * @ATH_DBG_RESET: reset processing + * @ATH_DBG_QUEUE: hardware queue management + * @ATH_DBG_EEPROM: eeprom processing + * @ATH_DBG_CALIBRATE: periodic calibration + * @ATH_DBG_INTERRUPT: interrupt processing + * @ATH_DBG_REGULATORY: regulatory processing + * @ATH_DBG_ANI: adaptive noise immunitive processing + * @ATH_DBG_XMIT: basic xmit operation + * @ATH_DBG_BEACON: beacon handling + * @ATH_DBG_CONFIG: configuration of the hardware + * @ATH_DBG_FATAL: fatal errors, this is the default, DBG_DEFAULT + * @ATH_DBG_PS: power save processing + * @ATH_DBG_HWTIMER: hardware timer handling + * @ATH_DBG_BTCOEX: bluetooth coexistance + * @ATH_DBG_BSTUCK: stuck beacons + * @ATH_DBG_ANY: enable all debugging + * + * The debug level is used to control the amount and type of debugging output + * we want to see. Each driver has its own method for enabling debugging and + * modifying debug level states -- but this is typically done through a + * module parameter 'debug' along with a respective 'debug' debugfs file + * entry. + */ +enum ATH_DEBUG { + ATH_DBG_RESET = 0x00000001, + ATH_DBG_QUEUE = 0x00000002, + ATH_DBG_EEPROM = 0x00000004, + ATH_DBG_CALIBRATE = 0x00000008, + ATH_DBG_INTERRUPT = 0x00000010, + ATH_DBG_REGULATORY = 0x00000020, + ATH_DBG_ANI = 0x00000040, + ATH_DBG_XMIT = 0x00000080, + ATH_DBG_BEACON = 0x00000100, + ATH_DBG_CONFIG = 0x00000200, + ATH_DBG_FATAL = 0x00000400, + ATH_DBG_PS = 0x00000800, + ATH_DBG_HWTIMER = 0x00001000, + ATH_DBG_BTCOEX = 0x00002000, + ATH_DBG_WMI = 0x00004000, + ATH_DBG_BSTUCK = 0x00008000, + ATH_DBG_ANY = 0xffffffff +}; + +#define ATH_DBG_DEFAULT (ATH_DBG_FATAL) + +#ifdef CONFIG_ATH_DEBUG + +#define ath_dbg(common, dbg_mask, fmt, ...) \ +({ \ + int rtn; \ + if ((common)->debug_mask & dbg_mask) \ + rtn = ath_printk(KERN_DEBUG, common, fmt, \ + ##__VA_ARGS__); \ + else \ + rtn = 0; \ + \ + rtn; \ +}) +#define ATH_DBG_WARN(foo, arg...) WARN(foo, arg) + +#else + +static inline __attribute__ ((format (printf, 3, 4))) int +ath_dbg(struct ath_common *common, enum ATH_DEBUG dbg_mask, + const char *fmt, ...) +{ + return 0; +} +#define ATH_DBG_WARN(foo, arg) do {} while (0) + +#endif /* CONFIG_ATH_DEBUG */ + +/** Returns string describing opmode, or NULL if unknown mode. */ +#ifdef CONFIG_ATH_DEBUG +const char *ath_opmode_to_string(enum nl80211_iftype opmode); +#else +static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode) +{ + return "UNKNOWN"; +} +#endif + #endif /* ATH_H */ diff --git a/drivers/net/wireless/ath/debug.c b/drivers/net/wireless/ath/debug.c index a9600ba..5367b10 100644 --- a/drivers/net/wireless/ath/debug.c +++ b/drivers/net/wireless/ath/debug.c @@ -15,26 +15,6 @@ */ #include "ath.h" -#include "debug.h" - -void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) -{ - struct va_format vaf; - va_list args; - - if (likely(!(common->debug_mask & dbg_mask))) - return; - - va_start(args, fmt); - - vaf.fmt = fmt; - vaf.va = &args; - - printk(KERN_DEBUG "ath: %pV", &vaf); - - va_end(args); -} -EXPORT_SYMBOL(ath_print); const char *ath_opmode_to_string(enum nl80211_iftype opmode) { diff --git a/drivers/net/wireless/ath/debug.h b/drivers/net/wireless/ath/debug.h index f207007..cec951c 100644 --- a/drivers/net/wireless/ath/debug.h +++ b/drivers/net/wireless/ath/debug.h @@ -17,76 +17,6 @@ #ifndef ATH_DEBUG_H #define ATH_DEBUG_H -#include "ath.h" - -/** - * enum ath_debug_level - atheros wireless debug level - * - * @ATH_DBG_RESET: reset processing - * @ATH_DBG_QUEUE: hardware queue management - * @ATH_DBG_EEPROM: eeprom processing - * @ATH_DBG_CALIBRATE: periodic calibration - * @ATH_DBG_INTERRUPT: interrupt processing - * @ATH_DBG_REGULATORY: regulatory processing - * @ATH_DBG_ANI: adaptive noise immunitive processing - * @ATH_DBG_XMIT: basic xmit operation - * @ATH_DBG_BEACON: beacon handling - * @ATH_DBG_CONFIG: configuration of the hardware - * @ATH_DBG_FATAL: fatal errors, this is the default, DBG_DEFAULT - * @ATH_DBG_PS: power save processing - * @ATH_DBG_HWTIMER: hardware timer handling - * @ATH_DBG_BTCOEX: bluetooth coexistance - * @ATH_DBG_BSTUCK: stuck beacons - * @ATH_DBG_ANY: enable all debugging - * - * The debug level is used to control the amount and type of debugging output - * we want to see. Each driver has its own method for enabling debugging and - * modifying debug level states -- but this is typically done through a - * module parameter 'debug' along with a respective 'debug' debugfs file - * entry. - */ -enum ATH_DEBUG { - ATH_DBG_RESET = 0x00000001, - ATH_DBG_QUEUE = 0x00000002, - ATH_DBG_EEPROM = 0x00000004, - ATH_DBG_CALIBRATE = 0x00000008, - ATH_DBG_INTERRUPT = 0x00000010, - ATH_DBG_REGULATORY = 0x00000020, - ATH_DBG_ANI = 0x00000040, - ATH_DBG_XMIT = 0x00000080, - ATH_DBG_BEACON = 0x00000100, - ATH_DBG_CONFIG = 0x00000200, - ATH_DBG_FATAL = 0x00000400, - ATH_DBG_PS = 0x00000800, - ATH_DBG_HWTIMER = 0x00001000, - ATH_DBG_BTCOEX = 0x00002000, - ATH_DBG_WMI = 0x00004000, - ATH_DBG_BSTUCK = 0x00008000, - ATH_DBG_ANY = 0xffffffff -}; - -#define ATH_DBG_DEFAULT (ATH_DBG_FATAL) - -#ifdef CONFIG_ATH_DEBUG -void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) - __attribute__ ((format (printf, 3, 4))); -#define ATH_DBG_WARN(foo, arg...) WARN(foo, arg) -#else -static inline void __attribute__ ((format (printf, 3, 4))) -ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) -{ -} -#define ATH_DBG_WARN(foo, arg) -#endif /* CONFIG_ATH_DEBUG */ - -/** Returns string describing opmode, or NULL if unknown mode. */ -#ifdef CONFIG_ATH_DEBUG -const char *ath_opmode_to_string(enum nl80211_iftype opmode); -#else -static inline const char *ath_opmode_to_string(enum nl80211_iftype opmode) -{ - return "UNKNOWN"; -} -#endif +#define ath_print ath_dbg #endif /* ATH_DEBUG_H */ diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c index 487193f..c325202 100644 --- a/drivers/net/wireless/ath/main.c +++ b/drivers/net/wireless/ath/main.c @@ -56,3 +56,23 @@ struct sk_buff *ath_rxbuf_alloc(struct ath_common *common, return skb; } EXPORT_SYMBOL(ath_rxbuf_alloc); + +int ath_printk(const char *level, struct ath_common *common, + const char *fmt, ...) +{ + struct va_format vaf; + va_list args; + int rtn; + + va_start(args, fmt); + + vaf.fmt = fmt; + vaf.va = &args; + + rtn = printk("%sath: %pV", level, &vaf); + + va_end(args); + + return rtn; +} +EXPORT_SYMBOL(ath_printk); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/