Re: [PATCH] xhci: fix reporting of 0-sized URBs in control endpoint

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 21 Feb 2015, Aleksander Morgado wrote:

> When the control TD doesn't have TRBs in the data stage, the URB actual length
> is set equal to the transfer buffer length. E.g. with a 64 byte transfer buffer
> length:
> 
>   Transfer event len = 0, COMP_SHORT_SUCCESS  (status)
>    |---------> URB actual length set to transfer buffer length = 64

How can a control TD have a transfer buffer length that is > 0 and also 
have no TRBs in the data stage?  That doesn't make sense.

> When the control TD has TRBs in the data stage, the URB actual length is
> computed based on the transfer event length reported in the data stage TRB. In
> this case, the URB actual length won't be modified in the status stage. E.g.
> again with a 64 byte transfer buffer length:
> 
>   Transfer event len = 63, COMP_SHORT_TX (data)
>    |---------> URB actual length = 64 - 63 = 1
>   Transfer event len = 0, COMP_SHORT_SUCCESS (status)
>    |---------> URB actual length not modified = 1
> 
> The current logic, though, doesn't seem to contemplate the case where a TD has a
> TDR in the data stage which actually reports 0 bytes (i.e. transfer event len
> equal to transfer buffer len). The logic is currently handling this case in the
> following way:
> 
>   Transfer event len = 64, COMP_SHORT_TX (data)
>    |---------> URB actual length = 64 - 64 = 0
>   Transfer event len = 0, COMP_SHORT_SUCCESS (status)
>    |---------> URB actual length set to transfer buffer length = 64   <------
> 
> In this case, the logic shouldn't update the URB actual length during the status
> stage; instead it should leave the URB actual length that was computed during
> the data stage, even if it's 0:
> 
>   Transfer event len = 63, COMP_SHORT_TX (data)
>    |---------> URB actual length = 64 - 64 = 0
>   Transfer event len = 0, COMP_SHORT_SUCCESS (status)
>    |---------> URB actual length not modified = 0                     <------

In fact, is there _ever_ any reason for changing the actual length 
during the status stage?  It seems to me that by the end of the data 
stage, we already know the actual length.

> The proposed fix re-uses the 'last_td_was_short' flag in the endpoint ring. The
> setting of this flag is moved to after the actual TRB processing, so that the
> flag value indicating whether the last TRB was short can be read during the
> processing. The control TDR processing will use this flag to determine whether
> it has to reset the URB actual length or instead leave it as it was already
> precomputed by the previous short TRB.

Why not always leave the actual length as is?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux