[U-Boot] [PATCH 1/2] ush: ehci: initialize altnext pointers in QH

[Marek Vasut]

On Friday, February 07, 2014 at 07:47:14 AM, Stephen Warren wrote:
> On 02/06/2014 07:53 PM, Marek Vasut wrote:
> > On Thursday, February 06, 2014 at 09:13:05 PM, Stephen Warren wrote:
> >> From: Stephen Warren <swarren at nvidia.com>
> >> 
> >> A QH's overlay QTD altnext pointer should be explicitly marked invalid
> >> so that the EHCI controller knows to look at the QTD next pointer.
> >> Update the driver to do this.
> > 
> > Can you please add a reference to the spec where this is stated ?
> 
> Section 4.10.2 "Advance Queue". Are you simply asking me to repost with
> that added to the commit description, or justify the change?

Justify it. Of course, clear and helpful commit message like so would be very 
nice, so feel free to repost this please.

> FWIW, the situation is that the QH set up by ehci_submit_async() does
> not have the active or halt bits set the first time around, so after the
> "Fetch Queue Head" state, the "Advance Queue" state is entered, and this
> is when the T bits of the Alternate Next qTD Pointer and Next qTD
> Pointer are checked. I guess since I can download the PDF from Intel's
> website without agreeing to any kind of license, I can quote it:
> 
> If the field Bytes to Transfer is not zero and the T-bit in the
> Alternate Next qTD Pointer is set to zero, then the host controller uses
> the Alternate Next qTD Pointer. Otherwise, the host controller uses the
> Next qTD Pointer. If Next qTD Pointer’s T-bit is set to a one, then the
> host controller exits this state and uses the horizontal pointer to the
> next schedule data structure.
> 
> >> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> >> 
> >> @@ -1186,6 +1187,7 @@ create_int_queue(struct usb_device *dev, unsigned
> >> long pipe, int queuesize, qh->qh_link = QH_LINK_TERMINATE;
> >> 
> >>  		qh->qh_overlay.qt_next = (uint32_t)td;
> >> 
> >> +		qh->qh_overlay.qt_altnext = QT_NEXT_TERMINATE;
> > 
> > So next is td and altnext is terminate here ?
> 
> Yes. TERMINATE really just means INVALID, so the EHCI controller gets
> the pointer from qt_next rather than qt_altnext.

Hmmmm ... not really clear or self-explaining, is it. If you have time to 
produce a RFC to clean this up, I'm all for checking it.

Best regards,
Marek Vasut