On Tue, Apr 22, 2025, Kuen-Han Tsai wrote:
> On Tue, Apr 22, 2025 at 7:20 AM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
> >
> > On Mon, Apr 21, 2025, Kuen-Han Tsai wrote:
> > > On Sat, Apr 19, 2025 at 9:24 AM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
> > > >
> > > > On Wed, Apr 16, 2025, Kuen-Han Tsai wrote:
> >
> > <snip>
> >
> > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > > index 89a4dc8ebf94..630fd5f0ce97 100644
> > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > @@ -4776,26 +4776,22 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
> > > > > int ret;
> > > > >
> > > > > ret = dwc3_gadget_soft_disconnect(dwc);
> > > > > - if (ret)
> > > > > - goto err;
> > > > > -
> > > > > - spin_lock_irqsave(&dwc->lock, flags);
> > > > > - if (dwc->gadget_driver)
> > > > > - dwc3_disconnect_gadget(dwc);
> > > > > - spin_unlock_irqrestore(&dwc->lock, flags);
> > > > > -
> > > > > - return 0;
> > > > > -
> > > > > -err:
> > > > > /*
> > > > > * Attempt to reset the controller's state. Likely no
> > > > > * communication can be established until the host
> > > > > * performs a port reset.
> > > > > */
> > > > > - if (dwc->softconnect)
> > > > > + if (ret && dwc->softconnect) {
> > > > > dwc3_gadget_soft_connect(dwc);
> > > > > + return -EAGAIN;
> > > >
> > > > This may make sense to have -EAGAIN for runtime suspend. I supposed this
> > > > should be fine for system suspend since it doesn't do anything special
> > > > for this error code.
> > > >
> > > > When you tested runtime suspend, did you observe that the device
> > > > successfully going into suspend on retry?
> > >
> > > Hi Thinh,
> > >
> > > Yes, the dwc3 device can be suspended using either
> > > pm_runtime_suspend() or dwc3_gadget_pullup(), the latter of which
> > > ultimately invokes pm_runtime_put().
> > >
> > > One question: Do you know how to naturally cause a run stop failure?
> > > Based on the spec, the controller cannot halt until the event buffer
> > > becomes empty. If the driver doesn't acknowledge the events, this
> > > should lead to the run stop failure. However, since I cannot naturally
> > > reproduce this problem, I am simulating this scenario by modifying
> > > dwc3_gadget_run_stop() to return a timeout error directly.
> > >
> >
> > I'm not clear what you meant by "naturally" here. The driver is
> > implemented in such a way that this should not happen. If it does, we
> > need to take look to see what we missed.
> >
> > However, to force the driver to hit the controller halt timeout, just
> > wait/generate some events and don't clear the GEVNTCOUNT of event bytes
> > before clearing the run_stop bit.
> >
> > BR,
> > Thinh
>
> Hi Thinh,
>
> Thank you for getting back to me and the method to force the timeout!
>
> By "naturally," I meant reproducing the issue without artificial steps
Ok.
> designed solely to trigger it. You're right; since the driver is
> designed to prevent this state, reproducing it "naturally" is
> difficult.
>
> I really appreciate your patience, and thank you once more for the
> helpful clarification.
>
You're welcome.
BR,
Thinh
