Skip to contentSkip to conversation

EHCI: zero out QH transfer overlay in ehci_submit_async()

PATCH New Sergei Shtylyov 2 participants 2 messages Jun 28, 2010 – Jun 29, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 EHCI: zero out QH transfer overlay in ehci_submit_async()

ehci_submit_async() doesn't really zero out the QH transfer overlay (as the EHCI specification suggests) which leads to the controller seeing the "token" field as the previous call has left it, i.e.:
- if a timeout occured on the previous call (Active bit left as 1), controller incorrectly tries to complete a previous transaction on a newly programmed endpoint;
- if a halt occured on the previous call (Halted bit set to 1), controller just ignores the newly programmed TD(s) and the function then keeps returning error ad infinitum.

This turned out to be caused by the wrong orger of the arguments to the memset() call in ehci_alloc(), so the allocated TDs weren't cleared either.

While at it, stop needlessly initializing the alternate next TD pointer in the QH transfer overlay...

---
Initialization of 'qh->qh_overlay.qt_next' turned out to be needed by the error cleanup code (seemingly useless, in its turn).

This is quite serious bug, so would be good to have the patch in v2010.06...

 drivers/usb/host/ehci-hcd.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/usb/host/ehci-hcd.c +1 -2
Index: u-boot/drivers/usb/host/ehci-hcd.c
===================================================================
@@ -275,7 +275,7 @@ static void *ehci_alloc(size_t sz, size_
 		return NULL;
 	}
 
-	memset(p, sz, 0);
+	memset(p, 0, sz);
 	return p;
 }
 
@@ -350,7 +350,6 @@ ehci_submit_async(struct usb_device *dev
 	    (dev->parent->devnum << 16) | (0 << 8) | (0 << 0);
 	qh->qh_endpt2 = cpu_to_hc32(endpt);
 	qh->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
-	qh->qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 
 	td = NULL;
 	tdp = &qh->qh_overlay.qt_next;
#1 lore raw

PATCH v2 1/1 EHCI: zero out QH transfer overlay in ehci_submit_async()

Dear Sergei Shtylyov,

Quoted 18 lines · In message <[email protected]> you wrote:

ehci_submit_async() doesn't really zero out the QH transfer overlay (as the EHCI specification suggests) which leads to the controller seeing the "token" field as the previous call has left it, i.e.:
- if a timeout occured on the previous call (Active bit left as 1), controller incorrectly tries to complete a previous transaction on a newly programmed endpoint;
- if a halt occured on the previous call (Halted bit set to 1), controller just ignores the newly programmed TD(s) and the function then keeps returning error ad infinitum.

This turned out to be caused by the wrong orger of the arguments to the memset() call in ehci_alloc(), so the allocated TDs weren't cleared either.

While at it, stop needlessly initializing the alternate next TD pointer in the QH transfer overlay...

Quoted 8 lines

---
Initialization of 'qh->qh_overlay.qt_next' turned out to be needed by the error cleanup code (seemingly useless, in its turn).

This is quite serious bug, so would be good to have the patch in v2010.06...

 drivers/usb/host/ehci-hcd.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Small is beautiful.