Greg:
This patch reintroduces the usb_lock_device_for_reset() routine, which is
specially tailored to meet the needs of drivers that have to reset a
device either during probe() or during normal operations. It updates a
few drivers that do device resets, to make them use the new routine. It
also adds a new field to struct usb_interface, to keep track of whether
the interface is in the process of being bound to or unbound from a
driver. This is necessary, because during binding we know the device is
already locked so we don't want to try to acquire the lock again!
With this patch in place, USB device resets should finally become
reliable. Please apply.
Alan Stern
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
diff -Nru a/drivers/net/irda/stir4200.c b/drivers/net/irda/stir4200.c
--- a/drivers/net/irda/stir4200.c Thu Aug 19 11:49:30 2004
+++ b/drivers/net/irda/stir4200.c Thu Aug 19 11:49:30 2004
@@ -168,6 +168,7 @@
struct stir_cb {
struct usb_device *usbdev; /* init: probe_irda */
+ struct usb_interface *usbintf;
struct net_device *netdev; /* network layer */
struct irlap_cb *irlap; /* The link layer we are binded to */
struct net_device_stats stats; /* network statistics */
@@ -508,6 +509,7 @@
{
int i, err;
__u8 mode;
+ int rc;
for (i = 0; i < ARRAY_SIZE(stir_modes); ++i) {
if (speed == stir_modes[i].speed)
@@ -521,7 +523,14 @@
pr_debug("speed change from %d to %d\n", stir->speed, speed);
/* sometimes needed to get chip out of stuck state */
+ rc = usb_lock_device_for_reset(stir->usbdev, stir->usbintf);
+ if (rc < 0) {
+ err = rc;
+ goto out;
+ }
err = usb_reset_device(stir->usbdev);
+ if (rc)
+ usb_unlock_device(stir->usbdev);
if (err)
goto out;
@@ -1066,6 +1075,7 @@
stir = net->priv;
stir->netdev = net;
stir->usbdev = dev;
+ stir->usbintf = intf;
ret = usb_reset_configuration(dev);
if (ret != 0) {
diff -Nru a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
--- a/drivers/usb/core/devio.c Thu Aug 19 11:49:30 2004
+++ b/drivers/usb/core/devio.c Thu Aug 19 11:49:30 2004
@@ -734,7 +734,7 @@
static int proc_resetdevice(struct dev_state *ps)
{
- return __usb_reset_device(ps->dev);
+ return usb_reset_device(ps->dev);
}
diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c Thu Aug 19 11:49:30 2004
+++ b/drivers/usb/core/hub.c Thu Aug 19 11:49:30 2004
@@ -814,7 +814,7 @@
else
return -1;
- if (__usb_reset_device(hdev))
+ if (usb_reset_device(hdev))
return -1;
hub->urb->dev = hdev;
@@ -2656,8 +2656,10 @@
*
* The caller must own the device lock. For example, it's safe to use
* this from a driver probe() routine after downloading new firmware.
+ * For calls that might not occur during probe(), drivers should lock
+ * the device using usb_lock_device_for_reset().
*/
-int __usb_reset_device(struct usb_device *udev)
+int usb_reset_device(struct usb_device *udev)
{
struct usb_device *parent = udev->parent;
struct usb_device_descriptor descriptor = udev->descriptor;
@@ -2692,6 +2694,11 @@
return -ENOENT;
}
+ /* ep0 maxpacket size may change; let the HCD know about it.
+ * Other endpoints will be handled by re-enumeration. */
+ usb_disable_endpoint(udev, 0);
+ usb_disable_endpoint(udev, 0 + USB_DIR_IN);
+
ret = hub_port_init(parent, udev, port);
if (ret < 0)
goto re_enumerate;
@@ -2744,16 +2751,4 @@
re_enumerate:
hub_port_logical_disconnect(parent, port);
return -ENODEV;
-}
-EXPORT_SYMBOL(__usb_reset_device);
-
-int usb_reset_device(struct usb_device *udev)
-{
- int r;
-
- down(&udev->serialize);
- r = __usb_reset_device(udev);
- up(&udev->serialize);
-
- return r;
}
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Thu Aug 19 11:49:30 2004
+++ b/drivers/usb/core/usb.c Thu Aug 19 11:49:30 2004
@@ -102,7 +102,10 @@
id = usb_match_id (intf, driver->id_table);
if (id) {
dev_dbg (dev, "%s - got id\n", __FUNCTION__);
+ intf->condition = USB_INTERFACE_BINDING;
error = driver->probe (intf, id);
+ intf->condition = error ? USB_INTERFACE_UNBOUND :
+ USB_INTERFACE_BOUND;
}
return error;
@@ -114,6 +117,8 @@
struct usb_interface *intf = to_usb_interface(dev);
struct usb_driver *driver = to_usb_driver(intf->dev.driver);
+ intf->condition = USB_INTERFACE_UNBINDING;
+
/* release all urbs for this interface */
usb_disable_interface(interface_to_usbdev(intf), intf);
@@ -125,6 +130,7 @@
intf->altsetting[0].desc.bInterfaceNumber,
0);
usb_set_intfdata(intf, NULL);
+ intf->condition = USB_INTERFACE_UNBOUND;
return 0;
}
@@ -324,6 +330,7 @@
dev->driver = &driver->driver;
usb_set_intfdata(iface, priv);
+ iface->condition = USB_INTERFACE_BOUND;
/* if interface was already added, bind now; else let
* the future device_add() bind it, bypassing probe()
@@ -363,6 +370,7 @@
dev->driver = NULL;
usb_set_intfdata(iface, NULL);
+ iface->condition = USB_INTERFACE_UNBOUND;
}
/**
@@ -911,6 +919,50 @@
}
/**
+ * usb_lock_device_for_reset - cautiously acquire the lock for a
+ * usb device structure
+ * @udev: device that's being locked
+ * @iface: interface bound to the driver making the request (optional)
+ *
+ * Attempts to acquire the device lock, but fails if the device is
+ * NOTATTACHED or SUSPENDED, or if iface is specified and the interface
+ * is neither BINDING nor BOUND. Rather than sleeping to wait for the
+ * lock, the routine polls repeatedly. This is to prevent deadlock with
+ * disconnect; in some drivers (such as usb-storage) the disconnect()
+ * callback will block waiting for a device reset to complete.
+ *
+ * Returns a negative error code for failure, otherwise 1 or 0 to indicate
+ * that the device will or will not have to be unlocked. (0 can be
+ * returned when an interface is given and is BINDING, because in that
+ * case the driver already owns the device lock.)
+ */
+int usb_lock_device_for_reset(struct usb_device *udev,
+ struct usb_interface *iface)
+{
+ if (udev->state == USB_STATE_NOTATTACHED)
+ return -ENODEV;
+ if (iface) {
+ switch (iface->condition) {
+ case USB_INTERFACE_BINDING:
+ return 0;
+ case USB_INTERFACE_BOUND:
+ break;
+ default:
+ return -EINTR;
+ }
+ }
+
+ while (!usb_trylock_device(udev)) {
+ msleep(15);
+ if (udev->state == USB_STATE_NOTATTACHED)
+ return -ENODEV;
+ if (iface && iface->condition != USB_INTERFACE_BOUND)
+ return -EINTR;
+ }
+ return 1;
+}
+
+/**
* usb_unlock_device - release the lock for a usb device structure
* @udev: device that's being unlocked
*
@@ -1493,6 +1545,7 @@
EXPORT_SYMBOL(usb_lock_device);
EXPORT_SYMBOL(usb_trylock_device);
+EXPORT_SYMBOL(usb_lock_device_for_reset);
EXPORT_SYMBOL(usb_unlock_device);
EXPORT_SYMBOL(usb_driver_claim_interface);
diff -Nru a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c
--- a/drivers/usb/image/microtek.c Thu Aug 19 11:49:30 2004
+++ b/drivers/usb/image/microtek.c Thu Aug 19 11:49:30 2004
@@ -341,12 +341,18 @@
static int mts_scsi_host_reset (Scsi_Cmnd *srb)
{
struct mts_desc* desc = (struct mts_desc*)(srb->device->host->hostdata[0]);
+ int result, rc;
MTS_DEBUG_GOT_HERE();
mts_debug_dump(desc);
- usb_reset_device(desc->usb_dev); /*FIXME: untested on new reset code */
- return 0; /* RANT why here 0 and not SUCCESS */
+ rc = usb_lock_device_for_reset(desc->usb_dev, desc->usb_intf);
+ if (rc < 0)
+ return FAILED;
+ result = usb_reset_device(desc->usb_dev);;
+ if (rc)
+ usb_unlock_device(desc->usb_dev);
+ return result ? FAILED : SUCCESS;
}
static
@@ -777,6 +783,7 @@
goto out_kfree;
new_desc->usb_dev = dev;
+ new_desc->usb_intf = intf;
init_MUTEX(&new_desc->lock);
/* endpoints */
diff -Nru a/drivers/usb/image/microtek.h b/drivers/usb/image/microtek.h
--- a/drivers/usb/image/microtek.h Thu Aug 19 11:49:30 2004
+++ b/drivers/usb/image/microtek.h Thu Aug 19 11:49:30 2004
@@ -31,6 +31,7 @@
struct mts_desc *prev;
struct usb_device *usb_dev;
+ struct usb_interface *usb_intf;
/* Endpoint addresses */
u8 ep_out;
diff -Nru a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
--- a/drivers/usb/storage/scsiglue.c Thu Aug 19 11:49:30 2004
+++ b/drivers/usb/storage/scsiglue.c Thu Aug 19 11:49:30 2004
@@ -266,7 +266,7 @@
static int bus_reset(struct scsi_cmnd *srb)
{
struct us_data *us = (struct us_data *)srb->device->host->hostdata[0];
- int result;
+ int result, rc;
US_DEBUGP("%s called\n", __FUNCTION__);
if (us->sm_state != US_STATE_IDLE) {
@@ -291,8 +291,16 @@
result = -EBUSY;
US_DEBUGP("Refusing to reset a multi-interface device\n");
} else {
- result = usb_reset_device(us->pusb_dev);
- US_DEBUGP("usb_reset_device returns %d\n", result);
+ rc = usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf);
+ if (rc < 0) {
+ US_DEBUGP("unable to lock device for reset: %d\n", rc);
+ result = rc;
+ } else {
+ result = usb_reset_device(us->pusb_dev);
+ if (rc)
+ usb_unlock_device(us->pusb_dev);
+ US_DEBUGP("usb_reset_device returns %d\n", result);
+ }
}
up(&(us->dev_semaphore));
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Thu Aug 19 11:49:30 2004
+++ b/include/linux/usb.h Thu Aug 19 11:49:30 2004
@@ -61,6 +61,13 @@
int extralen;
};
+enum usb_interface_condition {
+ USB_INTERFACE_UNBOUND = 0,
+ USB_INTERFACE_BINDING,
+ USB_INTERFACE_BOUND,
+ USB_INTERFACE_UNBINDING,
+};
+
/**
* struct usb_interface - what usb device drivers talk to
* @altsetting: array of interface structures, one for each alternate
@@ -75,6 +82,8 @@
* be unused. The driver should set this value in the probe()
* function of the driver, after it has been assigned a minor
* number from the USB core by calling usb_register_dev().
+ * @condition: binding state of the interface: not bound, binding
+ * (in probe()), bound to a driver, or unbinding (in disconnect())
* @dev: driver model's view of this device
* @class_dev: driver model's class view of this device.
*
@@ -113,6 +122,7 @@
unsigned num_altsetting; /* number of alternate settings */
int minor; /* minor number this interface is bound to */
+ enum usb_interface_condition condition; /* state of binding */
struct device dev; /* interface specific device info */
struct class_device *class_dev;
};
@@ -343,11 +353,12 @@
extern void usb_lock_device(struct usb_device *udev);
extern int usb_trylock_device(struct usb_device *udev);
+extern int usb_lock_device_for_reset(struct usb_device *udev,
+ struct usb_interface *iface);
extern void usb_unlock_device(struct usb_device *udev);
-/* mostly for devices emulating SCSI over USB */
+/* USB port reset for device reinitialization */
extern int usb_reset_device(struct usb_device *dev);
-extern int __usb_reset_device(struct usb_device *dev);
extern struct usb_device *usb_find_device(u16 vendor_id, u16 product_id);
-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
