On 22.8.2024 6.35, Łukasz Bartosik wrote:
On Mon, Aug 5, 2024 at 5:50 PM Mathias Nyman
<mathias.nyman@xxxxxxxxxxxxxxx> wrote:
On 5.8.2024 9.49, Mathias Nyman wrote:
On 4.8.2024 0.51, Łukasz Bartosik wrote:
On Thu, Aug 1, 2024 at 5:02 PM Łukasz Bartosik <ukaszb@xxxxxxxxxxxx> wrote:
[ 103.707882] xhci_hcd 0000:00:0d.0: Stall error at bulk TRB
1943ad000, remaining 1024, ep deq 1943ad001
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 ?
Ok, Setup was unclear to me, I assumed the STALL transfer error was due to an
actual transfer issue on a bulk endpoint.
I don't think xHCI DbC should send STALL error transfer events for non existing
bulk transfers as a response to ClearFeature(ENDPOINT_HALT) control request.
Especially as it seems not a single byte was transferred on either bulk endpoint.
But we need to handle it in the driver properly
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.
I noticed yes, this is why the patch compared the TRB pointed to by the
STALL transfer event with the one in the endpoint context.
If they match the patch attempts to turn that TRB to no-op, forcing xHC
hardware to move to the next TRB.
The check did however not work as it didn't mask out the cycle bit.
Lops show we compare 1943ad000 with 1943ad001
[ 103.707882] xhci_hcd 0000:00:0d.0: Stall error at bulk TRB 1943ad000, remaining 1024, ep deq 1943ad001
Code in patch:
+if (ep_ctx->deq == event->trans_event.buffer) { // FIXME do we need to worry about cycle bit?
+ dev_warn(dbc->dev, "Stall error TRB matches ep_ctx->deq, clear it\n");
+ trb_to_noop(req->trb);
With the new information that the STALL transfer event can be completely
spurious and not related to any actual bytes being transmitted on bulk
endpoints I think we shouldn't give back he transfer req in those cases.
I'll make another patch
New patch ready for testing:
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/commit/?h=fix_dbc_halted_ep&id=96cd909cc8115b3d2dff1bdcf265171bb0fdab18
Hi Mathias,
I finally tested your latest patch 96cd909cc8115b3d2dff1bdcf265171bb0fdab18.
It resolves the issue.
Thanks for testing.
I did some minor additional cleanups to it.
I'll post it as a proper patch. can I ask you to give it one last run,
just to make sure everything still works.
Thanks
Mathias
