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

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

On Thu, 31 Oct 2024 09:13:47 +0100, Michał Pecio wrote:
> This looks like it should work on semi-well-behaved HC like NEC, but
> it doesn't account for hardware not restarting an EP "just because".
> 
> 
> while true; do ifconfig eth0 up; ifconfig eth0 down; done
> 
> locks up on ASM3142 with AX88179 adapter as expected, and when the NIC
> is disconnected I get those 'ep ctx error, ep still running' every few
> seconds. It seems lots of network code got locked up and I can't even
> ssh into the box anymore to copy exact dmesg output.

I apologize for rushed testing and providing bad information.

Your patch doesn't trigger infinite retries because it gives up after
just one retry. The infinite retries every few seconds I observed were
all caused by separate cancellations. The class driver timed out on its
control transfer, cancelled one URB, tried another one, timed out, ...

It takes a few minutes before it gives up, and only if I kill the
ifconfig loop, otherwise it seems to be forever.

Your patch prints one dev_dbg() each time, mine spams many of them for
100ms each time. I will remove this one retry limit from your patch to
see if starts spinning infinitely, but I strongly suspect it will.


One retry is not enough. This is what I got on the first try with a
random UVC webcam:

[ 7297.326596] usb 10-2: new high-speed USB device number 2 using xhci_hcd
[ 7297.465252] usb 10-2: New USB device found, idVendor=1e4e, idProduct=0103, bcdDevice= 0.02
[ 7297.465259] usb 10-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[ 7297.465261] usb 10-2: Product: USB2.0 Camera
[ 7297.465263] usb 10-2: Manufacturer: Etron Technology, Inc.
[ 7297.468898] usb 10-2: Found UVC 1.00 device USB2.0 Camera (1e4e:0103)
[ 7297.475995] usb 10-2: UVC non compliance - GET_DEF(PROBE) not supported. Enabling workaround.
[ 7297.476928] input: USB2.0 Camera: USB2.0 Camera as /devices/pci0000:00/0000:00:05.0/0000:03:00.0/usb10/10-2/10-2:1.0/input/input25
[ 7297.492153] usb 10-2: Warning! Unlikely big volume range (=6144), cval->res is probably wrong.
[ 7297.492159] usb 10-2: [3] FU [Mic Capture Volume] ch = 1, val = 5120/11264/1
[ 7299.301892] usb 10-2: Device requested 3060 B/frame bandwidth
[ 7299.301907] usb 10-2: Selecting alternate setting 12 (3060 B/frame bandwidth)
[ 7299.304772] usb 10-2: Allocated 5 URB buffers of 32x3060 bytes each
[ 7300.339246] xhci_hcd 0000:03:00.0: Stop ep ctx error, already stopped with pending start
[ 7300.339252] xhci_hcd 0000:03:00.0: Stop ep completion ctx error, ep is running
[ 7300.339283] xhci_hcd 0000:03:00.0: Context Error unhandled, ctx_state 3
[ 7300.339324] xhci_hcd 0000:03:00.0: Stop ep ctx error, already stopped with pending start
[ 7300.339326] xhci_hcd 0000:03:00.0: Stop ep completion ctx error, ep is running
[ 7300.339365] xhci_hcd 0000:03:00.0: Stop ep completion ctx error, ep is running
[ 7300.339492] xhci_hcd 0000:03:00.0: Stop ep ctx error, already stopped with pending start
[ 7300.339494] xhci_hcd 0000:03:00.0: Stop ep completion ctx error, ep is running
[ 7300.339533] xhci_hcd 0000:03:00.0: Context Error unhandled, ctx_state 3
[ 7300.339593] xhci_hcd 0000:03:00.0: Mismatch between completed Set TR Deq Ptr command & xHCI internal state.
[ 7300.339594] xhci_hcd 0000:03:00.0: ep deq seg = ffff88810b006000, deq ptr = ffff88810ae0e780
[ 7300.339634] xhci_hcd 0000:03:00.0: Stop ep completion ctx error, ep is running

Not sure what caused this mismatch, it might be a new bug of yours
because I don't recall seeing such things before.

This was with your three patches on v6.12-rc4 plus the following:

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7d036fda354c..7325729beac8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1182,6 +1182,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
 		default:
 			break;
 		}
+		xhci_err(xhci, "Context Error unhandled, ctx_state %d\n",
+				GET_EP_CTX_STATE(ep_ctx));
 	}
 
 	/* will queue a set TR deq if stopped on a cancelled, uncleared TD */