On Wed, Nov 12, 2008 at 02:38:02PM -0500, Alan Stern wrote:
> On Wed, 12 Nov 2008, Anton Vorontsov wrote:
>
> > Gadgets disable endpoints in their disconnect callbacks, so
> > we must call disconnect before unbinding. This also fixes
> > muram memory leak, since we free muram in the qe_ep_disable().
>
> > --- a/drivers/usb/gadget/fsl_qe_udc.c
> > +++ b/drivers/usb/gadget/fsl_qe_udc.c
> > @@ -2382,6 +2382,9 @@ int usb_gadget_unregister_driver(struct
> > usb_gadget_driver *driver)
> > nuke(loop_ep, -ESHUTDOWN);
> > spin_unlock_irqrestore(&udc_controller->lock, flags);
> >
> > + /* report disconnect; the driver is already quiesced */
> > + driver->disconnect(&udc_controller->gadget);
> > +
> > /* unbind gadget and unhook driver. */
> > driver->unbind(&udc_controller->gadget);
> > udc_controller->gadget.dev.driver = NULL;
>
> Wouldn't it be better to do this before nuking the existing requests?
> The comment is wrong; the gadget driver is _not_ quiesced at this
> point. In fact the disconnect call is what quiesces the driver!
composite_unbind() says:
/* composite_disconnect() must already have been called
* by the underlying peripheral controller driver!
* so there's no i/o concurrency that could affect the
* state protected by cdev->lock.
*/
Which I read as "at disconnect time the controller should
already be disabled, no further i/o can happen". Which means
that we should nuke all pending requests and stop the
controller.
In this comment:
"/* report disconnect; the driver is already quiesced */"
"the driver" means "the udc driver", not the gadget driver.
FWIW, the PXA27x UDC controller also stops all activity and
completely disables the controller before calling the
disconnect().
> And wouldn't it be better to _skip_ doing this if the gadget wasn't
> connected before?
Composite framework handles this. If there were no connections,
then the disconnect() is a nop (except the spin lock/unlock pair).
I'm not sure how the controller driver could tell if there
was a connection or not: it doesn't operate these terms.
What the udc controller knows is: how to report bus reset
and how to receive or transmit the data...
Thanks,
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
