Re: [PATCH 1/2] usb: gadget: uvc: clean and rename uvcg_queue_next_buffer to uvcg_complete_buffer

[paul.elder@ideasonboard.com]

Hi Michael,

Thanks for the patch.

On Mon, Feb 28, 2022 at 03:16:58PM +0100, Michael Grzeschik wrote:
> The functions purpose is different to its name. We change the function
> name and remove all unused code.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/usb/gadget/function/uvc_queue.c | 18 +-----------------
>  drivers/usb/gadget/function/uvc_queue.h |  2 +-
>  drivers/usb/gadget/function/uvc_video.c |  6 +++---
>  3 files changed, 5 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
> index d852ac9e47e72c..73ff56043d2e6a 100644
> --- a/drivers/usb/gadget/function/uvc_queue.c
> +++ b/drivers/usb/gadget/function/uvc_queue.c
> @@ -326,24 +326,10 @@ int uvcg_queue_enable(struct uvc_video_queue *queue, int enable)
>  }
>  
>  /* called with &queue_irqlock held.. */
> -struct uvc_buffer *uvcg_queue_next_buffer(struct uvc_video_queue *queue,
> +void uvcg_complete_buffer(struct uvc_video_queue *queue,
>  					  struct uvc_buffer *buf)
>  {
> -	struct uvc_buffer *nextbuf;
> -
> -	if ((queue->flags & UVC_QUEUE_DROP_INCOMPLETE) &&
> -	     buf->length != buf->bytesused) {
> -		buf->state = UVC_BUF_STATE_QUEUED;
> -		vb2_set_plane_payload(&buf->buf.vb2_buf, 0, 0);
> -		return buf;
> -	}
> -
>  	list_del(&buf->queue);
> -	if (!list_empty(&queue->irqqueue))
> -		nextbuf = list_first_entry(&queue->irqqueue, struct uvc_buffer,
> -					   queue);
> -	else
> -		nextbuf = NULL;

Is it fine to drop these? They look important to me. If they're not,
then the reason should be explained in the commit message.

>  
>  	buf->buf.field = V4L2_FIELD_NONE;
>  	buf->buf.sequence = queue->sequence++;
> @@ -351,8 +337,6 @@ struct uvc_buffer *uvcg_queue_next_buffer(struct uvc_video_queue *queue,
>  
>  	vb2_set_plane_payload(&buf->buf.vb2_buf, 0, buf->bytesused);
>  	vb2_buffer_done(&buf->buf.vb2_buf, VB2_BUF_STATE_DONE);
> -
> -	return nextbuf;

This looks fine since all callers ignore it anyway.


Paul

>  }
>  
>  struct uvc_buffer *uvcg_queue_head(struct uvc_video_queue *queue)
> diff --git a/drivers/usb/gadget/function/uvc_queue.h b/drivers/usb/gadget/function/uvc_queue.h
> index 05360a0767f61f..b668927b5d2c4e 100644
> --- a/drivers/usb/gadget/function/uvc_queue.h
> +++ b/drivers/usb/gadget/function/uvc_queue.h
> @@ -93,7 +93,7 @@ void uvcg_queue_cancel(struct uvc_video_queue *queue, int disconnect);
>  
>  int uvcg_queue_enable(struct uvc_video_queue *queue, int enable);
>  
> -struct uvc_buffer *uvcg_queue_next_buffer(struct uvc_video_queue *queue,
> +void uvcg_complete_buffer(struct uvc_video_queue *queue,
>  					  struct uvc_buffer *buf);
>  
>  struct uvc_buffer *uvcg_queue_head(struct uvc_video_queue *queue);
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index 7f59a0c4740209..0c8d146b840321 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -112,7 +112,7 @@ uvc_video_encode_bulk(struct usb_request *req, struct uvc_video *video,
>  	if (buf->bytesused == video->queue.buf_used) {
>  		video->queue.buf_used = 0;
>  		buf->state = UVC_BUF_STATE_DONE;
> -		uvcg_queue_next_buffer(&video->queue, buf);
> +		uvcg_complete_buffer(&video->queue, buf);
>  		video->fid ^= UVC_STREAM_FID;
>  
>  		video->payload_size = 0;
> @@ -183,7 +183,7 @@ uvc_video_encode_isoc_sg(struct usb_request *req, struct uvc_video *video,
>  		video->queue.buf_used = 0;
>  		buf->state = UVC_BUF_STATE_DONE;
>  		buf->offset = 0;
> -		uvcg_queue_next_buffer(&video->queue, buf);
> +		uvcg_complete_buffer(&video->queue, buf);
>  		video->fid ^= UVC_STREAM_FID;
>  	}
>  }
> @@ -210,7 +210,7 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
>  	if (buf->bytesused == video->queue.buf_used) {
>  		video->queue.buf_used = 0;
>  		buf->state = UVC_BUF_STATE_DONE;
> -		uvcg_queue_next_buffer(&video->queue, buf);
> +		uvcg_complete_buffer(&video->queue, buf);
>  		video->fid ^= UVC_STREAM_FID;
>  	}
>  }
> -- 
> 2.30.2
>