[coreboot-gerrit] New patch to review for coreboot: 64489a6 libpayload: usb: Try to avoid reusing device addresses

[Marc Jones (marc.jones@se-eng.com)]

Marc Jones (marc.jones at se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/7881

-gerrit

commit 64489a6834b204d9bbdd823742b502e3541012c8
Author: Julius Werner <jwerner at chromium.org>
Date:   Mon Apr 28 16:04:24 2014 -0700

    libpayload: usb: Try to avoid reusing device addresses
    
    We recently changed the USB stack to detach devices aggressively that we
    don't intend to use. This alone is not really a problem, but it
    exarcerbates the fact that our device detachment itself is not very
    good. We destroy any local info about the device, but we don't properly
    disable the offending port. The device keeps thinking that it's active,
    and if we later try to reuse that device address for another device
    things become confused.
    
    The real fix would be to properly disable all ports that we don't intend
    to use. Unfortunately, this isn't really possible in our current
    device/hub polymorphism structure, and I don't want to hack a new
    disable_port() callback into usbdev_t that really doesn't belong there.
    We will only be able to fix this cleanly after we ported all root hubs
    to the generic_hub interface.
    
    Until then, an easy workaround is to just avoid reusing addresses as
    long as possible. This is firmware, so the chance that we'll ever run
    through 127 devices is really small in practice. Even if we ever fix the
    underlying issue, it's probably a smart precaution to keep.
    
    BRANCH=nyan,rambi
    BUG=chrome-os-partner:28328
    TEST=Boot from a hub that has an "unknown" device in an earlier port
    than the stick you want to boot from, make sure you can still boot.
    
    Original-Change-Id: I9b522dd8cbcd441e8c3b8781fcecd2effa0f23ee
    Original-Signed-off-by: Julius Werner <jwerner at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/197420
    Original-Reviewed-by: Shawn Nematbakhsh <shawnn at chromium.org>
    Original-Reviewed-by: David Hendricks <dhendrix at chromium.org>
    (cherry picked from commit 28b48aa69b55a983226edf2ea616f33cd4b959e2)
    Signed-off-by: Marc Jones <marc.jones at se-eng.com>
    
    Change-Id: Id4c5c92e75d6b5a7e8f0ee3e396c69c4efd13176
---
 payloads/libpayload/drivers/usb/usb.c | 14 +++++++++++---
 payloads/libpayload/include/usb/usb.h |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/usb.c b/payloads/libpayload/drivers/usb/usb.c
index 25bd954..53006ff 100644
--- a/payloads/libpayload/drivers/usb/usb.c
+++ b/payloads/libpayload/drivers/usb/usb.c
@@ -206,10 +206,18 @@ clear_stall (endpoint_t *ep)
 static int
 get_free_address (hci_t *controller)
 {
-	int i;
-	for (i = 1; i < 128; i++) {
-		if (controller->devices[i] == 0)
+	int i = controller->latest_address + 1;
+	for (; i != controller->latest_address; i++) {
+		if (i >= ARRAY_SIZE(controller->devices) || i < 1) {
+			usb_debug("WARNING: Device addresses for controller %#x"
+				  " wrapped around!\n", controller->reg_base);
+			i = 0;
+			continue;
+		}
+		if (controller->devices[i] == 0) {
+			controller->latest_address = i;
 			return i;
+		}
 	}
 	usb_debug ("no free address found\n");
 	return -1;		// no free address
diff --git a/payloads/libpayload/include/usb/usb.h b/payloads/libpayload/include/usb/usb.h
index 381d039..ec9912a 100644
--- a/payloads/libpayload/include/usb/usb.h
+++ b/payloads/libpayload/include/usb/usb.h
@@ -213,6 +213,7 @@ struct usbdev_hc {
 	u32 reg_base;
 	pcidev_t pcidev; // 0 if not used (eg on ARM)
 	hc_type type;
+	int latest_address;
 	usbdev_t *devices[128];	// dev 0 is root hub, 127 is last addressable
 
 	/* start():     Resume operation. */