Skip to contentSkip to conversation

media: v4l: ctrls: Add debug messages

PATCH In review Ezequiel Garcia 2 participants 2 messages Jul 20, 2019 – Jul 22, 2019

Subsystems (2)
Documentation Media (V4L/DVB)
lore
Share & tools
lore

Ezequiel Garcia

author
#0 lore raw

PATCH v5 1/1 media: v4l: ctrls: Add debug messages

Currently, the v4l2 control code is a bit silent on errors. Add debug messages on (hopefully) most of the error paths.

Signed-off-by Ezequiel Garcia <[email protected]>

---
Changes from v3:
* Add missing parenthesis around macro argument.

Changes from v3:
* Fix checkpatch.pl warnings about long lines.

Changes from v2:
* Check and noisy-warn if vdev is NULL.

Changes from v1:
* Drop changes in the debug parameter semantics.
* Drop new module debug parameter.
* Add documentation.
* Add a debug error in all places where control can fail.
* Reorder the vdev parameter, to make the patch less invasive.

---
 Documentation/media/kapi/v4l2-dev.rst      |   1 +
 drivers/media/platform/omap3isp/ispvideo.c |   4 +-
 drivers/media/v4l2-core/v4l2-ctrls.c       | 126 ++++++++++++++++-----
 drivers/media/v4l2-core/v4l2-ioctl.c       |  18 ++-
 drivers/media/v4l2-core/v4l2-subdev.c      |   6 +-
 include/media/v4l2-ctrls.h                 |   9 +-
 include/media/v4l2-ioctl.h                 |   2 +
 7 files changed, 127 insertions(+), 39 deletions(-)
Documentation/media/kapi/v4l2-dev.rst +1 -0
index b359f1804bbe..4c5a15c53dbf 100644
@@ -288,6 +288,7 @@ Mask  Description
 0x08  Log the read and write file operations and the VIDIOC_QBUF and
       VIDIOC_DQBUF ioctls.
 0x10  Log the poll file operation.
+0x20  Log error and messages in the control operations.
 ===== ================================================================
 
 Video device cleanup
drivers/media/platform/omap3isp/ispvideo.c +2 -2
index 175bbed9a235..083cce840dc1 100644
@@ -1027,8 +1027,8 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
 
 	ctrls.count = 1;
 	ctrls.controls = &ctrl;
