[PATCH] USB device reference counting api cleanup changes

This patch replaces the awkwardly named usb_inc_dev_use() and
usb_dec_dev_use() with usb_get_dev() and usb_put_dev() to match the
naming convention of the rest of the kernel's reference counted
structures.  It also does away with the special case of usb_free_dev(),
and has usb_put_dev() be the same thing (through a #define, just like
usb_free_urb() works.)

Now when the last person calls usb_put_dev() or usb_free_dev() the
structure is cleaned up.  This allows the different host controller
drivers to implement their logic differently if they want to (as they
do), and everyone can be happy and stop arguing about the "proper" way
to write their host controller drivers :)

drivers/usb/core/hcd.c

@@ -1320,7 +1320,7 @@ static void urb_unlink (struct urb *urb)
 	list_del_init (&urb->urb_list);
 	dev = urb->dev;
 	urb->dev = NULL;
-	usb_dec_dev_use (dev);
+	usb_put_dev (dev);
 	spin_unlock_irqrestore (&hcd_data_lock, flags);
 }
 
@@ -1516,7 +1516,7 @@ static int hcd_submit_urb (struct urb *urb, int mem_flags)
 
 	spin_lock_irqsave (&hcd_data_lock, flags);
 	if (HCD_IS_RUNNING (hcd->state) && hcd->state != USB_STATE_QUIESCING) {
-		usb_inc_dev_use (urb->dev);
+		usb_get_dev (urb->dev);
 		list_add (&urb->urb_list, &dev->urb_list);
 		status = 0;
 	} else {

drivers/usb/core/usb.c

@@ -969,21 +969,47 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, struct usb_bus *bus)
 }
 
 /**
- * usb_free_dev - free a usb device structure (usbcore-internal)
+ * usb_get_dev - increments the reference count of the device
+ * @dev: the device being referenced
+ *
+ * Each live reference to a device should be refcounted.
+ *
+ * Drivers for USB interfaces should normally record such references in
+ * their probe() methods, when they bind to an interface, and release
+ * them by calling usb_put_dev(), in their disconnect() methods.
+ *
+ * A pointer to the device with the incremented reference counter is returned.
+ */
+struct usb_device *usb_get_dev (struct usb_device *dev)
+{
+	if (dev) {
+		atomic_inc (&dev->refcnt);
+		return dev;
+	}
+	return NULL;
+}
+
+/**
+ * usb_free_dev - free a usb device structure when all users of it are finished.
  * @dev: device that's been disconnected
  * Context: !in_interrupt ()
  *
+ * Must be called when a user of a device is finished with it.  When the last
+ * user of the device calls this function, the memory of the device is freed.
+ *
  * Used by hub and virtual root hub drivers.  The device is completely
  * gone, everything is cleaned up, so it's time to get rid of these last
  * records of this device.
  */
 void usb_free_dev(struct usb_device *dev)
 {
-	if (dev->bus->op->deallocate)
-		dev->bus->op->deallocate(dev);
-	usb_destroy_configuration (dev);
-	usb_bus_put (dev->bus);
-	kfree (dev);
+	if (atomic_dec_and_test(&dev->refcnt)) {
+		if (dev->bus->op->deallocate)
+			dev->bus->op->deallocate(dev);
+		usb_destroy_configuration (dev);
+		usb_bus_put (dev->bus);
+		kfree (dev);
+	}
 }
 
 /**
@@ -1039,7 +1065,7 @@ void usb_free_urb(struct urb *urb)
 }
 
 /**
- * usb_get_urb - incrementes the reference count of the urb
+ * usb_get_urb - increments the reference count of the urb
  * @urb: pointer to the urb to modify
  *
  * This must be  called whenever a urb is transfered from a device driver to a
@@ -1928,7 +1954,7 @@ void usb_disconnect(struct usb_device **pdev)
 
 	/* Decrement the reference count, it'll auto free everything when */
 	/* it hits 0 which could very well be now */
