USB: idmouse: disconnect clean ups
cover USB: idmouse: disconnect clean ups
The idmouse driver does not suffer from the disconnect-related bugs recently fixed in the other misc drivers. It does however have some redundant constructs in place to manage the open-disconnect race, which is already taken care of by USB core.
Johan
Johan Hovold (3):
USB: idmouse: simplify disconnect handling
USB: idmouse: drop redundant open-count check from release
USB: idmouse: clean up runaway white space
drivers/usb/misc/idmouse.c | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-)
Signature · 1 line
2.23.0
PATCH 1/3 USB: idmouse: simplify disconnect handling
Since commit d4ead16f50f9 ("USB: prevent char device open/deregister race") core prevents further calls to open() after usb_deregister_dev() returns so there's no need to use the interface data for synchronisation.
This effectively reverts commit 54d2bc068fd2 ("USB: fix locking in idmouse") with respect to the open-disconnect race.
Note that the driver already uses a present flag to suppress I/O post disconnect (even if all USB I/O take place at open).
--- drivers/usb/misc/idmouse.c | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-)
drivers/usb/misc/idmouse.c +2 -18
@@ -60,7 +60,6 @@ static const struct usb_device_id idmouse_table[] = { USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)MODULE_DEVICE_TABLE (usb, idmouse_table); -static DEFINE_MUTEX (open_disc_mutex);/* structure to hold all of our device specific stuff */ struct usb_idmouse {
@@ -227,17 +226,13 @@ static int idmouse_open(struct inode *inode, struct file *file)if (!interface)return -ENODEV ; -mutex_lock (&open_disc_mutex);/* get the device information block from the interface */ dev =usb_get_intfdata (interface); -if (!dev) { -mutex_unlock (&open_disc_mutex); +if (!dev)return -ENODEV ; - }/* lock this device */ mutex_lock (&dev->lock); -mutex_unlock (&open_disc_mutex);/* check if already open */ if (dev->open) {
@@ -280,14 +275,12 @@ static int idmouse_release(struct inode *inode, struct file *file)if (dev ==NULL )return -ENODEV ; -mutex_lock (&open_disc_mutex);/* lock our device */ mutex_lock (&dev->lock);/* are we really open? */ if (dev->open <=0 ) {mutex_unlock (&dev->lock); -mutex_unlock (&open_disc_mutex);return -ENODEV ; }
@@ -296,11 +289,9 @@ static int idmouse_release(struct inode *inode, struct file *file)if (!dev->present) {/* the device was unplugged before the file was released */ mutex_unlock (&dev->lock); -mutex_unlock (&open_disc_mutex);idmouse_delete (dev); }else {mutex_unlock (&dev->lock); -mutex_unlock (&open_disc_mutex); }return 0 ; }
@@ -379,7 +370,6 @@ static int idmouse_probe(struct usb_interface *interface,if (result) {/* something prevented us from registering this device */ dev_err (&interface->dev,"Unable to allocate minor number.\n" ); -usb_set_intfdata (interface,NULL );idmouse_delete (dev);return result; }
@@ -392,19 +382,13 @@ static int idmouse_probe(struct usb_interface *interface,static void idmouse_disconnect (struct usb_interface *interface) { -struct usb_idmouse *dev; - -/* get device structure */ - dev =usb_get_intfdata (interface); +struct usb_idmouse *dev =usb_get_intfdata (interface);/* give back our minor */ usb_deregister_dev (interface, &idmouse_class); -mutex_lock (&open_disc_mutex); -usb_set_intfdata (interface,NULL );/* lock the device */ mutex_lock (&dev->lock); -mutex_unlock (&open_disc_mutex);/* prevent device read, write and ioctl */ dev->present =0 ;
Signature · 1 line
2.23.0
PATCH 2/3 USB: idmouse: drop redundant open-count check from release
In reply to Johan HovoldThe open count will always be exactly one when release is called, so drop the redundant sanity check.
--- drivers/usb/misc/idmouse.c | 6 ------ 1 file changed, 6 deletions(-)
drivers/usb/misc/idmouse.c +0 -6
@@ -278,12 +278,6 @@ static int idmouse_release(struct inode *inode, struct file *file)/* lock our device */ mutex_lock (&dev->lock); -/* are we really open? */ -if (dev->open <=0 ) { -mutex_unlock (&dev->lock); -return -ENODEV ; - } - --dev->open;if (!dev->present) {
Signature · 1 line
2.23.0
PATCH 3/3 USB: idmouse: clean up runaway white space
In reply to Johan HovoldDrop space between function identifiers and opening parenthesis, which was no longer even used consistently within the driver.
--- drivers/usb/misc/idmouse.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/usb/misc/idmouse.c +5 -5
@@ -56,7 +56,7 @@ static const struct usb_device_id idmouse_table[] = {#define FTIP_SCROLL 0x24#define ftip_command (dev, command, value, index) \ - usb_control_msg (dev->udev, usb_sndctrlpipe (dev->udev, 0), command, \ + usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), command, \ USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT, value, index, NULL, 0, 1000)MODULE_DEVICE_TABLE (usb, idmouse_table);
@@ -157,8 +157,8 @@ static int idmouse_create_image(struct usb_idmouse *dev)/* loop over a blocking bulk read to get data from the device */ while (bytes_read <IMGSIZE ) { - result =usb_bulk_msg (dev->udev, -usb_rcvbulkpipe (dev->udev, dev->bulk_in_endpointAddr), + result =usb_bulk_msg (dev->udev, +usb_rcvbulkpipe (dev->udev, dev->bulk_in_endpointAddr), dev->bulk_in_buffer + bytes_read, dev->bulk_in_size, &bulk_read,5000 );if (result <0 ) {
@@ -222,7 +222,7 @@ static int idmouse_open(struct inode *inode, struct file *file)int result;/* get the interface from minor number and driver information */ - interface =usb_find_interface (&idmouse_driver,iminor (inode)); + interface =usb_find_interface (&idmouse_driver,iminor (inode));if (!interface)return -ENODEV ;
@@ -246,7 +246,7 @@ static int idmouse_open(struct inode *inode, struct file *file) result =usb_autopm_get_interface (interface);if (result) goto error; - result =idmouse_create_image (dev); + result =idmouse_create_image (dev);usb_autopm_put_interface (interface);if (result) goto error;
Signature · 1 line
2.23.0