From: Ezequiel Garcia <ezequiel@collabora.com>
To: linux-media@vger.kernel.org
Cc: Hans Verkuil <hans.verkuil@cisco.com>,
kernel@collabora.com, Ezequiel Garcia <ezequiel@collabora.com>
Subject: [PATCH 4/4] media: v4l: ctrls: Add debug messages
Date: Mon, 18 Feb 2019 17:15:28 -0300 [thread overview]
Message-ID: <20190218201528.21545-5-ezequiel@collabora.com> (raw)
In-Reply-To: <20190218201528.21545-1-ezequiel@collabora.com>
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
next prev parent reply other threads:[~2019-02-18 20:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-18 20:15 [PATCH 0/4] Add debug messages to v4l2-ctrls Ezequiel Garcia
2019-02-18 20:15 ` [PATCH 1/4] media: v4l: Simplify dev_debug flags Ezequiel Garcia
2019-02-18 20:15 ` [PATCH 2/4] media: v4l: Improve debug dprintk macro Ezequiel Garcia
2019-02-18 20:15 ` [PATCH 3/4] media: v4l: Add a module parameter to control global debugging Ezequiel Garcia
2019-02-18 20:15 ` Ezequiel Garcia [this message]
2019-02-25 21:25 ` [PATCH 4/4] media: v4l: ctrls: Add debug messages Ezequiel Garcia
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190218201528.21545-5-ezequiel@collabora.com \
--to=ezequiel@collabora.com \
--cc=hans.verkuil@cisco.com \
--cc=kernel@collabora.com \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox