Re: [PATCH] usb: dwc3: Increase no-IOC ENDTRANSFER delay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: dwc3: unupdated 1 ms ENDTRANSFER delay in dwc3_remove_requests()
--

commit fee87ffa001e1c9181771b93176db403ea8ec8d4
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>

usb: dwc3: Increase no-IOC ENDTRANSFER delay

This commit increases the fixed delay after issuing an ENDTRANSFER command
without CMDIOC from 1 ms to 2 ms in dwc3_send_gadget_ep_cmd() to prevent
SMMU faults when unmapping requests during teardown.

> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index a76f669a796c2..a907fdfac8d71 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -445,7 +445,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
>  
>  	if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_ENDTRANSFER &&
>  	    !(cmd & DWC3_DEPCMD_CMDIOC))
> -		mdelay(1);
> +		mdelay(2);

[Severity: High]
Should the companion mdelay(1) in dwc3_remove_requests() also be increased
to 2 ms?

When __dwc3_gadget_ep_disable() tears down requests, commit 1338410c21a5
("usb: dwc3: gadget: Wait for End Transfer completion on ep disable")
changed it to pass interrupt = true:

drivers/usb/dwc3/gadget.c:__dwc3_gadget_ep_disable()
	dwc3_remove_requests(dwc, dep, -ESHUTDOWN, true);

With interrupt = true, __dwc3_stop_active_transfer() issues ENDTRANSFER
with DWC3_DEPCMD_CMDIOC set and sets DWC3_EP_END_TRANSFER_PENDING, so
dwc3_send_gadget_ep_cmd() skips the !(cmd & DWC3_DEPCMD_CMDIOC) delay
above. Similarly, if dwc3_remove_requests() is called while an earlier
CMDIOC ENDTRANSFER (such as from dwc3_gadget_ep_dequeue()) is already
pending, dwc3_stop_active_transfer() returns early when it sees
DWC3_EP_END_TRANSFER_PENDING.

In both paths, on controllers without DWC3_GUCTL2_RST_ACTBITLATER (such as
DWC_usb31), dwc3_remove_requests() relies on its own delay before
synchronously unmapping request buffers via dwc3_gadget_giveback():

drivers/usb/dwc3/gadget.c:dwc3_remove_requests() {
	dwc3_stop_active_transfer(dep, false, interrupt);
	...
	/* Give the controller time to stop accessing the request buffers */
	if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
		mdelay(1);

	/* - giveback all requests to gadget driver */
	while (!list_empty(&dep->started_list)) {
		req = next_request(&dep->started_list);

		dwc3_gadget_giveback(dep, req, status);
	}
	...
}

Leaving this delay at 1 ms means those teardown paths can still unmap DMA
buffers in started_list and cancelled_list before the controller finishes
processing ENDTRANSFER.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103182953.3437202-1-elson.serrao@oss.qualcomm.com?part=1