From 9e0aea63d90fc678e3d2ed5c038d657e4fabaa2c Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 3 Sep 2004 17:25:50 +0200 Subject: [PATCH] USB: Make usbcore use usb_kill_urb() This patch changes the only places in usbcore where usb_unlink_urb() is still used for synchronous unlinking; now they will use usb_kill_urb(). As it turns out, there were only a couple of changes needed. This still leaves all the drivers to audit! Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 776c1bf0df9b..17ca7690dbde 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -286,9 +286,10 @@ static void destroy_async (struct dev_state *ps, struct list_head *list) while (!list_empty(list)) { as = list_entry(list->next, struct async, asynclist); list_del_init(&as->asynclist); + + /* drop the spinlock so the completion handler can run */ spin_unlock_irqrestore(&ps->lock, flags); - /* usb_unlink_urb calls the completion handler with status == -ENOENT */ - usb_unlink_urb(as->urb); + usb_kill_urb(as->urb); spin_lock_irqsave(&ps->lock, flags); } spin_unlock_irqrestore(&ps->lock, flags); @@ -976,7 +977,7 @@ static int proc_unlinkurb(struct dev_state *ps, void __user *arg) as = async_getpending(ps, arg); if (!as) return -EINVAL; - usb_unlink_urb(as->urb); + usb_kill_urb(as->urb); return 0; } -- cgit v1.2.3 From ee124eaab952e381d81ab0f4c63f31a4d7bc8bd3 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sun, 5 Sep 2004 08:53:18 +0200 Subject: USB: remove usbdevfs filesystem name, usbfs is the proper one to use. This has been publicised for years now, and the usvfs name will work just fine with a 2.4 kernel, so we are not breaking backwards compatibility. Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devices.c | 4 +- drivers/usb/core/devio.c | 10 ++-- drivers/usb/core/inode.c | 121 +++++-------------------------------------- drivers/usb/core/usb.h | 6 +++ include/linux/usb.h | 2 - include/linux/usbdevice_fs.h | 10 ---- 6 files changed, 26 insertions(+), 127 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c index 8e535789fce7..15d773fc539d 100644 --- a/drivers/usb/core/devices.c +++ b/drivers/usb/core/devices.c @@ -149,7 +149,7 @@ static const struct class_info clas_info[] = /*****************************************************************/ -void usbdevfs_conn_disc_event(void) +void usbfs_conn_disc_event(void) { conndiscevcnt++; wake_up(&deviceconndiscwq); @@ -682,7 +682,7 @@ static loff_t usb_device_lseek(struct file * file, loff_t offset, int orig) return ret; } -struct file_operations usbdevfs_devices_fops = { +struct file_operations usbfs_devices_fops = { .llseek = usb_device_lseek, .read = usb_device_read, .poll = usb_device_poll, diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 17ca7690dbde..0b3bf6634ee4 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -21,7 +21,7 @@ * * $Id: devio.c,v 1.7 2000/02/01 17:28:48 fliegl Exp $ * - * This file implements the usbdevfs/x/y files, where + * This file implements the usbfs/x/y files, where * x is the bus number and y the device number. * * It allows user space programs/"drivers" to communicate directly @@ -354,7 +354,7 @@ static void driver_disconnect(struct usb_interface *intf) destroy_async_on_interface(ps, ifnum); } -struct usb_driver usbdevfs_driver = { +struct usb_driver usbfs_driver = { .owner = THIS_MODULE, .name = "usbfs", .probe = driver_probe, @@ -379,7 +379,7 @@ static int claimintf(struct dev_state *ps, unsigned int ifnum) if (!intf) err = -ENOENT; else - err = usb_driver_claim_interface(&usbdevfs_driver, intf, ps); + err = usb_driver_claim_interface(&usbfs_driver, intf, ps); up_write(&usb_bus_type.subsys.rwsem); if (err == 0) set_bit(ifnum, &ps->ifclaimed); @@ -402,7 +402,7 @@ static int releaseintf(struct dev_state *ps, unsigned int ifnum) if (!intf) err = -ENOENT; else if (test_and_clear_bit(ifnum, &ps->ifclaimed)) { - usb_driver_release_interface(&usbdevfs_driver, intf); + usb_driver_release_interface(&usbfs_driver, intf); err = 0; } up_write(&usb_bus_type.subsys.rwsem); @@ -1315,7 +1315,7 @@ static unsigned int usbdev_poll(struct file *file, struct poll_table_struct *wai return mask; } -struct file_operations usbdevfs_device_file_operations = { +struct file_operations usbfs_device_file_operations = { .llseek = usbdev_lseek, .read = usbdev_read, .poll = usbdev_poll, diff --git a/drivers/usb/core/inode.c b/drivers/usb/core/inode.c index c9f57e334a4f..24452d66d576 100644 --- a/drivers/usb/core/inode.c +++ b/drivers/usb/core/inode.c @@ -4,7 +4,7 @@ * inode.c -- Inode/Dentry functions for the USB device file system. * * Copyright (C) 2000 Thomas Sailer (sailer@ife.ee.ethz.ch) - * Copyright (C) 2001,2002 Greg Kroah-Hartman (greg@kroah.com) + * Copyright (C) 2001,2002,2004 Greg Kroah-Hartman (greg@kroah.com) * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -40,17 +40,15 @@ #include #include #include +#include "usb.h" static struct super_operations usbfs_ops; static struct file_operations default_file_operations; static struct inode_operations usbfs_dir_inode_operations; -static struct vfsmount *usbdevfs_mount; static struct vfsmount *usbfs_mount; -static int usbdevfs_mount_count; /* = 0 */ static int usbfs_mount_count; /* = 0 */ static int ignore_mount = 0; -static struct dentry *devices_usbdevfs_dentry; static struct dentry *devices_usbfs_dentry; static int num_buses; /* = 0 */ @@ -240,9 +238,6 @@ static int remount(struct super_block *sb, int *flags, char *data) if (usbfs_mount && usbfs_mount->mnt_sb) update_sb(usbfs_mount->mnt_sb); - if (usbdevfs_mount && usbdevfs_mount->mnt_sb) - update_sb(usbdevfs_mount->mnt_sb); - return 0; } @@ -561,28 +556,12 @@ static void fs_remove_file (struct dentry *dentry) /* --------------------------------------------------------------------- */ - - -/* - * The usbdevfs name is now deprecated (as of 2.5.1). - * It will be removed when the 2.7.x development cycle is started. - * You have been warned :) - */ -static struct file_system_type usbdevice_fs_type; - static struct super_block *usb_get_sb(struct file_system_type *fs_type, int flags, const char *dev_name, void *data) { return get_sb_single(fs_type, flags, data, usbfs_fill_super); } -static struct file_system_type usbdevice_fs_type = { - .owner = THIS_MODULE, - .name = "usbdevfs", - .get_sb = usb_get_sb, - .kill_sb = kill_litter_super, -}; - static struct file_system_type usb_fs_type = { .owner = THIS_MODULE, .name = "usbfs", @@ -603,16 +582,10 @@ static int create_special_files (void) ignore_mount = 1; /* create the devices special file */ - retval = simple_pin_fs("usbdevfs", &usbdevfs_mount, &usbdevfs_mount_count); - if (retval) { - err ("Unable to get usbdevfs mount"); - goto exit; - } - retval = simple_pin_fs("usbfs", &usbfs_mount, &usbfs_mount_count); if (retval) { err ("Unable to get usbfs mount"); - goto error_clean_usbdevfs_mount; + goto exit; } ignore_mount = 0; @@ -620,7 +593,7 @@ static int create_special_files (void) parent = usbfs_mount->mnt_sb->s_root; devices_usbfs_dentry = fs_create_file ("devices", listmode | S_IFREG, parent, - NULL, &usbdevfs_devices_fops, + NULL, &usbfs_devices_fops, listuid, listgid); if (devices_usbfs_dentry == NULL) { err ("Unable to create devices usbfs file"); @@ -628,42 +601,19 @@ static int create_special_files (void) goto error_clean_mounts; } - parent = usbdevfs_mount->mnt_sb->s_root; - devices_usbdevfs_dentry = fs_create_file ("devices", - listmode | S_IFREG, parent, - NULL, &usbdevfs_devices_fops, - listuid, listgid); - if (devices_usbdevfs_dentry == NULL) { - err ("Unable to create devices usbfs file"); - retval = -ENODEV; - goto error_remove_file; - } - goto exit; -error_remove_file: - fs_remove_file (devices_usbfs_dentry); - devices_usbfs_dentry = NULL; - error_clean_mounts: simple_release_fs(&usbfs_mount, &usbfs_mount_count); - -error_clean_usbdevfs_mount: - simple_release_fs(&usbdevfs_mount, &usbdevfs_mount_count); - exit: return retval; } static void remove_special_files (void) { - if (devices_usbdevfs_dentry) - fs_remove_file (devices_usbdevfs_dentry); if (devices_usbfs_dentry) fs_remove_file (devices_usbfs_dentry); - devices_usbdevfs_dentry = NULL; devices_usbfs_dentry = NULL; - simple_release_fs(&usbdevfs_mount, &usbdevfs_mount_count); simple_release_fs(&usbfs_mount, &usbfs_mount_count); } @@ -671,11 +621,6 @@ void usbfs_update_special (void) { struct inode *inode; - if (devices_usbdevfs_dentry) { - inode = devices_usbdevfs_dentry->d_inode; - if (inode) - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; - } if (devices_usbfs_dentry) { inode = devices_usbfs_dentry->d_inode; if (inode) @@ -707,29 +652,16 @@ void usbfs_add_bus(struct usb_bus *bus) return; } - parent = usbdevfs_mount->mnt_sb->s_root; - bus->usbdevfs_dentry = fs_create_file (name, busmode | S_IFDIR, parent, - bus, NULL, busuid, busgid); - if (bus->usbdevfs_dentry == NULL) { - err ("error creating usbdevfs bus entry"); - return; - } - usbfs_update_special(); - usbdevfs_conn_disc_event(); + usbfs_conn_disc_event(); } - void usbfs_remove_bus(struct usb_bus *bus) { if (bus->usbfs_dentry) { fs_remove_file (bus->usbfs_dentry); bus->usbfs_dentry = NULL; } - if (bus->usbdevfs_dentry) { - fs_remove_file (bus->usbdevfs_dentry); - bus->usbdevfs_dentry = NULL; - } --num_buses; if (num_buses <= 0) { @@ -738,7 +670,7 @@ void usbfs_remove_bus(struct usb_bus *bus) } usbfs_update_special(); - usbdevfs_conn_disc_event(); + usbfs_conn_disc_event(); } void usbfs_add_device(struct usb_device *dev) @@ -750,20 +682,12 @@ void usbfs_add_device(struct usb_device *dev) sprintf (name, "%03d", dev->devnum); dev->usbfs_dentry = fs_create_file (name, devmode | S_IFREG, dev->bus->usbfs_dentry, dev, - &usbdevfs_device_file_operations, + &usbfs_device_file_operations, devuid, devgid); if (dev->usbfs_dentry == NULL) { err ("error creating usbfs device entry"); return; } - dev->usbdevfs_dentry = fs_create_file (name, devmode | S_IFREG, - dev->bus->usbdevfs_dentry, dev, - &usbdevfs_device_file_operations, - devuid, devgid); - if (dev->usbdevfs_dentry == NULL) { - err ("error creating usbdevfs device entry"); - return; - } /* Set the size of the device's file to be * equal to the size of the device descriptors. */ @@ -775,11 +699,9 @@ void usbfs_add_device(struct usb_device *dev) } if (dev->usbfs_dentry->d_inode) dev->usbfs_dentry->d_inode->i_size = i_size; - if (dev->usbdevfs_dentry->d_inode) - dev->usbdevfs_dentry->d_inode->i_size = i_size; usbfs_update_special(); - usbdevfs_conn_disc_event(); + usbfs_conn_disc_event(); } void usbfs_remove_device(struct usb_device *dev) @@ -791,10 +713,6 @@ void usbfs_remove_device(struct usb_device *dev) fs_remove_file (dev->usbfs_dentry); dev->usbfs_dentry = NULL; } - if (dev->usbdevfs_dentry) { - fs_remove_file (dev->usbdevfs_dentry); - dev->usbdevfs_dentry = NULL; - } while (!list_empty(&dev->filelist)) { ds = list_entry(dev->filelist.next, struct dev_state, list); list_del_init(&ds->list); @@ -807,51 +725,38 @@ void usbfs_remove_device(struct usb_device *dev) } } usbfs_update_special(); - usbdevfs_conn_disc_event(); + usbfs_conn_disc_event(); } /* --------------------------------------------------------------------- */ -#ifdef CONFIG_PROC_FS static struct proc_dir_entry *usbdir = NULL; -#endif int __init usbfs_init(void) { int retval; - retval = usb_register(&usbdevfs_driver); + retval = usb_register(&usbfs_driver); if (retval) return retval; retval = register_filesystem(&usb_fs_type); if (retval) { - usb_deregister(&usbdevfs_driver); - return retval; - } - retval = register_filesystem(&usbdevice_fs_type); - if (retval) { - unregister_filesystem(&usb_fs_type); - usb_deregister(&usbdevfs_driver); + usb_deregister(&usbfs_driver); return retval; } -#ifdef CONFIG_PROC_FS - /* create mount point for usbdevfs */ + /* create mount point for usbfs */ usbdir = proc_mkdir("usb", proc_bus); -#endif return 0; } void usbfs_cleanup(void) { - usb_deregister(&usbdevfs_driver); + usb_deregister(&usbfs_driver); unregister_filesystem(&usb_fs_type); - unregister_filesystem(&usbdevice_fs_type); -#ifdef CONFIG_PROC_FS if (usbdir) remove_proc_entry("usb", proc_bus); -#endif } diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index f1dff4f4d5d6..b44bb8d7cdfa 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -27,3 +27,9 @@ extern void usb_set_device_state(struct usb_device *udev, /* for labeling diagnostics */ extern const char *usbcore_name; + +/* usbfs stuff */ +extern struct usb_driver usbfs_driver; +extern struct file_operations usbfs_devices_fops; +extern struct file_operations usbfs_device_file_operations; +extern void usbfs_conn_disc_event(void); diff --git a/include/linux/usb.h b/include/linux/usb.h index a43c95a016d7..d07d5aba9e7f 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -264,7 +264,6 @@ struct usb_bus { int bandwidth_isoc_reqs; /* number of Isoc. requests */ struct dentry *usbfs_dentry; /* usbfs dentry entry for the bus */ - struct dentry *usbdevfs_dentry; /* usbdevfs dentry entry for the bus */ struct class_device class_dev; /* class device for this bus */ void (*release)(struct usb_bus *bus); /* function to destroy this bus's memory */ @@ -315,7 +314,6 @@ struct usb_device { struct list_head filelist; struct dentry *usbfs_dentry; /* usbfs dentry entry for the device */ - struct dentry *usbdevfs_dentry; /* usbdevfs dentry entry for the device */ /* * Child devices - these can be either new devices diff --git a/include/linux/usbdevice_fs.h b/include/linux/usbdevice_fs.h index 78f47434b757..af49afaf7bb4 100644 --- a/include/linux/usbdevice_fs.h +++ b/include/linux/usbdevice_fs.h @@ -166,16 +166,6 @@ struct dev_state { unsigned long ifclaimed; }; -/* internal methods & data */ -extern struct usb_driver usbdevfs_driver; -extern struct file_operations usbdevfs_drivers_fops; -extern struct file_operations usbdevfs_devices_fops; -extern struct file_operations usbdevfs_device_file_operations; -extern struct inode_operations usbdevfs_device_inode_operations; -extern struct inode_operations usbdevfs_bus_inode_operations; -extern struct file_operations usbdevfs_bus_file_operations; -extern void usbdevfs_conn_disc_event(void); - #endif /* __KERNEL__ */ /* --------------------------------------------------------------------- */ -- cgit v1.2.3 From 11c763cc105faf3f3148fa209b87ded8bb1fa07e Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Mon, 13 Sep 2004 20:33:44 -0700 Subject: [PATCH] USB: Updated USB device locking This patch reintroduces the USB device locking code we tried out earlier. As before, it solves the problem of effectively locking all the devices while drivers are registered and unregistered by introducing an rwsem. Unlike the earlier attempt, this version does not ever try to acquire a lock re-entrantly. I trust that will eliminate the races and hang-ups you observed with the earlier version. There are also copious comments explaining exactly how things should work. The patch interacts slightly with the locktree() code introduced by David for suspend/resume support. It doesn't change the functionality at all; it just updates the routine to follow the new locking rules. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devices.c | 6 +- drivers/usb/core/devio.c | 38 ++++++------ drivers/usb/core/hcd.c | 4 +- drivers/usb/core/hub.c | 93 ++++++++++++++++------------- drivers/usb/core/message.c | 16 ++--- drivers/usb/core/sysfs.c | 4 +- drivers/usb/core/usb.c | 138 +++++++++++++++++++++++++++++++++++++++++--- drivers/usb/core/usb.h | 2 + drivers/usb/host/ehci-hub.c | 2 +- drivers/usb/host/ohci-hub.c | 10 ++-- drivers/usb/host/ohci-pci.c | 8 +-- include/linux/usb.h | 12 ++++ 12 files changed, 242 insertions(+), 91 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c index 195e365675af..50009ed51e8d 100644 --- a/drivers/usb/core/devices.c +++ b/drivers/usb/core/devices.c @@ -451,7 +451,7 @@ static char *usb_dump_string(char *start, char *end, const struct usb_device *de * nbytes - the maximum number of bytes to write * skip_bytes - the number of bytes to skip before writing anything * file_offset - the offset into the devices file on completion - * The caller must own the usbdev->serialize semaphore. + * The caller must own the device lock. */ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes, loff_t *skip_bytes, loff_t *file_offset, struct usb_device *usbdev, struct usb_bus *bus, int level, int index, int count) @@ -586,9 +586,9 @@ static ssize_t usb_device_read(struct file *file, char __user *buf, size_t nbyte /* recurse through all children of the root hub */ if (!bus->root_hub) continue; - down(&bus->root_hub->serialize); + usb_lock_device(bus->root_hub); ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos, bus->root_hub, bus, 0, 0, 0); - up(&bus->root_hub->serialize); + usb_unlock_device(bus->root_hub); if (ret < 0) { up(&usb_bus_list_lock); return ret; diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 0b3bf6634ee4..c3e76e465c2b 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -113,7 +113,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l int i; pos = *ppos; - down(&dev->serialize); + usb_lock_device(dev); if (!connected(dev)) { ret = -ENODEV; goto err; @@ -175,7 +175,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l } err: - up(&dev->serialize); + usb_unlock_device(dev); return ret; } @@ -517,7 +517,7 @@ static int usbdev_release(struct inode *inode, struct file *file) struct usb_device *dev = ps->dev; unsigned int ifnum; - down(&dev->serialize); + usb_lock_device(dev); list_del_init(&ps->list); if (connected(dev)) { @@ -526,7 +526,7 @@ static int usbdev_release(struct inode *inode, struct file *file) releaseintf(ps, ifnum); destroy_all_async(ps); } - up(&dev->serialize); + usb_unlock_device(dev); usb_put_dev(dev); ps->dev = NULL; kfree(ps); @@ -558,10 +558,10 @@ static int proc_control(struct dev_state *ps, void __user *arg) snoop(&dev->dev, "control read: bRequest=%02x bRrequestType=%02x wValue=%04x wIndex=%04x\n", ctrl.bRequest, ctrl.bRequestType, ctrl.wValue, ctrl.wIndex); - up(&dev->serialize); + usb_unlock_device(dev); i = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), ctrl.bRequest, ctrl.bRequestType, ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, tmo); - down(&dev->serialize); + usb_lock_device(dev); if ((i > 0) && ctrl.wLength) { if (usbfs_snoop) { dev_info(&dev->dev, "control read: data "); @@ -589,10 +589,10 @@ static int proc_control(struct dev_state *ps, void __user *arg) printk ("%02x ", (unsigned char)(tbuf)[j]); printk("\n"); } - up(&dev->serialize); + usb_unlock_device(dev); i = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), ctrl.bRequest, ctrl.bRequestType, ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, tmo); - down(&dev->serialize); + usb_lock_device(dev); } free_page((unsigned long)tbuf); if (i<0) { @@ -636,9 +636,9 @@ static int proc_bulk(struct dev_state *ps, void __user *arg) kfree(tbuf); return -EINVAL; } - up(&dev->serialize); + usb_unlock_device(dev); i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo); - down(&dev->serialize); + usb_lock_device(dev); if (!i && len2) { if (copy_to_user(bulk.data, tbuf, len2)) { kfree(tbuf); @@ -652,9 +652,9 @@ static int proc_bulk(struct dev_state *ps, void __user *arg) return -EFAULT; } } - up(&dev->serialize); + usb_unlock_device(dev); i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo); - down(&dev->serialize); + usb_lock_device(dev); } kfree(tbuf); if (i < 0) { @@ -1025,9 +1025,9 @@ static int proc_reapurb(struct dev_state *ps, void __user *arg) break; if (signal_pending(current)) break; - up(&dev->serialize); + usb_unlock_device(dev); schedule(); - down(&dev->serialize); + usb_lock_device(dev); } remove_wait_queue(&ps->wait, &wait); set_current_state(TASK_RUNNING); @@ -1150,7 +1150,11 @@ static int proc_ioctl (struct dev_state *ps, void __user *arg) /* let kernel drivers try to (re)bind to the interface */ case USBDEVFS_CONNECT: + usb_unlock_device(ps->dev); + usb_lock_all_devices(); bus_rescan_devices(intf->dev.bus); + usb_unlock_all_devices(); + usb_lock_device(ps->dev); break; /* talk directly to the interface's driver */ @@ -1193,9 +1197,9 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd if (!(file->f_mode & FMODE_WRITE)) return -EPERM; - down(&dev->serialize); + usb_lock_device(dev); if (!connected(dev)) { - up(&dev->serialize); + usb_unlock_device(dev); return -ENODEV; } @@ -1295,7 +1299,7 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd ret = proc_ioctl(ps, p); break; } - up(&dev->serialize); + usb_unlock_device(dev); if (ret >= 0) inode->i_atime = CURRENT_TIME; return ret; diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index c1de465a10d1..ed3e723cc9c5 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -797,9 +797,9 @@ int usb_register_root_hub (struct usb_device *usb_dev, struct device *parent_dev return (retval < 0) ? retval : -EMSGSIZE; } - down (&usb_dev->serialize); + usb_lock_device (usb_dev); retval = usb_new_device (usb_dev); - up (&usb_dev->serialize); + usb_unlock_device (usb_dev); if (retval) { usb_dev->bus->root_hub = NULL; dev_err (parent_dev, "can't register root hub for %s, %d\n", diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 29bba5e78da3..36ef3db882a2 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -36,7 +36,9 @@ #include "hcd.h" #include "hub.h" -/* Protect struct usb_device state and children members */ +/* Protect struct usb_device->state and ->children members + * Note: Both are also protected by ->serialize, except that ->state can + * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ static spinlock_t device_state_lock = SPIN_LOCK_UNLOCKED; /* khubd's worklist and its lock */ @@ -857,17 +859,6 @@ static void hub_start_disconnect(struct usb_device *hdev) } -static void recursively_mark_NOTATTACHED(struct usb_device *udev) -{ - int i; - - for (i = 0; i < udev->maxchild; ++i) { - if (udev->children[i]) - recursively_mark_NOTATTACHED(udev->children[i]); - } - udev->state = USB_STATE_NOTATTACHED; -} - /* grab device/port lock, returning index of that port (zero based). * protects the upstream link used by this device from concurrent * tree operations like suspend, resume, reset, and disconnect, which @@ -884,21 +875,16 @@ static int locktree(struct usb_device *udev) /* root hub is always the first lock in the series */ hdev = udev->parent; if (!hdev) { - down(&udev->serialize); + usb_lock_device(udev); return 0; } /* on the path from root to us, lock everything from * top down, dropping parent locks when not needed - * - * NOTE: if disconnect were to ignore the locking, we'd need - * to get extra refcounts to everything since hdev->children - * and udev->parent could be invalidated while we work... */ t = locktree(hdev); if (t < 0) return t; - spin_lock_irq(&device_state_lock); for (t = 0; t < hdev->maxchild; t++) { if (hdev->children[t] == udev) { /* everything is fail-fast once disconnect @@ -910,33 +896,45 @@ static int locktree(struct usb_device *udev) /* when everyone grabs locks top->bottom, * non-overlapping work may be concurrent */ - spin_unlock_irq(&device_state_lock); down(&udev->serialize); up(&hdev->serialize); return t; } } - spin_unlock_irq(&device_state_lock); - up(&hdev->serialize); + usb_unlock_device(hdev); return -ENODEV; } +static void recursively_mark_NOTATTACHED(struct usb_device *udev) +{ + int i; + + for (i = 0; i < udev->maxchild; ++i) { + if (udev->children[i]) + recursively_mark_NOTATTACHED(udev->children[i]); + } + udev->state = USB_STATE_NOTATTACHED; +} + /** * usb_set_device_state - change a device's current state (usbcore, hcds) * @udev: pointer to device whose state should be changed * @new_state: new state value to be stored * - * udev->state is _not_ protected by the device lock. This + * udev->state is _not_ fully protected by the device lock. Although + * most transitions are made only while holding the lock, the state can + * can change to USB_STATE_NOTATTACHED at almost any time. This * is so that devices can be marked as disconnected as soon as possible, - * without having to wait for the semaphore to be released. Instead, - * changes to the state must be protected by the device_state_lock spinlock. + * without having to wait for any semaphores to be released. As a result, + * all changes to any device's state must be protected by the + * device_state_lock spinlock. * * Once a device has been added to the device tree, all changes to its state * should be made using this routine. The state should _not_ be set directly. * * If udev->state is already USB_STATE_NOTATTACHED then no change is made. * Otherwise udev->state is set to new_state, and if new_state is - * USB_STATE_NOTATTACHED then all of udev's descendant's states are also set + * USB_STATE_NOTATTACHED then all of udev's descendants' states are also set * to USB_STATE_NOTATTACHED. */ void usb_set_device_state(struct usb_device *udev, @@ -987,11 +985,12 @@ static void release_address(struct usb_device *udev) /** * usb_disconnect - disconnect a device (usbcore-internal) - * @pdev: pointer to device being disconnected, into a locked hub + * @pdev: pointer to device being disconnected * Context: !in_interrupt () * - * Something got disconnected. Get rid of it, and all of its children. - * If *pdev is a normal device then the parent hub should be locked. + * Something got disconnected. Get rid of it and all of its children. + * + * If *pdev is a normal device then the parent hub must already be locked. * If *pdev is a root hub then this routine will acquire the * usb_bus_list_lock on behalf of the caller. * @@ -1017,9 +1016,11 @@ void usb_disconnect(struct usb_device **pdev) usb_set_device_state(udev, USB_STATE_NOTATTACHED); /* lock the bus list on behalf of HCDs unregistering their root hubs */ - if (!udev->parent) + if (!udev->parent) { down(&usb_bus_list_lock); - down(&udev->serialize); + usb_lock_device(udev); + } else + down(&udev->serialize); dev_info (&udev->dev, "USB disconnect, address %d\n", udev->devnum); @@ -1044,14 +1045,16 @@ void usb_disconnect(struct usb_device **pdev) usbfs_remove_device(udev); usb_remove_sysfs_dev_files(udev); - /* Avoid races with recursively_mark_NOTATTACHED() and locktree() */ + /* Avoid races with recursively_mark_NOTATTACHED() */ spin_lock_irq(&device_state_lock); *pdev = NULL; spin_unlock_irq(&device_state_lock); - up(&udev->serialize); - if (!udev->parent) + if (!udev->parent) { + usb_unlock_device(udev); up(&usb_bus_list_lock); + } else + up(&udev->serialize); device_unregister(&udev->dev); } @@ -1516,16 +1519,14 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state) { int status; + /* caller owns the udev device lock */ if (port < 0) return port; - /* NOTE: udev->serialize released on all real returns! */ - if (state <= udev->dev.power.power_state || state < PM_SUSPEND_MEM || udev->state == USB_STATE_SUSPENDED || udev->state == USB_STATE_NOTATTACHED) { - up(&udev->serialize); return 0; } @@ -1605,7 +1606,6 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state) if (status == 0) udev->dev.power.power_state = state; - up(&udev->serialize); return status; } @@ -1629,7 +1629,15 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state) */ int usb_suspend_device(struct usb_device *udev, u32 state) { - return __usb_suspend_device(udev, locktree(udev), state); + int port, status; + + port = locktree(udev); + if (port < 0) + return port; + + status = __usb_suspend_device(udev, port, state); + usb_unlock_device(udev); + return status; } /* @@ -1642,7 +1650,7 @@ static int finish_port_resume(struct usb_device *udev) int status; u16 devstatus; - /* caller owns udev->serialize */ + /* caller owns the udev device lock */ dev_dbg(&udev->dev, "usb resume\n"); udev->dev.power.power_state = PM_SUSPEND_ON; @@ -1822,10 +1830,12 @@ int usb_resume_device(struct usb_device *udev) status); } - up(&udev->serialize); + usb_unlock_device(udev); /* rebind drivers that had no suspend() */ + usb_lock_all_devices(); bus_rescan_devices(&usb_bus_type); + usb_unlock_all_devices(); return status; } @@ -1867,6 +1877,7 @@ static int hub_suspend(struct usb_interface *intf, u32 state) continue; down(&udev->serialize); status = __usb_suspend_device(udev, port, state); + up(&udev->serialize); if (status < 0) dev_dbg(&intf->dev, "suspend port %d --> %d\n", port, status); @@ -2595,7 +2606,7 @@ static void hub_events(void) } loop: - up(&hdev->serialize); + usb_unlock_device(hdev); usb_put_dev(hdev); } /* end while (1) */ diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 98695c68d687..e20dde5d793a 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1132,6 +1132,8 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) * use usb_set_interface() on the interfaces it claims. Resetting the whole * configuration would affect other drivers' interfaces. * + * The caller must own the device lock. + * * Returns zero on success, else a negative error code. */ int usb_reset_configuration(struct usb_device *dev) @@ -1142,9 +1144,9 @@ int usb_reset_configuration(struct usb_device *dev) if (dev->state == USB_STATE_SUSPENDED) return -EHOSTUNREACH; - /* caller must own dev->serialize (config won't change) - * and the usb bus readlock (so driver bindings are stable); - * so calls during probe() are fine + /* caller must have locked the device and must own + * the usb bus readlock (so driver bindings are stable); + * calls during probe() are fine */ for (i = 1; i < 16; ++i) { @@ -1199,7 +1201,7 @@ static void release_interface(struct device *dev) * usb_set_configuration - Makes a particular device setting be current * @dev: the device whose configuration is being updated * @configuration: the configuration being chosen. - * Context: !in_interrupt(), caller holds dev->serialize + * Context: !in_interrupt(), caller owns the device lock * * This is used to enable non-default device modes. Not all devices * use this kind of configurability; many devices only have one @@ -1220,8 +1222,8 @@ static void release_interface(struct device *dev) * usb_set_interface(). * * This call is synchronous. The calling context must be able to sleep, - * and must not hold the driver model lock for USB; usb device driver - * probe() methods may not use this routine. + * must own the device lock, and must not hold the driver model's USB + * bus rwsem; usb device driver probe() methods cannot use this routine. * * Returns zero on success, or else the status code returned by the * underlying call that failed. On succesful completion, each interface @@ -1236,8 +1238,6 @@ int usb_set_configuration(struct usb_device *dev, int configuration) struct usb_interface **new_interfaces = NULL; int n, nintf; - /* dev->serialize guards all config changes */ - for (i = 0; i < dev->descriptor.bNumConfigurations; i++) { if (dev->config[i].desc.bConfigurationValue == configuration) { cp = &dev->config[i]; diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 78c5ca2f1051..007baee22013 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -55,9 +55,9 @@ set_bConfigurationValue (struct device *dev, const char *buf, size_t count) if (sscanf (buf, "%u", &config) != 1 || config > 255) return -EINVAL; - down(&udev->serialize); + usb_lock_device(udev); value = usb_set_configuration (udev, config); - up(&udev->serialize); + usb_unlock_device(udev); return (value < 0) ? value : count; } diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index de2edfb08582..93e2000043e4 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -62,6 +63,8 @@ const char *usbcore_name = "usbcore"; int nousb; /* Disable USB when built into kernel image */ /* Not honored on modular build */ +static DECLARE_RWSEM(usb_all_devices_rwsem); + static int generic_probe (struct device *dev) { @@ -151,7 +154,9 @@ int usb_register(struct usb_driver *new_driver) new_driver->driver.probe = usb_probe_interface; new_driver->driver.remove = usb_unbind_interface; + usb_lock_all_devices(); retval = driver_register(&new_driver->driver); + usb_unlock_all_devices(); if (!retval) { pr_info("%s: registered new driver %s\n", @@ -180,7 +185,9 @@ void usb_deregister(struct usb_driver *driver) { pr_info("%s: deregistering driver %s\n", usbcore_name, driver->name); + usb_lock_all_devices(); driver_unregister (&driver->driver); + usb_unlock_all_devices(); usbfs_update_special(); } @@ -202,7 +209,7 @@ void usb_deregister(struct usb_driver *driver) * alternate settings available for this interfaces. * * Don't call this function unless you are bound to one of the interfaces - * on this device or you own the dev->serialize semaphore! + * on this device or you have locked the device! */ struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum) { @@ -235,7 +242,7 @@ struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum) * drivers avoid such mistakes. * * Don't call this function unless you are bound to the intf interface - * or you own the device's ->serialize semaphore! + * or you have locked the device! */ struct usb_host_interface *usb_altnum_to_altsetting(struct usb_interface *intf, unsigned int altnum) @@ -303,11 +310,12 @@ usb_epnum_to_ep_desc(struct usb_device *dev, unsigned epnum) * way to bind to an interface is to return the private data from * the driver's probe() method. * - * Callers must own the driver model's usb bus writelock. So driver - * probe() entries don't need extra locking, but other call contexts - * may need to explicitly claim that lock. + * Callers must own the device lock and the driver model's usb_bus_type.subsys + * writelock. So driver probe() entries don't need extra locking, + * but other call contexts may need to explicitly claim those locks. */ -int usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface *iface, void* priv) +int usb_driver_claim_interface(struct usb_driver *driver, + struct usb_interface *iface, void* priv) { struct device *dev = &iface->dev; @@ -336,8 +344,8 @@ int usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface * * also causes the driver disconnect() method to be called. * * This call is synchronous, and may not be used in an interrupt context. - * Callers must own the usb_device serialize semaphore and the driver model's - * usb bus writelock. So driver disconnect() entries don't need extra locking, + * Callers must own the device lock and the driver model's usb_bus_type.subsys + * writelock. So driver disconnect() entries don't need extra locking, * but other call contexts may need to explicitly claim those locks. */ void usb_driver_release_interface(struct usb_driver *driver, @@ -830,6 +838,112 @@ void usb_put_intf(struct usb_interface *intf) put_device(&intf->dev); } + +/* USB device locking + * + * Although locking USB devices should be straightforward, it is + * complicated by the way the driver-model core works. When a new USB + * driver is registered or unregistered, the core will automatically + * probe or disconnect all matching interfaces on all USB devices while + * holding the USB subsystem writelock. There's no good way for us to + * tell which devices will be used or to lock them beforehand; our only + * option is to effectively lock all the USB devices. + * + * We do that by using a private rw-semaphore, usb_all_devices_rwsem. + * When locking an individual device you must first acquire the rwsem's + * readlock. When a driver is registered or unregistered the writelock + * must be held. These actions are encapsulated in the subroutines + * below, so all a driver needs to do is call usb_lock_device() and + * usb_unlock_device(). + * + * Complications arise when several devices are to be locked at the same + * time. Only hub-aware drivers that are part of usbcore ever have to + * do this; nobody else needs to worry about it. The problem is that + * usb_lock_device() must not be called to lock a second device since it + * would acquire the rwsem's readlock reentrantly, leading to deadlock if + * another thread was waiting for the writelock. The solution is simple: + * + * When locking more than one device, call usb_lock_device() + * to lock the first one. Lock the others by calling + * down(&udev->serialize) directly. + * + * When unlocking multiple devices, use up(&udev->serialize) + * to unlock all but the last one. Unlock the last one by + * calling usb_unlock_device(). + * + * When locking both a device and its parent, always lock the + * the parent first. + */ + +/** + * usb_lock_device - acquire the lock for a usb device structure + * @udev: device that's being locked + * + * Use this routine when you don't hold any other device locks; + * to acquire nested inner locks call down(&udev->serialize) directly. + * This is necessary for proper interaction with usb_lock_all_devices(). + */ +void usb_lock_device(struct usb_device *udev) +{ + down_read(&usb_all_devices_rwsem); + down(&udev->serialize); +} + +/** + * usb_trylock_device - attempt to acquire the lock for a usb device structure + * @udev: device that's being locked + * + * Don't use this routine if you already hold a device lock; + * use down_trylock(&udev->serialize) instead. + * This is necessary for proper interaction with usb_lock_all_devices(). + * + * Returns 1 if successful, 0 if contention. + */ +int usb_trylock_device(struct usb_device *udev) +{ + if (!down_read_trylock(&usb_all_devices_rwsem)) + return 0; + if (down_trylock(&udev->serialize)) { + up_read(&usb_all_devices_rwsem); + return 0; + } + return 1; +} + +/** + * usb_unlock_device - release the lock for a usb device structure + * @udev: device that's being unlocked + * + * Use this routine when releasing the only device lock you hold; + * to release inner nested locks call up(&udev->serialize) directly. + * This is necessary for proper interaction with usb_lock_all_devices(). + */ +void usb_unlock_device(struct usb_device *udev) +{ + up(&udev->serialize); + up_read(&usb_all_devices_rwsem); +} + +/** + * usb_lock_all_devices - acquire the lock for all usb device structures + * + * This is necessary when registering a new driver or probing a bus, + * since the driver-model core may try to use any usb_device. + */ +void usb_lock_all_devices(void) +{ + down_write(&usb_all_devices_rwsem); +} + +/** + * usb_unlock_all_devices - release the lock for all usb device structures + */ +void usb_unlock_all_devices(void) +{ + up_write(&usb_all_devices_rwsem); +} + + static struct usb_device *match_device(struct usb_device *dev, u16 vendor_id, u16 product_id) { @@ -851,8 +965,10 @@ static struct usb_device *match_device(struct usb_device *dev, /* look through all of the children of this device */ for (child = 0; child < dev->maxchild; ++child) { if (dev->children[child]) { + down(&dev->children[child]->serialize); ret_dev = match_device(dev->children[child], vendor_id, product_id); + up(&dev->children[child]->serialize); if (ret_dev) goto exit; } @@ -887,7 +1003,9 @@ struct usb_device *usb_find_device(u16 vendor_id, u16 product_id) bus = container_of(buslist, struct usb_bus, bus_list); if (!bus->root_hub) continue; + usb_lock_device(bus->root_hub); dev = match_device(bus->root_hub, vendor_id, product_id); + usb_unlock_device(bus->root_hub); if (dev) goto exit; } @@ -1373,6 +1491,10 @@ EXPORT_SYMBOL(usb_put_dev); EXPORT_SYMBOL(usb_get_dev); EXPORT_SYMBOL(usb_hub_tt_clear_buffer); +EXPORT_SYMBOL(usb_lock_device); +EXPORT_SYMBOL(usb_trylock_device); +EXPORT_SYMBOL(usb_unlock_device); + EXPORT_SYMBOL(usb_driver_claim_interface); EXPORT_SYMBOL(usb_driver_release_interface); EXPORT_SYMBOL(usb_match_id); diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 8e9c923bff0b..30d2cf7dd762 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -22,6 +22,8 @@ extern int usb_get_device_descriptor(struct usb_device *dev, unsigned int size); extern int usb_set_configuration(struct usb_device *dev, int configuration); +extern void usb_lock_all_devices(void); +extern void usb_unlock_all_devices(void); /* for labeling diagnostics */ extern const char *usbcore_name; diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 438701e17946..4a23fbf1f7af 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -81,7 +81,7 @@ static int ehci_hub_suspend (struct usb_hcd *hcd) } -/* caller owns root->serialize, and should reset/reinit on error */ +/* caller has locked the root hub, and should reset/reinit on error */ static int ehci_hub_resume (struct usb_hcd *hcd) { struct ehci_hcd *ehci = hcd_to_ehci (hcd); diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c index 1f21cf815ad4..d7a28d8eefb9 100644 --- a/drivers/usb/host/ohci-hub.c +++ b/drivers/usb/host/ohci-hub.c @@ -138,7 +138,7 @@ static inline struct ed *find_head (struct ed *ed) return ed; } -/* caller owns root->serialize */ +/* caller has locked the root hub */ static int ohci_hub_resume (struct usb_hcd *hcd) { struct ohci_hcd *ohci = hcd_to_ohci (hcd); @@ -282,9 +282,9 @@ static void ohci_rh_resume (void *_hcd) { struct usb_hcd *hcd = _hcd; - down (&hcd->self.root_hub->serialize); + usb_lock_device (hcd->self.root_hub); (void) ohci_hub_resume (hcd); - up (&hcd->self.root_hub->serialize); + usb_unlock_device (hcd->self.root_hub); } #else @@ -362,12 +362,12 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf) && ((OHCI_CTRL_HCFS | OHCI_SCHED_ENABLES) & ohci->hc_control) == OHCI_USB_OPER - && down_trylock (&hcd->self.root_hub->serialize) == 0 + && usb_trylock_device (hcd->self.root_hub) ) { ohci_vdbg (ohci, "autosuspend\n"); (void) ohci_hub_suspend (&ohci->hcd); ohci->hcd.state = USB_STATE_RUNNING; - up (&hcd->self.root_hub->serialize); + usb_unlock_device (hcd->self.root_hub); } #endif diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c index b1dbea9e4b23..23f6b5f3fee5 100644 --- a/drivers/usb/host/ohci-pci.c +++ b/drivers/usb/host/ohci-pci.c @@ -121,9 +121,9 @@ static int ohci_pci_suspend (struct usb_hcd *hcd, u32 state) #ifdef CONFIG_USB_SUSPEND (void) usb_suspend_device (hcd->self.root_hub, state); #else - down (&hcd->self.root_hub->serialize); + usb_lock_device (hcd->self.root_hub); (void) ohci_hub_suspend (hcd); - up (&hcd->self.root_hub->serialize); + usb_unlock_device (hcd->self.root_hub); #endif /* let things settle down a bit */ @@ -169,9 +169,9 @@ static int ohci_pci_resume (struct usb_hcd *hcd) /* get extra cleanup even if remote wakeup isn't in use */ retval = usb_resume_device (hcd->self.root_hub); #else - down (&hcd->self.root_hub->serialize); + usb_lock_device (hcd->self.root_hub); retval = ohci_hub_resume (hcd); - up (&hcd->self.root_hub->serialize); + usb_unlock_device (hcd->self.root_hub); #endif if (retval == 0) { diff --git a/include/linux/usb.h b/include/linux/usb.h index d07d5aba9e7f..67a8d7bce43b 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -281,6 +281,14 @@ struct usb_bus { struct usb_tt; +/* + * struct usb_device - kernel's representation of a USB device + * + * FIXME: Write the kerneldoc! + * + * Usbcore drivers should not set usbdev->state directly. Instead use + * usb_set_device_state(). + */ struct usb_device { int devnum; /* Address on USB bus */ char devpath [16]; /* Use in messages: /port/port/... */ @@ -331,6 +339,10 @@ struct usb_device { extern struct usb_device *usb_get_dev(struct usb_device *dev); extern void usb_put_dev(struct usb_device *dev); +extern void usb_lock_device(struct usb_device *udev); +extern int usb_trylock_device(struct usb_device *udev); +extern void usb_unlock_device(struct usb_device *udev); + /* mostly for devices emulating SCSI over USB */ extern int usb_reset_device(struct usb_device *dev); extern int __usb_reset_device(struct usb_device *dev); -- cgit v1.2.3 From 7d8d0405f07a1777cad6898346127b478b68dd0e Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Mon, 13 Sep 2004 20:34:46 -0700 Subject: [PATCH] USB: Add locking support for USB device resets 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. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/net/irda/stir4200.c | 10 ++++++++ drivers/usb/core/devio.c | 2 +- drivers/usb/core/hub.c | 23 +++++++---------- drivers/usb/core/usb.c | 57 ++++++++++++++++++++++++++++++++++++++++++ drivers/usb/image/microtek.c | 11 ++++++-- drivers/usb/image/microtek.h | 1 + drivers/usb/storage/scsiglue.c | 14 ++++++++--- include/linux/usb.h | 15 +++++++++-- 8 files changed, 111 insertions(+), 22 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/net/irda/stir4200.c b/drivers/net/irda/stir4200.c index 81282d94f91d..5f26d9ff30e9 100644 --- a/drivers/net/irda/stir4200.c +++ b/drivers/net/irda/stir4200.c @@ -168,6 +168,7 @@ enum StirTestMask { 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 @@ static int change_speed(struct stir_cb *stir, unsigned speed) { 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 @@ static int change_speed(struct stir_cb *stir, unsigned speed) 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 @@ static int stir_probe(struct usb_interface *intf, stir = net->priv; stir->netdev = net; stir->usbdev = dev; + stir->usbintf = intf; ret = usb_reset_configuration(dev); if (ret != 0) { diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index c3e76e465c2b..0e0ea0806606 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -735,7 +735,7 @@ static int proc_connectinfo(struct dev_state *ps, void __user *arg) static int proc_resetdevice(struct dev_state *ps) { - return __usb_reset_device(ps->dev); + return usb_reset_device(ps->dev); } diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 36ef3db882a2..848ab62e71a6 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -826,7 +826,7 @@ static int hub_reset(struct usb_hub *hub) else return -1; - if (__usb_reset_device(hdev)) + if (usb_reset_device(hdev)) return -1; hub->urb->dev = hdev; @@ -2759,8 +2759,10 @@ static int config_descriptors_changed(struct usb_device *udev) * * 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; @@ -2795,6 +2797,11 @@ int __usb_reset_device(struct usb_device *udev) 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; @@ -2848,15 +2855,3 @@ 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 --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 93e2000043e4..c4c457975ae3 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -102,7 +102,10 @@ int usb_probe_interface(struct device *dev) 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 @@ int usb_unbind_interface(struct device *dev) 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 @@ int usb_unbind_interface(struct device *dev) intf->altsetting[0].desc.bInterfaceNumber, 0); usb_set_intfdata(intf, NULL); + intf->condition = USB_INTERFACE_UNBOUND; return 0; } @@ -324,6 +330,7 @@ int usb_driver_claim_interface(struct usb_driver *driver, 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 @@ void usb_driver_release_interface(struct usb_driver *driver, dev->driver = NULL; usb_set_intfdata(iface, NULL); + iface->condition = USB_INTERFACE_UNBOUND; } /** @@ -910,6 +918,54 @@ int usb_trylock_device(struct usb_device *udev) return 1; } +/** + * 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 (udev->state == USB_STATE_SUSPENDED) + return -EHOSTUNREACH; + 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 (udev->state == USB_STATE_SUSPENDED) + return -EHOSTUNREACH; + 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 +1549,7 @@ EXPORT_SYMBOL(usb_hub_tt_clear_buffer); 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 --git a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c index 14b218239a8c..2a18c35629eb 100644 --- a/drivers/usb/image/microtek.c +++ b/drivers/usb/image/microtek.c @@ -341,12 +341,18 @@ static int mts_scsi_abort (Scsi_Cmnd *srb) 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 @@ static int mts_usb_probe(struct usb_interface *intf, goto out_kfree; new_desc->usb_dev = dev; + new_desc->usb_intf = intf; init_MUTEX(&new_desc->lock); /* endpoints */ diff --git a/drivers/usb/image/microtek.h b/drivers/usb/image/microtek.h index 206994ddcedf..3271deb8c001 100644 --- a/drivers/usb/image/microtek.h +++ b/drivers/usb/image/microtek.h @@ -31,6 +31,7 @@ struct mts_desc { struct mts_desc *prev; struct usb_device *usb_dev; + struct usb_interface *usb_intf; /* Endpoint addresses */ u8 ep_out; diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index 29f4e59759e6..83d7003861ff 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -283,7 +283,7 @@ static int device_reset(struct scsi_cmnd *srb) 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) { @@ -308,8 +308,16 @@ static int bus_reset(struct scsi_cmnd *srb) 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 --git a/include/linux/usb.h b/include/linux/usb.h index 67a8d7bce43b..18ee0751a32b 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -61,6 +61,13 @@ struct usb_host_interface { 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 @@ struct usb_host_interface { * 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 @@ struct usb_interface { 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; }; @@ -341,11 +351,12 @@ extern void usb_put_dev(struct usb_device *dev); 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); -- cgit v1.2.3