Re: [PATCH] usb: gadget/uvc: Remove non-required locking from 'uvc_queue_next_buffer' routine

[Sergei Shtylyov <sshtylyov@mvista.com>]

Hello.

On 22-03-2012 8:50, Bhupesh Sharma wrote:

> This patch removes the non-required spinlock acquire/release calls on
> 'queue_irqlock' from 'uvc_queue_next_buffer' routine.

    'queue->irqlock' maybe?

> This routine is called from 'video->encode' function (which translates to either
> 'uvc_video_encode_bulk' or 'uvc_video_encode_isoc') in 'uvc_video.c'.
> As, the 'video->encode' routines are called with 'queue_irqlock' already held,
> so acquiring a 'queue_irqlock' again in 'uvc_queue_next_buffer' routine causes
> a spin lock recursion.

> Signed-off-by: Bhupesh Sharma<bhupesh.sharma@st.com>
> Acked-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> ---
>   drivers/usb/gadget/uvc_queue.c |    4 +---
>   1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
> index d776adb..104ae9c 100644
> --- a/drivers/usb/gadget/uvc_queue.c
> +++ b/drivers/usb/gadget/uvc_queue.c
> @@ -543,11 +543,11 @@ done:
>   	return ret;
>   }
>
> +/* called with &queue_irqlock held.. */

    'queue->irqlock' maybe?

> @@ -556,14 +556,12 @@ uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer *buf)
>   		return buf;
>   	}
>
> -	spin_lock_irqsave(&queue->irqlock, flags);
>   	list_del(&buf->queue);
>   	if (!list_empty(&queue->irqqueue))
>   		nextbuf = list_first_entry(&queue->irqqueue, struct uvc_buffer,
>   					   queue);
>   	else
>   		nextbuf = NULL;
> -	spin_unlock_irqrestore(&queue->irqlock, flags);
>
>   	buf->buf.sequence = queue->sequence++;
>   	do_gettimeofday(&buf->buf.timestamp);

WBR, Sergei