This fixes a couple issues I noted when I finally spent some time
looking at the first version of driverfs support for usb:

- "name" fields (really descriptions) aren't very useful.

     * for devices, they always said "USB device 0000:0000"
         --> Now they'll only say that when there's
             nothing better to be said ...
         --> ... and it's really device 0000:0000!  It was
             using device descriptor fields before they were
             fetched from the device.
         --> Uses product and/or manufacturer strings, which
             most devices have, much like PCI uses the PCI ID
             database (when it's compiled in)

     * for interfaces, it was "figure out some name..."
         --> Now it combines the strings used in the
             usb_make_path() call with interface number
         --> Or in the remote chance a device provides
             an interface string, that's preferred.
         --> In general, I think the driver for each
             interface is best suited to describe it;
             I modified the hub driver to do so.

- "bus_id" field

     * For hub ports, it was wasting code: we know the port
       already, no need to search for it.  Plus, it used
       0-index ids not the 1-index ones matching physical
       labels on hubs, and other user-visible diagnostics.

     * For interfaces, it mixed the device address with the
       interface number ... producing unstable IDs that were
       moreover rather cryptic.  Changed:  "if0" now, using
       the interface ID (not index).

     * For busses, left "usb_bus" alone ... :)

- Adds two files exposing current configuration (for devices)
   and altsetting (for interfaces).

- I was getting a useless diagnostic from the hub driver,
   now it's less useless (it fully identifies the hub)

Please merge to Linus' latest, or let me know if you have
any issues with these changes.

- Dave

--- ./drivers/usb-dist/core/hub.c       Thu May 30 15:16:45 2002
+++ ./drivers/usb/core/hub.c    Thu Jun 27 14:20:41 2002
@@ -490,8 +490,11 @@
        list_add(&hub->hub_list, &hub_list);
        spin_unlock_irqrestore(&hub_event_lock, flags);
 
-       if (usb_hub_configure(hub, endpoint) >= 0)
+       if (usb_hub_configure(hub, endpoint) >= 0) {
+               strcpy (dev->actconfig->interface[i].dev.name,
+                       "Hub/Port Status Changes");
                return hub;
+       }
 
        err("hub configuration failed for device at %s", dev->devpath);
 
@@ -637,7 +640,8 @@
        if (portsts) {
                ret = usb_get_port_status(hub, port + 1, portsts);
                if (ret < 0)
-                       err("%s(%s) failed (err = %d)", __FUNCTION__, hub->devpath, 
ret);
+                       err("%s(%s-%s) failed (err = %d)", __FUNCTION__,
+                               hub->bus->bus_name, hub->devpath, ret);
                else {
                        *status = le16_to_cpu(portsts->wPortStatus);
                        *change = le16_to_cpu(portsts->wPortChange); 
@@ -884,19 +888,11 @@
                info("new USB device %s-%s, assigned address %d",
                        dev->bus->bus_name, dev->devpath, dev->devnum);
 
-               /* put the device in the global device tree */
+               /* put the device in the global device tree. the hub port
+                * is the "bus_id"; hubs show in hierarchy like bridges
+                */
                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;
-                       }
-               }
+               sprintf (&dev->dev.bus_id[0], "%d", port + 1);
 
                /* Run it through the hoops (find a driver, etc) */
                if (!usb_new_device(dev))
--- ./drivers/usb-dist/core/usb.c       Mon Jun  3 10:18:29 2002
+++ ./drivers/usb/core/usb.c    Thu Jun 27 14:26:37 2002
@@ -886,6 +886,46 @@
 
 #endif /* CONFIG_HOTPLUG */
 
