Re: [PATCH 06/14] [media] cx18: Use current logging styles

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2011-08-24 at 14:42 -0700, Joe Perches wrote:
> On Wed, 2011-08-24 at 06:34 -0400, Andy Walls wrote:
> > On Sun, 2011-08-21 at 15:56 -0700, Joe Perches wrote:
> > > Add pr_fmt.
> > > Convert printks to pr_<level>.
> > > Convert printks without KERN_<level> to appropriate pr_<level>.
> > > Removed embedded prefixes when pr_fmt was added.
> > > Use ##__VA_ARGS__ for variadic macros.
> > > Coalesce format strings.
> > 1. It is important to preserve the per-card prefixes emitted by the
> > driver: cx18-0, cx18-1, cx18-2, etc.  With a quick skim, I think your
> > change preserves the format of all output messages (except removing
> > periods).  Can you confirm this?
> 
> Here's the output diff of
> strings built-in.o | grep "^<.>" | sort
> new and old
> $ diff -u0 cx18.old cx18.new
> --- cx18.old	2011-08-24 13:18:41.000000000 -0700
> +++ cx18.new	2011-08-24 14:04:10.000000000 -0700
> @@ -1,2 +1,9 @@
> -<3>cx18-alsa cx is NULL
> -<3>cx18-alsa: %s: struct v4l2_device * is NULL
> +<3>cx18_alsa: cx is NULL
> +<3>cx18_alsa: %s-alsa: %s: failed to create struct snd_cx18_card
> +<3>cx18_alsa: %s-alsa: %s: snd_card_create() failed with err %d
> +<3>cx18_alsa: %s-alsa: %s: snd_card_register() failed with err %d
> +<3>cx18_alsa: %s-alsa: %s: snd_cx18_card_create() failed with err %d
> +<3>cx18_alsa: %s-alsa: %s: snd_cx18_pcm_create() failed with err %d
> +<3>cx18_alsa: %s-alsa: %s: snd_cx18_pcm_create() failed with err %d
> +<3>cx18_alsa: %s-alsa: %s: struct snd_cx18_card * already exists
> +<3>cx18_alsa: %s: struct v4l2_device * is NULL
> @@ -17,7 +23,0 @@
> -<3>%s-alsa: %s: failed to create struct snd_cx18_card
> -<3>%s-alsa: %s: snd_card_create() failed with err %d
> -<3>%s-alsa: %s: snd_card_register() failed with err %d
> -<3>%s-alsa: %s: snd_cx18_card_create() failed with err %d
> -<3>%s-alsa: %s: snd_cx18_pcm_create() failed with err %d
> -<3>%s-alsa: %s: snd_cx18_pcm_create() failed with err %d
> -<3>%s-alsa: %s: struct snd_cx18_card * already exists

Yuck.

> @@ -62 +62 @@
> -<3>%s: Prefix your subject line with [UNKNOWN CX18 CARD].
> +<3>%s: Prefix your subject line with [UNKNOWN CX18 CARD]

> @@ -80 +80 @@
> -<4>%s-alsa: %s: struct snd_cx18_card * is NULL
> +<4>cx18_alsa: %s-alsa: %s: struct snd_cx18_card * is NULL

Yuck.

> @@ -82 +82 @@
> -<4>%s: Could not register GPIO reset controllersubdevice; proceeding anyway.
> +<4>%s: Could not register GPIO reset controller subdevice; proceeding anyway.
> @@ -85 +85 @@
> -<4>%s: MPEG Index stream cannot be claimed directly, but something tried.
> +<4>%s: MPEG Index stream cannot be claimed directly, but something tried
> @@ -99,12 +99,14 @@
> -<6>cx18-alsa: module loading...
> -<6>cx18-alsa: module unload complete
> -<6>cx18-alsa: module unloading...
> -<6>cx18-alsa-pcm %s: Allocating vbuffer
> -<6>cx18-alsa-pcm %s: cx18 alsa announce ptr=%p data=%p num_bytes=%zd
> -<6>cx18-alsa-pcm %s: dma area was NULL - ignoring
> -<6>cx18-alsa-pcm %s: freeing pcm capture region
> -<6>cx18-alsa-pcm %s: runtime was NULL
> -<6>cx18-alsa-pcm %s: %s called
> -<6>cx18-alsa-pcm %s: %s: length was zero
> -<6>cx18-alsa-pcm %s: stride is zero
> -<6>cx18-alsa-pcm %s: substream was NULL
> +<6>cx18_alsa: module loading...
> +<6>cx18_alsa: module unload complete
> +<6>cx18_alsa: module unloading...
> +<6>cx18_alsa: %s: Allocating vbuffer
> +<6>cx18_alsa: %s: created cx18 ALSA interface instance 
> +<6>cx18_alsa: %s: cx18 alsa announce ptr=%p data=%p num_bytes=%zd
> +<6>cx18_alsa: %s: dma area was NULL - ignoring
> +<6>cx18_alsa: %s: freeing pcm capture region
> +<6>cx18_alsa: %s: PCM stream for card is disabled - skipping
> +<6>cx18_alsa: %s: runtime was NULL
> +<6>cx18_alsa: %s: %s called
> +<6>cx18_alsa: %s: %s: length was zero
> +<6>cx18_alsa: %s: stride is zero
> +<6>cx18_alsa: %s: substream was NULL
> @@ -172 +174 @@
> -<6>%s:  info: dualwatch: change stereo flag from 0x%x to 0x%x.
> +<6>%s:  info: dualwatch: change stereo flag from 0x%x to 0x%x
> @@ -188 +190 @@
> -<6>%s:  info: Preparing for firmware halt.
> +<6>%s:  info: Preparing for firmware halt
> @@ -206 +208 @@
> -<6>%s:  info: Switching standard to %llx.
> +<6>%s:  info: Switching standard to %llx
> @@ -236 +237,0 @@
> -<6>%s: %s: created cx18 ALSA interface instance 
> @@ -239 +239,0 @@
> -<6>%s: %s: PCM stream for card is disabled - skipping



> > 2. PLease don't add a pr_fmt() #define to exevry file.  Just put one
> > where all the other CX18_*() macros are defined.  Every file picks those
> > up.
> 
> It's not the first #include of every file.
> printk.h has a default #define pr_fmt(fmt) fmt
> 

Well then don't use "pr_fmt(fmt)" in cx18, if it overloads a define
somewhere else in the kernel and has a dependency on its order relative
to #include statements.  That sort of thing just ups maintenance hours
later.  That's not a good trade off for subjectively better log
messages.

Won't redifining the 'pr_fmt(fmt)' generate preprocessor warnings
anyway?


NACK.

Regards,
Andy 




--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux