On Wed, Jul 31, 2024 at 3:42 PM Łukasz Bartosik <ukaszb@xxxxxxxxxxxx> wrote: > > On Wed, Jul 31, 2024 at 2:47 PM Mathias Nyman > <mathias.nyman@xxxxxxxxxxxxxxx> wrote: > > > > On 31.7.2024 15.28, Łukasz Bartosik wrote: > > > 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. > > >>>> > > >> > > >> > > >> 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, that patch was missing a "ctrl = readl(&dbc->regs->control);" line > > > > It's now fixed here: > > https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=fix_dbc_halted_ep&id=cf99b473a1477c1b3510af0021877197a039c43f > > > > Can you try that instead > > > > I will. Thanks for the update. > > Łukasz > Hi Matthias, I ran the test with your fix cf99b473a1477c1b3510af0021877197a039c43f however the issue still exists. Trace capture: # tracer: nop # # entries-in-buffer/entries-written: 12/12 #P:12 # # _-----=> irqs-off/BH-disabled # / _----=> need-resched # | / _---=> hardirq/softirq # || / _--=> preempt-depth # ||| / _-=> migrate-disable # |||| / delay # TASK-PID CPU# ||||| TIMESTAMP FUNCTION # | | | ||||| | | kworker/7:1-418 [007] d..1. 103.737275: xhci_dbc_handle_event: EVENT: TRB 00000001943ae000 status 'Stall Error' len 0 slot 1 ep 2 type 'Transfer Event' flags e:C kworker/7:1-418 [007] d..1. 103.742975: xhci_dbc_handle_event: EVENT: TRB 00000001943ad000 status 'Stall Error' len 1024 slot 1 ep 3 type 'Transfer Event' flags e:C kworker/7:1-418 [007] d..1. 103.742976: xhci_dbc_handle_transfer: BULK: Buffer 000000017988e400 length 1024 TD size 0 intr 0 type 'Normal' flags b:i:I:c:s:i:e:C kworker/7:1-418 [007] d..1. 103.753393: xhci_dbc_giveback_request: bulk-in: req 00000000f2919fbe length 0/1024 ==> 0 kworker/7:1-418 [007] dNs2. 103.753407: xhci_dbc_gadget_ep_queue: BULK: Buffer 000000017988e400 length 1024 TD size 0 intr 0 type 'Normal' flags b:i:I:c:s:i:e:c kworker/7:1-418 [007] dNs1. 103.753409: xhci_dbc_queue_request: bulk-in: req 00000000f2919fbe length 0/1024 ==> -115 kworker/7:1-418 [007] d..1. 103.767607: xhci_dbc_handle_event: EVENT: TRB 00000001943ad000 status 'Short Packet' len 1000 slot 1 ep 3 type 'Transfer Event' flags e:C kworker/7:1-418 [007] d..1. 103.773260: xhci_dbc_handle_event: EVENT: TRB 00000001943ad010 status 'Short Packet' len 763 slot 1 ep 3 type 'Transfer Event' flags e:C kworker/7:1-418 [007] d..1. 103.773260: xhci_dbc_handle_transfer: BULK: Buffer 000000017988e800 length 1024 TD size 0 intr 0 type 'Normal' flags b:i:I:c:s:i:e:C kworker/7:1-418 [007] d..1. 103.773261: xhci_dbc_giveback_request: bulk-in: req 00000000eb22d5f2 length 261/1024 ==> 0 kworker/7:1-418 [007] dNs2. 103.773281: xhci_dbc_gadget_ep_queue: BULK: Buffer 000000017988e800 length 1024 TD size 0 intr 0 type 'Normal' flags b:i:I:c:s:i:e:c kworker/7:1-418 [007] dNs1. 103.773283: xhci_dbc_queue_request: bulk-in: req 00000000eb22d5f2 length 0/1024 ==> -115 A 24 bytes packet is dropped which is confirmed in kernel logs by "no matched request" entries. Kernel logs related to DbC: [ 55.569943] xhci_hcd 0000:00:0d.0: DbC connected [ 55.861946] xhci_hcd 0000:00:0d.0: DbC configured [ 103.702190] xhci_hcd 0000:00:0d.0: no matched request [ 103.707882] xhci_hcd 0000:00:0d.0: Stall error at bulk TRB 1943ad000, remaining 1024, ep deq 1943ad001 [ 103.726161] xhci_hcd 0000:00:0d.0: DbC Endpoint halt cleared [ 103.732517] xhci_hcd 0000:00:0d.0: no matched request > 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. > I would like to ask you about it again because 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 ? Thanks, Lukasz > > Thanks > > Mathias > >
