On Mon June 18 2012 12:01:47 Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> On Sunday 10 June 2012 12:25:51 Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >
> > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> > ---
> > drivers/media/video/v4l2-dev.c | 41 ++++++++++++++++++++++++++-----------
> > 1 file changed, 29 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
> > index 5c0bb18..54f387d 100644
> > --- a/drivers/media/video/v4l2-dev.c
> > +++ b/drivers/media/video/v4l2-dev.c
> > @@ -305,6 +305,9 @@ static ssize_t v4l2_read(struct file *filp, char __user
> > *buf, ret = vdev->fops->read(filp, buf, sz, off);
> > if (test_bit(V4L2_FL_LOCK_ALL_FOPS, &vdev->flags))
> > mutex_unlock(vdev->lock);
> > + if (vdev->debug)
>
> As vdev->debug is a bitmask, shouldn't we add an fops debug bit ?
I actually want to move away from the bitmask idea. I've never really liked
it here.
>
> > + pr_info("%s: read: %zd (%d)\n",
> > + video_device_node_name(vdev), sz, ret);
>
> Shouldn't we use KERN_DEBUG instead of KERN_INFO ? BTW, what about replacing
> the pr_* calls with dev_* calls ?
KERN_DEBUG vs KERN_INFO is actually a good question. My reasoning is that you
explicitly enable logging, and so you really want to see it in the log, so we
use KERN_INFO. With KERN_DEBUG you might have the situation where the debug
level of the logging is disabled, so the messages are ignored.
However, if people disagree with this, then I'm happy to move it back to
KERN_DEBUG.
With regards to dev_ vs pr_: I'd have to test this to see what dev_ prints as
prefix.
Regards,
Hans
--
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
