On Fri, 4 Jun 2021 13:08:20 +0300
Tony Lindgren [off-list ref] wrote:

* Andreas Kemnade [off-list ref] [210604 10:00]:

quoted

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

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 state    

Well 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..

yes, phy needs to be enabled when a charger is connected. That was the
reason why we do not use phy_power_off() for turning it off. But if
no charger is connected, the phy does not need to be turned on.

I will test your patch at some time in the afternoon.

Regards,
Andreas