On Sun, 27 Mar 2011, bradhuang wrote:
> Dears,
>
> Excuse me!
> I am new in linux ehci, so I do not have a lot of ideas for ehci issues.
> After I trace ehci code, I found scan_periodic() take a lot of time for
> loops. When I goole the solution for watchdog reset issue. I found the patch
> from you. Then I apply that for my platform. But I do not get fully
> understand in scan_periodic().
> Could you give me the hints about the meaning of your patch below?
> Even briefly for the flow such that I can trace in depth. I really
> appreciate your idea.
> BTW. I have test for 2 days(48 hours). No any issues for webcamtest if I
> ignore the line I mentioned. I do not understand why it will not work if the
> line is ignored as your comment.
Okay. Here's how the patch works.
Each time scan_periodic() runs, it checks every frame that has elapsed
since the last time it was called. Within each frame, it checks all
the iTDs, siTDs, and QHs linked to that frame.
For the iTDs and siTDs this makes sense, because an iTD or siTD can
belong to only one frame. (The driver doesn't support siTD back
pointers.) But a QH will be linked into more than one frame, because
it appears in the linked list for one frame every interval. This means
the loop will call qh_completions() repeatedly for the same QH each
time it runs, which is a waste of time.
The patch introduces a new variable, ehci->periodic_stamp, which gets
incremented every time the loop starts. For each QH, qh->stamp stores
the value of ehci->periodic_stamp as of the last time the QH was
checked in qh_completions().
Index: usb-2.6/drivers/usb/host/ehci-sched.c
===================================================================
--- usb-2.6.orig/drivers/usb/host/ehci-sched.c
+++ usb-2.6/drivers/usb/host/ehci-sched.c
@@ -2261,6 +2261,7 @@ scan_periodic (struct ehci_hcd *ehci)
}
clock &= mod - 1;
clock_frame = clock >> 3;
+ ++ehci->periodic_stamp;
Here we increment the periodic_stamp variable because the loop is about
to start.
for (;;) {
union ehci_shadow q, *q_p;
@@ -2289,10 +2290,14 @@ restart:
temp.qh = qh_get (q.qh);
type = Q_NEXT_TYPE(ehci, q.qh->hw->hw_next);
q = q.qh->qh_next;
- modified = qh_completions (ehci, temp.qh);
- if (unlikely(list_empty(&temp.qh->qtd_list) ||
- temp.qh->needs_rescan))
- intr_deschedule (ehci, temp.qh);
+ if (temp.qh->stamp != ehci->periodic_stamp) {
This line causes the loop to avoid examining temp.qh if it has already
been seen earlier during the loop, that is, if its stamp is the same as
ehci->periodic_stamp.
+ modified = qh_completions(ehci, temp.qh);
+ if (!modified)
+ temp.qh->stamp = ehci->periodic_stamp;
Here we update temp.qh->stamp to indicate that it has been seen during
the current loop. But we do this only if qh_completions() returns 0,
meaning that no URBs have completed. If an URB completed then the
completion handler might have unlinked other URBs, so we will need to
examine temp.qh again; therefore temp.qh->stamp is not updated.
+ if (unlikely(list_empty(&temp.qh->qtd_list) ||
+ temp.qh->needs_rescan))
+ intr_deschedule(ehci, temp.qh);
+ }
qh_put (temp.qh);
break;
case Q_TYPE_FSTN:
@@ -2427,6 +2432,7 @@ restart:
free_cached_lists(ehci);
ehci->clock_frame = clock_frame;
}
+ ++ehci->periodic_stamp;
This line gets executed if the controller advances to a new microframe
while the loop was running. It causes us to call qh_completions() in
the case where an interrupt URB completed while the scan_periodic()
loop was running. It isn't necessary after all, because the URB
completion would cause another interrupt to occur and we would call
qh_completions() then.
So you're right, this line can be removed and everything will still
work.
} else {
now_uframe++;
now_uframe &= mod - 1;
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
