[PATCH] USB core cleanups

Moves some functions that are only used by usbfs to be private, and
documents some of the interface issues that need to be cleaned up.

drivers/usb/core/devio.c

@@ -46,6 +46,7 @@
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
 
+#include "hcd.h"	/* for usbcore internals */
 
 struct async {
         struct list_head asynclist;
@@ -724,7 +725,7 @@ static int proc_resetdevice(struct dev_state *ps)
 
 		lock_kernel();
 		if (intf->driver && ps->dev) {
-			usb_bind_driver(intf->driver,ps->dev, i);
+			usb_bind_driver (intf->driver, intf);
 		}
 		unlock_kernel();
 	}
@@ -1104,7 +1105,7 @@ static int proc_ioctl (struct dev_state *ps, void *arg)
 
 	/* let kernel drivers try to (re)bind to the interface */
 	case USBDEVFS_CONNECT:
-               usb_find_interface_driver_for_ifnum (ps->dev, ctrl.ifno);
+		usb_find_interface_driver (ps->dev, ifp);
 		break;
 
        /* talk directly to the interface's driver */
@@ -1144,6 +1145,11 @@ static int proc_ioctl (struct dev_state *ps, void *arg)
 	return retval;
 }
 
+/*
+ * NOTE:  All requests here that have interface numbers as parameters
+ * are assuming that somehow the configuration has been prevented from
+ * changing.  But there's no mechanism to ensure that...
+ */
 static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct dev_state *ps = (struct dev_state *)file->private_data;

drivers/usb/core/hcd.h

@@ -348,6 +348,16 @@ extern struct semaphore usb_bus_list_lock;
 extern void usb_bus_get (struct usb_bus *bus);
 extern void usb_bus_put (struct usb_bus *bus);
 
+extern struct usb_interface *usb_ifnum_to_if (struct usb_device *dev,
+	unsigned ifnum);
+
+extern int usb_find_interface_driver (struct usb_device *dev,
+	struct usb_interface *interface);
+
+/* for probe/disconnect with correct module usage counting */
+void *usb_bind_driver(struct usb_driver *driver, struct usb_interface *intf);
+void usb_unbind_driver(struct usb_device *device, struct usb_interface *intf);
+
 /*-------------------------------------------------------------------------*/
 
 /* hub.h ... DeviceRemovable in 2.4.2-ac11, gone in 2.4.10 */

drivers/usb/core/hub.h

