[PATCH] driverfs support for USB

[PATCH] driverfs support for USB

[Greg KH]

Hi,

Here's a first cut at adding driverfs support to the USB core code.
This patch is against 2.5.3-pre5.  I've tested this out with the
ehci-hcd and uhci drivers, and everything seems to work ok (it doesn't
oops for me anymore :)

Right now all devices show up under the pci usb controller directory,
just like usbfs works.  I'll be changing things so that hubs are iobus
structures too, which should allow us to show the topology of the trees
correctly (unless Pat changes all devices to be iobusses too :)

Pat,
You need to export the put_device and put_iobus functions.  The end of
this patch has that fix.  Also, put_iobus doesn't seem to remove the
directory that iobus_register created (but a later call to
iobus_register() seems to place everything in the same place again.)  Is
this the correct thing that should be happening?

Just to show what this looks like, here is what driverfs looks like on
my box right now (device pci0/pci1/usb/002 is a hub that device 003 is
hanging off of).

[greg@soap x]$ tree
.
`-- root
    `-- pci0
        |-- 00:00.0
        |   |-- power
        |   `-- status
        |-- 00:01.0
        |   |-- power
        |   `-- status
        |-- 00:1e.0
        |   |-- power
        |   `-- status
        |-- 00:1f.0
        |   |-- power
        |   `-- status
        |-- 00:1f.1
        |   |-- power
        |   `-- status
        |-- 00:1f.2
        |   |-- power
        |   `-- status
        |-- 00:1f.3
        |   |-- power
        |   `-- status
        |-- 00:1f.5
        |   |-- power
        |   `-- status
        |-- pci1
        |   |-- 01:08.0
        |   |   |-- power
        |   |   `-- status
        |   |-- 01:0d.2
        |   |   |-- power
        |   |   `-- status
        |   `-- usb
        |       |-- 002
        |       |   |-- power
        |       |   `-- status
        |       `-- 003
        |           |-- power
        |           `-- status
        |-- pci2
        |   `-- 02:00.0
        |       |-- power
        |       `-- status
        `-- usb
            `-- 002
                |-- power
                `-- status


If anyone has any problems or fixes to this patch, please let me know.

thanks,

greg k-h


diff -Nru a/drivers/usb/hcd.c b/drivers/usb/hcd.c
--- a/drivers/usb/hcd.c	Thu Jan 24 17:26:21 2002
+++ b/drivers/usb/hcd.c	Thu Jan 24 17:26:21 2002
@@ -666,6 +666,12 @@
 	if ((retval = driver->start (hcd)) < 0)
 		usb_hcd_pci_remove (dev);
 
+	/* initialize the iobus info for this usb pci device */
+	bus->iobus.self = &dev->dev;
+	bus->iobus.parent = bus->iobus.self->parent;
+	strcpy (bus->iobus.bus_id, "usb");
+	iobus_register (&bus->iobus);
+
 	return retval;
 } 
 EXPORT_SYMBOL (usb_hcd_pci_probe);
diff -Nru a/drivers/usb/hub.c b/drivers/usb/hub.c
--- a/drivers/usb/hub.c	Thu Jan 24 17:26:21 2002
+++ b/drivers/usb/hub.c	Thu Jan 24 17:26:21 2002
@@ -722,8 +722,16 @@
 			dev->bus->busnum, dev->devpath, dev->devnum);
 
 		/* Run it through the hoops (find a driver, etc) */
-		if (!usb_new_device(dev))
+		if (!usb_new_device(dev)) {
+			/* put the device in the global device tree */
+			dev->dev.parent = &dev->bus->iobus;
+			sprintf (&dev->dev.name[0], "USB device %04x:%04x",
+				 dev->descriptor.idVendor,
+				 dev->descriptor.idProduct);
+			sprintf (&dev->dev.bus_id[0], "%03d", dev->devnum);
+			device_register (&dev->dev);
 			goto done;
+		}
 
 		/* Free the configuration if there was an error */
 		usb_free_dev(dev);
diff -Nru a/drivers/usb/uhci.c b/drivers/usb/uhci.c
--- a/drivers/usb/uhci.c	Thu Jan 24 17:26:21 2002
+++ b/drivers/usb/uhci.c	Thu Jan 24 17:26:21 2002
@@ -2843,6 +2847,12 @@
 		retval = -ENOMEM;
 		goto err_start_root_hub;
 	}
+
+	/* initialize the iobus info for this usb pci device */
+	bus->iobus.self = &dev->dev;
+	bus->iobus.parent = bus->iobus.self->parent;
+	strcpy (bus->iobus.bus_id, "usb");
+	iobus_register (&bus->iobus);
 
 	return 0;
 
diff -Nru a/drivers/usb/usb-ohci.c b/drivers/usb/usb-ohci.c
--- a/drivers/usb/usb-ohci.c	Thu Jan 24 17:26:21 2002
+++ b/drivers/usb/usb-ohci.c	Thu Jan 24 17:26:21 2002
@@ -2508,6 +2508,13 @@
 #ifdef	DEBUG
 	ohci_dump (ohci, 1);
 #endif
+
+	/* initialize the iobus info for this usb pci device */
+	ohci->bus->iobus.self = &dev->dev;
+	ohci->bus->iobus.parent = ohci->bus->iobus.self->parent;
+	strcpy (ohci->bus->iobus.bus_id, "usb");
+	iobus_register (&ohci->bus->iobus);
+
 	return 0;
 }
 
diff -Nru a/drivers/usb/usb-uhci.c b/drivers/usb/usb-uhci.c
--- a/drivers/usb/usb-uhci.c	Thu Jan 24 17:26:20 2002
+++ b/drivers/usb/usb-uhci.c	Thu Jan 24 17:26:20 2002
@@ -3015,6 +3027,12 @@
 	//chain new uhci device into global list
 	pci_set_drvdata(dev, s);
 	devs=s;
+
+	/* initialize the iobus info for this usb pci device */
+	bus->iobus.self = &dev->dev;
+	bus->iobus.parent = bus->iobus.self->parent;
+	strcpy (bus->iobus.bus_id, "usb");
+	iobus_register (&bus->iobus);
 
 	return 0;
 }
diff -Nru a/drivers/usb/usb.c b/drivers/usb/usb.c
--- a/drivers/usb/usb.c	Thu Jan 24 17:26:21 2002
+++ b/drivers/usb/usb.c	Thu Jan 24 17:26:21 2002
@@ -502,6 +503,7 @@
 	up (&usb_bus_list_lock);
 
 	usbfs_remove_bus(bus);
+	put_iobus(&bus->iobus);
 
 	clear_bit(bus->busnum, busmap.busmap);
 
@@ -1924,6 +1926,7 @@
 	if (dev->devnum > 0) {
 		clear_bit(dev->devnum, &dev->bus->devmap.devicemap);
 		usbfs_remove_device(dev);
+		put_device(&dev->dev);
 	}
 
 	/* Free up the device itself */
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h	Thu Jan 24 17:26:21 2002
+++ b/include/linux/usb.h	Thu Jan 24 17:26:21 2002
@@ -1,6 +1,8 @@
 #ifndef __LINUX_USB_H
 #define __LINUX_USB_H
 
+#include <linux/device.h>
+
 /* USB constants */
 
 /*
@@ -935,6 +938,8 @@
 	struct dentry *dentry;		/* usbfs dentry entry for the bus */
 
 	atomic_t refcnt;
+
+	struct iobus iobus;		/* Generic device interface */
 };
 
 extern struct usb_bus *usb_alloc_bus(struct usb_operations *);
@@ -1037,6 +1042,8 @@
 	struct usb_device *parent;
 	struct usb_bus *bus;		/* Bus we're part of */
 
+	struct device dev;		/* Generic device interface */
+
 	struct usb_device_descriptor descriptor;/* Descriptor */
 	struct usb_config_descriptor *config;	/* All of the configs */
 	struct usb_config_descriptor *actconfig;/* the active configuration */
diff -Nru a/kernel/device.c b/kernel/device.c
--- a/kernel/device.c	Thu Jan 24 17:26:21 2002
+++ b/kernel/device.c	Thu Jan 24 17:26:21 2002
@@ -308,7 +308,7 @@
 }
 
 /**
- * iobus_unregister - remove bus and children from device tree
+ * put_iobus - remove bus and children from device tree
  * @bus:	pointer to bus structure
  *
  * Remove device from parent's list of children and decrement
@@ -521,7 +521,9 @@
 __setup("device=",device_setup);
 
 EXPORT_SYMBOL(device_register);
+EXPORT_SYMBOL(put_device);
 EXPORT_SYMBOL(device_create_file);
 EXPORT_SYMBOL(device_remove_file);
 EXPORT_SYMBOL(iobus_register);
+EXPORT_SYMBOL(put_iobus);
 EXPORT_SYMBOL(device_driver_init);

Re: [PATCH] driverfs support for USB

[Patrick Mochel]

Hi there. 

On Thu, 24 Jan 2002, Greg KH wrote:

> Hi,
> 
> Here's a first cut at adding driverfs support to the USB core code.
> This patch is against 2.5.3-pre5.  I've tested this out with the
> ehci-hcd and uhci drivers, and everything seems to work ok (it doesn't
> oops for me anymore :)
> 
> Right now all devices show up under the pci usb controller directory,
> just like usbfs works.  I'll be changing things so that hubs are iobus
> structures too, which should allow us to show the topology of the trees
> correctly (unless Pat changes all devices to be iobusses too :)

This looks good; thanks. A couple of things:

- For the record, in order to register a device in the device tree, it 
must know it's parent and have a unique, non-NULL ->bus_id. (Unique only 
for the on which it resides).

For an iobus to be registered, it must have a unique, non-NULL bus_id. If 
possible, it should know its parent and self. If an iobus is an orphan, it 
gets adopted by the system root. If it doesn't know it's self, well it has 
identity problems, but nothing fatal. :)

- You're naming every usb iobus 'usb'. Is it possible for there to be more 
than one usb controller on the same bus? 

> Pat,
> You need to export the put_device and put_iobus functions.  The end of
> this patch has that fix.  Also, put_iobus doesn't seem to remove the
> directory that iobus_register created (but a later call to
> iobus_register() seems to place everything in the same place again.)  Is
> this the correct thing that should be happening?

Thanks for catching that. It's fixed in my tree. I have a couple of other 
changes pending with Linus, so I'm waiting for him to release -pre6 to 
sync before I send him anything else :)

	-pat

Re: [PATCH] driverfs support for USB

[Pavel Machek]

Hi!

> .
> --- root
>     --- pci0
>         |-- 00:00.0
>         |   |-- power
>         |   --- status
>         |-- 00:01.0
>         |   |-- power
>         |   --- status
>         |-- 00:1e.0
>         |   |-- power
>         |   --- status
>         |-- 00:1f.0
>         |   |-- power
>         |   --- status
>         |-- 00:1f.1
>         |   |-- power
>         |   --- status
>         |-- 00:1f.2
>         |   |-- power
>         |   --- status
>         |-- 00:1f.3
>         |   |-- power
>         |   --- status
>         |-- 00:1f.5
>         |   |-- power
>         |   --- status
>         |-- pci1
>         |   |-- 01:08.0
>         |   |   |-- power
>         |   |   --- status
>         |   |-- 01:0d.2
>         |   |   |-- power
>         |   |   --- status
>         |   --- usb
>         |       |-- 002
>         |       |   |-- power
>         |       |   --- status
>         |       --- 003
>         |           |-- power
>         |           --- status
>         |-- pci2
>         |   --- 02:00.0
>         |       |-- power
>         |       --- status
>         --- usb
>             --- 002
>                 |-- power
>                 --- status

Should not usb bus hang off some pci device (uhci/ohci?)

						Pavel

-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: [PATCH] driverfs support for USB

[Greg KH]

On Mon, Jan 28, 2002 at 12:52:04PM +0000, Pavel Machek wrote:
> Hi!
> 
> > .
> >         |-- pci1
> >         |   |-- 01:08.0
> >         |   |   |-- power
> >         |   |   --- status
> >         |   |-- 01:0d.2
> >         |   |   |-- power
> >         |   |   --- status
> >         |   --- usb
> >         |       |-- 002
> >         |       |   |-- power
> >         |       |   --- status
> >         |       --- 003
> >         |           |-- power
> >         |           --- status
> 
> Should not usb bus hang off some pci device (uhci/ohci?)

You are correct, The "usb" device should hang off of the actual pci
device that is the USB host controller.  But due to the current split
between iobus and devices in the device core code, a usb bus has to
attach to the pci bus, which places it in the tree like you see here.

So to be correct the above portion of the tree should look something
like:
        |-- pci1
        |   |-- 01:08.0
        |   |   |-- power
        |   |   --- status
        |   |-- 01:0d.2
        |       |-- power
        |       |-- status
        |       --- usb
        |           |-- 002
        |               |-- power
        |               |-- status
        |               --- 003
        |                   |-- power
        |                   --- status


It's still a work in progress :)

thanks,

greg k-h

[PATCH] driverfs support for USB

[Greg KH]

Ah, finally got something to work properly...

Here's a patch against 2.5.4-pre2 that adds driverfs support to the USB
subsystem.  It differs from my previous patches, in that this one works
well :)

First off, here's what a USB tree on my machine looks like right now:
    |   |-- 00:1f.2
    |   |   |-- irq
    |   |   |-- name
    |   |   |-- power
    |   |   |-- resources
    |   |   |-- status
    |   |   `-- usb_bus
    |   |       |-- 000
    |   |       |   |-- name
    |   |       |   |-- power
    |   |       |   `-- status
    |   |       |-- 01
    |   |       |   |-- 000
    |   |       |   |   |-- name
    |   |       |   |   |-- power
    |   |       |   |   `-- status
    |   |       |   |-- 01
    |   |       |   |   |-- 000
    |   |       |   |   |   |-- name
    |   |       |   |   |   |-- power
    |   |       |   |   |   `-- status
    |   |       |   |   |-- name
    |   |       |   |   |-- power
    |   |       |   |   `-- status
    |   |       |   |-- name
    |   |       |   |-- power
    |   |       |   `-- status
    |   |       |-- name
    |   |       |-- power
    |   |       `-- status

