* [RFC PATCH] usb: dwc3: Poll CMDACT after issuing EndXfer command
@ 2024-04-22 9:05 Prashanth K
2024-04-24 1:33 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Prashanth K @ 2024-04-22 9:05 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman
Cc: Wesley Cheng, linux-kernel, linux-usb, Prashanth K
Currently DWC3 controller revisions 3.10a and later supports
GUCTL[14: Rst_actbitlater] bit which allows polling CMDACT bit
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@quicinc.com>
---
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;
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
+ * the DWC3 controller (This is enabled by setting GUCTL2[14])
+ */
+ do {
+ reg = dwc3_readl(dep->regs, DWC3_DEPCMD);
+ if (!(reg & DWC3_DEPCMD_CMDACT))
+ break;
+ udelay(2);
+ } 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] usb: dwc3: Poll CMDACT after issuing EndXfer command
2024-04-22 9:05 [RFC PATCH] usb: dwc3: Poll CMDACT after issuing EndXfer command Prashanth K
@ 2024-04-24 1:33 ` Thinh Nguyen
2024-04-24 8:44 ` Prashanth K
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2024-04-24 1:33 UTC (permalink / raw)
To: Prashanth K
Cc: Thinh Nguyen, Greg Kroah-Hartman, Wesley Cheng,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
On Mon, Apr 22, 2024, Prashanth K wrote:
> 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@quicinc.com>
> ---
> 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).
> + * 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?
> + */
> + 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.
> + } 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 any
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.
Thanks,
Thinh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] usb: dwc3: Poll CMDACT after issuing EndXfer command
2024-04-24 1:33 ` Thinh Nguyen
@ 2024-04-24 8:44 ` Prashanth K
2024-04-24 22:36 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Prashanth K @ 2024-04-24 8:44 UTC (permalink / raw)
To: Thinh Nguyen
Cc: Greg Kroah-Hartman, Wesley Cheng, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
On 24-04-24 07:03 am, Thinh Nguyen wrote:
> On Mon, Apr 22, 2024, Prashanth K wrote:
>> 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@quicinc.com>
>> ---
>> 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).
>
Will update all of the above in next patch.
>> + * 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?
GUCTL2[14] is set only for DWC3_usb3 controllers prior to version 310a,
because the register is not available for other versions/revisions.
dwc3/core.c - dwc3_core_init()
/*
* ENDXFER polling is available on version 3.10a and later of
* the DWC_usb3 controller. It is NOT available in the
* DWC_usb31 controller.
*/
if (DWC3_VER_IS_WITHIN(DWC3, 310A, ANY)) {
reg = dwc3_readl(dwc->regs, DWC3_GUCTL2);
reg |= DWC3_GUCTL2_RST_ACTBITLATER;
dwc3_writel(dwc->regs, DWC3_GUCTL2, reg);
}
Hence the we have added the CMDACT polling in else case of the following
check in __dwc3_stop_active_transfer().
if (!DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC3, 310A))
>
>> + */
>> + 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.
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.
>
>> + } 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?
>
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.
90.872628: dbg_send_ep_cmd: ep1in: cmd 'End Transfer' [30c08] params
00000000 00000000 00000000 --> status: UNKNOWN
90.872652: dbg_ep_queue: ep1in: req 937bc75c length 0/1069 zsI ==> -108
From the above traces, we can we see that the time gap between
send_ep_cmd and dequeue/unmap is 24us, which is pretty small.
And immediately we got an smmu fault (since ENDXFER didn't complete in
this 24us time-frame). I dumped the DWC3 regs and saw that cmdact was
not cleared for ep1in which means the cmd never got completed.
90.873360: [DEPCMD(3): 0xc83c 0x00030C08]
--> CMDACT (bit10) still set.
> I would prefer a solution that applies for all IPs. Do you observe any
> 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.
>
I 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.
But currently for DWC3_usb3 >= 310a, we don't poll cmdact (even though
it supports it) nor we wait 1ms for command completion. For me waiting
1ms was also helping, but that doesn't guarantee the command completion
though (we just assume that it gets completed by that time).
* As of IP version 3.10a of the DWC_usb3 IP, the controller
* supports a mode to work around the above limitation. The
* software can poll the CMDACT bit in the DEPCMD register
* after issuing a EndTransfer command. This mode is enabled
* by writing GUCTL2[14]. This polling is already done in the
* dwc3_send_gadget_ep_cmd() function so if the mode is
* enabled, the EndTransfer command will have completed upon
* returning from this function.
*
* This mode is NOT available on the DWC_usb31 IP. In this
* case, if the IOC bit is not set, then delay by 1ms
* after issuing the EndTransfer command. This allows for the
* controller to handle the command completely before DWC3
* remove requests attempts to unmap USB request buffers.
But anyways I conducted a small experiment to calculate the ENDXFER cmd
completion, added traces before writing to DEPCMD and immediately polled
for the CMDACT to get cleared. Observed that it takes 10-15us on average
(checked on DWC3_REVISION_310A), but these are the best case scenarios.
191.623016: dwc3_writel: addr xx offset 000c value 00040c08 - write to
DEPCMD
191.623027: dwc3_readl: addr xx offset 000c value 00040808 -- CMACT cleared
191.623076: dwc3_gadget_ep_cmd: ep2out: cmd 'End Transfer' [40c08]
params 00000000 00000000 00000000 --> status: Successful
.
191.623425: dwc3_writel: addr xx offset 000c value 00070c08
191.623436: dwc3_readl: addr xx offset 000c value 00070808
191.623483: dwc3_gadget_ep_cmd: ep3in: cmd 'End Transfer' [70c08] params
00000000 00000000 00000000 --> status: Successful
Thanks,
Prashanth K
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] usb: dwc3: Poll CMDACT after issuing EndXfer command
2024-04-24 8:44 ` Prashanth K
@ 2024-04-24 22:36 ` Thinh Nguyen
2024-04-25 3:55 ` Prashanth K
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2024-04-24 22:36 UTC (permalink / raw)
To: Prashanth K
Cc: Thinh Nguyen, Greg Kroah-Hartman, Wesley Cheng,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Hi Prashanth,
On Wed, Apr 24, 2024, Prashanth K wrote:
> On 24-04-24 07:03 am, Thinh Nguyen wrote:
> > On Mon, Apr 22, 2024, Prashanth K wrote:
> > > + * 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?
> GUCTL2[14] is set only for DWC3_usb3 controllers prior to version 310a,
> because the register is not available for other versions/revisions.
>
> dwc3/core.c - dwc3_core_init()
> /*
> * ENDXFER polling is available on version 3.10a and later of
> * the DWC_usb3 controller. It is NOT available in the
> * DWC_usb31 controller.
> */
> if (DWC3_VER_IS_WITHIN(DWC3, 310A, ANY)) {
> reg = dwc3_readl(dwc->regs, DWC3_GUCTL2);
> reg |= DWC3_GUCTL2_RST_ACTBITLATER;
> dwc3_writel(dwc->regs, DWC3_GUCTL2, reg);
> }
>
> Hence the we have added the CMDACT polling in else case of the following
> check in __dwc3_stop_active_transfer().
>
> if (!DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC3, 310A))
>
> >
> > > + */
> > > + 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.
> 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.
> >
> > > + } 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?
> >
> 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.
>
> 90.872628: dbg_send_ep_cmd: ep1in: cmd 'End Transfer' [30c08] params
> 00000000 00000000 00000000 --> status: UNKNOWN
> 90.872652: dbg_ep_queue: ep1in: req 937bc75c length 0/1069 zsI ==> -108
>
> From the above traces, we can we see that the time gap between send_ep_cmd
> and dequeue/unmap is 24us, which is pretty small.
> And immediately we got an smmu fault (since ENDXFER didn't complete in this
> 24us time-frame). I dumped the DWC3 regs and saw that cmdact was not cleared
> for ep1in which means the cmd never got completed.
>
> 90.873360: [DEPCMD(3): 0xc83c 0x00030C08]
> --> CMDACT (bit10) still set.
>
> > I would prefer a solution that applies for all IPs. Do you observe any
> > 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.
> >
> I 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.
>
> But currently for DWC3_usb3 >= 310a, we don't poll cmdact (even though it
> supports it) nor we wait 1ms for command completion. For me waiting 1ms was
> also helping, but that doesn't guarantee the command completion though (we
> just assume that it gets completed by that time).
>
> * As of IP version 3.10a of the DWC_usb3 IP, the controller
> * supports a mode to work around the above limitation. The
> * software can poll the CMDACT bit in the DEPCMD register
> * after issuing a EndTransfer command. This mode is enabled
> * by writing GUCTL2[14]. This polling is already done in the
> * dwc3_send_gadget_ep_cmd() function so if the mode is
> * enabled, the EndTransfer command will have completed upon
> * returning from this function.
> *
> * This mode is NOT available on the DWC_usb31 IP. In this
> * case, if the IOC bit is not set, then delay by 1ms
> * after issuing the EndTransfer command. This allows for the
> * controller to handle the command completely before DWC3
> * remove requests attempts to unmap USB request buffers.
>
> But anyways I conducted a small experiment to calculate the ENDXFER cmd
> completion, added traces before writing to DEPCMD and immediately polled for
> the CMDACT to get cleared. Observed that it takes 10-15us on average
> (checked on DWC3_REVISION_310A), but these are the best case scenarios.
>
> 191.623016: dwc3_writel: addr xx offset 000c value 00040c08 - write to
> DEPCMD
> 191.623027: dwc3_readl: addr xx offset 000c value 00040808 -- CMACT cleared
> 191.623076: dwc3_gadget_ep_cmd: ep2out: cmd 'End Transfer' [40c08] params
> 00000000 00000000 00000000 --> status: Successful
> .
> 191.623425: dwc3_writel: addr xx offset 000c value 00070c08
> 191.623436: dwc3_readl: addr xx offset 000c value 00070808
> 191.623483: dwc3_gadget_ep_cmd: ep3in: cmd 'End Transfer' [70c08] params
> 00000000 00000000 00000000 --> status: Successful
>
Thanks for the data.
Ok, I remember now why we did what we did. I just notice the Fixes
commit you tag: b353eb6dc285 ("usb: dwc3: gadget: Skip waiting for
CMDACT cleared during endxfer")
I forgot that at one point we skip CMDACT for End Transfer command.
Let's not poll for CMDACT for End Transfer command and unconditionally
wait 1ms. Otherwise we may run into the issue being stuck with CMDACT
again while SETUP packet is not DMA out again. 1ms should be plenty of
time for the End Transfer command to complete.
It should look like this:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f94f68f1e7d2..dad30c6ab19d 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1724,8 +1724,7 @@ static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool int
dep->resource_index = 0;
if (!interrupt) {
- if (!DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC3, 310A))
- mdelay(1);
+ mdelay(1);
dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
} else if (!ret) {
dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
Thanks,
Thinh
Ps. also please Cc stable.
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] usb: dwc3: Poll CMDACT after issuing EndXfer command
2024-04-24 22:36 ` Thinh Nguyen
@ 2024-04-25 3:55 ` Prashanth K
0 siblings, 0 replies; 5+ messages in thread
From: Prashanth K @ 2024-04-25 3:55 UTC (permalink / raw)
To: Thinh Nguyen
Cc: Greg Kroah-Hartman, Wesley Cheng, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
On 25-04-24 04:06 am, Thinh Nguyen wrote:
>
> Thanks for the data.
>
> Ok, I remember now why we did what we did. I just notice the Fixes
> commit you tag: b353eb6dc285 ("usb: dwc3: gadget: Skip waiting for
> CMDACT cleared during endxfer")
>
> I forgot that at one point we skip CMDACT for End Transfer command.
> Let's not poll for CMDACT for End Transfer command and unconditionally
> wait 1ms. Otherwise we may run into the issue being stuck with CMDACT
> again while SETUP packet is not DMA out again. 1ms should be plenty of
> time for the End Transfer command to complete.
>
> It should look like this:
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index f94f68f1e7d2..dad30c6ab19d 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1724,8 +1724,7 @@ static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool int
> dep->resource_index = 0;
>
> if (!interrupt) {
> - if (!DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC3, 310A))
> - mdelay(1);
> + mdelay(1);
> dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
> } else if (!ret) {
> dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
>
> Thanks,
> Thinh
>
> Ps. also please Cc stable.
Thanks for the suggestion, I had tried 1ms initially when encountering
the issue and it was helping. Then I thought that CMDACT polling was
better approach. But anyways I echo you, having common solution for all
controller revisions is the cleaner way. I'll send V2 with mdelay(1).
Thanks again,
Prashanth K
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-25 3:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-22 9:05 [RFC PATCH] usb: dwc3: Poll CMDACT after issuing EndXfer command Prashanth K
2024-04-24 1:33 ` Thinh Nguyen
2024-04-24 8:44 ` Prashanth K
2024-04-24 22:36 ` Thinh Nguyen
2024-04-25 3:55 ` Prashanth K
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).