Greg:
This patch creates a special locking routine intended specifically for use
when calling usb_reset_device(). A special routine like this is needed to
avoid deadlocks with disconnect(). Symbolically:
Khubd Driver
----- ------
Wants to reset a device, so calls
its own do_reset() function
Receives connect-change
notification and calls
usb_disconnect()
Locks the device and calls
driver->disconnect()
do_reset() tries to lock the
device for the reset, and blocks
disconnect() can't return
until do_reset() is finished
One simple-minded way out would be for the driver's do_reset routine to
use usb_trylock_device(). I don't like this because it will fail if the
device happens to be locked for some innocuous reason, like somebody
reading its entry in /proc/bus/usb/devices.
Instead, usb_lock_device_for_reset() calls usb_trylock_device()
repeatedly, in a polling loop. It is careful to check that the device
state isn't NOTATTACHED and that the driver's interface isn't being
unbound; this will allow the lock-and-reset sequence to fail in scenarios
like the one above.
For the new routine to be able to tell when an interface is being unbound,
I had to add an additional field to struct usb_interface. It's a simple
enumeration with four possible values:
USB_INTERFACE_UNBOUND, USB_INTERFACE_BINDING,
USB_INTERFACE_BOUND, USB_INTERFACE_UNBINDING
The extra overhead of the new field is not large enough to matter.
A quick summary:
Rename __usb_reset_device to usb_reset_device and get rid of
the alternate entry point.
Notify the HCD that endpoint-0's maxpacket size may change
during a device reset (should have been written earlier).
Add the new usb_interface_condition enumeration field to
struct usb_interface.
Set the new field appropriately as interfaces are bound,
unbound, claimed, and released.
Add usb_lock_device_for_reset().
Change the usb-storage, microtek, and stir4200 IRDA drivers
to make them use the new locking routine.
With this patch applied, and using an altered gadget driver that forces
usb-storage to request a device reset, I've been able to go through a
large number of different tests successfully. Reset during probe(),
reset after probe(), disconnect during reset(), reset with device
descriptor changing... They all work beautifully.
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 Jun 24 14:09:11 2004
+++ b/drivers/net/irda/stir4200.c Thu Jun 24 14:09:11 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 Jun 24 14:09:11 2004
+++ b/drivers/usb/core/devio.c Thu Jun 24 14:09:11 2004
@@ -722,7 +722,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 Jun 24 14:09:11 2004
+++ b/drivers/usb/core/hub.c Thu Jun 24 14:09:11 2004
@@ -2014,8 +2014,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;
@@ -2051,6 +2053,13 @@
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);
+ usb_endpoint_running(udev, 0, 0);
+ usb_endpoint_running(udev, 0, 1);
+
ret = hub_port_init(parent, udev, port);
if (ret < 0)
goto re_enumerate;
@@ -2114,16 +2123,4 @@
spin_unlock_irq(&hub_event_lock);
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 Jun 24 14:09:11 2004
+++ b/drivers/usb/core/usb.c Thu Jun 24 14:09:11 2004
@@ -100,7 +100,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;
@@ -112,6 +115,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);
@@ -123,6 +128,7 @@
intf->altsetting[0].desc.bInterfaceNumber,
0);
usb_set_intfdata(intf, NULL);
+ intf->condition = USB_INTERFACE_UNBOUND;
return 0;
}
@@ -321,6 +327,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()
@@ -360,6 +367,7 @@
dev->driver = NULL;
usb_set_intfdata(iface, NULL);
+ iface->condition = USB_INTERFACE_UNBOUND;
}
/**
@@ -869,6 +877,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
*
@@ -1442,6 +1494,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 Jun 24 14:09:11 2004
+++ b/drivers/usb/image/microtek.c Thu Jun 24 14:09:11 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 Jun 24 14:09:11 2004
+++ b/drivers/usb/image/microtek.h Thu Jun 24 14:09:11 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 Jun 24 14:09:11 2004
+++ b/drivers/usb/storage/scsiglue.c Thu Jun 24 14:09:11 2004
@@ -261,7 +261,7 @@
static int bus_reset( 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) {
@@ -286,8 +286,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 Jun 24 14:09:11 2004
+++ b/include/linux/usb.h Thu Jun 24 14:09:11 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;
};
@@ -346,11 +356,12 @@
/* for device locking -- don't manipulate usbdev->serialize directly! */
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);
-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 -
digital self defense, top technical experts, no vendor pitches,
unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
