Re: [PATCH] rc-core: don't use dynamic_pr_debug for IR_dprintk()

[Mauro Carvalho Chehab <m.chehab@samsung.com>]

Em Sat, 26 Jul 2014 14:08:04 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> 
> 
> On 07/26/2014 05:25 AM, Mauro Carvalho Chehab wrote:
> > The hole point of IR_dprintk() is that, once a level is
> > given at debug parameter, all enabled IR parsers will show their
> > debug messages.
> >
> > While converting it to dynamic_printk might be a good idea,
> > right now it just makes very hard to debug the drivers, as
> > one needs to both pass debug=1 or debug=2 to rc-core and
> > to use the dynamic printk to enable all the desired lines.
> 
> Did you know you could enable debugs as whole module too? Also per 
> function or source file, not only per line you seems to use.
> 
> That is basic command to enable all debugs for module rc-core
> modprobe rc-core; echo -n 'module rc-core +p' > 
> /sys/kernel/debug/dynamic_debug/control
> 
> Look also other flags than '+p' from documentation
> Documentation/dynamic-debug-howto.txt

Antti,

Yes, I'm aware of that. I'm not against using pr_debug(),
but the patch that converted from printk to pr_debug()
made a crappy job, because it didn't replace all occurrences
of IR_dprintk() with pr_debug(). Instead, it did:

#define IR_dprintk(level, foo) if (level >= debug) pr_debug(foo)

So, if you do:
	module rc-core +p > .../dynamic_debug/control

it won't work, because debug is zero.

And, if you load rc-core with debug=1, it also doesn't work.
It is the worse of both worlds.

So, the hole point is that we should either remove IR_dprintk
macro everywhere or to remove pr_debug(). I took the shortest
way, as the hole point were to make the debug useful again, for me to
be able to test some of the IR patches I merged.

Btw, in the specific case of rc-core debug, the debug flag
is used on all RC raw decoders, and not only by the core.

I think that IR_dprintk is more intuitive than pr_debug in
this particular usecase, especially since it takes a longer
time to be able to enable the "level 1" type of messages on
all 9 IR decoders, and such "level 1" type of debug activation
can't easily be scriptable, as there's no way to do something like:

module rc-core ir* +p level=1 > .../dynamic_debug/control

Regards,
Mauro
> 
> 
> > That doesn't make sense!
> >
> > So, revert to the old way, as a single line is changed,
> > and the debug parameter will now work as expected.
> >
> > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > ---
> >   include/media/rc-core.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> > index 3047837db1cc..2c7fbca40b69 100644
> > --- a/include/media/rc-core.h
> > +++ b/include/media/rc-core.h
> > @@ -26,7 +26,7 @@ extern int rc_core_debug;
> >   #define IR_dprintk(level, fmt, ...)				\
> >   do {								\
> >   	if (rc_core_debug >= level)				\
> > -		pr_debug("%s: " fmt, __func__, ##__VA_ARGS__);	\
> > +		printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
> >   } while (0)
> >
> >   enum rc_driver_type {
> >
>