Hi,
On 24-Feb-25 11:34, Ricardo Ribalda wrote:
> If we wail to commit an entity, we need to restore the
> UVC_CTRL_DATA_BACKUP for the other uncommitted entities. Otherwise the
> control cache and the device would be out of sync.
>
> Cc: stable@xxxxxxxxxx
> Fixes: b4012002f3a3 ("[media] uvcvideo: Add support for control events")
> Reported-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-media/fe845e04-9fde-46ee-9763-a6f00867929a@xxxxxxxxxx/
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 7d074686eef4..89b946151b16 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1864,7 +1864,7 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
> unsigned int processed_ctrls = 0;
> struct uvc_control *ctrl;
> unsigned int i;
> - int ret;
> + int ret = 0;
>
> if (entity == NULL)
> return 0;
> @@ -1893,8 +1893,6 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
> dev->intfnum, ctrl->info.selector,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT),
> ctrl->info.size);
> - else
> - ret = 0;
>
> if (!ret)
> processed_ctrls++;
> @@ -1906,10 +1904,14 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
>
> ctrl->dirty = 0;
>
> - if (ret < 0) {
> + if (ret < 0 && !rollback) {
> if (err_ctrl)
> *err_ctrl = ctrl;
> - return ret;
> + /*
> + * If we fail to set a control, we need to rollback
> + * the next ones.
> + */
> + rollback = 1;
> }
>
> if (!rollback && handle &&
> @@ -1917,6 +1919,9 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
> uvc_ctrl_set_handle(handle, ctrl, handle);
> }
>
> + if (ret)
> + return ret;
> +
> return processed_ctrls;
> }
>
> @@ -1947,7 +1952,8 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
> struct uvc_video_chain *chain = handle->chain;
> struct uvc_control *err_ctrl;
> struct uvc_entity *entity;
> - int ret = 0;
> + int ret_out = 0;
> + int ret;
>
> /* Find the control. */
> list_for_each_entry(entity, &chain->entities, chain) {
> @@ -1958,17 +1964,23 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
> ctrls->error_idx =
> uvc_ctrl_find_ctrl_idx(entity, ctrls,
> err_ctrl);
> - goto done;
> + /*
> + * When we fail to commit an entity, we need to
> + * restore the UVC_CTRL_DATA_BACKUP for all the
> + * controls in the other entities, otherwise our cache
> + * and the hardware will be out of sync.
> + */
> + rollback = 1;
> +
> + ret_out = ret;
> } else if (ret > 0 && !rollback) {
> uvc_ctrl_send_events(handle, entity,
> ctrls->controls, ctrls->count);
> }
> }
>
> - ret = 0;
> -done:
> mutex_unlock(&chain->ctrl_mutex);
> - return ret;
> + return ret_out;
> }
>
> int uvc_ctrl_get(struct uvc_video_chain *chain,
>