+/* driverfs files */
+
+/* devices have one current configuration, with one
+ * or more interfaces that are used concurrently 
+ */
+static ssize_t
+show_config (struct device *dev, char *buf, size_t count, loff_t off)
+{
+       struct usb_device       *udev;
+
+       if (off)
+               return 0;
+       udev = list_entry (dev, struct usb_device, dev);
+       return sprintf (buf, "%u\n", udev->actconfig->bConfigurationValue);
+}
+static struct driver_file_entry usb_config_entry = {
+       name:   "configuration",
+       mode:   S_IRUGO,
+       show:   show_config,
+};
+
+/* interfaces have one current setting; alternates
+ * can have different endpoints and class info.
+ */
+static ssize_t
+show_altsetting (struct device *dev, char *buf, size_t count, loff_t off)
+{
+       struct usb_interface    *interface;
+
+       if (off)
+               return 0;
+       interface = list_entry (dev, struct usb_interface, dev);
+       return sprintf (buf, "%u\n", interface->altsetting->bAlternateSetting);
+}
+static struct driver_file_entry usb_altsetting_entry = {
+       name:   "altsetting",
+       mode:   S_IRUGO,
+       show:   show_altsetting,
+};
+
 
 /*
  * This entrypoint gets called for each new device.
@@ -898,15 +938,35 @@
        unsigned rejected = 0;
        unsigned claimed = 0;
 
+       /* FIXME should get called for each new configuration not just the
+        * first one for a device. switching configs (or altesettings) should
+        * undo driverfs and HCD state for the previous interfaces.
+        */
        for (ifnum = 0; ifnum < dev->actconfig->bNumInterfaces; ifnum++) {
                struct usb_interface *interface = &dev->actconfig->interface[ifnum];
+               struct usb_interface_descriptor *desc = interface->altsetting;
                
                /* register this interface with driverfs */
                interface->dev.parent = &dev->dev;
                interface->dev.bus = &usb_bus_type;
-               sprintf (&interface->dev.bus_id[0], "%03d%03d", dev->devnum,ifnum);
-               sprintf (&interface->dev.name[0], "figure out some name...");
+               sprintf (&interface->dev.bus_id[0], "if%d",
+                       interface->altsetting->bInterfaceNumber);
+               if (!desc->iInterface
+                               || usb_string (dev, desc->iInterface,
+                                       interface->dev.name,
+                                       sizeof interface->dev.name) <= 0) {
+                       /* typically devices won't bother with interface
+                        * descriptions; this is the normal case.  an
+                        * interface's driver might describe it better.
+                        * (also: iInterface is per-altsetting ...)
+                        */
+                       sprintf (&interface->dev.name[0],
+                               "usb-%s-%s interface %d",
+                               dev->bus->bus_name, dev->devpath,
+                               interface->altsetting->bInterfaceNumber);
+               }
                device_register (&interface->dev);
+               device_create_file (&interface->dev, &usb_altsetting_entry);
 
                /* if this interface hasn't already been claimed */
                if (!usb_interface_claimed(interface)) {
@@ -1219,10 +1279,6 @@
        }
 }
 
-/*
- * These are the actual routines to send
- * and receive control messages.
- */
 
 // hub-only!! ... and only exported for reset/reinit path.
 // otherwise used internally, for usb_new_device()
@@ -1234,6 +1290,64 @@
 }
 
 
+/* improve on the default device description, if we can ... and
+ * while we're at it, maybe show the vendor and product strings.
+ */
+static void set_device_description (struct usb_device *dev)
+{
+       char    *buf, *here, *end;
+       int     mfgr = dev->descriptor.iManufacturer;
+       int     prod = dev->descriptor.iProduct;
+
+       /* set default; keep it if there are no strings */
+       sprintf (dev->dev.name, "USB device %04x:%04x",
+                dev->descriptor.idVendor,
+                dev->descriptor.idProduct);
+       if (!mfgr && !prod)
+               return;
+
+       if (!(buf = kmalloc(256, GFP_KERNEL)))
+               return;
+       here = dev->dev.name;
+       end = here + sizeof dev->dev.name - 2;
+       *end = 0;
+
+       /* much like pci ... describe as either:
+        * - both strings:   'product descr (vendor descr)'
+        * - product only:   'product descr (USB device vvvv:pppp)'
+        * - vendor only:    'USB device vvvv:pppp (vendor descr)'
+        * - neither string: 'USB device vvvv:pppp'
+        */
+       if (prod && usb_string (dev, prod, buf, 256) > 0) {
+               strncpy (here, buf, end - here);
+#ifdef DEBUG
+               printk (KERN_INFO "Product: %s\n", buf);
+#endif
+       } else {
+               buf [0] = 0;
+               prod = -1;
+       }
+       here = strchr (here, 0);
+       if (mfgr && usb_string (dev, mfgr, buf, 256) > 0) {
+               *here++ = ' ';
+               *here++ = '(';
+               strncpy (here, buf, end - here - 1);
+               here = strchr (here, 0);
+               *here++ = ')';
+#ifdef DEBUG
+               printk (KERN_INFO "Manufacturer: %s\n", buf);
+#endif
+       } else {
+               if (prod != -1)
+                       snprintf (here, end - here - 1,
+                               " (USB device %04x:%04x)",
+                               dev->descriptor.idVendor,
+                               dev->descriptor.idProduct);
+               /* both strings unavailable, keep the default */
+       }
+       kfree(buf);
+}
+
 /*
  * By the time we get here, the device has gotten a new device ID
  * and is in the default state. We need to identify the thing and
@@ -1331,11 +1445,9 @@
 
        dbg("new device strings: Mfr=%d, Product=%d, SerialNumber=%d",
                dev->descriptor.iManufacturer, dev->descriptor.iProduct, 
dev->descriptor.iSerialNumber);
+       set_device_description (dev);
+
 #ifdef DEBUG
-       if (dev->descriptor.iManufacturer)
-               usb_show_string(dev, "Manufacturer", dev->descriptor.iManufacturer);
-       if (dev->descriptor.iProduct)
-               usb_show_string(dev, "Product", dev->descriptor.iProduct);
        if (dev->descriptor.iSerialNumber)
                usb_show_string(dev, "SerialNumber", dev->descriptor.iSerialNumber);
 #endif
@@ -1344,6 +1456,7 @@
        err = device_register (&dev->dev);
        if (err)
                return err;
+       device_create_file (&dev->dev, &usb_config_entry);
 
        /* now that the basic setup is over, add a /proc/bus/usb entry */
        usbfs_add_device(dev);

Reply via email to