-
-	ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, NULL, &ctrls);
+	ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, &video->video,
+			       NULL, &ctrls);
 	if (ret < 0) {
 		dev_warn(isp->dev, "no pixel rate control in subdev %s\n",
 			 pipe->external->name);
drivers/media/v4l2-core/v4l2-ctrls.c +100 -26

281 lines not on this page · raw message

drivers/media/v4l2-core/v4l2-ioctl.c +12 -6
index b1f4b991dba6..6a08fc00d077 100644
@@ -2165,9 +2165,11 @@ static int v4l_g_ext_ctrls(const struct v4l2_ioctl_ops *ops,
 
 	p->error_idx = p->count;
 	if (vfh && vfh->ctrl_handler)
-		return v4l2_g_ext_ctrls(vfh->ctrl_handler, vfd->v4l2_dev->mdev, p);
+		return v4l2_g_ext_ctrls(vfh->ctrl_handler,
+					vfd, vfd->v4l2_dev->mdev, p);
 	if (vfd->ctrl_handler)
-		return v4l2_g_ext_ctrls(vfd->ctrl_handler, vfd->v4l2_dev->mdev, p);
+		return v4l2_g_ext_ctrls(vfd->ctrl_handler,
+					vfd, vfd->v4l2_dev->mdev, p);
 	if (ops->vidioc_g_ext_ctrls == NULL)
 		return -ENOTTY;
 	return check_ext_ctrls(p, 0) ? ops->vidioc_g_ext_ctrls(file, fh, p) :

@@ -2184,9 +2186,11 @@ static int v4l_s_ext_ctrls(const struct v4l2_ioctl_ops *ops,
 
 	p->error_idx = p->count;
 	if (vfh && vfh->ctrl_handler)
-		return v4l2_s_ext_ctrls(vfh, vfh->ctrl_handler, vfd->v4l2_dev->mdev, p);
+		return v4l2_s_ext_ctrls(vfh, vfh->ctrl_handler,
+					vfd, vfd->v4l2_dev->mdev, p);
 	if (vfd->ctrl_handler)
-		return v4l2_s_ext_ctrls(NULL, vfd->ctrl_handler, vfd->v4l2_dev->mdev, p);
+		return v4l2_s_ext_ctrls(NULL, vfd->ctrl_handler,
+					vfd, vfd->v4l2_dev->mdev, p);
 	if (ops->vidioc_s_ext_ctrls == NULL)
 		return -ENOTTY;
 	return check_ext_ctrls(p, 0) ? ops->vidioc_s_ext_ctrls(file, fh, p) :

@@ -2203,9 +2207,11 @@ static int v4l_try_ext_ctrls(const struct v4l2_ioctl_ops *ops,
 
 	p->error_idx = p->count;
 	if (vfh && vfh->ctrl_handler)
-		return v4l2_try_ext_ctrls(vfh->ctrl_handler, vfd->v4l2_dev->mdev, p);
+		return v4l2_try_ext_ctrls(vfh->ctrl_handler,
+					  vfd, vfd->v4l2_dev->mdev, p);
 	if (vfd->ctrl_handler)
-		return v4l2_try_ext_ctrls(vfd->ctrl_handler, vfd->v4l2_dev->mdev, p);
+		return v4l2_try_ext_ctrls(vfd->ctrl_handler,
+					  vfd, vfd->v4l2_dev->mdev, p);
 	if (ops->vidioc_try_ext_ctrls == NULL)
 		return -ENOTTY;
 	return check_ext_ctrls(p, 0) ? ops->vidioc_try_ext_ctrls(file, fh, p) :
drivers/media/v4l2-core/v4l2-subdev.c +3 -3
index f24978b80440..1b5edd3b1e6c 100644
@@ -211,19 +211,19 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (!vfh->ctrl_handler)
 			return -ENOTTY;
 		return v4l2_g_ext_ctrls(vfh->ctrl_handler,
-					sd->v4l2_dev->mdev, arg);
+					vdev, sd->v4l2_dev->mdev, arg);
 
 	case VIDIOC_S_EXT_CTRLS:
 		if (!vfh->ctrl_handler)
 			return -ENOTTY;
 		return v4l2_s_ext_ctrls(vfh, vfh->ctrl_handler,
-					sd->v4l2_dev->mdev, arg);
+					vdev, sd->v4l2_dev->mdev, arg);
 
 	case VIDIOC_TRY_EXT_CTRLS:
 		if (!vfh->ctrl_handler)
 			return -ENOTTY;
 		return v4l2_try_ext_ctrls(vfh->ctrl_handler,
-					  sd->v4l2_dev->mdev, arg);
+					  vdev, sd->v4l2_dev->mdev, arg);
 
 	case VIDIOC_DQEVENT:
 		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
include/media/v4l2-ctrls.h +7 -2
index b4433483af23..c08d6cc56743 100644
@@ -1265,25 +1265,28 @@ int v4l2_s_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
  *	:ref:`VIDIOC_G_EXT_CTRLS <vidioc_g_ext_ctrls>` ioctl
  *
  * @hdl: pointer to &struct v4l2_ctrl_handler
+ * @vdev: pointer to &struct video_device
  * @mdev: pointer to &struct media_device
  * @c: pointer to &struct v4l2_ext_controls
  *
  * If hdl == NULL then they will all return -EINVAL.
  */
-int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct media_device *mdev,
-		     struct v4l2_ext_controls *c);
+int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct video_device *vdev,
+		     struct media_device *mdev, struct v4l2_ext_controls *c);
 
 /**
  * v4l2_try_ext_ctrls - Helper function to implement
  *	:ref:`VIDIOC_TRY_EXT_CTRLS <vidioc_g_ext_ctrls>` ioctl
  *
  * @hdl: pointer to &struct v4l2_ctrl_handler
+ * @vdev: pointer to &struct video_device
  * @mdev: pointer to &struct media_device
  * @c: pointer to &struct v4l2_ext_controls
  *
  * If hdl == NULL then they will all return -EINVAL.
  */
 int v4l2_try_ext_ctrls(struct v4l2_ctrl_handler *hdl,
+		       struct video_device *vdev,
 		       struct media_device *mdev,
 		       struct v4l2_ext_controls *c);
 

@@ -1293,12 +1296,14 @@ int v4l2_try_ext_ctrls(struct v4l2_ctrl_handler *hdl,
  *
  * @fh: pointer to &struct v4l2_fh
  * @hdl: pointer to &struct v4l2_ctrl_handler
+ * @vdev: pointer to &struct video_device
  * @mdev: pointer to &struct media_device
  * @c: pointer to &struct v4l2_ext_controls
  *
  * If hdl == NULL then they will all return -EINVAL.
  */
 int v4l2_s_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
+		     struct video_device *vdev,
 		     struct media_device *mdev,
 		     struct v4l2_ext_controls *c);
include/media/v4l2-ioctl.h +2 -0
index 400f2e46c108..4bba65a59d46 100644
@@ -602,6 +602,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  */
Signature · 1 line
2.22.0
#1 lore raw

Hi Ezequiel,

Quoted 8 lines · Ezequiel Garcia

Currently, the v4l2 control code is a bit silent on errors. Add debug messages on (hopefully) most of the error paths.

Signed-off-by Ezequiel Garcia <[email protected]>

---
Changes from v3:
* Add missing parenthesis around macro argument.

Quoted 5 lines

Changes from v3:
* Fix checkpatch.pl warnings about long lines.

Changes from v2:
* Check and noisy-warn if vdev is NULL.

As you require the vdev argument to be non-NULL, you may as well drop the mdev argument: video device is always specific to a media device, and the pointer to the media device can be found in vdev->entity.graph_obj.mdev if MEDIA_CONTROLLER is enabled (AFAIR).

Signature · 3 lines
Kind regards,

Sakari Ailus