Hi Ricardo,

Thank you for the patch.

On Friday 12 June 2015 18:46:23 Ricardo Ribalda Delgado wrote:
> Callback needed by ioctl VIDIOC_G_DEF_EXT_CTRLS as this driver does not
> use the controller framework.
> 
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.riba...@gmail.com>
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index 2764f43607c1..e2698a77138a 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1001,6 +1001,35 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
> void *fh, return uvc_ctrl_rollback(handle);
>  }
> 
> +static int uvc_ioctl_g_def_ext_ctrls(struct file *file, void *fh,
> +                                  struct v4l2_ext_controls *ctrls)
> +{
> +     struct uvc_fh *handle = fh;
> +     struct uvc_video_chain *chain = handle->chain;
> +     struct v4l2_ext_control *ctrl = ctrls->controls;
> +     unsigned int i;
> +     int ret;
> +     struct v4l2_queryctrl qc;
> +
> +     ret = uvc_ctrl_begin(chain);

There's no need to call uvc_ctrl_begin() here (and if there was, you'd have to 
call uvc_ctrl_rollback() or uvc_ctrl_commit() before returning).

> +     if (ret < 0)
> +             return ret;
> +
> +     for (i = 0; i < ctrls->count; ++ctrl, ++i) {
> +             qc.id = ctrl->id;
> +             ret = uvc_query_v4l2_ctrl(chain, &qc);
> +             if (ret < 0) {
> +                     ctrls->error_idx = i;
> +                     return ret;
> +             }
> +             ctrl->value = qc.default_value;
> +     }
> +
> +     ctrls->error_idx = 0;
> +
> +     return 0;
> +}

Instead of open-coding this in multiple drivers, how about adding a helper 
function to the core ? Something like (totally untested)

int v4l2_ioctl_g_def_ext_ctrls(struct file *file, void *fh,
                               struct v4l2_ext_controls *ctrls)
{
        struct video_device *vdev = video_devdata(file);
        unsigned int i;
        int ret;

        for (i = 0; i < ctrls->count; ++i) {
                struct v4l2_queryctrl qc;

                qc.id = ctrl->id;
                ret = vdev->ioctl_ops->vidioc_queryctrl(file, fh, &qc);
                if (ret < 0) {
                        ctrls->error_idx = i;
                        return ret;
                }

                ctrls->controls[i].value = qc.default_value;
        }

        ctrls->error_idx = 0;

        return 0;
}

The function could be called by v4l_g_def_ext_ctrls() when ops-
>vidioc_g_def_ext_ctrls is NULL.

>  static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
>                                    struct v4l2_ext_controls *ctrls,
>                                    bool commit)
> @@ -1500,6 +1529,7 @@ const struct v4l2_ioctl_ops uvc_ioctl_ops = {
>       .vidioc_g_ctrl = uvc_ioctl_g_ctrl,
>       .vidioc_s_ctrl = uvc_ioctl_s_ctrl,
>       .vidioc_g_ext_ctrls = uvc_ioctl_g_ext_ctrls,
> +     .vidioc_g_def_ext_ctrls = uvc_ioctl_g_def_ext_ctrls,
>       .vidioc_s_ext_ctrls = uvc_ioctl_s_ext_ctrls,
>       .vidioc_try_ext_ctrls = uvc_ioctl_try_ext_ctrls,
>       .vidioc_querymenu = uvc_ioctl_querymenu,

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to