[PATCH 0/4] Add debug messages to v4l2-ctrls

[PATCH 0/4] Add debug messages to v4l2-ctrls

[Ezequiel Garcia]

Even though the goal was fairly simple (adding debug to v4l2-ctrls)
it ended up spanning this little patchset.

The motivation for this cleanup is being able to turn on/off
debugging messages in v4l2-ctrls.c, using a knob common to the
videodev driver's core.

So, in addition to the dev_debug attribute, this series introduces
a debug module parameter, and a new debug level, which enables
v4l2-ctrl debugging.

Having this is quite useful when bringing-up stateless codecs,
using the Request API.

Ezequiel Garcia (4):
  media: v4l: Simplify dev_debug flags
  media: v4l: Improve debug dprintk macro
  media: v4l: Add a module parameter to control global debugging
  media: v4l: ctrls: Add debug messages

 drivers/media/v4l2-core/v4l2-ctrls.c | 63 +++++++++++++++++++++++-----
 drivers/media/v4l2-core/v4l2-dev.c   | 48 ++++++++++-----------
 include/media/v4l2-ioctl.h           |  2 +
 3 files changed, 78 insertions(+), 35 deletions(-)

-- 
2.20.1

[PATCH 1/4] media: v4l: Simplify dev_debug flags

[Ezequiel Garcia]

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(-)

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;
-- 
2.20.1

[PATCH 2/4] media: v4l: Improve debug dprintk macro

[Ezequiel Garcia]

Instead of checking the dev_debug flags before each dprintk call,
make the macro smarter by passing the parameters.

This makes the code simpler and will allow to add more debug logic.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/v4l2-core/v4l2-dev.c | 38 +++++++++++++-----------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 34e4958663bf..35e429ac888f 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -36,7 +36,8 @@
 #define VIDEO_NUM_DEVICES	256
 #define VIDEO_NAME              "video4linux"
 
-#define dprintk(fmt, arg...) do {					\
+#define dprintk(vdev, flags, fmt, arg...) do {				\
+	if (vdev->dev_debug & flags)					\
 		printk(KERN_DEBUG pr_fmt("%s: " fmt),			\
 		       __func__, ##arg);				\
 } while (0)
@@ -315,9 +316,8 @@ 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_STREAMING)
-		dprintk("%s: read: %zd (%d)\n",
-			video_device_node_name(vdev), sz, ret);
+	dprintk(vdev, V4L2_DEV_DEBUG_STREAMING, "%s: read: %zd (%d)\n",
+		video_device_node_name(vdev), sz, ret);
 	return ret;
 }
 
@@ -331,9 +331,8 @@ 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_STREAMING)
-		dprintk("%s: write: %zd (%d)\n",
-			video_device_node_name(vdev), sz, ret);
+	dprintk(vdev, V4L2_DEV_DEBUG_STREAMING, "%s: write: %zd (%d)\n",
+		video_device_node_name(vdev), sz, ret);
 	return ret;
 }
 
@@ -346,9 +345,8 @@ static __poll_t v4l2_poll(struct file *filp, struct poll_table_struct *poll)
 		return DEFAULT_POLLMASK;
 	if (video_is_registered(vdev))
 		res = vdev->fops->poll(filp, poll);
-	if (vdev->dev_debug & V4L2_DEV_DEBUG_POLL)
-		dprintk("%s: poll: %08x\n",
-			video_device_node_name(vdev), res);
+	dprintk(vdev, V4L2_DEV_DEBUG_POLL, "%s: poll: %08x\n",
+		video_device_node_name(vdev), res);
 	return res;
 }
 
@@ -381,9 +379,8 @@ static unsigned long v4l2_get_unmapped_area(struct file *filp,
 	if (!video_is_registered(vdev))
 		return -ENODEV;
 	ret = vdev->fops->get_unmapped_area(filp, addr, len, pgoff, flags);
-	if (vdev->dev_debug & V4L2_DEV_DEBUG_FOP)
-		dprintk("%s: get_unmapped_area (%d)\n",
-			video_device_node_name(vdev), ret);
+	dprintk(vdev, V4L2_DEV_DEBUG_FOP, "%s: get_unmapped_area (%d)\n",
+		video_device_node_name(vdev), ret);
 	return ret;
 }
 #endif
