libpayload: usb: Unify USB speed between XHCI stack and USB core

This patch removes the confusing concept of a special "xhci_speed" with
a different numeric value from the usual speed used throughout the USB
core (except for the places directly interacting with the xHC, which are
explicitly marked). It also moves the MPS0 decoding function into the
core and moves some definitions around in preparation of later changes
that will make the stack SuperSpeed-ready. It makes both set_address
implementations share a constant for the specification-defined
SetAddress() recovery delay and removes pointless additional delays from
the non-XHCI version.

Change-Id: I422379d05d4a502b12dae183504e5231add5466a
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/170664
Reviewed-by: Stefan Reinauer <reinauer@google.com>
Commit-Queue: Ronald Minnich <rminnich@chromium.org>
(cherry picked from commit f160d4439c0d7cea1d2e6b97207935d61dcbb2f2)
Signed-off-by: Isaac Christensen <isaac.christensen@se-eng.com>
Reviewed-on: http://review.coreboot.org/6776
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>

9 files changed, 97 insertions, 95 deletions

diff --git a/payloads/libpayload/drivers/usb/generic_hub.c b/payloads/libpayload/drivers/usb/generic_hub.c
index d568d4b8a4..fa95969f2a 100644
--- a/payloads/libpayload/drivers/usb/generic_hub.c
+++ b/payloads/libpayload/drivers/usb/generic_hub.c
@@ -200,7 +200,7 @@ generic_hub_attach_dev(usbdev_t *const dev, const int port)
 				  "disabled after 10ms\n", port);
 	}
 
-	const int speed = hub->ops->port_speed(dev, port);
+	const usb_speed speed = hub->ops->port_speed(dev, port);
 	if (speed >= 0) {
 		usb_debug("generic_hub: Success at port %d\n", port);
 		if (hub->ops->reset_port)
diff --git a/payloads/libpayload/drivers/usb/generic_hub.h b/payloads/libpayload/drivers/usb/generic_hub.h
index 7be20e8a9e..cd4ebb6616 100644
--- a/payloads/libpayload/drivers/usb/generic_hub.h
+++ b/payloads/libpayload/drivers/usb/generic_hub.h
@@ -46,7 +46,7 @@ typedef struct generic_hub_ops {
 	/* returns 1 if the port is enabled */
 	int (*port_enabled)(usbdev_t *, int port);
 	/* returns speed if port is enabled, negative value if not */
-	int (*port_speed)(usbdev_t *, int port);
+	usb_speed (*port_speed)(usbdev_t *, int port);
 
 	/* enables (powers up) a port (optional) */
 	int (*enable_port)(usbdev_t *, int port);
diff --git a/payloads/libpayload/drivers/usb/ohci_rh.c b/payloads/libpayload/drivers/usb/ohci_rh.c
index aa44f338ec..b3bf7ef3cb 100644
--- a/payloads/libpayload/drivers/usb/ohci_rh.c
+++ b/payloads/libpayload/drivers/usb/ohci_rh.c
@@ -121,7 +121,7 @@ ohci_rh_scanport (usbdev_t *dev, int port)
 		return;
 	}
 
-	int speed = (OHCI_INST(dev->controller)->opreg->HcRhPortStatus[port] & LowSpeedDeviceAttached) != 0;
+	usb_speed speed = (OHCI_INST(dev->controller)->opreg->HcRhPortStatus[port] & LowSpeedDeviceAttached) != 0;
 	RH_INST (dev)->port[port] = usb_attach_device(dev->controller, dev->address, port, speed);
 }
 
diff --git a/payloads/libpayload/drivers/usb/uhci_rh.c b/payloads/libpayload/drivers/usb/uhci_rh.c
index 44ba498274..5cb18b9516 100644
--- a/payloads/libpayload/drivers/usb/uhci_rh.c
+++ b/payloads/libpayload/drivers/usb/uhci_rh.c
@@ -133,7 +133,7 @@ uhci_rh_scanport (usbdev_t *dev, int port)
 		uhci_rh_disable_port (dev, port);
 		uhci_rh_enable_port (dev, port);
 
