Re: [PATCH 0/2] xhci: Fix the NEC stop bug workaround

["Michał Pecio" <michal.pecio@gmail.com>]

On Thu, 31 Oct 2024 16:22:14 +0200, Mathias Nyman wrote:
> Ok, good to know, then using flag is not enough.
> 
> Using a retry timeout for failed stop endpoint commands also sounds
> good to me.
> It has a slight downside of a possible 100ms aggressive 'Stop
> Endpoint' retry loop in cases where endpoint was stopped earlier for
> some other reason.

Waiting 100ms is rare. It happens in cases when the original bad
workaround would retry infinitely, and this bug still hasn't been
reported by users for half a year. But I triggered it with patched
drivers or by stopping a device with flaky cable, so it can happen.

EP_RESTARTING flag could be used to avoid wasting 100ms in those
cases, but I found that I can predict its value in advance while
queuing the command, without adding noise to unrelated code. That
was the STOP_CMD_REDUNDANT patch, and now I'm trying to just avoid
queuing such redundant commands at all. And timeout if that fails.

I found this "redundant" prediction very accurate and pointless
commands are practically eliminated. Correctness is guaranteed by
ring_ep_doorbell() always starting an endpoint with pending URBs
if a few ep_state flags aren't set, and always being called when
any of those flags are cleared. Only two exceptions are known:

1. The "transferless tx events", which may trigger a hard reset
   without Set Deq. In this case ring_ep_doorbell() isn't called.
2. The bizarre ASM3142 ifconfig up/down issue, which crashes the
   whole bus and really looks like a hardware bug.

Generally, all cases of failing to restart an active endpoint are
very user-visible and problematic on their own right, so a bit of
extra delay wouldn't be the worst problem at this point, I hope.

> • When software rings the Doorbell of an endpoint to transition it
> from the Stopped to Running state, it should update its image of EP
> State to Running.

Making this assumption is exactly why we have problems, because the
start/stop race is tricky for hardware and some chips clearly don't
behave in such simple manner as suggested.

Regards,
Michal