ChangeSet 1.1371.759.10, 2004/04/23 15:45:24-07:00, [EMAIL PROTECTED]

[PATCH] USB: khubd fixes

This goes on top of the other enumeration patch I just sent,
to handle some dubious and/or broken hub configurations better.


Make khubd handle some cases better:

 - Track power budget for bus-powered hubs.  This version only warns
   when the budgets are exceeded.  Eventually, the budgets should help
   prevent such errors.

 - Rejects illegal USB setup:  two consecutive bus powered hubs
   would exceed the voltage drop budget, causing much flakiness.

 - For hosts with high speed hubs, warn when devices are hooked up
   to full speed hubs if they'd be faster on a high speed one.

 - For hubs that don't do power switching, don't try to use it

 - For hubs that aren't self-powered, don't report local power status


 drivers/usb/core/hub.c |  158 ++++++++++++++++++++++++++++++++++++++++++++-----
 drivers/usb/core/hub.h |    2 
 2 files changed, 145 insertions(+), 15 deletions(-)


diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c    Fri May 14 15:34:04 2004
+++ b/drivers/usb/core/hub.c    Fri May 14 15:34:04 2004
@@ -373,12 +373,13 @@
        struct usb_device *dev;
        int i;
 
-       /* Enable power to the ports */
-       dev_dbg(hubdev(interface_to_usbdev(hub->intf)),
-               "enabling power on all ports\n");
-       dev = interface_to_usbdev(hub->intf);
-       for (i = 0; i < hub->descriptor->bNbrPorts; i++)
-               set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
+       /* if hub supports power switching, enable power on each port */
+       if ((hub->descriptor->wHubCharacteristics & HUB_CHAR_LPSM) < 2) {
+               dev_dbg(&hub->intf->dev, "enabling power on all ports\n");
+               dev = interface_to_usbdev(hub->intf);
+               for (i = 0; i < hub->descriptor->bNbrPorts; i++)
+                       set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
+       }
 
        /* Wait for power to be enabled */
        wait_ms(hub->descriptor->bPwrOn2PwrGood * 2);
@@ -545,8 +546,25 @@
 
        dev_dbg(hub_dev, "power on to power good time: %dms\n",
                hub->descriptor->bPwrOn2PwrGood * 2);
-       dev_dbg(hub_dev, "hub controller current requirement: %dmA\n",
-               hub->descriptor->bHubContrCurrent);
+
+       /* power budgeting mostly matters with bus-powered hubs,
+        * and battery-powered root hubs (may provide just 8 mA).
+        */
+       ret = usb_get_status(dev, USB_RECIP_DEVICE, 0, &hubstatus);
+       if (ret < 0) {
+               message = "can't get hubdev status";
+               goto fail;
+       }
+       cpu_to_le16s(&hubstatus);
+       if ((hubstatus & (1 << USB_DEVICE_SELF_POWERED)) == 0) {
+               dev_dbg(hub_dev, "hub controller current requirement: %dmA\n",
+                       hub->descriptor->bHubContrCurrent);
+               hub->power_budget = (501 - hub->descriptor->bHubContrCurrent)
+                                       / 2;
+               dev_dbg(hub_dev, "%dmA bus power budget for children\n",
+                       hub->power_budget * 2);
+       }
+
 
        ret = hub_hub_status(hub, &hubstatus, &hubchange);
        if (ret < 0) {
@@ -554,12 +572,11 @@
                goto fail;
        }
 
-       /* FIXME implement per-port power budgeting;
-        * enable it for bus-powered hubs.
-        */
-       dev_dbg(hub_dev, "local power source is %s\n",
-               (hubstatus & HUB_STATUS_LOCAL_POWER)
-               ? "lost (inactive)" : "good");
+       /* local power status reports aren't always correct */
+       if (dev->actconfig->desc.bmAttributes & USB_CONFIG_ATT_SELFPOWER)
+               dev_dbg(hub_dev, "local power source is %s\n",
+                       (hubstatus & HUB_STATUS_LOCAL_POWER)
+                       ? "lost (inactive)" : "good");
 
        if ((hub->descriptor->wHubCharacteristics & HUB_CHAR_OCPM) == 0)
                dev_dbg(hub_dev, "%sover-current condition exists\n",
@@ -611,6 +628,8 @@
        return ret;
 }
 
+static unsigned highspeed_hubs;
+
 static void hub_disconnect(struct usb_interface *intf)
 {
        struct usb_hub *hub = usb_get_intfdata (intf);
@@ -620,6 +639,9 @@
        if (!hub)
                return;
 
+       if (interface_to_usbdev(intf)->speed == USB_SPEED_HIGH)
+               highspeed_hubs--;
+
        usb_set_intfdata (intf, NULL);
        spin_lock_irqsave(&hub_event_lock, flags);
        hub->urb_complete = &urb_complete;
@@ -731,6 +753,9 @@
 
        usb_set_intfdata (intf, hub);
 
+       if (dev->speed == USB_SPEED_HIGH)
+               highspeed_hubs++;
+
        if (hub_configure(hub, endpoint) >= 0)
                return 0;
 
@@ -1178,6 +1203,63 @@
        return 0;
 }
 
