Re: [PATCH v2] xhci: dbc: fix handling ClearFeature Halt requests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 1, 2024 at 5:02 PM Łukasz Bartosik <ukaszb@xxxxxxxxxxxx> wrote:
>
> 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
>

Hi Mathias,

One more thing which I would like to add to the previous is the observation
which I made during debugging the issue. Looking at the above trace
there is stall
on IN endpoint:
xhci_dbc_handle_event: EVENT: TRB 00000001943ad000 status 'Stall
Error' len 1024 slot 1 ep 3 type 'Transfer Event' flags e:C

And then when 24 bytes packet arrives
xhci_dbc_handle_event: EVENT: TRB 00000001943ad000 status 'Short
Packet' len 1000 slot 1 ep 3 type 'Transfer Event' flags e:C

the same TRB is being used 00000001943ad000 but the DbC driver already
moved forward and the IN endpoint's pending list does not include a
dbc_request pointing
to the TRB 00000001943ad000 which results in "no matched request"
error and dropping
of the packet.

Thanks,
Lukasz

> > > Thanks
> > > Mathias
> > >





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux