This patch will fix QUEUE_BULK. I'm still having some issues, but the only
device I could test with was a hacked up driver for the kawasaki ethernet
chip and I'm not convinced where the problem is exactly.
The kawasaki seems to send 64 bytes packets, but doesn't send a 0 byte
packet to signal an end of transfer.
So, some packets get appended back to back, and some packets get spread
across multiple buffers.
If I disable QUEUE_BULK, then everything is very reliable. I got an
average of 2.2ms latency on one end, and 3.2ms latency on the other
end for pings.
Here's a complete list of changes:
- Fix bug with printing queue on error (debugging only)
- Fix bug with SPD turned off on transfers
- Re-enable FSBR if a transfer had a timeout when data starts coming
- New uhci_show_urb_queue which shows all TD's for an URB. Includes
improved heuristics to cut out long strings of active TD's
The patch is against linux-2.3.99-pre6-5
Randy, please send this on to Linus
JE
--- linux-2.3.99-pre6-5/drivers/usb/uhci.c Tue Apr 11 09:57:43 2000
+++ linux-2.3.99-pre4-5/drivers/usb/uhci.c Fri Apr 21 19:35:01 2000
@@ -284,42 +284,6 @@
prevtd->link = UHCI_PTR_TERM;
}
-/* This function will append one URB's QH to another URB's QH. This is for */
-/* USB_QUEUE_BULK support */
-static void uhci_append_urb_qh(struct uhci *uhci, struct urb *eurb, struct urb *urb)
-{
- struct urb *nurb;
- struct urb_priv *eurbp, *urbp, *nurbp;
- struct list_head *tmp;
- struct uhci_td *td, *ntd;
- unsigned long flags;
-
- eurbp = eurb->hcpriv;
- urbp = urb->hcpriv;
-
- spin_lock_irqsave(&eurb->lock, flags);
-
- /* Grab the last URB in the queue */
- tmp = eurbp->urb_queue_list.prev;
-
- /* Add this one to the end */
- list_add_tail(&urbp->urb_queue_list, &eurbp->urb_queue_list);
-
- spin_unlock_irqrestore(&eurb->lock, flags);
-
- nurbp = list_entry(tmp, struct urb_priv, urb_queue_list);
- nurb = nurbp->urb;
-
- tmp = nurbp->list.prev;
- td = list_entry(tmp, struct uhci_td, list);
-
- tmp = urbp->list.next;
- ntd = list_entry(tmp, struct uhci_td, list);
-
- /* No breadth since this will only be called for bulk transfers */
- td->link = virt_to_bus(ntd);
-}
-
static void uhci_free_td(struct uhci_td *td)
{
if (!list_empty(&td->list))
@@ -395,6 +359,7 @@
if (qh->nextqh)
qh->nextqh->prevqh = qh->prevqh;
qh->prevqh = qh->nextqh = NULL;
+ qh->element = qh->link = UHCI_PTR_TERM;
spin_unlock_irqrestore(&uhci->framelist_lock, flags);
if (delayed) {
@@ -413,6 +378,101 @@
uhci_free_qh(qh);
}
+static spinlock_t uhci_append_urb_lock = SPIN_LOCK_UNLOCKED;
+
+/* This function will append one URB's QH to another URB's QH. This is for */
+/* USB_QUEUE_BULK support */
+static void uhci_append_queued_urb(struct uhci *uhci, struct urb *eurb, struct urb
+*urb)
+{
+ struct urb_priv *eurbp, *urbp, *furbp, *lurbp;
+ struct list_head *tmp;
+ struct uhci_td *td, *ltd;
+ unsigned long flags;
+
+ eurbp = eurb->hcpriv;
+ urbp = urb->hcpriv;
+
+ spin_lock_irqsave(&uhci_append_urb_lock, flags);
+
+ /* Find the beginning URB in the queue */
+ if (eurbp->queued) {
+ struct list_head *head = &eurbp->urb_queue_list;
+
+ tmp = head->next;
+ while (tmp != head) {
+ struct urb_priv *turbp =
+ list_entry(tmp, struct urb_priv, urb_queue_list);
+
+ tmp = tmp->next;
+
+ if (!turbp->queued)
+ break;
+ }
+ } else
+ tmp = &eurbp->urb_queue_list;
+
+ furbp = list_entry(tmp, struct urb_priv, urb_queue_list);
+
+ tmp = furbp->urb_queue_list.prev;
+ lurbp = list_entry(tmp, struct urb_priv, urb_queue_list);
+
+ /* Add this one to the end */
+ list_add_tail(&urbp->urb_queue_list, &furbp->urb_queue_list);
+
+ /* Grab the last TD from the last URB */
+ ltd = list_entry(lurbp->list.prev, struct uhci_td, list);
+
+ /* Grab the first TD from the first URB */
+ td = list_entry(urbp->list.next, struct uhci_td, list);
+
+ /* No breadth since this will only be called for bulk transfers */
+ ltd->link = virt_to_bus(td);
+
+ spin_unlock_irqrestore(&uhci_append_urb_lock, flags);
+}
+
+static void uhci_delete_queued_urb(struct uhci *uhci, struct urb *urb)
+{
+ struct urb_priv *urbp, *nurbp;
+ unsigned long flags;
+
+ urbp = urb->hcpriv;
+
+ spin_lock_irqsave(&uhci_append_urb_lock, flags);
+
+ nurbp = list_entry(urbp->urb_queue_list.next, struct urb_priv,
+ urb_queue_list);
+
+ if (!urbp->queued) {
+ /* We're the head, so just insert the QH for the next URB */
+ uhci_insert_qh(uhci, &uhci->skel_bulk_qh, nurbp->qh);
+ nurbp->queued = 0;
+ } else {
+ struct urb_priv *purbp;
+ struct uhci_td *ptd;
+
+ /* We're somewhere in the middle (or end). A bit trickier */
+ /* than the head scenario */
+ purbp = list_entry(urbp->urb_queue_list.prev, struct urb_priv,
+ urb_queue_list);
+
+ ptd = list_entry(purbp->list.prev, struct uhci_td, list);
+ if (nurbp->queued)
+ /* Close the gap between the two */
+ ptd->link = virt_to_bus(list_entry(nurbp->list.next,
+ struct uhci_td, list));
+ else
+ /* The next URB happens to be the beggining, so */
+ /* we're the last, end the chain */
+ ptd->link = UHCI_PTR_TERM;
+
+ }
+
+ list_del(&urbp->urb_queue_list);
+
+ spin_unlock_irqrestore(&uhci_append_urb_lock, flags);
+}
+
struct urb_priv *uhci_alloc_urb_priv(struct urb *urb)
{
struct urb_priv *urbp;
@@ -491,11 +551,6 @@
uhci_free_td(td);
}
- if (!list_empty(&urbp->urb_queue_list)) {
- list_del(&urbp->urb_queue_list);
- INIT_LIST_HEAD(&urbp->urb_queue_list);
- }
-
urb->hcpriv = NULL;
kmem_cache_free(uhci_up_cachep, urbp);
@@ -672,12 +727,10 @@
if (urb->pipe & TD_CTRL_LS) {
uhci_insert_tds_in_qh(qh, urb, 0);
uhci_insert_qh(uhci, &uhci->skel_ls_control_qh, qh);
- urbp->queued = 0;
} else {
uhci_insert_tds_in_qh(qh, urb, 1);
uhci_insert_qh(uhci, &uhci->skel_hs_control_qh, qh);
uhci_inc_fsbr(uhci, urb);
- urbp->queued = 0;
}
urbp->qh = qh;
@@ -697,18 +750,21 @@
struct urb_priv *urbp = urb->hcpriv;
struct uhci_td *td;
unsigned int status;
+ int ret = 0;
if (!urbp)
return -EINVAL;
head = &urbp->list;
- tmp = head->next;
- if (head == tmp)
+ if (head->next == head)
return -EINVAL;
- if (urbp->short_control_packet)
+ if (urbp->short_control_packet) {
+ tmp = head->prev;
goto status_phase;
+ }
+ tmp = head->next;
td = list_entry(tmp, struct uhci_td, list);
/* The first TD is the SETUP phase, check the status, but skip */
@@ -729,20 +785,34 @@
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;
+ td->status &= ~TD_CTRL_IOC;
+ }
+
status = uhci_status_bits(td->status);
if (status & TD_CTRL_ACTIVE)
return -EINPROGRESS;
urb->actual_length += uhci_actual_length(td->status);
- /* If SPD is set then we received a short packet */
- /* There will be no status phase at the end */
- if ((td->status & TD_CTRL_SPD) &&
- (uhci_actual_length(td->status) < uhci_expected_length(td->info)))
- return usb_control_retrigger_status(urb);
-
if (status)
goto td_error;
+
+ /* Check to see if we received a short packet */
+ if (uhci_actual_length(td->status) < uhci_expected_length(td->info)) {
+ if (urb->transfer_flags & USB_DISABLE_SPD) {
+ ret = -EREMOTEIO;
+ goto err;
+ }
+
+ if (uhci_packetid(td->info) == USB_PID_IN)
+ return usb_control_retrigger_status(urb);
+ else
+ return 0;
+ }
}
status_phase:
@@ -768,19 +838,22 @@
return 0;
td_error:
- if (status & TD_CTRL_STALLED)
+ ret = uhci_map_status(status, uhci_packetout(td->info));
+ if (ret == -EPIPE)
/* endpoint has stalled - mark it halted */
usb_endpoint_halt(urb->dev, uhci_endpoint(td->info),
uhci_packetout(td->info));
- else if (debug) {
+
+err:
+ if (debug && ret != -EPIPE) {
/* Some debugging code */
dbg("uhci_result_control() failed with status %x", status);
/* Print the chain for debugging purposes */
- uhci_show_queue(urbp->qh);
+ uhci_show_urb_queue(urb);
}
- return uhci_map_status(status, uhci_packetout(td->info));
+ return ret;
}
static int usb_control_retrigger_status(struct urb *urb)
@@ -823,7 +896,6 @@
uhci_insert_qh(uhci, &uhci->skel_ls_control_qh, urbp->qh);
else
uhci_insert_qh(uhci, &uhci->skel_hs_control_qh, urbp->qh);
- urbp->queued = 0;
return -EINPROGRESS;
}
@@ -869,8 +941,9 @@
{
struct list_head *tmp, *head;
struct urb_priv *urbp = urb->hcpriv;
- unsigned int status;
struct uhci_td *td;
+ unsigned int status;
+ int ret = 0;
if (!urbp)
return -EINVAL;
@@ -884,46 +957,58 @@
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;
+ td->status &= ~TD_CTRL_IOC;
+ }
+
status = uhci_status_bits(td->status);
if (status & TD_CTRL_ACTIVE)
return -EINPROGRESS;
urb->actual_length += uhci_actual_length(td->status);
- /* If SPD is set then we received a short packet */
- if ((td->status & TD_CTRL_SPD) &&
- (uhci_actual_length(td->status) < uhci_expected_length(td->info)))
{
+ if (status)
+ goto td_error;
+
+ if (uhci_actual_length(td->status) < uhci_expected_length(td->info)) {
usb_settoggle(urb->dev, uhci_endpoint(td->info),
uhci_packetout(td->info),
uhci_toggle(td->info) ^ 1);
- return 0;
+ if (urb->transfer_flags & USB_DISABLE_SPD) {
+ ret = -EREMOTEIO;
+ goto err;
+ } else
+ return 0;
}
-
- if (status)
- goto td_error;
}
return 0;
td_error:
- if (status & TD_CTRL_STALLED)
+ ret = uhci_map_status(status, uhci_packetout(td->info));
+ if (ret == -EPIPE)
/* endpoint has stalled - mark it halted */
usb_endpoint_halt(urb->dev, uhci_endpoint(td->info),
uhci_packetout(td->info));
- else if (debug) {
+
+err:
+ if (debug && ret != -EPIPE) {
/* Some debugging code */
dbg("uhci_result_interrupt/bulk() failed with status %x",
status);
/* Print the chain for debugging purposes */
if (urbp->qh)
- uhci_show_queue(urbp->qh);
+ uhci_show_urb_queue(urb);
else
uhci_show_td(td);
}
- return uhci_map_status(status, uhci_packetout(td->info));
+ return ret;
}
static void uhci_reset_interrupt(struct urb *urb)
@@ -974,6 +1059,7 @@
/* 3 errors */
status = TD_CTRL_ACTIVE | (3 << TD_CTRL_C_ERR_SHIFT);
+
if (!(urb->transfer_flags & USB_DISABLE_SPD))
status |= TD_CTRL_SPD;
@@ -1016,12 +1102,10 @@
uhci_insert_tds_in_qh(qh, urb, 1);
if (urb->transfer_flags & USB_QUEUE_BULK && eurb) {
- uhci_append_urb_qh(uhci, eurb, urb);
urbp->queued = 1;
- } else {
+ uhci_append_queued_urb(uhci, eurb, urb);
+ } else
uhci_insert_qh(uhci, &uhci->skel_bulk_qh, qh);
- urbp->queued = 0;
- }
uhci_add_urb_list(uhci, urb);
@@ -1049,6 +1133,8 @@
while (tmp != head) {
struct urb *u = list_entry(tmp, struct urb, urb_list);
+ tmp = tmp->next;
+
/* look for pending URB's with identical pipe handle */
if ((urb->pipe == u->pipe) && (urb->dev == u->dev) &&
(u->status == -EINPROGRESS) && (u != urb)) {
@@ -1056,7 +1142,6 @@
*start = u->start_frame;
last_urb = u;
}
- tmp = tmp->next;
}
if (last_urb) {
@@ -1363,14 +1448,8 @@
/* The interrupt loop will reclaim the QH's */
uhci_remove_qh(uhci, urbp->qh);
- if (!list_empty(&urbp->urb_queue_list)) {
- struct list_head *tmp = urbp->urb_queue_list.next;
- struct urb_priv *nurbp = list_entry(tmp, struct urb_priv,
urb_queue_list);
- if (nurbp->queued) {
- uhci_insert_qh(uhci, &uhci->skel_bulk_qh, nurbp->qh);
- nurbp->queued = 0;
- }
- }
+ if (!list_empty(&urbp->urb_queue_list))
+ uhci_delete_queued_urb(uhci, urb);
uhci_destroy_urb_priv(urb);
@@ -1430,6 +1509,35 @@
return ret;
}
+static int uhci_fsbr_timeout(struct uhci *uhci, struct urb *urb)
+{
+ struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
+ struct list_head *head, *tmp;
+
+ uhci_dec_fsbr(uhci, urb);
+
+ /* There is a race with updating IOC in here, but it's not worth */
+ /* trying to fix since this is merely an optimization. The only */
+ /* time we'd lose is if the status of the packet got updated */
+ /* and we'd be turning on FSBR next frame anyway, so it's a wash */
+ urbp->fsbr_timeout = 1;
+
+ head = &urbp->list;
+ tmp = head->next;
+ while (tmp != head) {
+ struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
+
+ tmp = tmp->next;
+
+ if (td->status & TD_CTRL_ACTIVE) {
+ td->status |= TD_CTRL_IOC;
+ break;
+ }
+ }
+
+ return 0;
+}
+
/*
* uhci_get_current_frame_number()
*
@@ -1586,7 +1694,7 @@
if (urbp) {
/* Check if the FSBR timed out */
if (urbp->fsbr && time_after(urbp->inserttime + IDLE_TIMEOUT,
jiffies))
- uhci_dec_fsbr(uhci, u);
+ uhci_fsbr_timeout(uhci, u);
/* Check if the URB timed out */
if (u->timeout && time_after(u->timeout, jiffies)) {
@@ -1762,7 +1870,7 @@
case RH_PORT_POWER:
OK(0); /* port power ** */
case RH_PORT_ENABLE:
- SET_RH_PORTSTAT (USBPORTSC_PE);
+ SET_RH_PORTSTAT(USBPORTSC_PE);
OK(0);
}
break;
--- linux-2.3.99-pre6-5/drivers/usb/uhci.h Mon Apr 10 22:57:06 2000
+++ linux-2.3.99-pre4-5/drivers/usb/uhci.h Thu Apr 20 16:10:12 2000
@@ -339,10 +339,10 @@
struct uhci_qh *qh; /* QH for this URB */
- int fsbr : 1; /* Did this URB turn on FSBR? */
- int queued : 1; /* 0 if QH was linked in */
-
- char short_control_packet; /* If we get a short packet during */
+ int fsbr : 1; /* URB turned on FSBR */
+ int fsbr_timeout : 1; /* URB timed out on FSBR */
+ int queued : 1; /* QH was queued (not linked in) */
+ int short_control_packet : 1; /* If we get a short packet during */
/* a control transfer, retrigger */
/* the status phase */
@@ -414,6 +414,7 @@
/* Debugging code */
void uhci_show_td(struct uhci_td *td);
void uhci_show_status(struct uhci *uhci);
+void uhci_show_urb_queue(struct urb *urb);
void uhci_show_queue(struct uhci_qh *qh);
void uhci_show_queues(struct uhci *uhci);
--- linux-2.3.99-pre6-5/drivers/usb/uhci-debug.h Sun Feb 20 13:07:04 2000
+++ linux-2.3.99-pre4-5/drivers/usb/uhci-debug.h Fri Apr 21 18:14:03 2000
@@ -14,7 +14,7 @@
#include "uhci.h"
-void uhci_show_td(struct uhci_td * td)
+void uhci_show_td(struct uhci_td *td)
{
char *spid;
@@ -127,6 +127,62 @@
return NULL;
return bus_to_virt(link & ~UHCI_PTR_BITS);
+}
+
+void uhci_show_urb_queue(struct urb *urb)
+{
+ struct urb_priv *urbp = urb->hcpriv;
+ struct list_head *head, *tmp;
+ int i, checked = 0, prevactive = 0;
+
+ printk(" URB [%p] urbp [%p]\n", urb, urbp);
+
+ if (urbp->qh)
+ printk(" QH [%p]\n", urbp->qh);
+ else
+ printk(" QH [%p] element (%08x) link (%08x)\n", urbp->qh,
+ urbp->qh->element, urbp->qh->link);
+
+ i = 0;
+
+ head = &urbp->list;
+ tmp = head->next;
+ while (tmp != head) {
+ struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
+
+ tmp = tmp->next;
+
+ printk(" td %d: [%p]\n", i++, td);
+ printk(" ");
+ uhci_show_td(td);
+
+ if (i > 10 && !checked && prevactive && tmp != head) {
+ struct list_head *ntmp = tmp;
+ struct uhci_td *ntd = td;
+ int active = 1, ni = i;
+
+ checked = 1;
+
+ while (ntmp != head && ntmp->next != head && active) {
+ ntd = list_entry(ntmp, struct uhci_td, list);
+
+ ntmp = ntmp->next;
+
+ active = ntd->status & TD_CTRL_ACTIVE;
+
+ ni++;
+ }
+
+ if (active && ni > i) {
+ printk(" [skipped %d active TD's]\n", ni - i);
+ tmp = ntmp;
+ td = ntd;
+ i = ni;
+ }
+ }
+
+ prevactive = td->status & TD_CTRL_ACTIVE;
+ }
}
void uhci_show_queue(struct uhci_qh *qh)
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
