All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Joe Perches <joe@perches.com>
Cc: alsa-devel@alsa-project.org,
	Alan Stern <stern@rowland.harvard.edu>,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: Improving or replacing snd_printk()
Date: Fri, 31 May 2013 09:23:57 +0200	[thread overview]
Message-ID: <s5hip1zipde.wl%tiwai@suse.de> (raw)
In-Reply-To: <1369983967.10556.48.camel@joe-AO722>

At Fri, 31 May 2013 00:06:07 -0700,
Joe Perches wrote:
> 
> On Fri, 2013-05-31 at 08:37 +0200, Takashi Iwai wrote:
> > It's a problem only with snd_printk(), as CONFIG_SND_VERBOSE_PRINTK
> > influences on the behavior of snd_printk() and not on the debug prints
> > with snd_printd() & co.
> 
> Are you're perhaps confused about how CONFIG_SND_VERBOSE_PRINTK
> works with or varies between snd_printd and snd_printk?
> 
> #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, ...);
> 
> ...
> 
> #define snd_printk(fmt, args...) \
> 	__snd_printk(0, __FILE__, __LINE__, fmt, ##args)
> 
> #ifdef CONFIG_SND_DEBUG
> [...]
> #define snd_printd(fmt, args...) \
> 	__snd_printk(1, __FILE__, __LINE__, fmt, ##args)
> 
> I don't see a difference.

Erm, indeed.  I thought we enabled the verobse prinkt always for debug
prints.  But it's not.  It's more reason for dropping
CONFIG_SND_VERBOSE_PRINTK :)

> > Hence, the goal we should achieve is rather to drop
> > CONFIG_SND_VERBOSE_PRINTK.  *This* is the useless thing.
> 
> Maybe.
> 
> > Meanwhile, many snd_printk() messages might start looking annoying
> > with the extra information.  If so, such lines should be replaced with
> > the standard prints like dev_*().  I think almost all snd_printk()
> > like below are better replaced with standard ones.
> 
> below?  Was there something that was supposed to be below?
> Are you referring to the KERN_ERR/urb example above?

Sorry, scratch it.  Just a typo during changing the lines.

> > So, alternatively, we can begin with replacing some snd_printk() with
> > the standard functions, then dropping CONFIG_SND_VERBOSE_PRINTK.
> 
> Dropping CONFIG_SND_VERBOSE_PRINTK would simplify code a bit.
> 
> > I think most of snd_printd() and snd_printdd() can be kept as is.
> > These are just debug messages, after all.
> 
> Some of those are emitted at levels other than KERN_DEBUG.
> I think that odd.

Yeah, they aren't good, but it's a different topic.


Takashi

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Joe Perches <joe@perches.com>
Cc: Jaroslav Kysela <perex@perex.cz>,
	Alan Stern <stern@rowland.harvard.edu>,
	alsa-devel@alsa-project.org,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: [alsa-devel] Improving or replacing snd_printk()
Date: Fri, 31 May 2013 09:23:57 +0200	[thread overview]
Message-ID: <s5hip1zipde.wl%tiwai@suse.de> (raw)
In-Reply-To: <1369983967.10556.48.camel@joe-AO722>

At Fri, 31 May 2013 00:06:07 -0700,
Joe Perches wrote:
> 
> On Fri, 2013-05-31 at 08:37 +0200, Takashi Iwai wrote:
> > It's a problem only with snd_printk(), as CONFIG_SND_VERBOSE_PRINTK
> > influences on the behavior of snd_printk() and not on the debug prints
> > with snd_printd() & co.
> 
> Are you're perhaps confused about how CONFIG_SND_VERBOSE_PRINTK
> works with or varies between snd_printd and snd_printk?
> 
> #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, ...);
> 
> ...
> 
> #define snd_printk(fmt, args...) \
> 	__snd_printk(0, __FILE__, __LINE__, fmt, ##args)
> 
> #ifdef CONFIG_SND_DEBUG
> [...]
> #define snd_printd(fmt, args...) \
> 	__snd_printk(1, __FILE__, __LINE__, fmt, ##args)
> 
> I don't see a difference.

Erm, indeed.  I thought we enabled the verobse prinkt always for debug
prints.  But it's not.  It's more reason for dropping
CONFIG_SND_VERBOSE_PRINTK :)

> > Hence, the goal we should achieve is rather to drop
> > CONFIG_SND_VERBOSE_PRINTK.  *This* is the useless thing.
> 
> Maybe.
> 
> > Meanwhile, many snd_printk() messages might start looking annoying
> > with the extra information.  If so, such lines should be replaced with
> > the standard prints like dev_*().  I think almost all snd_printk()
> > like below are better replaced with standard ones.
> 
> below?  Was there something that was supposed to be below?
> Are you referring to the KERN_ERR/urb example above?

Sorry, scratch it.  Just a typo during changing the lines.

> > So, alternatively, we can begin with replacing some snd_printk() with
> > the standard functions, then dropping CONFIG_SND_VERBOSE_PRINTK.
> 
> Dropping CONFIG_SND_VERBOSE_PRINTK would simplify code a bit.
> 
> > I think most of snd_printd() and snd_printdd() can be kept as is.
> > These are just debug messages, after all.
> 
> Some of those are emitted at levels other than KERN_DEBUG.
> I think that odd.

Yeah, they aren't good, but it's a different topic.


Takashi

  reply	other threads:[~2013-05-31  7:23 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-29 19:27 Improving or replacing snd_printk() Alan Stern
2013-05-29 19:51 ` Joe Perches
2013-05-30 18:14   ` Alan Stern
2013-05-30 18:14     ` Alan Stern
2013-05-30 18:49     ` Jaroslav Kysela
2013-05-30 18:49       ` [alsa-devel] " Jaroslav Kysela
2013-05-30 19:43       ` Alan Stern
2013-05-30 19:43         ` Alan Stern
2013-05-30 20:30         ` Joe Perches
2013-05-31  6:37       ` Takashi Iwai
2013-05-31  7:06         ` Joe Perches
2013-05-31  7:23           ` Takashi Iwai [this message]
2013-05-31  7:23             ` Takashi Iwai
2013-05-31  7:30             ` Joe Perches
2013-05-31  7:30               ` [alsa-devel] " Joe Perches
2013-05-31  7:35               ` Takashi Iwai
2013-05-31  7:35                 ` [alsa-devel] " Takashi Iwai
2013-05-31  7:38                 ` Joe Perches
2013-05-31  7:58                   ` Takashi Iwai
2013-05-31  7:58                     ` [alsa-devel] " Takashi Iwai
2013-05-31 14:24                     ` Alan Stern
2013-05-31 14:24                       ` [alsa-devel] " Alan Stern
2013-05-31 14:34                       ` Takashi Iwai
2013-05-31 14:34                         ` [alsa-devel] " Takashi Iwai
2013-05-31 15:33                         ` Alan Stern
2013-05-31 15:33                           ` [alsa-devel] " Alan Stern
2013-05-31 15:38                           ` Takashi Iwai
2013-05-31 15:38                             ` Takashi Iwai
2013-05-31 17:06                         ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hip1zipde.wl%tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.