On Mon, Apr 22, 2024, Prashanth K wrote:Will update all of the above in next patch.
Currently DWC3 controller revisions 3.10a and later supports
DWC3 -> DWC_usb3 to highlight not being DWC_usb31 and DWC_usb32.
GUCTL[14: Rst_actbitlater] bit which allows polling CMDACT bit
GUCTL -> GUCTL2
to know whether ENDXFER command is completed. Other revisions
wait 1ms for ENDXFER completion after issuing the command.
Consider a case where an IN request was queued, and parallelly
soft_disconnect was called (due to ffs_epfile_release). This
eventually calls stop_active_transfer with IOC cleared, hence
send_gadget_ep_cmd() skips waiting for CMDACT cleared during
endxfer. For DWC3 controllers with revisions >= 310a, we don't
forcefully wait for 1ms either, and we proceed by unmapping the
requests. If ENDXFER didn't complete by this time, it leads to
SMMU faults since the controller would still be accessing those
requests.
DWC3 databook specifies that CMDACT bit can be polled to check
completion of the EndXfer. Hence use it in stop_active_transfer
to know whether the ENDXFER got completed.
Section 3.2.2.7 Command 8: End Transfer (DEPENDXFER)
Note: If GUCTL2[Rst_actbitlater] is set, Software can poll the
completion of the End Transfer command by polling the command
active bit to be cleared to 0.
Fixes: b353eb6dc285 ("usb: dwc3: gadget: Skip waiting for CMDACT cleared during endxfer")
Signed-off-by: Prashanth K <quic_prashk@xxxxxxxxxxx>
---
drivers/usb/dwc3/gadget.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 4df2661f6675..acb54c48451f 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1701,8 +1701,8 @@ static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool int
{
struct dwc3 *dwc = dep->dwc;
struct dwc3_gadget_ep_cmd_params params;
- u32 cmd;
- int ret;
+ u32 cmd, reg;
+ int ret, retries = 500;
Please separate variable declarations per line.
cmd = DWC3_DEPCMD_ENDTRANSFER;
cmd |= force ? DWC3_DEPCMD_HIPRI_FORCERM : 0;
@@ -1726,6 +1726,24 @@ static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool int
if (!interrupt) {
if (!DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC3, 310A))
mdelay(1);
+ else {
+ /*
+ * ENDXFER polling is available on version 3.10a and later of
Try to note the IP along with version (eg. DWC_usb3 v3.10a).
GUCTL2[14] is set only for DWC3_usb3 controllers prior to version 310a, because the register is not available for other versions/revisions.+ * the DWC3 controller (This is enabled by setting GUCTL2[14])
Did we check to know that we set GUCTL2.Rst_actbitlater to start
polling for CMDACT?
Agreed, Will make it 200us with 5 retries. I'm not really sure how many retries we need here. I just made the aggregate to 1ms since we use 1ms delay for other revisions.
+ */
+ do {
+ reg = dwc3_readl(dep->regs, DWC3_DEPCMD);
+ if (!(reg & DWC3_DEPCMD_CMDACT))
+ break;
+ udelay(2);
udelay of 2 is really small. Try at least 200us.
The problem here is that we are immediately unmapping the request after issuing the ENDXER, we aren't waiting for the completion unlike other commands.
+ } while (--retries);
+
+ if (!retries && (dwc->ep0state != EP0_SETUP_PHASE)) {
+ dep->flags |= DWC3_EP_DELAY_STOP;
+ return -ETIMEDOUT;
+ }
+ }
+
dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
} else if (!ret) {
dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
--
2.25.1
Did you observe issues with DWC_usb31? How much longer did your setup
need to complete End Transfer command?
I would prefer a solution that applies for all IPs. Do you observe anyI checked the following comments in dwc3_stop_active_transfer(), since we don't use IOC for ENDXFER, we are use CMDACT polling on revisions which supports it, other revisions uses delay of 1ms instead.
impact should we increase the mdelay()? I don't expect much impact since
this should only happen during endpoint disbling, which is not a common
operation.