| Message ID | 20241202-uvc-fix-async-v5-5-6658c1fe312b@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,1/5] media: uvcvideo: Only save async fh if success | expand |
On Mon, Dec 02, 2024 at 03:45:57PM +0100, Hans de Goede wrote: > Hi, > > On 2-Dec-24 3:24 PM, Ricardo Ribalda wrote: > > Asynchronous controls trigger an event when they have completed their > > operation. > > > > This can make that the control cached value does not match the value in > > the device. > > > > Let's flush the cache to be on the safe side. > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > drivers/media/usb/uvc/uvc_ctrl.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > > index e90bf2aeb5e4..75d534072f50 100644 > > --- a/drivers/media/usb/uvc/uvc_ctrl.c > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > > @@ -1672,6 +1672,9 @@ bool uvc_ctrl_status_event_async(struct urb *urb, struct uvc_video_chain *chain, > > struct uvc_device *dev = chain->dev; > > struct uvc_ctrl_work *w = &dev->async_ctrl; > > > > + /* Flush the control cache, the data might have changed. */ > > + ctrl->loaded = 0; > > + > > if (list_empty(&ctrl->info.mappings)) > > return false; > > > > Thank you for the patch. > > I'm not familiar enough with UVC yet to really have an opinion on this one, > so I'll defer reviewing this one to Laurent. Conceptually this change looks fine, but the commit message needs to explain why this is safe to do without protecting ctrl->loaded with a lock.
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index e90bf2aeb5e4..75d534072f50 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1672,6 +1672,9 @@ bool uvc_ctrl_status_event_async(struct urb *urb, struct uvc_video_chain *chain, struct uvc_device *dev = chain->dev; struct uvc_ctrl_work *w = &dev->async_ctrl; + /* Flush the control cache, the data might have changed. */ + ctrl->loaded = 0; + if (list_empty(&ctrl->info.mappings)) return false;
Asynchronous controls trigger an event when they have completed their operation. This can make that the control cached value does not match the value in the device. Let's flush the cache to be on the safe side. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_ctrl.c | 3 +++ 1 file changed, 3 insertions(+)