On 1/20/2021 10:49 PM, Jung Daehwan wrote: > Hi, > > On Thu, Jan 21, 2021 at 01:00:32AM +0000, Thinh Nguyen wrote: >> Hi, >> >> Daehwan Jung wrote: >>> Sometimes dwc3_gadget_pullup and dwc3_gadget_set_speed are called after >>> entering suspend. That's why it needs to check whether suspend >>> >>> 1. dwc3 sends disconnect uevent and turn off. (suspend) >>> 2. Platform side causes pullup or set_speed(e.g., adbd closes ffs node) >>> 3. It causes unexpected behavior like ITMON error. >>> >>> Signed-off-by: Daehwan Jung <dh10.jung@xxxxxxxxxxx> >>> --- >>> drivers/usb/dwc3/gadget.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index ee44321..d7d4202 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -2093,6 +2093,9 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) >>> unsigned long flags; >>> int ret; >>> >>> + if (pm_runtime_suspended(dwc->dev)) >>> + return 0; >>> + >>> is_on = !!is_on; >>> >>> /* >>> @@ -2403,6 +2406,9 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g, >>> unsigned long flags; >>> u32 reg; >>> >>> + if (pm_runtime_suspended(dwc->dev)) >>> + return; >>> + >>> spin_lock_irqsave(&dwc->lock, flags); >>> reg = dwc3_readl(dwc->regs, DWC3_DCFG); >>> reg &= ~(DWC3_DCFG_SPEED_MASK); >> >> This is already addressed in Wesley Cheng's patches. Can you try the >> latest changes of DWC3 in Greg's usb-next branch? >> >> Thanks, >> Thinh > > I checked Wesly Cheng's pathces but it's not same. > What I want to do for this patch is to avoid pullup from platform side. > (android in my case) > > It's possible that platform side tries to pullup by UDC_Store after usb is already disconnected. > It can finally run controller and enable irq. > > I think we have to avoid it and other possible things related to platform side. > > Hi Daehwan, I think what you're trying to do is to avoid the unnecessary runtime resume if the cable is disconnected and userspace attempts to bind/unbind the UDC. I'm not exactly sure what patches you've pulled in, but assuming you didn't pull in any of the recent suspend changes: usb: dwc3: gadget: Allow runtime suspend if UDC unbinded usb: dwc3: gadget: Preserve UDC max speed setting Please consider the following scenario: 1. USB connected 2. UDC unbinded 3. DWC3 will continue to stay in runtime active, since dwc->connected = true In this scenario, we should allow the DWC3 to enter runtime suspend, since runstop has been disabled. If you have pulled in the above changes, and adding your changes on top of it, then consider the following: 1. USB connected 2. UDC unbinded 3. DWC enters runtime suspend (due to the above changes) 4. UDC binded The check for pm_runtime_suspended() will block step#4 from re-enabling the runstop bit even if the USB cable is connected. Thanks Wesley Cheng -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
