On Wed, 28 Aug 2013, Ming Lei wrote:
> Below is the approach I proposed(mentioned in another thread), which should be
> simper than this one, any comments?
>
> drivers/usb/host/ehci-sched.c | 53 ++++++++++++++++++++++++++++++++++++++---
> drivers/usb/host/ehci.h | 1 +
> 2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index 83be03f..80ef95d 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -1370,6 +1370,33 @@ sitd_slot_ok (
> return 1;
> }
>
> +/* rebase in case of underun without ISO_ASAP together */
> +static int iso_stream_rebase(struct ehci_hcd *ehci, struct urb *urb,
> + struct ehci_iso_stream *stream, u32 span, u32 period)
> +{
> + u32 i, end;
> + u32 mod = ehci->periodic_size;
> +
> + if (ehci->last_base == -1)
> + return 0;
> +
> + end = ((stream->next_uframe + span - period) >> 3) & (mod - 1);
> + for (i = ehci->last_base; i != ehci->last_iso_frame;
> + i = (i + 1) & (mod - 1)) {
> + /* don't schedule URB which is behind base totally */
> + if (end == i) {
> + for (i = 0; i < urb->number_of_packets; i++) {
> + urb->iso_frame_desc[i].length = 0;
> + urb->iso_frame_desc[i].status = 0;
> + }
> + return 1;
> + }
> + if (((stream->next_uframe >> 3) & (mod - 1)) == i)
> + ehci->last_iso_frame = i;
> + }
Why do you use a loop here? This looks like a straightforward thing to
test: Starting from last_base, which comes first: next_uframe or
last_iso_frame?
It's not a very good idea to change iso_frame_desc[i].length. HCDs
aren't suppose to touch that field. Also, why set the frame
descriptor status to 0? It should remain equal to -EXDEV, because the
frame never gets sent over the bus.
You must never alter ehci->last_iso_frame like this. It violates the
driver's invariants for time to run "backward". After all, there may
already be other TDs scheduled for the frames you are about to scan
again; they mustn't get scanned until the frame pointer has wrapped
around.
This last problem in particular means your proposal can't be accepted.
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
