From 2212df84bf7b5cf81759829a7854e6ebe9335fcf Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 25 Mar 2004 01:17:18 -0800 Subject: [PATCH] [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 file changed, 24 insertions(+), 75 deletions(-) diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c index 2c4de26d8a264..1815206c760f8 100644 --- a/drivers/usb/host/uhci-hcd.c +++ b/drivers/usb/host/uhci-hcd.c @@ -417,13 +417,8 @@ static void uhci_remove_qh(struct uhci_hcd *uhci, struct uhci_qh *qh) 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 @@ static void uhci_remove_qh(struct uhci_hcd *uhci, struct uhci_qh *qh) /* 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 @@ static void uhci_delete_queued_urb(struct uhci_hcd *uhci, struct urb *urb) 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 @@ static int uhci_submit_control(struct uhci_hcd *uhci, struct urb *urb, struct ur } /* - * 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; } -- 2.53.0