From stern@rowland.harvard.edu Thu Oct 13 14:04:19 2005 Date: Thu, 13 Oct 2005 17:00:24 -0400 (EDT) From: Alan Stern To: Greg KH Subject: [PATCH] UHCI: Improve handling of iso TDs Message-ID: The uhci-hcd driver is fairly lax about the way it handles isochronous transfers. This patch (as579) improves it in three respects: TDs for a new URB aren't added to the schedule until all of them have been allocated. This way there's no risk of the controller executing some of them when an allocation fails. TDs for an unlinked URB are removed from the schedule as soon as the URB is unlinked, rather than waiting until the URB is given back. This way there's no risk of the controller still executing a TD after the URB completes. The urb->error_count values are now reported correctly. Although since they aren't used in any drivers except for debug messages in the system log, probably nobody cares. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/uhci-q.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) --- gregkh-2.6.orig/drivers/usb/host/uhci-q.c +++ gregkh-2.6/drivers/usb/host/uhci-q.c @@ -108,13 +108,16 @@ static void uhci_insert_td_frame_list(st } } -static void uhci_remove_td(struct uhci_hcd *uhci, struct uhci_td *td) +static inline void uhci_remove_td_frame_list(struct uhci_hcd *uhci, + struct uhci_td *td) { /* If it's not inserted, don't remove it */ - if (td->frame == -1 && list_empty(&td->fl_list)) + if (td->frame == -1) { + WARN_ON(!list_empty(&td->fl_list)); return; + } - if (td->frame != -1 && uhci->frame_cpu[td->frame] == td) { + if (uhci->frame_cpu[td->frame] == td) { if (list_empty(&td->fl_list)) { uhci->frame[td->frame] = td->link; uhci->frame_cpu[td->frame] = NULL; @@ -132,13 +135,20 @@ static void uhci_remove_td(struct uhci_h ptd->link = td->link; } - wmb(); - td->link = UHCI_PTR_TERM; - list_del_init(&td->fl_list); td->frame = -1; } +static void unlink_isochronous_tds(struct uhci_hcd *uhci, struct urb *urb) +{ + struct urb_priv *urbp = (struct urb_priv *) urb->hcpriv; + struct uhci_td *td; + + list_for_each_entry(td, &urbp->td_list, list) + uhci_remove_td_frame_list(uhci, td); + wmb(); +} + /* * Inserts a td list into qh. */ @@ -498,7 +508,6 @@ static void uhci_destroy_urb_priv(struct list_for_each_entry_safe(td, tmp, &urbp->td_list, list) { uhci_remove_td_from_urb(td); - uhci_remove_td(uhci, td); list_add(&td->remove_list, &uhci->td_remove_list); } @@ -1068,6 +1077,7 @@ static int uhci_submit_isochronous(struc struct uhci_td *td; int i, ret, frame; int status, destination; + struct urb_priv *urbp = (struct urb_priv *) urb->hcpriv; status = TD_CTRL_ACTIVE | TD_CTRL_IOS; destination = (urb->pipe & PIPE_DEVEP_MASK) | usb_packetid(urb->pipe); @@ -1076,11 +1086,7 @@ static int uhci_submit_isochronous(struc if (ret) return ret; - frame = urb->start_frame; - for (i = 0; i < urb->number_of_packets; i++, frame += urb->interval) { - if (!urb->iso_frame_desc[i].length) - continue; - + for (i = 0; i < urb->number_of_packets; i++) { td = uhci_alloc_td(uhci); if (!td) return -ENOMEM; @@ -1091,8 +1097,12 @@ static int uhci_submit_isochronous(struc if (i + 1 >= urb->number_of_packets) td->status |= cpu_to_le32(TD_CTRL_IOC); + } + frame = urb->start_frame; + list_for_each_entry(td, &urbp->td_list, list) { uhci_insert_td_frame_list(uhci, td, frame); + frame += urb->interval; } return -EINPROGRESS; @@ -1105,7 +1115,7 @@ static int uhci_result_isochronous(struc int status; int i, ret = 0; - urb->actual_length = 0; + urb->actual_length = urb->error_count = 0; i = 0; list_for_each_entry(td, &urbp->td_list, list) { @@ -1129,6 +1139,7 @@ static int uhci_result_isochronous(struc i++; } + unlink_isochronous_tds(uhci, urb); return ret; } @@ -1361,6 +1372,8 @@ static int uhci_urb_dequeue(struct usb_h goto done; list_del_init(&urbp->urb_list); + if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS) + unlink_isochronous_tds(uhci, urb); uhci_unlink_generic(uhci, urb); uhci_get_current_frame_number(uhci);