-	usb_dec_dev_use(dev);
+	usb_put_dev(dev);
 }
 
 /**
@@ -2760,6 +2786,7 @@ EXPORT_SYMBOL(usb_deregister_dev);
 
 EXPORT_SYMBOL(usb_alloc_dev);
 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);

drivers/usb/host/uhci.c

@@ -165,7 +165,7 @@ static struct uhci_td *uhci_alloc_td(struct uhci *uhci, struct usb_device *dev)
 	INIT_LIST_HEAD(&td->list);
 	INIT_LIST_HEAD(&td->fl_list);
 
-	usb_inc_dev_use(dev);
+	usb_get_dev(dev);
 
 	return td;
 }
@@ -317,7 +317,7 @@ static void uhci_free_td(struct uhci *uhci, struct uhci_td *td)
 		dbg("td is still in URB list!");
 
 	if (td->dev)
-		usb_dec_dev_use(td->dev);
+		usb_put_dev(td->dev);
 
 	pci_pool_free(uhci->td_pool, td, td->dma_handle);
 }
@@ -342,7 +342,7 @@ static struct uhci_qh *uhci_alloc_qh(struct uhci *uhci, struct usb_device *dev)
 	INIT_LIST_HEAD(&qh->list);
 	INIT_LIST_HEAD(&qh->remove_list);
 
-	usb_inc_dev_use(dev);
+	usb_get_dev(dev);
 
 	return qh;
 }
@@ -355,7 +355,7 @@ static void uhci_free_qh(struct uhci *uhci, struct uhci_qh *qh)
 		dbg("qh still in remove_list!");
 
 	if (qh->dev)
-		usb_dec_dev_use(qh->dev);
+		usb_put_dev(qh->dev);
 
 	pci_pool_free(uhci->qh_pool, qh, qh->dma_handle);
 }
@@ -1492,7 +1492,7 @@ static int uhci_submit_urb(struct urb *urb, int mem_flags)
 	uhci = (struct uhci *)urb->dev->bus->hcpriv;
 
 	INIT_LIST_HEAD(&urb->urb_list);
-	usb_inc_dev_use(urb->dev);
+	usb_get_dev(urb->dev);
 
 	spin_lock_irqsave(&uhci->urb_list_lock, flags);
 	spin_lock(&urb->lock);
@@ -1503,7 +1503,7 @@ static int uhci_submit_urb(struct urb *urb, int mem_flags)
 		/* Since we can have problems on the out path */
 		spin_unlock(&urb->lock);
 		spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
-		usb_dec_dev_use(urb->dev);
+		usb_put_dev(urb->dev);
 		usb_put_urb(urb);
 
 		return ret;
@@ -2376,7 +2376,7 @@ static void uhci_call_completion(struct urb *urb)
 		} else {
 			/* We decrement the usage count after we're done */
 			/*  with everything */
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			usb_put_urb(urb);
 		}
 	}

drivers/usb/host/usb-ohci.c

@@ -210,7 +210,7 @@ static void urb_rm_priv_locked (struct urb * urb)
 		}
 
 		urb_free_priv ((struct ohci *)urb->dev->bus->hcpriv, urb_priv);
-		usb_dec_dev_use (urb->dev);
+		usb_put_dev (urb->dev);
 		urb->dev = NULL;
 		usb_put_urb (urb);
 	}
@@ -563,7 +563,7 @@ static int sohci_submit_urb (struct urb * urb, int mem_flags)
 	/* increment the reference count of the urb, as we now also control it */
 	urb = usb_get_urb (urb);
 
-	usb_inc_dev_use (urb->dev);
+	usb_get_dev (urb->dev);
 	ohci = (ohci_t *) urb->dev->bus->hcpriv;
 	
 #ifdef DEBUG
@@ -577,14 +577,14 @@ static int sohci_submit_urb (struct urb * urb, int mem_flags)
 	/* when controller's hung, permit only roothub cleanup attempts
 	 * such as powering down ports */
 	if (ohci->disabled) {
-		usb_dec_dev_use (urb->dev);	
+		usb_put_dev (urb->dev);	
 		usb_put_urb (urb);
 		return -ESHUTDOWN;
 	}
 
 	/* every endpoint has a ed, locate and fill it */
 	if (!(ed = ep_add_ed (urb->dev, pipe, urb->interval, 1, mem_flags))) {
-		usb_dec_dev_use (urb->dev);	
+		usb_put_dev (urb->dev);	
 		usb_put_urb (urb);
 		return -ENOMEM;
 	}
@@ -606,7 +606,7 @@ static int sohci_submit_urb (struct urb * urb, int mem_flags)
 		case PIPE_ISOCHRONOUS: /* number of packets from URB */
 			size = urb->number_of_packets;
 			if (size <= 0) {
-				usb_dec_dev_use (urb->dev);	
+				usb_put_dev (urb->dev);	
 				usb_put_urb (urb);
 				return -EINVAL;
 			}
