Hi, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: >>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: >>>> If a request is dequeued, the transfer is cancelled. Give back all >>>> the started requests. >>>> >>>> In most scenarios, the function driver dequeues all requests of a >>>> transfer when there's a failure. If the function driver follows this, >>>> then it's fine. If not, then we'd be skipping TRBs at different points >>>> within the dequeue and enqueue pointers, making dequeue/enqueue pointers >>>> useless. To enforce and make sure that we're properly skipping TRBs, >>>> cancel all the started requests and give back all the cancelled requests >>>> to the function drivers. >>> Which function driver is *not* cancelling transfers correctly? We can >>> (and should) be defensive on dwc3, but let's not hide bugs on function >>> drivers either. >>> >> I didn't review all the function drivers for this. I just see a >> potential issue and go for a more defensive approach. What's your >> suggestion? > Fair enough, that's good for my understading of why the patch was > created. Is there a way to add a WARN() or something like that so we > catch erroneous gadget drivers easily? Also, could you check if we need > a documentation update for the gadget API with regards to this finding? > > cheers > It's a bit difficult to add a WARN() as the function drivers can dequeue the requests in any order and any time they like. We may be able to put some contraints via documentation so we could do a WARN(), but that is also not easy. For example, in dwc3, the current way we implement bulk transfer is that there's no concept of where one transfer starts and ends (at least for non-stream transfers). So, the function driver may queue multiple transfers but dwc3 only sees a single transfer. Dequeuing one transfer in the function driver means dequeuing all for dwc3. Updating the documentation for this requires a bit of thoughts, and it also means we need to review all the different controller drivers. We may need to document this, but in the mean time, what do you think of this approach for now? BR, Thinh