@@ -397,9 +394,8 @@ static int v4l2_mmap(struct file *filp, struct vm_area_struct *vm)
 		return -ENODEV;
 	if (video_is_registered(vdev))
 		ret = vdev->fops->mmap(filp, vm);
-	if (vdev->dev_debug & V4L2_DEV_DEBUG_FOP)
-		dprintk("%s: mmap (%d)\n",
-			video_device_node_name(vdev), ret);
+	dprintk(vdev, V4L2_DEV_DEBUG_FOP, "%s: mmap (%d)\n",
+		video_device_node_name(vdev), ret);
 	return ret;
 }
 
@@ -427,9 +423,8 @@ static int v4l2_open(struct inode *inode, struct file *filp)
 			ret = -ENODEV;
 	}
 
-	if (vdev->dev_debug & V4L2_DEV_DEBUG_FOP)
-		dprintk("%s: open (%d)\n",
-			video_device_node_name(vdev), ret);
+	dprintk(vdev, V4L2_DEV_DEBUG_FOP, "%s: open (%d)\n",
+		video_device_node_name(vdev), ret);
 	/* decrease the refcount in case of an error */
 	if (ret)
 		video_put(vdev);
@@ -458,9 +453,8 @@ static int v4l2_release(struct inode *inode, struct file *filp)
 		}
 	}
 
-	if (vdev->dev_debug & V4L2_DEV_DEBUG_FOP)
-		dprintk("%s: release\n",
-			video_device_node_name(vdev));
+	dprintk(vdev, V4L2_DEV_DEBUG_FOP, "%s: release\n",
+		video_device_node_name(vdev));
 
 	/* decrease the refcount unconditionally since the release()
 	   return value is ignored. */
-- 
2.20.1

[PATCH 3/4] media: v4l: Add a module parameter to control global debugging

[Ezequiel Garcia]

In addition to the dev_debug device attribute, which controls
per-device debugging, we now add a module parameter to control
debugging globally.

This will allow to add debugging of v4l2 control logic,
using the newly introduced debug parameter.

In addition, this module parameter adds consistency to the
subsystem, since other v4l2 modules expose the same parameter.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/v4l2-core/v4l2-dev.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 35e429ac888f..4f7821b13721 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -36,8 +36,16 @@
 #define VIDEO_NUM_DEVICES	256
 #define VIDEO_NAME              "video4linux"
 
+unsigned int videodev_debug;
+module_param_named(debug, videodev_debug, uint, 0644);
+
+/*
+ * The videodev_debug module parameter controls the global debug level,
+ * while the dev_debug device attribute controls the local
+ * per-device debug level.
+ */
 #define dprintk(vdev, flags, fmt, arg...) do {				\
-	if (vdev->dev_debug & flags)					\
+	if ((videodev_debug & flags) || (vdev->dev_debug & flags))		\
 		printk(KERN_DEBUG pr_fmt("%s: " fmt),			\
 		       __func__, ##arg);				\
 } while (0)
-- 
2.20.1

[PATCH 4/4] media: v4l: ctrls: Add debug messages

[Ezequiel Garcia]

Currently, the v4l2 control code is a bit silent on errors.
To ease debugging of the control logic, add debug messages
on (hopefully) most of the error paths.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 63 +++++++++++++++++++++++-----
 include/media/v4l2-ioctl.h           |  2 +
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 99308dac2daa..c9f4e00f2229 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -18,6 +18,8 @@
     Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#define pr_fmt(fmt) "v4l2-ctrls: " fmt
+
 #include <linux/ctype.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
@@ -28,6 +30,14 @@
 #include <media/v4l2-event.h>
 #include <media/v4l2-dev.h>
 
+extern unsigned int videodev_debug;
+
+#define dprintk(fmt, arg...) do {					\
+	if (videodev_debug & V4L2_DEV_DEBUG_CTRL)			\
+		printk(KERN_DEBUG pr_fmt("%s: " fmt),			\
+		       __func__, ##arg);				\
+} while (0)
+
 #define has_op(master, op) \
 	(master->ops && master->ops->op)
 #define call_op(master, op) \
@@ -1952,8 +1962,11 @@ static int validate_new(const struct v4l2_ctrl *ctrl, union v4l2_ctrl_ptr p_new)
 	unsigned idx;
 	int err = 0;
 