+static void
+check_highspeed (struct usb_hub *hub, struct usb_device *dev, int port)
+{
+       struct usb_qualifier_descriptor *qual;
+       int                             status;
+
+       qual = kmalloc (sizeof *qual, SLAB_KERNEL);
+       if (qual == 0)
+               return;
+
+       status = usb_get_descriptor (dev, USB_DT_DEVICE_QUALIFIER, 0,
+                       qual, sizeof *qual);
+       if (status == sizeof *qual) {
+               dev_info(&dev->dev, "not running at top speed; "
+                       "connect to a high speed hub\n");
+               /* hub LEDs are probably harder to miss than syslog */
+               if (hub->has_indicators) {
+                       hub->indicator[port] = INDICATOR_GREEN_BLINK;
+                       schedule_work (&hub->leds);
+               }
+       }
+       kfree (qual);
+}
+
+static unsigned
+hub_power_remaining (struct usb_hub *hubstate, struct usb_device *hub)
+{
+       int remaining;
+       unsigned i;
+
+       remaining = hubstate->power_budget;
+       if (!remaining)         /* self-powered */
+               return 0;
+
+       for (i = 0; i < hub->maxchild; i++) {
+               struct usb_device       *dev = hub->children[i];
+               int                     delta;
+
+               if (!dev)
+                       continue;
+
+               if (dev->actconfig)
+                       delta = dev->actconfig->desc.bMaxPower;
+               else
+                       delta = 50;
+               // dev_dbg(&dev->dev, "budgeted %dmA\n", 2 * delta);
+               remaining -= delta;
+       }
+       if (remaining < 0) {
+               dev_warn(&hubstate->intf->dev,
+                       "%dmA over power budget!\n",
+                       -2 * remaining);
+               remaining = 0;
+       }
+       return remaining;
+}
+ 
 static void hub_port_connect_change(struct usb_hub *hubstate, int port,
                                        u16 portstatus, u16 portchange)
 {
@@ -1241,17 +1323,63 @@
                        break;
                if (status < 0)
                        continue;
+
+               /* consecutive bus-powered hubs aren't reliable; they can
+                * violate the voltage drop budget.  if the new child has
+                * a "powered" LED, users should notice we didn't enable it
+                * (without reading syslog), even without per-port LEDs
+                * on the parent.
+                */
+               if (dev->descriptor.bDeviceClass == USB_CLASS_HUB
+                               && hubstate->power_budget) {
+                       u16     devstat;
+
+                       status = usb_get_status(dev, USB_RECIP_DEVICE, 0,
+                                       &devstat);
+                       if (status < 0) {
+                               dev_dbg(&dev->dev, "get status %d ?\n", status);
+                               continue;
+                       }
+                       cpu_to_le16s(&hubstatus);
+                       if ((devstat & (1 << USB_DEVICE_SELF_POWERED)) == 0) {
+                               dev_err(&dev->dev,
+                                       "can't connect bus-powered hub "
+                                       "to this port\n");
+                               if (hubstate->has_indicators) {
+                                       hubstate->indicator[port] =
+                                               INDICATOR_AMBER_BLINK;
+                                       schedule_work (&hubstate->leds);
+                               }
+                               hub->children[port] = NULL;
+                               usb_put_dev(dev);
+                               hub_port_disable(hub, port);
+                               return;
+                       }
+               }
  
+               /* check for devices running slower than they could */
+               if (dev->descriptor.bcdUSB >= 0x0200
+                               && dev->speed == USB_SPEED_FULL
+                               && highspeed_hubs != 0)
+                       check_highspeed (hubstate, dev, port);
+
                /* Run it through the hoops (find a driver, etc) */
                status = usb_new_device(dev);
                if (status != 0) {
                        hub->children[port] = NULL;
                        continue;
                }
-
                up (&dev->serialize);
+
+               status = hub_power_remaining(hubstate, hub);
+               if (status)
+                       dev_dbg(&hubstate->intf->dev,
+                               "%dmA power budget left\n",
+                               2 * status);
+
                return;
        }
+ 
 done:
        hub_port_disable(hub, port);
 }
diff -Nru a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
--- a/drivers/usb/core/hub.h    Fri May 14 15:34:04 2004
+++ b/drivers/usb/core/hub.h    Fri May 14 15:34:04 2004
@@ -209,6 +209,8 @@
        struct semaphore        khubd_sem;
        struct usb_tt           tt;             /* Transaction Translator */
 
+       u8                      power_budget;   /* in 2mA units; or zero */
+
        unsigned                has_indicators:1;
        enum hub_led_mode       indicator[USB_MAXCHILDREN];
        struct work_struct      leds;



-------------------------------------------------------
This SF.Net email is sponsored by: SourceForge.net Broadband
Sign-up now for SourceForge Broadband and get the fastest
6.0/768 connection for only $19.95/mo for the first 3 months!
http://ads.osdn.com/?ad_id%62&alloc_ida84&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to