On Thu, 19 Apr 2012, Alexander Shishkin wrote:
> > You'd be right except for one thing: You listed the code from
> > udc-core.c:usb_gadget_remove_driver() wrongly. In my tree it actually
> > looks like this:
> >
> > if (udc_is_newstyle(udc)) {
> > udc->driver->disconnect(udc->gadget);
> > udc->driver->unbind(udc->gadget);
> > usb_gadget_udc_stop(udc->gadget, udc->driver);
> > usb_gadget_disconnect(udc->gadget);
> > } else {
> > usb_gadget_stop(udc->gadget, udc->driver);
> > }
>
> I'm using linux-next, and as of today, the call order is still like I
> listed before. The change that introduced this order is 8ae8090c (usb:
> gadget: udc-core: fix asymmetric calls in remove_driver), which is also
> present in "usb-linus" branch of linux-usb tree (that's how, I assume
> it's got to linux-next) and Felipe's "fixes" and "master".
>
> So no, I didn't make this up. ;)
I'm sorry; you're right. I misread the commit.
> The change seems logical though: udc_stop() may have disabled the usb
> controller, and calling pullup() after that doesn't look very useful.
Agreed.
On Thu, 19 Apr 2012, Felipe Balbi wrote:
> I still think your original patch is correct. Alan, is your tree really
> clean ?
Yes, it is clean but it is a little out of date -- it hasn't been
updated for a little over a week. However the original patch is still
wrong.
With the new code, it seems clear that pullup() still gets called at
the wrong time. usb_gadget_remove_driver() now does this:
udc->driver->disconnect(udc->gadget);
udc->driver->unbind(udc->gadget);
usb_gadget_disconnect(udc->gadget);
usb_gadget_udc_stop(udc->gadget, udc->driver);
and of course, usb_gadget_disconnect() is what calls pullup(). As you
can see, the pullup() call occurs at a time when the gadget driver
believes it has been unbound but the udc driver believes it is still
bound. Hence the breakage.
Therefore the correct fix is to move the call up a line, so that it
looks like:
udc->driver->disconnect(udc->gadget);
usb_gadget_disconnect(udc->gadget);
udc->driver->unbind(udc->gadget);
usb_gadget_udc_stop(udc->gadget, udc->driver);
Alexander, does that fix the problem?
Alan Stern
--
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
