Re: [v15 3/6] usb: dwc3: core: Host wake up support from system suspend

[Matthias Kaehlcke <mka@chromium.org>]

On Fri, May 06, 2022 at 02:18:06PM -0400, Alan Stern wrote:
> [CC: list drastically reduced]
> 
> On Fri, May 06, 2022 at 09:51:46AM -0700, Matthias Kaehlcke wrote:
> > I found this, as I commented on the other thread:
> > 
> >   commit c4a5153e87fdf6805f63ff57556260e2554155a5
> >   Author: Manu Gautam <mgautam@codeaurora.org>
> >   Date:   Thu Jan 18 16:54:30 2018 +0530
> > 
> >   usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode
> > 
> >   Commit 689bf72c6e0d ("usb: dwc3: Don't reinitialize core during
> >   host bus-suspend/resume") updated suspend/resume routines to not
> >   power_off and reinit PHYs/core for host mode.
> >   It broke platforms that rely on DWC3 core to power_off PHYs to
> >   enter low power state on system suspend.
> > 
> >   Perform dwc3_core_exit/init only during host mode system_suspend/
> >   resume to addresses power regression from above mentioned patch
> >   and also allow USB session to stay connected across
> >   runtime_suspend/resume in host mode. While at it also replace
> >   existing checks for HOST only dr_mode with current_dr_role to
> >   have similar core driver behavior for both Host-only and DRD+Host
> >   configurations.
> > 
> >   Fixes: 689bf72c6e0d ("usb: dwc3: Don't reinitialize core during host bus-suspend/resume")
> >   Reviewed-by: Roger Quadros <rogerq@ti.com>
> >   Signed-off-by: Manu Gautam <mgautam@codeaurora.org>
> >   Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> > 
> > 
> > So apparently powering off the core and PHYs is needed on some
> > platforms.
> > 
> > Let's move forward with the core/PHYs off for now and try to
> > come up with a solution (e.g. a DT property that indicates
> > that the core/PHYs can remain powererd) in a separate
> > patch/series.
> 
> Isn't it true that if you power off the PHY, the controller will be 
> unable to detect resume requests from attached devices? Similarly, won't 
> the controller will be unable to detect plug and unplug events on the 
> root hub?
> 
> Doesn't this mean that if wakeup is enabled on the root hub or any of 
> the devices downstream from a root-hub port, the port's PHY must remain 
> powered during suspend?

Yes.

Currently the core/PHYs are always powered off during suspend in host mode:

static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
{
	...

	switch (dwc->current_dr_role) {
	case DWC3_GCTL_PRTCAP_HOST:
		if (!PMSG_IS_AUTO(msg)) {
			dwc3_core_exit(dwc);
			break;
		}

	...
}

With that I would expect wakeup to be broken for all dwc3. I'm a bit confused
though, since dwc3-imx8mp.c seems to support wakeup and the driver landed
after the above patch ...

This series intends to change the above code to something like this:

	if (!PMSG_IS_AUTO(msg)) {
	       if (device_may_wakeup(dwc->dev) &&
	                       device_wakeup_path(dwc->dev)) {
	               dwc->phy_power_off = false;
	       } else {
	               dwc->phy_power_off = true;
	               dwc3_core_exit(dwc);
	       }
	}

i.e. the core/PHYs would only be powered down if wakeup is disabled or no
wakeup capable devices are connected. With that plug/unplug events still
wouldn't be detected.