| Message ID | 20210326095840.364424-16-ribalda@chromium.org |
|---|---|
| State | Accepted |
| Commit | 6350d6a4ed487d16a3a021f76a7edcb9cb60fdbf |
| Headers | show |
| Series | uvcvideo: Fix v4l2-compliance errors | expand |
Hi Ricardo, Thank you for the patch. On Fri, Mar 26, 2021 at 10:58:33AM +0100, Ricardo Ribalda wrote: > If we have an error setting a control, return the affected control in > the error_idx field. > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 42 ++++++++++++++++++++++++++------ > drivers/media/usb/uvc/uvc_v4l2.c | 2 +- > drivers/media/usb/uvc/uvcvideo.h | 10 +++----- > 3 files changed, 40 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 24fd5afc4e4f..bcebf9d1a46f 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -1586,7 +1586,7 @@ int uvc_ctrl_begin(struct uvc_video_chain *chain) > } > > static int uvc_ctrl_commit_entity(struct uvc_device *dev, > - struct uvc_entity *entity, int rollback) > + struct uvc_entity *entity, int rollback, struct uvc_control **err_ctrl) > { > struct uvc_control *ctrl; > unsigned int i; > @@ -1628,31 +1628,59 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev, > > ctrl->dirty = 0; > > - if (ret < 0) > + if (ret < 0) { > + if (err_ctrl) > + *err_ctrl = ctrl; > return ret; > + } > } > > return 0; > } > > +static int uvc_ctrl_find_ctrlidx(struct uvc_entity *entity, s/uvc_ctrl_find_ctrlidx/uvc_ctrl_find_ctrl_idx/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + struct v4l2_ext_controls *ctrls, > + struct uvc_control *uvc_control) > +{ > + struct uvc_control_mapping *mapping; > + struct uvc_control *ctrl_found; > + unsigned int i; > + > + if (!entity) > + return ctrls->count; > + > + for (i = 0; i < ctrls->count; i++) { > + __uvc_find_control(entity, ctrls->controls[i].id, &mapping, > + &ctrl_found, 0); > + if (uvc_control == ctrl_found) > + return i; > + } > + > + return ctrls->count; > +} > + > int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback, > - const struct v4l2_ext_control *xctrls, > - unsigned int xctrls_count) > + struct v4l2_ext_controls *ctrls) > { > struct uvc_video_chain *chain = handle->chain; > + struct uvc_control *err_ctrl; > struct uvc_entity *entity; > int ret = 0; > > /* Find the control. */ > list_for_each_entry(entity, &chain->entities, chain) { > - ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback); > + ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback, > + &err_ctrl); > if (ret < 0) > goto done; > } > > if (!rollback) > - uvc_ctrl_send_events(handle, xctrls, xctrls_count); > + uvc_ctrl_send_events(handle, ctrls->controls, ctrls->count); > done: > + if (ret < 0 && ctrls) > + ctrls->error_idx = uvc_ctrl_find_ctrlidx(entity, ctrls, > + err_ctrl); > mutex_unlock(&chain->ctrl_mutex); > return ret; > } > @@ -2110,7 +2138,7 @@ int uvc_ctrl_restore_values(struct uvc_device *dev) > ctrl->dirty = 1; > } > > - ret = uvc_ctrl_commit_entity(dev, entity, 0); > + ret = uvc_ctrl_commit_entity(dev, entity, 0, NULL); > if (ret < 0) > return ret; > } > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index a3ee1dc003fc..8d8b12a4db34 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -1088,7 +1088,7 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle, > ctrls->error_idx = 0; > > if (ioctl == VIDIOC_S_EXT_CTRLS) > - return uvc_ctrl_commit(handle, ctrls->controls, ctrls->count); > + return uvc_ctrl_commit(handle, ctrls); > else > return uvc_ctrl_rollback(handle); > } > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 9471c342a310..0313b30f0cea 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -887,17 +887,15 @@ void uvc_ctrl_status_event(struct uvc_video_chain *chain, > > int uvc_ctrl_begin(struct uvc_video_chain *chain); > int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback, > - const struct v4l2_ext_control *xctrls, > - unsigned int xctrls_count); > + struct v4l2_ext_controls *ctrls); > static inline int uvc_ctrl_commit(struct uvc_fh *handle, > - const struct v4l2_ext_control *xctrls, > - unsigned int xctrls_count) > + struct v4l2_ext_controls *ctrls) > { > - return __uvc_ctrl_commit(handle, 0, xctrls, xctrls_count); > + return __uvc_ctrl_commit(handle, 0, ctrls); > } > static inline int uvc_ctrl_rollback(struct uvc_fh *handle) > { > - return __uvc_ctrl_commit(handle, 1, NULL, 0); > + return __uvc_ctrl_commit(handle, 1, NULL); > } > > int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl); -- Regards, Laurent Pinchart
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 24fd5afc4e4f..bcebf9d1a46f 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1586,7 +1586,7 @@ int uvc_ctrl_begin(struct uvc_video_chain *chain) } static int uvc_ctrl_commit_entity(struct uvc_device *dev, - struct uvc_entity *entity, int rollback) + struct uvc_entity *entity, int rollback, struct uvc_control **err_ctrl) { struct uvc_control *ctrl; unsigned int i; @@ -1628,31 +1628,59 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev, ctrl->dirty = 0; - if (ret < 0) + if (ret < 0) { + if (err_ctrl) + *err_ctrl = ctrl; return ret; + } } return 0; } +static int uvc_ctrl_find_ctrlidx(struct uvc_entity *entity, + struct v4l2_ext_controls *ctrls, + struct uvc_control *uvc_control) +{ + struct uvc_control_mapping *mapping; + struct uvc_control *ctrl_found; + unsigned int i; + + if (!entity) + return ctrls->count; + + for (i = 0; i < ctrls->count; i++) { + __uvc_find_control(entity, ctrls->controls[i].id, &mapping, + &ctrl_found, 0); + if (uvc_control == ctrl_found) + return i; + } + + return ctrls->count; +} + int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback, - const struct v4l2_ext_control *xctrls, - unsigned int xctrls_count) + struct v4l2_ext_controls *ctrls) { struct uvc_video_chain *chain = handle->chain; + struct uvc_control *err_ctrl; struct uvc_entity *entity; int ret = 0; /* Find the control. */ list_for_each_entry(entity, &chain->entities, chain) { - ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback); + ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback, + &err_ctrl); if (ret < 0) goto done; } if (!rollback) - uvc_ctrl_send_events(handle, xctrls, xctrls_count); + uvc_ctrl_send_events(handle, ctrls->controls, ctrls->count); done: + if (ret < 0 && ctrls) + ctrls->error_idx = uvc_ctrl_find_ctrlidx(entity, ctrls, + err_ctrl); mutex_unlock(&chain->ctrl_mutex); return ret; } @@ -2110,7 +2138,7 @@ int uvc_ctrl_restore_values(struct uvc_device *dev) ctrl->dirty = 1; } - ret = uvc_ctrl_commit_entity(dev, entity, 0); + ret = uvc_ctrl_commit_entity(dev, entity, 0, NULL); if (ret < 0) return ret; } diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index a3ee1dc003fc..8d8b12a4db34 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -1088,7 +1088,7 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle, ctrls->error_idx = 0; if (ioctl == VIDIOC_S_EXT_CTRLS) - return uvc_ctrl_commit(handle, ctrls->controls, ctrls->count); + return uvc_ctrl_commit(handle, ctrls); else return uvc_ctrl_rollback(handle); } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 9471c342a310..0313b30f0cea 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -887,17 +887,15 @@ void uvc_ctrl_status_event(struct uvc_video_chain *chain, int uvc_ctrl_begin(struct uvc_video_chain *chain); int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback, - const struct v4l2_ext_control *xctrls, - unsigned int xctrls_count); + struct v4l2_ext_controls *ctrls); static inline int uvc_ctrl_commit(struct uvc_fh *handle, - const struct v4l2_ext_control *xctrls, - unsigned int xctrls_count) + struct v4l2_ext_controls *ctrls) { - return __uvc_ctrl_commit(handle, 0, xctrls, xctrls_count); + return __uvc_ctrl_commit(handle, 0, ctrls); } static inline int uvc_ctrl_rollback(struct uvc_fh *handle) { - return __uvc_ctrl_commit(handle, 1, NULL, 0); + return __uvc_ctrl_commit(handle, 1, NULL); } int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl);