On 07/29/2014 03:38 AM, Julius Werner wrote: > I don't think this works. As I understand it, ep_ring->cycle_state > contains the current cycle state at the enqueue pointer, not the > dequeue pointer (it gets updated in inc_enq() but not in inc_deq() for > transfer rings). That's the only reason we need to pull it out of the > Endpoint Context at all... because we have long since overwritten our > own software copy, when we originally enqueued the TD (and an > arbitrary amount of further ones after it). > > I think if we really want to play it safe, any solution for this must > keep track of and only stop searching after both passing last_trb and > reaching hw_dequeue (taking the cycle_state off the latter), since > there might be cases where either of them could be more than one TRB > after the other. Maybe if you add another last_found variable and > depend the loop on both you could implement that in a clear way? > (Also, I'm wondering if we should just drop the code that skips whole > segments to make it simpler, since I can hardly think of any > real-world examples where a single TD would cover a whole segment.) > Your right. ep_ring->cycle_state is not a valid starting cycle bit for the dequeue poineter cycle. Dropping the whole segment jump sounds good, now thinking about it, it also caused a possibility to loop forever. I'll do a new version, and I'll try to think it through a bit more. btw the last version already proved to fix one instance of this bug: https://bugzilla.kernel.org/show_bug.cgi?id=75521 (tested by Evan Langlois) -Mathias -- 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