@@ -123,6 +123,10 @@ struct usb_hub_status {
  * Hub descriptor 
  * See USB 2.0 spec Table 11-13
  */
+
+#define USB_DT_HUB			(USB_TYPE_CLASS | 0x09)
+#define USB_DT_HUB_NONVAR_SIZE		7
+
 struct usb_hub_descriptor {
 	__u8  bDescLength;
 	__u8  bDescriptorType;

drivers/usb/core/message.c

@@ -8,6 +8,8 @@
 #include <linux/init.h>
 #include <asm/byteorder.h>
 
+#include "hcd.h"	/* for usbcore internals */
+
 struct usb_api_data {
 	wait_queue_head_t wqh;
 	int done;

drivers/usb/core/usb.c

@@ -52,7 +52,6 @@ extern void usb_major_cleanup(void);
  * Prototypes for the device driver probing/loading functions
  */
 static void usb_find_drivers(struct usb_device *);
-static int  usb_find_interface_driver(struct usb_device *, unsigned int);
 static void usb_check_support(struct usb_device *);
 
 /*
@@ -119,7 +118,7 @@ void usb_scan_devices(void)
 }
 
 /**
- *	usb_unbind_driver - disconnects a driver from a device
+ *	usb_unbind_driver - disconnects a driver from a device (usbcore-internal)
  *	@device: usb device to be disconnected
  *	@intf: interface of the device to be disconnected
  *	Context: BKL held
@@ -164,21 +163,27 @@ void usb_unbind_driver(struct usb_device *device, struct usb_interface *intf)
 }
 
 /**
- *	usb_bind_driver - connect a driver to a device's interface
- *	@driver: device driver to be bound to a devices interface
- *	@dev: device to be bound
- *	@ifnum: index number of the interface to be used
+ *	usb_bind_driver - connect a driver to a device's interface (usbcore-internal)
+ *	@driver: device driver to be bound to interface
+ *	@interface: interface that the driver will be using
+ *	Context: BKL held
+ *
+ *	Does a safe binding of a driver to one of a device's interfaces.
+ *	Returns the driver's data for the binding, or null indicating
+ *	that the driver did not bind to this interface.
  *
- *	Does a save binding of a driver to a device's interface
- *	Returns a pointer to the drivers private description of the binding
+ *	This differs from usb_driver_claim_interface(), which is called from
+ *	drivers and neither calls the driver's probe() entry nor does any
+ *	locking to guard against removing driver modules.
  */
-
-void *usb_bind_driver(struct usb_driver *driver, struct usb_device *dev, unsigned int ifnum)
+void *
+usb_bind_driver (struct usb_driver *driver, struct usb_interface *interface)
 {
 	int i,m;
 	void *private = NULL;
 	const struct usb_device_id *id;
-	struct usb_interface *interface;
+	struct usb_device *dev = interface_to_usbdev (interface);
+	int ifnum;
 
 	if (driver->owner) {
 		m = try_inc_mod_count(driver->owner);
@@ -187,7 +192,14 @@ void *usb_bind_driver(struct usb_driver *driver, struct usb_device *dev, unsigne
 		unlock_kernel();
 	}
 
-	interface = &dev->actconfig->interface[ifnum];
+	// START TEMPORARY
+	// driver->probe() hasn't yet changed to take interface not dev+ifnum,
+	// so we still need ifnum here.
+	for (ifnum = 0; ifnum < dev->actconfig->bNumInterfaces; ifnum++)
+		if (&dev->actconfig->interface [ifnum] == interface)
+			break;
+	BUG_ON (ifnum == dev->actconfig->bNumInterfaces);
+	// END TEMPORARY
 
 	id = driver->id_table;
 	/* new style driver? */
@@ -252,7 +264,7 @@ static void usb_drivers_purge(struct usb_driver *driver,struct usb_device *dev)
 			 * This will go through the list looking for another
 			 * driver that can handle the device
 			 */
-			usb_find_interface_driver(dev, i);
+			usb_find_interface_driver(dev, interface);
 		}
 	}
 }
@@ -294,29 +306,7 @@ void usb_deregister(struct usb_driver *driver)
 }
 
 /**
- * usb_ifnum_to_ifpos - convert the interface number to the interface position
- * @dev: the device to use
- * @ifnum: the interface number (bInterfaceNumber); not interface position
- *
- * This is used to convert the interface _number_ (as in
- * interface.bInterfaceNumber) to the interface _position_ (as in
- * dev->actconfig->interface + position).  Note that the number is the same as
- * the position for all interfaces _except_ devices with interfaces not
- * sequentially numbered (e.g., 0, 2, 3, etc).
- */
-int usb_ifnum_to_ifpos(struct usb_device *dev, unsigned ifnum)
-{
-	int i;
-
-	for (i = 0; i < dev->actconfig->bNumInterfaces; i++)
-		if (dev->actconfig->interface[i].altsetting[0].bInterfaceNumber == ifnum)
-			return i;
-
-	return -EINVAL;
-}
-
-/**
- * usb_ifnum_to_if - get the interface object with a given interface number
+ * usb_ifnum_to_if - get the interface object with a given interface number (usbcore-internal)
  * @dev: the device whose current configuration is considered
  * @ifnum: the desired interface
  *
@@ -392,7 +382,8 @@ static void usb_check_support(struct usb_device *dev)
 	/* now we check this device */
 	if (dev->devnum > 0)
 		for (i = 0; i < dev->actconfig->bNumInterfaces; i++)
-			usb_find_interface_driver(dev, i);
+			usb_find_interface_driver (dev,
+				dev->actconfig->interface + i);
 }
 
 
