On Wednesday 04 February 2009, Sergei Shtylyov wrote:
> @@ -2141,13 +2142,22 @@ musb_h_disable(struct usb_hcd *hcd, stru
>  
>                 /* cleanup */
>                 musb_cleanup_urb(urb, qh, urb->pipe & USB_DIR_IN);
> -       } else
> -               urb = NULL;
> -
> -       /* then just nuke all the others */
> -       list_for_each_entry_safe_from(urb, tmp, &hep->urb_list, urb_list)
> -               musb_giveback(qh, urb, -ESHUTDOWN);
>  
> +               /* Then just nuke all the others */
> +               while (!list_empty(&hep->urb_list)) {
> +                       urb = next_urb(qh);
> +                       urb->status = -ESHUTDOWN;
> +                       musb_advance_schedule(musb, urb, qh->hw_ep, is_in);
> +               }
> +       } else {
> +               while (!list_empty(&hep->urb_list))
> +                       __musb_giveback(musb, next_urb(qh), -ESHUTDOWN);

Why does this have two different code paths for the
"unlink everything that the hardware doesn't have its
mittens on" branch?

It certainly *looks* goofy this way, and if it's not
buggy today then it'll probably grow some in the future.

Could you update this to settle on a single while()
loop?  I'd suggest the "else" branch as being simpler.

Then this patch should be suitable for 2.6.29-rc, as
a fix for an infrequent oops.

- Dave

  

> +
> +               hep->hcpriv = NULL;
> +               list_del(&qh->ring);
> +               kfree(qh);
> +       }
> +exit:
>         spin_unlock_irqrestore(&musb->lock, flags);
>  }
>  
> 



_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to