[alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK

Alan Stern stern at rowland.harvard.edu
Tue Jun 4 22:54:12 CEST 2013


On Tue, 4 Jun 2013, Joe Perches wrote:

> A somewhat common convention is to use
> 
> 	prefix_dbg(level, fmt, ...)
> 
> where level is either a numeric value or bitmask,
> and also is either a #define or a MODULE_PARAM
> 
> today sound/misc.c has:
> 
> ------------------------------------------
> 
> #ifdef CONFIG_SND_DEBUG
> 
> #ifdef CONFIG_SND_DEBUG_VERBOSE
> #define DEFAULT_DEBUG_LEVEL	2
> #else
> #define DEFAULT_DEBUG_LEVEL	1
> #endif
> 
> static int debug = DEFAULT_DEBUG_LEVEL;
> module_param(debug, int, 0644);
> MODULE_PARM_DESC(debug, "Debug level (0 = disable)");
> 
> #endif /* CONFIG_SND_DEBUG */

Yes, I have read this part of the code.

> ------------------------------------------
> 
> I suggest converting all the remaining
> 
> snd_printd(...) to snd_dbg(1, ...)
> and
> snd_printdd(...) to snd_dbg(2, ...)
> 
> so that debug module param control can be used for
> all these and if the DEFAULT_DEBUG_LEVEL isn't
> high enough, the various snd_dbg(2, ...) can be
> completely optimized away if appropriate.

I don't see how DEFAULT_DEBUG_LEVEL can be used to optimize away
anything.  The user can always change the value of the "debug" module
parameter while the system is running.  The only valid opportunity for
optimization would be if CONFIG_SND_DEBUG was disabled; then all these 
messages would disappear.

You didn't respond to the first point I raised.  Since these messages
are all meant for debugging, there's no point allowing them to have
prefixes like KERN_ERR or KERN_INFO.  They should always be printed at
the KERN_DEBUG level.  Or did you think this was so obviously true that
it didn't require any comment?

Alan Stern



More information about the Alsa-devel mailing list