* [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" [not found] <20101012163519.a88f2040.sfr@canb.auug.org.au> @ 2010-10-12 14:53 ` John W. Linville 2010-10-12 15:43 ` Joe Perches ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: John W. Linville @ 2010-10-12 14:53 UTC (permalink / raw) To: linux-wireless Cc: Stephen Rothwell, Ben Greear, linux-kernel, John W. Linville Without this and with CONFIG_ATH_DEBUG not set... ERROR: ".ath_opmode_to_string" [drivers/net/wireless/ath/ath5k/ath5k.ko] undefined! Signed-off-by: John W. Linville <linville@tuxdriver.com> --- Please don't make me commit such an ugly patch... drivers/net/wireless/ath/ath5k/base.c | 4 ++++ drivers/net/wireless/ath/ath5k/debug.c | 6 +++++- 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c index c9732a6..b839a6b 100644 --- a/drivers/net/wireless/ath/ath5k/base.c +++ b/drivers/net/wireless/ath/ath5k/base.c @@ -562,10 +562,14 @@ static void ath_do_set_opmode(struct ath5k_softc *sc) { struct ath5k_hw *ah = sc->ah; ath5k_hw_set_opmode(ah, sc->opmode); +#ifdef CONFIG_ATH_DEBUG ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d (%s)\n", sc->opmode, ath_opmode_to_string(sc->opmode) ? ath_opmode_to_string(sc->opmode) : "UKNOWN"); +#else + ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d\n", sc->opmode); +#endif } void ath5k_update_bssid_mask_and_opmode(struct ath5k_softc *sc, diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c index a3b2171..329187a 100644 --- a/drivers/net/wireless/ath/ath5k/debug.c +++ b/drivers/net/wireless/ath/ath5k/debug.c @@ -493,7 +493,7 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, char buf[700]; unsigned int len = 0; u32 filt = ath5k_hw_get_rx_filter(sc->ah); - const char *tmp; + const char *tmp __maybe_unused; len += snprintf(buf+len, sizeof(buf)-len, "bssid-mask: %pM\n", sc->bssidmask); @@ -526,6 +526,7 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, else len += snprintf(buf+len, sizeof(buf)-len, "\n"); +#ifdef CONFIG_ATH_DEBUG tmp = ath_opmode_to_string(sc->opmode); if (tmp) len += snprintf(buf+len, sizeof(buf)-len, "opmode: %s\n", @@ -533,6 +534,9 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, else len += snprintf(buf+len, sizeof(buf)-len, "opmode: UNKNOWN-%i\n", sc->opmode); +#else + len += snprintf(buf+len, sizeof(buf)-len, "opmode: %i\n", sc->opmode); +#endif if (len > sizeof(buf)) len = sizeof(buf); -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 14:53 ` [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" John W. Linville @ 2010-10-12 15:43 ` Joe Perches 2010-10-12 17:15 ` John W. Linville 2010-10-12 16:54 ` Bob Copeland 2010-10-12 18:13 ` Linus Torvalds 2 siblings, 1 reply; 9+ messages in thread From: Joe Perches @ 2010-10-12 15:43 UTC (permalink / raw) To: John W. Linville Cc: linux-wireless, Stephen Rothwell, Ben Greear, linux-kernel On Tue, 2010-10-12 at 10:53 -0400, John W. Linville wrote: > Without this and with CONFIG_ATH_DEBUG not set... > ERROR: ".ath_opmode_to_string" [drivers/net/wireless/ath/ath5k/ath5k.ko] undefined! Perhaps this instead? Signed-off-by: Joe Perches <joe@perches.com> --- drivers/net/wireless/ath/debug.h | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/ath/debug.h b/drivers/net/wireless/ath/debug.h index a3a5a62..5013045 100644 --- a/drivers/net/wireless/ath/debug.h +++ b/drivers/net/wireless/ath/debug.h @@ -78,6 +78,13 @@ ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) #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 NULL; +} +#endif #endif /* ATH_DEBUG_H */ ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 15:43 ` Joe Perches @ 2010-10-12 17:15 ` John W. Linville 0 siblings, 0 replies; 9+ messages in thread From: John W. Linville @ 2010-10-12 17:15 UTC (permalink / raw) To: Joe Perches; +Cc: linux-wireless, Stephen Rothwell, Ben Greear, linux-kernel On Tue, Oct 12, 2010 at 08:43:24AM -0700, Joe Perches wrote: > On Tue, 2010-10-12 at 10:53 -0400, John W. Linville wrote: > > Without this and with CONFIG_ATH_DEBUG not set... > > ERROR: ".ath_opmode_to_string" [drivers/net/wireless/ath/ath5k/ath5k.ko] undefined! > > Perhaps this instead? Thanks, I like yours better. :-) -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 14:53 ` [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" John W. Linville 2010-10-12 15:43 ` Joe Perches @ 2010-10-12 16:54 ` Bob Copeland 2010-10-12 17:42 ` Bob Copeland 2010-10-12 18:13 ` Linus Torvalds 2 siblings, 1 reply; 9+ messages in thread From: Bob Copeland @ 2010-10-12 16:54 UTC (permalink / raw) To: John W. Linville Cc: linux-wireless, Stephen Rothwell, Ben Greear, linux-kernel On Tue, Oct 12, 2010 at 10:53 AM, John W. Linville <linville@tuxdriver.com> wrote: > Without this and with CONFIG_ATH_DEBUG not set... > > ERROR: ".ath_opmode_to_string" [drivers/net/wireless/ath/ath5k/ath5k.ko] undefined! > > Signed-off-by: John W. Linville <linville@tuxdriver.com> > --- > Please don't make me commit such an ugly patch... > > drivers/net/wireless/ath/ath5k/base.c | 4 ++++ > drivers/net/wireless/ath/ath5k/debug.c | 6 +++++- > 2 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c > index c9732a6..b839a6b 100644 > --- a/drivers/net/wireless/ath/ath5k/base.c > +++ b/drivers/net/wireless/ath/ath5k/base.c > @@ -562,10 +562,14 @@ static void ath_do_set_opmode(struct ath5k_softc *sc) > { > struct ath5k_hw *ah = sc->ah; > ath5k_hw_set_opmode(ah, sc->opmode); > +#ifdef CONFIG_ATH_DEBUG > ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d (%s)\n", > sc->opmode, > ath_opmode_to_string(sc->opmode) ? > ath_opmode_to_string(sc->opmode) : "UKNOWN"); >From the original patch: UKNOWN? I guess making ath_opmode_to_string always around (no-op in non-debug case) would also suffice. (reads on...) ah, yes, Joe's patch does this. -- Bob Copeland %% www.bobcopeland.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 16:54 ` Bob Copeland @ 2010-10-12 17:42 ` Bob Copeland 2010-10-12 18:07 ` Joe Perches 0 siblings, 1 reply; 9+ messages in thread From: Bob Copeland @ 2010-10-12 17:42 UTC (permalink / raw) To: John W. Linville Cc: linux-wireless, Stephen Rothwell, Ben Greear, linux-kernel On Tue, Oct 12, 2010 at 12:54 PM, Bob Copeland <me@bobcopeland.com> wrote: >> +#ifdef CONFIG_ATH_DEBUG >> ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d (%s)\n", >> sc->opmode, >> ath_opmode_to_string(sc->opmode) ? >> ath_opmode_to_string(sc->opmode) : "UKNOWN"); Just one other bikeshed color: if ath_opmode_to_string() returned "UNKNOWN" in the failure case, we wouldn't need the double-call above. Yes, it's used by ath9k too, but IMHO a debugging function that only converts some of the possible values to strings is pretty developer unfriendly. That said, I still think tracepoints are a nicer way to do this kind of thing (you can even do symbolic names for enums). -- Bob Copeland %% www.bobcopeland.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 17:42 ` Bob Copeland @ 2010-10-12 18:07 ` Joe Perches 0 siblings, 0 replies; 9+ messages in thread From: Joe Perches @ 2010-10-12 18:07 UTC (permalink / raw) To: Bob Copeland Cc: John W. Linville, linux-wireless, Stephen Rothwell, Ben Greear, linux-kernel On Tue, 2010-10-12 at 13:42 -0400, Bob Copeland wrote: > On Tue, Oct 12, 2010 at 12:54 PM, Bob Copeland <me@bobcopeland.com> wrote: > >> +#ifdef CONFIG_ATH_DEBUG > >> ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d (%s)\n", > >> sc->opmode, > >> ath_opmode_to_string(sc->opmode) ? > >> ath_opmode_to_string(sc->opmode) : "UKNOWN"); > > Just one other bikeshed color: > > if ath_opmode_to_string() returned "UNKNOWN" in the failure case, > we wouldn't need the double-call above. Yes, it's used by ath9k > too, but IMHO a debugging function that only converts some of the > possible values to strings is pretty developer unfriendly. Like this better? Signed-off-by: Joe Perches <joe@perches.com> --- drivers/net/wireless/ath/ath5k/base.c | 4 +--- drivers/net/wireless/ath/ath5k/debug.c | 16 ++++------------ drivers/net/wireless/ath/debug.c | 2 +- drivers/net/wireless/ath/debug.h | 7 +++++++ 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c index c9732a6..7baaf04 100644 --- a/drivers/net/wireless/ath/ath5k/base.c +++ b/drivers/net/wireless/ath/ath5k/base.c @@ -563,9 +563,7 @@ static void ath_do_set_opmode(struct ath5k_softc *sc) struct ath5k_hw *ah = sc->ah; ath5k_hw_set_opmode(ah, sc->opmode); ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d (%s)\n", - sc->opmode, - ath_opmode_to_string(sc->opmode) ? - ath_opmode_to_string(sc->opmode) : "UKNOWN"); + sc->opmode, ath_opmode_to_string(sc->opmode)); } void ath5k_update_bssid_mask_and_opmode(struct ath5k_softc *sc, diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c index a3b2171..7c77e5b7 100644 --- a/drivers/net/wireless/ath/ath5k/debug.c +++ b/drivers/net/wireless/ath/ath5k/debug.c @@ -493,7 +493,6 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, char buf[700]; unsigned int len = 0; u32 filt = ath5k_hw_get_rx_filter(sc->ah); - const char *tmp; len += snprintf(buf+len, sizeof(buf)-len, "bssid-mask: %pM\n", sc->bssidmask); @@ -522,17 +521,10 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, if (filt & AR5K_RX_FILTER_PHYERR_5211) snprintf(buf+len, sizeof(buf)-len, " PHYERR-5211"); if (filt & AR5K_RX_FILTER_RADARERR_5211) - len += snprintf(buf+len, sizeof(buf)-len, " RADARERR-5211\n"); - else - len += snprintf(buf+len, sizeof(buf)-len, "\n"); - - tmp = ath_opmode_to_string(sc->opmode); - if (tmp) - len += snprintf(buf+len, sizeof(buf)-len, "opmode: %s\n", - tmp); - else - len += snprintf(buf+len, sizeof(buf)-len, - "opmode: UNKNOWN-%i\n", sc->opmode); + len += snprintf(buf+len, sizeof(buf)-len, " RADARERR-5211"); + + len += snprintf(buf+len, sizeof(buf)-len, "\nopmode: %s (%d)\n", + ath_opmode_to_string(sc->opmode), sc->opmode); if (len > sizeof(buf)) len = sizeof(buf); diff --git a/drivers/net/wireless/ath/debug.c b/drivers/net/wireless/ath/debug.c index a9eb787..dacfb23 100644 --- a/drivers/net/wireless/ath/debug.c +++ b/drivers/net/wireless/ath/debug.c @@ -55,7 +55,7 @@ const char *ath_opmode_to_string(enum nl80211_iftype opmode) case NL80211_IFTYPE_P2P_GO: return "P2P-GO"; default: - return NULL; + return "UNKNOWN"; } } EXPORT_SYMBOL(ath_opmode_to_string); diff --git a/drivers/net/wireless/ath/debug.h b/drivers/net/wireless/ath/debug.h index a3a5a62..64e4af2 100644 --- a/drivers/net/wireless/ath/debug.h +++ b/drivers/net/wireless/ath/debug.h @@ -78,6 +78,13 @@ ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) #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_DEBUG_H */ ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 14:53 ` [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" John W. Linville 2010-10-12 15:43 ` Joe Perches 2010-10-12 16:54 ` Bob Copeland @ 2010-10-12 18:13 ` Linus Torvalds 2010-10-12 18:18 ` John W. Linville 2 siblings, 1 reply; 9+ messages in thread From: Linus Torvalds @ 2010-10-12 18:13 UTC (permalink / raw) To: John W. Linville Cc: linux-wireless, Stephen Rothwell, Ben Greear, linux-kernel On Tue, Oct 12, 2010 at 7:53 AM, John W. Linville <linville@tuxdriver.com> wrote: > > Please don't make me commit such an ugly patch... Why don't you just create a dummy inline ath_opmode_to_string() that returns NULL in the non-debug case? The compiler will make the conditionals all go away, and it will end up being "UNKNOWN" which is just what you want. No? Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 18:13 ` Linus Torvalds @ 2010-10-12 18:18 ` John W. Linville 2010-10-12 23:31 ` Ben Greear 0 siblings, 1 reply; 9+ messages in thread From: John W. Linville @ 2010-10-12 18:18 UTC (permalink / raw) To: Linus Torvalds; +Cc: linux-wireless, Stephen Rothwell, Ben Greear, linux-kernel On Tue, Oct 12, 2010 at 11:13:31AM -0700, Linus Torvalds wrote: > On Tue, Oct 12, 2010 at 7:53 AM, John W. Linville > <linville@tuxdriver.com> wrote: > > > > Please don't make me commit such an ugly patch... > > Why don't you just create a dummy inline ath_opmode_to_string() that > returns NULL in the non-debug case? The compiler will make the > conditionals all go away, and it will end up being "UNKNOWN" which is > just what you want. No? Shhh! Your messing-up my manipulative mind games! :-) John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" 2010-10-12 18:18 ` John W. Linville @ 2010-10-12 23:31 ` Ben Greear 0 siblings, 0 replies; 9+ messages in thread From: Ben Greear @ 2010-10-12 23:31 UTC (permalink / raw) To: John W. Linville Cc: Linus Torvalds, linux-wireless, Stephen Rothwell, linux-kernel On 10/12/2010 11:18 AM, John W. Linville wrote: > On Tue, Oct 12, 2010 at 11:13:31AM -0700, Linus Torvalds wrote: >> On Tue, Oct 12, 2010 at 7:53 AM, John W. Linville >> <linville@tuxdriver.com> wrote: >>> >>> Please don't make me commit such an ugly patch... >> >> Why don't you just create a dummy inline ath_opmode_to_string() that >> returns NULL in the non-debug case? The compiler will make the >> conditionals all go away, and it will end up being "UNKNOWN" which is >> just what you want. No? > > Shhh! Your messing-up my manipulative mind games! :-) Looks like this was my bug, and I lost these emails in a folder I don't read so often. If you're not happy with the fixes posted, let me know and I'll work on one. Thanks, Ben -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-10-12 23:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20101012163519.a88f2040.sfr@canb.auug.org.au>
2010-10-12 14:53 ` [PATCH] ath5k: fix build break from "ath5k: Print out opmode in debugfs" John W. Linville
2010-10-12 15:43 ` Joe Perches
2010-10-12 17:15 ` John W. Linville
2010-10-12 16:54 ` Bob Copeland
2010-10-12 17:42 ` Bob Copeland
2010-10-12 18:07 ` Joe Perches
2010-10-12 18:13 ` Linus Torvalds
2010-10-12 18:18 ` John W. Linville
2010-10-12 23:31 ` Ben Greear
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).