| Message ID | 20210316180004.1605727-11-ribalda@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | uvcvideo: Fix v4l2-compliance errors | expand |
On 16/03/2021 19:00, Ricardo Ribalda wrote: > If a control is inactive return -EACCES to let the userspace know that > the value will not be applied automatically when the control is active > again. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > Suggested-by: Hans Verkuil <hverkuil@xs4all.nl> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index ba14733db757..98614e1be829 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -1578,6 +1578,18 @@ int uvc_ctrl_begin(struct uvc_video_chain *chain) > return mutex_lock_interruptible(&chain->ctrl_mutex) ? -ERESTARTSYS : 0; > } > > +static bool uvc_ctrl_is_inactive(struct uvc_control *ctrl) This doesn't test if the control is inactive, it tests if it *might* be inactive. To test if it is really inactive would require checking the value of the master control. > +{ > + struct uvc_control_mapping *map; > + > + list_for_each_entry(map, &ctrl->info.mappings, list) { > + if (map->master_id) > + return true; > + } > + > + return false; > +} > + > static int uvc_ctrl_commit_entity(struct uvc_device *dev, > struct uvc_entity *entity, int rollback) > { > @@ -1621,8 +1633,11 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev, > > ctrl->dirty = 0; > > - if (ret < 0) > + if (ret < 0) { > + if (uvc_ctrl_is_inactive(ctrl)) > + return -EACCES; So here you assume that if setting the control failed, and if the control might be inactive, then it probably was inactive. I feel a bit uncomfortable by this assumption. Regards, Hans > return ret; > + } > } > > return 0; >
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index ba14733db757..98614e1be829 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1578,6 +1578,18 @@ int uvc_ctrl_begin(struct uvc_video_chain *chain) return mutex_lock_interruptible(&chain->ctrl_mutex) ? -ERESTARTSYS : 0; } +static bool uvc_ctrl_is_inactive(struct uvc_control *ctrl) +{ + struct uvc_control_mapping *map; + + list_for_each_entry(map, &ctrl->info.mappings, list) { + if (map->master_id) + return true; + } + + return false; +} + static int uvc_ctrl_commit_entity(struct uvc_device *dev, struct uvc_entity *entity, int rollback) { @@ -1621,8 +1633,11 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev, ctrl->dirty = 0; - if (ret < 0) + if (ret < 0) { + if (uvc_ctrl_is_inactive(ctrl)) + return -EACCES; return ret; + } } return 0;
If a control is inactive return -EACCES to let the userspace know that the value will not be applied automatically when the control is active again. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> Suggested-by: Hans Verkuil <hverkuil@xs4all.nl> --- drivers/media/usb/uvc/uvc_ctrl.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)