On 18/03/2021 21:29, Ricardo Ribalda wrote:
> Take a v4l2_ext_controls instead of an array of controls, this way we
> can access the error_idx in future changes.
>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 5 ++---
> drivers/media/usb/uvc/uvc_v4l2.c | 8 ++++++--
> drivers/media/usb/uvc/uvcvideo.h | 10 ++++------
> 3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 1ec8333811bc..fb8155ca0c0d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1664,8 +1664,7 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
> }
>
> 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_entity *entity;
> @@ -1679,7 +1678,7 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
> }
>
> if (!rollback)
> - uvc_ctrl_send_events(handle, xctrls, xctrls_count);
> + uvc_ctrl_send_events(handle, ctrls->controls, ctrls->count);
> done:
> mutex_unlock(&chain->ctrl_mutex);
> return ret;
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index ddebdeb5a81b..ea2c41b04664 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1025,6 +1025,10 @@ static int uvc_ioctl_s_ctrl(struct file *file, void *fh,
> struct uvc_fh *handle = fh;
> struct uvc_video_chain *chain = handle->chain;
> struct v4l2_ext_control xctrl;
> + struct v4l2_ext_controls ctrls = {
> + .count = 1,
> + .controls = &xctrl,
> + };
Rather than creating this extra ctrls struct, I think this can be simplified by just
removing uvc_ioctl_s_ctrl and uvc_ioctl_g_ctrl altogether. The v4l2-ioctl.c source
will call vidioc_g/s_ext_ctrls if the driver doesn't provide vidioc_g/s_ctrl ops.
Let's just simplify this by adding another patch before this one that just removes
uvc_ioctl_s_ctrl and uvc_ioctl_g_ctrl.
Otherwise this patch looks good.
Regards,
Hans
> int ret;
>
> ret = uvc_ctrl_is_accessible(chain, ctrl->id, VIDIOC_S_CTRL);
> @@ -1045,7 +1049,7 @@ static int uvc_ioctl_s_ctrl(struct file *file, void *fh,
> return ret;
> }
>
> - ret = uvc_ctrl_commit(handle, &xctrl, 1);
> + ret = uvc_ctrl_commit(handle, &ctrls);
> if (ret < 0)
> return ret;
>
> @@ -1149,7 +1153,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 a93aeedb5499..4e7b6da3c6d2 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);
>
