On Tue, Jul 30, 2024 at 5:45 PM Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> wrote: > > On 30.7.2024 3.17, Łukasz Bartosik wrote: > > On Mon, Jul 29, 2024 at 4:11 PM Mathias Nyman > > <mathias.nyman@xxxxxxxxxxxxxxx> wrote: > >> > >> Hi > >> > >> On 25.7.2024 10.48, Łukasz Bartosik wrote: > >>> DbC driver does not handle ClearFeature Halt requests correctly > >>> which in turn may lead to dropping packets on the receive path. > >> > >> Nice catch. > >> Looks like a halted endpoint is treated almost as a disconnect. > >> > > ... > Sorry I didn't get that ? > >> > >> Hmm, I need to dig into this. > >> > >> I don't think we should push this problem up to the request completion handler. > >> Maybe we are flushing requests that should not be flushed? > >> > > > > Section 7.6.4.3 "Halted DbC Endpoints" in Intel's xHCI specification > > says the endpoint can > > be halted by HW in case of error. Also it can be halted by software > > through HIT or HOT flags for DbC. > > I wonder how to recover properly from the Halted state caused by HW > > error ? Does it make sense to continue with > > the requests or just restart the endpoint (flush all requests) as this > > patch does ? > > DbC should respond with STALL packets to host if HIT or HOT is set. > Host side should react to this by sending a ClearFeature(HALT) request to DbC, > which should clear the halted endpoint and HIT/HOT flags. > Based on that I wonder when the DbC endpoint is not halted and it receives the ClearFeature(Halt) request whether this is correct behavior for the DbC endpoint to report a STALL error in such a case ? > I think we may lose more data in DbC bulk-out stall cases (data from DbC to host) > when flushing the requests. > Data is copied from kfifo to the requests while queuing them, if we then flush them > we will never send that data to host. > > > > >> Do you have an easy way to reproduce the stall error case? > >> > > > > Yes I can easily reproduce the case with the stall errors. > > Would you like me to run any specific scenarios ? > > I pushed my thoughts to a "fix_dbc_halted_ep" branch, it compiles but is not complete. > It mostly focuses on getting the STALL case for bulk-in working which this report was > about. > > I think the code itself best describes what I'm trying to do. > > git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git fix_dbc_halted_ep > https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=fix_dbc_halted_ep&id=8532b621314e93336535528d5d45e41974c75e01 > > If you can try it out it would be great. > Sure I will test your patch and get back with the result. Thanks, Lukasz > Thanks > -Mathias >
