Re: [PATCH 1/3] usb: gadget: uvc: Factor out video USB request queueing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 18, 2018 at 01:35:30PM +0300, Laurent Pinchart wrote:
> USB requests for video data are queued from two different locations in
> the driver, with the same code block occurring twice. Factor it out to a
> function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

For the whole series:

Looks good to me.

Reviewed-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
Tested-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>

> ---
>  drivers/usb/gadget/function/uvc_video.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index d3567b90343a..a95c8e2364ed 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -125,6 +125,19 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
>   * Request handling
>   */
>  
> +static int uvcg_video_ep_queue(struct uvc_video *video, struct usb_request *req)
> +{
> +	int ret;
> +
> +	ret = usb_ep_queue(video->ep, req, GFP_ATOMIC);
> +	if (ret < 0) {
> +		printk(KERN_INFO "Failed to queue request (%d).\n", ret);
> +		usb_ep_set_halt(video->ep);
> +	}
> +
> +	return ret;
> +}
> +
>  /*
>   * I somehow feel that synchronisation won't be easy to achieve here. We have
>   * three events that control USB requests submission:
> @@ -189,14 +202,13 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
>  
>  	video->encode(req, video, buf);
>  
> -	if ((ret = usb_ep_queue(ep, req, GFP_ATOMIC)) < 0) {
> -		printk(KERN_INFO "Failed to queue request (%d).\n", ret);
> -		usb_ep_set_halt(ep);
> -		spin_unlock_irqrestore(&video->queue.irqlock, flags);
> +	ret = uvcg_video_ep_queue(video, req);
> +	spin_unlock_irqrestore(&video->queue.irqlock, flags);
> +
> +	if (ret < 0) {
>  		uvcg_queue_cancel(queue, 0);
>  		goto requeue;
>  	}
> -	spin_unlock_irqrestore(&video->queue.irqlock, flags);
>  
>  	return;
>  
> @@ -316,15 +328,13 @@ int uvcg_video_pump(struct uvc_video *video)
>  		video->encode(req, video, buf);
>  
>  		/* Queue the USB request */
> -		ret = usb_ep_queue(video->ep, req, GFP_ATOMIC);
> +		ret = uvcg_video_ep_queue(video, req);
> +		spin_unlock_irqrestore(&queue->irqlock, flags);
> +
>  		if (ret < 0) {
> -			printk(KERN_INFO "Failed to queue request (%d)\n", ret);
> -			usb_ep_set_halt(video->ep);
> -			spin_unlock_irqrestore(&queue->irqlock, flags);
>  			uvcg_queue_cancel(queue, 0);
>  			break;
>  		}
> -		spin_unlock_irqrestore(&queue->irqlock, flags);
>  	}
>  
>  	spin_lock_irqsave(&video->req_lock, flags);
> -- 
> Regards,
> 
> Laurent Pinchart
> 



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux