On Sat, 2019-07-20 at 11:36 +0200, Hans Verkuil wrote:
> On 7/19/19 11:53 PM, Ezequiel Garcia wrote:
> > On Thu, 2019-06-27 at 09:38 +0200, Hans Verkuil wrote:
> > > Hi Ezequiel,
> > > 
> > > 'checkpatch.pl --strict' finds way too many issues.
> > > 
> > > I'm pretty certain you didn't run it.
> > > 
> > 
> > Shame on me, I did run it and skipped fixing the long lines :-(
> > 
> > > Please fix the issues and post a v4.
> > > 
> > > A lot (but not all) of the warnings report lines that are too long
> > > and from what I can see most are easily fixed without reducing
> > > readability.
> > > 
> > 
> > I have fixed all the long lines. IMHO, the remaining warnings
> > are not worth fixing:
> > 
> > CHECK: Macro argument reuse 'vdev' - possible side-effects?
> > #77: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:21:
> > +#define dprintk(vdev, fmt, arg...) do {                                    
> > \
> > +   if (!WARN_ON(!vdev) && ((vdev)->dev_debug & V4L2_DEV_DEBUG_CTRL)) \
> > +           printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),               \
> > +                  __func__, video_device_node_name(vdev), ##arg);  \
> > +} while (0)
> 
> You really need to use 'if (!WARN_ON(!(vdev)) &&' here. It is a little thing 
> that
> simply makes the code more robust.
> 
> > CHECK: Macro argument 'vdev' may be better as '(vdev)' to avoid precedence 
> > issues
> > #77: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:21:
> > +#define dprintk(vdev, fmt, arg...) do {                                    
> > \
> > +   if (!WARN_ON(!vdev) && ((vdev)->dev_debug & V4L2_DEV_DEBUG_CTRL)) \
> > +           printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),               \
> > +                  __func__, video_device_node_name(vdev), ##arg);  \
> > +} while (0)
> > 
> > WARNING: Prefer [subsystem eg: netdev]_dbg([subsystem]dev, ... then 
> > dev_dbg(dev, ... then pr_debug(...  to printk(KERN_DEBUG ...
> > #79: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:23:
> > +           printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),               \
> > 
> > CHECK: Comparison to NULL could be written "!ref"
> > #116: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:3255:
> > +           if (ref == NULL) {
> > 
> > CHECK: Comparison to NULL could be written "!hdl"
> > #249: FILE: drivers/media/v4l2-core/v4l2-ctrls.c:3730:
> > +   if (hdl == NULL) {
> > 
> > I believe the dprintk macro is OK as-is. We use printk KERN_DEBUG because
> > we already have a parameter guarding it. This is consistent
> > with the v4l2 way.
> > 
> > The NULL comparison warning is valid, but it was already
> > like this before. I'd rather not change existing code.
> > 
> > I'll post a v4.
> 
> Skipping the other warnings is OK, but the first really needs to be addressed.
> So we need a v5, I'm afraid.
> 

No worries!

> Regards,
> 
>       Hans
> 
> > Ezequiel
> > 
> > > Regards,
> > > 
> > >   Hans
> > > 
> > > On 6/22/19 12:28 PM, Ezequiel Garcia wrote:
> > > > 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 <ezequ...@collabora.com>
> > > > ---
> > > > 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 |   2 +-
> > > >  drivers/media/v4l2-core/v4l2-ctrls.c       | 106 ++++++++++++++++-----
> > > >  drivers/media/v4l2-core/v4l2-ioctl.c       |  12 +--
> > > >  drivers/media/v4l2-core/v4l2-subdev.c      |   6 +-
> > > >  include/media/v4l2-ctrls.h                 |   9 +-
> > > >  include/media/v4l2-ioctl.h                 |   2 +
> > > >  7 files changed, 100 insertions(+), 38 deletions(-)
> > > > 
> > > > diff --git a/Documentation/media/kapi/v4l2-dev.rst 
> > > > b/Documentation/media/kapi/v4l2-dev.rst
> > > > index b359f1804bbe..4c5a15c53dbf 100644
> > > > --- a/Documentation/media/kapi/v4l2-dev.rst
> > > > +++ b/Documentation/media/kapi/v4l2-dev.rst
> > > > @@ -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
> > > > diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
> > > > b/drivers/media/platform/omap3isp/ispvideo.c
> > > > index 175bbed9a235..abc945cc05c9 100644
> > > > --- a/drivers/media/platform/omap3isp/ispvideo.c
> > > > +++ b/drivers/media/platform/omap3isp/ispvideo.c
> > > > @@ -1028,7 +1028,7 @@ 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);
> > > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> > > > b/drivers/media/v4l2-core/v4l2-ctrls.c
> > > > index 2d7525e2d9eb..1c8ae4501870 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > > > @@ -6,6 +6,8 @@
> > > >  
> > > >   */
> > > >  
> > > > +#define pr_fmt(fmt) "v4l2-ctrls: " fmt
> > > > +
> > > >  #include <linux/ctype.h>
> > > >  #include <linux/mm.h>
> > > >  #include <linux/slab.h>
> > > > @@ -16,6 +18,12 @@
> > > >  #include <media/v4l2-event.h>
> > > >  #include <media/v4l2-dev.h>
> > > >  
> > > > +#define dprintk(vdev, fmt, arg...) do {                                
> > > >         \
> > > > +       if (!WARN_ON(!vdev) && ((vdev)->dev_debug & 
> > > > V4L2_DEV_DEBUG_CTRL)) \
> > > > +               printk(KERN_DEBUG pr_fmt("%s: %s: " fmt),               
> > > > \
> > > > +                      __func__, video_device_node_name(vdev), ##arg);  
> > > > \
> > > > +} while (0)
> > > > +
> > > >  #define has_op(master, op) \
> > > >         (master->ops && master->ops->op)
> > > >  #define call_op(master, op) \
> > > > @@ -3211,6 +3219,7 @@ static int v4l2_ctrl_request_bind(struct 
> > > > media_request *req,
> > > >  static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> > > >                              struct v4l2_ext_controls *cs,
> > > >                              struct v4l2_ctrl_helper *helpers,
> > > > +                            struct video_device *vdev,
> > > >                              bool get)
> > > >  {
> > > >         struct v4l2_ctrl_helper *h;
> > > > @@ -3228,20 +3237,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(vdev, "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(vdev, "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(vdev, "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(vdev, "control id 0x%x is disabled\n", 
> > > > id);
> > > >                         return -EINVAL;
> > > > +               }
> > > >  
> > > >                 if (ctrl->cluster[0]->ncontrols > 1)
> > > >                         have_clusters = true;
> > > > @@ -3251,10 +3268,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(vdev, "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;
> > > > @@ -3315,7 +3338,8 @@ static int class_check(struct v4l2_ctrl_handler 
> > > > *hdl, u32 which)
> > > >  
> > > >  /* Get extended controls. Allocates the helpers array if needed. */
> > > >  static int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl,
> > > > -                                  struct v4l2_ext_controls *cs)
> > > > +                                  struct v4l2_ext_controls *cs,
> > > > +                                  struct video_device *vdev)
> > > >  {
> > > >         struct v4l2_ctrl_helper helper[4];
> > > >         struct v4l2_ctrl_helper *helpers = helper;
> > > > @@ -3341,7 +3365,7 @@ static int v4l2_g_ext_ctrls_common(struct 
> > > > v4l2_ctrl_handler *hdl,
> > > >                         return -ENOMEM;
> > > >         }
> > > >  
> > > > -       ret = prepare_ext_ctrls(hdl, cs, helpers, true);
> > > > +       ret = prepare_ext_ctrls(hdl, cs, helpers, vdev, true);
> > > >         cs->error_idx = cs->count;
> > > >  
> > > >         for (i = 0; !ret && i < cs->count; i++)
> > > > @@ -3434,8 +3458,8 @@ v4l2_ctrls_find_req_obj(struct v4l2_ctrl_handler 
> > > > *hdl,
> > > >         return obj;
> > > >  }
> > > >  
> > > > -int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct 
> > > > media_device *mdev,
> > > > -                    struct v4l2_ext_controls *cs)
> > > > +int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct 
> > > > video_device *vdev,
> > > > +                    struct media_device *mdev, struct 
> > > > v4l2_ext_controls *cs)
> > > >  {
> > > >         struct media_request_object *obj = NULL;
> > > >         struct media_request *req = NULL;
> > > > @@ -3471,7 +3495,7 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler 
> > > > *hdl, struct media_device *mdev,
> > > >                                    req_obj);
> > > >         }
> > > >  
> > > > -       ret = v4l2_g_ext_ctrls_common(hdl, cs);
> > > > +       ret = v4l2_g_ext_ctrls_common(hdl, cs, vdev);
> > > >  
> > > >         if (obj) {
> > > >                 media_request_unlock_for_access(req);
> > > > @@ -3614,7 +3638,9 @@ static int try_or_set_cluster(struct v4l2_fh *fh, 
> > > > struct v4l2_ctrl *master,
> > > >  
> > > >  /* Validate controls. */
> > > >  static int validate_ctrls(struct v4l2_ext_controls *cs,
> > > > -                         struct v4l2_ctrl_helper *helpers, bool set)
> > > > +                         struct v4l2_ctrl_helper *helpers,
> > > > +                         struct video_device *vdev,
> > > > +                         bool set)
> > > >  {
> > > >         unsigned i;
> > > >         int ret = 0;
> > > > @@ -3626,16 +3652,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(vdev, "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(vdev, "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.
> > > > @@ -3670,7 +3700,8 @@ static void update_from_auto_cluster(struct 
> > > > v4l2_ctrl *master)
> > > >  /* Try or try-and-set controls */
> > > >  static int try_set_ext_ctrls_common(struct v4l2_fh *fh,
> > > >                                     struct v4l2_ctrl_handler *hdl,
> > > > -                                   struct v4l2_ext_controls *cs, bool 
> > > > set)
> > > > +                                   struct v4l2_ext_controls *cs,
> > > > +                                   struct video_device *vdev, bool set)
> > > >  {
> > > >         struct v4l2_ctrl_helper helper[4];
> > > >         struct v4l2_ctrl_helper *helpers = helper;
> > > > @@ -3680,13 +3711,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(vdev, "%s: cannot change default value\n", 
> > > > video_device_node_name(vdev));
> > > >                 return -EINVAL;
> > > > +       }
> > > >  
> > > >         cs->which = V4L2_CTRL_ID2WHICH(cs->which);
> > > >  
> > > > -       if (hdl == NULL)
> > > > +       if (hdl == NULL) {
> > > > +               dprintk(vdev, "%s: invalid null control handler\n", 
> > > > video_device_node_name(vdev));
> > > >                 return -EINVAL;
> > > > +       }
> > > >  
> > > >         if (cs->count == 0)
> > > >                 return class_check(hdl, cs->which);
> > > > @@ -3697,9 +3732,9 @@ static int try_set_ext_ctrls_common(struct 
> > > > v4l2_fh *fh,
> > > >                 if (!helpers)
> > > >                         return -ENOMEM;
> > > >         }
> > > > -       ret = prepare_ext_ctrls(hdl, cs, helpers, false);
> > > > +       ret = prepare_ext_ctrls(hdl, cs, helpers, vdev, false);
> > > >         if (!ret)
> > > > -               ret = validate_ctrls(cs, helpers, set);
> > > > +               ret = validate_ctrls(cs, helpers, vdev, set);
> > > >         if (ret && set)
> > > >                 cs->error_idx = cs->count;
> > > >         for (i = 0; !ret && i < cs->count; i++) {
> > > > @@ -3784,7 +3819,8 @@ static int try_set_ext_ctrls_common(struct 
> > > > v4l2_fh *fh,
> > > >  }
> > > >  
> > > >  static int try_set_ext_ctrls(struct v4l2_fh *fh,
> > > > -                            struct v4l2_ctrl_handler *hdl, struct 
> > > > media_device *mdev,
> > > > +                            struct v4l2_ctrl_handler *hdl,
> > > > +                            struct video_device *vdev, struct 
> > > > media_device *mdev,
> > > >                              struct v4l2_ext_controls *cs, bool set)
> > > >  {
> > > >         struct media_request_object *obj = NULL;
> > > > @@ -3792,21 +3828,32 @@ 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) {
> > > > +                       dprintk(vdev, "%s: missing media device\n", 
> > > > video_device_node_name(vdev));
> > > > +                       return -EINVAL;
> > > > +               }
> > > > +
> > > > +               if (cs->request_fd < 0) {
> > > > +                       dprintk(vdev, "%s: invalid request fd %d\n", 
> > > > video_device_node_name(vdev), cs->request_fd);
> > > >                         return -EINVAL;
> > > > +               }
> > > >  
> > > >                 req = media_request_get_by_fd(mdev, cs->request_fd);
> > > > -               if (IS_ERR(req))
> > > > +               if (IS_ERR(req)) {
> > > > +                       dprintk(vdev, "%s: cannot find request fd 
> > > > %d\n", video_device_node_name(vdev), cs->request_fd);
> > > >                         return PTR_ERR(req);
> > > > +               }
> > > >  
> > > >                 ret = media_request_lock_for_update(req);
> > > >                 if (ret) {
> > > > +                       dprintk(vdev, "%s: cannot lock request fd 
> > > > %d\n", video_device_node_name(vdev), cs->request_fd);
> > > >                         media_request_put(req);
> > > >                         return ret;
> > > >                 }
> > > >  
> > > >                 obj = v4l2_ctrls_find_req_obj(hdl, req, set);
> > > >                 if (IS_ERR(obj)) {
> > > > +                       dprintk(vdev, "%s: cannot find request object 
> > > > for request fd %d\n", video_device_node_name(vdev), cs->request_fd);
> > > >                         media_request_unlock_for_update(req);
> > > >                         media_request_put(req);
> > > >                         return PTR_ERR(obj);
> > > > @@ -3815,7 +3862,9 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh,
> > > >                                    req_obj);
> > > >         }
> > > >  
> > > > -       ret = try_set_ext_ctrls_common(fh, hdl, cs, set);
> > > > +       ret = try_set_ext_ctrls_common(fh, hdl, cs, vdev, set);
> > > > +       if (ret)
> > > > +               dprintk(vdev, "%s: try_set_ext_ctrls_common failed 
> > > > (%d)\n", video_device_node_name(vdev), ret);
> > > >  
> > > >         if (obj) {
> > > >                 media_request_unlock_for_update(req);
> > > > @@ -3826,17 +3875,22 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh,
> > > >         return ret;
> > > >  }
> > > >  
> > > > -int v4l2_try_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct 
> > > > media_device *mdev,
> > > > +int v4l2_try_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> > > > +                      struct video_device *vdev,
> > > > +                      struct media_device *mdev,
> > > >                        struct v4l2_ext_controls *cs)
> > > >  {
> > > > -       return try_set_ext_ctrls(NULL, hdl, mdev, cs, false);
> > > > +       return try_set_ext_ctrls(NULL, hdl, vdev, mdev, cs, false);
> > > >  }
> > > >  EXPORT_SYMBOL(v4l2_try_ext_ctrls);
> > > >  
> > > > -int v4l2_s_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
> > > > -                    struct media_device *mdev, struct 
> > > > v4l2_ext_controls *cs)
> > > > +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 *cs)
> > > >  {
> > > > -       return try_set_ext_ctrls(fh, hdl, mdev, cs, true);
> > > > +       return try_set_ext_ctrls(fh, hdl, vdev, mdev, cs, true);
> > > >  }
> > > >  EXPORT_SYMBOL(v4l2_s_ext_ctrls);
> > > >  
> > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> > > > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > index b1f4b991dba6..e95efea1a9ca 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > @@ -2165,9 +2165,9 @@ 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 +2184,9 @@ 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 +2203,9 @@ 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) :
> > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
> > > > b/drivers/media/v4l2-core/v4l2-subdev.c
> > > > index f24978b80440..1b5edd3b1e6c 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > > > @@ -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))
> > > > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > > > index b4433483af23..c08d6cc56743 100644
> > > > --- a/include/media/v4l2-ctrls.h
> > > > +++ b/include/media/v4l2-ctrls.h
> > > > @@ -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);
> > > >  
> > > > diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> > > > index 400f2e46c108..4bba65a59d46 100644
> > > > --- a/include/media/v4l2-ioctl.h
> > > > +++ b/include/media/v4l2-ioctl.h
> > > > @@ -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  */
> > > >  
> > > > 


Reply via email to