[coreboot] New patch to review for coreboot: 2f51d92 libpayload: Add timeouts in the EHCI USB driver

Nico Huber (nico.huber@secunet.com) gerrit at coreboot.org
Thu May 31 15:41:34 CEST 2012


Nico Huber (nico.huber at secunet.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1077

-gerrit

commit 2f51d9239824baaac28029b5cabb69f1cad06236
Author: Nico Huber <nico.huber at secunet.com>
Date:   Mon May 21 14:53:43 2012 +0200

    libpayload: Add timeouts in the EHCI USB driver
    
    We should always have some timeout when we wait for the hardware. This adds
    missing timeouts to the EHCI driver.
    
    Change-Id: I13ba532a6daf47510b16b8fdbe572a21f1d8b09c
    Signed-off-by: Nico Huber <nico.huber at secunet.com>
---
 payloads/libpayload/drivers/usb/ehci.c    |   90 ++++++++++++++++++++--------
 payloads/libpayload/drivers/usb/ehci_rh.c |   32 +++++++++--
 2 files changed, 91 insertions(+), 31 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/ehci.c b/payloads/libpayload/drivers/usb/ehci.c
index 9500059..cd1e867 100644
--- a/payloads/libpayload/drivers/usb/ehci.c
+++ b/payloads/libpayload/drivers/usb/ehci.c
@@ -116,7 +116,21 @@ static int wait_for_tds(qtd_t *head)
 	qtd_t *cur = head;
 	while (1) {
 		if (0) dump_td(virt_to_phys(cur));
-		while ((cur->token & QTD_ACTIVE) && !(cur->token & QTD_HALTED)) udelay(60);
+
+		/* wait for results */
+		/* TOTEST: how long to wait?
+		 *         tested with some USB2.0 flash sticks:
+		 *         slowest took around 180ms
+		 */
+		int timeout = 10000; /* time out after 10000 * 50us == 500ms */
+		while ((cur->token & QTD_ACTIVE) && !(cur->token & QTD_HALTED)
+				&& timeout--)
+			udelay(50);
+		if (timeout < 0) {
+			printf("Error: ehci: queue transfer "
+				"processing timed out.\n");
+			return 1;
+		}
 		if (cur->token & QTD_HALTED) {
 			printf("ERROR with packet\n");
 			dump_td(virt_to_phys(cur));
@@ -134,6 +148,51 @@ static int wait_for_tds(qtd_t *head)
 	return result;
 }
 
+static int ehci_set_async_schedule(ehci_t *ehcic, int enable)
+{
+	/* Set async schedule status. */
+	if (enable)
+		ehcic->operation->usbcmd |= HC_OP_ASYNC_SCHED_EN;
+	else
+		ehcic->operation->usbcmd &= ~HC_OP_ASYNC_SCHED_EN;
+	/* Wait for the controller to accept async schedule status.
+	 * This shouldn't take too long, but we should timeout nevertheless.
+	 */
+	enable = enable ? HC_OP_ASYNC_SCHED_STAT : 0;
+	int timeout = 100; /* time out after 100ms */
+	while (((ehcic->operation->usbsts & HC_OP_ASYNC_SCHED_STAT) != enable)
+			&& timeout--)
+		mdelay(1);
+	if (timeout < 0) {
+		debug("ehci async schedule status change timed out.\n");
+		return 1;
+	}
+	return 0;
+}
+
+static int ehci_process_async_schedule(
+		ehci_t *ehcic, ehci_qh_t *qhead, qtd_t *head)
+{
+	int result;
+
+	/* make sure async schedule is disabled */
+	if (ehci_set_async_schedule(ehcic, 0)) return 1;
+
+	/* hook up QH */
+	ehcic->operation->asynclistaddr = virt_to_phys(qhead);
+
+	/* start async schedule */
+	if (ehci_set_async_schedule(ehcic, 1)) return 1;
+
+	/* wait for result */
+	result = wait_for_tds(head);
+
+	/* disable async schedule */
+	ehci_set_async_schedule(ehcic, 0);
+
+	return result;
+}
+
 static int ehci_bulk (endpoint_t *ep, int size, u8 *data, int finalize)
 {
 	int result = 0;
@@ -179,19 +238,8 @@ static int ehci_bulk (endpoint_t *ep, int size, u8 *data, int finalize)
 	qh->td.token |= (ep->toggle?QTD_TOGGLE_DATA1:0);
 	head->token |= (ep->toggle?QTD_TOGGLE_DATA1:0);
 
-	/* hook up QH */
-	EHCI_INST(ep->dev->controller)->operation->asynclistaddr = virt_to_phys(qh);
-
-	/* start async schedule */
-	EHCI_INST(ep->dev->controller)->operation->usbcmd |= HC_OP_ASYNC_SCHED_EN;
-	while (!(EHCI_INST(ep->dev->controller)->operation->usbsts & HC_OP_ASYNC_SCHED_STAT)) ; /* wait */
-
-	/* wait for result */
-	result = wait_for_tds(head);
-
-	/* disable async schedule */
-	EHCI_INST(ep->dev->controller)->operation->usbcmd &= ~HC_OP_ASYNC_SCHED_EN;
-	while (EHCI_INST(ep->dev->controller)->operation->usbsts & HC_OP_ASYNC_SCHED_STAT) ; /* wait */
+	result = ehci_process_async_schedule(
+			EHCI_INST(ep->dev->controller), qh, head);
 
 	ep->toggle = (cur->token & QTD_TOGGLE_MASK) >> QTD_TOGGLE_SHIFT;
 
@@ -268,18 +316,8 @@ static int ehci_control (usbdev_t *dev, direction_t dir, int drlen, void *devreq
 	qh->epcaps = 3 << QH_PIPE_MULTIPLIER_SHIFT;
 	qh->td.next_qtd = virt_to_phys(head);
 
-	/* hook up QH */
-	EHCI_INST(dev->controller)->operation->asynclistaddr = virt_to_phys(qh);
-
-	/* start async schedule */
-	EHCI_INST(dev->controller)->operation->usbcmd |= HC_OP_ASYNC_SCHED_EN;
-	while (!(EHCI_INST(dev->controller)->operation->usbsts & HC_OP_ASYNC_SCHED_STAT)) ; /* wait */
-
-	result = wait_for_tds(head);
-
-	/* disable async schedule */
-	EHCI_INST(dev->controller)->operation->usbcmd &= ~HC_OP_ASYNC_SCHED_EN;
-	while (EHCI_INST(dev->controller)->operation->usbsts & HC_OP_ASYNC_SCHED_STAT) ; /* wait */
+	result = ehci_process_async_schedule(
+			EHCI_INST(dev->controller), qh, head);
 
 	free_qh_and_tds(qh, head);
 	return result;
diff --git a/payloads/libpayload/drivers/usb/ehci_rh.c b/payloads/libpayload/drivers/usb/ehci_rh.c
index dd073a3..aeead3b 100644
--- a/payloads/libpayload/drivers/usb/ehci_rh.c
+++ b/payloads/libpayload/drivers/usb/ehci_rh.c
@@ -55,9 +55,21 @@ ehci_rh_hand_over_port (usbdev_t *dev, int port)
 {
 	debug("giving up port %x, it's USB1\n", port+1);
 
+	/* Clear ConnectStatusChange before evaluation */
+	/* RW/C register, so clear it by writing 1 */
+	RH_INST(dev)->ports[port] |= P_CONN_STATUS_CHANGE;
+
 	/* Lowspeed device. Hand over to companion */
 	RH_INST(dev)->ports[port] |= P_PORT_OWNER;
-	do {} while (!(RH_INST(dev)->ports[port] & P_CONN_STATUS_CHANGE));
+
+	/* TOTEST: how long to wait? trying 100ms for now */
+	int timeout = 10; /* timeout after 10 * 10ms == 100ms */
+	while (!(RH_INST(dev)->ports[port] & P_CONN_STATUS_CHANGE) && timeout--)
+		mdelay(10);
+	if (!(RH_INST(dev)->ports[port] & P_CONN_STATUS_CHANGE)) {
+		debug("Warning: Handing port over to companion timed out.\n");
+	}
+
 	/* RW/C register, so clear it by writing 1 */
 	RH_INST(dev)->ports[port] |= P_CONN_STATUS_CHANGE;
 	return;
@@ -73,7 +85,7 @@ ehci_rh_scanport (usbdev_t *dev, int port)
 	}
 	/* device connected, handle */
 	if (RH_INST(dev)->ports[port] & P_CURR_CONN_STATUS) {
-		mdelay(100);
+		mdelay(100); // usb20 spec 9.1.2
 		if ((RH_INST(dev)->ports[port] & P_LINE_STATUS) == P_LINE_STATUS_LOWSPEED) {
 			ehci_rh_hand_over_port(dev, port);
 			return;
@@ -85,13 +97,23 @@ ehci_rh_scanport (usbdev_t *dev, int port)
 		RH_INST(dev)->ports[port] = (RH_INST(dev)->ports[port] & ~P_PORT_ENABLE) | P_PORT_RESET;
 
 		/* Wait a bit while reset is active. */
-		mdelay(50);
+		mdelay(50); // usb20 spec 7.1.7.5 (TDRSTR)
 
 		/* Deassert reset. */
 		RH_INST(dev)->ports[port] &= ~P_PORT_RESET;
 
-		/* Wait for flag change to finish. The controller might take a while */
-		while (RH_INST(dev)->ports[port] & P_PORT_RESET) ;
+		/* Wait max. 2ms (ehci spec 2.3.9) for flag change to finish. */
+		int timeout = 20; /* time out after 20 * 100us == 2ms */
+		while ((RH_INST(dev)->ports[port] & P_PORT_RESET) && timeout--)
+			udelay(100);
+		if (RH_INST(dev)->ports[port] & P_PORT_RESET) {
+			printf("Error: ehci_rh: port reset timed out.\n");
+			return;
+		}
+
+		/* If the host controller enabled the port, it's a high-speed
+		 * device, otherwise it's full-speed.
+		 */
 		if (!(RH_INST(dev)->ports[port] & P_PORT_ENABLE)) {
 			ehci_rh_hand_over_port(dev, port);
 			return;




More information about the coreboot mailing list