Commit 55c527187c9d78f840b284d596a0b298bc1493af

Authored by Alan Stern
Committed by Greg Kroah-Hartman
1 parent 949bf64311

[PATCH] USB: Consider power budget when choosing configuration

This patch (as609) changes the way we keep track of power budgeting for
USB hubs and devices, and it updates the choose_configuration routine to
take this information into account.  (This is something we should have
been doing all along.)  A new field in struct usb_device holds the amount
of bus current available from the upstream port, and the usb_hub structure
keeps track of the current available for each downstream port.

Two new rules for configuration selection are added:

	Don't select a self-powered configuration when only bus power
	is available.

	Don't select a configuration requiring more bus power than is
	available.

However the first rule is #if-ed out, because I found that the internal
hub in my HP USB keyboard claims that its only configuration is
self-powered.  The rule would prevent the configuration from being chosen,
leaving the hub & keyboard unconfigured.  Since similar descriptor errors
may turn out to be fairly common, it seemed wise not to include a rule
that would break automatic configuration unnecessarily for such devices.

The second rule may also trigger unnecessarily, although this should be
less common.  More likely it will annoy people by sometimes failing to
accept configurations that should never have been chosen in the first
place.

The patch also changes usbcore's reaction when no configuration is
suitable.  Instead of raising an error and rejecting the device, now
the core will simply leave the device unconfigured.  People can always
work around such problems by installing configurations manually through
sysfs.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

Showing 5 changed files with 164 additions and 81 deletions Side-by-side Diff

drivers/usb/core/hcd.c
... ... @@ -1825,8 +1825,6 @@
1825 1825 retval = -ENOMEM;
1826 1826 goto err_allocate_root_hub;
1827 1827 }
1828   - rhdev->speed = (hcd->driver->flags & HCD_USB2) ? USB_SPEED_HIGH :
1829   - USB_SPEED_FULL;
1830 1828  
1831 1829 /* Although in principle hcd->driver->start() might need to use rhdev,
1832 1830 * none of the current drivers do.
... ... @@ -1844,6 +1842,9 @@
1844 1842 dev_dbg(hcd->self.controller, "supports USB remote wakeup\n");
1845 1843 hcd->remote_wakeup = hcd->can_wakeup;
1846 1844  
  1845 + rhdev->speed = (hcd->driver->flags & HCD_USB2) ? USB_SPEED_HIGH :
  1846 + USB_SPEED_FULL;
  1847 + rhdev->bus_mA = min(500u, hcd->power_budget);
1847 1848 if ((retval = register_root_hub(rhdev, hcd)) != 0)
1848 1849 goto err_register_root_hub;
1849 1850  
drivers/usb/core/hub.c
... ... @@ -702,27 +702,41 @@
702 702 * and battery-powered root hubs (may provide just 8 mA).
703 703 */
704 704 ret = usb_get_status(hdev, USB_RECIP_DEVICE, 0, &hubstatus);
705   - if (ret < 0) {
  705 + if (ret < 2) {
706 706 message = "can't get hub status";
707 707 goto fail;
708 708 }
709 709 le16_to_cpus(&hubstatus);
710 710 if (hdev == hdev->bus->root_hub) {
711   - struct usb_hcd *hcd =
712   - container_of(hdev->bus, struct usb_hcd, self);
713   -
714   - hub->power_budget = min(500u, hcd->power_budget) / 2;
  711 + if (hdev->bus_mA == 0 || hdev->bus_mA >= 500)
  712 + hub->mA_per_port = 500;
  713 + else {
  714 + hub->mA_per_port = hdev->bus_mA;
  715 + hub->limited_power = 1;
  716 + }
715 717 } else if ((hubstatus & (1 << USB_DEVICE_SELF_POWERED)) == 0) {
716 718 dev_dbg(hub_dev, "hub controller current requirement: %dmA\n",
717 719 hub->descriptor->bHubContrCurrent);
718   - hub->power_budget = (501 - hub->descriptor->bHubContrCurrent)
719   - / 2;
  720 + hub->limited_power = 1;
  721 + if (hdev->maxchild > 0) {
  722 + int remaining = hdev->bus_mA -
  723 + hub->descriptor->bHubContrCurrent;
  724 +
  725 + if (remaining < hdev->maxchild * 100)
  726 + dev_warn(hub_dev,
  727 + "insufficient power available "
  728 + "to use all downstream ports\n");
  729 + hub->mA_per_port = 100; /* 7.2.1.1 */
  730 + }
  731 + } else { /* Self-powered external hub */
  732 + /* FIXME: What about battery-powered external hubs that
  733 + * provide less current per port? */
  734 + hub->mA_per_port = 500;
720 735 }
721   - if (hub->power_budget)
722   - dev_dbg(hub_dev, "%dmA bus power budget for children\n",
723   - hub->power_budget * 2);
  736 + if (hub->mA_per_port < 500)
  737 + dev_dbg(hub_dev, "%umA bus power budget for each child\n",
  738 + hub->mA_per_port);
724 739  
725   -
726 740 ret = hub_hub_status(hub, &hubstatus, &hubchange);
727 741 if (ret < 0) {
728 742 message = "can't get hub status";
729 743  
730 744  
731 745  
732 746  
733 747  
734 748  
735 749  
... ... @@ -1136,45 +1150,107 @@
1136 1150 device_unregister(&udev->dev);
1137 1151 }
1138 1152  
  1153 +static inline const char *plural(int n)
  1154 +{
  1155 + return (n == 1 ? "" : "s");
  1156 +}
  1157 +
1139 1158 static int choose_configuration(struct usb_device *udev)
1140 1159 {
1141   - int c, i;
  1160 + int i;
  1161 + u16 devstatus;
  1162 + int bus_powered;
  1163 + int num_configs;
  1164 + struct usb_host_config *c, *best;
1142 1165  
1143   - /* NOTE: this should interact with hub power budgeting */
  1166 + /* If this fails, assume the device is bus-powered */
  1167 + devstatus = 0;
  1168 + usb_get_status(udev, USB_RECIP_DEVICE, 0, &devstatus);
  1169 + le16_to_cpus(&devstatus);
  1170 + bus_powered = ((devstatus & (1 << USB_DEVICE_SELF_POWERED)) == 0);
  1171 + dev_dbg(&udev->dev, "device is %s-powered\n",
  1172 + bus_powered ? "bus" : "self");
1144 1173  
1145   - c = udev->config[0].desc.bConfigurationValue;
1146   - if (udev->descriptor.bNumConfigurations != 1) {
1147   - for (i = 0; i < udev->descriptor.bNumConfigurations; i++) {
1148   - struct usb_interface_descriptor *desc;
  1174 + best = NULL;
  1175 + c = udev->config;
  1176 + num_configs = udev->descriptor.bNumConfigurations;
  1177 + for (i = 0; i < num_configs; (i++, c++)) {
  1178 + struct usb_interface_descriptor *desc =
  1179 + &c->intf_cache[0]->altsetting->desc;
1149 1180  
1150   - /* heuristic: Linux is more likely to have class
1151   - * drivers, so avoid vendor-specific interfaces.
1152   - */
1153   - desc = &udev->config[i].intf_cache[0]
1154   - ->altsetting->desc;
1155   - if (desc->bInterfaceClass == USB_CLASS_VENDOR_SPEC)
1156   - continue;
1157   - /* COMM/2/all is CDC ACM, except 0xff is MSFT RNDIS.
1158   - * MSFT needs this to be the first config; never use
1159   - * it as the default unless Linux has host-side RNDIS.
1160   - * A second config would ideally be CDC-Ethernet, but
1161   - * may instead be the "vendor specific" CDC subset
1162   - * long used by ARM Linux for sa1100 or pxa255.
1163   - */
1164   - if (desc->bInterfaceClass == USB_CLASS_COMM
1165   - && desc->bInterfaceSubClass == 2
1166   - && desc->bInterfaceProtocol == 0xff) {
1167   - c = udev->config[1].desc.bConfigurationValue;
1168   - continue;
1169   - }
1170   - c = udev->config[i].desc.bConfigurationValue;
  1181 + /*
  1182 + * HP's USB bus-powered keyboard has only one configuration
  1183 + * and it claims to be self-powered; other devices may have
  1184 + * similar errors in their descriptors. If the next test
  1185 + * were allowed to execute, such configurations would always
  1186 + * be rejected and the devices would not work as expected.
  1187 + */
  1188 +#if 0
  1189 + /* Rule out self-powered configs for a bus-powered device */
  1190 + if (bus_powered && (c->desc.bmAttributes &
  1191 + USB_CONFIG_ATT_SELFPOWER))
  1192 + continue;
  1193 +#endif
  1194 +
  1195 + /*
  1196 + * The next test may not be as effective as it should be.
  1197 + * Some hubs have errors in their descriptor, claiming
  1198 + * to be self-powered when they are really bus-powered.
  1199 + * We will overestimate the amount of current such hubs
  1200 + * make available for each port.
  1201 + *
  1202 + * This is a fairly benign sort of failure. It won't
  1203 + * cause us to reject configurations that we should have
  1204 + * accepted.
  1205 + */
  1206 +
  1207 + /* Rule out configs that draw too much bus current */
  1208 + if (c->desc.bMaxPower * 2 > udev->bus_mA)
  1209 + continue;
  1210 +
  1211 + /* If the first config's first interface is COMM/2/0xff
  1212 + * (MSFT RNDIS), rule it out unless Linux has host-side
  1213 + * RNDIS support. */
  1214 + if (i == 0 && desc->bInterfaceClass == USB_CLASS_COMM
  1215 + && desc->bInterfaceSubClass == 2
  1216 + && desc->bInterfaceProtocol == 0xff) {
  1217 +#ifndef CONFIG_USB_NET_RNDIS
  1218 + continue;
  1219 +#else
  1220 + best = c;
  1221 +#endif
  1222 + }
  1223 +
  1224 + /* From the remaining configs, choose the first one whose
  1225 + * first interface is for a non-vendor-specific class.
  1226 + * Reason: Linux is more likely to have a class driver
  1227 + * than a vendor-specific driver. */
  1228 + else if (udev->descriptor.bDeviceClass !=
  1229 + USB_CLASS_VENDOR_SPEC &&
  1230 + desc->bInterfaceClass !=
  1231 + USB_CLASS_VENDOR_SPEC) {
  1232 + best = c;
1171 1233 break;
1172 1234 }
  1235 +
  1236 + /* If all the remaining configs are vendor-specific,
  1237 + * choose the first one. */
  1238 + else if (!best)
  1239 + best = c;
  1240 + }
  1241 +
  1242 + if (best) {
  1243 + i = best->desc.bConfigurationValue;
1173 1244 dev_info(&udev->dev,
1174   - "configuration #%d chosen from %d choices\n",
1175   - c, udev->descriptor.bNumConfigurations);
  1245 + "configuration #%d chosen from %d choice%s\n",
  1246 + i, num_configs, plural(num_configs));
  1247 + } else {
  1248 + i = -1;
  1249 + dev_warn(&udev->dev,
  1250 + "no configuration chosen from %d choice%s\n",
  1251 + num_configs, plural(num_configs));
1176 1252 }
1177   - return c;
  1253 + return i;
1178 1254 }
1179 1255  
1180 1256 #ifdef DEBUG
1181 1257  
... ... @@ -1327,17 +1403,13 @@
1327 1403 * with the driver core, and lets usb device drivers bind to them.
1328 1404 */
1329 1405 c = choose_configuration(udev);
1330   - if (c < 0)
1331   - dev_warn(&udev->dev,
1332   - "can't choose an initial configuration\n");
1333   - else {
  1406 + if (c >= 0) {
1334 1407 err = usb_set_configuration(udev, c);
1335 1408 if (err) {
1336 1409 dev_err(&udev->dev, "can't set config #%d, error %d\n",
1337 1410 c, err);
1338   - usb_remove_sysfs_dev_files(udev);
1339   - device_del(&udev->dev);
1340   - goto fail;
  1411 + /* This need not be fatal. The user can try to
  1412 + * set other configurations. */
1341 1413 }
1342 1414 }
1343 1415  
... ... @@ -1702,7 +1774,7 @@
1702 1774 * and device drivers will know about any resume quirks.
1703 1775 */
1704 1776 status = usb_get_status(udev, USB_RECIP_DEVICE, 0, &devstatus);
1705   - if (status < 0)
  1777 + if (status < 2)
1706 1778 dev_dbg(&udev->dev,
1707 1779 "gone after usb resume? status %d\n",
1708 1780 status);
... ... @@ -1711,7 +1783,7 @@
1711 1783 int (*resume)(struct device *);
1712 1784  
1713 1785 le16_to_cpus(&devstatus);
1714   - if (devstatus & (1 << USB_DEVICE_REMOTE_WAKEUP)
  1786 + if ((devstatus & (1 << USB_DEVICE_REMOTE_WAKEUP))
1715 1787 && udev->parent) {
1716 1788 status = usb_control_msg(udev,
1717 1789 usb_sndctrlpipe(udev, 0),
1718 1790  
1719 1791  
1720 1792  
1721 1793  
1722 1794  
1723 1795  
... ... @@ -2374,39 +2446,36 @@
2374 2446 {
2375 2447 struct usb_device *hdev = hub->hdev;
2376 2448 int remaining;
2377   - unsigned i;
  2449 + int port1;
2378 2450  
2379   - remaining = hub->power_budget;
2380   - if (!remaining) /* self-powered */
  2451 + if (!hub->limited_power)
2381 2452 return 0;
2382 2453  
2383   - for (i = 0; i < hdev->maxchild; i++) {
2384   - struct usb_device *udev = hdev->children[i];
2385   - int delta, ceiling;
  2454 + remaining = hdev->bus_mA - hub->descriptor->bHubContrCurrent;
  2455 + for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
  2456 + struct usb_device *udev = hdev->children[port1 - 1];
  2457 + int delta;
2386 2458  
2387 2459 if (!udev)
2388 2460 continue;
2389 2461  
2390   - /* 100mA per-port ceiling, or 8mA for OTG ports */
2391   - if (i != (udev->bus->otg_port - 1) || hdev->parent)
2392   - ceiling = 50;
2393   - else
2394   - ceiling = 4;
2395   -
  2462 + /* Unconfigured devices may not use more than 100mA,
  2463 + * or 8mA for OTG ports */
2396 2464 if (udev->actconfig)
2397   - delta = udev->actconfig->desc.bMaxPower;
  2465 + delta = udev->actconfig->desc.bMaxPower * 2;
  2466 + else if (port1 != udev->bus->otg_port || hdev->parent)
  2467 + delta = 100;
2398 2468 else
2399   - delta = ceiling;
2400   - // dev_dbg(&udev->dev, "budgeted %dmA\n", 2 * delta);
2401   - if (delta > ceiling)
2402   - dev_warn(&udev->dev, "%dmA over %dmA budget!\n",
2403   - 2 * (delta - ceiling), 2 * ceiling);
  2469 + delta = 8;
  2470 + if (delta > hub->mA_per_port)
  2471 + dev_warn(&udev->dev, "%dmA is over %umA budget "
  2472 + "for port %d!\n",
  2473 + delta, hub->mA_per_port, port1);
2404 2474 remaining -= delta;
2405 2475 }
2406 2476 if (remaining < 0) {
2407   - dev_warn(hub->intfdev,
2408   - "%dmA over power budget!\n",
2409   - -2 * remaining);
  2477 + dev_warn(hub->intfdev, "%dmA over power budget!\n",
  2478 + - remaining);
2410 2479 remaining = 0;
2411 2480 }
2412 2481 return remaining;
... ... @@ -2501,7 +2570,8 @@
2501 2570  
2502 2571 usb_set_device_state(udev, USB_STATE_POWERED);
2503 2572 udev->speed = USB_SPEED_UNKNOWN;
2504   -
  2573 + udev->bus_mA = hub->mA_per_port;
  2574 +
2505 2575 /* set the address */
2506 2576 choose_address(udev);
2507 2577 if (udev->devnum <= 0) {
2508 2578  
2509 2579  
... ... @@ -2521,16 +2591,16 @@
2521 2591 * on the parent.
2522 2592 */
2523 2593 if (udev->descriptor.bDeviceClass == USB_CLASS_HUB
2524   - && hub->power_budget) {
  2594 + && udev->bus_mA <= 100) {
2525 2595 u16 devstat;
2526 2596  
2527 2597 status = usb_get_status(udev, USB_RECIP_DEVICE, 0,
2528 2598 &devstat);
2529   - if (status < 0) {
  2599 + if (status < 2) {
2530 2600 dev_dbg(&udev->dev, "get status %d ?\n", status);
2531 2601 goto loop_disable;
2532 2602 }
2533   - cpu_to_le16s(&devstat);
  2603 + le16_to_cpus(&devstat);
2534 2604 if ((devstat & (1 << USB_DEVICE_SELF_POWERED)) == 0) {
2535 2605 dev_err(&udev->dev,
2536 2606 "can't connect bus-powered hub "
... ... @@ -2583,9 +2653,7 @@
2583 2653  
2584 2654 status = hub_power_remaining(hub);
2585 2655 if (status)
2586   - dev_dbg(hub_dev,
2587   - "%dmA power budget left\n",
2588   - 2 * status);
  2656 + dev_dbg(hub_dev, "%dmA power budget left\n", status);
2589 2657  
2590 2658 return;
2591 2659  
... ... @@ -2797,6 +2865,11 @@
2797 2865 if (hubchange & HUB_CHANGE_LOCAL_POWER) {
2798 2866 dev_dbg (hub_dev, "power change\n");
2799 2867 clear_hub_feature(hdev, C_HUB_LOCAL_POWER);
  2868 + if (hubstatus & HUB_STATUS_LOCAL_POWER)
  2869 + /* FIXME: Is this always true? */
  2870 + hub->limited_power = 0;
  2871 + else
  2872 + hub->limited_power = 1;
2800 2873 }
2801 2874 if (hubchange & HUB_CHANGE_OVERCURRENT) {
2802 2875 dev_dbg (hub_dev, "overcurrent change\n");
drivers/usb/core/hub.h
... ... @@ -220,8 +220,9 @@
220 220 struct usb_hub_descriptor *descriptor; /* class descriptor */
221 221 struct usb_tt tt; /* Transaction Translator */
222 222  
223   - u8 power_budget; /* in 2mA units; or zero */
  223 + unsigned mA_per_port; /* current for each child */
224 224  
  225 + unsigned limited_power:1;
225 226 unsigned quiescing:1;
226 227 unsigned activating:1;
227 228 unsigned resume_root_hub:1;
drivers/usb/core/message.c
... ... @@ -1387,6 +1387,12 @@
1387 1387 if (dev->state != USB_STATE_ADDRESS)
1388 1388 usb_disable_device (dev, 1); // Skip ep0
1389 1389  
  1390 + n = dev->bus_mA - cp->desc.bMaxPower * 2;
  1391 + if (n < 0)
  1392 + dev_warn(&dev->dev, "new config #%d exceeds power "
  1393 + "limit by %dmA\n",
  1394 + configuration, -n);
  1395 +
1390 1396 if ((ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
1391 1397 USB_REQ_SET_CONFIGURATION, 0, configuration, 0,
1392 1398 NULL, 0, USB_CTRL_SET_TIMEOUT)) < 0)
... ... @@ -347,6 +347,8 @@
347 347  
348 348 char **rawdescriptors; /* Raw descriptors for each config */
349 349  
  350 + unsigned short bus_mA; /* Current available from the bus */
  351 +
350 352 int have_langid; /* whether string_langid is valid */
351 353 int string_langid; /* language ID for strings */
352 354