Skip to contentSkip to conversation

ipw2100: wraps the debug module param within #ifdefs

PATCH In review Zhu Yi 2 participants 6 messages Apr 13, 2006 – Apr 17, 2006

Subsystems (3)
lore
Share & tools
lore

Zhu Yi

author
#0 lore raw

PATCH 1/1 ipw2100: wraps the debug module param within #ifdefs

---

 drivers/net/wireless/ipw2100.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

e78cfdab91a809722be99c06574d2e0a60467644

drivers/net/wireless/ipw2100.c +6 -0
index 72335c8..2cd32a4 100644
@@ -184,7 +184,9 @@ MODULE_VERSION(DRV_VERSION);
 MODULE_AUTHOR(DRV_COPYRIGHT);
 MODULE_LICENSE("GPL");
 
+#ifdef CONFIG_IPW2100_DEBUG
 static int debug = 0;
+#endif
 static int mode = 0;
 static int channel = 0;
 static int associate = 1;
@@ -194,13 +196,17 @@ static struct ipw2100_fw ipw2100_firmwar
 #endif
 
 #include <linux/moduleparam.h>
+#ifdef CONFIG_IPW2100_DEBUG
 module_param(debug, int, 0444);
+#endif
 module_param(mode, int, 0444);
 module_param(channel, int, 0444);
 module_param(associate, int, 0444);
 module_param(disable, int, 0444);
 
+#ifdef CONFIG_IPW2100_DEBUG
 MODULE_PARM_DESC(debug, "debug level");
+#endif
 MODULE_PARM_DESC(mode, "network mode (0=BSS,1=IBSS,2=Monitor)");
 MODULE_PARM_DESC(channel, "channel");
 MODULE_PARM_DESC(associate, "auto associate when scanning (default on)");
Signature · 1 line
1.2.6
#1 lore raw

Please get rid of the debug config option. Make the debug code, default off and be low enough impact that everyone can ship with it.

author
#2 lore raw
Quoted 2 lines · Stephen Hemminger

Please get rid of the debug config option. Make the debug code, default off and be low enough impact that everyone can ship with it.

This patch is exactly doing so. There won't be the debug module param if CONFIG_IPW2200_DEBUG is not defined. Otherwise it could be used to set different debug level, which helps a lot for the driver debuging.

Thanks,
-yi

#3 lore raw
Quoted 8 lines · On Fri, 14 Apr 2006 13:18:31 +0800 Zhu Yi <[email protected]> wrote:
Quoted 2 lines · Stephen Hemminger

Please get rid of the debug config option. Make the debug code, default off and be low enough impact that everyone can ship with it.

This patch is exactly doing so. There won't be the debug module param if CONFIG_IPW2200_DEBUG is not defined. Otherwise it could be used to set different debug level, which helps a lot for the driver debuging.

I meant get rid of CONFIG_IPW2200_DEBUG completely. Having the debug code isn't bad, and there is no reason not to have it always there.

author
#4 lore raw
Quoted 2 lines · Stephen Hemminger

I meant get rid of CONFIG_IPW2200_DEBUG completely. Having the debug code isn't bad, and there is no reason not to have it always there.

There are lots of IPW_DEBUG_XXX in the driver, even in TX and RX paths. It cause extra cycles for those who really don't need it.

Thanks,
-yi

#5 lore raw
Quoted 9 lines · On Mon, 17 Apr 2006 10:17:43 +0800 Zhu Yi <[email protected]> wrote:
Quoted 2 lines · Stephen Hemminger

I meant get rid of CONFIG_IPW2200_DEBUG completely. Having the debug code isn't bad, and there is no reason not to have it always there.

There are lots of IPW_DEBUG_XXX in the driver, even in TX and RX paths. It cause extra cycles for those who really don't need it.

Thanks,
-yi

IMHO. There is never a reason for a driver specific config option for debugging. Either it the debug option is generally useful for support (ie distro's will want it on), in which case the debugging should be runtime settable but always available; or the debug option is so meant for the driver developers in which case it should be enabled by a source modification (see #define DEBUG and pr_debug).

But, those are opinions only and certainly, you can do what you want with your driver.