| Message ID | 20231027201959.1869181-3-arakesh@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v9,1/4] usb: gadget: uvc: prevent use of disabled endpoint | expand |
Hi Avichal On 27/10/2023 21:19, Avichal Rakesh wrote: > This patch refactors the video disable logic in uvcg_video_enable > into its own separate function 'uvcg_video_disable'. This function > is now used anywhere uvcg_video_enable(video, 0) was used. > > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> For this patch you can keep the R-b - it's fine by me now :) > Suggested-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > Signed-off-by: Avichal Rakesh <arakesh@google.com> > --- > v6: Introduced this patch to make the next one easier to review > v6 -> v7: Add Suggested-by > v7 -> v8: No change. Getting back in review queue > v8 -> v9: Call uvcg_video_disable directly instead of uvcg_video_enable(video, 0) > > drivers/usb/gadget/function/uvc_v4l2.c | 6 ++-- > drivers/usb/gadget/function/uvc_video.c | 40 ++++++++++++++++--------- > drivers/usb/gadget/function/uvc_video.h | 3 +- > 3 files changed, 31 insertions(+), 18 deletions(-) > > diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c > index 7cb8d027ff0c..904dd283cbf7 100644 > --- a/drivers/usb/gadget/function/uvc_v4l2.c > +++ b/drivers/usb/gadget/function/uvc_v4l2.c > @@ -443,7 +443,7 @@ uvc_v4l2_streamon(struct file *file, void *fh, enum v4l2_buf_type type) > return -EINVAL; > > /* Enable UVC video. */ > - ret = uvcg_video_enable(video, 1); > + ret = uvcg_video_enable(video); > if (ret < 0) > return ret; > > @@ -469,7 +469,7 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) > return -EINVAL; > > uvc->state = UVC_STATE_CONNECTED; > - ret = uvcg_video_enable(video, 0); > + ret = uvcg_video_disable(video); > if (ret < 0) > return ret; > > @@ -515,7 +515,7 @@ static void uvc_v4l2_disable(struct uvc_device *uvc) > if (uvc->state == UVC_STATE_STREAMING) > uvc->state = UVC_STATE_CONNECTED; > > - uvcg_video_enable(&uvc->video, 0); > + uvcg_video_disable(&uvc->video); > uvcg_free_buffers(&uvc->video.queue); > uvc->func_connected = false; > wake_up_interruptible(&uvc->func_connected_queue); > diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c > index f8f9209fee50..1081dd790fd6 100644 > --- a/drivers/usb/gadget/function/uvc_video.c > +++ b/drivers/usb/gadget/function/uvc_video.c > @@ -494,31 +494,43 @@ static void uvcg_video_pump(struct work_struct *work) > } > > /* > - * Enable or disable the video stream. > + * Disable the video stream > */ > -int uvcg_video_enable(struct uvc_video *video, int enable) > +int > +uvcg_video_disable(struct uvc_video *video) > { > - int ret; > struct uvc_request *ureq; > > if (video->ep == NULL) { > uvcg_info(&video->uvc->func, > - "Video enable failed, device is uninitialized.\n"); > + "Video disable failed, device is uninitialized.\n"); > return -ENODEV; > } > > - if (!enable) { > - cancel_work_sync(&video->pump); > - uvcg_queue_cancel(&video->queue, 0); > + cancel_work_sync(&video->pump); > + uvcg_queue_cancel(&video->queue, 0); > > - list_for_each_entry(ureq, &video->ureqs, list) { > - if (ureq->req) > - usb_ep_dequeue(video->ep, ureq->req); > - } > + list_for_each_entry(ureq, &video->ureqs, list) { > + if (ureq->req) > + usb_ep_dequeue(video->ep, ureq->req); > + } > > - uvc_video_free_requests(video); > - uvcg_queue_enable(&video->queue, 0); > - return 0; > + uvc_video_free_requests(video); > + uvcg_queue_enable(&video->queue, 0); > + return 0; > +} > + > +/* > + * Enable the video stream. > + */ > +int uvcg_video_enable(struct uvc_video *video) > +{ > + int ret; > + > + if (video->ep == NULL) { > + uvcg_info(&video->uvc->func, > + "Video enable failed, device is uninitialized.\n"); > + return -ENODEV; > } > > if ((ret = uvcg_queue_enable(&video->queue, 1)) < 0) > diff --git a/drivers/usb/gadget/function/uvc_video.h b/drivers/usb/gadget/function/uvc_video.h > index 03adeefa343b..8ef6259741f1 100644 > --- a/drivers/usb/gadget/function/uvc_video.h > +++ b/drivers/usb/gadget/function/uvc_video.h > @@ -14,7 +14,8 @@ > > struct uvc_video; > > -int uvcg_video_enable(struct uvc_video *video, int enable); > +int uvcg_video_enable(struct uvc_video *video); > +int uvcg_video_disable(struct uvc_video *video); > > int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc); > > -- > 2.42.0.820.g83a721a137-goog
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 7cb8d027ff0c..904dd283cbf7 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -443,7 +443,7 @@ uvc_v4l2_streamon(struct file *file, void *fh, enum v4l2_buf_type type) return -EINVAL; /* Enable UVC video. */ - ret = uvcg_video_enable(video, 1); + ret = uvcg_video_enable(video); if (ret < 0) return ret; @@ -469,7 +469,7 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) return -EINVAL; uvc->state = UVC_STATE_CONNECTED; - ret = uvcg_video_enable(video, 0); + ret = uvcg_video_disable(video); if (ret < 0) return ret; @@ -515,7 +515,7 @@ static void uvc_v4l2_disable(struct uvc_device *uvc) if (uvc->state == UVC_STATE_STREAMING) uvc->state = UVC_STATE_CONNECTED; - uvcg_video_enable(&uvc->video, 0); + uvcg_video_disable(&uvc->video); uvcg_free_buffers(&uvc->video.queue); uvc->func_connected = false; wake_up_interruptible(&uvc->func_connected_queue); diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index f8f9209fee50..1081dd790fd6 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -494,31 +494,43 @@ static void uvcg_video_pump(struct work_struct *work) } /* - * Enable or disable the video stream. + * Disable the video stream */ -int uvcg_video_enable(struct uvc_video *video, int enable) +int +uvcg_video_disable(struct uvc_video *video) { - int ret; struct uvc_request *ureq; if (video->ep == NULL) { uvcg_info(&video->uvc->func, - "Video enable failed, device is uninitialized.\n"); + "Video disable failed, device is uninitialized.\n"); return -ENODEV; } - if (!enable) { - cancel_work_sync(&video->pump); - uvcg_queue_cancel(&video->queue, 0); + cancel_work_sync(&video->pump); + uvcg_queue_cancel(&video->queue, 0); - list_for_each_entry(ureq, &video->ureqs, list) { - if (ureq->req) - usb_ep_dequeue(video->ep, ureq->req); - } + list_for_each_entry(ureq, &video->ureqs, list) { + if (ureq->req) + usb_ep_dequeue(video->ep, ureq->req); + } - uvc_video_free_requests(video); - uvcg_queue_enable(&video->queue, 0); - return 0; + uvc_video_free_requests(video); + uvcg_queue_enable(&video->queue, 0); + return 0; +} + +/* + * Enable the video stream. + */ +int uvcg_video_enable(struct uvc_video *video) +{ + int ret; + + if (video->ep == NULL) { + uvcg_info(&video->uvc->func, + "Video enable failed, device is uninitialized.\n"); + return -ENODEV; } if ((ret = uvcg_queue_enable(&video->queue, 1)) < 0) diff --git a/drivers/usb/gadget/function/uvc_video.h b/drivers/usb/gadget/function/uvc_video.h index 03adeefa343b..8ef6259741f1 100644 --- a/drivers/usb/gadget/function/uvc_video.h +++ b/drivers/usb/gadget/function/uvc_video.h @@ -14,7 +14,8 @@ struct uvc_video; -int uvcg_video_enable(struct uvc_video *video, int enable); +int uvcg_video_enable(struct uvc_video *video); +int uvcg_video_disable(struct uvc_video *video); int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc);