| Message ID | 20190227170706.6258-2-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add debug messages to v4l2-ctrls | expand |
On 2/27/19 6:07 PM, Ezequiel Garcia wrote: > In preparation to cleanup the debug logic, simplify the dev_debug > usage. In particular, make sure that a single flag is used to > control each debug print. > > Before this commit V4L2_DEV_DEBUG_STREAMING and V4L2_DEV_DEBUG_FOP > were needed to enable read and write debugging. After this commit > only the former is needed. The original idea was that ioctls are logged with V4L2_DEV_DEBUG_IOCTL and file ops with V4L2_DEV_DEBUG_FOP. And to see the streaming ioctls or fops you would have to add V4L2_DEV_DEBUG_STREAMING in addition to DEBUG_IOCTL/FOP. This patch changes the behavior in that the streaming fops are now solely controlled by V4L2_DEV_DEBUG_STREAMING. I do agree with this change, but this requires that the same change is done for the streaming ioctls (DQBUF/QBUF) and that the documentation in Documentation/media/kapi/v4l2-dev.rst is updated (section "video device debugging"). Of course, the documentation should also mention the new dev_debug module parameter and the new debug flag for debugging controls. Regards, Hans > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > drivers/media/v4l2-core/v4l2-dev.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > index d7528f82a66a..34e4958663bf 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -315,8 +315,7 @@ static ssize_t v4l2_read(struct file *filp, char __user *buf, > return -EINVAL; > if (video_is_registered(vdev)) > ret = vdev->fops->read(filp, buf, sz, off); > - if ((vdev->dev_debug & V4L2_DEV_DEBUG_FOP) && > - (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING)) > + if (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING) > dprintk("%s: read: %zd (%d)\n", > video_device_node_name(vdev), sz, ret); > return ret; > @@ -332,8 +331,7 @@ static ssize_t v4l2_write(struct file *filp, const char __user *buf, > return -EINVAL; > if (video_is_registered(vdev)) > ret = vdev->fops->write(filp, buf, sz, off); > - if ((vdev->dev_debug & V4L2_DEV_DEBUG_FOP) && > - (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING)) > + if (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING) > dprintk("%s: write: %zd (%d)\n", > video_device_node_name(vdev), sz, ret); > return ret; >
Hi Hans, I'm sorry for the delay. This series still sounds useful, so let's try to revive it. On Mon, 2019-03-11 at 12:16 +0100, Hans Verkuil wrote: > On 2/27/19 6:07 PM, Ezequiel Garcia wrote: > > In preparation to cleanup the debug logic, simplify the dev_debug > > usage. In particular, make sure that a single flag is used to > > control each debug print. > > > > Before this commit V4L2_DEV_DEBUG_STREAMING and V4L2_DEV_DEBUG_FOP > > were needed to enable read and write debugging. After this commit > > only the former is needed. > > The original idea was that ioctls are logged with V4L2_DEV_DEBUG_IOCTL > and file ops with V4L2_DEV_DEBUG_FOP. And to see the streaming ioctls > or fops you would have to add V4L2_DEV_DEBUG_STREAMING in addition to > DEBUG_IOCTL/FOP. > > This patch changes the behavior in that the streaming fops are now > solely controlled by V4L2_DEV_DEBUG_STREAMING. > > I do agree with this change, but this requires that the same change is > done for the streaming ioctls (DQBUF/QBUF) and that the documentation in > Documentation/media/kapi/v4l2-dev.rst is updated (section "video device > debugging"). > Oops, I managed to somehow miss (D)QBUF, even though it's perfectly well documented in the header! Will fix, and will fix the documentation as well. > Of course, the documentation should also mention the new dev_debug > module parameter and the new debug flag for debugging controls. > > Regards, > > Hans > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > > --- > > drivers/media/v4l2-core/v4l2-dev.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > > index d7528f82a66a..34e4958663bf 100644 > > --- a/drivers/media/v4l2-core/v4l2-dev.c > > +++ b/drivers/media/v4l2-core/v4l2-dev.c > > @@ -315,8 +315,7 @@ static ssize_t v4l2_read(struct file *filp, char __user *buf, > > return -EINVAL; > > if (video_is_registered(vdev)) > > ret = vdev->fops->read(filp, buf, sz, off); > > - if ((vdev->dev_debug & V4L2_DEV_DEBUG_FOP) && > > - (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING)) > > + if (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING) > > dprintk("%s: read: %zd (%d)\n", > > video_device_node_name(vdev), sz, ret); > > return ret; > > @@ -332,8 +331,7 @@ static ssize_t v4l2_write(struct file *filp, const char __user *buf, > > return -EINVAL; > > if (video_is_registered(vdev)) > > ret = vdev->fops->write(filp, buf, sz, off); > > - if ((vdev->dev_debug & V4L2_DEV_DEBUG_FOP) && > > - (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING)) > > + if (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING) > > dprintk("%s: write: %zd (%d)\n", > > video_device_node_name(vdev), sz, ret); > > return ret; > >
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index d7528f82a66a..34e4958663bf 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -315,8 +315,7 @@ static ssize_t v4l2_read(struct file *filp, char __user *buf, return -EINVAL; if (video_is_registered(vdev)) ret = vdev->fops->read(filp, buf, sz, off); - if ((vdev->dev_debug & V4L2_DEV_DEBUG_FOP) && - (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING)) + if (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING) dprintk("%s: read: %zd (%d)\n", video_device_node_name(vdev), sz, ret); return ret; @@ -332,8 +331,7 @@ static ssize_t v4l2_write(struct file *filp, const char __user *buf, return -EINVAL; if (video_is_registered(vdev)) ret = vdev->fops->write(filp, buf, sz, off); - if ((vdev->dev_debug & V4L2_DEV_DEBUG_FOP) && - (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING)) + if (vdev->dev_debug & V4L2_DEV_DEBUG_STREAMING) dprintk("%s: write: %zd (%d)\n", video_device_node_name(vdev), sz, ret); return ret;
In preparation to cleanup the debug logic, simplify the dev_debug usage. In particular, make sure that a single flag is used to control each debug print. Before this commit V4L2_DEV_DEBUG_STREAMING and V4L2_DEV_DEBUG_FOP were needed to enable read and write debugging. After this commit only the former is needed. Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> --- drivers/media/v4l2-core/v4l2-dev.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)