-	for (idx = 0; !err && idx < ctrl->elems; idx++)
+	for (idx = 0; !err && idx < ctrl->elems; idx++) {
 		err = ctrl->type_ops->validate(ctrl, idx, p_new);
+		if (err)
+			dprintk("failed to validate control id 0x%x (%d)\n", ctrl->id, err);
+	}
 	return err;
 }
 
@@ -3136,20 +3149,28 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
 		if (cs->which &&
 		    cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
 		    cs->which != V4L2_CTRL_WHICH_REQUEST_VAL &&
-		    V4L2_CTRL_ID2WHICH(id) != cs->which)
+		    V4L2_CTRL_ID2WHICH(id) != cs->which) {
+			dprintk("invalid which 0x%x or control id 0x%x\n", cs->which, id);
 			return -EINVAL;
+		}
 
 		/* Old-style private controls are not allowed for
 		   extended controls */
-		if (id >= V4L2_CID_PRIVATE_BASE)
+		if (id >= V4L2_CID_PRIVATE_BASE) {
+			dprintk("old-style private controls not allowed for extended controls\n");
 			return -EINVAL;
+		}
 		ref = find_ref_lock(hdl, id);
-		if (ref == NULL)
+		if (ref == NULL) {
+			dprintk("cannot find control id 0x%x\n", id);
 			return -EINVAL;
+		}
 		h->ref = ref;
 		ctrl = ref->ctrl;
-		if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED)
+		if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED) {
+			dprintk("control id 0x%x is disabled\n", id);
 			return -EINVAL;
+		}
 
 		if (ctrl->cluster[0]->ncontrols > 1)
 			have_clusters = true;
@@ -3159,10 +3180,16 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
 			unsigned tot_size = ctrl->elems * ctrl->elem_size;
 
 			if (c->size < tot_size) {
+				/*
+				 * In the get case the application first queries
+				 * to obtain the size of the control.
+				 */
 				if (get) {
 					c->size = tot_size;
 					return -ENOSPC;
 				}
+				dprintk("pointer control id 0x%x size too small, %d bytes but %d bytes needed\n",
+					id, c->size, tot_size);
 				return -EFAULT;
 			}
 			c->size = tot_size;
@@ -3534,16 +3561,20 @@ static int validate_ctrls(struct v4l2_ext_controls *cs,
 
 		cs->error_idx = i;
 
-		if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
+		if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY) {
+			dprintk("control id 0x%x is read-only\n", ctrl->id);
 			return -EACCES;
+		}
 		/* This test is also done in try_set_control_cluster() which
 		   is called in atomic context, so that has the final say,
 		   but it makes sense to do an up-front check as well. Once
 		   an error occurs in try_set_control_cluster() some other
 		   controls may have been set already and we want to do a
 		   best-effort to avoid that. */
-		if (set && (ctrl->flags & V4L2_CTRL_FLAG_GRABBED))
+		if (set && (ctrl->flags & V4L2_CTRL_FLAG_GRABBED)) {
+			dprintk("control id 0x%x is grabbed, cannot set\n", ctrl->id);
 			return -EBUSY;
+		}
 		/*
 		 * Skip validation for now if the payload needs to be copied
 		 * from userspace into kernelspace. We'll validate those later.
@@ -3588,13 +3619,17 @@ static int try_set_ext_ctrls_common(struct v4l2_fh *fh,
 	cs->error_idx = cs->count;
 
 	/* Default value cannot be changed */
-	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL)
+	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL) {
+		dprintk("cannot change default value\n");
 		return -EINVAL;
+	}
 
 	cs->which = V4L2_CTRL_ID2WHICH(cs->which);
 
-	if (hdl == NULL)
+	if (hdl == NULL) {
+		dprintk("invalid null control handler\n");
 		return -EINVAL;
+	}
 
 	if (cs->count == 0)
 		return class_check(hdl, cs->which);
