On Mon, Jun 18, 2007 at 11:00:10AM -0400, Alan Stern wrote:
> Olav,
>
> What would be involved in refactoring isp116x-hcd so that the driver
> called usb_hcd_giveback_urb() as soon as the URB was completed? If I
> understand the current code correctly, postproc_atl_queue() goes
> through all the active endpoints and stores the status for the
> completed URBs, and then later finish_atl_transfers() gives them back.
>
> It looks like this shouldn't present any difficulty except for one
> thing: When you get a short Control-IN transfer with URB_SHORT_NOT_OK
> set, the error status gets stored while the URB's status stage
> executes. That would have to be changed so that the status stage was
> skipped entirely. Doing this shouldn't cause any problems in practice;
> the other HCDs skip the status stage when this sort of error occurs.
>
> The point of making this change is that it would allow the driver to
> avoid saving values in urb->status, leading eventually to the
> possibility of removing the status field from struct urb.
Hi Alan,
Do you mean something like that below? I haven't tested it.
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-20
22:49:27.000000000 +0300
@@ -210,6 +210,9 @@
/*---------------------------------------------------------------*/
+static void finish_request(struct isp116x *isp116x, struct isp116x_ep *ep,
+ struct urb *urb);
+
/*
Set up PTD's.
*/
@@ -296,11 +299,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 +306,16 @@
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];
+ finish_request(isp116x, ep, urb);
spin_unlock(&urb->lock);
continue;
}
}
- /* 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,6 +324,7 @@
urb->status = cc_to_error[cc];
if (ep->nextpid == USB_PID_ACK)
ep->nextpid = 0;
+ finish_request(isp116x, ep, urb);
spin_unlock(&urb->lock);
continue;
}
@@ -341,6 +334,7 @@
if (usb_pipeint(urb->pipe) && !PTD_GET_LEN(ptd)) {
if (urb->status == -EINPROGRESS)
urb->status = 0;
+ finish_request(isp116x, ep, urb);
spin_unlock(&urb->lock);
continue;
}
@@ -409,6 +403,8 @@
default:
BUG();
}
+ if (urb->status != -EINPROGRESS)
+ finish_request(isp116x, ep, urb);
spin_unlock(&urb->lock);
}
}
@@ -570,9 +566,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 +575,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/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
