On 11/11/2011 04:36 AM, Sarah Sharp wrote: > On Thu, Nov 10, 2011 at 05:12:04PM +0800, Andiry Xu wrote: >> When trying to expand a ring, if enqueue pointer is behind dequeue pointer >> and they're in the same segment, we can not expand the ring until the >> dequeue pointer move pass the link TRB. This is because if the hardware >> has not cached the link TRB, it will jump to the new segments, but the >> data there is not right and the hardware may stop if the cycle_bit >> indicates it's software owned. >> >> Wait for the dequeue pointer move pass the link TRB, and then it's safe >> to expand the ring. Pending all the urbs submitted during the waiting period >> and queue them after the ring successfully expanded. When an endpoint is >> stopped, clear all the pending urbs. > > I'm not sure why you're giving back pending URBs when the endpoint > stops. There's no reason that canceling a URB that has made it onto > the ring should suddenly cancel all the URBs waiting to get on the ring. > Only the watchdog timer running should cause all the pending URBs to get > given back. > > What I wanted you to do was check in urb_dequeue whether the URB to be > canceled was in the pending queue and just take it out of the pending > queue. That you wouldn't have to even issue the stop endpoint command. > You're going to have to decrement the pending_num_trbs when you take it > out of the list, of course. > Oh, forgot the usb_dequeue part. Will modify it. > Also, why do you need both pending_num_trbs and waiting_deq_ptr? When > checking to see if the ring needs to be expanded, why not just check if > the TRB count is non-zero or if the pending_urb list is empty? > Yes, the waiting_deq_ptr can be removed. > I think there might be a couple other subtle issues, but I need to apply > your patchset and check. > Thanks, will submit again after your test. Thanks, Andiry -- 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
