* [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
@ 2013-06-03 20:18 Alan Stern
2013-06-03 20:24 ` Joe Perches
2013-06-04 9:13 ` Takashi Iwai
0 siblings, 2 replies; 44+ messages in thread
From: Alan Stern @ 2013-06-03 20:18 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
it does is add filenames and line numbers to certain log messages.
This patch gets rid of it.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Joe Perches <joe@perches.com>
---
Updating all the snd_printk() and snd_printd() calls will be a huge
job. Getting rid of the verbose logging is a lot easier, so that's
where I'm starting.
Alan Stern
[as1688]
Documentation/DocBook/writing-an-alsa-driver.tmpl | 7 +---
arch/arm/configs/eseries_pxa_defconfig | 1
arch/arm/configs/omap2plus_defconfig | 1
arch/arm/configs/s3c2410_defconfig | 1
arch/arm/configs/trizeps4_defconfig | 1
arch/ia64/configs/generic_defconfig | 1
arch/ia64/configs/gensparse_defconfig | 1
arch/mips/configs/pnx8335_stb225_defconfig | 1
arch/powerpc/configs/ppc6xx_defconfig | 1
arch/sh/configs/edosk7760_defconfig | 1
arch/sh/configs/sh7785lcr_32bit_defconfig | 1
include/sound/core.h | 4 +-
sound/core/Kconfig | 9 ------
sound/core/misc.c | 33 ----------------------
sound/pci/hda/hda_intel.c | 4 --
15 files changed, 5 insertions(+), 62 deletions(-)
Index: usb-3.10/Documentation/DocBook/writing-an-alsa-driver.tmpl
===================================================================
--- usb-3.10.orig/Documentation/DocBook/writing-an-alsa-driver.tmpl
+++ usb-3.10/Documentation/DocBook/writing-an-alsa-driver.tmpl
@@ -6087,11 +6087,8 @@ struct _snd_pcm_runtime {
<title><function>snd_printk()</function> and friends</title>
<para>
ALSA provides a verbose version of the
- <function>printk()</function> function. If a kernel config
- <constant>CONFIG_SND_VERBOSE_PRINTK</constant> is set, this
- function prints the given message together with the file name
- and the line of the caller. The <constant>KERN_XXX</constant>
- prefix is processed as
+ <function>printk()</function> function.
+ The <constant>KERN_XXX</constant> prefix is processed as
well as the original <function>printk()</function> does, so it's
recommended to add this prefix, e.g.
Index: usb-3.10/arch/arm/configs/eseries_pxa_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/eseries_pxa_defconfig
+++ usb-3.10/arch/arm/configs/eseries_pxa_defconfig
@@ -90,7 +90,6 @@ CONFIG_SND=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_DYNAMIC_MINORS=y
-CONFIG_SND_VERBOSE_PRINTK=y
# CONFIG_SND_PCMCIA is not set
CONFIG_SND_SOC=m
CONFIG_SND_PXA2XX_SOC=m
Index: usb-3.10/arch/arm/configs/omap2plus_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/omap2plus_defconfig
+++ usb-3.10/arch/arm/configs/omap2plus_defconfig
@@ -192,7 +192,6 @@ CONFIG_SOUND=m
CONFIG_SND=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_USB_AUDIO=m
CONFIG_SND_SOC=m
Index: usb-3.10/arch/arm/configs/s3c2410_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/s3c2410_defconfig
+++ usb-3.10/arch/arm/configs/s3c2410_defconfig
@@ -315,7 +315,6 @@ CONFIG_SND_SEQUENCER=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
# CONFIG_SND_DRIVERS is not set
# CONFIG_SND_ARM is not set
# CONFIG_SND_SPI is not set
Index: usb-3.10/arch/arm/configs/trizeps4_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/trizeps4_defconfig
+++ usb-3.10/arch/arm/configs/trizeps4_defconfig
@@ -159,7 +159,6 @@ CONFIG_SND=y
CONFIG_SND_SEQUENCER=y
CONFIG_SND_MIXER_OSS=y
CONFIG_SND_PCM_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_PXA2XX_AC97=y
CONFIG_SND_USB_AUDIO=m
Index: usb-3.10/arch/ia64/configs/generic_defconfig
===================================================================
--- usb-3.10.orig/arch/ia64/configs/generic_defconfig
+++ usb-3.10/arch/ia64/configs/generic_defconfig
@@ -127,7 +127,6 @@ CONFIG_SND_SEQ_DUMMY=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DUMMY=m
CONFIG_SND_VIRMIDI=m
CONFIG_SND_MTPAV=m
Index: usb-3.10/arch/ia64/configs/gensparse_defconfig
===================================================================
--- usb-3.10.orig/arch/ia64/configs/gensparse_defconfig
+++ usb-3.10/arch/ia64/configs/gensparse_defconfig
@@ -115,7 +115,6 @@ CONFIG_SND_SEQ_DUMMY=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DUMMY=m
CONFIG_SND_VIRMIDI=m
CONFIG_SND_MTPAV=m
Index: usb-3.10/arch/mips/configs/pnx8335_stb225_defconfig
===================================================================
--- usb-3.10.orig/arch/mips/configs/pnx8335_stb225_defconfig
+++ usb-3.10/arch/mips/configs/pnx8335_stb225_defconfig
@@ -73,7 +73,6 @@ CONFIG_SND_SEQUENCER=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_EXT2_FS=m
# CONFIG_DNOTIFY is not set
Index: usb-3.10/arch/powerpc/configs/ppc6xx_defconfig
===================================================================
--- usb-3.10.orig/arch/powerpc/configs/ppc6xx_defconfig
+++ usb-3.10/arch/powerpc/configs/ppc6xx_defconfig
@@ -870,7 +870,6 @@ CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
CONFIG_SND_DYNAMIC_MINORS=y
# CONFIG_SND_SUPPORT_OLD_API is not set
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_DEBUG_VERBOSE=y
CONFIG_SND_PCM_XRUN_DEBUG=y
Index: usb-3.10/arch/sh/configs/edosk7760_defconfig
===================================================================
--- usb-3.10.orig/arch/sh/configs/edosk7760_defconfig
+++ usb-3.10/arch/sh/configs/edosk7760_defconfig
@@ -90,7 +90,6 @@ CONFIG_SOUND=y
CONFIG_SND=y
# CONFIG_SND_SUPPORT_OLD_API is not set
# CONFIG_SND_VERBOSE_PROCFS is not set
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_SOC=y
# CONFIG_HID_SUPPORT is not set
# CONFIG_USB_SUPPORT is not set
Index: usb-3.10/arch/sh/configs/sh7785lcr_32bit_defconfig
===================================================================
--- usb-3.10.orig/arch/sh/configs/sh7785lcr_32bit_defconfig
+++ usb-3.10/arch/sh/configs/sh7785lcr_32bit_defconfig
@@ -105,7 +105,6 @@ CONFIG_SND_HRTIMER=y
CONFIG_SND_DYNAMIC_MINORS=y
# CONFIG_SND_SUPPORT_OLD_API is not set
# CONFIG_SND_VERBOSE_PROCFS is not set
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_DEBUG_VERBOSE=y
# CONFIG_SND_DRIVERS is not set
Index: usb-3.10/include/sound/core.h
===================================================================
--- usb-3.10.orig/include/sound/core.h
+++ usb-3.10/include/sound/core.h
@@ -335,7 +335,7 @@ enum {
SND_PR_VERBOSE,
};
-#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
+#if defined(CONFIG_SND_DEBUG)
__printf(4, 5)
void __snd_printk(unsigned int level, const char *file, int line,
const char *format, ...);
@@ -349,7 +349,7 @@ void __snd_printk(unsigned int level, co
* @fmt: format string
*
* Works like printk() but prints the file and the line of the caller
- * when configured with CONFIG_SND_VERBOSE_PRINTK.
+ * when configured with CONFIG_SND_DEBUG.
*/
#define snd_printk(fmt, args...) \
__snd_printk(0, __FILE__, __LINE__, fmt, ##args)
Index: usb-3.10/sound/core/Kconfig
===================================================================
--- usb-3.10.orig/sound/core/Kconfig
+++ usb-3.10/sound/core/Kconfig
@@ -173,15 +173,6 @@ config SND_VERBOSE_PROCFS
useful information to developers when a problem occurs). On the
other side, it makes the ALSA subsystem larger.
-config SND_VERBOSE_PRINTK
- bool "Verbose printk"
- help
- Say Y here to enable verbose log messages. These messages
- will help to identify source file and position containing
- printed messages.
-
- You don't need this unless you're debugging ALSA.
-
config SND_DEBUG
bool "Debug"
help
Index: usb-3.10/sound/core/misc.c
===================================================================
--- usb-3.10.orig/sound/core/misc.c
+++ usb-3.10/sound/core/misc.c
@@ -51,27 +51,11 @@ void release_and_free_resource(struct re
EXPORT_SYMBOL(release_and_free_resource);
-#ifdef CONFIG_SND_VERBOSE_PRINTK
-/* strip the leading path if the given path is absolute */
-static const char *sanity_file_name(const char *path)
-{
- if (*path == '/')
- return strrchr(path, '/') + 1;
- else
- return path;
-}
-#endif
-
-#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
+#if defined(CONFIG_SND_DEBUG)
void __snd_printk(unsigned int level, const char *path, int line,
const char *format, ...)
{
va_list args;
-#ifdef CONFIG_SND_VERBOSE_PRINTK
- int kern_level;
- struct va_format vaf;
- char verbose_fmt[] = KERN_DEFAULT "ALSA %s:%d %pV";
-#endif
#ifdef CONFIG_SND_DEBUG
if (debug < level)
@@ -79,22 +63,7 @@ void __snd_printk(unsigned int level, co
#endif
va_start(args, format);
-#ifdef CONFIG_SND_VERBOSE_PRINTK
- vaf.fmt = format;
- vaf.va = &args;
-
- kern_level = printk_get_level(format);
- if (kern_level) {
- const char *end_of_header = printk_skip_level(format);
- memcpy(verbose_fmt, format, end_of_header - format);
- vaf.fmt = end_of_header;
- } else if (level)
- memcpy(verbose_fmt, KERN_DEBUG, sizeof(KERN_DEBUG) - 1);
- printk(verbose_fmt, sanity_file_name(path), line, &vaf);
-
-#else
vprintk(format, args);
-#endif
va_end(args);
}
EXPORT_SYMBOL_GPL(__snd_printk);
Index: usb-3.10/sound/pci/hda/hda_intel.c
===================================================================
--- usb-3.10.orig/sound/pci/hda/hda_intel.c
+++ usb-3.10/sound/pci/hda/hda_intel.c
@@ -189,11 +189,7 @@ MODULE_SUPPORTED_DEVICE("{{Intel, ICH6},
"{ULI, M5461}}");
MODULE_DESCRIPTION("Intel HDA driver");
-#ifdef CONFIG_SND_VERBOSE_PRINTK
-#define SFX /* nop */
-#else
#define SFX "hda-intel "
-#endif
#if defined(CONFIG_PM) && defined(CONFIG_VGA_SWITCHEROO)
#ifdef CONFIG_SND_HDA_CODEC_HDMI
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-03 20:18 [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
@ 2013-06-03 20:24 ` Joe Perches
2013-06-03 20:40 ` Alan Stern
2013-06-04 9:13 ` Takashi Iwai
1 sibling, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-03 20:24 UTC (permalink / raw)
To: Alan Stern; +Cc: Takashi Iwai, alsa-devel
On Mon, 2013-06-03 at 16:18 -0400, Alan Stern wrote:
> According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
> it does is add filenames and line numbers to certain log messages.
> This patch gets rid of it.
[]
> Updating all the snd_printk() and snd_printd() calls will be a huge
> job. Getting rid of the verbose logging is a lot easier, so that's
> where I'm starting.
cocci and emacs can make this pretty simple.
> --- usb-3.10.orig/include/sound/core.h
[]
> -#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
> +#if defined(CONFIG_SND_DEBUG)
> __printf(4, 5)
> void __snd_printk(unsigned int level, const char *file, int line,
> const char *format, ...);
> @@ -349,7 +349,7 @@ void __snd_printk(unsigned int level, co
> * @fmt: format string
> *
> * Works like printk() but prints the file and the line of the caller
> - * when configured with CONFIG_SND_VERBOSE_PRINTK.
> + * when configured with CONFIG_SND_DEBUG.
> */
> #define snd_printk(fmt, args...) \
> __snd_printk(0, __FILE__, __LINE__, fmt, ##args)
Please change the __snd_printk macros to remove __FILE__ and
__LINE__ (or 0, and 0) to remove some text from the objects.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-03 20:24 ` Joe Perches
@ 2013-06-03 20:40 ` Alan Stern
2013-06-03 20:49 ` Joe Perches
0 siblings, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-03 20:40 UTC (permalink / raw)
To: Joe Perches; +Cc: Takashi Iwai, alsa-devel
On Mon, 3 Jun 2013, Joe Perches wrote:
> On Mon, 2013-06-03 at 16:18 -0400, Alan Stern wrote:
> > According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
> > it does is add filenames and line numbers to certain log messages.
> > This patch gets rid of it.
> []
> > Updating all the snd_printk() and snd_printd() calls will be a huge
> > job. Getting rid of the verbose logging is a lot easier, so that's
> > where I'm starting.
>
> cocci and emacs can make this pretty simple.
I wish they could, but I sincerely doubt that this conversion can be
mechanized. At least, not if it is to be done correctly.
> > --- usb-3.10.orig/include/sound/core.h
> []
> > -#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
> > +#if defined(CONFIG_SND_DEBUG)
> > __printf(4, 5)
> > void __snd_printk(unsigned int level, const char *file, int line,
> > const char *format, ...);
> > @@ -349,7 +349,7 @@ void __snd_printk(unsigned int level, co
> > * @fmt: format string
> > *
> > * Works like printk() but prints the file and the line of the caller
> > - * when configured with CONFIG_SND_VERBOSE_PRINTK.
> > + * when configured with CONFIG_SND_DEBUG.
> > */
> > #define snd_printk(fmt, args...) \
> > __snd_printk(0, __FILE__, __LINE__, fmt, ##args)
>
> Please change the __snd_printk macros to remove __FILE__ and
> __LINE__ (or 0, and 0) to remove some text from the objects.
Will do. I should have realized that originally...
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-03 20:40 ` Alan Stern
@ 2013-06-03 20:49 ` Joe Perches
0 siblings, 0 replies; 44+ messages in thread
From: Joe Perches @ 2013-06-03 20:49 UTC (permalink / raw)
To: Alan Stern; +Cc: Takashi Iwai, alsa-devel
On Mon, 2013-06-03 at 16:40 -0400, Alan Stern wrote:
> On Mon, 3 Jun 2013, Joe Perches wrote:
> > On Mon, 2013-06-03 at 16:18 -0400, Alan Stern wrote:
> > > According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
> > > it does is add filenames and line numbers to certain log messages.
> > > This patch gets rid of it.
> > []
> > > Updating all the snd_printk() and snd_printd() calls will be a huge
> > > job. Getting rid of the verbose logging is a lot easier, so that's
> > > where I'm starting.
> >
> > cocci and emacs can make this pretty simple.
>
> I wish they could, but I sincerely doubt that this conversion can be
> mechanized. At least, not if it is to be done correctly.
Completely mechanized/automated no, simpler though.
I still think the snd_printk uses should be converted to:
<snd_type>_<level>(<snd_type> *foo, fmt, ...)
[]
> > Please change the __snd_printk macros to remove __FILE__ and
> > __LINE__ (or 0, and 0) to remove some text from the objects.
>
> Will do.
OK.
> I should have realized that originally...
No worries, it's a process.
Thanks for getting it started.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-03 20:18 [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
2013-06-03 20:24 ` Joe Perches
@ 2013-06-04 9:13 ` Takashi Iwai
2013-06-04 14:49 ` Alan Stern
1 sibling, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-04 9:13 UTC (permalink / raw)
To: Alan Stern; +Cc: Joe Perches, alsa-devel
At Mon, 3 Jun 2013 16:18:49 -0400 (EDT),
Alan Stern wrote:
>
> According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
> it does is add filenames and line numbers to certain log messages.
No, I did't mean it. The file name and line number are useful.
What's useless is to let user select CONFIG_SND_VERBOSE_PRINTK.
If the extra information can be annoying, the driver should use the
plain printk() or whatever from the beginning. If the driver code
chooses snd_printk(), it implies that it wants extra information.
thanks,
Takashi
>
> This patch gets rid of it.
>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> CC: Joe Perches <joe@perches.com>
>
> ---
>
> Updating all the snd_printk() and snd_printd() calls will be a huge
> job. Getting rid of the verbose logging is a lot easier, so that's
> where I'm starting.
>
> Alan Stern
>
> [as1688]
>
> Documentation/DocBook/writing-an-alsa-driver.tmpl | 7 +---
> arch/arm/configs/eseries_pxa_defconfig | 1
> arch/arm/configs/omap2plus_defconfig | 1
> arch/arm/configs/s3c2410_defconfig | 1
> arch/arm/configs/trizeps4_defconfig | 1
> arch/ia64/configs/generic_defconfig | 1
> arch/ia64/configs/gensparse_defconfig | 1
> arch/mips/configs/pnx8335_stb225_defconfig | 1
> arch/powerpc/configs/ppc6xx_defconfig | 1
> arch/sh/configs/edosk7760_defconfig | 1
> arch/sh/configs/sh7785lcr_32bit_defconfig | 1
> include/sound/core.h | 4 +-
> sound/core/Kconfig | 9 ------
> sound/core/misc.c | 33 ----------------------
> sound/pci/hda/hda_intel.c | 4 --
> 15 files changed, 5 insertions(+), 62 deletions(-)
>
> Index: usb-3.10/Documentation/DocBook/writing-an-alsa-driver.tmpl
> ===================================================================
> --- usb-3.10.orig/Documentation/DocBook/writing-an-alsa-driver.tmpl
> +++ usb-3.10/Documentation/DocBook/writing-an-alsa-driver.tmpl
> @@ -6087,11 +6087,8 @@ struct _snd_pcm_runtime {
> <title><function>snd_printk()</function> and friends</title>
> <para>
> ALSA provides a verbose version of the
> - <function>printk()</function> function. If a kernel config
> - <constant>CONFIG_SND_VERBOSE_PRINTK</constant> is set, this
> - function prints the given message together with the file name
> - and the line of the caller. The <constant>KERN_XXX</constant>
> - prefix is processed as
> + <function>printk()</function> function.
> + The <constant>KERN_XXX</constant> prefix is processed as
> well as the original <function>printk()</function> does, so it's
> recommended to add this prefix, e.g.
>
> Index: usb-3.10/arch/arm/configs/eseries_pxa_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/arm/configs/eseries_pxa_defconfig
> +++ usb-3.10/arch/arm/configs/eseries_pxa_defconfig
> @@ -90,7 +90,6 @@ CONFIG_SND=m
> CONFIG_SND_MIXER_OSS=m
> CONFIG_SND_PCM_OSS=m
> CONFIG_SND_DYNAMIC_MINORS=y
> -CONFIG_SND_VERBOSE_PRINTK=y
> # CONFIG_SND_PCMCIA is not set
> CONFIG_SND_SOC=m
> CONFIG_SND_PXA2XX_SOC=m
> Index: usb-3.10/arch/arm/configs/omap2plus_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/arm/configs/omap2plus_defconfig
> +++ usb-3.10/arch/arm/configs/omap2plus_defconfig
> @@ -192,7 +192,6 @@ CONFIG_SOUND=m
> CONFIG_SND=m
> CONFIG_SND_MIXER_OSS=m
> CONFIG_SND_PCM_OSS=m
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_DEBUG=y
> CONFIG_SND_USB_AUDIO=m
> CONFIG_SND_SOC=m
> Index: usb-3.10/arch/arm/configs/s3c2410_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/arm/configs/s3c2410_defconfig
> +++ usb-3.10/arch/arm/configs/s3c2410_defconfig
> @@ -315,7 +315,6 @@ CONFIG_SND_SEQUENCER=m
> CONFIG_SND_MIXER_OSS=m
> CONFIG_SND_PCM_OSS=m
> CONFIG_SND_SEQUENCER_OSS=y
> -CONFIG_SND_VERBOSE_PRINTK=y
> # CONFIG_SND_DRIVERS is not set
> # CONFIG_SND_ARM is not set
> # CONFIG_SND_SPI is not set
> Index: usb-3.10/arch/arm/configs/trizeps4_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/arm/configs/trizeps4_defconfig
> +++ usb-3.10/arch/arm/configs/trizeps4_defconfig
> @@ -159,7 +159,6 @@ CONFIG_SND=y
> CONFIG_SND_SEQUENCER=y
> CONFIG_SND_MIXER_OSS=y
> CONFIG_SND_PCM_OSS=y
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_DEBUG=y
> CONFIG_SND_PXA2XX_AC97=y
> CONFIG_SND_USB_AUDIO=m
> Index: usb-3.10/arch/ia64/configs/generic_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/ia64/configs/generic_defconfig
> +++ usb-3.10/arch/ia64/configs/generic_defconfig
> @@ -127,7 +127,6 @@ CONFIG_SND_SEQ_DUMMY=m
> CONFIG_SND_MIXER_OSS=m
> CONFIG_SND_PCM_OSS=m
> CONFIG_SND_SEQUENCER_OSS=y
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_DUMMY=m
> CONFIG_SND_VIRMIDI=m
> CONFIG_SND_MTPAV=m
> Index: usb-3.10/arch/ia64/configs/gensparse_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/ia64/configs/gensparse_defconfig
> +++ usb-3.10/arch/ia64/configs/gensparse_defconfig
> @@ -115,7 +115,6 @@ CONFIG_SND_SEQ_DUMMY=m
> CONFIG_SND_MIXER_OSS=m
> CONFIG_SND_PCM_OSS=m
> CONFIG_SND_SEQUENCER_OSS=y
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_DUMMY=m
> CONFIG_SND_VIRMIDI=m
> CONFIG_SND_MTPAV=m
> Index: usb-3.10/arch/mips/configs/pnx8335_stb225_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/mips/configs/pnx8335_stb225_defconfig
> +++ usb-3.10/arch/mips/configs/pnx8335_stb225_defconfig
> @@ -73,7 +73,6 @@ CONFIG_SND_SEQUENCER=m
> CONFIG_SND_MIXER_OSS=m
> CONFIG_SND_PCM_OSS=m
> CONFIG_SND_SEQUENCER_OSS=y
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_DEBUG=y
> CONFIG_EXT2_FS=m
> # CONFIG_DNOTIFY is not set
> Index: usb-3.10/arch/powerpc/configs/ppc6xx_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/powerpc/configs/ppc6xx_defconfig
> +++ usb-3.10/arch/powerpc/configs/ppc6xx_defconfig
> @@ -870,7 +870,6 @@ CONFIG_SND_PCM_OSS=m
> CONFIG_SND_SEQUENCER_OSS=y
> CONFIG_SND_DYNAMIC_MINORS=y
> # CONFIG_SND_SUPPORT_OLD_API is not set
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_DEBUG=y
> CONFIG_SND_DEBUG_VERBOSE=y
> CONFIG_SND_PCM_XRUN_DEBUG=y
> Index: usb-3.10/arch/sh/configs/edosk7760_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/sh/configs/edosk7760_defconfig
> +++ usb-3.10/arch/sh/configs/edosk7760_defconfig
> @@ -90,7 +90,6 @@ CONFIG_SOUND=y
> CONFIG_SND=y
> # CONFIG_SND_SUPPORT_OLD_API is not set
> # CONFIG_SND_VERBOSE_PROCFS is not set
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_SOC=y
> # CONFIG_HID_SUPPORT is not set
> # CONFIG_USB_SUPPORT is not set
> Index: usb-3.10/arch/sh/configs/sh7785lcr_32bit_defconfig
> ===================================================================
> --- usb-3.10.orig/arch/sh/configs/sh7785lcr_32bit_defconfig
> +++ usb-3.10/arch/sh/configs/sh7785lcr_32bit_defconfig
> @@ -105,7 +105,6 @@ CONFIG_SND_HRTIMER=y
> CONFIG_SND_DYNAMIC_MINORS=y
> # CONFIG_SND_SUPPORT_OLD_API is not set
> # CONFIG_SND_VERBOSE_PROCFS is not set
> -CONFIG_SND_VERBOSE_PRINTK=y
> CONFIG_SND_DEBUG=y
> CONFIG_SND_DEBUG_VERBOSE=y
> # CONFIG_SND_DRIVERS is not set
> Index: usb-3.10/include/sound/core.h
> ===================================================================
> --- usb-3.10.orig/include/sound/core.h
> +++ usb-3.10/include/sound/core.h
> @@ -335,7 +335,7 @@ enum {
> SND_PR_VERBOSE,
> };
>
> -#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
> +#if defined(CONFIG_SND_DEBUG)
> __printf(4, 5)
> void __snd_printk(unsigned int level, const char *file, int line,
> const char *format, ...);
> @@ -349,7 +349,7 @@ void __snd_printk(unsigned int level, co
> * @fmt: format string
> *
> * Works like printk() but prints the file and the line of the caller
> - * when configured with CONFIG_SND_VERBOSE_PRINTK.
> + * when configured with CONFIG_SND_DEBUG.
> */
> #define snd_printk(fmt, args...) \
> __snd_printk(0, __FILE__, __LINE__, fmt, ##args)
> Index: usb-3.10/sound/core/Kconfig
> ===================================================================
> --- usb-3.10.orig/sound/core/Kconfig
> +++ usb-3.10/sound/core/Kconfig
> @@ -173,15 +173,6 @@ config SND_VERBOSE_PROCFS
> useful information to developers when a problem occurs). On the
> other side, it makes the ALSA subsystem larger.
>
> -config SND_VERBOSE_PRINTK
> - bool "Verbose printk"
> - help
> - Say Y here to enable verbose log messages. These messages
> - will help to identify source file and position containing
> - printed messages.
> -
> - You don't need this unless you're debugging ALSA.
> -
> config SND_DEBUG
> bool "Debug"
> help
> Index: usb-3.10/sound/core/misc.c
> ===================================================================
> --- usb-3.10.orig/sound/core/misc.c
> +++ usb-3.10/sound/core/misc.c
> @@ -51,27 +51,11 @@ void release_and_free_resource(struct re
>
> EXPORT_SYMBOL(release_and_free_resource);
>
> -#ifdef CONFIG_SND_VERBOSE_PRINTK
> -/* strip the leading path if the given path is absolute */
> -static const char *sanity_file_name(const char *path)
> -{
> - if (*path == '/')
> - return strrchr(path, '/') + 1;
> - else
> - return path;
> -}
> -#endif
> -
> -#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
> +#if defined(CONFIG_SND_DEBUG)
> void __snd_printk(unsigned int level, const char *path, int line,
> const char *format, ...)
> {
> va_list args;
> -#ifdef CONFIG_SND_VERBOSE_PRINTK
> - int kern_level;
> - struct va_format vaf;
> - char verbose_fmt[] = KERN_DEFAULT "ALSA %s:%d %pV";
> -#endif
>
> #ifdef CONFIG_SND_DEBUG
> if (debug < level)
> @@ -79,22 +63,7 @@ void __snd_printk(unsigned int level, co
> #endif
>
> va_start(args, format);
> -#ifdef CONFIG_SND_VERBOSE_PRINTK
> - vaf.fmt = format;
> - vaf.va = &args;
> -
> - kern_level = printk_get_level(format);
> - if (kern_level) {
> - const char *end_of_header = printk_skip_level(format);
> - memcpy(verbose_fmt, format, end_of_header - format);
> - vaf.fmt = end_of_header;
> - } else if (level)
> - memcpy(verbose_fmt, KERN_DEBUG, sizeof(KERN_DEBUG) - 1);
> - printk(verbose_fmt, sanity_file_name(path), line, &vaf);
> -
> -#else
> vprintk(format, args);
> -#endif
> va_end(args);
> }
> EXPORT_SYMBOL_GPL(__snd_printk);
> Index: usb-3.10/sound/pci/hda/hda_intel.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/hda/hda_intel.c
> +++ usb-3.10/sound/pci/hda/hda_intel.c
> @@ -189,11 +189,7 @@ MODULE_SUPPORTED_DEVICE("{{Intel, ICH6},
> "{ULI, M5461}}");
> MODULE_DESCRIPTION("Intel HDA driver");
>
> -#ifdef CONFIG_SND_VERBOSE_PRINTK
> -#define SFX /* nop */
> -#else
> #define SFX "hda-intel "
> -#endif
>
> #if defined(CONFIG_PM) && defined(CONFIG_VGA_SWITCHEROO)
> #ifdef CONFIG_SND_HDA_CODEC_HDMI
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 9:13 ` Takashi Iwai
@ 2013-06-04 14:49 ` Alan Stern
2013-06-04 15:03 ` Takashi Iwai
0 siblings, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-04 14:49 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
On Tue, 4 Jun 2013, Takashi Iwai wrote:
> At Mon, 3 Jun 2013 16:18:49 -0400 (EDT),
> Alan Stern wrote:
> >
> > According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
> > it does is add filenames and line numbers to certain log messages.
>
> No, I did't mean it. The file name and line number are useful.
> What's useless is to let user select CONFIG_SND_VERBOSE_PRINTK.
> If the extra information can be annoying, the driver should use the
> plain printk() or whatever from the beginning. If the driver code
> chooses snd_printk(), it implies that it wants extra information.
Although most of the calls to snd_printk() are for errors and warnings,
some of them are just informational. You don't mind having those
messages suddenly including the filename and line number? (At least,
until someone gets around to changing them to printk() or something
else...)
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 14:49 ` Alan Stern
@ 2013-06-04 15:03 ` Takashi Iwai
2013-06-04 17:20 ` [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info(" Alan Stern
` (2 more replies)
0 siblings, 3 replies; 44+ messages in thread
From: Takashi Iwai @ 2013-06-04 15:03 UTC (permalink / raw)
To: Alan Stern; +Cc: Joe Perches, alsa-devel
At Tue, 4 Jun 2013 10:49:22 -0400 (EDT),
Alan Stern wrote:
>
> On Tue, 4 Jun 2013, Takashi Iwai wrote:
>
> > At Mon, 3 Jun 2013 16:18:49 -0400 (EDT),
> > Alan Stern wrote:
> > >
> > > According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
> > > it does is add filenames and line numbers to certain log messages.
> >
> > No, I did't mean it. The file name and line number are useful.
> > What's useless is to let user select CONFIG_SND_VERBOSE_PRINTK.
> > If the extra information can be annoying, the driver should use the
> > plain printk() or whatever from the beginning. If the driver code
> > chooses snd_printk(), it implies that it wants extra information.
>
> Although most of the calls to snd_printk() are for errors and warnings,
> some of them are just informational. You don't mind having those
> messages suddenly including the filename and line number? (At least,
> until someone gets around to changing them to printk() or something
> else...)
I wouldn't mind but some might do :)
So we should clean up the excessive usage of snd_printk() and convert
them to the standard printk & co at first.
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info("
2013-06-04 15:03 ` Takashi Iwai
@ 2013-06-04 17:20 ` Alan Stern
2013-06-05 5:52 ` Takashi Iwai
2013-06-04 17:20 ` [PATCH 2/2 v.2] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
2013-06-04 19:32 ` [PATCH] " Alan Stern
2 siblings, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-04 17:20 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
The snd_printk() function prints kernel log messages, including the
filename and line number if CONFIG_SND_PRINTK_VERBOSE is enabled.
This may make sense for errors and warnings, but not for informational
messages. For those, a simple pr_info() is what we want.
This patch mechanically converts all occurrences of
"snd_printk(KERN_INFO" to "pr_info(". It doesn't try to tell whether
the message really is informational; it relies on the existing
KERN_INFO tag.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
---
[as1689]
sound/drivers/ml403-ac97cr.c | 6 +++---
sound/drivers/mts64.c | 2 +-
sound/drivers/portman2x4.c | 2 +-
sound/isa/opti9xx/miro.c | 4 ++--
sound/isa/sb/sb16.c | 2 +-
sound/isa/sb/sb_common.c | 2 +-
sound/isa/sscape.c | 4 ++--
sound/isa/wavefront/wavefront.c | 2 +-
sound/pci/asihpi/asihpi.c | 14 ++++++--------
sound/pci/bt87x.c | 4 ++--
sound/pci/cs4281.c | 2 +-
sound/pci/cs46xx/cs46xx_lib.c | 4 ++--
sound/pci/echoaudio/echoaudio.c | 2 +-
sound/pci/emu10k1/emu10k1_main.c | 38 +++++++++++++++++++-------------------
sound/pci/emu10k1/emu10k1x.c | 8 ++++----
sound/pci/emu10k1/emufx.c | 8 ++++----
sound/pci/emu10k1/irq.c | 4 ++--
sound/pci/es1968.c | 2 +-
sound/pci/fm801.c | 4 ++--
sound/pci/hda/hda_hwdep.c | 2 +-
sound/pci/hda/hda_intel.c | 15 ++++++---------
sound/pci/hda/patch_analog.c | 2 +-
sound/pci/ice1712/aureon.c | 4 ++--
sound/pci/oxygen/oxygen_lib.c | 2 +-
sound/pci/rme9652/hdsp.c | 14 +++++++-------
sound/pci/rme9652/hdspm.c | 26 +++++++++++++-------------
sound/pci/sonicvibes.c | 4 ++--
sound/usb/card.c | 2 +-
sound/usb/clock.c | 3 +--
sound/usb/format.c | 6 +++---
sound/usb/mixer.c | 21 +++++++--------------
sound/usb/quirks.c | 6 +++---
32 files changed, 104 insertions(+), 117 deletions(-)
Index: usb-3.10/sound/drivers/ml403-ac97cr.c
===================================================================
--- usb-3.10.orig/sound/drivers/ml403-ac97cr.c
+++ usb-3.10/sound/drivers/ml403-ac97cr.c
@@ -1148,7 +1148,7 @@ snd_ml403_ac97cr_create(struct snd_card
snd_ml403_ac97cr_free(ml403_ac97cr);
return -EBUSY;
}
- snd_printk(KERN_INFO SND_ML403_AC97CR_DRIVER ": "
+ pr_info(SND_ML403_AC97CR_DRIVER ": "
"remap controller memory region to "
"0x%x done\n", (unsigned int)ml403_ac97cr->port);
/* get irq */
@@ -1162,7 +1162,7 @@ snd_ml403_ac97cr_create(struct snd_card
return -EBUSY;
}
ml403_ac97cr->irq = irq;
- snd_printk(KERN_INFO SND_ML403_AC97CR_DRIVER ": "
+ pr_info(SND_ML403_AC97CR_DRIVER ": "
"request (playback) irq %d done\n",
ml403_ac97cr->irq);
irq = platform_get_irq(pfdev, 1);
@@ -1175,7 +1175,7 @@ snd_ml403_ac97cr_create(struct snd_card
return -EBUSY;
}
ml403_ac97cr->capture_irq = irq;
- snd_printk(KERN_INFO SND_ML403_AC97CR_DRIVER ": "
+ pr_info(SND_ML403_AC97CR_DRIVER ": "
"request (capture) irq %d done\n",
ml403_ac97cr->capture_irq);
Index: usb-3.10/sound/drivers/mts64.c
===================================================================
--- usb-3.10.orig/sound/drivers/mts64.c
+++ usb-3.10/sound/drivers/mts64.c
@@ -1017,7 +1017,7 @@ static int snd_mts64_probe(struct platfo
goto __err;
}
- snd_printk(KERN_INFO "ESI Miditerminal 4140 on 0x%lx\n", p->base);
+ pr_info("ESI Miditerminal 4140 on 0x%lx\n", p->base);
return 0;
__err:
Index: usb-3.10/sound/drivers/portman2x4.c
===================================================================
--- usb-3.10.orig/sound/drivers/portman2x4.c
+++ usb-3.10/sound/drivers/portman2x4.c
@@ -806,7 +806,7 @@ static int snd_portman_probe(struct plat
goto __err;
}
- snd_printk(KERN_INFO "Portman 2x4 on 0x%lx\n", p->base);
+ pr_info("Portman 2x4 on 0x%lx\n", p->base);
return 0;
__err:
Index: usb-3.10/sound/isa/opti9xx/miro.c
===================================================================
--- usb-3.10.orig/sound/isa/opti9xx/miro.c
+++ usb-3.10/sound/isa/opti9xx/miro.c
@@ -1346,11 +1346,11 @@ static int snd_miro_probe(struct snd_car
default:
sprintf(card->shortname,
"unknown miro");
- snd_printk(KERN_INFO "unknown miro aci id\n");
+ pr_info("unknown miro aci id\n");
break;
}
} else {
- snd_printk(KERN_INFO "found unsupported aci card\n");
+ pr_info("found unsupported aci card\n");
sprintf(card->shortname, "unknown Cardinal Technologies");
}
Index: usb-3.10/sound/isa/sb/sb16.c
===================================================================
--- usb-3.10.orig/sound/isa/sb/sb16.c
+++ usb-3.10/sound/isa/sb/sb16.c
@@ -435,7 +435,7 @@ static int snd_sb16_probe(struct snd_car
chip->csp = xcsp->private_data;
chip->hardware = SB_HW_16CSP;
} else {
- snd_printk(KERN_INFO PFX "warning - CSP chip not detected on soundcard #%i\n", dev + 1);
+ pr_info(PFX "warning - CSP chip not detected on soundcard #%i\n", dev + 1);
}
}
#endif
Index: usb-3.10/sound/isa/sb/sb_common.c
===================================================================
--- usb-3.10.orig/sound/isa/sb/sb_common.c
+++ usb-3.10/sound/isa/sb/sb_common.c
@@ -154,7 +154,7 @@ static int snd_sbdsp_probe(struct snd_sb
str = "16";
break;
default:
- snd_printk(KERN_INFO "SB [0x%lx]: unknown DSP chip version %i.%i\n",
+ pr_info("SB [0x%lx]: unknown DSP chip version %i.%i\n",
chip->port, major, minor);
return -ENODEV;
}
Index: usb-3.10/sound/isa/sscape.c
===================================================================
--- usb-3.10.orig/sound/isa/sscape.c
+++ usb-3.10/sound/isa/sscape.c
@@ -590,7 +590,7 @@ static int sscape_upload_microcode(struc
}
err = upload_dma_data(sscape, init_fw->data, init_fw->size);
if (err == 0)
- snd_printk(KERN_INFO "sscape: MIDI firmware loaded %d KBs\n",
+ pr_info("sscape: MIDI firmware loaded %d KBs\n",
init_fw->size >> 10);
release_firmware(init_fw);
@@ -1251,7 +1251,7 @@ static int sscape_pnp_detect(struct pnp_
if (!pnp_is_active(dev)) {
if (pnp_activate_dev(dev) < 0) {
- snd_printk(KERN_INFO "sscape: device is inactive\n");
+ pr_info("sscape: device is inactive\n");
return -EBUSY;
}
}
Index: usb-3.10/sound/pci/asihpi/asihpi.c
===================================================================
--- usb-3.10.orig/sound/pci/asihpi/asihpi.c
+++ usb-3.10/sound/pci/asihpi/asihpi.c
@@ -1347,7 +1347,7 @@ static inline int ctl_add(struct snd_car
if (err < 0)
return err;
else if (mixer_dump)
- snd_printk(KERN_INFO "added %s(%d)\n", ctl->name, ctl->index);
+ pr_info("added %s(%d)\n", ctl->name, ctl->index);
return 0;
}
@@ -2583,8 +2583,7 @@ static int snd_card_asihpi_mixer_new(str
if (err) {
if (err == HPI_ERROR_CONTROL_DISABLED) {
if (mixer_dump)
- snd_printk(KERN_INFO
- "Disabled HPI Control(%d)\n",
+ pr_info("Disabled HPI Control(%d)\n",
idx);
continue;
} else
@@ -2648,8 +2647,7 @@ static int snd_card_asihpi_mixer_new(str
case HPI_CONTROL_COMPANDER:
default:
if (mixer_dump)
- snd_printk(KERN_INFO
- "Untranslated HPI Control"
+ pr_info("Untranslated HPI Control"
"(%d) %d %d %d %d %d\n",
idx,
hpi_ctl.control_type,
@@ -2665,7 +2663,7 @@ static int snd_card_asihpi_mixer_new(str
if (HPI_ERROR_INVALID_OBJ_INDEX != err)
hpi_handle_error(err);
- snd_printk(KERN_INFO "%d mixer controls found\n", idx);
+ pr_info("%d mixer controls found\n", idx);
return 0;
}
@@ -2844,7 +2842,7 @@ static int snd_asihpi_probe(struct pci_d
asihpi->pci = pci_dev;
asihpi->hpi = hpi;
- snd_printk(KERN_INFO "adapter ID=%4X index=%d\n",
+ pr_info("adapter ID=%4X index=%d\n",
asihpi->hpi->adapter->type, adapter_index);
err = hpi_adapter_get_property(adapter_index,
@@ -2893,7 +2891,7 @@ static int snd_asihpi_probe(struct pci_d
asihpi->in_min_chans = 1;
}
- snd_printk(KERN_INFO "Has dma:%d, grouping:%d, mrx:%d\n",
+ pr_info("Has dma:%d, grouping:%d, mrx:%d\n",
asihpi->can_dma,
asihpi->support_grouping,
asihpi->support_mrx
Index: usb-3.10/sound/pci/bt87x.c
===================================================================
--- usb-3.10.orig/sound/pci/bt87x.c
+++ usb-3.10/sound/pci/bt87x.c
@@ -856,7 +856,7 @@ static int snd_bt87x_detect_card(struct
return -EBUSY;
}
- snd_printk(KERN_INFO "unknown card %#04x-%#04x:%#04x\n",
+ pr_info("unknown card %#04x-%#04x:%#04x\n",
pci->device, pci->subsystem_vendor, pci->subsystem_device);
snd_printk(KERN_DEBUG "please mail id, board name, and, "
"if it works, the correct digital_rate option to "
@@ -925,7 +925,7 @@ static int snd_bt87x_probe(struct pci_de
if (err < 0)
goto _error;
}
- snd_printk(KERN_INFO "bt87x%d: Using board %d, %sanalog, %sdigital "
+ pr_info("bt87x%d: Using board %d, %sanalog, %sdigital "
"(rate %d Hz)\n", dev, boardid,
chip->board.no_analog ? "no " : "",
chip->board.no_digital ? "no " : "", chip->board.dig_rate);
Index: usb-3.10/sound/pci/cs4281.c
===================================================================
--- usb-3.10.orig/sound/pci/cs4281.c
+++ usb-3.10/sound/pci/cs4281.c
@@ -1539,7 +1539,7 @@ static int snd_cs4281_chip_init(struct c
goto __codec2_ok;
schedule_timeout_uninterruptible(1);
} while (time_after_eq(end_time, jiffies));
- snd_printk(KERN_INFO "secondary codec doesn't respond. disable it...\n");
+ pr_info("secondary codec doesn't respond. disable it...\n");
chip->dual_codec = 0;
__codec2_ok: ;
}
Index: usb-3.10/sound/pci/cs46xx/cs46xx_lib.c
===================================================================
--- usb-3.10.orig/sound/pci/cs46xx/cs46xx_lib.c
+++ usb-3.10/sound/pci/cs46xx/cs46xx_lib.c
@@ -3806,12 +3806,12 @@ int snd_cs46xx_create(struct snd_card *c
}
if (external_amp) {
- snd_printk(KERN_INFO "Crystal EAPD support forced on.\n");
+ pr_info("Crystal EAPD support forced on.\n");
chip->amplifier_ctrl = amp_voyetra;
}
if (thinkpad) {
- snd_printk(KERN_INFO "Activating CLKRUN hack for Thinkpad.\n");
+ pr_info("Activating CLKRUN hack for Thinkpad.\n");
chip->active_ctrl = clkrun_hack;
clkrun_init(chip);
}
Index: usb-3.10/sound/pci/echoaudio/echoaudio.c
===================================================================
--- usb-3.10.orig/sound/pci/echoaudio/echoaudio.c
+++ usb-3.10/sound/pci/echoaudio/echoaudio.c
@@ -2189,7 +2189,7 @@ static int snd_echo_probe(struct pci_dev
err = snd_card_register(card);
if (err < 0)
goto ctl_error;
- snd_printk(KERN_INFO "Card registered: %s\n", card->longname);
+ pr_info("Card registered: %s\n", card->longname);
pci_set_drvdata(pci, chip);
dev++;
Index: usb-3.10/sound/pci/emu10k1/emu10k1_main.c
===================================================================
--- usb-3.10.orig/sound/pci/emu10k1/emu10k1_main.c
+++ usb-3.10/sound/pci/emu10k1/emu10k1_main.c
@@ -217,7 +217,7 @@ static int snd_emu10k1_init(struct snd_e
}
if (emu->card_capabilities->ca0108_chip) { /* audigy2 Value */
/* Hacks for Alice3 to work independent of haP16V driver */
- snd_printk(KERN_INFO "Audigy2 value: Special config.\n");
+ pr_info("Audigy2 value: Special config.\n");
/* Setup SRCMulti_I2S SamplingRate */
tmp = snd_emu10k1_ptr_read(emu, A_SPDIF_SAMPLERATE, 0);
tmp &= 0xfffff1ff;
@@ -723,7 +723,7 @@ static int emu1010_firmware_thread(void
if (reg & EMU_HANA_OPTION_DOCK_OFFLINE) {
/* Audio Dock attached */
/* Return to Audio Dock programming mode */
- snd_printk(KERN_INFO "emu1010: Loading Audio Dock Firmware\n");
+ pr_info("emu1010: Loading Audio Dock Firmware\n");
snd_emu1010_fpga_write(emu, EMU_HANA_FPGA_CONFIG, EMU_HANA_FPGA_CONFIG_AUDIODOCK);
if (!emu->dock_fw) {
@@ -756,19 +756,19 @@ static int emu1010_firmware_thread(void
snd_emu1010_fpga_write(emu, EMU_HANA_FPGA_CONFIG, 0);
snd_emu1010_fpga_read(emu, EMU_HANA_IRQ_STATUS, ®);
- snd_printk(KERN_INFO "emu1010: EMU_HANA+DOCK_IRQ_STATUS = 0x%x\n", reg);
+ pr_info("emu1010: EMU_HANA+DOCK_IRQ_STATUS = 0x%x\n", reg);
/* ID, should read & 0x7f = 0x55 when FPGA programmed. */
snd_emu1010_fpga_read(emu, EMU_HANA_ID, ®);
- snd_printk(KERN_INFO "emu1010: EMU_HANA+DOCK_ID = 0x%x\n", reg);
+ pr_info("emu1010: EMU_HANA+DOCK_ID = 0x%x\n", reg);
if ((reg & 0x1f) != 0x15) {
/* FPGA failed to be programmed */
- snd_printk(KERN_INFO "emu1010: Loading Audio Dock Firmware file failed, reg = 0x%x\n", reg);
+ pr_info("emu1010: Loading Audio Dock Firmware file failed, reg = 0x%x\n", reg);
continue;
}
- snd_printk(KERN_INFO "emu1010: Audio Dock Firmware loaded\n");
+ pr_info("emu1010: Audio Dock Firmware loaded\n");
snd_emu1010_fpga_read(emu, EMU_DOCK_MAJOR_REV, &tmp);
snd_emu1010_fpga_read(emu, EMU_DOCK_MINOR_REV, &tmp2);
- snd_printk(KERN_INFO "Audio Dock ver: %u.%u\n",
+ pr_info("Audio Dock ver: %u.%u\n",
tmp, tmp2);
/* Sync clocking between 1010 and Dock */
/* Allow DLL to settle */
@@ -777,7 +777,7 @@ static int emu1010_firmware_thread(void
snd_emu1010_fpga_write(emu, EMU_HANA_UNMUTE, EMU_UNMUTE);
}
}
- snd_printk(KERN_INFO "emu1010: firmware thread stopping\n");
+ pr_info("emu1010: firmware thread stopping\n");
return 0;
}
@@ -818,7 +818,7 @@ static int snd_emu10k1_emu1010_init(stru
u32 tmp, tmp2, reg;
int err;
- snd_printk(KERN_INFO "emu1010: Special config.\n");
+ pr_info("emu1010: Special config.\n");
/* AC97 2.1, Any 16Meg of 4Gig address, Auto-Mute, EMU32 Slave,
* Lock Sound Memory Cache, Lock Tank Memory Cache,
* Mute all codecs.
@@ -854,10 +854,10 @@ static int snd_emu10k1_emu1010_init(stru
snd_printdd("reg2 = 0x%x\n", reg);
if ((reg & 0x3f) == 0x15) {
/* FPGA failed to return to programming mode */
- snd_printk(KERN_INFO "emu1010: FPGA failed to return to programming mode\n");
+ pr_info("emu1010: FPGA failed to return to programming mode\n");
return -ENODEV;
}
- snd_printk(KERN_INFO "emu1010: EMU_HANA_ID = 0x%x\n", reg);
+ pr_info("emu1010: EMU_HANA_ID = 0x%x\n", reg);
if (!emu->firmware) {
const char *filename;
@@ -883,13 +883,13 @@ static int snd_emu10k1_emu1010_init(stru
snd_printk(KERN_ERR "emu1010: firmware: %s not found. Err = %d\n", filename, err);
return err;
}
- snd_printk(KERN_INFO "emu1010: firmware file = %s, size = 0x%zx\n",
+ pr_info("emu1010: firmware file = %s, size = 0x%zx\n",
filename, emu->firmware->size);
}
err = snd_emu1010_load_firmware(emu, emu->firmware);
if (err != 0) {
- snd_printk(KERN_INFO "emu1010: Loading Firmware failed\n");
+ pr_info("emu1010: Loading Firmware failed\n");
return err;
}
@@ -897,21 +897,21 @@ static int snd_emu10k1_emu1010_init(stru
snd_emu1010_fpga_read(emu, EMU_HANA_ID, ®);
if ((reg & 0x3f) != 0x15) {
/* FPGA failed to be programmed */
- snd_printk(KERN_INFO "emu1010: Loading Hana Firmware file failed, reg = 0x%x\n", reg);
+ pr_info("emu1010: Loading Hana Firmware file failed, reg = 0x%x\n", reg);
return -ENODEV;
}
- snd_printk(KERN_INFO "emu1010: Hana Firmware loaded\n");
+ pr_info("emu1010: Hana Firmware loaded\n");
snd_emu1010_fpga_read(emu, EMU_HANA_MAJOR_REV, &tmp);
snd_emu1010_fpga_read(emu, EMU_HANA_MINOR_REV, &tmp2);
- snd_printk(KERN_INFO "emu1010: Hana version: %u.%u\n", tmp, tmp2);
+ pr_info("emu1010: Hana version: %u.%u\n", tmp, tmp2);
/* Enable 48Volt power to Audio Dock */
snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, EMU_HANA_DOCK_PWR_ON);
snd_emu1010_fpga_read(emu, EMU_HANA_OPTION_CARDS, ®);
- snd_printk(KERN_INFO "emu1010: Card options = 0x%x\n", reg);
+ pr_info("emu1010: Card options = 0x%x\n", reg);
snd_emu1010_fpga_read(emu, EMU_HANA_OPTION_CARDS, ®);
- snd_printk(KERN_INFO "emu1010: Card options = 0x%x\n", reg);
+ pr_info("emu1010: Card options = 0x%x\n", reg);
snd_emu1010_fpga_read(emu, EMU_HANA_OPTICAL_TYPE, &tmp);
/* Optical -> ADAT I/O */
/* 0 : SPDIF
@@ -950,7 +950,7 @@ static int snd_emu10k1_emu1010_init(stru
snd_emu1010_fpga_write(emu, EMU_HANA_IRQ_ENABLE, 0x00);
snd_emu1010_fpga_read(emu, EMU_HANA_OPTION_CARDS, ®);
- snd_printk(KERN_INFO "emu1010: Card options3 = 0x%x\n", reg);
+ pr_info("emu1010: Card options3 = 0x%x\n", reg);
/* Default WCLK set to 48kHz. */
snd_emu1010_fpga_write(emu, EMU_HANA_DEFCLOCK, 0x00);
/* Word Clock source, Internal 48kHz x1 */
Index: usb-3.10/sound/pci/emu10k1/emu10k1x.c
===================================================================
--- usb-3.10.orig/sound/pci/emu10k1/emu10k1x.c
+++ usb-3.10/sound/pci/emu10k1/emu10k1x.c
@@ -369,7 +369,7 @@ static void snd_emu10k1x_pcm_interrupt(s
if (epcm->substream == NULL)
return;
#if 0
- snd_printk(KERN_INFO "IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
+ pr_info("IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
epcm->substream->ops->pointer(epcm->substream),
snd_pcm_lib_period_bytes(epcm->substream),
snd_pcm_lib_buffer_bytes(epcm->substream));
@@ -487,7 +487,7 @@ static int snd_emu10k1x_pcm_trigger(stru
int channel = epcm->voice->number;
int result = 0;
-// snd_printk(KERN_INFO "trigger - emu10k1x = 0x%x, cmd = %i, pointer = %d\n", (int)emu, cmd, (int)substream->ops->pointer(substream));
+// pr_info("trigger - emu10k1x = 0x%x, cmd = %i, pointer = %d\n", (int)emu, cmd, (int)substream->ops->pointer(substream));
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
@@ -826,7 +826,7 @@ static irqreturn_t snd_emu10k1x_interrup
// acknowledge the interrupt if necessary
outl(status, chip->port + IPR);
- // snd_printk(KERN_INFO "interrupt %08x\n", status);
+ // pr_info("interrupt %08x\n", status);
return IRQ_HANDLED;
}
@@ -964,7 +964,7 @@ static int snd_emu10k1x_create(struct sn
chip->revision = pci->revision;
pci_read_config_dword(pci, PCI_SUBSYSTEM_VENDOR_ID, &chip->serial);
pci_read_config_word(pci, PCI_SUBSYSTEM_ID, &chip->model);
- snd_printk(KERN_INFO "Model %04x Rev %08x Serial %08x\n", chip->model,
+ pr_info("Model %04x Rev %08x Serial %08x\n", chip->model,
chip->revision, chip->serial);
outl(0, chip->port + INTE);
Index: usb-3.10/sound/pci/emu10k1/emufx.c
===================================================================
--- usb-3.10.orig/sound/pci/emu10k1/emufx.c
+++ usb-3.10/sound/pci/emu10k1/emufx.c
@@ -1542,7 +1542,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
/* A_PUT_STEREO_OUTPUT(A_EXTOUT_FRONT_L, A_EXTOUT_FRONT_R, playback + SND_EMU10K1_PLAYBACK_CHANNELS); */
if (emu->card_capabilities->emu_model) {
/* EMU1010 Outputs from PCM Front, Rear, Center, LFE, Side */
- snd_printk(KERN_INFO "EMU outputs on\n");
+ pr_info("EMU outputs on\n");
for (z = 0; z < 8; z++) {
if (emu->card_capabilities->ca0108_chip) {
A_OP(icode, &ptr, iACC3, A3_EMU32OUT(z), A_GPR(playback + SND_EMU10K1_PLAYBACK_CHANNELS + z), A_C_00000000, A_C_00000000);
@@ -1566,7 +1566,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
A_SWITCH(icode, &ptr, tmp + 1, playback + SND_EMU10K1_PLAYBACK_CHANNELS + z, tmp + 1);
if ((z==1) && (emu->card_capabilities->spdif_bug)) {
/* Due to a SPDIF output bug on some Audigy cards, this code delays the Right channel by 1 sample */
- snd_printk(KERN_INFO "Installing spdif_bug patch: %s\n", emu->card_capabilities->name);
+ pr_info("Installing spdif_bug patch: %s\n", emu->card_capabilities->name);
A_OP(icode, &ptr, iACC3, A_EXTOUT(A_EXTOUT_FRONT_L + z), A_GPR(gpr - 3), A_C_00000000, A_C_00000000);
A_OP(icode, &ptr, iACC3, A_GPR(gpr - 3), A_GPR(tmp + 0), A_GPR(tmp + 1), A_C_00000000);
} else {
@@ -1590,7 +1590,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
if (emu->card_capabilities->emu_model) {
if (emu->card_capabilities->ca0108_chip) {
- snd_printk(KERN_INFO "EMU2 inputs on\n");
+ pr_info("EMU2 inputs on\n");
for (z = 0; z < 0x10; z++) {
snd_emu10k1_audigy_dsp_convert_32_to_2x16( icode, &ptr, tmp,
bit_shifter16,
@@ -1598,7 +1598,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
A_FXBUS2(z*2) );
}
} else {
- snd_printk(KERN_INFO "EMU inputs on\n");
+ pr_info("EMU inputs on\n");
/* Capture 16 (originally 8) channels of S32_LE sound */
/*
Index: usb-3.10/sound/pci/emu10k1/irq.c
===================================================================
--- usb-3.10.orig/sound/pci/emu10k1/irq.c
+++ usb-3.10/sound/pci/emu10k1/irq.c
@@ -41,7 +41,7 @@ irqreturn_t snd_emu10k1_interrupt(int ir
orig_status = status;
handled = 1;
if ((status & 0xffffffff) == 0xffffffff) {
- snd_printk(KERN_INFO "snd-emu10k1: Suspected sound card removal\n");
+ pr_info("snd-emu10k1: Suspected sound card removal\n");
break;
}
if (status & IPR_PCIERROR) {
@@ -202,7 +202,7 @@ irqreturn_t snd_emu10k1_interrupt(int ir
outl(orig_status, emu->port + IPR); /* ack all */
}
if (timeout == 1000)
- snd_printk(KERN_INFO "emu10k1 irq routine failure\n");
+ pr_info("emu10k1 irq routine failure\n");
return IRQ_RETVAL(handled);
}
Index: usb-3.10/sound/pci/es1968.c
===================================================================
--- usb-3.10.orig/sound/pci/es1968.c
+++ usb-3.10/sound/pci/es1968.c
@@ -2109,7 +2109,7 @@ static void snd_es1968_ac97_reset(struct
outw(inw(ioaddr + 0x3c) & 0xfffc, ioaddr + 0x3c);
#if 0 /* the loop here needs to be much better if we want it.. */
- snd_printk(KERN_INFO "trying software reset\n");
+ pr_info("trying software reset\n");
/* try and do a software reset */
outb(0x80 | 0x7c, ioaddr + 0x30);
for (w = 0;; w++) {
Index: usb-3.10/sound/pci/fm801.c
===================================================================
--- usb-3.10.orig/sound/pci/fm801.c
+++ usb-3.10/sound/pci/fm801.c
@@ -1100,7 +1100,7 @@ static int snd_fm801_chip_init(struct fm
if (wait_for_codec(chip, 0, AC97_RESET, msecs_to_jiffies(750)) < 0)
if (!resume) {
- snd_printk(KERN_INFO "Primary AC'97 codec not found, "
+ pr_info("Primary AC'97 codec not found, "
"assume SF64-PCR (tuner-only)\n");
chip->tea575x_tuner = 3 | TUNER_ONLY;
goto __ac97_ok;
@@ -1276,7 +1276,7 @@ static int snd_fm801_create(struct snd_c
for (tea575x_tuner = 1; tea575x_tuner <= 3; tea575x_tuner++) {
chip->tea575x_tuner = tea575x_tuner;
if (!snd_tea575x_init(&chip->tea, THIS_MODULE)) {
- snd_printk(KERN_INFO "detected TEA575x radio type %s\n",
+ pr_info("detected TEA575x radio type %s\n",
get_tea575x_gpio(chip)->name);
break;
}
Index: usb-3.10/sound/pci/hda/hda_hwdep.c
===================================================================
--- usb-3.10.orig/sound/pci/hda/hda_hwdep.c
+++ usb-3.10/sound/pci/hda/hda_hwdep.c
@@ -218,7 +218,7 @@ static int reconfig_codec(struct hda_cod
int err;
snd_hda_power_up(codec);
- snd_printk(KERN_INFO "hda-codec: reconfiguring\n");
+ pr_info("hda-codec: reconfiguring\n");
err = snd_hda_codec_reset(codec);
if (err < 0) {
snd_printk(KERN_ERR
Index: usb-3.10/sound/pci/hda/hda_intel.c
===================================================================
--- usb-3.10.orig/sound/pci/hda/hda_intel.c
+++ usb-3.10/sound/pci/hda/hda_intel.c
@@ -3030,8 +3030,7 @@ static void azx_vs_set_state(struct pci_
if (!chip->bus) {
chip->disabled = disabled;
if (!disabled) {
- snd_printk(KERN_INFO SFX
- "%s: Start delayed initialization\n",
+ pr_info(SFX "%s: Start delayed initialization\n",
pci_name(chip->pci));
if (azx_first_init(chip) < 0 ||
azx_probe_continue(chip) < 0) {
@@ -3042,8 +3041,7 @@ static void azx_vs_set_state(struct pci_
}
}
} else {
- snd_printk(KERN_INFO SFX
- "%s: %s via VGA-switcheroo\n", pci_name(chip->pci),
+ pr_info(SFX "%s: %s via VGA-switcheroo\n", pci_name(chip->pci),
disabled ? "Disabling" : "Enabling");
if (disabled) {
azx_suspend(&pci->dev);
@@ -3079,8 +3077,7 @@ static void init_vga_switcheroo(struct a
{
struct pci_dev *p = get_bound_vga(chip->pci);
if (p) {
- snd_printk(KERN_INFO SFX
- "%s: Handle VGA-switcheroo audio client\n",
+ pr_info(SFX "%s: Handle VGA-switcheroo audio client\n",
pci_name(chip->pci));
chip->use_vga_switcheroo = 1;
pci_dev_put(p);
@@ -3392,7 +3389,7 @@ static void azx_check_snoop_available(st
}
if (snoop != chip->snoop) {
- snd_printk(KERN_INFO SFX "%s: Force to %s mode\n",
+ pr_info(SFX "%s: Force to %s mode\n",
pci_name(chip->pci), snoop ? "snoop" : "non-snoop");
chip->snoop = snoop;
}
@@ -3723,9 +3720,9 @@ static int azx_probe(struct pci_dev *pci
}
if (check_hdmi_disabled(pci)) {
- snd_printk(KERN_INFO SFX "%s: VGA controller is disabled\n",
+ pr_info(SFX "%s: VGA controller is disabled\n",
pci_name(pci));
- snd_printk(KERN_INFO SFX "%s: Delaying initialization\n", pci_name(pci));
+ pr_info(SFX "%s: Delaying initialization\n", pci_name(pci));
chip->disabled = true;
}
Index: usb-3.10/sound/pci/hda/patch_analog.c
===================================================================
--- usb-3.10.orig/sound/pci/hda/patch_analog.c
+++ usb-3.10/sound/pci/hda/patch_analog.c
@@ -3289,7 +3289,7 @@ static int patch_ad1988(struct hda_codec
spec = codec->spec;
if (is_rev2(codec))
- snd_printk(KERN_INFO "patch_analog: AD1988A rev.2 is detected, enable workarounds\n");
+ pr_info("patch_analog: AD1988A rev.2 is detected, enable workarounds\n");
err = snd_hda_attach_beep_device(codec, 0x10);
if (err < 0) {
Index: usb-3.10/sound/pci/ice1712/aureon.c
===================================================================
--- usb-3.10.orig/sound/pci/ice1712/aureon.c
+++ usb-3.10/sound/pci/ice1712/aureon.c
@@ -1937,9 +1937,9 @@ static int aureon_add_controls(struct sn
snd_ice1712_save_gpio_status(ice);
id = aureon_cs8415_get(ice, CS8415_ID);
if (id != 0x41)
- snd_printk(KERN_INFO "No CS8415 chip. Skipping CS8415 controls.\n");
+ pr_info("No CS8415 chip. Skipping CS8415 controls.\n");
else if ((id & 0x0F) != 0x01)
- snd_printk(KERN_INFO "Detected unsupported CS8415 rev. (%c)\n", (char)((id & 0x0F) + 'A' - 1));
+ pr_info("Detected unsupported CS8415 rev. (%c)\n", (char)((id & 0x0F) + 'A' - 1));
else {
for (i = 0; i < ARRAY_SIZE(cs8415_controls); i++) {
struct snd_kcontrol *kctl;
Index: usb-3.10/sound/pci/oxygen/oxygen_lib.c
===================================================================
--- usb-3.10.orig/sound/pci/oxygen/oxygen_lib.c
+++ usb-3.10/sound/pci/oxygen/oxygen_lib.c
@@ -313,7 +313,7 @@ static void oxygen_restore_eeprom(struct
oxygen_clear_bits8(chip, OXYGEN_MISC,
OXYGEN_MISC_WRITE_PCI_SUBID);
- snd_printk(KERN_INFO "EEPROM ID restored\n");
+ pr_info("EEPROM ID restored\n");
}
}
Index: usb-3.10/sound/pci/rme9652/hdsp.c
===================================================================
--- usb-3.10.orig/sound/pci/rme9652/hdsp.c
+++ usb-3.10/sound/pci/rme9652/hdsp.c
@@ -770,7 +770,7 @@ static int snd_hdsp_load_firmware_from_c
}
if (hdsp->state & HDSP_InitializationComplete) {
- snd_printk(KERN_INFO "Hammerfall-DSP: firmware loaded from cache, restoring defaults\n");
+ pr_info("Hammerfall-DSP: firmware loaded from cache, restoring defaults\n");
spin_lock_irqsave(&hdsp->lock, flags);
snd_hdsp_set_defaults(hdsp);
spin_unlock_irqrestore(&hdsp->lock, flags);
@@ -1153,11 +1153,11 @@ static int hdsp_set_rate(struct hdsp *hd
int spdif_freq = hdsp_spdif_sample_rate(hdsp);
if ((spdif_freq == external_freq*2) && (hdsp_autosync_ref(hdsp) >= HDSP_AUTOSYNC_FROM_ADAT1))
- snd_printk(KERN_INFO "Hammerfall-DSP: Detected ADAT in double speed mode\n");
+ pr_info("Hammerfall-DSP: Detected ADAT in double speed mode\n");
else if (hdsp->io_type == H9632 && (spdif_freq == external_freq*4) && (hdsp_autosync_ref(hdsp) >= HDSP_AUTOSYNC_FROM_ADAT1))
- snd_printk(KERN_INFO "Hammerfall-DSP: Detected ADAT in quad speed mode\n");
+ pr_info("Hammerfall-DSP: Detected ADAT in quad speed mode\n");
else if (rate != external_freq) {
- snd_printk(KERN_INFO "Hammerfall-DSP: No AutoSync source for requested rate\n");
+ pr_info("Hammerfall-DSP: No AutoSync source for requested rate\n");
return -1;
}
}
@@ -4863,7 +4863,7 @@ static int snd_hdsp_hwdep_ioctl(struct s
if (hdsp->state & (HDSP_FirmwareCached | HDSP_FirmwareLoaded))
return -EBUSY;
- snd_printk(KERN_INFO "Hammerfall-DSP: initializing firmware upload\n");
+ pr_info("Hammerfall-DSP: initializing firmware upload\n");
firmware = (struct hdsp_firmware __user *)argp;
if (get_user(firmware_data, &firmware->firmware_data))
@@ -5291,12 +5291,12 @@ static int snd_hdsp_create(struct snd_ca
/* init is complete, we return */
return 0;
/* we defer initialization */
- snd_printk(KERN_INFO "Hammerfall-DSP: card initialization pending : waiting for firmware\n");
+ pr_info("Hammerfall-DSP: card initialization pending : waiting for firmware\n");
if ((err = snd_hdsp_create_hwdep(card, hdsp)) < 0)
return err;
return 0;
} else {
- snd_printk(KERN_INFO "Hammerfall-DSP: Firmware already present, initializing card.\n");
+ pr_info("Hammerfall-DSP: Firmware already present, initializing card.\n");
if (hdsp_read(hdsp, HDSP_status2Register) & HDSP_version2)
hdsp->io_type = RPM;
else if (hdsp_read(hdsp, HDSP_status2Register) & HDSP_version1)
Index: usb-3.10/sound/pci/rme9652/hdspm.c
===================================================================
--- usb-3.10.orig/sound/pci/rme9652/hdspm.c
+++ usb-3.10/sound/pci/rme9652/hdspm.c
@@ -5143,7 +5143,7 @@ static irqreturn_t snd_hdspm_interrupt(i
* 0 64 ~3998231 ~8191558
**/
/*
- snd_printk(KERN_INFO "snd_hdspm_interrupt %llu @ %llx\n",
+ pr_info("snd_hdspm_interrupt %llu @ %llx\n",
now-hdspm->last_interrupt, status & 0xFFC0);
hdspm->last_interrupt = now;
*/
@@ -5280,7 +5280,7 @@ static int snd_hdspm_hw_params(struct sn
spin_lock_irq(&hdspm->lock);
err = hdspm_set_rate(hdspm, params_rate(params), 0);
if (err < 0) {
- snd_printk(KERN_INFO "err on hdspm_set_rate: %d\n", err);
+ pr_info("err on hdspm_set_rate: %d\n", err);
spin_unlock_irq(&hdspm->lock);
_snd_pcm_hw_param_setempty(params,
SNDRV_PCM_HW_PARAM_RATE);
@@ -5291,7 +5291,7 @@ static int snd_hdspm_hw_params(struct sn
err = hdspm_set_interrupt_interval(hdspm,
params_period_size(params));
if (err < 0) {
- snd_printk(KERN_INFO "err on hdspm_set_interrupt_interval: %d\n", err);
+ pr_info("err on hdspm_set_interrupt_interval: %d\n", err);
_snd_pcm_hw_param_setempty(params,
SNDRV_PCM_HW_PARAM_PERIOD_SIZE);
return err;
@@ -5307,7 +5307,7 @@ static int snd_hdspm_hw_params(struct sn
err =
snd_pcm_lib_malloc_pages(substream, HDSPM_DMA_AREA_BYTES);
if (err < 0) {
- snd_printk(KERN_INFO "err on snd_pcm_lib_malloc_pages: %d\n", err);
+ pr_info("err on snd_pcm_lib_malloc_pages: %d\n", err);
return err;
}
@@ -5354,12 +5354,12 @@ static int snd_hdspm_hw_params(struct sn
/* Switch to native float format if requested */
if (SNDRV_PCM_FORMAT_FLOAT_LE == params_format(params)) {
if (!(hdspm->control_register & HDSPe_FLOAT_FORMAT))
- snd_printk(KERN_INFO "hdspm: Switching to native 32bit LE float format.\n");
+ pr_info("hdspm: Switching to native 32bit LE float format.\n");
hdspm->control_register |= HDSPe_FLOAT_FORMAT;
} else if (SNDRV_PCM_FORMAT_S32_LE == params_format(params)) {
if (hdspm->control_register & HDSPe_FLOAT_FORMAT)
- snd_printk(KERN_INFO "hdspm: Switching to native 32bit LE integer format.\n");
+ pr_info("hdspm: Switching to native 32bit LE integer format.\n");
hdspm->control_register &= ~HDSPe_FLOAT_FORMAT;
}
@@ -5402,12 +5402,12 @@ static int snd_hdspm_channel_info(struct
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
if (snd_BUG_ON(info->channel >= hdspm->max_channels_out)) {
- snd_printk(KERN_INFO "snd_hdspm_channel_info: output channel out of range (%d)\n", info->channel);
+ pr_info("snd_hdspm_channel_info: output channel out of range (%d)\n", info->channel);
return -EINVAL;
}
if (hdspm->channel_map_out[info->channel] < 0) {
- snd_printk(KERN_INFO "snd_hdspm_channel_info: output channel %d mapped out\n", info->channel);
+ pr_info("snd_hdspm_channel_info: output channel %d mapped out\n", info->channel);
return -EINVAL;
}
@@ -5415,12 +5415,12 @@ static int snd_hdspm_channel_info(struct
HDSPM_CHANNEL_BUFFER_BYTES;
} else {
if (snd_BUG_ON(info->channel >= hdspm->max_channels_in)) {
- snd_printk(KERN_INFO "snd_hdspm_channel_info: input channel out of range (%d)\n", info->channel);
+ pr_info("snd_hdspm_channel_info: input channel out of range (%d)\n", info->channel);
return -EINVAL;
}
if (hdspm->channel_map_in[info->channel] < 0) {
- snd_printk(KERN_INFO "snd_hdspm_channel_info: input channel %d mapped out\n", info->channel);
+ pr_info("snd_hdspm_channel_info: input channel %d mapped out\n", info->channel);
return -EINVAL;
}
@@ -6460,7 +6460,7 @@ static int snd_hdspm_create(struct snd_c
case AIO:
if (0 == (hdspm_read(hdspm, HDSPM_statusRegister2) & HDSPM_s2_AEBI_D)) {
- snd_printk(KERN_INFO "HDSPM: AEB input board found, but not supported\n");
+ pr_info("HDSPM: AEB input board found, but not supported\n");
}
hdspm->ss_in_channels = AIO_IN_SS_CHANNELS;
@@ -6531,7 +6531,7 @@ static int snd_hdspm_create(struct snd_c
if (NULL != hdspm->tco) {
hdspm_tco_write(hdspm);
}
- snd_printk(KERN_INFO "HDSPM: AIO/RayDAT TCO module found\n");
+ pr_info("HDSPM: AIO/RayDAT TCO module found\n");
} else {
hdspm->tco = NULL;
}
@@ -6545,7 +6545,7 @@ static int snd_hdspm_create(struct snd_c
if (NULL != hdspm->tco) {
hdspm_tco_write(hdspm);
}
- snd_printk(KERN_INFO "HDSPM: MADI TCO module found\n");
+ pr_info("HDSPM: MADI TCO module found\n");
} else {
hdspm->tco = NULL;
}
Index: usb-3.10/sound/pci/sonicvibes.c
===================================================================
--- usb-3.10.orig/sound/pci/sonicvibes.c
+++ usb-3.10/sound/pci/sonicvibes.c
@@ -1310,12 +1310,12 @@ static int snd_sonicvibes_create(struct
if (!dmaa) {
dmaa = dmaio;
dmaio += 0x10;
- snd_printk(KERN_INFO "BIOS did not allocate DDMA channel A i/o, allocated at 0x%x\n", dmaa);
+ pr_info("BIOS did not allocate DDMA channel A i/o, allocated at 0x%x\n", dmaa);
}
if (!dmac) {
dmac = dmaio;
dmaio += 0x10;
- snd_printk(KERN_INFO "BIOS did not allocate DDMA channel C i/o, allocated at 0x%x\n", dmac);
+ pr_info("BIOS did not allocate DDMA channel C i/o, allocated at 0x%x\n", dmac);
}
pci_write_config_dword(pci, 0x40, dmaa);
pci_write_config_dword(pci, 0x48, dmac);
Index: usb-3.10/sound/usb/card.c
===================================================================
--- usb-3.10.orig/sound/usb/card.c
+++ usb-3.10/sound/usb/card.c
@@ -227,7 +227,7 @@ static int snd_usb_create_streams(struct
struct uac1_ac_header_descriptor *h1 = control_header;
if (!h1->bInCollection) {
- snd_printk(KERN_INFO "skipping empty audio interface (v1)\n");
+ pr_info("skipping empty audio interface (v1)\n");
return -EINVAL;
}
Index: usb-3.10/sound/usb/clock.c
===================================================================
--- usb-3.10.orig/sound/usb/clock.c
+++ usb-3.10/sound/usb/clock.c
@@ -237,8 +237,7 @@ static int __uac_clock_find_source(struc
if (err < 0)
continue;
- snd_printk(KERN_INFO
- "usb-audio:%d: found and selected valid clock source %d\n",
+ pr_info("usb-audio:%d: found and selected valid clock source %d\n",
chip->dev->devnum, ret);
return ret;
}
Index: usb-3.10/sound/usb/format.c
===================================================================
--- usb-3.10.orig/sound/usb/format.c
+++ usb-3.10/sound/usb/format.c
@@ -84,7 +84,7 @@ static u64 parse_audio_format_i_type(str
sample_width == 24 && sample_bytes == 2)
sample_bytes = 3;
else if (sample_width > sample_bytes * 8) {
- snd_printk(KERN_INFO "%d:%u:%d : sample bitwidth %d in over sample bytes %d\n",
+ pr_info("%d:%u:%d : sample bitwidth %d in over sample bytes %d\n",
chip->dev->devnum, fp->iface, fp->altsetting,
sample_width, sample_bytes);
}
@@ -109,7 +109,7 @@ static u64 parse_audio_format_i_type(str
pcm_formats |= SNDRV_PCM_FMTBIT_S32_LE;
break;
default:
- snd_printk(KERN_INFO "%d:%u:%d : unsupported sample bitwidth %d in %d bytes\n",
+ pr_info("%d:%u:%d : unsupported sample bitwidth %d in %d bytes\n",
chip->dev->devnum, fp->iface, fp->altsetting,
sample_width, sample_bytes);
break;
@@ -133,7 +133,7 @@ static u64 parse_audio_format_i_type(str
pcm_formats |= SNDRV_PCM_FMTBIT_MU_LAW;
}
if (format & ~0x3f) {
- snd_printk(KERN_INFO "%d:%u:%d : unsupported format bits %#x\n",
+ pr_info("%d:%u:%d : unsupported format bits %#x\n",
chip->dev->devnum, fp->iface, fp->altsetting, format);
}
Index: usb-3.10/sound/usb/mixer.c
===================================================================
--- usb-3.10.orig/sound/usb/mixer.c
+++ usb-3.10/sound/usb/mixer.c
@@ -839,8 +839,7 @@ static void volume_control_quirks(struct
case USB_ID(0x0763, 0x2081): /* M-Audio Fast Track Ultra 8R */
case USB_ID(0x0763, 0x2080): /* M-Audio Fast Track Ultra */
if (strcmp(kctl->id.name, "Effect Duration") == 0) {
- snd_printk(KERN_INFO
- "usb-audio: set quirk for FTU Effect Duration\n");
+ pr_info("usb-audio: set quirk for FTU Effect Duration\n");
cval->min = 0x0000;
cval->max = 0x7f00;
cval->res = 0x0100;
@@ -848,8 +847,7 @@ static void volume_control_quirks(struct
}
if (strcmp(kctl->id.name, "Effect Volume") == 0 ||
strcmp(kctl->id.name, "Effect Feedback Volume") == 0) {
- snd_printk(KERN_INFO
- "usb-audio: set quirks for FTU Effect Feedback/Volume\n");
+ pr_info("usb-audio: set quirks for FTU Effect Feedback/Volume\n");
cval->min = 0x00;
cval->max = 0x7f;
break;
@@ -867,16 +865,14 @@ static void volume_control_quirks(struct
*/
if (!strcmp(kctl->id.name, "PCM Playback Volume") &&
cval->min == -15616) {
- snd_printk(KERN_INFO
- "set volume quirk for UDA1321/N101 chip\n");
+ pr_info("set volume quirk for UDA1321/N101 chip\n");
cval->max = -256;
}
break;
case USB_ID(0x046d, 0x09a4):
if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
- snd_printk(KERN_INFO
- "set volume quirk for QuickCam E3500\n");
+ pr_info("set volume quirk for QuickCam E3500\n");
cval->min = 6080;
cval->max = 8768;
cval->res = 192;
@@ -892,8 +888,7 @@ static void volume_control_quirks(struct
* Proboly there is some logitech magic behind this number --fishor
*/
if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
- snd_printk(KERN_INFO
- "set resolution quirk: cval->res = 384\n");
+ pr_info("set resolution quirk: cval->res = 384\n");
cval->res = 384;
}
break;
@@ -1366,14 +1361,12 @@ static int parse_audio_feature_unit(stru
/* master configuration quirks */
switch (state->chip->usb_id) {
case USB_ID(0x08bb, 0x2702):
- snd_printk(KERN_INFO
- "usbmixer: master volume quirk for PCM2702 chip\n");
+ pr_info("usbmixer: master volume quirk for PCM2702 chip\n");
/* disable non-functional volume control */
master_bits &= ~UAC_CONTROL_BIT(UAC_FU_VOLUME);
break;
case USB_ID(0x1130, 0xf211):
- snd_printk(KERN_INFO
- "usbmixer: volume control quirk for Tenx TP6911 Audio Headset\n");
+ pr_info("usbmixer: volume control quirk for Tenx TP6911 Audio Headset\n");
/* disable non-functional volume control */
channels = 0;
break;
Index: usb-3.10/sound/usb/quirks.c
===================================================================
--- usb-3.10.orig/sound/usb/quirks.c
+++ usb-3.10/sound/usb/quirks.c
@@ -381,7 +381,7 @@ static int snd_usb_fasttrackpro_boot_qui
int err;
if (dev->actconfig->desc.bConfigurationValue == 1) {
- snd_printk(KERN_INFO "usb-audio: "
+ pr_info("usb-audio: "
"Fast Track Pro switching to config #2\n");
/* This function has to be available by the usb core module.
* if it is not avialable the boot quirk has to be left out
@@ -397,7 +397,7 @@ static int snd_usb_fasttrackpro_boot_qui
configuration */
return -ENODEV;
} else
- snd_printk(KERN_INFO "usb-audio: Fast Track Pro config OK\n");
+ pr_info("usb-audio: Fast Track Pro config OK\n");
return 0;
}
@@ -593,7 +593,7 @@ static int snd_usb_mbox2_boot_quirk(stru
mbox2_setup_48_24_magic(dev);
- snd_printk(KERN_INFO "usb-audio: Digidesign Mbox 2: 24bit 48kHz");
+ pr_info("usb-audio: Digidesign Mbox 2: 24bit 48kHz");
return 0; /* Successful boot */
}
Index: usb-3.10/sound/isa/wavefront/wavefront.c
===================================================================
--- usb-3.10.orig/sound/isa/wavefront/wavefront.c
+++ usb-3.10/sound/isa/wavefront/wavefront.c
@@ -195,7 +195,7 @@ snd_wavefront_pnp (int dev, snd_wavefron
cs4232_mpu_irq[dev] = pnp_irq(pdev, 0);
}
- snd_printk (KERN_INFO "CS4232 MPU: port=0x%lx, irq=%i\n",
+ pr_info("CS4232 MPU: port=0x%lx, irq=%i\n",
cs4232_mpu_port[dev],
cs4232_mpu_irq[dev]);
}
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 2/2 v.2] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 15:03 ` Takashi Iwai
2013-06-04 17:20 ` [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info(" Alan Stern
@ 2013-06-04 17:20 ` Alan Stern
2013-06-04 19:32 ` [PATCH] " Alan Stern
2 siblings, 0 replies; 44+ messages in thread
From: Alan Stern @ 2013-06-04 17:20 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
According to Takashi Iwai, CONFIG_SND_VERBOSE_PRINTK is "useless". All
it does is add filenames and line numbers to certain log messages. If
these messages are supposed to have filenames and line numbers, they
should have them all the time. (These messages are all errors and
warnings, anyway.)
This patch gets rid of the symbol, acting as though it is always
enabled.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Joe Perches <joe@perches.com>
v.2: Enable the filenames and line numbers always, rather than
eliminating them.
---
[as1688b]
Documentation/DocBook/writing-an-alsa-driver.tmpl | 3 +--
arch/arm/configs/eseries_pxa_defconfig | 1 -
arch/arm/configs/omap2plus_defconfig | 1 -
arch/arm/configs/s3c2410_defconfig | 1 -
arch/arm/configs/trizeps4_defconfig | 1 -
arch/ia64/configs/generic_defconfig | 1 -
arch/ia64/configs/gensparse_defconfig | 1 -
arch/mips/configs/pnx8335_stb225_defconfig | 1 -
arch/powerpc/configs/ppc6xx_defconfig | 1 -
arch/sh/configs/edosk7760_defconfig | 1 -
arch/sh/configs/sh7785lcr_32bit_defconfig | 1 -
include/sound/core.h | 8 +-------
sound/core/Kconfig | 9 ---------
sound/core/misc.c | 10 ----------
sound/pci/hda/hda_intel.c | 4 ----
15 files changed, 2 insertions(+), 42 deletions(-)
Index: usb-3.10/Documentation/DocBook/writing-an-alsa-driver.tmpl
===================================================================
--- usb-3.10.orig/Documentation/DocBook/writing-an-alsa-driver.tmpl
+++ usb-3.10/Documentation/DocBook/writing-an-alsa-driver.tmpl
@@ -6087,8 +6087,7 @@ struct _snd_pcm_runtime {
<title><function>snd_printk()</function> and friends</title>
<para>
ALSA provides a verbose version of the
- <function>printk()</function> function. If a kernel config
- <constant>CONFIG_SND_VERBOSE_PRINTK</constant> is set, this
+ <function>printk()</function> function. This
function prints the given message together with the file name
and the line of the caller. The <constant>KERN_XXX</constant>
prefix is processed as
Index: usb-3.10/arch/arm/configs/eseries_pxa_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/eseries_pxa_defconfig
+++ usb-3.10/arch/arm/configs/eseries_pxa_defconfig
@@ -90,7 +90,6 @@ CONFIG_SND=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_DYNAMIC_MINORS=y
-CONFIG_SND_VERBOSE_PRINTK=y
# CONFIG_SND_PCMCIA is not set
CONFIG_SND_SOC=m
CONFIG_SND_PXA2XX_SOC=m
Index: usb-3.10/arch/arm/configs/omap2plus_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/omap2plus_defconfig
+++ usb-3.10/arch/arm/configs/omap2plus_defconfig
@@ -192,7 +192,6 @@ CONFIG_SOUND=m
CONFIG_SND=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_USB_AUDIO=m
CONFIG_SND_SOC=m
Index: usb-3.10/arch/arm/configs/s3c2410_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/s3c2410_defconfig
+++ usb-3.10/arch/arm/configs/s3c2410_defconfig
@@ -315,7 +315,6 @@ CONFIG_SND_SEQUENCER=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
# CONFIG_SND_DRIVERS is not set
# CONFIG_SND_ARM is not set
# CONFIG_SND_SPI is not set
Index: usb-3.10/arch/arm/configs/trizeps4_defconfig
===================================================================
--- usb-3.10.orig/arch/arm/configs/trizeps4_defconfig
+++ usb-3.10/arch/arm/configs/trizeps4_defconfig
@@ -159,7 +159,6 @@ CONFIG_SND=y
CONFIG_SND_SEQUENCER=y
CONFIG_SND_MIXER_OSS=y
CONFIG_SND_PCM_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_PXA2XX_AC97=y
CONFIG_SND_USB_AUDIO=m
Index: usb-3.10/arch/ia64/configs/generic_defconfig
===================================================================
--- usb-3.10.orig/arch/ia64/configs/generic_defconfig
+++ usb-3.10/arch/ia64/configs/generic_defconfig
@@ -127,7 +127,6 @@ CONFIG_SND_SEQ_DUMMY=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DUMMY=m
CONFIG_SND_VIRMIDI=m
CONFIG_SND_MTPAV=m
Index: usb-3.10/arch/ia64/configs/gensparse_defconfig
===================================================================
--- usb-3.10.orig/arch/ia64/configs/gensparse_defconfig
+++ usb-3.10/arch/ia64/configs/gensparse_defconfig
@@ -115,7 +115,6 @@ CONFIG_SND_SEQ_DUMMY=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DUMMY=m
CONFIG_SND_VIRMIDI=m
CONFIG_SND_MTPAV=m
Index: usb-3.10/arch/mips/configs/pnx8335_stb225_defconfig
===================================================================
--- usb-3.10.orig/arch/mips/configs/pnx8335_stb225_defconfig
+++ usb-3.10/arch/mips/configs/pnx8335_stb225_defconfig
@@ -73,7 +73,6 @@ CONFIG_SND_SEQUENCER=m
CONFIG_SND_MIXER_OSS=m
CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_EXT2_FS=m
# CONFIG_DNOTIFY is not set
Index: usb-3.10/arch/powerpc/configs/ppc6xx_defconfig
===================================================================
--- usb-3.10.orig/arch/powerpc/configs/ppc6xx_defconfig
+++ usb-3.10/arch/powerpc/configs/ppc6xx_defconfig
@@ -870,7 +870,6 @@ CONFIG_SND_PCM_OSS=m
CONFIG_SND_SEQUENCER_OSS=y
CONFIG_SND_DYNAMIC_MINORS=y
# CONFIG_SND_SUPPORT_OLD_API is not set
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_DEBUG_VERBOSE=y
CONFIG_SND_PCM_XRUN_DEBUG=y
Index: usb-3.10/arch/sh/configs/edosk7760_defconfig
===================================================================
--- usb-3.10.orig/arch/sh/configs/edosk7760_defconfig
+++ usb-3.10/arch/sh/configs/edosk7760_defconfig
@@ -90,7 +90,6 @@ CONFIG_SOUND=y
CONFIG_SND=y
# CONFIG_SND_SUPPORT_OLD_API is not set
# CONFIG_SND_VERBOSE_PROCFS is not set
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_SOC=y
# CONFIG_HID_SUPPORT is not set
# CONFIG_USB_SUPPORT is not set
Index: usb-3.10/arch/sh/configs/sh7785lcr_32bit_defconfig
===================================================================
--- usb-3.10.orig/arch/sh/configs/sh7785lcr_32bit_defconfig
+++ usb-3.10/arch/sh/configs/sh7785lcr_32bit_defconfig
@@ -105,7 +105,6 @@ CONFIG_SND_HRTIMER=y
CONFIG_SND_DYNAMIC_MINORS=y
# CONFIG_SND_SUPPORT_OLD_API is not set
# CONFIG_SND_VERBOSE_PROCFS is not set
-CONFIG_SND_VERBOSE_PRINTK=y
CONFIG_SND_DEBUG=y
CONFIG_SND_DEBUG_VERBOSE=y
# CONFIG_SND_DRIVERS is not set
Index: usb-3.10/sound/core/Kconfig
===================================================================
--- usb-3.10.orig/sound/core/Kconfig
+++ usb-3.10/sound/core/Kconfig
@@ -173,15 +173,6 @@ config SND_VERBOSE_PROCFS
useful information to developers when a problem occurs). On the
other side, it makes the ALSA subsystem larger.
-config SND_VERBOSE_PRINTK
- bool "Verbose printk"
- help
- Say Y here to enable verbose log messages. These messages
- will help to identify source file and position containing
- printed messages.
-
- You don't need this unless you're debugging ALSA.
-
config SND_DEBUG
bool "Debug"
help
Index: usb-3.10/sound/pci/hda/hda_intel.c
===================================================================
--- usb-3.10.orig/sound/pci/hda/hda_intel.c
+++ usb-3.10/sound/pci/hda/hda_intel.c
@@ -189,11 +189,7 @@ MODULE_SUPPORTED_DEVICE("{{Intel, ICH6},
"{ULI, M5461}}");
MODULE_DESCRIPTION("Intel HDA driver");
-#ifdef CONFIG_SND_VERBOSE_PRINTK
#define SFX /* nop */
-#else
-#define SFX "hda-intel "
-#endif
#if defined(CONFIG_PM) && defined(CONFIG_VGA_SWITCHEROO)
#ifdef CONFIG_SND_HDA_CODEC_HDMI
Index: usb-3.10/include/sound/core.h
===================================================================
--- usb-3.10.orig/include/sound/core.h
+++ usb-3.10/include/sound/core.h
@@ -335,21 +335,15 @@ enum {
SND_PR_VERBOSE,
};
-#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
__printf(4, 5)
void __snd_printk(unsigned int level, const char *file, int line,
const char *format, ...);
-#else
-#define __snd_printk(level, file, line, format, args...) \
- printk(format, ##args)
-#endif
/**
* snd_printk - printk wrapper
* @fmt: format string
*
- * Works like printk() but prints the file and the line of the caller
- * when configured with CONFIG_SND_VERBOSE_PRINTK.
+ * Works like printk() but prints the file and the line of the caller.
*/
#define snd_printk(fmt, args...) \
__snd_printk(0, __FILE__, __LINE__, fmt, ##args)
Index: usb-3.10/sound/core/misc.c
===================================================================
--- usb-3.10.orig/sound/core/misc.c
+++ usb-3.10/sound/core/misc.c
@@ -51,7 +51,6 @@ void release_and_free_resource(struct re
EXPORT_SYMBOL(release_and_free_resource);
-#ifdef CONFIG_SND_VERBOSE_PRINTK
/* strip the leading path if the given path is absolute */
static const char *sanity_file_name(const char *path)
{
@@ -60,18 +59,14 @@ static const char *sanity_file_name(cons
else
return path;
}
-#endif
-#if defined(CONFIG_SND_DEBUG) || defined(CONFIG_SND_VERBOSE_PRINTK)
void __snd_printk(unsigned int level, const char *path, int line,
const char *format, ...)
{
va_list args;
-#ifdef CONFIG_SND_VERBOSE_PRINTK
int kern_level;
struct va_format vaf;
char verbose_fmt[] = KERN_DEFAULT "ALSA %s:%d %pV";
-#endif
#ifdef CONFIG_SND_DEBUG
if (debug < level)
@@ -79,7 +74,6 @@ void __snd_printk(unsigned int level, co
#endif
va_start(args, format);
-#ifdef CONFIG_SND_VERBOSE_PRINTK
vaf.fmt = format;
vaf.va = &args;
@@ -92,13 +86,9 @@ void __snd_printk(unsigned int level, co
memcpy(verbose_fmt, KERN_DEBUG, sizeof(KERN_DEBUG) - 1);
printk(verbose_fmt, sanity_file_name(path), line, &vaf);
-#else
- vprintk(format, args);
-#endif
va_end(args);
}
EXPORT_SYMBOL_GPL(__snd_printk);
-#endif
#ifdef CONFIG_PCI
#include <linux/pci.h>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 15:03 ` Takashi Iwai
2013-06-04 17:20 ` [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info(" Alan Stern
2013-06-04 17:20 ` [PATCH 2/2 v.2] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
@ 2013-06-04 19:32 ` Alan Stern
2013-06-04 19:45 ` Joe Perches
2 siblings, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-04 19:32 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
On Tue, 4 Jun 2013, Takashi Iwai wrote:
> I wouldn't mind but some might do :)
>
> So we should clean up the excessive usage of snd_printk() and convert
> them to the standard printk & co at first.
This part is now done (already posted).
The next step involves snd_printd() and snd_printdd(). Although these
are clearly intended to be used for debugging messages -- that's why
the names end in 'd' -- they nevertheless take a KERN_* level
indicator.
This doesn't make much sense. Are these debugging messages or aren't
they? If they are, they should always use KERN_DEBUG.
As I see it, the best thing to do is replace snd_printd() with a
dev_dbg() wrapper and eliminate the level indicators. This will
require changing all the Makefiles; they will need to have a line
saying
ccflags-$(CONFIG_SND_DEBUG) := -DDEBUG
at the top, because dev_dbg() is defined to do nothing if DEBUG isn't
defined as a preprocessor symbol.
Similarly, snd_printdd() can be translated to a macro that expands to
dev_dbg when CONFIG_SND_VERBOSE_DEBUG is enabled, and to nothing
otherwise.
Does this seem reasonable?
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 19:32 ` [PATCH] " Alan Stern
@ 2013-06-04 19:45 ` Joe Perches
2013-06-04 20:54 ` Alan Stern
0 siblings, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-04 19:45 UTC (permalink / raw)
To: Alan Stern; +Cc: Takashi Iwai, alsa-devel
On Tue, 2013-06-04 at 15:32 -0400, Alan Stern wrote:
> Similarly, snd_printdd() can be translated to a macro that expands to
> dev_dbg when CONFIG_SND_VERBOSE_DEBUG is enabled, and to nothing
> otherwise.
>
> Does this seem reasonable?
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 */
------------------------------------------
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.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 19:45 ` Joe Perches
@ 2013-06-04 20:54 ` Alan Stern
2013-06-04 21:19 ` Joe Perches
2013-06-05 6:04 ` Takashi Iwai
0 siblings, 2 replies; 44+ messages in thread
From: Alan Stern @ 2013-06-04 20:54 UTC (permalink / raw)
To: Joe Perches; +Cc: Takashi Iwai, alsa-devel
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 20:54 ` Alan Stern
@ 2013-06-04 21:19 ` Joe Perches
2013-06-06 20:42 ` Alan Stern
2013-06-05 6:04 ` Takashi Iwai
1 sibling, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-04 21:19 UTC (permalink / raw)
To: Alan Stern; +Cc: Takashi Iwai, alsa-devel
On Tue, 2013-06-04 at 16:54 -0400, Alan Stern wrote:
> 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.
Not really.
Think of CONFIG_SND_DEBUG as a level. (0, 1, 2)
or maybe think of it as CONFIG_SND_DEBUG_VERBOSITY.
There could still be ability to have CONFIG_SND_DEBUG
limit the compiled-in messages to those below the
#define value and still then have runtime control over
which ones are displayed.
> 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?
Yes. My perspective is _all_ debugging messages should
be at KERN_DEBUG.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info("
2013-06-04 17:20 ` [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info(" Alan Stern
@ 2013-06-05 5:52 ` Takashi Iwai
2013-06-05 6:07 ` Joe Perches
2013-06-06 20:54 ` Alan Stern
0 siblings, 2 replies; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 5:52 UTC (permalink / raw)
To: Alan Stern; +Cc: Joe Perches, alsa-devel
At Tue, 4 Jun 2013 13:20:40 -0400 (EDT),
Alan Stern wrote:
>
> The snd_printk() function prints kernel log messages, including the
> filename and line number if CONFIG_SND_PRINTK_VERBOSE is enabled.
> This may make sense for errors and warnings, but not for informational
> messages. For those, a simple pr_info() is what we want.
>
> This patch mechanically converts all occurrences of
> "snd_printk(KERN_INFO" to "pr_info(". It doesn't try to tell whether
> the message really is informational; it relies on the existing
> KERN_INFO tag.
I agree conversion in this way. But looking at the patch, some places
should be better convert with pr_warning() or pr_err().
Also, many places miss proper prefix, thus you'll still see the
original issue the thread started from (no clue who prints the stuff).
This is because originally snd_printk() printed the prefix and line
number always. CONFIG_SND_VERBOSE_PRINTK was introduced later since
some people complained about too verbose output, IIRC, while many
codes weren't fixed to give a proper prefix with
CONFIG_SND_VERBOSE_PRINTK=n.
Just taking a quick glance:
> --- usb-3.10.orig/sound/isa/opti9xx/miro.c
> +++ usb-3.10/sound/isa/opti9xx/miro.c
> @@ -1346,11 +1346,11 @@ static int snd_miro_probe(struct snd_car
> default:
> sprintf(card->shortname,
> "unknown miro");
> - snd_printk(KERN_INFO "unknown miro aci id\n");
> + pr_info("unknown miro aci id\n");
> break;
> }
> } else {
> - snd_printk(KERN_INFO "found unsupported aci card\n");
> + pr_info("found unsupported aci card\n");
> sprintf(card->shortname, "unknown Cardinal Technologies");
These need proper prefix, and should be rather pr_warning().
> }
>
> Index: usb-3.10/sound/isa/sb/sb16.c
> ===================================================================
> --- usb-3.10.orig/sound/isa/sb/sb16.c
> +++ usb-3.10/sound/isa/sb/sb16.c
> @@ -435,7 +435,7 @@ static int snd_sb16_probe(struct snd_car
> chip->csp = xcsp->private_data;
> chip->hardware = SB_HW_16CSP;
> } else {
> - snd_printk(KERN_INFO PFX "warning - CSP chip not detected on soundcard #%i\n", dev + 1);
> + pr_info(PFX "warning - CSP chip not detected on soundcard #%i\n", dev + 1);
Ditto, pr_warning().
> }
> }
> #endif
> Index: usb-3.10/sound/isa/sb/sb_common.c
> ===================================================================
> --- usb-3.10.orig/sound/isa/sb/sb_common.c
> +++ usb-3.10/sound/isa/sb/sb_common.c
> @@ -154,7 +154,7 @@ static int snd_sbdsp_probe(struct snd_sb
> str = "16";
> break;
> default:
> - snd_printk(KERN_INFO "SB [0x%lx]: unknown DSP chip version %i.%i\n",
> + pr_info("SB [0x%lx]: unknown DSP chip version %i.%i\n",
> chip->port, major, minor);
> return -ENODEV;
Looks like pr_err().
> }
> Index: usb-3.10/sound/isa/sscape.c
> ===================================================================
> --- usb-3.10.orig/sound/isa/sscape.c
> +++ usb-3.10/sound/isa/sscape.c
> @@ -590,7 +590,7 @@ static int sscape_upload_microcode(struc
> }
> err = upload_dma_data(sscape, init_fw->data, init_fw->size);
> if (err == 0)
> - snd_printk(KERN_INFO "sscape: MIDI firmware loaded %d KBs\n",
> + pr_info("sscape: MIDI firmware loaded %d KBs\n",
> init_fw->size >> 10);
>
> release_firmware(init_fw);
> @@ -1251,7 +1251,7 @@ static int sscape_pnp_detect(struct pnp_
>
> if (!pnp_is_active(dev)) {
> if (pnp_activate_dev(dev) < 0) {
> - snd_printk(KERN_INFO "sscape: device is inactive\n");
> + pr_info("sscape: device is inactive\n");
Again pr_err().
> return -EBUSY;
> }
> }
> Index: usb-3.10/sound/pci/asihpi/asihpi.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/asihpi/asihpi.c
> +++ usb-3.10/sound/pci/asihpi/asihpi.c
> @@ -1347,7 +1347,7 @@ static inline int ctl_add(struct snd_car
> if (err < 0)
> return err;
> else if (mixer_dump)
> - snd_printk(KERN_INFO "added %s(%d)\n", ctl->name, ctl->index);
> + pr_info("added %s(%d)\n", ctl->name, ctl->index);
Need a prefix.
>
> return 0;
> }
> @@ -2583,8 +2583,7 @@ static int snd_card_asihpi_mixer_new(str
> if (err) {
> if (err == HPI_ERROR_CONTROL_DISABLED) {
> if (mixer_dump)
> - snd_printk(KERN_INFO
> - "Disabled HPI Control(%d)\n",
> + pr_info("Disabled HPI Control(%d)\n",
> idx);
> continue;
> } else
> @@ -2648,8 +2647,7 @@ static int snd_card_asihpi_mixer_new(str
> case HPI_CONTROL_COMPANDER:
> default:
> if (mixer_dump)
> - snd_printk(KERN_INFO
> - "Untranslated HPI Control"
> + pr_info("Untranslated HPI Control"
> "(%d) %d %d %d %d %d\n",
> idx,
> hpi_ctl.control_type,
> @@ -2665,7 +2663,7 @@ static int snd_card_asihpi_mixer_new(str
> if (HPI_ERROR_INVALID_OBJ_INDEX != err)
> hpi_handle_error(err);
>
> - snd_printk(KERN_INFO "%d mixer controls found\n", idx);
> + pr_info("%d mixer controls found\n", idx);
>
> return 0;
> }
> @@ -2844,7 +2842,7 @@ static int snd_asihpi_probe(struct pci_d
> asihpi->pci = pci_dev;
> asihpi->hpi = hpi;
>
> - snd_printk(KERN_INFO "adapter ID=%4X index=%d\n",
> + pr_info("adapter ID=%4X index=%d\n",
> asihpi->hpi->adapter->type, adapter_index);
>
> err = hpi_adapter_get_property(adapter_index,
> @@ -2893,7 +2891,7 @@ static int snd_asihpi_probe(struct pci_d
> asihpi->in_min_chans = 1;
> }
>
> - snd_printk(KERN_INFO "Has dma:%d, grouping:%d, mrx:%d\n",
> + pr_info("Has dma:%d, grouping:%d, mrx:%d\n",
> asihpi->can_dma,
> asihpi->support_grouping,
> asihpi->support_mrx
All these need prefix. Maybe asihpi driver implicitly assumes the
verbose printk.
> Index: usb-3.10/sound/pci/bt87x.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/bt87x.c
> +++ usb-3.10/sound/pci/bt87x.c
> @@ -856,7 +856,7 @@ static int snd_bt87x_detect_card(struct
> return -EBUSY;
> }
>
> - snd_printk(KERN_INFO "unknown card %#04x-%#04x:%#04x\n",
> + pr_info("unknown card %#04x-%#04x:%#04x\n",
> pci->device, pci->subsystem_vendor, pci->subsystem_device);
Need a prefix.
> Index: usb-3.10/sound/pci/cs4281.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/cs4281.c
> +++ usb-3.10/sound/pci/cs4281.c
> @@ -1539,7 +1539,7 @@ static int snd_cs4281_chip_init(struct c
> goto __codec2_ok;
> schedule_timeout_uninterruptible(1);
> } while (time_after_eq(end_time, jiffies));
> - snd_printk(KERN_INFO "secondary codec doesn't respond. disable it...\n");
> + pr_info("secondary codec doesn't respond. disable it...\n");
Need a prefix.
> chip->dual_codec = 0;
> __codec2_ok: ;
> }
> Index: usb-3.10/sound/pci/cs46xx/cs46xx_lib.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/cs46xx/cs46xx_lib.c
> +++ usb-3.10/sound/pci/cs46xx/cs46xx_lib.c
> @@ -3806,12 +3806,12 @@ int snd_cs46xx_create(struct snd_card *c
> }
>
> if (external_amp) {
> - snd_printk(KERN_INFO "Crystal EAPD support forced on.\n");
> + pr_info("Crystal EAPD support forced on.\n");
Need a prefix.
> chip->amplifier_ctrl = amp_voyetra;
> }
>
> if (thinkpad) {
> - snd_printk(KERN_INFO "Activating CLKRUN hack for Thinkpad.\n");
> + pr_info("Activating CLKRUN hack for Thinkpad.\n");
Need a prefix.
> chip->active_ctrl = clkrun_hack;
> clkrun_init(chip);
> }
> Index: usb-3.10/sound/pci/echoaudio/echoaudio.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/echoaudio/echoaudio.c
> +++ usb-3.10/sound/pci/echoaudio/echoaudio.c
> @@ -2189,7 +2189,7 @@ static int snd_echo_probe(struct pci_dev
> err = snd_card_register(card);
> if (err < 0)
> goto ctl_error;
> - snd_printk(KERN_INFO "Card registered: %s\n", card->longname);
> + pr_info("Card registered: %s\n", card->longname);
Need a prefix.
> pci_set_drvdata(pci, chip);
> dev++;
> Index: usb-3.10/sound/pci/emu10k1/emu10k1_main.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/emu10k1/emu10k1_main.c
> +++ usb-3.10/sound/pci/emu10k1/emu10k1_main.c
> @@ -217,7 +217,7 @@ static int snd_emu10k1_init(struct snd_e
> }
> if (emu->card_capabilities->ca0108_chip) { /* audigy2 Value */
> /* Hacks for Alice3 to work independent of haP16V driver */
> - snd_printk(KERN_INFO "Audigy2 value: Special config.\n");
> + pr_info("Audigy2 value: Special config.\n");
Need a prefix.
> Index: usb-3.10/sound/pci/emu10k1/emu10k1x.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/emu10k1/emu10k1x.c
> +++ usb-3.10/sound/pci/emu10k1/emu10k1x.c
> @@ -369,7 +369,7 @@ static void snd_emu10k1x_pcm_interrupt(s
> if (epcm->substream == NULL)
> return;
> #if 0
> - snd_printk(KERN_INFO "IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
> + pr_info("IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
> epcm->substream->ops->pointer(epcm->substream),
> snd_pcm_lib_period_bytes(epcm->substream),
> snd_pcm_lib_buffer_bytes(epcm->substream));
Should be pr_debug().
> @@ -487,7 +487,7 @@ static int snd_emu10k1x_pcm_trigger(stru
> int channel = epcm->voice->number;
> int result = 0;
>
> -// snd_printk(KERN_INFO "trigger - emu10k1x = 0x%x, cmd = %i, pointer = %d\n", (int)emu, cmd, (int)substream->ops->pointer(substream));
> +// pr_info("trigger - emu10k1x = 0x%x, cmd = %i, pointer = %d\n", (int)emu, cmd, (int)substream->ops->pointer(substream));
>
> switch (cmd) {
> case SNDRV_PCM_TRIGGER_START:
> @@ -826,7 +826,7 @@ static irqreturn_t snd_emu10k1x_interrup
> // acknowledge the interrupt if necessary
> outl(status, chip->port + IPR);
>
> - // snd_printk(KERN_INFO "interrupt %08x\n", status);
> + // pr_info("interrupt %08x\n", status);
Both look like leftover debug codes...
> return IRQ_HANDLED;
> }
>
> @@ -964,7 +964,7 @@ static int snd_emu10k1x_create(struct sn
> chip->revision = pci->revision;
> pci_read_config_dword(pci, PCI_SUBSYSTEM_VENDOR_ID, &chip->serial);
> pci_read_config_word(pci, PCI_SUBSYSTEM_ID, &chip->model);
> - snd_printk(KERN_INFO "Model %04x Rev %08x Serial %08x\n", chip->model,
> + pr_info("Model %04x Rev %08x Serial %08x\n", chip->model,
> chip->revision, chip->serial);
Need a prefix.
> outl(0, chip->port + INTE);
> Index: usb-3.10/sound/pci/emu10k1/emufx.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/emu10k1/emufx.c
> +++ usb-3.10/sound/pci/emu10k1/emufx.c
> @@ -1542,7 +1542,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
> /* A_PUT_STEREO_OUTPUT(A_EXTOUT_FRONT_L, A_EXTOUT_FRONT_R, playback + SND_EMU10K1_PLAYBACK_CHANNELS); */
> if (emu->card_capabilities->emu_model) {
> /* EMU1010 Outputs from PCM Front, Rear, Center, LFE, Side */
> - snd_printk(KERN_INFO "EMU outputs on\n");
> + pr_info("EMU outputs on\n");
Need a prefix.
> for (z = 0; z < 8; z++) {
> if (emu->card_capabilities->ca0108_chip) {
> A_OP(icode, &ptr, iACC3, A3_EMU32OUT(z), A_GPR(playback + SND_EMU10K1_PLAYBACK_CHANNELS + z), A_C_00000000, A_C_00000000);
> @@ -1566,7 +1566,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
> A_SWITCH(icode, &ptr, tmp + 1, playback + SND_EMU10K1_PLAYBACK_CHANNELS + z, tmp + 1);
> if ((z==1) && (emu->card_capabilities->spdif_bug)) {
> /* Due to a SPDIF output bug on some Audigy cards, this code delays the Right channel by 1 sample */
> - snd_printk(KERN_INFO "Installing spdif_bug patch: %s\n", emu->card_capabilities->name);
> + pr_info("Installing spdif_bug patch: %s\n", emu->card_capabilities->name);
Need a prefix.
> A_OP(icode, &ptr, iACC3, A_EXTOUT(A_EXTOUT_FRONT_L + z), A_GPR(gpr - 3), A_C_00000000, A_C_00000000);
> A_OP(icode, &ptr, iACC3, A_GPR(gpr - 3), A_GPR(tmp + 0), A_GPR(tmp + 1), A_C_00000000);
> } else {
> @@ -1590,7 +1590,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
>
> if (emu->card_capabilities->emu_model) {
> if (emu->card_capabilities->ca0108_chip) {
> - snd_printk(KERN_INFO "EMU2 inputs on\n");
> + pr_info("EMU2 inputs on\n");
Need a prefix.
> for (z = 0; z < 0x10; z++) {
> snd_emu10k1_audigy_dsp_convert_32_to_2x16( icode, &ptr, tmp,
> bit_shifter16,
> @@ -1598,7 +1598,7 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
> A_FXBUS2(z*2) );
> }
> } else {
> - snd_printk(KERN_INFO "EMU inputs on\n");
> + pr_info("EMU inputs on\n");
Ditto.
> /* Capture 16 (originally 8) channels of S32_LE sound */
>
> /*
> Index: usb-3.10/sound/pci/emu10k1/irq.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/emu10k1/irq.c
> +++ usb-3.10/sound/pci/emu10k1/irq.c
> @@ -41,7 +41,7 @@ irqreturn_t snd_emu10k1_interrupt(int ir
> orig_status = status;
> handled = 1;
> if ((status & 0xffffffff) == 0xffffffff) {
> - snd_printk(KERN_INFO "snd-emu10k1: Suspected sound card removal\n");
> + pr_info("snd-emu10k1: Suspected sound card removal\n");
> break;
> }
> if (status & IPR_PCIERROR) {
> @@ -202,7 +202,7 @@ irqreturn_t snd_emu10k1_interrupt(int ir
> outl(orig_status, emu->port + IPR); /* ack all */
> }
> if (timeout == 1000)
> - snd_printk(KERN_INFO "emu10k1 irq routine failure\n");
> + pr_info("emu10k1 irq routine failure\n");
Better with pr_warning().
> return IRQ_RETVAL(handled);
> }
> Index: usb-3.10/sound/pci/es1968.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/es1968.c
> +++ usb-3.10/sound/pci/es1968.c
> @@ -2109,7 +2109,7 @@ static void snd_es1968_ac97_reset(struct
> outw(inw(ioaddr + 0x3c) & 0xfffc, ioaddr + 0x3c);
>
> #if 0 /* the loop here needs to be much better if we want it.. */
> - snd_printk(KERN_INFO "trying software reset\n");
> + pr_info("trying software reset\n");
Need a prefix.
> /* try and do a software reset */
> outb(0x80 | 0x7c, ioaddr + 0x30);
> for (w = 0;; w++) {
> Index: usb-3.10/sound/pci/fm801.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/fm801.c
> +++ usb-3.10/sound/pci/fm801.c
> @@ -1100,7 +1100,7 @@ static int snd_fm801_chip_init(struct fm
>
> if (wait_for_codec(chip, 0, AC97_RESET, msecs_to_jiffies(750)) < 0)
> if (!resume) {
> - snd_printk(KERN_INFO "Primary AC'97 codec not found, "
> + pr_info("Primary AC'97 codec not found, "
> "assume SF64-PCR (tuner-only)\n");
Need a prefix.
> chip->tea575x_tuner = 3 | TUNER_ONLY;
> goto __ac97_ok;
> @@ -1276,7 +1276,7 @@ static int snd_fm801_create(struct snd_c
> for (tea575x_tuner = 1; tea575x_tuner <= 3; tea575x_tuner++) {
> chip->tea575x_tuner = tea575x_tuner;
> if (!snd_tea575x_init(&chip->tea, THIS_MODULE)) {
> - snd_printk(KERN_INFO "detected TEA575x radio type %s\n",
> + pr_info("detected TEA575x radio type %s\n",
> get_tea575x_gpio(chip)->name);
Need a prefix.
> Index: usb-3.10/sound/pci/ice1712/aureon.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/ice1712/aureon.c
> +++ usb-3.10/sound/pci/ice1712/aureon.c
> @@ -1937,9 +1937,9 @@ static int aureon_add_controls(struct sn
> snd_ice1712_save_gpio_status(ice);
> id = aureon_cs8415_get(ice, CS8415_ID);
> if (id != 0x41)
> - snd_printk(KERN_INFO "No CS8415 chip. Skipping CS8415 controls.\n");
> + pr_info("No CS8415 chip. Skipping CS8415 controls.\n");
Need a prefix.
> else if ((id & 0x0F) != 0x01)
> - snd_printk(KERN_INFO "Detected unsupported CS8415 rev. (%c)\n", (char)((id & 0x0F) + 'A' - 1));
> + pr_info("Detected unsupported CS8415 rev. (%c)\n", (char)((id & 0x0F) + 'A' - 1));
Need a prefix.
> else {
> for (i = 0; i < ARRAY_SIZE(cs8415_controls); i++) {
> struct snd_kcontrol *kctl;
> Index: usb-3.10/sound/pci/oxygen/oxygen_lib.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/oxygen/oxygen_lib.c
> +++ usb-3.10/sound/pci/oxygen/oxygen_lib.c
> @@ -313,7 +313,7 @@ static void oxygen_restore_eeprom(struct
> oxygen_clear_bits8(chip, OXYGEN_MISC,
> OXYGEN_MISC_WRITE_PCI_SUBID);
>
> - snd_printk(KERN_INFO "EEPROM ID restored\n");
> + pr_info("EEPROM ID restored\n");
Need a prefix.
> Index: usb-3.10/sound/pci/rme9652/hdspm.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/rme9652/hdspm.c
> +++ usb-3.10/sound/pci/rme9652/hdspm.c
> @@ -5143,7 +5143,7 @@ static irqreturn_t snd_hdspm_interrupt(i
> * 0 64 ~3998231 ~8191558
> **/
> /*
> - snd_printk(KERN_INFO "snd_hdspm_interrupt %llu @ %llx\n",
> + pr_info("snd_hdspm_interrupt %llu @ %llx\n",
Should be pr_debug().
> now-hdspm->last_interrupt, status & 0xFFC0);
> hdspm->last_interrupt = now;
> */
> @@ -5280,7 +5280,7 @@ static int snd_hdspm_hw_params(struct sn
> spin_lock_irq(&hdspm->lock);
> err = hdspm_set_rate(hdspm, params_rate(params), 0);
> if (err < 0) {
> - snd_printk(KERN_INFO "err on hdspm_set_rate: %d\n", err);
> + pr_info("err on hdspm_set_rate: %d\n", err);
Better with pr_debug(). This is a software issue and each ioctl call
can trigger such a condition. Repeated such messages may be
annoying. Also this needs a prefix, too.
> spin_unlock_irq(&hdspm->lock);
> _snd_pcm_hw_param_setempty(params,
> SNDRV_PCM_HW_PARAM_RATE);
> @@ -5291,7 +5291,7 @@ static int snd_hdspm_hw_params(struct sn
> err = hdspm_set_interrupt_interval(hdspm,
> params_period_size(params));
> if (err < 0) {
> - snd_printk(KERN_INFO "err on hdspm_set_interrupt_interval: %d\n", err);
> + pr_info("err on hdspm_set_interrupt_interval: %d\n", err);
Ditto.
> _snd_pcm_hw_param_setempty(params,
> SNDRV_PCM_HW_PARAM_PERIOD_SIZE);
> return err;
> @@ -5307,7 +5307,7 @@ static int snd_hdspm_hw_params(struct sn
> err =
> snd_pcm_lib_malloc_pages(substream, HDSPM_DMA_AREA_BYTES);
> if (err < 0) {
> - snd_printk(KERN_INFO "err on snd_pcm_lib_malloc_pages: %d\n", err);
> + pr_info("err on snd_pcm_lib_malloc_pages: %d\n", err);
Ditto.
> return err;
> }
>
> @@ -5354,12 +5354,12 @@ static int snd_hdspm_hw_params(struct sn
> /* Switch to native float format if requested */
> if (SNDRV_PCM_FORMAT_FLOAT_LE == params_format(params)) {
> if (!(hdspm->control_register & HDSPe_FLOAT_FORMAT))
> - snd_printk(KERN_INFO "hdspm: Switching to native 32bit LE float format.\n");
> + pr_info("hdspm: Switching to native 32bit LE float format.\n");
>
> hdspm->control_register |= HDSPe_FLOAT_FORMAT;
> } else if (SNDRV_PCM_FORMAT_S32_LE == params_format(params)) {
> if (hdspm->control_register & HDSPe_FLOAT_FORMAT)
> - snd_printk(KERN_INFO "hdspm: Switching to native 32bit LE integer format.\n");
> + pr_info("hdspm: Switching to native 32bit LE integer format.\n");
>
> hdspm->control_register &= ~HDSPe_FLOAT_FORMAT;
> }
> @@ -5402,12 +5402,12 @@ static int snd_hdspm_channel_info(struct
>
> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> if (snd_BUG_ON(info->channel >= hdspm->max_channels_out)) {
> - snd_printk(KERN_INFO "snd_hdspm_channel_info: output channel out of range (%d)\n", info->channel);
> + pr_info("snd_hdspm_channel_info: output channel out of range (%d)\n", info->channel);
Again, a thing for debugging (and needs a prefix).
> return -EINVAL;
> }
>
> if (hdspm->channel_map_out[info->channel] < 0) {
> - snd_printk(KERN_INFO "snd_hdspm_channel_info: output channel %d mapped out\n", info->channel);
> + pr_info("snd_hdspm_channel_info: output channel %d mapped out\n", info->channel);
Ditto.
> return -EINVAL;
> }
>
> @@ -5415,12 +5415,12 @@ static int snd_hdspm_channel_info(struct
> HDSPM_CHANNEL_BUFFER_BYTES;
> } else {
> if (snd_BUG_ON(info->channel >= hdspm->max_channels_in)) {
> - snd_printk(KERN_INFO "snd_hdspm_channel_info: input channel out of range (%d)\n", info->channel);
> + pr_info("snd_hdspm_channel_info: input channel out of range (%d)\n", info->channel);
Ditto.
> return -EINVAL;
> }
>
> if (hdspm->channel_map_in[info->channel] < 0) {
> - snd_printk(KERN_INFO "snd_hdspm_channel_info: input channel %d mapped out\n", info->channel);
> + pr_info("snd_hdspm_channel_info: input channel %d mapped out\n", info->channel);
Ditto.
> return -EINVAL;
> }
>
> @@ -6460,7 +6460,7 @@ static int snd_hdspm_create(struct snd_c
>
> case AIO:
> if (0 == (hdspm_read(hdspm, HDSPM_statusRegister2) & HDSPM_s2_AEBI_D)) {
> - snd_printk(KERN_INFO "HDSPM: AEB input board found, but not supported\n");
> + pr_info("HDSPM: AEB input board found, but not supported\n");
> }
>
> hdspm->ss_in_channels = AIO_IN_SS_CHANNELS;
> @@ -6531,7 +6531,7 @@ static int snd_hdspm_create(struct snd_c
> if (NULL != hdspm->tco) {
> hdspm_tco_write(hdspm);
> }
> - snd_printk(KERN_INFO "HDSPM: AIO/RayDAT TCO module found\n");
> + pr_info("HDSPM: AIO/RayDAT TCO module found\n");
> } else {
> hdspm->tco = NULL;
> }
> @@ -6545,7 +6545,7 @@ static int snd_hdspm_create(struct snd_c
> if (NULL != hdspm->tco) {
> hdspm_tco_write(hdspm);
> }
> - snd_printk(KERN_INFO "HDSPM: MADI TCO module found\n");
> + pr_info("HDSPM: MADI TCO module found\n");
> } else {
> hdspm->tco = NULL;
> }
> Index: usb-3.10/sound/pci/sonicvibes.c
> ===================================================================
> --- usb-3.10.orig/sound/pci/sonicvibes.c
> +++ usb-3.10/sound/pci/sonicvibes.c
> @@ -1310,12 +1310,12 @@ static int snd_sonicvibes_create(struct
> if (!dmaa) {
> dmaa = dmaio;
> dmaio += 0x10;
> - snd_printk(KERN_INFO "BIOS did not allocate DDMA channel A i/o, allocated at 0x%x\n", dmaa);
> + pr_info("BIOS did not allocate DDMA channel A i/o, allocated at 0x%x\n", dmaa);
Need a prefix.
> Index: usb-3.10/sound/usb/card.c
> ===================================================================
> --- usb-3.10.orig/sound/usb/card.c
> +++ usb-3.10/sound/usb/card.c
> @@ -227,7 +227,7 @@ static int snd_usb_create_streams(struct
> struct uac1_ac_header_descriptor *h1 = control_header;
>
> if (!h1->bInCollection) {
> - snd_printk(KERN_INFO "skipping empty audio interface (v1)\n");
> + pr_info("skipping empty audio interface (v1)\n");
Need a prefix.
> Index: usb-3.10/sound/usb/format.c
> ===================================================================
> --- usb-3.10.orig/sound/usb/format.c
> +++ usb-3.10/sound/usb/format.c
> @@ -84,7 +84,7 @@ static u64 parse_audio_format_i_type(str
> sample_width == 24 && sample_bytes == 2)
> sample_bytes = 3;
> else if (sample_width > sample_bytes * 8) {
> - snd_printk(KERN_INFO "%d:%u:%d : sample bitwidth %d in over sample bytes %d\n",
> + pr_info("%d:%u:%d : sample bitwidth %d in over sample bytes %d\n",
> chip->dev->devnum, fp->iface, fp->altsetting,
> sample_width, sample_bytes);
Need a prefix.
> }
> @@ -109,7 +109,7 @@ static u64 parse_audio_format_i_type(str
> pcm_formats |= SNDRV_PCM_FMTBIT_S32_LE;
> break;
> default:
> - snd_printk(KERN_INFO "%d:%u:%d : unsupported sample bitwidth %d in %d bytes\n",
> + pr_info("%d:%u:%d : unsupported sample bitwidth %d in %d bytes\n",
> chip->dev->devnum, fp->iface, fp->altsetting,
> sample_width, sample_bytes);
Ditto.
> break;
> @@ -133,7 +133,7 @@ static u64 parse_audio_format_i_type(str
> pcm_formats |= SNDRV_PCM_FMTBIT_MU_LAW;
> }
> if (format & ~0x3f) {
> - snd_printk(KERN_INFO "%d:%u:%d : unsupported format bits %#x\n",
> + pr_info("%d:%u:%d : unsupported format bits %#x\n",
> chip->dev->devnum, fp->iface, fp->altsetting, format);
Ditto.
> }
>
> Index: usb-3.10/sound/usb/mixer.c
> ===================================================================
> --- usb-3.10.orig/sound/usb/mixer.c
> +++ usb-3.10/sound/usb/mixer.c
> @@ -839,8 +839,7 @@ static void volume_control_quirks(struct
> case USB_ID(0x0763, 0x2081): /* M-Audio Fast Track Ultra 8R */
> case USB_ID(0x0763, 0x2080): /* M-Audio Fast Track Ultra */
> if (strcmp(kctl->id.name, "Effect Duration") == 0) {
> - snd_printk(KERN_INFO
> - "usb-audio: set quirk for FTU Effect Duration\n");
> + pr_info("usb-audio: set quirk for FTU Effect Duration\n");
> cval->min = 0x0000;
> cval->max = 0x7f00;
> cval->res = 0x0100;
> @@ -848,8 +847,7 @@ static void volume_control_quirks(struct
> }
> if (strcmp(kctl->id.name, "Effect Volume") == 0 ||
> strcmp(kctl->id.name, "Effect Feedback Volume") == 0) {
> - snd_printk(KERN_INFO
> - "usb-audio: set quirks for FTU Effect Feedback/Volume\n");
> + pr_info("usb-audio: set quirks for FTU Effect Feedback/Volume\n");
> cval->min = 0x00;
> cval->max = 0x7f;
> break;
> @@ -867,16 +865,14 @@ static void volume_control_quirks(struct
> */
> if (!strcmp(kctl->id.name, "PCM Playback Volume") &&
> cval->min == -15616) {
> - snd_printk(KERN_INFO
> - "set volume quirk for UDA1321/N101 chip\n");
> + pr_info("set volume quirk for UDA1321/N101 chip\n");
Need a prefix.
> cval->max = -256;
> }
> break;
>
> case USB_ID(0x046d, 0x09a4):
> if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
> - snd_printk(KERN_INFO
> - "set volume quirk for QuickCam E3500\n");
> + pr_info("set volume quirk for QuickCam E3500\n");
Ditto.
> cval->min = 6080;
> cval->max = 8768;
> cval->res = 192;
> @@ -892,8 +888,7 @@ static void volume_control_quirks(struct
> * Proboly there is some logitech magic behind this number --fishor
> */
> if (!strcmp(kctl->id.name, "Mic Capture Volume")) {
> - snd_printk(KERN_INFO
> - "set resolution quirk: cval->res = 384\n");
> + pr_info("set resolution quirk: cval->res = 384\n");
Ditto.
> Index: usb-3.10/sound/isa/wavefront/wavefront.c
> ===================================================================
> --- usb-3.10.orig/sound/isa/wavefront/wavefront.c
> +++ usb-3.10/sound/isa/wavefront/wavefront.c
> @@ -195,7 +195,7 @@ snd_wavefront_pnp (int dev, snd_wavefron
> cs4232_mpu_irq[dev] = pnp_irq(pdev, 0);
> }
>
> - snd_printk (KERN_INFO "CS4232 MPU: port=0x%lx, irq=%i\n",
> + pr_info("CS4232 MPU: port=0x%lx, irq=%i\n",
> cs4232_mpu_port[dev],
> cs4232_mpu_irq[dev]);
Need a prefix. (CS4232 codec is used by many drivers, thus you can't
identify uniquely only by that.)
thanks,
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 20:54 ` Alan Stern
2013-06-04 21:19 ` Joe Perches
@ 2013-06-05 6:04 ` Takashi Iwai
2013-06-05 6:15 ` Joe Perches
2013-06-06 21:28 ` [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
1 sibling, 2 replies; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 6:04 UTC (permalink / raw)
To: Alan Stern; +Cc: Joe Perches, alsa-devel
At Tue, 4 Jun 2013 16:54:12 -0400 (EDT),
Alan Stern wrote:
>
> 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?
Unfortunately, it's not so straightforward. Many messages are better
with KERN_INFO indeed. In such places, snd_printd() is used rather as
snd_chattier_printk_with_prefix().
This comes from the subsystem development history. In the era of 2.4
kernels, many systems did load/unload the module frequently on demand.
Thus we didn't want to put a message from driver at each module probe
or removal. Otherwise dmesg will be flood of such messages at each
time you play a WAV song or ring a beep via PCM.
Instead, we tended to put such informational messages as snd_printd(),
while keeping the driver itself reticent as much as possible for
"productive" systems. This style was kept for a while even after
merged to 2.5 kernels until recently.
Also, some places use KERN_WARNING or KERN_ERR with snd_printd(),
mostly because they are in the context with CONFIG_SND_DEBUG.
They can be well pr_warning() or pr_err().
Last but not least, as mentioned in the reply to patch 1/2, many
messages miss proper prefixes.
These are the main reasons I stated that systematic replacements would
be difficult. We need to take a look through the whole snd_print*()
and replace appropriately case by case...
thanks,
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info("
2013-06-05 5:52 ` Takashi Iwai
@ 2013-06-05 6:07 ` Joe Perches
2013-06-05 6:16 ` Takashi Iwai
2013-06-06 20:54 ` Alan Stern
1 sibling, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-05 6:07 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Alan Stern
On Wed, 2013-06-05 at 07:52 +0200, Takashi Iwai wrote:
> At Tue, 4 Jun 2013 13:20:40 -0400 (EDT),
> > --- usb-3.10.orig/sound/isa/opti9xx/miro.c
[]
> > - snd_printk(KERN_INFO "unknown miro aci id\n");
> > + pr_info("unknown miro aci id\n");
[]
> need proper prefix, and should be rather pr_warning().
pr_warn should be preferred over pr_warning
and most everything should have a
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
added. Another option would be to add
ccflags-y += -D "pr_fmt(fmt)=KBUILD_MODNAME \": \" fmt"
to the top level sound makefile.
Some eon son I'll actually submit this
https://lkml.org/lkml/2012/3/27/247
and most all of the #define pr_fmt(...
will be unncessary.
> > --- usb-3.10.orig/sound/pci/emu10k1/emu10k1x.c
[]
> > #if 0
> > - snd_printk(KERN_INFO "IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
> > + pr_info("IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
> > epcm->substream->ops->pointer(epcm->substream),
> > snd_pcm_lib_period_bytes(epcm->substream),
> > snd_pcm_lib_buffer_bytes(epcm->substream));
>
> Should be pr_debug().
Or the block deleted
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 6:04 ` Takashi Iwai
@ 2013-06-05 6:15 ` Joe Perches
2013-06-05 6:32 ` Takashi Iwai
2013-06-06 21:28 ` [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
1 sibling, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-05 6:15 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Alan Stern
On Wed, 2013-06-05 at 08:04 +0200, Takashi Iwai wrote:
> At Tue, 4 Jun 2013 16:54:12 -0400 (EDT), Alan Stern wrote:
> > On Tue, 4 Jun 2013, Joe Perches wrote:
[]
> > 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?
>
> Unfortunately, it's not so straightforward. Many messages are better
> with KERN_INFO indeed. In such places, snd_printd() is used rather as
> snd_chattier_printk_with_prefix().
In those cases, it's likely true that most of those
should not be snd_printd but promoted to pr_<level>
> Instead, we tended to put such informational messages as snd_printd(),
> while keeping the driver itself reticent as much as possible for
> "productive" systems. This style was kept for a while even after
> merged to 2.5 kernels until recently.
>
> Also, some places use KERN_WARNING or KERN_ERR with snd_printd(),
> mostly because they are in the context with CONFIG_SND_DEBUG.
> They can be well pr_warning() or pr_err().
I thought the idea was to rationalize all that with
the new printing styles. So on the whole, it seems
we are agreeing strongly.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info("
2013-06-05 6:07 ` Joe Perches
@ 2013-06-05 6:16 ` Takashi Iwai
0 siblings, 0 replies; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 6:16 UTC (permalink / raw)
To: Joe Perches; +Cc: alsa-devel, Alan Stern
At Tue, 04 Jun 2013 23:07:51 -0700,
Joe Perches wrote:
>
> On Wed, 2013-06-05 at 07:52 +0200, Takashi Iwai wrote:
> > At Tue, 4 Jun 2013 13:20:40 -0400 (EDT),
>
> > > --- usb-3.10.orig/sound/isa/opti9xx/miro.c
> []
> > > - snd_printk(KERN_INFO "unknown miro aci id\n");
> > > + pr_info("unknown miro aci id\n");
> []
> > need proper prefix, and should be rather pr_warning().
>
> pr_warn should be preferred over pr_warning
Oh, then shouldn't we define them in other way round?
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -214,9 +214,9 @@ extern void dump_stack(void) __cold;
printk(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
#define pr_err(fmt, ...) \
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warning(fmt, ...) \
+#define pr_warn(fmt, ...) \
printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warn pr_warning
+#define pr_warning pr_warn
#define pr_notice(fmt, ...) \
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
#define pr_info(fmt, ...) \
> and most everything should have a
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> added. Another option would be to add
> ccflags-y += -D "pr_fmt(fmt)=KBUILD_MODNAME \": \" fmt"
> to the top level sound makefile.
Right, these would work well.
With this addition, we can go rather to remove superfluous prefix from
pr_*().
> Some eon son I'll actually submit this
> https://lkml.org/lkml/2012/3/27/247
> and most all of the #define pr_fmt(...
> will be unncessary.
>
> > > --- usb-3.10.orig/sound/pci/emu10k1/emu10k1x.c
> []
> > > #if 0
> > > - snd_printk(KERN_INFO "IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
> > > + pr_info("IRQ: position = 0x%x, period = 0x%x, size = 0x%x\n",
> > > epcm->substream->ops->pointer(epcm->substream),
> > > snd_pcm_lib_period_bytes(epcm->substream),
> > > snd_pcm_lib_buffer_bytes(epcm->substream));
> >
> > Should be pr_debug().
>
> Or the block deleted
Yes, another obvious option :)
thanks,
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 6:15 ` Joe Perches
@ 2013-06-05 6:32 ` Takashi Iwai
2013-06-05 6:52 ` Joe Perches
0 siblings, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 6:32 UTC (permalink / raw)
To: Joe Perches; +Cc: alsa-devel, Alan Stern
At Tue, 04 Jun 2013 23:15:21 -0700,
Joe Perches wrote:
>
> On Wed, 2013-06-05 at 08:04 +0200, Takashi Iwai wrote:
> > At Tue, 4 Jun 2013 16:54:12 -0400 (EDT), Alan Stern wrote:
> > > On Tue, 4 Jun 2013, Joe Perches wrote:
> []
> > > 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?
> >
> > Unfortunately, it's not so straightforward. Many messages are better
> > with KERN_INFO indeed. In such places, snd_printd() is used rather as
> > snd_chattier_printk_with_prefix().
>
> In those cases, it's likely true that most of those
> should not be snd_printd but promoted to pr_<level>
Yes. These are snd_printd() just to be conditionally built in.
But in most cases it's rather useful to print them (as most distros
set CONFIG_SND_DEBUG=y). Hence practically they can be pr_info()
nowadays.
> > Instead, we tended to put such informational messages as snd_printd(),
> > while keeping the driver itself reticent as much as possible for
> > "productive" systems. This style was kept for a while even after
> > merged to 2.5 kernels until recently.
> >
> > Also, some places use KERN_WARNING or KERN_ERR with snd_printd(),
> > mostly because they are in the context with CONFIG_SND_DEBUG.
> > They can be well pr_warning() or pr_err().
>
> I thought the idea was to rationalize all that with
> the new printing styles. So on the whole, it seems
> we are agreeing strongly.
Yep, we can convert almost all snd_printk() with pr_*(), and usual
snd_printd() with pr_debug(). There must be some exceptions, and they
need care manually.
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 6:32 ` Takashi Iwai
@ 2013-06-05 6:52 ` Joe Perches
2013-06-05 6:54 ` Takashi Iwai
0 siblings, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-05 6:52 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Alan Stern
On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> Yes. These are snd_printd() just to be conditionally built in.
> But in most cases it's rather useful to print them (as most distros
> set CONFIG_SND_DEBUG=y).
Ubuntu doesn't, I believe Fedora doesn't.
What are the common distros that do?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 6:52 ` Joe Perches
@ 2013-06-05 6:54 ` Takashi Iwai
2013-06-05 7:07 ` Joe Perches
0 siblings, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 6:54 UTC (permalink / raw)
To: Joe Perches; +Cc: alsa-devel, Alan Stern
At Tue, 04 Jun 2013 23:52:01 -0700,
Joe Perches wrote:
>
> On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> > Yes. These are snd_printd() just to be conditionally built in.
> > But in most cases it's rather useful to print them (as most distros
> > set CONFIG_SND_DEBUG=y).
>
> Ubuntu doesn't, I believe Fedora doesn't.
Then they should have done so :)
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 6:54 ` Takashi Iwai
@ 2013-06-05 7:07 ` Joe Perches
2013-06-05 7:22 ` Takashi Iwai
0 siblings, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-05 7:07 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel, Alan Stern
On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
> At Tue, 04 Jun 2013 23:52:01 -0700,
> Joe Perches wrote:
> >
> > On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> > > Yes. These are snd_printd() just to be conditionally built in.
> > > But in most cases it's rather useful to print them (as most distros
> > > set CONFIG_SND_DEBUG=y).
> >
> > Ubuntu doesn't, I believe Fedora doesn't.
>
> Then they should have done so :)
But they don't, so what distros do?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 7:07 ` Joe Perches
@ 2013-06-05 7:22 ` Takashi Iwai
2013-06-05 7:34 ` Joe Perches
0 siblings, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 7:22 UTC (permalink / raw)
To: Joe Perches; +Cc: alsa-devel, Alan Stern
At Wed, 05 Jun 2013 00:07:57 -0700,
Joe Perches wrote:
>
> On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
> > At Tue, 04 Jun 2013 23:52:01 -0700,
> > Joe Perches wrote:
> > >
> > > On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> > > > Yes. These are snd_printd() just to be conditionally built in.
> > > > But in most cases it's rather useful to print them (as most distros
> > > > set CONFIG_SND_DEBUG=y).
> > >
> > > Ubuntu doesn't, I believe Fedora doesn't.
> >
> > Then they should have done so :)
>
> But they don't, so what distros do?
RedHat (including Fedora) and SUSE do at least.
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 7:22 ` Takashi Iwai
@ 2013-06-05 7:34 ` Joe Perches
2013-06-05 7:47 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK) David Henningsson
0 siblings, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-05 7:34 UTC (permalink / raw)
To: Takashi Iwai
Cc: Andy Whitcroft, alsa-devel, Alan Stern, leann.ogasawara,
David Henningsson
On Wed, 2013-06-05 at 09:22 +0200, Takashi Iwai wrote:
> At Wed, 05 Jun 2013 00:07:57 -0700, Joe Perches wrote:
> > On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
> > > At Tue, 04 Jun 2013 23:52:01 -0700, Joe Perches wrote:
> > > > On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> > > > > Yes. These are snd_printd() just to be conditionally built in.
> > > > > But in most cases it's rather useful to print them (as most distros
> > > > > set CONFIG_SND_DEBUG=y).
> > > > Ubuntu doesn't, I believe Fedora doesn't.
> > > Then they should have done so :)
> > But they don't, so what distros do?
>
> RedHat (including Fedora) and SUSE do at least.
Mandriva does too. (still looking around for others)
We can ask Ubuntu to enable CONFIG_SND_DEBUG.
(cc'd Andy Whitcroft, Leann Ogasawara and David Henningsson)
Maybe there are others Canonical folk that
should be cc'd?
https://wiki.ubuntu.com/KernelTeam#Ubuntu_Platform_Kernel_Team
^ permalink raw reply [flat|nested] 44+ messages in thread
* CONFIG_SND_DEBUG (was: Re: [alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK)
2013-06-05 7:34 ` Joe Perches
@ 2013-06-05 7:47 ` David Henningsson
2013-06-05 8:46 ` CONFIG_SND_DEBUG (was: " Takashi Iwai
2013-06-05 10:53 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] " Andy Whitcroft
0 siblings, 2 replies; 44+ messages in thread
From: David Henningsson @ 2013-06-05 7:47 UTC (permalink / raw)
To: Joe Perches
Cc: alsa-devel, Takashi Iwai, Kernel team list, Alan Stern, Andy Whitcroft
On 06/05/2013 09:34 AM, Joe Perches wrote:
> On Wed, 2013-06-05 at 09:22 +0200, Takashi Iwai wrote:
>> At Wed, 05 Jun 2013 00:07:57 -0700, Joe Perches wrote:
>>> On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
>>>> At Tue, 04 Jun 2013 23:52:01 -0700, Joe Perches wrote:
>>>>> On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
>>>>>> Yes. These are snd_printd() just to be conditionally built in.
>>>>>> But in most cases it's rather useful to print them (as most distros
>>>>>> set CONFIG_SND_DEBUG=y).
>>>>> Ubuntu doesn't, I believe Fedora doesn't.
>>>> Then they should have done so :)
>>> But they don't, so what distros do?
>>
>> RedHat (including Fedora) and SUSE do at least.
>
> Mandriva does too. (still looking around for others)
>
> We can ask Ubuntu to enable CONFIG_SND_DEBUG.
> (cc'd Andy Whitcroft, Leann Ogasawara and David Henningsson)
>
> Maybe there are others Canonical folk that
> should be cc'd?
Adding kernel team mailing list to CC.
Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from upstream,
and we explicitly disable it. Is there any reason why we do that?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: CONFIG_SND_DEBUG (was: Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK)
2013-06-05 7:47 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK) David Henningsson
@ 2013-06-05 8:46 ` Takashi Iwai
2013-06-05 10:53 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] " Andy Whitcroft
1 sibling, 0 replies; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 8:46 UTC (permalink / raw)
To: David Henningsson
Cc: alsa-devel, Andy Whitcroft, Kernel team list, Alan Stern,
Joe Perches, leann.ogasawara
At Wed, 05 Jun 2013 09:47:30 +0200,
David Henningsson wrote:
>
> On 06/05/2013 09:34 AM, Joe Perches wrote:
> > On Wed, 2013-06-05 at 09:22 +0200, Takashi Iwai wrote:
> >> At Wed, 05 Jun 2013 00:07:57 -0700, Joe Perches wrote:
> >>> On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
> >>>> At Tue, 04 Jun 2013 23:52:01 -0700, Joe Perches wrote:
> >>>>> On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> >>>>>> Yes. These are snd_printd() just to be conditionally built in.
> >>>>>> But in most cases it's rather useful to print them (as most distros
> >>>>>> set CONFIG_SND_DEBUG=y).
> >>>>> Ubuntu doesn't, I believe Fedora doesn't.
> >>>> Then they should have done so :)
> >>> But they don't, so what distros do?
> >>
> >> RedHat (including Fedora) and SUSE do at least.
> >
> > Mandriva does too. (still looking around for others)
> >
> > We can ask Ubuntu to enable CONFIG_SND_DEBUG.
> > (cc'd Andy Whitcroft, Leann Ogasawara and David Henningsson)
> >
> > Maybe there are others Canonical folk that
> > should be cc'd?
>
> Adding kernel team mailing list to CC.
>
> Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from upstream,
> and we explicitly disable it. Is there any reason why we do that?
Inheriting from Debian?
Note that CONFIG_SND_DEBUG=n is correct if you are really sure that
the driver works fine on your device and you want to keep the kernel
as minimal as possible. For generic kernels, better to set this
option because it also enables more safety checks.
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: CONFIG_SND_DEBUG (was: Re: [alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK)
2013-06-05 7:47 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK) David Henningsson
2013-06-05 8:46 ` CONFIG_SND_DEBUG (was: " Takashi Iwai
@ 2013-06-05 10:53 ` Andy Whitcroft
2013-06-05 11:38 ` CONFIG_SND_DEBUG David Henningsson
1 sibling, 1 reply; 44+ messages in thread
From: Andy Whitcroft @ 2013-06-05 10:53 UTC (permalink / raw)
To: David Henningsson
Cc: alsa-devel, Takashi Iwai, Kernel team list, Alan Stern, Joe Perches
On Wed, Jun 05, 2013 at 09:47:30AM +0200, David Henningsson wrote:
> >>>On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
> >>>>At Tue, 04 Jun 2013 23:52:01 -0700, Joe Perches wrote:
> >>>>>On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> >>>>>>Yes. These are snd_printd() just to be conditionally built in.
> >>>>>>But in most cases it's rather useful to print them (as most distros
> >>>>>>set CONFIG_SND_DEBUG=y).
> >>>>>Ubuntu doesn't, I believe Fedora doesn't.
> >>>>Then they should have done so :)
> >>>But they don't, so what distros do?
> >>
> >>RedHat (including Fedora) and SUSE do at least.
> >
> >Mandriva does too. (still looking around for others)
> >
> >We can ask Ubuntu to enable CONFIG_SND_DEBUG.
> >(cc'd Andy Whitcroft, Leann Ogasawara and David Henningsson)
> >
> >Maybe there are others Canonical folk that
> >should be cc'd?
>
> Adding kernel team mailing list to CC.
>
> Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from
> upstream, and we explicitly disable it. Is there any reason why we
> do that?
config SND_DEBUG
bool "Debug"
help
Say Y here to enable ALSA debug code.
It is off by default in upstream, and the really helpful description
would cirtainly tend to lead to it being disabled. But if it is helpful
to your debugging efforts David then I suspect we can enable it in Saucy
and see what happens.
-apw
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: CONFIG_SND_DEBUG
2013-06-05 10:53 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] " Andy Whitcroft
@ 2013-06-05 11:38 ` David Henningsson
2013-06-05 11:43 ` CONFIG_SND_DEBUG Takashi Iwai
0 siblings, 1 reply; 44+ messages in thread
From: David Henningsson @ 2013-06-05 11:38 UTC (permalink / raw)
To: Andy Whitcroft
Cc: alsa-devel, Takashi Iwai, Kernel team list, Alan Stern, Joe Perches
On 06/05/2013 12:53 PM, Andy Whitcroft wrote:
> On Wed, Jun 05, 2013 at 09:47:30AM +0200, David Henningsson wrote:
>>>>> On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
>>>>>> At Tue, 04 Jun 2013 23:52:01 -0700, Joe Perches wrote:
>>>>>>> On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
>>>>>>>> Yes. These are snd_printd() just to be conditionally built in.
>>>>>>>> But in most cases it's rather useful to print them (as most distros
>>>>>>>> set CONFIG_SND_DEBUG=y).
>>>>>>> Ubuntu doesn't, I believe Fedora doesn't.
>>>>>> Then they should have done so :)
>>>>> But they don't, so what distros do?
>>>>
>>>> RedHat (including Fedora) and SUSE do at least.
>>>
>>> Mandriva does too. (still looking around for others)
>>>
>>> We can ask Ubuntu to enable CONFIG_SND_DEBUG.
>>> (cc'd Andy Whitcroft, Leann Ogasawara and David Henningsson)
>>>
>>> Maybe there are others Canonical folk that
>>> should be cc'd?
>>
>> Adding kernel team mailing list to CC.
>>
>> Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from
>> upstream, and we explicitly disable it. Is there any reason why we
>> do that?
>
> config SND_DEBUG
> bool "Debug"
> help
> Say Y here to enable ALSA debug code.
>
> It is off by default in upstream, and the really helpful description
> would cirtainly tend to lead to it being disabled. But if it is helpful
> to your debugging efforts David then I suspect we can enable it in Saucy
> and see what happens.
Okay, so then the ball is back in Takashi's area - if we're recommended
to turn CONFIG_SND_DEBUG on, why is it off by default in the upstream
Linux kernel?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: CONFIG_SND_DEBUG
2013-06-05 11:38 ` CONFIG_SND_DEBUG David Henningsson
@ 2013-06-05 11:43 ` Takashi Iwai
2013-06-05 14:11 ` CONFIG_SND_DEBUG Alan Stern
0 siblings, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 11:43 UTC (permalink / raw)
To: David Henningsson
Cc: alsa-devel, Joe Perches, Kernel team list, Alan Stern,
Andy Whitcroft, leann.ogasawara
At Wed, 05 Jun 2013 13:38:55 +0200,
David Henningsson wrote:
>
> On 06/05/2013 12:53 PM, Andy Whitcroft wrote:
> > On Wed, Jun 05, 2013 at 09:47:30AM +0200, David Henningsson wrote:
> >>>>> On Wed, 2013-06-05 at 08:54 +0200, Takashi Iwai wrote:
> >>>>>> At Tue, 04 Jun 2013 23:52:01 -0700, Joe Perches wrote:
> >>>>>>> On Wed, 2013-06-05 at 08:32 +0200, Takashi Iwai wrote:
> >>>>>>>> Yes. These are snd_printd() just to be conditionally built in.
> >>>>>>>> But in most cases it's rather useful to print them (as most distros
> >>>>>>>> set CONFIG_SND_DEBUG=y).
> >>>>>>> Ubuntu doesn't, I believe Fedora doesn't.
> >>>>>> Then they should have done so :)
> >>>>> But they don't, so what distros do?
> >>>>
> >>>> RedHat (including Fedora) and SUSE do at least.
> >>>
> >>> Mandriva does too. (still looking around for others)
> >>>
> >>> We can ask Ubuntu to enable CONFIG_SND_DEBUG.
> >>> (cc'd Andy Whitcroft, Leann Ogasawara and David Henningsson)
> >>>
> >>> Maybe there are others Canonical folk that
> >>> should be cc'd?
> >>
> >> Adding kernel team mailing list to CC.
> >>
> >> Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from
> >> upstream, and we explicitly disable it. Is there any reason why we
> >> do that?
> >
> > config SND_DEBUG
> > bool "Debug"
> > help
> > Say Y here to enable ALSA debug code.
> >
> > It is off by default in upstream, and the really helpful description
> > would cirtainly tend to lead to it being disabled. But if it is helpful
> > to your debugging efforts David then I suspect we can enable it in Saucy
> > and see what happens.
>
> Okay, so then the ball is back in Takashi's area - if we're recommended
> to turn CONFIG_SND_DEBUG on, why is it off by default in the upstream
> Linux kernel?
It's not off as default. Simply there is no default, just like most
of other options.
As already mentioned, if the device is known to work well with the
kernel, there is no reason to set it on. Then it'll saves the memory
and code space. That is, for custom kernels, it's good to be off.
But for generic kernels like distro kernel, it'd be better to take a
safer side with more safety checks that is built in by that option.
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: CONFIG_SND_DEBUG
2013-06-05 11:43 ` CONFIG_SND_DEBUG Takashi Iwai
@ 2013-06-05 14:11 ` Alan Stern
2013-06-05 14:28 ` CONFIG_SND_DEBUG Takashi Iwai
0 siblings, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-05 14:11 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, Joe Perches, Kernel team list, Andy Whitcroft,
leann.ogasawara, David Henningsson
On Wed, 5 Jun 2013, Takashi Iwai wrote:
> > >> Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from
> > >> upstream, and we explicitly disable it. Is there any reason why we
> > >> do that?
> > >
> > > config SND_DEBUG
> > > bool "Debug"
> > > help
> > > Say Y here to enable ALSA debug code.
> > >
> > > It is off by default in upstream, and the really helpful description
> > > would cirtainly tend to lead to it being disabled. But if it is helpful
> > > to your debugging efforts David then I suspect we can enable it in Saucy
> > > and see what happens.
> >
> > Okay, so then the ball is back in Takashi's area - if we're recommended
> > to turn CONFIG_SND_DEBUG on, why is it off by default in the upstream
> > Linux kernel?
>
> It's not off as default. Simply there is no default, just like most
> of other options.
>
> As already mentioned, if the device is known to work well with the
> kernel, there is no reason to set it on. Then it'll saves the memory
> and code space. That is, for custom kernels, it's good to be off.
> But for generic kernels like distro kernel, it'd be better to take a
> safer side with more safety checks that is built in by that option.
Given this description, the symbol's name is very misleading. Instead
of being called CONFIG_SND_DEBUG, it should be called
CONFIG_SND_SAFETY_CHECKS or something like that.
IMO, distributions are completely justified in turning off any symbol
which is named (and described in the help text!) as being used for
debugging only.
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: CONFIG_SND_DEBUG
2013-06-05 14:11 ` CONFIG_SND_DEBUG Alan Stern
@ 2013-06-05 14:28 ` Takashi Iwai
2013-06-05 14:47 ` CONFIG_SND_DEBUG Alan Stern
0 siblings, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-05 14:28 UTC (permalink / raw)
To: Alan Stern
Cc: alsa-devel, Joe Perches, Kernel team list, Andy Whitcroft,
leann.ogasawara, David Henningsson
At Wed, 5 Jun 2013 10:11:12 -0400 (EDT),
Alan Stern wrote:
>
> On Wed, 5 Jun 2013, Takashi Iwai wrote:
>
> > > >> Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from
> > > >> upstream, and we explicitly disable it. Is there any reason why we
> > > >> do that?
> > > >
> > > > config SND_DEBUG
> > > > bool "Debug"
> > > > help
> > > > Say Y here to enable ALSA debug code.
> > > >
> > > > It is off by default in upstream, and the really helpful description
> > > > would cirtainly tend to lead to it being disabled. But if it is helpful
> > > > to your debugging efforts David then I suspect we can enable it in Saucy
> > > > and see what happens.
> > >
> > > Okay, so then the ball is back in Takashi's area - if we're recommended
> > > to turn CONFIG_SND_DEBUG on, why is it off by default in the upstream
> > > Linux kernel?
> >
> > It's not off as default. Simply there is no default, just like most
> > of other options.
> >
> > As already mentioned, if the device is known to work well with the
> > kernel, there is no reason to set it on. Then it'll saves the memory
> > and code space. That is, for custom kernels, it's good to be off.
> > But for generic kernels like distro kernel, it'd be better to take a
> > safer side with more safety checks that is built in by that option.
>
> Given this description, the symbol's name is very misleading. Instead
> of being called CONFIG_SND_DEBUG, it should be called
> CONFIG_SND_SAFETY_CHECKS or something like that.
Very true. Actually this option has multiple implicit meanings. It
also enables some debug things, of course, too.
> IMO, distributions are completely justified in turning off any symbol
> which is named (and described in the help text!) as being used for
> debugging only.
Heh, I don't blame distros but just recommend to turn it on :)
Anyway, since 3.10, safety checks with snd_BUG_ON() have been already
enabled even without CONFIG_SND_DEBUG. But this won't give an error
message via WARN() but simply returns the error when built
CONFIG_SND_DEBUG, so it won't help for debugging much, as expected.
And, Alan, please don't work on a patch to correct Kconfig text yet.
I see on my crystalball that you'd send a patch in this very minute ;)
We'll need to work on the debug print things, so the whole kconfig
options need revisited. Let's sort out what to be changed for
messages, split the sanity check codes and debug codes, and then go on
fixing/improving Kconfig appropriately.
thanks,
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: CONFIG_SND_DEBUG
2013-06-05 14:28 ` CONFIG_SND_DEBUG Takashi Iwai
@ 2013-06-05 14:47 ` Alan Stern
0 siblings, 0 replies; 44+ messages in thread
From: Alan Stern @ 2013-06-05 14:47 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, Joe Perches, Kernel team list, Andy Whitcroft,
leann.ogasawara, David Henningsson
On Wed, 5 Jun 2013, Takashi Iwai wrote:
> At Wed, 5 Jun 2013 10:11:12 -0400 (EDT),
> Alan Stern wrote:
> >
> > On Wed, 5 Jun 2013, Takashi Iwai wrote:
> >
> > > > >> Andy/Leann - apparently CONFIG_SND_DEBUG is on by default from
> > > > >> upstream, and we explicitly disable it. Is there any reason why we
> > > > >> do that?
> > > > >
> > > > > config SND_DEBUG
> > > > > bool "Debug"
> > > > > help
> > > > > Say Y here to enable ALSA debug code.
> > > > >
> > > > > It is off by default in upstream, and the really helpful description
> > > > > would cirtainly tend to lead to it being disabled. But if it is helpful
> > > > > to your debugging efforts David then I suspect we can enable it in Saucy
> > > > > and see what happens.
> > > >
> > > > Okay, so then the ball is back in Takashi's area - if we're recommended
> > > > to turn CONFIG_SND_DEBUG on, why is it off by default in the upstream
> > > > Linux kernel?
> > >
> > > It's not off as default. Simply there is no default, just like most
> > > of other options.
> > >
> > > As already mentioned, if the device is known to work well with the
> > > kernel, there is no reason to set it on. Then it'll saves the memory
> > > and code space. That is, for custom kernels, it's good to be off.
> > > But for generic kernels like distro kernel, it'd be better to take a
> > > safer side with more safety checks that is built in by that option.
> >
> > Given this description, the symbol's name is very misleading. Instead
> > of being called CONFIG_SND_DEBUG, it should be called
> > CONFIG_SND_SAFETY_CHECKS or something like that.
>
> Very true. Actually this option has multiple implicit meanings. It
> also enables some debug things, of course, too.
>
> > IMO, distributions are completely justified in turning off any symbol
> > which is named (and described in the help text!) as being used for
> > debugging only.
>
> Heh, I don't blame distros but just recommend to turn it on :)
>
> Anyway, since 3.10, safety checks with snd_BUG_ON() have been already
> enabled even without CONFIG_SND_DEBUG. But this won't give an error
> message via WARN() but simply returns the error when built
> CONFIG_SND_DEBUG, so it won't help for debugging much, as expected.
>
>
> And, Alan, please don't work on a patch to correct Kconfig text yet.
> I see on my crystalball that you'd send a patch in this very minute ;)
Okay, I'll try to restrain myself. :-)
> We'll need to work on the debug print things, so the whole kconfig
> options need revisited. Let's sort out what to be changed for
> messages, split the sanity check codes and debug codes, and then go on
> fixing/improving Kconfig appropriately.
Given the amount of detailed attention that will be needed for fixing
up all those debug messages, I'd prefer to ignore the whole issue for
now. My intention is to concentrate just on the sound/usb drivers,
where I actually have some idea of what the code is doing, and maybe
also sound/core.
I'll go into more detail in a separate email that doesn't have all the
extra people on the CC: list.
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-04 21:19 ` Joe Perches
@ 2013-06-06 20:42 ` Alan Stern
2013-06-06 20:59 ` Joe Perches
0 siblings, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-06 20:42 UTC (permalink / raw)
To: Joe Perches; +Cc: Takashi Iwai, alsa-devel
On Tue, 4 Jun 2013, Joe Perches wrote:
> On Tue, 2013-06-04 at 16:54 -0400, Alan Stern wrote:
> > 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.
>
> Not really.
>
> Think of CONFIG_SND_DEBUG as a level. (0, 1, 2)
> or maybe think of it as CONFIG_SND_DEBUG_VERBOSITY.
>
> There could still be ability to have CONFIG_SND_DEBUG
> limit the compiled-in messages to those below the
> #define value and still then have runtime control over
> which ones are displayed.
How useful really is it to be able to limit the amount of debugging
messages at runtime? Does anybody ever actually adjust the "debug"
module parameter?
In my opinion, this is not worth the extra space required. Virtually
all the benefit of different debugging levels can be obtained by
defining different symbols at compile time, such as CONFIG_SND_DEBUG
and CONFIG_SND_VERBOSE_DEBUG. That gives you two levels of debugging
(not counting the "none" setting), which is enough for everything but
snd_printddd(). Does anybody really need a third level?
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info("
2013-06-05 5:52 ` Takashi Iwai
2013-06-05 6:07 ` Joe Perches
@ 2013-06-06 20:54 ` Alan Stern
2013-06-07 5:41 ` Takashi Iwai
1 sibling, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-06 20:54 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
On Wed, 5 Jun 2013, Takashi Iwai wrote:
> At Tue, 4 Jun 2013 13:20:40 -0400 (EDT),
> Alan Stern wrote:
> >
> > The snd_printk() function prints kernel log messages, including the
> > filename and line number if CONFIG_SND_PRINTK_VERBOSE is enabled.
> > This may make sense for errors and warnings, but not for informational
> > messages. For those, a simple pr_info() is what we want.
> >
> > This patch mechanically converts all occurrences of
> > "snd_printk(KERN_INFO" to "pr_info(". It doesn't try to tell whether
> > the message really is informational; it relies on the existing
> > KERN_INFO tag.
>
> I agree conversion in this way. But looking at the patch, some places
> should be better convert with pr_warning() or pr_err().
How often do those places get printed? I would assume that warnings
and errors are relatively infrequent, in which case there would be no
harm in changing "snd_printk(KERN_INFO" to "snd_printk(KERN_WARN" or
"snd_printk(KERN_ERR".
> Also, many places miss proper prefix, thus you'll still see the
What exactly do you mean by "proper prefix"? KBUILD_MODNAME? The
filename and line number? The device and driver names? Or something
else?
> original issue the thread started from (no clue who prints the stuff).
> This is because originally snd_printk() printed the prefix and line
> number always. CONFIG_SND_VERBOSE_PRINTK was introduced later since
> some people complained about too verbose output, IIRC, while many
> codes weren't fixed to give a proper prefix with
> CONFIG_SND_VERBOSE_PRINTK=n.
After this patch (or after an updated version of this patch), none of
the snd_printk calls will be informational. They will all be things
like errors and warnings. This means they won't get printed very
often, so always using the verbose form shouldn't make things too bad.
> Just taking a quick glance:
>
> > --- usb-3.10.orig/sound/isa/opti9xx/miro.c
> > +++ usb-3.10/sound/isa/opti9xx/miro.c
> > @@ -1346,11 +1346,11 @@ static int snd_miro_probe(struct snd_car
> > default:
> > sprintf(card->shortname,
> > "unknown miro");
> > - snd_printk(KERN_INFO "unknown miro aci id\n");
> > + pr_info("unknown miro aci id\n");
> > break;
> > }
> > } else {
> > - snd_printk(KERN_INFO "found unsupported aci card\n");
> > + pr_info("found unsupported aci card\n");
> > sprintf(card->shortname, "unknown Cardinal Technologies");
>
> These need proper prefix, and should be rather pr_warning().
I don't understand. pr_warning doesn't print any prefixes, by default.
They would have to added to the format string. Why not change it to
"snd_printk(KERN_WARN"?
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-06 20:42 ` Alan Stern
@ 2013-06-06 20:59 ` Joe Perches
2013-06-07 14:40 ` Alan Stern
0 siblings, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-06 20:59 UTC (permalink / raw)
To: Alan Stern; +Cc: Takashi Iwai, alsa-devel
On Thu, 2013-06-06 at 16:42 -0400, Alan Stern wrote:
> On Tue, 4 Jun 2013, Joe Perches wrote:
>
> > On Tue, 2013-06-04 at 16:54 -0400, Alan Stern wrote:
> > > 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.
> >
> > Not really.
> >
> > Think of CONFIG_SND_DEBUG as a level. (0, 1, 2)
> > or maybe think of it as CONFIG_SND_DEBUG_VERBOSITY.
> >
> > There could still be ability to have CONFIG_SND_DEBUG
> > limit the compiled-in messages to those below the
> > #define value and still then have runtime control over
> > which ones are displayed.
>
> How useful really is it to be able to limit the amount of debugging
> messages at runtime? Does anybody ever actually adjust the "debug"
> module parameter?
When it's a bitmask, yes.
It then becomes similar to ethtool.
> In my opinion, this is not worth the extra space required. Virtually
> all the benefit of different debugging levels can be obtained by
> defining different symbols at compile time, such as CONFIG_SND_DEBUG
> and CONFIG_SND_VERBOSE_DEBUG.
I think the whole verbosity thing should be done at runtime
via pr_debug and classifications by type via bitmasks at
compile-time instead of the silliness of something like
CONFIG_SND_DEBUG_VERBOSITY.
> Does anybody really need a third level?
Hard to say. There are several additional "private"
debugging level controls for sound/...
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-05 6:04 ` Takashi Iwai
2013-06-05 6:15 ` Joe Perches
@ 2013-06-06 21:28 ` Alan Stern
2013-06-06 21:50 ` Joe Perches
2013-06-07 5:53 ` Takashi Iwai
1 sibling, 2 replies; 44+ messages in thread
From: Alan Stern @ 2013-06-06 21:28 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
On Wed, 5 Jun 2013, Takashi Iwai wrote:
> > 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?
>
> Unfortunately, it's not so straightforward. Many messages are better
> with KERN_INFO indeed. In such places, snd_printd() is used rather as
> snd_chattier_printk_with_prefix().
I don't understand the point of this. The idea of using snd_printd()
is to prevent these messages from being printed unless debugging is
enabled. Since the only time you are going to see these messages is
when CONFIG_SND_DEBUG is on, why shouldn't they use KERN_DEBUG?
> This comes from the subsystem development history. In the era of 2.4
> kernels, many systems did load/unload the module frequently on demand.
> Thus we didn't want to put a message from driver at each module probe
> or removal. Otherwise dmesg will be flood of such messages at each
> time you play a WAV song or ring a beep via PCM.
>
> Instead, we tended to put such informational messages as snd_printd(),
> while keeping the driver itself reticent as much as possible for
> "productive" systems. This style was kept for a while even after
> merged to 2.5 kernels until recently.
Changing snd_printd() to use KERN_DEBUG always won't make the driver
any less reticent.
> Also, some places use KERN_WARNING or KERN_ERR with snd_printd(),
> mostly because they are in the context with CONFIG_SND_DEBUG.
> They can be well pr_warning() or pr_err().
Okay. Those can be changed separately.
> Last but not least, as mentioned in the reply to patch 1/2, many
> messages miss proper prefixes.
>
>
> These are the main reasons I stated that systematic replacements would
> be difficult. We need to take a look through the whole snd_print*()
> and replace appropriately case by case...
I'm not sure if my original point behind all this is still clear. I
don't really want to make a lot of small updates to the logging API for
the sound subsystem, or clean up messages that have the wrong logging
level.
The important point is that these files are _drivers_; what matters at
runtime is the device name. On systems with more than one sound card
of the same type, the device name is vital, but even on others it still
helps a lot.
What I want to do is make the messages include the device and driver
names. Basically this means calling dev_info() and friends instead of
"snd_printk(KERN_INFO" or pr_info(). As part of the fallout from this
change, it should no longer be necessary to print the filename or
module name. If a driver contains multiple identical messages, they
can be made non-identical (assuming it really matters; messages like
"can't allocate memory" usually don't need to be pinned down exactly).
Line numbers are even less useful because they can change from one
kernel version to the next.
I can't convert the entire sound subsystem at once -- it's way too big.
But I can at least do the sound/usb subsystem. If this means that two
different logging APIs are used by different sets of sound drivers, so
be it.
In theory, this would be more or less independent of the patches posted
so far. In fact, when taken to its logical extreme, _all_ the messages
would use dev_*() or some variant. There wouldn't be any snd_printk(),
snd_printd(), pr_info(), ... messages left to worry about.
I don't expect that to happen any time soon. Still, I think it makes
more sense to concentrate on converting the messages below sound/usb
rather than fiddling around with the existing API, which is inadequate
anyway since it doesn't include the device.
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-06 21:28 ` [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
@ 2013-06-06 21:50 ` Joe Perches
2013-06-07 5:57 ` Takashi Iwai
2013-06-07 5:53 ` Takashi Iwai
1 sibling, 1 reply; 44+ messages in thread
From: Joe Perches @ 2013-06-06 21:50 UTC (permalink / raw)
To: Alan Stern; +Cc: Takashi Iwai, alsa-devel
On Thu, 2013-06-06 at 17:28 -0400, Alan Stern wrote:
> The important point is that these files are _drivers_; what matters at
> runtime is the device name. On systems with more than one sound card
> of the same type, the device name is vital, but even on others it still
> helps a lot.
>
> What I want to do is make the messages include the device and driver
> names. Basically this means calling dev_info() and friends instead of
> "snd_printk(KERN_INFO" or pr_info(). As part of the fallout from this
> change, it should no longer be necessary to print the filename or
> module name. If a driver contains multiple identical messages, they
> can be made non-identical (assuming it really matters; messages like
> "can't allocate memory" usually don't need to be pinned down exactly).
> Line numbers are even less useful because they can change from one
> kernel version to the next.
>
> I can't convert the entire sound subsystem at once -- it's way too big.
> But I can at least do the sound/usb subsystem. If this means that two
> different logging APIs are used by different sets of sound drivers, so
> be it.
>
> In theory, this would be more or less independent of the patches posted
> so far. In fact, when taken to its logical extreme, _all_ the messages
> would use dev_*() or some variant. There wouldn't be any snd_printk(),
> snd_printd(), pr_info(), ... messages left to worry about.
>
> I don't expect that to happen any time soon. Still, I think it makes
> more sense to concentrate on converting the messages below sound/usb
> rather than fiddling around with the existing API, which is inadequate
> anyway since it doesn't include the device.
I agree with that.
It'd be pretty easy to take that large patch that did
snd_printk(fmt, ...) -> snd_dbg(1, fmt, ...) and
convert it to snd_card_dbg(card *, fmt, ...)
whenever a struct snd_card * was available.
snd_card_dbg() would be either a macro or a function
to deference the snd_card * dev and maybe emit
"struct snd_card.shortname" with it too.
Want to start with that?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info("
2013-06-06 20:54 ` Alan Stern
@ 2013-06-07 5:41 ` Takashi Iwai
2013-06-07 15:51 ` Alan Stern
0 siblings, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-07 5:41 UTC (permalink / raw)
To: Alan Stern; +Cc: Joe Perches, alsa-devel
At Thu, 6 Jun 2013 16:54:06 -0400 (EDT),
Alan Stern wrote:
>
> On Wed, 5 Jun 2013, Takashi Iwai wrote:
>
> > At Tue, 4 Jun 2013 13:20:40 -0400 (EDT),
> > Alan Stern wrote:
> > >
> > > The snd_printk() function prints kernel log messages, including the
> > > filename and line number if CONFIG_SND_PRINTK_VERBOSE is enabled.
> > > This may make sense for errors and warnings, but not for informational
> > > messages. For those, a simple pr_info() is what we want.
> > >
> > > This patch mechanically converts all occurrences of
> > > "snd_printk(KERN_INFO" to "pr_info(". It doesn't try to tell whether
> > > the message really is informational; it relies on the existing
> > > KERN_INFO tag.
> >
> > I agree conversion in this way. But looking at the patch, some places
> > should be better convert with pr_warning() or pr_err().
>
> How often do those places get printed? I would assume that warnings
> and errors are relatively infrequent, in which case there would be no
> harm in changing "snd_printk(KERN_INFO" to "snd_printk(KERN_WARN" or
> "snd_printk(KERN_ERR".
Yes. That's my intention, too. These shouldn't have been KERN_INFO
from the beginning.
> > Also, many places miss proper prefix, thus you'll still see the
>
> What exactly do you mean by "proper prefix"? KBUILD_MODNAME? The
> filename and line number? The device and driver names? Or something
> else?
I don't mind any way if it's unique enough to identify from where the
message comes, but some people prefer (over-)unification.
Usually KBUILD_MODNAME should suffice, I guess, but still function
name or file name might be needed in addition if similar messages
appear multiple times in the same driver. This must be all
case-by-case decision.
> > original issue the thread started from (no clue who prints the stuff).
> > This is because originally snd_printk() printed the prefix and line
> > number always. CONFIG_SND_VERBOSE_PRINTK was introduced later since
> > some people complained about too verbose output, IIRC, while many
> > codes weren't fixed to give a proper prefix with
> > CONFIG_SND_VERBOSE_PRINTK=n.
>
> After this patch (or after an updated version of this patch), none of
> the snd_printk calls will be informational. They will all be things
> like errors and warnings. This means they won't get printed very
> often, so always using the verbose form shouldn't make things too bad.
Yes, it's the reason I suggested to apply always
CONFIG_SND_VERBOSE_PRINTK and drop the kconfig :)
> > Just taking a quick glance:
> >
> > > --- usb-3.10.orig/sound/isa/opti9xx/miro.c
> > > +++ usb-3.10/sound/isa/opti9xx/miro.c
> > > @@ -1346,11 +1346,11 @@ static int snd_miro_probe(struct snd_car
> > > default:
> > > sprintf(card->shortname,
> > > "unknown miro");
> > > - snd_printk(KERN_INFO "unknown miro aci id\n");
> > > + pr_info("unknown miro aci id\n");
> > > break;
> > > }
> > > } else {
> > > - snd_printk(KERN_INFO "found unsupported aci card\n");
> > > + pr_info("found unsupported aci card\n");
> > > sprintf(card->shortname, "unknown Cardinal Technologies");
> >
> > These need proper prefix, and should be rather pr_warning().
>
> I don't understand. pr_warning doesn't print any prefixes, by default.
> They would have to added to the format string. Why not change it to
> "snd_printk(KERN_WARN"?
Yes, this should be snd_printk(KERN_WARN) *and* it needs a proper
prefix depending on CONFIG_SND_VERBOSE_PRINTK:
#ifdef CONFIG_SND_VERBOSE_PRINTK
#define PREFIX ""
#else
#define PREFIX "miro: "
#endif
....
snd_printk(KERN_WARNING PFX "found unsupported aci card\n");
The same check should be applied to all snd_printk() lines.
Once after all snd_printk() have the unified prefix, we can convert
straightforwardly like
#define pr_fmt "miro: "
....
pr_warn("found unsupported aci card\n");
In other words, toward the final conversions, we need to audit:
- Whether each message is really marked properly with KERN_* level,
- Whether messages have consistent prefix through the whole module,
- whether messages are unique enough to be identified,
- optionally, check typos
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-06 21:28 ` [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
2013-06-06 21:50 ` Joe Perches
@ 2013-06-07 5:53 ` Takashi Iwai
1 sibling, 0 replies; 44+ messages in thread
From: Takashi Iwai @ 2013-06-07 5:53 UTC (permalink / raw)
To: Alan Stern; +Cc: Joe Perches, alsa-devel
At Thu, 6 Jun 2013 17:28:52 -0400 (EDT),
Alan Stern wrote:
>
> On Wed, 5 Jun 2013, Takashi Iwai wrote:
>
> > > 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?
> >
> > Unfortunately, it's not so straightforward. Many messages are better
> > with KERN_INFO indeed. In such places, snd_printd() is used rather as
> > snd_chattier_printk_with_prefix().
>
> I don't understand the point of this. The idea of using snd_printd()
> is to prevent these messages from being printed unless debugging is
> enabled.
Originally, yes, but differently developed along time.
> Since the only time you are going to see these messages is
> when CONFIG_SND_DEBUG is on, why shouldn't they use KERN_DEBUG?
KERN_DEBUG appears in dmesg normally, no?
> > This comes from the subsystem development history. In the era of 2.4
> > kernels, many systems did load/unload the module frequently on demand.
> > Thus we didn't want to put a message from driver at each module probe
> > or removal. Otherwise dmesg will be flood of such messages at each
> > time you play a WAV song or ring a beep via PCM.
> >
> > Instead, we tended to put such informational messages as snd_printd(),
> > while keeping the driver itself reticent as much as possible for
> > "productive" systems. This style was kept for a while even after
> > merged to 2.5 kernels until recently.
>
> Changing snd_printd() to use KERN_DEBUG always won't make the driver
> any less reticent.
>
> > Also, some places use KERN_WARNING or KERN_ERR with snd_printd(),
> > mostly because they are in the context with CONFIG_SND_DEBUG.
> > They can be well pr_warning() or pr_err().
>
> Okay. Those can be changed separately.
>
> > Last but not least, as mentioned in the reply to patch 1/2, many
> > messages miss proper prefixes.
> >
> >
> > These are the main reasons I stated that systematic replacements would
> > be difficult. We need to take a look through the whole snd_print*()
> > and replace appropriately case by case...
>
> I'm not sure if my original point behind all this is still clear. I
> don't really want to make a lot of small updates to the logging API for
> the sound subsystem, or clean up messages that have the wrong logging
> level.
>
> The important point is that these files are _drivers_; what matters at
> runtime is the device name. On systems with more than one sound card
> of the same type, the device name is vital, but even on others it still
> helps a lot.
>
> What I want to do is make the messages include the device and driver
> names. Basically this means calling dev_info() and friends instead of
> "snd_printk(KERN_INFO" or pr_info(). As part of the fallout from this
> change, it should no longer be necessary to print the filename or
> module name. If a driver contains multiple identical messages, they
> can be made non-identical (assuming it really matters; messages like
> "can't allocate memory" usually don't need to be pinned down exactly).
> Line numbers are even less useful because they can change from one
> kernel version to the next.
>
> I can't convert the entire sound subsystem at once -- it's way too big.
> But I can at least do the sound/usb subsystem. If this means that two
> different logging APIs are used by different sets of sound drivers, so
> be it.
>
> In theory, this would be more or less independent of the patches posted
> so far. In fact, when taken to its logical extreme, _all_ the messages
> would use dev_*() or some variant. There wouldn't be any snd_printk(),
> snd_printd(), pr_info(), ... messages left to worry about.
>
> I don't expect that to happen any time soon. Still, I think it makes
> more sense to concentrate on converting the messages below sound/usb
> rather than fiddling around with the existing API, which is inadequate
> anyway since it doesn't include the device.
Yeah, dev_*() is more appropriate in many places.
Feel free to work on replacement with dev_*() variants for
sound/usb/*. I prefer such bottom up cleanups, actually.
thanks,
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-06 21:50 ` Joe Perches
@ 2013-06-07 5:57 ` Takashi Iwai
2013-06-07 15:34 ` Alan Stern
0 siblings, 1 reply; 44+ messages in thread
From: Takashi Iwai @ 2013-06-07 5:57 UTC (permalink / raw)
To: Joe Perches; +Cc: alsa-devel, Alan Stern
At Thu, 06 Jun 2013 14:50:32 -0700,
Joe Perches wrote:
>
> On Thu, 2013-06-06 at 17:28 -0400, Alan Stern wrote:
> > The important point is that these files are _drivers_; what matters at
> > runtime is the device name. On systems with more than one sound card
> > of the same type, the device name is vital, but even on others it still
> > helps a lot.
> >
> > What I want to do is make the messages include the device and driver
> > names. Basically this means calling dev_info() and friends instead of
> > "snd_printk(KERN_INFO" or pr_info(). As part of the fallout from this
> > change, it should no longer be necessary to print the filename or
> > module name. If a driver contains multiple identical messages, they
> > can be made non-identical (assuming it really matters; messages like
> > "can't allocate memory" usually don't need to be pinned down exactly).
> > Line numbers are even less useful because they can change from one
> > kernel version to the next.
> >
> > I can't convert the entire sound subsystem at once -- it's way too big.
> > But I can at least do the sound/usb subsystem. If this means that two
> > different logging APIs are used by different sets of sound drivers, so
> > be it.
> >
> > In theory, this would be more or less independent of the patches posted
> > so far. In fact, when taken to its logical extreme, _all_ the messages
> > would use dev_*() or some variant. There wouldn't be any snd_printk(),
> > snd_printd(), pr_info(), ... messages left to worry about.
> >
> > I don't expect that to happen any time soon. Still, I think it makes
> > more sense to concentrate on converting the messages below sound/usb
> > rather than fiddling around with the existing API, which is inadequate
> > anyway since it doesn't include the device.
>
> I agree with that.
>
> It'd be pretty easy to take that large patch that did
> snd_printk(fmt, ...) -> snd_dbg(1, fmt, ...) and
> convert it to snd_card_dbg(card *, fmt, ...)
> whenever a struct snd_card * was available.
>
> snd_card_dbg() would be either a macro or a function
> to deference the snd_card * dev and maybe emit
> "struct snd_card.shortname" with it too.
>
> Want to start with that?
As discussed earlier, the snd_card instance isn't always appropriate
for retrieving the device pointer. It might help some driver codes,
but not always.
In the case of sound/usb/*, most of codes rather handle directly the
USB device pointer (mostly struct usb_interface), thus it's more
natural to use it directly, IMO.
thanks,
Takashi
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-06 20:59 ` Joe Perches
@ 2013-06-07 14:40 ` Alan Stern
2013-06-07 16:10 ` Joe Perches
0 siblings, 1 reply; 44+ messages in thread
From: Alan Stern @ 2013-06-07 14:40 UTC (permalink / raw)
To: Joe Perches; +Cc: Takashi Iwai, alsa-devel
On Thu, 6 Jun 2013, Joe Perches wrote:
> > How useful really is it to be able to limit the amount of debugging
> > messages at runtime? Does anybody ever actually adjust the "debug"
> > module parameter?
>
> When it's a bitmask, yes.
> It then becomes similar to ethtool.
But it isn't a bitmask; it's just a level. From
sound/core/misc.c:__snd_printk():
#ifdef CONFIG_SND_DEBUG
if (debug < level)
return;
#endif
Since I have no intention of making it a bitmask at the moment, I will
presume your answer really means "No".
> > In my opinion, this is not worth the extra space required. Virtually
> > all the benefit of different debugging levels can be obtained by
> > defining different symbols at compile time, such as CONFIG_SND_DEBUG
> > and CONFIG_SND_VERBOSE_DEBUG.
>
> I think the whole verbosity thing should be done at runtime
> via pr_debug and classifications by type via bitmasks at
> compile-time instead of the silliness of something like
> CONFIG_SND_DEBUG_VERBOSITY.
The appeal of this approach is understandable. But it is a whole
different project from the conversion I'm considering for now.
> > Does anybody really need a third level?
>
> Hard to say. There are several additional "private"
> debugging level controls for sound/...
Then they can remain private. Takashi asked me not to change the
existing Kconfig options, and I won't.
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-07 5:57 ` Takashi Iwai
@ 2013-06-07 15:34 ` Alan Stern
0 siblings, 0 replies; 44+ messages in thread
From: Alan Stern @ 2013-06-07 15:34 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
On Fri, 7 Jun 2013, Takashi Iwai wrote:
> > > I don't expect that to happen any time soon. Still, I think it makes
> > > more sense to concentrate on converting the messages below sound/usb
> > > rather than fiddling around with the existing API, which is inadequate
> > > anyway since it doesn't include the device.
> >
> > I agree with that.
> >
> > It'd be pretty easy to take that large patch that did
> > snd_printk(fmt, ...) -> snd_dbg(1, fmt, ...) and
> > convert it to snd_card_dbg(card *, fmt, ...)
> > whenever a struct snd_card * was available.
> >
> > snd_card_dbg() would be either a macro or a function
> > to deference the snd_card * dev and maybe emit
> > "struct snd_card.shortname" with it too.
> >
> > Want to start with that?
>
> As discussed earlier, the snd_card instance isn't always appropriate
> for retrieving the device pointer. It might help some driver codes,
> but not always.
>
> In the case of sound/usb/*, most of codes rather handle directly the
> USB device pointer (mostly struct usb_interface), thus it's more
> natural to use it directly, IMO.
Agreed -- the lower-level driver should use the underlying device for
their messages. The natural device for the snd_usb_audio driver to
report is the USB interface. This would be the audio control interface
in many cases, or it could be the interface that contains the streaming
endpoints, or the interface containing the mixer, or the MIDI
interface...
The relevant data structures don't all store a pointer to the
interface, so I will have to add one. That's another example of why
the conversion can't simply be done mindlessly.
For log messages in the sound core, it does make sense to use
snd_card->dev, or (preferably) snd_card->card_dev when it is available.
As Takashi mentioned, the lower drivers often do not set snd_card->dev
as quickly as they should, i.e., they don't call snd_card_set_dev()
right after creating the snd_card. The best way to solve this is
probably to add a new routine, a version of snd_card_create() that
accepts the device pointer as an additional argument.
(Or maybe just change snd_card_create() and all its callers in one big
patch. It would be a fair amount of work, but doable.)
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info("
2013-06-07 5:41 ` Takashi Iwai
@ 2013-06-07 15:51 ` Alan Stern
0 siblings, 0 replies; 44+ messages in thread
From: Alan Stern @ 2013-06-07 15:51 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Joe Perches, alsa-devel
On Fri, 7 Jun 2013, Takashi Iwai wrote:
> > > I agree conversion in this way. But looking at the patch, some places
> > > should be better convert with pr_warning() or pr_err().
> >
> > How often do those places get printed? I would assume that warnings
> > and errors are relatively infrequent, in which case there would be no
> > harm in changing "snd_printk(KERN_INFO" to "snd_printk(KERN_WARN" or
> > "snd_printk(KERN_ERR".
>
> Yes. That's my intention, too. These shouldn't have been KERN_INFO
> from the beginning.
Okay.
> > > Also, many places miss proper prefix, thus you'll still see the
> >
> > What exactly do you mean by "proper prefix"? KBUILD_MODNAME? The
> > filename and line number? The device and driver names? Or something
> > else?
>
> I don't mind any way if it's unique enough to identify from where the
> message comes, but some people prefer (over-)unification.
> Usually KBUILD_MODNAME should suffice, I guess, but still function
> name or file name might be needed in addition if similar messages
> appear multiple times in the same driver. This must be all
> case-by-case decision.
For the messages that get converted to "snd_printk(KERN_ERR", the
prefix added by snd_printk() when CONFIG_SND_VERBOSE_PRINTK is enabled
should be sufficient. Following the 2/2 patch in this series, that
symbol will effectively _always_ be enabled. Therefore these messages
shouldn't need any extra prefix.
The ones that get converted to pr_info() probably will need a short
prefix of some kind, though.
> > > These need proper prefix, and should be rather pr_warning().
> >
> > I don't understand. pr_warning doesn't print any prefixes, by default.
> > They would have to added to the format string. Why not change it to
> > "snd_printk(KERN_WARN"?
>
> Yes, this should be snd_printk(KERN_WARN) *and* it needs a proper
> prefix depending on CONFIG_SND_VERBOSE_PRINTK:
>
> #ifdef CONFIG_SND_VERBOSE_PRINTK
> #define PREFIX ""
> #else
> #define PREFIX "miro: "
> #endif
> ....
> snd_printk(KERN_WARNING PFX "found unsupported aci card\n");
There's no point using PREFIX (or PFX) here, since the 2/2 patch would
eliminate the second #define anyway.
> The same check should be applied to all snd_printk() lines.
>
> Once after all snd_printk() have the unified prefix, we can convert
> straightforwardly like
>
> #define pr_fmt "miro: "
> ....
> pr_warn("found unsupported aci card\n");
Ultimately, it will be better to change these to dev_warn() or
something equivalent. Then the "miro:" prefix wouldn't be needed.
However, doing what you suggest might be a reasonable intermediate
step, aside from for all the additional work required.
> In other words, toward the final conversions, we need to audit:
> - Whether each message is really marked properly with KERN_* level,
> - Whether messages have consistent prefix through the whole module,
> - whether messages are unique enough to be identified,
> - optionally, check typos
I'm willing to do the first step for the messages that were altered in
the 1/2 patch (but I don't want to audit every message under sound/).
The second step shouldn't be needed, according to the reasoning above.
The third step is something that will have to be done on a
driver-by-driver basis, since it requires looking through all the
messages in each driver at once to determine if there are any
duplicates. I don't want to tackle that (except perhaps for the USB
drivers).
Finally, typos can be fixed separately.
Alan Stern
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
2013-06-07 14:40 ` Alan Stern
@ 2013-06-07 16:10 ` Joe Perches
0 siblings, 0 replies; 44+ messages in thread
From: Joe Perches @ 2013-06-07 16:10 UTC (permalink / raw)
To: Alan Stern; +Cc: Takashi Iwai, alsa-devel
On Fri, 2013-06-07 at 10:40 -0400, Alan Stern wrote:
> On Thu, 6 Jun 2013, Joe Perches wrote:
> > > How useful really is it to be able to limit the amount of debugging
> > > messages at runtime? Does anybody ever actually adjust the "debug"
> > > module parameter?
> > When it's a bitmask, yes.
> > It then becomes similar to ethtool.
> But it isn't a bitmask; it's just a level.
I understood that from the beginning and was
describing generically the different approaches
used in the kernel.
I was simply suggesting sound use classification
via bitmask instead of level.
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2013-06-07 16:10 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-03 20:18 [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
2013-06-03 20:24 ` Joe Perches
2013-06-03 20:40 ` Alan Stern
2013-06-03 20:49 ` Joe Perches
2013-06-04 9:13 ` Takashi Iwai
2013-06-04 14:49 ` Alan Stern
2013-06-04 15:03 ` Takashi Iwai
2013-06-04 17:20 ` [PATCH 1/2] ALSA: convert "snd_printk(KERN_INFO" to "pr_info(" Alan Stern
2013-06-05 5:52 ` Takashi Iwai
2013-06-05 6:07 ` Joe Perches
2013-06-05 6:16 ` Takashi Iwai
2013-06-06 20:54 ` Alan Stern
2013-06-07 5:41 ` Takashi Iwai
2013-06-07 15:51 ` Alan Stern
2013-06-04 17:20 ` [PATCH 2/2 v.2] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
2013-06-04 19:32 ` [PATCH] " Alan Stern
2013-06-04 19:45 ` Joe Perches
2013-06-04 20:54 ` Alan Stern
2013-06-04 21:19 ` Joe Perches
2013-06-06 20:42 ` Alan Stern
2013-06-06 20:59 ` Joe Perches
2013-06-07 14:40 ` Alan Stern
2013-06-07 16:10 ` Joe Perches
2013-06-05 6:04 ` Takashi Iwai
2013-06-05 6:15 ` Joe Perches
2013-06-05 6:32 ` Takashi Iwai
2013-06-05 6:52 ` Joe Perches
2013-06-05 6:54 ` Takashi Iwai
2013-06-05 7:07 ` Joe Perches
2013-06-05 7:22 ` Takashi Iwai
2013-06-05 7:34 ` Joe Perches
2013-06-05 7:47 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK) David Henningsson
2013-06-05 8:46 ` CONFIG_SND_DEBUG (was: " Takashi Iwai
2013-06-05 10:53 ` CONFIG_SND_DEBUG (was: Re: [alsa-devel] " Andy Whitcroft
2013-06-05 11:38 ` CONFIG_SND_DEBUG David Henningsson
2013-06-05 11:43 ` CONFIG_SND_DEBUG Takashi Iwai
2013-06-05 14:11 ` CONFIG_SND_DEBUG Alan Stern
2013-06-05 14:28 ` CONFIG_SND_DEBUG Takashi Iwai
2013-06-05 14:47 ` CONFIG_SND_DEBUG Alan Stern
2013-06-06 21:28 ` [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Alan Stern
2013-06-06 21:50 ` Joe Perches
2013-06-07 5:57 ` Takashi Iwai
2013-06-07 15:34 ` Alan Stern
2013-06-07 5:53 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.