# This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.538 -> 1.539 # drivers/usb/host/uhci.c 1.40 -> 1.41 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/05/11 johannes@erdfelt.com 1.539 # [PATCH] uhci.c FSBR timeout # # There was a discussion a long time ago about how safe the bit operations # were as well as recently. # # set_bit/clear_bit are not safe on x86 UP, nor are they safe on other # architectures. It's also unclear from the UHCI spec if the HC's are safe # with respect to atomic updates to the status field. # # This patch ditches all of the uses of set_bit and clear_bit and changes # the algorithm that depended on it. # # The FSBR timeout algorithm would reenable FSBR for transfers when they # started making progress again. So instead of trying for this best case, # we convert the transfer over to depth first from the standard breadth # first. To make sure the transfer doesn't hog all of the bandwidth, every # 5th TD is left in breadth first mode. This will ensure other transfers # get some bandwidth. # # It's not perfect, but I think it's a good compromise. # # Note: td->info is read only by the HC, so we can touch it whenever we # want. # -------------------------------------------- # diff -Nru a/drivers/usb/host/uhci.c b/drivers/usb/host/uhci.c --- a/drivers/usb/host/uhci.c Sat May 11 22:29:19 2002 +++ b/drivers/usb/host/uhci.c Sat May 11 22:29:19 2002 @@ -101,6 +101,11 @@ #define IDLE_TIMEOUT (HZ / 20) /* 50 ms */ #define FSBR_DELAY (HZ / 20) /* 50 ms */ +/* When we timeout an idle transfer for FSBR, we'll switch it over to */ +/* depth first traversal. We'll do it in groups of this number of TD's */ +/* to make sure it doesn't hog all of the bandwidth */ +#define DEPTH_INTERVAL 5 + #define MAX_URB_LOOP 2048 /* Maximum number of linked URB's */ /* @@ -116,12 +121,20 @@ return 0; } +/* + * Technically, updating td->status here is a race, but it's not really a + * problem. The worst that can happen is that we set the IOC bit again + * generating a spurios interrupt. We could fix this by creating another + * QH and leaving the IOC bit always set, but then we would have to play + * games with the FSBR code to make sure we get the correct order in all + * the cases. I don't think it's worth the effort + */ static inline void uhci_set_next_interrupt(struct uhci *uhci) { unsigned long flags; spin_lock_irqsave(&uhci->frame_list_lock, flags); - set_bit(TD_CTRL_IOC_BIT, &uhci->skel_term_td->status); + uhci->skel_term_td->status |= TD_CTRL_IOC_BIT; spin_unlock_irqrestore(&uhci->frame_list_lock, flags); } @@ -130,7 +143,7 @@ unsigned long flags; spin_lock_irqsave(&uhci->frame_list_lock, flags); - clear_bit(TD_CTRL_IOC_BIT, &uhci->skel_term_td->status); + uhci->skel_term_td->status &= ~TD_CTRL_IOC_BIT; spin_unlock_irqrestore(&uhci->frame_list_lock, flags); } @@ -475,9 +488,9 @@ tmp = tmp->next; if (toggle) - set_bit(TD_TOKEN_TOGGLE, &td->info); + td->info |= TD_TOKEN_TOGGLE; else - clear_bit(TD_TOKEN_TOGGLE, &td->info); + td->info &= ~TD_TOKEN_TOGGLE; toggle ^= 1; } @@ -956,14 +969,6 @@ tmp = tmp->next; - if (urbp->fsbr_timeout && (td->status & TD_CTRL_IOC) && - !(td->status & TD_CTRL_ACTIVE)) { - uhci_inc_fsbr(urb->dev->bus->hcpriv, urb); - urbp->fsbr_timeout = 0; - urbp->fsbrtime = jiffies; - clear_bit(TD_CTRL_IOC_BIT, &td->status); - } - status = uhci_status_bits(td->status); if (status & TD_CTRL_ACTIVE) return -EINPROGRESS; @@ -1132,14 +1137,6 @@ tmp = tmp->next; - if (urbp->fsbr_timeout && (td->status & TD_CTRL_IOC) && - !(td->status & TD_CTRL_ACTIVE)) { - uhci_inc_fsbr(urb->dev->bus->hcpriv, urb); - urbp->fsbr_timeout = 0; - urbp->fsbrtime = jiffies; - clear_bit(TD_CTRL_IOC_BIT, &td->status); - } - status = uhci_status_bits(td->status); if (status & TD_CTRL_ACTIVE) return -EINPROGRESS; @@ -1839,11 +1836,18 @@ { struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv; struct list_head *head, *tmp; + int count = 0; uhci_dec_fsbr(uhci, urb); urbp->fsbr_timeout = 1; + /* + * Ideally we would want to fix qh->element as well, but it's + * read/write by the HC, so that can introduce a race. It's not + * really worth the hassle + */ + head = &urbp->td_list; tmp = head->next; while (tmp != head) { @@ -1851,10 +1855,15 @@ tmp = tmp->next; - if (td->status & TD_CTRL_ACTIVE) { - set_bit(TD_CTRL_IOC_BIT, &td->status); - break; - } + /* + * Make sure we don't do the last one (since it'll have the + * TERM bit set) as well as we skip every so many TD's to + * make sure it doesn't hog the bandwidth + */ + if (tmp != head && (count % DEPTH_INTERVAL) == (DEPTH_INTERVAL - 1)) + td->link |= UHCI_PTR_DEPTH; + + count++; } return 0;