Hi Mauro,
On Thu, Apr 05, 2018 at 08:52:02AM -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 5 Apr 2018 14:12:10 +0300
> Sakari Ailus <sakari.ailus@xxxxxx> escreveu:
>
> > Hi Mauro,
> >
> > On Thu, Apr 05, 2018 at 07:34:39AM -0300, Mauro Carvalho Chehab wrote:
> > > Instead of using printk() directly, use the pr_foo()
> > > macros.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/media/v4l2-core/v4l2-dev.c | 45 ++++++++++++++++++++++----------------
> > > 1 file changed, 26 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> > > index 1d0b2208e8fb..530db8e482fb 100644
> > > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > > @@ -16,6 +16,8 @@
> > > * - Added procfs support
> > > */
> > >
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > #include <linux/module.h>
> > > #include <linux/types.h>
> > > #include <linux/kernel.h>
> > > @@ -34,6 +36,12 @@
> > > #define VIDEO_NUM_DEVICES 256
> > > #define VIDEO_NAME "video4linux"
> > >
> > > +#define dprintk(fmt, arg...) do { \
> > > + printk(KERN_DEBUG pr_fmt("%s: " fmt), \
> > > + __func__, ##arg); \
> > > +} while (0)
> >
> > Any particular reason for introducing a new macro rather than using
> > pr_debug()? I know it's adding the name of the function without requiring
> > to explicitly add that below, but pr_debug("%s: ...", __func__); would be
> > easier to read IMO.
>
> Yes, there is. Nowadays, most systems are built with CONFIG_DYNAMIC_DEBUG,
> as it is default on most distros.
>
> It means that, in order to enable a debug message, one has to
> explicitly enable the debug messages via /sys/kernel/debug/dynamic_debug.
>
> In the case of videodev core, the debug messages are enabled, instead,
> via vdev->dev_debug. It is really messy to have both mechanisms at the
> same time to enable a debug message. We need to use either one or the
> other.
>
> Also, a change from vdev->dev_debug approach to dynamic_debug approach
> is something that should happen at the entire subsystem.
>
> Even if this is a good idea (I'm not convinced), this should be
> done on a separate patch series.
Fair enough. Please add:
Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