@@ -3700,21 +3735,27 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh,
 	int ret;
 
 	if (cs->which == V4L2_CTRL_WHICH_REQUEST_VAL) {
-		if (!mdev || cs->request_fd < 0)
+		if (!mdev || cs->request_fd < 0) {
+			dprintk("missing media device or invalid request fd\n");
 			return -EINVAL;
+		}
 
 		req = media_request_get_by_fd(mdev, cs->request_fd);
-		if (IS_ERR(req))
+		if (IS_ERR(req)) {
+			dprintk("cannot find request fd %d\n", cs->request_fd);
 			return PTR_ERR(req);
+		}
 
 		ret = media_request_lock_for_update(req);
 		if (ret) {
+			dprintk("cannot lock request fd %d\n", cs->request_fd);
 			media_request_put(req);
 			return ret;
 		}
 
 		obj = v4l2_ctrls_find_req_obj(hdl, req, set);
 		if (IS_ERR(obj)) {
+			dprintk("cannot find request object for request fd %d\n", cs->request_fd);
 			media_request_unlock_for_update(req);
 			media_request_put(req);
 			return PTR_ERR(obj);
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 8533ece5026e..0ecd4e3e76a4 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -612,6 +612,8 @@ struct v4l2_ioctl_ops {
 #define V4L2_DEV_DEBUG_STREAMING	0x08
 /* Log poll() */
 #define V4L2_DEV_DEBUG_POLL		0x10
+/* Log controls */
+#define V4L2_DEV_DEBUG_CTRL		0x20
 
 /*  Video standard functions  */
 
-- 
2.20.1

Re: [PATCH 4/4] media: v4l: ctrls: Add debug messages

[Ezequiel Garcia]

On Mon, 2019-02-18 at 17:15 -0300, Ezequiel Garcia wrote:
> Currently, the v4l2 control code is a bit silent on errors.
> To ease debugging of the control logic, add debug messages
> on (hopefully) most of the error paths.
> 

I'm thinking some of these could receive a struct video_device,
and so use it to print the device node name.

I'll explore that and try to get a v2.

Thanks,
Eze

> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 63 +++++++++++++++++++++++-----
>  include/media/v4l2-ioctl.h           |  2 +
>  2 files changed, 54 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 99308dac2daa..c9f4e00f2229 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -18,6 +18,8 @@
>      Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> +#define pr_fmt(fmt) "v4l2-ctrls: " fmt
> +
>  #include <linux/ctype.h>
>  #include <linux/mm.h>
>  #include <linux/slab.h>
> @@ -28,6 +30,14 @@
>  #include <media/v4l2-event.h>
>  #include <media/v4l2-dev.h>
>  
> +extern unsigned int videodev_debug;
> +
> +#define dprintk(fmt, arg...) do {					\
> +	if (videodev_debug & V4L2_DEV_DEBUG_CTRL)			\
> +		printk(KERN_DEBUG pr_fmt("%s: " fmt),			\
> +		       __func__, ##arg);				\
> +} while (0)
> +
>  #define has_op(master, op) \
>  	(master->ops && master->ops->op)
>  #define call_op(master, op) \
> @@ -1952,8 +1962,11 @@ static int validate_new(const struct v4l2_ctrl *ctrl, union v4l2_ctrl_ptr p_new)
>  	unsigned idx;
>  	int err = 0;
>  
> -	for (idx = 0; !err && idx < ctrl->elems; idx++)
> +	for (idx = 0; !err && idx < ctrl->elems; idx++) {
>  		err = ctrl->type_ops->validate(ctrl, idx, p_new);
> +		if (err)
> +			dprintk("failed to validate control id 0x%x (%d)\n", ctrl->id, err);
> +	}
>  	return err;
>  }
>  
> @@ -3136,20 +3149,28 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
>  		if (cs->which &&
>  		    cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
>  		    cs->which != V4L2_CTRL_WHICH_REQUEST_VAL &&
> -		    V4L2_CTRL_ID2WHICH(id) != cs->which)
> +		    V4L2_CTRL_ID2WHICH(id) != cs->which) {
> +			dprintk("invalid which 0x%x or control id 0x%x\n", cs->which, id);
>  			return -EINVAL;
> +		}
>  
>  		/* Old-style private controls are not allowed for
>  		   extended controls */
> -		if (id >= V4L2_CID_PRIVATE_BASE)
> +		if (id >= V4L2_CID_PRIVATE_BASE) {
> +			dprintk("old-style private controls not allowed for extended controls\n");
>  			return -EINVAL;
> +		}
>  		ref = find_ref_lock(hdl, id);
> -		if (ref == NULL)
> +		if (ref == NULL) {
> +			dprintk("cannot find control id 0x%x\n", id);
>  			return -EINVAL;
> +		}
>  		h->ref = ref;
>  		ctrl = ref->ctrl;
> -		if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED)
> +		if (ctrl->flags & V4L2_CTRL_FLAG_DISABLED) {
> +			dprintk("control id 0x%x is disabled\n", id);
>  			return -EINVAL;
> +		}
>  
>  		if (ctrl->cluster[0]->ncontrols > 1)
>  			have_clusters = true;
> @@ -3159,10 +3180,16 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
>  			unsigned tot_size = ctrl->elems * ctrl->elem_size;
>  
>  			if (c->size < tot_size) {
> +				/*
> +				 * In the get case the application first queries
> +				 * to obtain the size of the control.
> +				 */
>  				if (get) {
>  					c->size = tot_size;
>  					return -ENOSPC;
>  				}
> +				dprintk("pointer control id 0x%x size too small, %d bytes but %d bytes needed\n",
> +					id, c->size, tot_size);
>  				return -EFAULT;
>  			}
>  			c->size = tot_size;
> @@ -3534,16 +3561,20 @@ static int validate_ctrls(struct v4l2_ext_controls *cs,
>  
>  		cs->error_idx = i;
>  
> -		if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
> +		if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY) {
> +			dprintk("control id 0x%x is read-only\n", ctrl->id);
>  			return -EACCES;
> +		}
>  		/* This test is also done in try_set_control_cluster() which
>  		   is called in atomic context, so that has the final say,
>  		   but it makes sense to do an up-front check as well. Once
>  		   an error occurs in try_set_control_cluster() some other
>  		   controls may have been set already and we want to do a
>  		   best-effort to avoid that. */
> -		if (set && (ctrl->flags & V4L2_CTRL_FLAG_GRABBED))
> +		if (set && (ctrl->flags & V4L2_CTRL_FLAG_GRABBED)) {
> +			dprintk("control id 0x%x is grabbed, cannot set\n", ctrl->id);
>  			return -EBUSY;
> +		}
>  		/*
>  		 * Skip validation for now if the payload needs to be copied
>  		 * from userspace into kernelspace. We'll validate those later.
> @@ -3588,13 +3619,17 @@ static int try_set_ext_ctrls_common(struct v4l2_fh *fh,
>  	cs->error_idx = cs->count;
>  
>  	/* Default value cannot be changed */
> -	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL)
> +	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL) {
> +		dprintk("cannot change default value\n");
>  		return -EINVAL;
> +	}
>  
>  	cs->which = V4L2_CTRL_ID2WHICH(cs->which);
>  
> -	if (hdl == NULL)
> +	if (hdl == NULL) {
> +		dprintk("invalid null control handler\n");
>  		return -EINVAL;
> +	}
>  
>  	if (cs->count == 0)
>  		return class_check(hdl, cs->which);
> @@ -3700,21 +3735,27 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh,
>  	int ret;
>  
>  	if (cs->which == V4L2_CTRL_WHICH_REQUEST_VAL) {
> -		if (!mdev || cs->request_fd < 0)
> +		if (!mdev || cs->request_fd < 0) {
> +			dprintk("missing media device or invalid request fd\n");
>  			return -EINVAL;
> +		}
>  
>  		req = media_request_get_by_fd(mdev, cs->request_fd);
> -		if (IS_ERR(req))
> +		if (IS_ERR(req)) {
> +			dprintk("cannot find request fd %d\n", cs->request_fd);
>  			return PTR_ERR(req);
> +		}
>  
>  		ret = media_request_lock_for_update(req);
>  		if (ret) {
> +			dprintk("cannot lock request fd %d\n", cs->request_fd);
>  			media_request_put(req);
>  			return ret;
>  		}
>  
>  		obj = v4l2_ctrls_find_req_obj(hdl, req, set);
>  		if (IS_ERR(obj)) {
> +			dprintk("cannot find request object for request fd %d\n", cs->request_fd);
>  			media_request_unlock_for_update(req);
>  			media_request_put(req);
>  			return PTR_ERR(obj);
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 8533ece5026e..0ecd4e3e76a4 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -612,6 +612,8 @@ struct v4l2_ioctl_ops {
>  #define V4L2_DEV_DEBUG_STREAMING	0x08
>  /* Log poll() */
>  #define V4L2_DEV_DEBUG_POLL		0x10
> +/* Log controls */
> +#define V4L2_DEV_DEBUG_CTRL		0x20
>  
>  /*  Video standard functions  */
>