From: Tony Lindgren <tony@atomide.com>
To: Andreas Kemnade <andreas@kemnade.info>
Cc: Bin Liu <b-liu@ti.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-usb@vger.kernel.org, linux-omap@vger.kernel.org,
letux-kernel@openphoenux.org
Subject: Re: [PATCH] usb: musb: Check devctl status again for a spurious session request
Date: Fri, 4 Jun 2021 12:39:54 +0300 [thread overview]
Message-ID: <YLn06uuntThMlaTQ@atomide.com> (raw)
In-Reply-To: <20210604103533.6392beeb@aktux>
* Andreas Kemnade <andreas@kemnade.info> [210604 08:35]:
> 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().
> [ 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().
> 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 state
That'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? :)
Regards,
Tony
8< --------------
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -332,6 +332,7 @@ static int omap2430_probe(struct platform_device *pdev)
glue->musb = musb;
glue->status = MUSB_UNKNOWN;
glue->control_otghs = ERR_PTR(-ENODEV);
+ glue->is_runtime_suspended = 1;
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
@@ -453,6 +454,9 @@ static int omap2430_runtime_suspend(struct device *dev)
if (!musb)
return 0;
+ if (glue->is_runtime_suspended)
+ return 0;
+
musb->context.otg_interfsel = musb_readl(musb->mregs,
OTG_INTERFSEL);
@@ -474,6 +478,9 @@ static int omap2430_runtime_resume(struct device *dev)
if (!musb)
return 0;
+ if (!glue->is_runtime_suspended)
+ return 0;
+
phy_init(musb->phy);
phy_power_on(musb->phy);
--
2.31.1
next prev parent reply other threads:[~2021-06-04 9:39 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-18 15:06 [PATCH] usb: musb: Check devctl status again for a spurious session request Tony Lindgren
2021-05-27 19:15 ` Andreas Kemnade
2021-05-28 5:57 ` Tony Lindgren
2021-05-28 9:37 ` Andreas Kemnade
2021-06-02 5:41 ` Tony Lindgren
2021-06-03 21:22 ` Andreas Kemnade
2021-06-04 8:35 ` Andreas Kemnade
2021-06-04 9:39 ` Tony Lindgren [this message]
2021-06-04 9:59 ` Andreas Kemnade
2021-06-04 10:08 ` Tony Lindgren
2021-06-04 10:20 ` Andreas Kemnade
2021-06-04 14:45 ` Andreas Kemnade
2021-06-04 16:59 ` Andreas Kemnade
2021-06-05 5:18 ` Tony Lindgren
2021-06-05 14:20 ` Andreas Kemnade
2021-06-06 6:01 ` Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YLn06uuntThMlaTQ@atomide.com \
--to=tony@atomide.com \
--cc=andreas@kemnade.info \
--cc=b-liu@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=letux-kernel@openphoenux.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).