On 07/15/15 23:05, Laurent Pinchart wrote:
> 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.ribalda@xxxxxxxxx>
>> ---
>> 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)
It's only open-coded in drivers that do not use the control framework. For
drivers that use the control framework it is completely transparent.
Regards,
Hans
>
> 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,
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
