On Thu, 14 Mar 2013, Felipe Balbi wrote:
> Here's the diff which needs testing:
Can't test the net2272 part.
> diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
> index 691cc65..d23c1b8 100644
> --- a/drivers/usb/gadget/net2280.c
> +++ b/drivers/usb/gadget/net2280.c
> @@ -1941,6 +1941,13 @@ stop_activity (struct net2280 *dev, struct usb_gadget_driver *driver)
> for (i = 0; i < 7; i++)
> nuke (&dev->ep [i]);
>
> + /* report disconnect; the driver is already quiesced */
> + if (driver) {
> + spin_unlock (&dev->lock);
> + driver->disconnect (&dev->gadget);
> + spin_lock (&dev->lock);
> + }
> +
> usb_reinit (dev);
> }
Adding this back in revealed a bug in udc-core. It is fixed as
follows:
Index: usb-3.9/drivers/usb/gadget/udc-core.c
===================================================================
--- usb-3.9.orig/drivers/usb/gadget/udc-core.c
+++ usb-3.9/drivers/usb/gadget/udc-core.c
@@ -216,7 +216,7 @@ static void usb_gadget_remove_driver(str
usb_gadget_disconnect(udc->gadget);
udc->driver->disconnect(udc->gadget);
udc->driver->unbind(udc->gadget);
- usb_gadget_udc_stop(udc->gadget, udc->driver);
+ usb_gadget_udc_stop(udc->gadget, NULL);
udc->driver = NULL;
udc->dev.driver = NULL;
At the point where the UDC driver's stop routine is called, the gadget
no longer has a driver. You might even want to move the line I changed
down below the following two assignments, to make this more clear.
By the way, what happens if the UDC hardware doesn't support software
control of the D+ pullup? The usb_gadget_disconnect call above won't
do anything, and that leaves open a window for a race. The host might
send a SETUP packet between the unbind and stop calls. Should we
insist that all UDC drivers do have a working ->pullup routine?
In addition, code-reading revealed a bug in an error pathway of
net2280.c:
Index: usb-3.9/drivers/usb/gadget/net2280.c
===================================================================
--- usb-3.9.orig/drivers/usb/gadget/net2280.c
+++ usb-3.9/drivers/usb/gadget/net2280.c
@@ -1924,7 +1924,6 @@ static int net2280_start(struct usb_gadg
err_func:
device_remove_file (&dev->pdev->dev, &dev_attr_function);
err_unbind:
- driver->unbind (&dev->gadget);
dev->gadget.dev.driver = NULL;
dev->driver = NULL;
return retval;
If the start method fails, udc_bind_to_driver() calls driver->unbind.
The UDC driver doesn't need to do it. I have not audited the other
gadget drivers for similar problems (i.e., unnecessary cleanup calls in
error pathways); it might be worth taking a quick look. net2272 at
least probably has the same bug.
With these two fixes applied, everything seemed to work okay. Should I
submit them officially?
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
