Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754466AbYH0S2g (ORCPT ); Wed, 27 Aug 2008 14:28:36 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752137AbYH0S20 (ORCPT ); Wed, 27 Aug 2008 14:28:26 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:39174 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752032AbYH0S2Y (ORCPT ); Wed, 27 Aug 2008 14:28:24 -0400 Date: Wed, 27 Aug 2008 14:28:22 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Greg KH cc: Oliver Neukum , , Stefan Kopp , Marcel Janssen , Felipe Balbi , Subject: Re: [PATCH] USB: add USB test and measurement class driver - round 2 In-Reply-To: <20080827172042.GA18774@kroah.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2191 Lines: 72 On Wed, 27 Aug 2008, Greg KH wrote: > Here's an updated version of the usbtmc driver, with all of the > different issues that have been raised, hopefully addressed. This is an example of what I was discussing with Oliver. In all likelihood you simply don't need usbtmc_mutex, and using it will cause a lockdep violation. That's why so many of the other USB class drivers don't have an analogous static mutex. > +static int usbtmc_open(struct inode *inode, struct file *filp) > +{ > + struct usb_interface *intf; > + struct usbtmc_device_data *data; > + int retval = -ENODEV; > + > + mutex_lock(&usbtmc_mutex); You must never acquire a lock in your open method if it will be held by your disconnect method while unregistering the minor. > +static int usbtmc_probe(struct usb_interface *intf, > + const struct usb_device_id *id) > +{ ... > + retcode = usb_register_dev(intf, &usbtmc_class); > + if (retcode) { > + dev_err(&intf->dev, "Not able to get a minor" > + " (base %u, slice default): %d\n", USBTMC_MINOR_BASE, > + retcode); > + goto error_register; > + } > + dev_dbg(&intf->dev, "Using minor number %d\n", intf->minor); You do call usb_register_dev() during probe... > + > + return 0; > + > +error_register: > + sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp); > + sysfs_remove_group(&intf->dev.kobj, &data_attr_grp); > + kref_put(&data->kref, usbtmc_delete); > + return retcode; > +} > + > +static void usbtmc_disconnect(struct usb_interface *intf) > +{ > + struct usbtmc_device_data *data; > + > + dev_dbg(&intf->dev, "usbtmc_disconnect called\n"); > + > + mutex_lock(&usbtmc_mutex); > + data = usb_get_intfdata(intf); > + sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp); > + sysfs_remove_group(&intf->dev.kobj, &data_attr_grp); > + kref_put(&data->kref, usbtmc_delete); > + mutex_unlock(&usbtmc_mutex); > +} But you don't call usb_unregister_dev() during disconnect. An oversight? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/