* [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 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 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 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).