ChangeSet 1.1673.8.20, 2004/03/25 17:17:18-08:00, stern@rowland.harvard.edu [PATCH] USB: UHCI: Improved handling of short control transfers This patch simplies the way the UHCI driver handles short control transfers. When a transfer is short the HC will stop handling that endpoint, and it's necessary to get it going again so that the status stage of the control transfer can take place. Currently the driver does this by allocating a new QH for the transfer and setting its element pointer to point at the final status TD. The old QH is recycled. But it's not necessary to go to all that trouble; the element pointer in the original QH can be updated directly. Normally the element pointer is supposed to be owned by the HC, and it's not safe to just change its value since the HC may overwrite it at any time. But when a transfer is stopped because of a short packet, the current TD is marked inactive and the HC will not update the element pointer. To write an unchanged pointer value back to memory would be a waste of PCI bus cycles. Now the UHCI spec doesn't say explicitly that an HC _can't_ do this, but I've tested both Intel and VIA hardware and neither of them does. As a side effect of this change, some of the code for removing QHs can be eliminated. drivers/usb/host/uhci-hcd.c | 99 ++++++++++---------------------------------- 1 files changed, 24 insertions(+), 75 deletions(-) diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c --- a/drivers/usb/host/uhci-hcd.c Wed Apr 14 14:38:41 2004 +++ b/drivers/usb/host/uhci-hcd.c Wed Apr 14 14:38:41 2004 @@ -417,13 +417,8 @@ if (!qh) return; - qh->urbp = NULL; - /* * Only go through the hoops if it's actually linked in - * Queued QHs are removed in uhci_delete_queued_urb, - * since (for queued URBs) the pqh is pointed to the next - * QH in the queue, not the next endpoint's QH. */ spin_lock_irqsave(&uhci->frame_list_lock, flags); if (!list_empty(&qh->list)) { @@ -450,10 +445,21 @@ /* continue the rest of the schedule */ qh->element = UHCI_PTR_TERM; + /* If our queue is nonempty, make the next URB the head */ + if (!list_empty(&qh->urbp->queue_list)) { + struct urb_priv *nurbp; + + nurbp = list_entry(qh->urbp->queue_list.next, + struct urb_priv, queue_list); + nurbp->queued = 0; + list_add_tail(&nurbp->qh->list, &qh->list); + } list_del_init(&qh->list); } spin_unlock_irqrestore(&uhci->frame_list_lock, flags); + qh->urbp = NULL; + spin_lock_irqsave(&uhci->qh_remove_list_lock, flags); /* Check to see if the remove list is empty. Set the IOC bit */ @@ -603,39 +609,9 @@ usb_pipeout(urb->pipe), toggle); } - if (!urbp->queued) { - struct uhci_qh *pqh; - - nurbp->queued = 0; - - /* - * Fixup the previous QH's queue to link to the new head - * of this queue. - */ - pqh = list_entry(urbp->qh->list.prev, struct uhci_qh, list); - - if (pqh->urbp) { - struct list_head *head, *tmp; - - head = &pqh->urbp->queue_list; - tmp = head->next; - while (head != tmp) { - struct urb_priv *turbp = - list_entry(tmp, struct urb_priv, queue_list); - - tmp = tmp->next; - - turbp->qh->link = cpu_to_le32(nurbp->qh->dma_handle) | UHCI_PTR_QH; - } - } - - pqh->link = cpu_to_le32(nurbp->qh->dma_handle) | UHCI_PTR_QH; - - list_add_tail(&nurbp->qh->list, &urbp->qh->list); - list_del_init(&urbp->qh->list); - } else { - /* We're somewhere in the middle (or end). A bit trickier */ - /* than the head scenario */ + if (urbp->queued) { + /* We're somewhere in the middle (or end). The case where + * we're at the head is handled in uhci_remove_qh(). */ purbp = list_entry(urbp->queue_list.prev, struct urb_priv, queue_list); @@ -926,50 +902,23 @@ } /* - * If control was short, then end status packet wasn't sent, so this - * reorganize s so it's sent to finish the transfer. The original QH is - * removed from the skel and discarded; all TDs except the last (status) - * are deleted; the last (status) TD is put on a new QH which is reinserted - * into the skel. Since the last TD and urb_priv are reused, the TD->link - * and urb_priv maintain any queued QHs. + * If control-IN transfer was short, the status packet wasn't sent. + * This routine changes the element pointer in the QH to point at the + * status TD. It's safe to do this even while the QH is live, because + * the hardware only updates the element pointer following a successful + * transfer. The inactive TD for the short packet won't cause an update, + * so the pointer won't get overwritten. The next time the controller + * sees this QH, it will send the status packet. */ static int usb_control_retrigger_status(struct uhci_hcd *uhci, struct urb *urb) { - struct list_head *tmp, *head; struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv; + struct uhci_td *td; urbp->short_control_packet = 1; - /* Create a new QH to avoid pointer overwriting problems */ - uhci_remove_qh(uhci, urbp->qh); - - /* Delete all of the TD's except for the status TD at the end */ - head = &urbp->td_list; - tmp = head->next; - while (tmp != head && tmp->next != head) { - struct uhci_td *td = list_entry(tmp, struct uhci_td, list); - - tmp = tmp->next; - - uhci_remove_td_from_urb(td); - uhci_remove_td(uhci, td); - uhci_free_td(uhci, td); - } - - urbp->qh = uhci_alloc_qh(uhci, urb->dev); - if (!urbp->qh) - return -ENOMEM; - - urbp->qh->urbp = urbp; - - /* One TD, who cares about Breadth first? */ - uhci_insert_tds_in_qh(urbp->qh, urb, UHCI_PTR_DEPTH); - - /* Low-speed transfers get a different queue */ - if (urb->dev->speed == USB_SPEED_LOW) - uhci_insert_qh(uhci, uhci->skel_ls_control_qh, urb); - else - uhci_insert_qh(uhci, uhci->skel_hs_control_qh, urb); + td = list_entry(urbp->td_list.prev, struct uhci_td, list); + urbp->qh->element = td->dma_handle; return -EINPROGRESS; }