@@ -627,7 +627,7 @@ static int sohci_submit_urb (struct urb * urb, int mem_flags)
 	/* allocate the private part of the URB */
 	urb_priv = kmalloc (sizeof (urb_priv_t) + size * sizeof (td_t *), mem_flags);
 	if (!urb_priv) {
-		usb_dec_dev_use (urb->dev);	
+		usb_put_dev (urb->dev);	
 		usb_put_urb (urb);
 		return -ENOMEM;
 	}
@@ -645,7 +645,7 @@ static int sohci_submit_urb (struct urb * urb, int mem_flags)
 			urb_priv->length = i;
 			urb_free_priv (ohci, urb_priv);
 			spin_unlock_irqrestore (&usb_ed_lock, flags);
-			usb_dec_dev_use (urb->dev);	
+			usb_put_dev (urb->dev);	
 			usb_put_urb (urb);
 			return -ENOMEM;
 		}
@@ -654,7 +654,7 @@ static int sohci_submit_urb (struct urb * urb, int mem_flags)
 	if (ed->state == ED_NEW || (ed->state & ED_DEL)) {
 		urb_free_priv (ohci, urb_priv);
 		spin_unlock_irqrestore (&usb_ed_lock, flags);
-		usb_dec_dev_use (urb->dev);	
+		usb_put_dev (urb->dev);	
 		usb_put_urb (urb);
 		return -EINVAL;
 	}
@@ -677,7 +677,7 @@ static int sohci_submit_urb (struct urb * urb, int mem_flags)
 			if (bustime < 0) {
 				urb_free_priv (ohci, urb_priv);
 				spin_unlock_irqrestore (&usb_ed_lock, flags);
-				usb_dec_dev_use (urb->dev);	
+				usb_put_dev (urb->dev);	
 				usb_put_urb (urb);
 				return bustime;
 			}
@@ -802,7 +802,7 @@ static int sohci_unlink_urb (struct urb * urb)
 					return -ETIMEDOUT;
 				}
 			} else {
-				/* usb_dec_dev_use done in dl_del_list() */
+				/* usb_put_dev done in dl_del_list() */
 				urb->status = -EINPROGRESS;
 				spin_unlock_irqrestore (&usb_ed_lock, flags);
 				return -EINPROGRESS;
@@ -2109,7 +2109,7 @@ static int rh_submit_urb (struct urb * urb)
 #endif
 
 	urb->hcpriv = NULL;
-	usb_dec_dev_use (usb_dev);
+	usb_put_dev (usb_dev);
 	urb->dev = NULL;
 	if (urb->complete)
 	    	urb->complete (urb);
@@ -2129,7 +2129,7 @@ static int rh_unlink_urb (struct urb * urb)
 		ohci->rh.urb = NULL;
 
 		urb->hcpriv = NULL;
-		usb_dec_dev_use(urb->dev);
+		usb_put_dev (urb->dev);
 		urb->dev = NULL;
 		if (urb->transfer_flags & USB_ASYNC_UNLINK) {
 			urb->status = -ECONNRESET;

drivers/usb/host/usb-uhci.c

@@ -1217,7 +1217,7 @@ _static int uhci_unlink_urb_sync (uhci_t *s, struct urb *urb)
 			urb->dev = NULL;
 			urb->complete ((struct urb *) urb);
 		}
-		usb_dec_dev_use (usb_dev);
+		usb_put_dev (usb_dev);
 		usb_put_urb (urb);
 	}
 	else
@@ -1301,7 +1301,7 @@ _static void uhci_cleanup_unlink(uhci_t *s, int force)
 	
 			uhci_urb_dma_unmap(s, urb, urb_priv);
 
-			usb_dec_dev_use (dev);
+			usb_put_dev (dev);
 #ifdef DEBUG_SLAB
 			kmem_cache_free (urb_priv_kmem, urb_priv);
 #else
@@ -1655,7 +1655,7 @@ _static int uhci_submit_urb (struct urb *urb, int mem_flags)
 	/* increment the reference count of the urb, as we now also control it */
 	urb = usb_get_urb (urb);
 
-	usb_inc_dev_use (urb->dev);
+	usb_get_dev (urb->dev);
 
 	spin_lock_irqsave (&s->urb_list_lock, flags);
 
@@ -1669,7 +1669,7 @@ _static int uhci_submit_urb (struct urb *urb, int mem_flags)
 		    ((type == PIPE_BULK) &&
 		     (!(urb->transfer_flags & USB_QUEUE_BULK) || !(queued_urb->transfer_flags & USB_QUEUE_BULK)))) {
 			spin_unlock_irqrestore (&s->urb_list_lock, flags);
-			usb_dec_dev_use (urb->dev);
+			usb_put_dev (urb->dev);
 			usb_put_urb (urb);
 			err("ENXIO %08x, flags %x, urb %p, burb %p",urb->pipe,urb->transfer_flags,urb,queued_urb);
 			return -ENXIO;	// urb already queued
@@ -1682,7 +1682,7 @@ _static int uhci_submit_urb (struct urb *urb, int mem_flags)
 	urb_priv = kmalloc (sizeof (urb_priv_t), mem_flags);
 #endif
 	if (!urb_priv) {
-		usb_dec_dev_use (urb->dev);
+		usb_put_dev (urb->dev);
 		spin_unlock_irqrestore (&s->urb_list_lock, flags);
 		usb_put_urb (urb);
 		return -ENOMEM;
@@ -1767,7 +1767,7 @@ _static int uhci_submit_urb (struct urb *urb, int mem_flags)
 	
 	if (ret != 0) {
 		uhci_urb_dma_unmap(s, urb, urb_priv);
-		usb_dec_dev_use (urb->dev);
+		usb_put_dev (urb->dev);
 #ifdef DEBUG_SLAB
 		kmem_cache_free(urb_priv_kmem, urb_priv);
 #else
@@ -2737,7 +2737,7 @@ _static int process_urb (uhci_t *s, struct list_head *p)
 				spin_lock(&s->urb_list_lock);
 			}
 			
-			usb_dec_dev_use (usb_dev);
+			usb_put_dev (usb_dev);
 			usb_put_urb (urb);
 		}
 	}

drivers/usb/media/usbvideo.c

@@ -970,7 +970,7 @@ static void usbvideo_Disconnect(struct usb_device *dev, void *ptr)
 	for (i=0; i < USBVIDEO_NUMSBUF; i++)
 		usb_free_urb(uvd->sbuf[i].urb);
 
-	usb_dec_dev_use(uvd->dev);
+	usb_put_dev(uvd->dev);
 	uvd->dev = NULL;    	    /* USB device is no more */
 
 	video_unregister_device(&uvd->vdev);
@@ -1176,7 +1176,7 @@ int usbvideo_RegisterVideoDevice(uvd_t *uvd)
 	}
 #endif
 
-	usb_inc_dev_use(uvd->dev);
+	usb_get_dev(uvd->dev);
 	return 0;
 }
 

drivers/usb/net/cdc-ether.c

@@ -1256,7 +1256,7 @@ static void * CDCEther_probe( struct usb_device *usb, unsigned int ifnum,
 	                            ether_dev );
 
 	// Does this REALLY do anything???
-	usb_inc_dev_use( usb );
+	usb_get_dev( usb );
 
 	// TODO - last minute HACK
 	ether_dev->comm_ep_in = 5;
@@ -1298,7 +1298,7 @@ static void CDCEther_disconnect( struct usb_device *usb, void *ptr )
 	ether_dev->net = NULL;
 
 	// I ask again, does this do anything???
-	usb_dec_dev_use( usb );
+	usb_put_dev( usb );
 
 	// We are done with this interface
 	usb_driver_release_interface( &CDCEther_driver, 

drivers/usb/net/pegasus.c

@@ -981,7 +981,7 @@ static void *pegasus_probe(struct usb_device *dev, unsigned int ifnum,
 		return NULL;
 	}
 
-	usb_inc_dev_use(dev);
+	usb_get_dev(dev);
 	memset(pegasus, 0, sizeof(struct pegasus));
 	pegasus->dev_index = dev_index;
 	init_waitqueue_head(&pegasus->ctrl_wait);
@@ -1086,7 +1086,7 @@ static void pegasus_disconnect(struct usb_device *dev, void *ptr)
 
 	pegasus->flags |= PEGASUS_UNPLUG;
 	unregister_netdev(pegasus->net);
-	usb_dec_dev_use(dev);
+	usb_put_dev(dev);
 	usb_unlink_urb(pegasus->intr_urb);
 	usb_unlink_urb(pegasus->tx_urb);
 	usb_unlink_urb(pegasus->rx_urb);

drivers/usb/net/usbnet.c

@@ -1951,7 +1951,7 @@ static void usbnet_disconnect (struct usb_device *udev, void *ptr)
 	flush_scheduled_tasks ();
 
 	kfree (dev);
-	usb_dec_dev_use (udev);
+	usb_put_dev (udev);
 }
 
 
