On Tue, 26 Jun 2007, ok wrote:
> > I took out the references to urb->lock in preproc_atl_queue and
> > isp116x_urb_enqueue. They didn't seem to serve any purpose; is that
> > correct?
>
> Seems to be correct. urb->hcpriv is already protected by isp116x->lock.
> Is urb->lock entirely to guard only urb->status? If so, is the urb->lock
> unnecessary also in preproc_atl_queue()?
That's right. The only part of an URB that usbcore will change while
the URB is active is the status, so that's the only part protected by
urb->lock. (Actually usbcore will also change urb->reject, but that
field isn't used by HCDs.)
> Sure this preproc_atl_queue() has suffered from my attitude "better not
> touch it if it works". Good that it can be simplified. Thanks for your
> review!
>
> Right now I don't have the hardware available. But I am working on it.
> These changes are big and have to be tested, no doubt. I will let you
> know.
Maybe the whole thing could be simplified even more. It's hard to say
exactly without being familiar with the hardware. For instance, is it
possible the multiple maxpacket-sized transactions succeeded and then
one failed?
I think the overall outline of the logic should look like this:
Find out how much has been successfully transferred and
accumulate the amount. If the ptd is still active then
exit immediately.
Find out if there was a low-level error (CRC, bit stuffing,
data toggle, stall, no response, invalid PID, buffer
over/underflow). Increment the error counter and retry the
last transaction if there are < 3 errors, otherwise fail the
transfer.
Find out if there was a high-level error (unexpected PID,
data overflow, data underflow + URB_SHORT_NOT_OK). These
cause the transfer to fail immediately.
If no short packet was received and the requested amount of
data has not yet been transferred, proceed to transfer the
remaining data.
Send a zero-length packet if needed. You might need an extra
flag in the endpoint structure to indicate whether the ZLP
has already been sent.
Do the status transaction for control transfers.
What I ended up with looked a lot like this, but not exactly the same.
It probably still needs some cleaning up.
Alan Stern
-------------------------------------------------------------------------
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
