On Mon Feb 24 10:34:55 2025 +0000, Ricardo Ribalda wrote:
> If we fail 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: sta...@kernel.org
> Fixes: b4012002f3a3 ("[media] uvcvideo: Add support for control events")
> Reported-by: Hans de Goede <hdego...@redhat.com>
> Closes:
> https://lore.kernel.org/linux-media/fe845e04-9fde-46ee-9763-a6f008679...@redhat.com/
> Signed-off-by: Ricardo Ribalda <riba...@chromium.org>
> Message-ID: <20250224-uvc-data-backup-v2-3-de993ed98...@chromium.org>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> Signed-off-by: Hans Verkuil <hverk...@xs4all.nl>
Patch committed.
Thanks,
Hans Verkuil
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 636ce1eb2a6b..44b6513c5264 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2119,7 +2119,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;
@@ -2148,8 +2148,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++;
@@ -2165,13 +2163,20 @@ static int uvc_ctrl_commit_entity(struct uvc_device
*dev,
ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
ret = uvc_ctrl_set_handle(handle, ctrl, handle);
- 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 (ret)
+ return ret;
+
return processed_ctrls;
}
@@ -2202,7 +2207,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) {
@@ -2213,17 +2219,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;
}
static int uvc_mapping_get_xctrl_compound(struct uvc_video_chain *chain,
