Re: [PATCH] usb: musb: Check devctl status again for a spurious session request
From: Tony Lindgren <tony@atomide.com>
Date: 2021-06-04 10:08:26
Also in:
linux-omap
* Andreas Kemnade [off-list ref] [210604 10:00]:
quoted hunk ↗ jump to hunk
On Fri, 4 Jun 2021 12:39:54 +0300 Tony Lindgren [off-list ref] wrote:quoted
* Andreas Kemnade [off-list ref] [210604 08:35]:quoted
I inserted some more dev-dbg [ 60.241790] PM: suspend entry (deep) [ 60.245513] Filesystems sync: 0.000 seconds [ 60.251312] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 60.260040] OOM killer disabled. [ 60.263275] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 60.272338] printk: Suspending console(s) (use no_console_suspend to debug) [ 60.281311] musb-omap2430 480ab000.usb_otg_hs: omap2430 runtime_resume -> this is triggered by what?I think that comes from the pm_runtime_get_sync() in musb_suspend().@@ -2825,6 +2826,7 @@ static int musb_suspend(struct device *dev) struct musb *musb = dev_to_musb(dev); unsigned long flags; int ret; + dev_dbg(musb->controller, "musb_suspend begin\n"); ret = pm_runtime_get_sync(dev); if (ret < 0) {my dev_dbg comes before that.
Hmm maybe try adding dump_stack() call there to see where it comes from?
quoted
quoted
[ 60.281341] twl4030_usb 48070000.i2c:twl@48:twl4030-usb: twl4030_usb_runtime_resume -> and here something stays on... [ 60.346374] twl4030_usb 48070000.i2c:twl@48:twl4030-usb: twl4030_phy_power_on [ 60.796630] musb-hdrc musb-hdrc.0.auto: musb_suspend begin [ 60.796722] musb-hdrc musb-hdrc.0.auto: musb_suspend end [ 60.796752] musb-omap2430 480ab000.usb_otg_hs: omap2430 suspend [ 60.796783] musb-omap2430 480ab000.usb_otg_hs: omap2430 runtime_suspend [ 60.796783] twl4030_usb 48070000.i2c:twl@48:twl4030-usb: twl4030_phy_power_off [ 60.796813] twl4030_usb 48070000.i2c:twl@48:twl4030-usb: twl4030_usb_suspend [ 60.806549] Disabling non-boot CPUs ... [ 60.806579] Successfully put all powerdomains to target stateWell since commit 88d26136a256 ("PM: Prevent runtime suspend during system resume") nothing gets runtime idled during suspend with the extra pm_runtime_get_noresume() call in device_prepare() that does not get released until in device_complete().well, the result is that the phy stays powered on. And that is the problem. static int twl4030_phy_power_off(struct phy *phy) and static int __maybe_unused twl4030_usb_suspend(struct device *dev) do not turn off the phy. I think the solution is to turn the phy off if no cable is connected.
I recall there's some errata where the phy needs to be always enabled to avoid PMIC damage when a charger is connected. But if the phy wakes up the glue layer then that should be ignored for the glue layer maybe with .prepare and .complete suspend calls..
quoted
quoted
forcing omap2430 runtime on before suspend: [ 160.467742] musb-omap2430 480ab000.usb_otg_hs: omap2430 runtime_resume [ 165.001495] PM: suspend entry (deep) [ 165.005218] Filesystems sync: 0.000 seconds [ 165.010284] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 165.018981] OOM killer disabled. [ 165.022247] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 165.031311] printk: Suspending console(s) (use no_console_suspend to debug) [ 165.040496] musb-hdrc musb-hdrc.0.auto: musb_suspend begin [ 165.040618] musb-hdrc musb-hdrc.0.auto: musb_suspend end [ 165.040618] musb-omap2430 480ab000.usb_otg_hs: omap2430 suspend [ 165.040649] musb-omap2430 480ab000.usb_otg_hs: omap2430 runtime_suspend [ 165.040679] twl4030_usb 48070000.i2c:twl@48:twl4030-usb: twl4030_usb_suspend [ 165.050506] Disabling non-boot CPUs ... [ 165.050537] Successfully put all powerdomains to target stateThat's interesting. Hmm so we bail out early based on glue->is_runtime_suspended, and omap3 is still probing devices with omap_device.c instead of ti-sysc.c, so sounds like the duplicate calls you noticed might cause the issue. Does the following patch fix things for you or does something else break again? :)looks like something I have already tried, but I will give it a look again.
It seemed to behave for me based on a quick test against Linux next. I saw suspend power consumption go down, this with no cable connected and device in my test rack. But maybe your test is a bit different :) Regards, Tony