On Fri March 15 2013 13:25:21 Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> On Friday 15 March 2013 11:27:25 Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >
> > The core debug code can now be simplified since all the write-only ioctls
> > are now const and will not modify the data they pass to the drivers.
> >
> > So instead of logging write-only ioctls before the driver is called this can
> > now be done afterwards, which is cleaner when it comes to error reporting
> > as well.
> >
> > This also fixes a logic error in the debugging code where there was one
> > 'else' too many.
> >
> > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> > ---
> > drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++-------------
> > 1 file changed, 2 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c index 2abd13a..b3fe148 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -2147,11 +2147,6 @@ static long __video_do_ioctl(struct file *file,
> > }
> >
> > write_only = _IOC_DIR(cmd) == _IOC_WRITE;
> > - if (write_only && debug > V4L2_DEBUG_IOCTL) {
> > - v4l_printk_ioctl(video_device_node_name(vfd), cmd);
> > - pr_cont(": ");
> > - info->debug(arg, write_only);
> > - }
> > if (info->flags & INFO_FL_STD) {
> > typedef int (*vidioc_op)(struct file *file, void *fh, void *p);
> > const void *p = vfd->ioctl_ops;
> > @@ -2170,16 +2165,10 @@ static long __video_do_ioctl(struct file *file,
> >
> > done:
> > if (debug) {
> > - if (write_only && debug > V4L2_DEBUG_IOCTL) {
> > - if (ret < 0)
> > - printk(KERN_DEBUG "%s: error %ld\n",
> > - video_device_node_name(vfd), ret);
> > - return ret;
> > - }
> > v4l_printk_ioctl(video_device_node_name(vfd), cmd);
> > if (ret < 0)
> > - pr_cont(": error %ld\n", ret);
> > - else if (debug == V4L2_DEBUG_IOCTL)
> > + pr_cont(": error %ld", ret);
> > + if (debug == V4L2_DEBUG_IOCTL)
>
> Shouldn't this be >= V4L2_DEBUG_IOCTL ?
No. V4L2_DEBUG_IOCTL is the lowest debug level and should just print the ioctl
and return code. So the else parts are for higher debug levels where all args
are also printed.
Regards,
Hans
>
> > pr_cont("\n");
> > else if (_IOC_DIR(cmd) == _IOC_NONE)
> > info->debug(arg, write_only);
>
>
--
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
