On Thu, 13 Nov 2014, Mathias Nyman wrote: > Currently we queue a reset endpoint command from the .endpoint_reset callback in host, this is far too late and should be moved > to when we get a STALL event. > > xhci needs to reset control endpoints on stall as well [1] > > I got a testpatch for this, but the more I look into how we handle reset endpoint for clearing halts, stop endpoint for urb dequeue, and reset device, > the more I notice that there are several other cases that needs fixing. testpatch for the halted ep is here: > > https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/commit/?h=ep_reset_halt_test&id=fe43d559e0816f65e5373e863a7da8062d311cd7 > > It's hard to see from patch diff itself what it does, but basically we call xhci_cleanup_halted_endpoint() in finish_td() if the transfer event status > is STALL, or a TX error that requires resetting the endpoint. That sounds right. You also need to ring the doorbell if the endpoint queue is non-empty. > There are still issues with setting the dequeue pointer correctly after stop or reset endpoint, I think this > is because we try to find the next TD based on a saved "stopped TD" value that might not valid anymore (i.e. a reset device in between reset endpoint and set dq pointer) > this issue is seen with DVB tuners when changing channels: I'm not aware of the details. Don't you always want to move to the start of the first TRB following the TD that got the error? The algorithm described in the DVB tuner bug is clearly wrong, since it doesn't move the dequeue pointer until usb_clear_halt() is called, which is far too late. The right approach is to fix up the dequeue pointer before giving back the URB (so there's no need to save a "stopped TD" value). Otherwise there will be TDs in the endpoint ring containing stale DMA pointers to buffers that have already been unmapped. 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
