On Mon, 7 Oct 2013, Sarah Sharp wrote:
> > i am using 3.10 kernel. Also i looked at tip i see same implementation for
> > process_ctrl_td()
> >
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/host/xhci-ring.c
>
> Yeah, the implementation has always been this way. I've known since the
> beginning of writing the xHCI driver that it needs to handle zero-length
> data phases, but hadn't had ever had a report that it was required by a
> device under Linux.
This code, near the end of process_ctrl_td(), looks a little strange:
/*
* Did we transfer any data, despite the errors that might have
* happened? I.e. did we get past the setup stage?
*/
if (event_trb != ep_ring->dequeue) {
/* The event was for the status stage */
if (event_trb == td->last_trb) {
if (td->urb->actual_length != 0) {
/* Don't overwrite a previously set error code
*/
if ((*status == -EINPROGRESS || *status == 0) &&
(td->urb->transfer_flags
& URB_SHORT_NOT_OK))
/* Did we already see a short data
* stage? */
*status = -EREMOTEIO;
If you already saw a short data stage, why isn't the status already set
to -EREMOTEIO?
Also what's the reason for the test "td->urb->actual_length != 0"?
What does this have to do with getting a short data stage? Are you
assuming that at this point, actual_length will be nonzero if and only
if there was a short data stage?
} else {
td->urb->actual_length =
td->urb->transfer_buffer_length;
}
What's the purpose of this clause?
It looks like the driver is confusing "actual_length == 0" with
"actual_length was never set". What if actual_length _was_ previously
set, but it was set to 0?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
