On Fri, May 11, 2012 at 10:18:47AM -0400, Alan Stern wrote: > On Fri, 11 May 2012, Andiry Xu wrote: > > > > Okay. Why do you care about short packets on isochronous transfers? I > > > wouldn't think they make any difference. > > > > > > > If we do not set ISP for ISOC IN transfers, there is no event produced > > when a short packet occurs and the driver cannot update the status and > > length field of iso_packet_descriptor accordingly. > > You are begging the question. Why do you care if there is no event > produced when a short packet occurs? After all, there is no event when > a full-sized packet occurs; what's special about short packets? > > As for the iso_packet_descriptor status field... Surely that is > already being updated. Otherwise it would always contain -EINPROGRESS, > because that's what it gets set to when URBs are submitted. When we get an event for the last TD in an isochronous URB, we go through the frame list and mark any iso_packet_descriptor status field as successful that was between the last TD that generated an event and the last TD in the frame list. So if we don't get an event that tells us a short packet occurred on a TD somewhere in the middle of the frame list, we will mark it as successful, and the driver will consume random bytes. If we get a "Missed Service" completion code, meaning some TDs on the isochronous ring were skipped, we mark each TD with an error status until we get to a TD that successfully completed. > Much the same is true for the length field. In fact, you must really > mean the actual_length field, because the length field does not get > updated. Besides, doesn't Sarah's most recent patch set the > actual_length field correctly when a short packet occurs? The actual_length field in the event TRB is only for the last TD in the frame list. It is not an accumulation of all the lengths in the frame list. The xHC hardware has no idea where the frame list for an URB ends or begins. It only sees that one TRB in a number of TDs has the IOC flag set, and so it will only generate an event for that TD. So we need to know if anything bad happened (including a short packet) for the other TDs in the frame list. My patch only changes the completion status, not the actual_length field. It actually doesn't touch the hardware's event TRB at all, it just modifies a local copy of the completion status from the event TRB. Sarah Sharp -- 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