-		int speed = ((uhci_reg_read16 (dev->controller, portsc) >> 8) & 1);
+		usb_speed speed = ((uhci_reg_read16 (dev->controller, portsc) >> 8) & 1);
 
 		RH_INST (dev)->port[offset] = usb_attach_device(dev->controller, dev->address, portsc, speed);
 	}
diff --git a/payloads/libpayload/drivers/usb/usb.c b/payloads/libpayload/drivers/usb/usb.c
index 8ee997ceca..0ec6f5c69e 100644
--- a/payloads/libpayload/drivers/usb/usb.c
+++ b/payloads/libpayload/drivers/usb/usb.c
@@ -243,44 +243,43 @@ get_free_address (hci_t *controller)
 }
 
 int
-generic_set_address (hci_t *controller, int speed, int hubport, int hubaddr)
+usb_decode_mps0(usb_speed speed, u8 bMaxPacketSize0)
 {
-	int adr = get_free_address (controller);	// address to set
-	dev_req_t dr;
-
-	memset (&dr, 0, sizeof (dr));
-	dr.data_dir = host_to_device;
-	dr.req_type = standard_type;
-	dr.req_recp = dev_recp;
-	dr.bRequest = SET_ADDRESS;
-	dr.wValue = adr;
-	dr.wIndex = 0;
-	dr.wLength = 0;
-
-	init_device_entry(controller, adr);
-	usbdev_t *dev = controller->devices[adr];
-	// dummy values for registering the address
-	dev->address = 0;
-	dev->hub = hubaddr;
-	dev->port = hubport;
-	dev->speed = speed;
-	dev->endpoints[0].dev = dev;
-	dev->endpoints[0].endpoint = 0;
-	dev->endpoints[0].maxpacketsize = 8;
-	dev->endpoints[0].toggle = 0;
-	dev->endpoints[0].direction = SETUP;
-	mdelay (50);
-	if (dev->controller->control (dev, OUT, sizeof (dr), &dr, 0, 0) < 0) {
-		return -1;
+	switch (speed) {
+	case LOW_SPEED:
+		if (bMaxPacketSize0 != 8) {
+			usb_debug("Invalid MPS0: 0x%02x\n", bMaxPacketSize0);
+			bMaxPacketSize0 = 8;
+		}
+		return bMaxPacketSize0;
+	case FULL_SPEED:
+		switch (bMaxPacketSize0) {
+		case 8: case 16: case 32: case 64:
+			return bMaxPacketSize0;
+		default:
+			usb_debug("Invalid MPS0: 0x%02x\n", bMaxPacketSize0);
+			return 8;
+		}
+	case HIGH_SPEED:
+		if (bMaxPacketSize0 != 64) {
+			usb_debug("Invalid MPS0: 0x%02x\n", bMaxPacketSize0);
+			bMaxPacketSize0 = 64;
+		}
+		return bMaxPacketSize0;
+	case SUPER_SPEED:
+		if (bMaxPacketSize0 != 9) {
+			usb_debug("Invalid MPS0: 0x%02x\n", bMaxPacketSize0);
+			bMaxPacketSize0 = 9;
+		}
+		return 2 << bMaxPacketSize0;
+	default: 	/* GCC is stupid and cannot deal with enums correctly */
+		return 8;
 	}
-	mdelay (50);
-
-	return adr;
 }
 
 /* Normalize bInterval to log2 of microframes */
 static int
-usb_decode_interval(const int speed, const endpoint_type type, const unsigned char bInterval)
+usb_decode_interval(usb_speed speed, const endpoint_type type, const unsigned char bInterval)
 {
 #define LOG2(a) ((sizeof(unsigned) << 3) - __builtin_clz(a) - 1)
 	switch (speed) {
@@ -320,8 +319,45 @@ usb_decode_interval(const int speed, const endpoint_type type, const unsigned ch
 #undef LOG2
 }
 
+int
+generic_set_address (hci_t *controller, usb_speed speed,
+		     int hubport, int hubaddr)
+{
+	int adr = get_free_address (controller);	// address to set
+	dev_req_t dr;
+
+	memset (&dr, 0, sizeof (dr));
+	dr.data_dir = host_to_device;
+	dr.req_type = standard_type;
+	dr.req_recp = dev_recp;
+	dr.bRequest = SET_ADDRESS;
+	dr.wValue = adr;
+	dr.wIndex = 0;
+	dr.wLength = 0;
+
+	init_device_entry(controller, adr);
+	usbdev_t *dev = controller->devices[adr];
+	// dummy values for registering the address
+	dev->address = 0;
+	dev->hub = hubaddr;
+	dev->port = hubport;
+	dev->speed = speed;
+	dev->endpoints[0].dev = dev;
+	dev->endpoints[0].endpoint = 0;
+	dev->endpoints[0].maxpacketsize = 8;
+	dev->endpoints[0].toggle = 0;
+	dev->endpoints[0].direction = SETUP;
+	if (dev->controller->control (dev, OUT, sizeof (dr), &dr, 0, 0) < 0) {
+		usb_debug ("set_address failed\n");
+		return -1;
+	}
+	mdelay (SET_ADDRESS_MDELAY);
+
+	return adr;
+}
+
 static int
-set_address (hci_t *controller, int speed, int hubport, int hubaddr)
+set_address (hci_t *controller, usb_speed speed, int hubport, int hubaddr)
 {
 	int adr = controller->set_address(controller, speed, hubport, hubaddr);
 	if (adr < 0 || !controller->devices[adr]) {
@@ -539,22 +575,19 @@ usb_detach_device(hci_t *controller, int devno)
 	   been called yet by the usb class driver */
 	if (controller->devices[devno]) {
 		controller->devices[devno]->destroy (controller->devices[devno]);
-		if (controller->destroy_device)
-			controller->destroy_device(controller, devno);
-		if (controller->devices[devno]->configuration)
-			free(controller->devices[devno]->configuration);
-		if (controller->devices[devno]->descriptor)
-			free(controller->devices[devno]->descriptor);
 		free(controller->devices[devno]);
 		controller->devices[devno] = NULL;
+		if (controller->destroy_device)
+			controller->destroy_device(controller, devno);
 	}
 }
 
 int
-usb_attach_device(hci_t *controller, int hubaddress, int port, int speed)
+usb_attach_device(hci_t *controller, int hubaddress, int port, usb_speed speed)
 {
-	static const char* speeds[] = { "full", "low", "high" };
-	usb_debug ("%sspeed device\n", (speed <= 2) ? speeds[speed] : "invalid value - no");
+	static const char* speeds[] = { "full", "low", "high", "super" };
+	usb_debug ("%sspeed device\n", (speed < sizeof(speeds) / sizeof(char*))
+		? speeds[speed] : "invalid value - no");
 	int newdev = set_address (controller, speed, port, hubaddress);
 	if (newdev == -1)
 		return -1;
diff --git a/payloads/libpayload/drivers/usb/usbhub.c b/payloads/libpayload/drivers/usb/usbhub.c
index ff7904cbd0..503a9a8528 100644
--- a/payloads/libpayload/drivers/usb/usbhub.c
+++ b/payloads/libpayload/drivers/usb/usbhub.c
@@ -77,7 +77,7 @@ usb_hub_port_enabled(usbdev_t *const dev, const int port)
 	return (buf[0] & PORT_ENABLE) != 0;
 }
 
-static int
+static usb_speed
 usb_hub_port_speed(usbdev_t *const dev, const int port)
 {
 	unsigned short buf[2] = { 0, 0 };
@@ -87,6 +87,7 @@ usb_hub_port_speed(usbdev_t *const dev, const int port)
 		 *      0   0  full speed
 		 *      0   1  low speed
 		 *      1   0  high speed
+		 *      1   1  super speed (hack, not in spec!)
 		 */
 		return (buf[0] >> 9) & 0x3;
 	} else {
diff --git a/payloads/libpayload/drivers/usb/xhci_devconf.c b/payloads/libpayload/drivers/usb/xhci_devconf.c
index f1064f0247..2b1de3bf9e 100644
--- a/payloads/libpayload/drivers/usb/xhci_devconf.c
+++ b/payloads/libpayload/drivers/usb/xhci_devconf.c
@@ -30,6 +30,7 @@
 //#define XHCI_SPEW_DEBUG
 
 #include <arch/virtual.h>
+#include <usb/usb.h>
 #include "xhci_private.h"
 
 static u32
@@ -57,7 +58,7 @@ xhci_get_rh_port(xhci_t *const xhci, const int hubport, const int hubaddr)
 }
 
 static int
-xhci_get_tt(xhci_t *const xhci, const int xhci_speed,
+xhci_get_tt(xhci_t *const xhci, const usb_speed speed,
 	    const int hubport, const int hubaddr,
 	    int *const tt, int *const tt_port)
 {
@@ -66,8 +67,8 @@ xhci_get_tt(xhci_t *const xhci, const int xhci_speed,
 	const slotctx_t *const slot = xhci->dev[hubaddr].ctx.slot;
 	if ((*tt = SC_GET(TTID, slot))) {
 		*tt_port = SC_GET(TTPORT, slot);
-	} else if (xhci_speed < XHCI_HIGH_SPEED &&
-			SC_GET(SPEED, slot) == XHCI_HIGH_SPEED) {
+	} else if (speed < HIGH_SPEED &&
+			SC_GET(SPEED1, slot) - 1 == HIGH_SPEED) {
 		*tt = hubaddr;
 		*tt_port = hubport;
 	}
@@ -75,37 +76,7 @@ xhci_get_tt(xhci_t *const xhci, const int xhci_speed,
 }
 
 static long
-xhci_decode_mps0(const int xhci_speed, const u8 b_mps)
-{
-	switch (xhci_speed) {
-	case XHCI_LOW_SPEED:
-	case XHCI_FULL_SPEED:
-	case XHCI_HIGH_SPEED:
-		switch (b_mps) {
-		case 8: case 16: case 32: case 64:
-			return b_mps;
-		default:
-			xhci_debug("Invalid MPS0: 0x%02x\n", b_mps);
-			return 8;
-		}
-		break;
-	case XHCI_SUPER_SPEED:
-		if (b_mps == 9) {
-			return 2 << b_mps;
-		} else {
-			xhci_debug("Invalid MPS0: 0x%02x\n", b_mps);
-			return 2 << 9;
-		}
-		break;
-	default:
-		xhci_debug("Invalid speed for MPS0: %d\n", xhci_speed);
-		return 8;
-	}
-}
-
-
-static long
-xhci_get_mps0(usbdev_t *const dev, const int xhci_speed)
+xhci_get_mps0(usbdev_t *const dev, const int speed)
 {
 	u8 buf[8];
 	dev_req_t dr = {
@@ -121,7 +92,7 @@ xhci_get_mps0(usbdev_t *const dev, const int xhci_speed)
 		xhci_debug("Failed to read MPS0\n");
 		return COMMUNICATION_ERROR;
 	} else {
-		return xhci_decode_mps0(xhci_speed, buf[7]);
+		return usb_decode_mps0(speed, buf[7]);
 	}
 }
 
@@ -150,10 +121,9 @@ xhci_make_inputctx(const size_t ctxsize)
 }
 
 int
-xhci_set_address (hci_t *controller, int speed, int hubport, int hubaddr)
+xhci_set_address (hci_t *controller, usb_speed speed, int hubport, int hubaddr)
 {
 	xhci_t *const xhci = XHCI_INST(controller);
-	const int xhci_speed = speed + 1;
 	const size_t ctxsize = CTXSIZE(xhci);
 	devinfo_t *di = NULL;
 
@@ -188,12 +158,12 @@ xhci_set_address (hci_t *controller, int speed, int hubport, int hubaddr)
 	*ic->add = (1 << 0) /* Slot Context */ | (1 << 1) /* EP0 Context */ ;
 
 	SC_SET(ROUTE,	ic->dev.slot, xhci_gen_route(xhci, hubport, hubaddr));
-	SC_SET(SPEED,	ic->dev.slot, xhci_speed);
+	SC_SET(SPEED1,	ic->dev.slot, speed + 1);
 	SC_SET(CTXENT,	ic->dev.slot, 1); /* the endpoint 0 context */
 	SC_SET(RHPORT,	ic->dev.slot, xhci_get_rh_port(xhci, hubport, hubaddr));
 
 	int tt, tt_port;
-	if (xhci_get_tt(xhci, xhci_speed, hubport, hubaddr, &tt, &tt_port)) {
+	if (xhci_get_tt(xhci, speed, hubport, hubaddr, &tt, &tt_port)) {
 		xhci_debug("TT for %d: %d[%d]\n", slot_id, tt, tt_port);
 		SC_SET(MTT, ic->dev.slot, SC_GET(MTT, xhci->dev[tt].ctx.slot));
 		SC_SET(TTID, ic->dev.slot, tt);
@@ -221,13 +191,13 @@ xhci_set_address (hci_t *controller, int speed, int hubport, int hubaddr)
 		xhci_debug("Addressed device %d (USB: %d)\n",
 			  slot_id, SC_GET(UADDR, di->ctx.slot));
 	}
-	mdelay(2); /* SetAddress() recovery interval (usb20 spec 9.2.6.3) */
+	mdelay(SET_ADDRESS_MDELAY);
 
 	init_device_entry(controller, slot_id);
 	controller->devices[slot_id]->address = slot_id;
 
 	const long mps0 = xhci_get_mps0(
-			controller->devices[slot_id], xhci_speed);
+			controller->devices[slot_id], speed);
 	if (mps0 < 0) {
 		goto _disable_return;
 	} else if (mps0 != 8) {
diff --git a/payloads/libpayload/drivers/usb/xhci_private.h b/payloads/libpayload/drivers/usb/xhci_private.h
index a851d8ec57..096ed2b8af 100644
--- a/payloads/libpayload/drivers/usb/xhci_private.h
+++ b/payloads/libpayload/drivers/usb/xhci_private.h
@@ -44,8 +44,6 @@
 
 #define MASK(startbit, lenbit) (((1<<(lenbit))-1)<<(startbit))
 
-enum { XHCI_FULL_SPEED = 1, XHCI_LOW_SPEED = 2, XHCI_HIGH_SPEED = 3, XHCI_SUPER_SPEED = 4 };
-
 #define TIMEOUT			-1
 #define CONTROLLER_ERROR	-2
 #define COMMUNICATION_ERROR	-3
@@ -164,9 +162,9 @@ typedef transfer_ring_t command_ring_t;
 #define SC_ROUTE_FIELD		f1		/* ROUTE - Route String */
 #define SC_ROUTE_START		0
 #define SC_ROUTE_LEN		20
-#define SC_SPEED_FIELD		f1
-#define SC_SPEED_START		20
-#define SC_SPEED_LEN		4
+#define SC_SPEED1_FIELD		f1		/* SPEED - Port speed plus one (compared to usb_speed enum) */
+#define SC_SPEED1_START		20
+#define SC_SPEED1_LEN		4
 #define SC_MTT_FIELD		f1		/* MTT - Multi Transaction Translator */
 #define SC_MTT_START		25
 #define SC_MTT_LEN		1
@@ -472,7 +470,7 @@ typedef struct xhci {
 
 void *xhci_align(const size_t min_align, const size_t size);
 void xhci_init_cycle_ring(transfer_ring_t *, const size_t ring_size);
-int xhci_set_address (hci_t *, int speed, int hubport, int hubaddr);
+int xhci_set_address (hci_t *, usb_speed speed, int hubport, int hubaddr);
 int xhci_finish_device_config(usbdev_t *);
 void xhci_destroy_dev(hci_t *, int slot_id);
 
diff --git a/payloads/libpayload/drivers/usb/xhci_rh.c b/payloads/libpayload/drivers/usb/xhci_rh.c
index ca6131f624..c4cbeeaab8 100644
--- a/payloads/libpayload/drivers/usb/xhci_rh.c
+++ b/payloads/libpayload/drivers/usb/xhci_rh.c
@@ -84,7 +84,7 @@ xhci_rh_port_enabled(usbdev_t *const dev, const int port)
 	return !!(*portsc & PORTSC_PED);
 }
 
-static int
+static usb_speed
 xhci_rh_port_speed(usbdev_t *const dev, const int port)
 {
 	xhci_t *const xhci = XHCI_INST(dev->controller);