[coreboot] New patch to review: b551855 Do not use C bit-fields to manipulate EHCI host bridge portsc_t register.
Steven A. Falco (sfalco@coincident.com)
gerrit at coreboot.org
Fri Jul 15 02:09:36 CEST 2011
Steven A. Falco (sfalco at coincident.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/101
-gerrit
commit b551855e895382b7570881bebcf28809a5619d63
Author: Steven A. Falco <sfalco at coincident.com>
Date: Thu Jul 14 19:56:50 2011 -0400
Do not use C bit-fields to manipulate EHCI host bridge portsc_t register.
I have observed two separate EHCI host bridges that do not tolerate
using C bit-fields to manipulate the portsc_t register. The reason
for this is that the EHCI spec says that PORT_PE must go to 0 at the
time that PORT_RESET goes to 1. Naturally this cannot be done using
separate bit-fields for each bit. Instead, we use a temporary register
and do explicit read-modify-write cycles.
Signed-off-by: Steven A. Falco <sfalco at coincident.com>
Change-Id: If138faee43e0293efa203b86f7893fdf1e811269
---
payloads/libpayload/drivers/usb/ehci_private.h | 29 +++++++++++++-
payloads/libpayload/drivers/usb/ehci_rh.c | 49 +++++++++++++++++------
2 files changed, 64 insertions(+), 14 deletions(-)
diff --git a/payloads/libpayload/drivers/usb/ehci_private.h b/payloads/libpayload/drivers/usb/ehci_private.h
index e63a81d..07c1a2f 100644
--- a/payloads/libpayload/drivers/usb/ehci_private.h
+++ b/payloads/libpayload/drivers/usb/ehci_private.h
@@ -37,8 +37,34 @@
#define FLADJ 0x61
#define FLADJ_framelength(x) (((x)-59488)/16)
+#define PORT_WKOC_E (1<<22)
+#define PORT_WKDISC_E (1<<21)
+#define PORT_WKCONN_E (1<<20)
+#define PORT_TEST_PKT (0x4<<16)
+#define PORT_LED_OFF (0<<14)
+#define PORT_LED_AMBER (1<<14)
+#define PORT_LED_GREEN (2<<14)
+#define PORT_LED_MASK (3<<14)
+#define PORT_OWNER (1<<13)
+#define PORT_POWER (1<<12)
+#define PORT_LINESTATUS_MASK (3<<10)
+#define PORT_LINESTATUS_KSTATE (1<<10)
+#define PORT_RESET (1<<8)
+#define PORT_SUSPEND (1<<7)
+#define PORT_RESUME (1<<6)
+#define PORT_OCC (1<<5)
+#define PORT_OC (1<<4)
+#define PORT_PEC (1<<3)
+#define PORT_PE (1<<2)
+#define PORT_CSC (1<<1)
+#define PORT_CONNECT (1<<0)
+
typedef union {
- u32 val;
+ volatile u32 val;
+ // WARNING - some controllers require port_enable and port_reset to
+ // change atomically. Therefore, using these separate bits is not
+ // recommended.
+#if 0
volatile struct {
unsigned long current_conn_status:1;
unsigned long conn_status_change:1;
@@ -60,6 +86,7 @@ typedef union {
unsigned long wake_on_overcurrent_en:1;
unsigned long:9;
} __attribute__ ((packed));
+#endif
} __attribute__ ((packed)) portsc_t;
typedef struct {
diff --git a/payloads/libpayload/drivers/usb/ehci_rh.c b/payloads/libpayload/drivers/usb/ehci_rh.c
index 5eaeeb6..c588a3d 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 u32 tmp;
printf("giving up port %x, it's USB1\n", port+1);
/* Lowspeed device. Hand over to companion */
- p->port_owner = 1;
- do {} while (!p->conn_status_change);
+ tmp = p->val;
+ tmp |= PORT_OWNER;
+ p->val = tmp;
+ while (!(p->val & PORT_CSC));
/* RW/C register, so clear it by writing 1 */
- p->conn_status_change = 1;
+ tmp = p->val;
+ tmp |= PORT_CSC;
+ p->val = tmp;
return;
}
@@ -69,41 +74,54 @@ static void
ehci_rh_scanport (usbdev_t *dev, int port)
{
volatile portsc_t *p = &(RH_INST(dev)->ports[port]);
+ volatile u32 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]);
RH_INST(dev)->devices[port]=-1;
}
/* device connected, handle */
- if (p->current_conn_status) {
+ if (p->val & PORT_CONNECT) {
mdelay(100);
- if (p->line_status == 0x1) {
+ if ((p->val & PORT_LINESTATUS_MASK) == PORT_LINESTATUS_KSTATE) {
ehci_rh_hand_over_port(dev, port);
return;
}
- p->port_enable = 0;
- p->port_reset = 1;
+
+ /* Remove port_enable, set port_reset. This must be atomic. */
+ tmp = p->val;
+ tmp &= ~PORT_PE;
+ tmp |= PORT_RESET;
+ p->val = tmp;
+
mdelay(50);
- p->port_reset = 0;
+ tmp &= ~PORT_RESET;
+ p->val = tmp;
+
/* Wait for flag change to finish. The controller might take a while */
- while (p->port_reset) ;
- if (!p->port_enable) {
+ while(p->val & PORT_RESET) ;
+ if (!(p->val & PORT_PE)) {
ehci_rh_hand_over_port(dev, port);
return;
}
printf("port %x hosts a USB2 device\n", port+1);
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->val;
+ tmp |= PORT_CSC;
+ p->val = tmp;
}
static int
ehci_rh_report_port_changes (usbdev_t *dev)
{
+ volatile portsc_t *p;
int i;
for (i=0; i<RH_INST(dev)->n_ports; i++) {
- if (RH_INST(dev)->ports[i].conn_status_change)
+ p = &(RH_INST(dev)->ports[i]);
+ if (p->val & PORT_CSC)
return i;
}
return -1;
@@ -121,6 +139,8 @@ ehci_rh_poll (usbdev_t *dev)
void
ehci_rh_init (usbdev_t *dev)
{
+ volatile portsc_t *p;
+ volatile u32 tmp;
int i;
dev->destroy = ehci_rh_destroy;
@@ -135,8 +155,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]);
+ tmp = p->val;
+ tmp |= PORT_POWER;
+ p->val = tmp;
RH_INST(dev)->devices[i] = -1;
- RH_INST(dev)->ports[i].pp = 1;
}
dev->address = 0;
More information about the coreboot
mailing list