@@ -605,7 +596,7 @@ usb_match_id(struct usb_device *dev, struct usb_interface *interface,
 }
 
 /*
- * This entrypoint gets called for each new device.
+ * This entrypoint gets called for unclaimed interfaces.
  *
  * We now walk the list of registered USB drivers,
  * looking for one that will accept this interface.
@@ -620,21 +611,27 @@ usb_match_id(struct usb_device *dev, struct usb_interface *interface,
  *
  * Returns: 0 if a driver accepted the interface, -1 otherwise
  */
-static int usb_find_interface_driver(struct usb_device *dev, unsigned ifnum)
+int usb_find_interface_driver (
+	struct usb_device *dev,
+	struct usb_interface *interface
+)
 {
 	struct list_head *tmp;
-	struct usb_interface *interface;
 	void *private;
 	struct usb_driver *driver;
-	
-	if ((!dev) || (ifnum >= dev->actconfig->bNumInterfaces)) {
-		err("bad find_interface_driver params");
-		return -1;
-	}
+	int ifnum;
 	
 	down(&dev->serialize);
 
-	interface = dev->actconfig->interface + ifnum;
+	/* FIXME It's just luck that for some devices with drivers that set
+	 * configuration in probe(), the interface numbers still make sense.
+	 * That's one of several unsafe assumptions involved in configuring
+	 * devices, and in binding drivers to their interfaces.
+	 */
+	for (ifnum = 0; ifnum < dev->actconfig->bNumInterfaces; ifnum++)
+		if (&dev->actconfig->interface [ifnum] == interface)
+			break;
+	BUG_ON (ifnum == dev->actconfig->bNumInterfaces);
 
 	if (usb_interface_claimed(interface))
 		goto out_err;
@@ -645,7 +642,7 @@ static int usb_find_interface_driver(struct usb_device *dev, unsigned ifnum)
 		driver = list_entry(tmp, struct usb_driver, driver_list);
 		tmp = tmp->next;
 
-		private = usb_bind_driver(driver, dev, ifnum);
+		private = usb_bind_driver(driver, interface);
 
 		/* probe() may have changed the config on us */
 		interface = dev->actconfig->interface + ifnum;
@@ -664,25 +661,6 @@ static int usb_find_interface_driver(struct usb_device *dev, unsigned ifnum)
 	return -1;
 }
 
