On Thu, Jun 21, 2007 at 05:17:42PM -0400, Alan Stern wrote:
> On Thu, 21 Jun 2007, ok wrote:
> > Do you mean something like that below? I haven't tested it.
>
> Yes, something very much like this. It could be simplified a little;
> in a couple of places where you have
>
> finish_request();
> spin_unlock();
> continue;
>
> you ought to be able to use a "goto" or "break" instead. Also there
> are a few minor problems, like calling finish_request before it is
> declared and calling it before releasing urb->lock, but those can
> easily be fixed.
Yes, I made a mistake with urb->lock. OK, moved finish_request
unmodified up in the file, got fatter patch. The one below.
> If you like, I'll try revising your patch. You'll have to test it
Yes, thanks.
> though; I don't have the hardware.
Sure.
Olav
--- linux-2.6.22-rc5-or/drivers/usb/host/isp116x-hcd.c 2007-06-17
05:09:12.000000000 +0300
+++ linux-2.6.22-rc5-isp/drivers/usb/host/isp116x-hcd.c 2007-06-23
00:22:01.000000000 +0300
@@ -275,6 +275,61 @@
}
/*
+ Take done or failed requests out of schedule. Give back
+ processed urbs.
+*/
+static void finish_request(struct isp116x *isp116x, struct isp116x_ep *ep,
+ struct urb *urb)
+__releases(isp116x->lock) __acquires(isp116x->lock)
+{
+ unsigned i;
+
+ urb->hcpriv = NULL;
+ ep->error_count = 0;
+
+ if (usb_pipecontrol(urb->pipe))
+ ep->nextpid = USB_PID_SETUP;
+
+ urb_dbg(urb, "Finish");
+
+ spin_unlock(&isp116x->lock);
+ usb_hcd_giveback_urb(isp116x_to_hcd(isp116x), urb);
+ spin_lock(&isp116x->lock);
+
+ /* take idle endpoints out of the schedule */
+ if (!list_empty(&ep->hep->urb_list))
+ return;
+
+ /* async deschedule */
+ if (!list_empty(&ep->schedule)) {
+ list_del_init(&ep->schedule);
+ return;
+ }
+
+ /* periodic deschedule */
+ DBG("deschedule qh%d/%p branch %d\n", ep->period, ep, ep->branch);
+ for (i = ep->branch; i < PERIODIC_SIZE; i += ep->period) {
+ struct isp116x_ep *temp;
+ struct isp116x_ep **prev = &isp116x->periodic[i];
+
+ while (*prev && ((temp = *prev) != ep))
+ prev = &temp->next;
+ if (*prev)
+ *prev = ep->next;
+ isp116x->load[i] -= ep->load;
+ }
+ ep->branch = PERIODIC_SIZE;
+ isp116x_to_hcd(isp116x)->self.bandwidth_allocated -=
+ ep->load / ep->period;
+
+ /* switch irq type? */
+ if (!--isp116x->periodic_count) {
+ isp116x->irqenb &= ~HCuPINT_SOF;
+ isp116x->irqenb |= HCuPINT_ATL;
+ }
+}
+
+/*
Analyze transfer results, handle partial transfers and errors
*/
static void postproc_atl_queue(struct isp116x *isp116x)
@@ -296,11 +351,6 @@
short_not_ok = 1;
spin_lock(&urb->lock);
- /* Data underrun is special. For allowed underrun
- we clear the error and continue as normal. For
- forbidden underrun we finish the DATA stage
- immediately while for control transfer,
- we do a STATUS stage. */
if (cc == TD_DATAUNDERRUN) {
if (!(urb->transfer_flags & URB_SHORT_NOT_OK)) {
DBG("Allowed data underrun\n");
@@ -308,22 +358,14 @@
short_not_ok = 0;
} else {
ep->error_count = 1;
- if (usb_pipecontrol(urb->pipe))
- ep->nextpid = USB_PID_ACK;
- else
- usb_settoggle(udev, ep->epnum,
- ep->nextpid ==
- USB_PID_OUT,
- PTD_GET_TOGGLE(ptd));
+ usb_settoggle(udev, ep->epnum,
+ ep->nextpid == USB_PID_OUT,
+ PTD_GET_TOGGLE(ptd));
urb->actual_length += PTD_GET_COUNT(ptd);
urb->status = cc_to_error[TD_DATAUNDERRUN];
- spin_unlock(&urb->lock);
- continue;
+ goto done;
}
}
- /* Keep underrun error through the STATUS stage */
- if (urb->status == cc_to_error[TD_DATAUNDERRUN])
- cc = TD_DATAUNDERRUN;
if (cc != TD_CC_NOERROR && cc != TD_NOTACCESSED
&& (++ep->error_count >= 3 || cc == TD_CC_STALL
@@ -332,8 +374,7 @@
urb->status = cc_to_error[cc];
if (ep->nextpid == USB_PID_ACK)
ep->nextpid = 0;
- spin_unlock(&urb->lock);
- continue;
+ goto done;
}
/* According to usb spec, zero-length Int transfer signals
finishing of the urb. Hey, does this apply only
@@ -341,8 +382,7 @@
if (usb_pipeint(urb->pipe) && !PTD_GET_LEN(ptd)) {
if (urb->status == -EINPROGRESS)
urb->status = 0;
- spin_unlock(&urb->lock);
- continue;
+ goto done;
}
/* Relax after previously failed, but later succeeded
@@ -409,62 +449,10 @@
default:
BUG();
}
+ done:
spin_unlock(&urb->lock);
- }
-}
-
-/*
- Take done or failed requests out of schedule. Give back
- processed urbs.
-*/
-static void finish_request(struct isp116x *isp116x, struct isp116x_ep *ep,
- struct urb *urb)
-__releases(isp116x->lock) __acquires(isp116x->lock)
-{
- unsigned i;
-
- urb->hcpriv = NULL;
- ep->error_count = 0;
-
- if (usb_pipecontrol(urb->pipe))
- ep->nextpid = USB_PID_SETUP;
-
- urb_dbg(urb, "Finish");
-
- spin_unlock(&isp116x->lock);
- usb_hcd_giveback_urb(isp116x_to_hcd(isp116x), urb);
- spin_lock(&isp116x->lock);
-
- /* take idle endpoints out of the schedule */
- if (!list_empty(&ep->hep->urb_list))
- return;
-
- /* async deschedule */
- if (!list_empty(&ep->schedule)) {
- list_del_init(&ep->schedule);
- return;
- }
-
- /* periodic deschedule */
- DBG("deschedule qh%d/%p branch %d\n", ep->period, ep, ep->branch);
- for (i = ep->branch; i < PERIODIC_SIZE; i += ep->period) {
- struct isp116x_ep *temp;
- struct isp116x_ep **prev = &isp116x->periodic[i];
-
- while (*prev && ((temp = *prev) != ep))
- prev = &temp->next;
- if (*prev)
- *prev = ep->next;
- isp116x->load[i] -= ep->load;
- }
- ep->branch = PERIODIC_SIZE;
- isp116x_to_hcd(isp116x)->self.bandwidth_allocated -=
- ep->load / ep->period;
-
- /* switch irq type? */
- if (!--isp116x->periodic_count) {
- isp116x->irqenb &= ~HCuPINT_SOF;
- isp116x->irqenb |= HCuPINT_ATL;
+ if (urb->status != -EINPROGRESS)
+ finish_request(isp116x, ep, urb);
}
}
@@ -570,9 +558,6 @@
*/
static void finish_atl_transfers(struct isp116x *isp116x)
{
- struct isp116x_ep *ep;
- struct urb *urb;
-
if (!isp116x->atl_active)
return;
/* Fifo not ready? */
@@ -582,16 +567,6 @@
atomic_inc(&isp116x->atl_finishing);
unpack_fifo(isp116x);
postproc_atl_queue(isp116x);
- for (ep = isp116x->atl_active; ep; ep = ep->active) {
- urb =
- container_of(ep->hep->urb_list.next, struct urb, urb_list);
- /* USB_PID_ACK check here avoids finishing of
- control transfers, for which TD_DATAUNDERRUN
- occured, while URB_SHORT_NOT_OK was set */
- if (urb && urb->status != -EINPROGRESS
- && ep->nextpid != USB_PID_ACK)
- finish_request(isp116x, ep, urb);
- }
atomic_dec(&isp116x->atl_finishing);
}
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
