Hi,
On 2-Dec-24 3:24 PM, Ricardo Ribalda wrote:
> Now we keep a reference to the active fh for any call to uvc_ctrl_set,
> regardless if it is an actual set or if it is a just a try or if the
> device refused the operation.
>
> We should only keep the file handle if the device actually accepted
> applying the operation.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")
> Suggested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
Thank you, nice patch, better then my original suggestion :)
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
I'll let this sit on the list to give others a chance to reply
and if there are no remarks I'll merge this next Monday.
Regards,
Hans
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 4fe26e82e3d1..9a80a7d8e73a 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1811,7 +1811,10 @@ 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_control **err_ctrl)
> + struct uvc_fh *handle,
> + struct uvc_entity *entity,
> + int rollback,
> + struct uvc_control **err_ctrl)
> {
> struct uvc_control *ctrl;
> unsigned int i;
> @@ -1859,6 +1862,10 @@ static int uvc_ctrl_commit_entity(struct uvc_device *dev,
> *err_ctrl = ctrl;
> return ret;
> }
> +
> + if (!rollback && handle &&
> + ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
> + ctrl->handle = handle;
> }
>
> return 0;
> @@ -1895,8 +1902,8 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
>
> /* Find the control. */
> list_for_each_entry(entity, &chain->entities, chain) {
> - ret = uvc_ctrl_commit_entity(chain->dev, entity, rollback,
> - &err_ctrl);
> + ret = uvc_ctrl_commit_entity(chain->dev, handle, entity,
> + rollback, &err_ctrl);
> if (ret < 0) {
> if (ctrls)
> ctrls->error_idx =
> @@ -2046,9 +2053,6 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> mapping->set(mapping, value,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
>
> - if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
> - ctrl->handle = handle;
> -
> ctrl->dirty = 1;
> ctrl->modified = 1;
> return 0;
> @@ -2377,7 +2381,7 @@ int uvc_ctrl_restore_values(struct uvc_device *dev)
> ctrl->dirty = 1;
> }
>
> - ret = uvc_ctrl_commit_entity(dev, entity, 0, NULL);
> + ret = uvc_ctrl_commit_entity(dev, NULL, entity, 0, NULL);
> if (ret < 0)
> return ret;
> }
>
