Patch set updated: 89461d8 port_enable and port_reset must change atomically.

[Steven A. Falco]

Steven A. Falco ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/101

-gerrit

commit 89461d88a0e163ab5fee96908a4195035fe800f5
Author: Steven A. Falco 
Date:   Fri Jul 15 21:44:35 2011 -0400

port_enable and port_reset must change atomically.

I have observed two separate EHCI host bridges that do not tolerate
    using C bit-fields to directly manipulate the portsc_t register.  The
    reason for this is that the EHCI spec says that port_enable must go
    to 0 at the time that port_reset goes to 1.  Naturally this cannot be
    done using direct bit-field manipulation.  Instead, we use a temporary
    variable, change the bit-fields there, then atomically write the new
    value back to the hardware.

Signed-off-by: Steven A. Falco 
    Change-Id: If138faee43e0293efa203b86f7893fdf1e811269
---
 payloads/libpayload/drivers/usb/ehci_rh.c |   39 +++++++++++++++++++++++-----
 1 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/ehci_rh.c b/payloads/libpayload/drivers/usb/ehci_rh.c
index 5eaeeb6..5f0db94 100644
--- a/payloads/libpayload/drivers/usb/ehci_rh.c
+++ b/payloads/libpayload/drivers/usb/ehci_rh.c
@@ -54,14 +54,19 @@ static void
 ehci_rh_hand_over_port (usbdev_t *dev, int port)
 {
    volatile portsc_t *p = &(RH_INST(dev)->ports[port]);
+	volatile portsc_t tmp;

printf("giving up port %x, it's USB1\n", port+1);

/* Lowspeed device. Hand over to companion */
-	p->port_owner = 1;
+	tmp = *p;
+	tmp.port_owner = 1;
+	*p = tmp;
    do {} while (!p->conn_status_change);
    /* RW/C register, so clear it by writing 1 */
-	p->conn_status_change = 1;
+	tmp = *p;
+	tmp.conn_status_change = 1;
+	*p = tmp;
    return;
 }

@@ -69,6 +74,7 @@ static void
 ehci_rh_scanport (usbdev_t *dev, int port)
 {
    volatile portsc_t *p = &(RH_INST(dev)->ports[port]);
+	volatile portsc_t tmp;
    if (RH_INST(dev)->devices[port]!=-1) {
    	printf("Unregister device at port %x\n", port+1);
    	usb_detach_device(dev->controller, RH_INST(dev)->devices[port]);
@@ -81,10 +87,22 @@ ehci_rh_scanport (usbdev_t *dev, int port)
    		ehci_rh_hand_over_port(dev, port);
    		return;
    	}
-		p->port_enable = 0;
-		p->port_reset = 1;
+
+		/* Deassert enable, assert reset.  These must change
+		 * atomically.
+		 */
+		tmp = *p;
+		tmp.port_enable = 0;
+		tmp.port_reset = 1;
+		*p = tmp;
+
+		/* Wait a bit while reset is active. */
    	mdelay(50);
-		p->port_reset = 0;
+
+		/* Deassert reset. */
+		tmp.port_reset = 0;
+		*p = tmp;
+
    	/* Wait for flag change to finish. The controller might take a while */
    	while (p->port_reset) ;
    	if (!p->port_enable) {
@@ -95,7 +113,9 @@ ehci_rh_scanport (usbdev_t *dev, int port)
    	RH_INST(dev)->devices[port] = usb_attach_device(dev->controller, dev->address, port, 2);
    }
    /* RW/C register, so clear it by writing 1 */
-	p->conn_status_change = 1;
+	tmp = *p;
+	tmp.conn_status_change = 1;
+	*p = tmp;
 }

static int
@@ -122,6 +142,8 @@ void
 ehci_rh_init (usbdev_t *dev)
 {
    int i;
+	volatile portsc_t *p;
+	volatile portsc_t tmp;

dev->destroy = ehci_rh_destroy;
    dev->poll = ehci_rh_poll;
@@ -135,8 +157,11 @@ ehci_rh_init (usbdev_t *dev)

RH_INST(dev)->devices = malloc(RH_INST(dev)->n_ports * sizeof(int));
    for (i=0; i < RH_INST(dev)->n_ports; i++) {
+		p = &(RH_INST(dev)->ports[i]);
    	RH_INST(dev)->devices[i] = -1;
-		RH_INST(dev)->ports[i].pp = 1;
+		tmp = *p;
+		tmp.pp = 1;
+		*p = tmp;
    }

dev->address = 0;