David:
My last patch for usbcore concerns usb_device_remove() in usb.c. The most
annoying things about it is its name. The routine doesn't remove USB
devices; it disconnects drivers from interfaces. So I renamed it
usb_remove_driver(). Luckily it's not used in many places.
The real problem is that it calls nuke_urbs() for the device containing
the interface it's given. That is clearly a mistake, since nuke_urbs()
gets rid of all URBs for the device, not just the one interface. I wrote
another helper function, disable_interface(), which does the same thing as
nuke_urbs() but only for endpoints belonging to a single interface.
The other half of the problem comes after the driver has been
disconnected. The endpoints will remain disabled, so the next driver that
tries to bind the interface is out of luck! I added enable_interface(),
which undoes the effects of disable_interface().
These bugs account for the problem I observed in usbtest, where the first
URB would fail upon submission. This happened after I rmmod'ed the
usbtest driver and then modprobe'd it back. Endpoint 0 was disabled
during the removal and never re-enabled.
Disabling and enabling interfaces would be a lot easier and more reliable
if struct urb contained a usb_interface field instead of (or in addition
to) its usb_device field. But that would be an immense change, something
for 2.7.
Alan Stern
===== usb.h 1.149 vs edited =====
--- 1.149/include/linux/usb.h Tue Jul 15 16:54:50 2003
+++ edited/include/linux/usb.h Mon Jul 21 11:36:08 2003
@@ -485,7 +485,7 @@
struct usb_class_driver *class_driver);
extern int usb_device_probe(struct device *dev);
-extern int usb_device_remove(struct device *dev);
+extern int usb_remove_driver(struct device *dev);
extern int usb_disabled(void);
/* -------------------------------------------------------------------------- */
===== devio.c 1.77 vs edited =====
--- 1.77/drivers/usb/core/devio.c Wed Jun 11 12:16:43 2003
+++ edited/drivers/usb/core/devio.c Mon Jul 21 11:37:04 2003
@@ -1105,7 +1105,7 @@
if (driver) {
dbg ("disconnect '%s' from dev %d interface %d",
driver->name, ps->dev->devnum, ctrl.ifno);
- usb_device_remove(&ifp->dev);
+ usb_remove_driver(&ifp->dev);
} else
retval = -ENODATA;
unlock_kernel();
===== usb.c 1.213 vs edited =====
--- 1.213/drivers/usb/core/usb.c Tue Jul 15 16:58:31 2003
+++ edited/drivers/usb/core/usb.c Mon Jul 21 13:41:18 2003
@@ -123,7 +123,52 @@
return error;
}
-int usb_device_remove(struct device *dev)
+/* deallocate hcd/hardware state and nuke all pending urbs for 1 interface */
+static void disable_interface(struct usb_interface *intf)
+{
+ struct usb_device *dev = interface_to_usbdev(intf);
+ struct usb_host_interface *hintf =
+ &intf->altsetting[intf->act_altsetting];
+ void (*disable)(struct usb_device *, int);
+ int i;
+
+ if (!dev || !dev->bus || !dev->bus->op || !dev->bus->op->disable)
+ return;
+ disable = dev->bus->op->disable;
+ dbg("disabling interface %s", intf->dev.bus_id);
+
+ for (i = 0; i < hintf->desc.bNumEndpoints; i++)
+ disable(dev, hintf->endpoint[i].desc.bEndpointAddress);
+}
+
+/* enable endpoints for 1 interface */
+static void enable_interface(struct usb_interface *intf)
+{
+ struct usb_device *dev = interface_to_usbdev(intf);
+ struct usb_host_interface *hintf =
+ &intf->altsetting[intf->act_altsetting];
+ int i;
+
+ if (!dev)
+ return;
+ dbg("enabling interface %s", intf->dev.bus_id);
+
+ for (i = 0; i < hintf->desc.bNumEndpoints; i++) {
+ int ep = hintf->endpoint[i].desc.bEndpointAddress;
+ int maxsize = hintf->endpoint[i].desc.wMaxPacketSize;
+ int epnum = ep & USB_ENDPOINT_NUMBER_MASK;
+
+ if (usb_endpoint_out(ep)) {
+ usb_endpoint_running(dev, epnum, 1);
+ dev->epmaxpacketout[epnum] = maxsize;
+ } else {
+ usb_endpoint_running(dev, epnum, 0);
+ dev->epmaxpacketin[epnum] = maxsize;
+ }
+ }
+}
+
+int usb_remove_driver(struct device *dev)
{
struct usb_interface *intf;
struct usb_driver *driver;
@@ -134,7 +179,7 @@
down(&driver->serialize);
/* release all urbs for this device */
- nuke_urbs(interface_to_usbdev(intf));
+ disable_interface(intf);
if (intf->driver && intf->driver->disconnect)
intf->driver->disconnect(intf);
@@ -145,6 +190,8 @@
up(&driver->serialize);
+ /* re-enable the endpoints for the next driver */
+ enable_interface(intf);
return 0;
}
@@ -171,7 +218,7 @@
new_driver->driver.name = (char *)new_driver->name;
new_driver->driver.bus = &usb_bus_type;
new_driver->driver.probe = usb_device_probe;
- new_driver->driver.remove = usb_device_remove;
+ new_driver->driver.remove = usb_remove_driver;
init_MUTEX(&new_driver->serialize);
@@ -1592,7 +1639,7 @@
EXPORT_SYMBOL(usb_disabled);
EXPORT_SYMBOL(usb_device_probe);
-EXPORT_SYMBOL(usb_device_remove);
+EXPORT_SYMBOL(usb_remove_driver);
EXPORT_SYMBOL(usb_alloc_dev);
EXPORT_SYMBOL(usb_put_dev);
-------------------------------------------------------
This SF.net email is sponsored by: VM Ware
With VMware you can run multiple operating systems on a single machine.
WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the
same time. Free trial click here: http://www.vmware.com/wl/offer/345/0
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
