Hi, > > This used to get the empty list warning, but now it's mere > > xhci_dbg(). Throwing out all queued TDs is not the common case and > > it may easily be a bug. Indeed, I can readily name two cases when > > it is a bug today: > > > > 1. Force Stopped Event on a NOP or Link following the missed TD. > > Then trb_in_td() doesn't match subsequent TD and the rest is > > trashed. > > > > Actually, this is a v6.11 regression since d56b0b2ab1429. Although > > past behavior was bad and broken too, it was broken differently. > > > > 2. Ring Underrun/Overrun if new TDs were queued before we handled > > it. If ep_trb_dma is NULL, nothing ever matches and everything goes > > out. > > > > Arguably, these are rare events and I haven't observed them yet. > > And one more problem that I don't think currently exists, but: > > > > 3. If you ever find yourself doing it on an ordinary event (Success, > > Transaction Error, Babble, etc.) then, seriously, WTF? > > > > Bottom line, empty list is a very suspicious thing to see here. I > > can only think of two legitimate cases: > > > > 1. Ring X-run, only if nothing new was queued since it occurred. > > 2. FSE outside transfer TDs, if no transfer TDs existed after it. > > I can change it from a debug to a warning. Then the edge case should > be more visible. Actually, I'm no longer sure. If that ever happens, the user will next see "WARN list empty" or "ERROR TRB not part of current TD" and this will show there is a problem, so dyndbg will be enabled to investigate. I would still like to see more comp_codes printed in those messages, because on isoc there are some "weird" codes like Missed Service or Underrun, which have a very different meaning from the usual ones and are a source of bugs. But I would have to think about which cases it is useful for, particularly after this change. Lastly, I'm not sure if this change is worthwile. Over the weekend, I have produced a fairly simple but dramatic simplification of this whole code. The 'empty list' branch that your patch deals with can be completely removed, because it duplicates functionality of other code. The skipping loop is reduced down to 8 lines of code, as it should have always been in the first place. I will try to clean those patches up and send them out today. They received varying degree of testing (some are two weeks old) and have not caused any regression so far. They are simple and revieable IMO. Regards, Michal
