Some EHCI controllers misbehave if bits in the portsc register do not
change atomically. Thus we use a temporary variable to do a single
read-modify-write when changing related bits in the register.
The symptom is an incorrect handoff from EHCI to OHCI.
Signed-off-by: Steven A. Falco <sfalco(a)coincident.com>
---
I've observed the problem with several EHCI controllers: an Intel
Topcliff IOH and a Pericom PI7C9X440SL. With the changes below, both
these controllers work properly.
Note that several other registers in ehci_private.h are described in
terms of separate bits too. I have not changed them because I have not
observed any problems with them.
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;