On Wednesday 04 February 2009, Sergei Shtylyov wrote:
> The urb_dequeue() method "forgets" to unlink 'struct musb_qh' from the control/
> bulk schedule list when an URB being cancelled is the only one queued to its
> endpoint, which will cause musb_advance_schedule() to "block" once it reaches
> 'struct musb_qh' with now empty URB list, and so URBs queued for other control/
> bulk endpoints after the one being dequeued will not be served. Hence add code
> to unlink and free 'struct musb_qh' if its URB list emptied to this method and
> remove now useless check from musb_advance_schedule().
>
> The only exception case is when we're called from the driver's callback --
> musb_giveback() doesn't expect us to pull out 'qh' from under it (it'll kill
> this qh later anyway) and we want to keep around a qh with is_ready flag clear
> because some drivers like usbaudio.c do submit a new URB after cancelling all
> pending ones which leads to URB being started when it shouldn't be and started
> all over again after the control returns to musb_giveback()...
>
> Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
I'm forwarding a slightly tweaked (slimmed-down, improved
patch comment) version to Greg for the 2.6.29-rc series.
Thanks.
>
> ---
> The patch is against the recent Linus' kernel...
>
> drivers/usb/musb/musb_host.c | 16 ++++++++++++++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/usb/musb/musb_host.c
> ===================================================================
> --- linux-2.6.orig/drivers/usb/musb/musb_host.c
> +++ linux-2.6/drivers/usb/musb/musb_host.c
> @@ -431,7 +431,7 @@ musb_advance_schedule(struct musb *musb,
> else
> qh = musb_giveback(qh, urb, urb->status);
>
> - if (qh && qh->is_ready && !list_empty(&qh->hep->urb_list)) {
> + if (qh != NULL && qh->is_ready) {
> DBG(4, "... next ep%d %cX urb %p\n",
> hw_ep->epnum, is_in ? 'R' : 'T',
> next_urb(qh));
> @@ -2071,7 +2071,19 @@ static int musb_urb_dequeue(struct usb_h
> ret = 0;
> qh->is_ready = 0;
> __musb_giveback(musb, urb, 0);
> - qh->is_ready = ready;
> +
> + /*
> + * If the URB list has emptied, it's time to kill this qh.
> + * However, if this was called from the driver callback,
> + * we don't really want to do it now, deferring it until
> + * we return to musb_giveback(), or bad things will happen...
> + */
> + if (ready && list_empty(&qh->hep->urb_list)) {
> + qh->hep->hcpriv = NULL;
> + list_del(&qh->ring);
> + kfree(qh);
> + } else
> + qh->is_ready = ready;
> } else
> ret = musb_cleanup_urb(urb, qh, urb->pipe & USB_DIR_IN);
> done:
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
