On Wed, 18 Apr 2012 14:19:13 +0300, Felipe Balbi <balbi@xxxxxx> wrote:
> On Wed, Apr 18, 2012 at 02:12:01PM +0300, Alexander Shishkin wrote:
> > On Wed, 18 Apr 2012 13:48:40 +0300, Felipe Balbi <balbi@xxxxxx> wrote:
> > > On Tue, Apr 17, 2012 at 05:02:30PM +0300, Alexander Shishkin wrote:
> > > > Currently, on gadget removal path, dummy_pullup() gets called after
> > > > gadget's disconnect(). Now, dummy_pullup() decides that it needs to
> > > > disconnect again, resulting in a crash. IOW, rmmod g_zero will crash
> > > > with dummy_hcd.
> > >
> > > Isn't the fix then simply:
> >
> > Nope. It never reaches dummy_udc_stop(), because dummy_pullup() that
> > crashes the kernel is called before it.
>
> there's another error on that driver, it should not be calling
> ->disconnect() directly. udc-core already did it:
>
> diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
> index 170cbe8..7ca9b75 100644
> --- a/drivers/usb/gadget/dummy_hcd.c
> +++ b/drivers/usb/gadget/dummy_hcd.c
> @@ -373,12 +373,8 @@ static void set_link_state(struct dummy_hcd *dum_hcd)
> */
> if ((dum_hcd->old_status & USB_PORT_STAT_CONNECTION) != 0 &&
> (dum_hcd->old_status & USB_PORT_STAT_RESET) == 0 &&
> - dum->driver) {
> + dum->driver)
> stop_activity(dum);
> - spin_unlock(&dum->lock);
> - dum->driver->disconnect(&dum->gadget);
> - spin_lock(&dum->lock);
> - }
> } else if (dum_hcd->active != dum_hcd->old_active) {
> if (dum_hcd->old_active && dum->driver->suspend) {
> spin_unlock(&dum->lock);
>
> so there are two bugs on dummy_hcd, does this fix the issue ? The only
This one does.
> problem is that now I'm not so sure this will work in all cases.
> ->disconnect() should be called based on a disconnect IRQ, but dummy
> fakes a disconnect IRQ.
>
> Alan, maybe you're more aware of how dummy_hcd works, but can we get a
> disconnect event initiated by the dummy_hcd instead of the dummy_udc ?
>
> --
> balbi
--
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