-/**
- * usb_find_interface_driver_for_ifnum - finds a usb interface driver for the specified ifnum
- * @dev: the device to use
- * @ifnum: the interface number (bInterfaceNumber); not interface position!
- *
- * This converts a ifnum to ifpos via a call to usb_ifnum_to_ifpos and then
- * calls usb_find_interface_driver() with the found ifpos.  Note
- * usb_find_interface_driver's ifnum parameter is actually interface position.
- */
-int usb_find_interface_driver_for_ifnum(struct usb_device *dev, unsigned ifnum)
-{
-	int ifpos = usb_ifnum_to_ifpos(dev, ifnum);
-
-	if (0 > ifpos)
-		return -EINVAL;
-
-	return usb_find_interface_driver(dev, ifpos);
-}
-
 #ifdef	CONFIG_HOTPLUG
 
 /*
@@ -956,7 +934,7 @@ static void usb_find_drivers(struct usb_device *dev)
 
 		/* if this interface hasn't already been claimed */
 		if (!usb_interface_claimed(interface)) {
-			if (usb_find_interface_driver(dev, ifnum))
+			if (usb_find_interface_driver(dev, interface))
 				rejected++;
 			else
 				claimed++;
@@ -1655,8 +1633,6 @@ module_exit(usb_exit);
  * These symbols are exported for device (or host controller)
  * driver modules to use.
  */
-EXPORT_SYMBOL(usb_ifnum_to_ifpos);
-EXPORT_SYMBOL(usb_ifnum_to_if);
 EXPORT_SYMBOL(usb_epnum_to_ep_desc);
 
 EXPORT_SYMBOL(usb_register);
@@ -1668,7 +1644,6 @@ EXPORT_SYMBOL(usb_free_dev);
 EXPORT_SYMBOL(usb_get_dev);
 EXPORT_SYMBOL(usb_hub_tt_clear_buffer);
 
-EXPORT_SYMBOL(usb_find_interface_driver_for_ifnum);
 EXPORT_SYMBOL(usb_driver_claim_interface);
 EXPORT_SYMBOL(usb_interface_claimed);
 EXPORT_SYMBOL(usb_driver_release_interface);
@@ -1679,8 +1654,6 @@ EXPORT_SYMBOL(usb_new_device);
 EXPORT_SYMBOL(usb_reset_device);
 EXPORT_SYMBOL(usb_connect);
 EXPORT_SYMBOL(usb_disconnect);
-EXPORT_SYMBOL(usb_bind_driver);
-EXPORT_SYMBOL(usb_unbind_driver);
 
 EXPORT_SYMBOL(__usb_get_extra_descriptor);
 

include/linux/usb.h

@@ -167,10 +167,6 @@ struct usb_device;
 #define USB_DT_OTHER_SPEED_CONFIG	0x07
 #define USB_DT_INTERFACE_POWER		0x08
 
-// FIXME should be internal to hub driver
-#define USB_DT_HUB			(USB_TYPE_CLASS | 0x09)
-#define USB_DT_HUB_NONVAR_SIZE		7
-
 /*
  * Descriptor sizes per descriptor type
  */
@@ -258,6 +254,8 @@ struct usb_interface {
 	void *private_data;
 };
 #define	to_usb_interface(d) container_of(d, struct usb_interface, dev)
+#define	interface_to_usbdev(intf) \
+	container_of(intf->dev.parent, struct usb_device, dev)
 
 /* USB_DT_CONFIG: Configuration descriptor information.
  *
@@ -303,10 +301,8 @@ struct usb_qualifier_descriptor {
 	__u8  bRESERVED;
 } __attribute__ ((packed));
 
-/* helpers for driver access to descriptors */
-extern int usb_ifnum_to_ifpos(struct usb_device *dev, unsigned ifnum);
-extern struct usb_interface *
-	usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum);
+// FIXME remove; exported only for drivers/usb/misc/auserwald.c
+// prefer usb_device->epnum[0..31]
 extern struct usb_endpoint_descriptor *
 	usb_epnum_to_ep_desc(struct usb_device *dev, unsigned epnum);
 
@@ -435,10 +431,6 @@ extern void usb_free_dev(struct usb_device *);
 /* for when layers above USB add new non-USB drivers */
 extern void usb_scan_devices(void);
 
-/* for probe/disconnect with correct module usage counting */
-void *usb_bind_driver(struct usb_driver *driver, struct usb_device *dev, unsigned int ifnum);
-void usb_unbind_driver(struct usb_device *device, struct usb_interface *intf);
-
 /* mostly for devices emulating SCSI over USB */
 extern int usb_reset_device(struct usb_device *dev);
 
@@ -446,7 +438,6 @@ extern int usb_reset_device(struct usb_device *dev);
 extern int usb_get_current_frame_number (struct usb_device *usb_dev);
 
 /* used these for multi-interface device registration */
-extern int usb_find_interface_driver_for_ifnum(struct usb_device *dev, unsigned int ifnum);
 extern void usb_driver_claim_interface(struct usb_driver *driver,
 			struct usb_interface *iface, void* priv);
 extern int usb_interface_claimed(struct usb_interface *iface);