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