@@ -1997,7 +1997,7 @@ usbnet_probe (struct usb_device *udev, unsigned ifnum,
 	memset (dev, 0, sizeof *dev);
 
 	init_MUTEX_LOCKED (&dev->mutex);
-	usb_inc_dev_use (udev);
+	usb_get_dev (udev);
 	dev->udev = udev;
 	dev->driver_info = info;
 	dev->msg_level = msg_level;

drivers/usb/storage/usb.c

@@ -676,7 +676,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 	}
 
 	/* At this point, we're committed to using the device */
-	usb_inc_dev_use(dev);
+	usb_get_dev(dev);
 
 	/* clear the GUID and fetch the strings */
 	GUID_CLEAR(guid);
@@ -735,14 +735,14 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 
 		/* allocate an IRQ callback if one is needed */
 		if ((ss->protocol == US_PR_CBI) && usb_stor_allocate_irq(ss)) {
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 		}
 
 		/* allocate the URB we're going to use */
 		ss->current_urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (!ss->current_urb) {
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 		}
 
@@ -760,7 +760,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 		if ((ss = (struct us_data *)kmalloc(sizeof(struct us_data), 
 						    GFP_KERNEL)) == NULL) {
 			printk(KERN_WARNING USB_STORAGE "Out of memory\n");
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 		}
 		memset(ss, 0, sizeof(struct us_data));
@@ -769,7 +769,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 		ss->current_urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (!ss->current_urb) {
 			kfree(ss);
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 		}
 
@@ -919,7 +919,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 			ss->transport_name = "Unknown";
 			kfree(ss->current_urb);
 			kfree(ss);
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 			break;
 		}
@@ -974,7 +974,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 			ss->protocol_name = "Unknown";
 			kfree(ss->current_urb);
 			kfree(ss);
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 			break;
 		}
@@ -984,7 +984,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 		if ((ss->protocol == US_PR_CBI) && usb_stor_allocate_irq(ss)) {
 			kfree(ss->current_urb);
 			kfree(ss);
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 		}
 
@@ -1020,7 +1020,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
 			       "Unable to start control thread\n");
 			kfree(ss->current_urb);
 			kfree(ss);
-			usb_dec_dev_use(dev);
+			usb_put_dev(dev);
 			return NULL;
 		}
 
@@ -1087,7 +1087,7 @@ static void storage_disconnect(struct usb_device *dev, void *ptr)
 	ss->current_urb = NULL;
 
 	/* mark the device as gone */
-	usb_dec_dev_use(ss->pusb_dev);
+	usb_put_dev(ss->pusb_dev);
 	ss->pusb_dev = NULL;
 	atomic_set(&ss->sm_state, US_STATE_DETACHED);
 

include/linux/usb.h

@@ -426,10 +426,10 @@ struct usb_device {
 	struct usb_device *children[USB_MAXCHILDREN];
 };
 
-/* usb_free_dev can be called anywhere from usb_dec_dev_use */
-extern struct usb_device *usb_alloc_dev(struct usb_device *parent,
-	struct usb_bus *);
+extern struct usb_device *usb_alloc_dev(struct usb_device *parent, struct usb_bus *);
+extern struct usb_device *usb_get_dev(struct usb_device *dev);
 extern void usb_free_dev(struct usb_device *);
+#define usb_put_dev usb_free_dev
 
 /* for when layers above USB add new non-USB drivers */
 extern void usb_scan_devices(void);
@@ -440,34 +440,6 @@ extern int usb_reset_device(struct usb_device *dev);
 /* for drivers using iso endpoints */
 extern int usb_get_current_frame_number (struct usb_device *usb_dev);
 
-/**
- * usb_inc_dev_use - record another reference to a device
- * @dev: the device being referenced
- *
- * Each live reference to a device should be refcounted.
- *
- * Drivers for USB interfaces should normally record such references in
- * their probe() methods, when they bind to an interface, and release
- * them usb_dec_dev_use(), in their disconnect() methods.
- */
-static inline void usb_inc_dev_use (struct usb_device *dev)
-{
-	atomic_inc (&dev->refcnt);
-}
-
-/**
- * usb_dec_dev_use - drop a reference to a device
- * @dev: the device no longer being referenced
- *
- * Each live reference to a device should be refcounted.
- */
-static inline void usb_dec_dev_use (struct usb_device *dev)
-{
-	if (atomic_dec_and_test(&dev->refcnt))
-		usb_free_dev(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,