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;
snd_printk(KERN_INFO "found unsupported aci card\n");
pr_info("found unsupported aci card\n");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