Hi Thinh,
On 5/23/2022 4:22 PM, Thinh Nguyen wrote:
Hi,
Pavan Kondeti wrote:
On Thu, Apr 21, 2022 at 07:22:50PM -0700, Thinh Nguyen wrote:
Since we can't guarantee that the host won't send new Setup packet
before going through the device-initiated disconnect, don't prepare
beyond the Setup stage and keep the device in EP0_SETUP_PHASE. This
ensures that the device-initated disconnect sequence can go through
gracefully. Note that the controller won't service the End Transfer
command if it can't DMA out the Setup packet.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
---
drivers/usb/dwc3/ep0.c | 2 +-
drivers/usb/dwc3/gadget.c | 29 +++++++++++++++++------------
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 1064be5518f6..c47c696316dd 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -813,7 +813,7 @@ static void dwc3_ep0_inspect_setup(struct dwc3 *dwc,
int ret = -EINVAL;
u32 len;
- if (!dwc->gadget_driver)
+ if (!dwc->gadget_driver || !dwc->connected)
goto out;
trace_dwc3_ctrl_req(ctrl);
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a86225dbaa2c..e5f07c0e8ad9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2505,6 +2505,23 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc)
spin_lock_irqsave(&dwc->lock, flags);
dwc->connected = false;
+ /*
+ * Per databook, when we want to stop the gadget, if a control transfer
+ * is still in process, complete it and get the core into setup phase.
+ */
+ if (dwc->ep0state != EP0_SETUP_PHASE) {
+ int ret;
+
+ reinit_completion(&dwc->ep0_in_setup);
+
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ ret = wait_for_completion_timeout(&dwc->ep0_in_setup,
+ msecs_to_jiffies(DWC3_PULL_UP_TIMEOUT));
+ spin_lock_irqsave(&dwc->lock, flags);
+ if (ret == 0)
+ dev_warn(dwc->dev, "timed out waiting for SETUP phase\n");
DWC3_PULL_UP_TIMEOUT is 500 msec. If the ongoing control transfer is delayed
(dwc3::delayed_status), for whatever reason, would there be a problem?
Sorry for the delayed response. I was away.
If the control transfer takes longer than 500ms, then we'd get this
timed out warning. However, it should be fine because
1) If the function driver hasn't sent the status, then the host won't be
sending a new SETUP packet.
2) If the delayed status was sent and completed immediately after the
timeout but before the dwc3_gadget_soft_disconnect holding the
spin_lock, then we may see End Transfer command timeout. It may not look
like the cleanup was done gracefully, but that should be fine. The
command should be able to complete once the spin_lock is released and
Setup packet handled. The controller should halt within the polling period.
3) If the host misbehaves and ignores the status stage/abort the control
transfer to send a new setup packet, I don't think the current dwc3
driver handles that case properly. But that should be for a separate
patch fix.
In the trace that I sent you where the controller halt fails, it is due
to the above condition that Pavan mentioned. We're in a situation where
if the function driver dequeues an USB request, and we are not in the
proper ep0state to handle, we'll set the DWC3_EP_DELAY_STOP flag.
Soon after, if a soft disconnect occurs, and we're in a situation where
delayed_status == 1, then most likely, we'll see the SETUP packet
timeout (depending on when the function queues the status phase) and
proceed w/ stop active xfers and gadget. Since we do not wait for the
delayed stop condition to be handled before attempting to halt the
controller, we'll run into a timeout when clearing Run/Stop. In this
situation, this is why you don't see the endxfer command being send for
endpoints.
Thanks
Wesley Cheng
