| Message ID | 1440163169-18047-7-git-send-email-ricardo.ribalda@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Laurent, can you review this? Regards, Hans On 08/21/2015 03:19 PM, Ricardo Ribalda Delgado wrote: > This driver does not use the control infrastructure. > Add support for the new field which on structure > v4l2_ext_controls > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > --- > drivers/media/usb/uvc/uvc_v4l2.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 2764f43607c1..e6d3a1bcfa2f 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -980,6 +980,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, > struct uvc_fh *handle = fh; > struct uvc_video_chain *chain = handle->chain; > struct v4l2_ext_control *ctrl = ctrls->controls; > + struct v4l2_queryctrl qc; > unsigned int i; > int ret; > > @@ -988,7 +989,14 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, > return ret; > > for (i = 0; i < ctrls->count; ++ctrl, ++i) { > - ret = uvc_ctrl_get(chain, ctrl); > + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) { > + qc.id = ctrl->id; > + ret = uvc_query_v4l2_ctrl(chain, &qc); > + if (!ret) > + ctrl->value = qc.default_value; > + } else > + ret = uvc_ctrl_get(chain, ctrl); > + > if (ret < 0) { > uvc_ctrl_rollback(handle); > ctrls->error_idx = i; > @@ -1010,6 +1018,10 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle, > unsigned int i; > int ret; > > + /* Default value cannot be changed */ > + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) > + return -EINVAL; > + > ret = uvc_ctrl_begin(chain); > if (ret < 0) > return ret; > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/04/15 12:56, Hans Verkuil wrote: > Laurent, can you review this? Ping! If I have an Ack on Monday at the latest, then I can make a pull request for this series before I leave for 2 1/2 weeks. Regards, Hans > > Regards, > > Hans > > On 08/21/2015 03:19 PM, Ricardo Ribalda Delgado wrote: >> This driver does not use the control infrastructure. >> Add support for the new field which on structure >> v4l2_ext_controls >> >> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> >> --- >> drivers/media/usb/uvc/uvc_v4l2.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >> index 2764f43607c1..e6d3a1bcfa2f 100644 >> --- a/drivers/media/usb/uvc/uvc_v4l2.c >> +++ b/drivers/media/usb/uvc/uvc_v4l2.c >> @@ -980,6 +980,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, >> struct uvc_fh *handle = fh; >> struct uvc_video_chain *chain = handle->chain; >> struct v4l2_ext_control *ctrl = ctrls->controls; >> + struct v4l2_queryctrl qc; >> unsigned int i; >> int ret; >> >> @@ -988,7 +989,14 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, >> return ret; >> >> for (i = 0; i < ctrls->count; ++ctrl, ++i) { >> - ret = uvc_ctrl_get(chain, ctrl); >> + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) { >> + qc.id = ctrl->id; >> + ret = uvc_query_v4l2_ctrl(chain, &qc); >> + if (!ret) >> + ctrl->value = qc.default_value; >> + } else >> + ret = uvc_ctrl_get(chain, ctrl); >> + >> if (ret < 0) { >> uvc_ctrl_rollback(handle); >> ctrls->error_idx = i; >> @@ -1010,6 +1018,10 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle, >> unsigned int i; >> int ret; >> >> + /* Default value cannot be changed */ >> + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) >> + return -EINVAL; >> + >> ret = uvc_ctrl_begin(chain); >> if (ret < 0) >> return ret; >> > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Laurent Could you take a look to this patch. Thanks! On Thu, Sep 17, 2015 at 3:17 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: > On 09/04/15 12:56, Hans Verkuil wrote: >> Laurent, can you review this? > > Ping! If I have an Ack on Monday at the latest, then I can make a pull request > for this series before I leave for 2 1/2 weeks. > > Regards, > > Hans > >> >> Regards, >> >> Hans >> >> On 08/21/2015 03:19 PM, Ricardo Ribalda Delgado wrote: >>> This driver does not use the control infrastructure. >>> Add support for the new field which on structure >>> v4l2_ext_controls >>> >>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> >>> --- >>> drivers/media/usb/uvc/uvc_v4l2.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >>> index 2764f43607c1..e6d3a1bcfa2f 100644 >>> --- a/drivers/media/usb/uvc/uvc_v4l2.c >>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c >>> @@ -980,6 +980,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, >>> struct uvc_fh *handle = fh; >>> struct uvc_video_chain *chain = handle->chain; >>> struct v4l2_ext_control *ctrl = ctrls->controls; >>> + struct v4l2_queryctrl qc; >>> unsigned int i; >>> int ret; >>> >>> @@ -988,7 +989,14 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, >>> return ret; >>> >>> for (i = 0; i < ctrls->count; ++ctrl, ++i) { >>> - ret = uvc_ctrl_get(chain, ctrl); >>> + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) { >>> + qc.id = ctrl->id; >>> + ret = uvc_query_v4l2_ctrl(chain, &qc); >>> + if (!ret) >>> + ctrl->value = qc.default_value; >>> + } else >>> + ret = uvc_ctrl_get(chain, ctrl); >>> + >>> if (ret < 0) { >>> uvc_ctrl_rollback(handle); >>> ctrls->error_idx = i; >>> @@ -1010,6 +1018,10 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle, >>> unsigned int i; >>> int ret; >>> >>> + /* Default value cannot be changed */ >>> + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) >>> + return -EINVAL; >>> + >>> ret = uvc_ctrl_begin(chain); >>> if (ret < 0) >>> return ret; >>> >>
Hi Ricardo, Thank you for the patch. On Friday 21 August 2015 15:19:25 Ricardo Ribalda Delgado wrote: > This driver does not use the control infrastructure. > Add support for the new field which on structure > v4l2_ext_controls > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > --- > drivers/media/usb/uvc/uvc_v4l2.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c > b/drivers/media/usb/uvc/uvc_v4l2.c index 2764f43607c1..e6d3a1bcfa2f 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -980,6 +980,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void > *fh, struct uvc_fh *handle = fh; > struct uvc_video_chain *chain = handle->chain; > struct v4l2_ext_control *ctrl = ctrls->controls; > + struct v4l2_queryctrl qc; > unsigned int i; > int ret; > > @@ -988,7 +989,14 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, > void *fh, return ret; > > for (i = 0; i < ctrls->count; ++ctrl, ++i) { > - ret = uvc_ctrl_get(chain, ctrl); > + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) { > + qc.id = ctrl->id; > + ret = uvc_query_v4l2_ctrl(chain, &qc); The uvc_ctrl_begin() call above locks chain->ctrl_mutex, and uvc_query_v4l2_ctrl() will then try to acquire the same lock. That's not a good idea :-) I propose moving the ctrls->which check before the uvc_ctrl_begin() call and implement it as if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) { for (i = 0; i < ctrls->count; ++ctrl, ++i) { struct v4l2_queryctrl 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; } return 0; } > + if (!ret) > + ctrl->value = qc.default_value; > + } else > + ret = uvc_ctrl_get(chain, ctrl); > + > if (ret < 0) { > uvc_ctrl_rollback(handle); > ctrls->error_idx = i; > @@ -1010,6 +1018,10 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh > *handle, unsigned int i; > int ret; > > + /* Default value cannot be changed */ > + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) > + return -EINVAL; > + > ret = uvc_ctrl_begin(chain); > if (ret < 0) > return ret;
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 2764f43607c1..e6d3a1bcfa2f 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -980,6 +980,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, struct uvc_fh *handle = fh; struct uvc_video_chain *chain = handle->chain; struct v4l2_ext_control *ctrl = ctrls->controls; + struct v4l2_queryctrl qc; unsigned int i; int ret; @@ -988,7 +989,14 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh, return ret; for (i = 0; i < ctrls->count; ++ctrl, ++i) { - ret = uvc_ctrl_get(chain, ctrl); + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) { + qc.id = ctrl->id; + ret = uvc_query_v4l2_ctrl(chain, &qc); + if (!ret) + ctrl->value = qc.default_value; + } else + ret = uvc_ctrl_get(chain, ctrl); + if (ret < 0) { uvc_ctrl_rollback(handle); ctrls->error_idx = i; @@ -1010,6 +1018,10 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle, unsigned int i; int ret; + /* Default value cannot be changed */ + if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) + return -EINVAL; + ret = uvc_ctrl_begin(chain); if (ret < 0) return ret;
This driver does not use the control infrastructure. Add support for the new field which on structure v4l2_ext_controls Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/media/usb/uvc/uvc_v4l2.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)