(portions were snipped for clarity.)

What this tree shows is a USB UHCI pci driver attached to the pci
device.  This driver has a root hub, with a single interface (the first
000 directory at the top.)  Then plugged into the second port on the
root hub, is another hub (with it's own interface).  Then plugged into
the second port on that hub, is a USB trackball (again, with only one
interface.)

This now provides a stable device tree that is mapped to the physical
topology of the tree (port numbers), and also shows the fact that USB
drivers bind to the individual USB interfaces, not the devices
themselves (yes, USB topology is odd the first 100 times you look at it,
but after that, you learn to relax and accept it...)

Feel free to play around with this patch, I've sent it on to Linus.

Next up, we start defining the different properties that we want to
export for every USB device, and the different USB drivers :)

thanks,

greg k-h


diff -Nru a/drivers/usb/hcd/ehci-hcd.c b/drivers/usb/hcd/ehci-hcd.c
--- a/drivers/usb/hcd/ehci-hcd.c	Thu Feb  7 13:55:10 2002
+++ b/drivers/usb/hcd/ehci-hcd.c	Thu Feb  7 13:55:10 2002
@@ -277,7 +277,7 @@
 	 */
 	usb_connect (udev);
 	udev->speed = USB_SPEED_HIGH;
-	if (usb_new_device (udev) != 0) {
+	if (usb_register_root_hub (udev, &ehci->hcd.pdev->dev) != 0) {
 		if (hcd->state == USB_STATE_RUNNING)
 			ehci_ready (ehci);
 		while (readl (&ehci->regs->status) & (STS_ASS | STS_PSS))
diff -Nru a/drivers/usb/hcd/ohci-hcd.c b/drivers/usb/hcd/ohci-hcd.c
--- a/drivers/usb/hcd/ohci-hcd.c	Thu Feb  7 13:55:11 2002
+++ b/drivers/usb/hcd/ohci-hcd.c	Thu Feb  7 13:55:11 2002
@@ -469,7 +469,7 @@
 
 	usb_connect (udev);
 	udev->speed = USB_SPEED_FULL;
-	if (usb_new_device (udev) != 0) {
+	if (usb_register_root_hub (udev, &ohci->hcd.pdev->dev) != 0) {
 		usb_free_dev (udev); 
 		ohci->disabled = 1;
 // FIXME cleanup
diff -Nru a/drivers/usb/hub.c b/drivers/usb/hub.c
--- a/drivers/usb/hub.c	Thu Feb  7 13:55:11 2002
+++ b/drivers/usb/hub.c	Thu Feb  7 13:55:11 2002
@@ -721,6 +721,20 @@
 		info("new USB device on bus %d path %s, assigned address %d",
 			dev->bus->busnum, dev->devpath, dev->devnum);
 
+		/* put the device in the global device tree */
+		dev->dev.parent = &dev->parent->dev;
+		sprintf (&dev->dev.name[0], "USB device %04x:%04x",
+			 dev->descriptor.idVendor,
+			 dev->descriptor.idProduct);
+		/* find the number of the port this device is connected to */
+		sprintf (&dev->dev.bus_id[0], "unknown_port_%03d", dev->devnum);
+		for (i = 0; i < USB_MAXCHILDREN; ++i) {
+			if (dev->parent->children[i] == dev) {
+				sprintf (&dev->dev.bus_id[0], "%02d", i);
+				break;
+			}
+		}
+
 		/* Run it through the hoops (find a driver, etc) */
 		if (!usb_new_device(dev))
 			goto done;
diff -Nru a/drivers/usb/uhci.c b/drivers/usb/uhci.c
--- a/drivers/usb/uhci.c	Thu Feb  7 13:55:11 2002
+++ b/drivers/usb/uhci.c	Thu Feb  7 13:55:11 2002
@@ -2842,7 +2842,7 @@
 
 	usb_connect(uhci->rh.dev);
 
-	if (usb_new_device(uhci->rh.dev) != 0) {
+	if (usb_register_root_hub(uhci->rh.dev, &dev->dev) != 0) {
 		err("unable to start root hub");
 		retval = -ENOMEM;
 		goto err_start_root_hub;
diff -Nru a/drivers/usb/usb-ohci.c b/drivers/usb/usb-ohci.c
--- a/drivers/usb/usb-ohci.c	Thu Feb  7 13:55:10 2002
+++ b/drivers/usb/usb-ohci.c	Thu Feb  7 13:55:10 2002
@@ -2231,7 +2231,7 @@
 	dev = usb_to_ohci (usb_dev);
 	ohci->bus->root_hub = usb_dev;
 	usb_connect (usb_dev);
-	if (usb_new_device (usb_dev) != 0) {
+	if (usb_register_root_hub (usb_dev, &ohci->ohci_dev->dev) != 0) {
 		usb_free_dev (usb_dev); 
 		ohci->disabled = 1;
 		return -ENODEV;
diff -Nru a/drivers/usb/usb-uhci.c b/drivers/usb/usb-uhci.c
--- a/drivers/usb/usb-uhci.c	Thu Feb  7 13:55:10 2002
+++ b/drivers/usb/usb-uhci.c	Thu Feb  7 13:55:10 2002
@@ -2907,7 +2907,7 @@
 	s->bus->root_hub = usb_dev;
 	usb_connect (usb_dev);
 
-	if (usb_new_device (usb_dev) != 0) {
+	if (usb_register_root_hub (usb_dev, &s->uhci_pci->dev) != 0) {
 		usb_free_dev (usb_dev);
 		return -1;
 	}
diff -Nru a/drivers/usb/usb.c b/drivers/usb/usb.c
--- a/drivers/usb/usb.c	Thu Feb  7 13:55:11 2002
+++ b/drivers/usb/usb.c	Thu Feb  7 13:55:11 2002
@@ -980,8 +980,16 @@
 	unsigned claimed = 0;
 
 	for (ifnum = 0; ifnum < dev->actconfig->bNumInterfaces; ifnum++) {
+		struct usb_interface *interface = &dev->actconfig->interface[ifnum];
+		
+		/* register this interface with driverfs */
+		interface->dev.parent = &dev->dev;
+		sprintf (&interface->dev.bus_id[0], "%03d", ifnum);
+		sprintf (&interface->dev.name[0], "figure out some name...");
+		device_register (&interface->dev);
+
 		/* if this interface hasn't already been claimed */
-		if (!usb_interface_claimed(dev->actconfig->interface + ifnum)) {
+		if (!usb_interface_claimed(interface)) {
 			if (usb_find_interface_driver(dev, ifnum))
 				rejected++;
 			else
@@ -1969,8 +1977,10 @@
 				if (driver->owner)
 					__MOD_DEC_USE_COUNT(driver->owner);
 				/* if driver->disconnect didn't release the interface */
-				if (interface->driver)
+				if (interface->driver) {
+					put_device (&interface->dev);
 					usb_driver_release_interface(driver, interface);
+				}
 			}
 		}
 	}
@@ -1989,6 +1999,7 @@
 	if (dev->devnum > 0) {
 		clear_bit(dev->devnum, &dev->bus->devmap.devicemap);
 		usbfs_remove_device(dev);
+		put_device(&dev->dev);
 	}
 
 	/* Free up the device itself */
@@ -2715,6 +2726,11 @@
 		usb_show_string(dev, "SerialNumber", dev->descriptor.iSerialNumber);
 #endif
 
+	/* register this device in the driverfs tree */
+	err = device_register (&dev->dev);
+	if (err)
+		return err;
+
 	/* now that the basic setup is over, add a /proc/bus/usb entry */
 	usbfs_add_device(dev);
 
@@ -2727,6 +2743,29 @@
 	return 0;
 }
 
+/**
+ * usb_register_root_hub - called by a usb host controller to register the root hub device in the system
+ * @usb_dev: the usb root hub device to be registered.
+ * @parent_dev: the parent device of this root hub.
+ *
+ * The USB host controller calls this function to register the root hub
+ * properly with the USB subsystem.  It sets up the device properly in
+ * the driverfs tree, and then calls usb_new_device() to register the
+ * usb device.
+ */
+int usb_register_root_hub (struct usb_device *usb_dev, struct device *parent_dev)
+{
+	int retval;
+
+	usb_dev->dev.parent = parent_dev;
+	strcpy (&usb_dev->dev.name[0], "usb_name");
+	strcpy (&usb_dev->dev.bus_id[0], "usb_bus");
+	retval = usb_new_device (usb_dev);
+	if (retval)
+		put_device (&usb_dev->dev);
+	return retval;
+}
+
 static int usb_open(struct inode * inode, struct file * file)
 {
 	int minor = minor(inode->i_rdev);
@@ -2832,6 +2871,7 @@
 EXPORT_SYMBOL(usb_free_bus);
 EXPORT_SYMBOL(usb_register_bus);
 EXPORT_SYMBOL(usb_deregister_bus);
+EXPORT_SYMBOL(usb_register_root_hub);
 EXPORT_SYMBOL(usb_alloc_dev);
 EXPORT_SYMBOL(usb_free_dev);
 EXPORT_SYMBOL(usb_inc_dev_use);
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h	Thu Feb  7 13:55:11 2002
+++ b/include/linux/usb.h	Thu Feb  7 13:55:11 2002
@@ -1,6 +1,8 @@
 #ifndef __LINUX_USB_H
 #define __LINUX_USB_H
 
+#include <linux/device.h>
+
 /* USB constants */
 
 /*
@@ -260,6 +262,7 @@
 	int max_altsetting;             /* total memory allocated */
  
 	struct usb_driver *driver;	/* driver */
+	struct device dev;		/* interface specific device info */
 	void *private_data;
 };
 
@@ -945,6 +948,7 @@
 extern void usb_free_bus(struct usb_bus *);
 extern void usb_register_bus(struct usb_bus *);
 extern void usb_deregister_bus(struct usb_bus *);
+extern int usb_register_root_hub(struct usb_device *usb_dev, struct device *parent_dev);
 
 extern int usb_check_bandwidth (struct usb_device *dev, struct urb *urb);
 extern void usb_claim_bandwidth (struct usb_device *dev, struct urb *urb,
@@ -1040,6 +1044,8 @@
 
 	struct usb_device *parent;
 	struct usb_bus *bus;		/* Bus we're part of */
+
+	struct device dev;		/* Generic device interface */
 
 	struct usb_device_descriptor descriptor;/* Descriptor */
 	struct usb_config_descriptor *config;	/* All of the configs */