From: Johan Hovold <johan@kernel.org>
To: Bin Liu <b-liu@ti.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-usb@vger.kernel.org, linux-omap@vger.kernel.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
Johan Hovold <johan@kernel.org>, stable <stable@vger.kernel.org>,
Alan Stern <stern@rowland.harvard.edu>,
Daniel Mack <zonque@gmail.com>, Dave Gerlach <d-gerlach@ti.com>,
"Rafael J . Wysocki" <rjw@rjwysocki.net>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Tony Lindgren <tony@atomide.com>
Subject: [PATCH] USB: musb: fix external abort on suspend
Date: Mon, 24 Jul 2017 11:49:39 +0200 [thread overview]
Message-ID: <20170724094939.21477-1-johan@kernel.org> (raw)
Make sure that the controller is runtime resumed when system suspending
to avoid an external abort when accessing the interrupt registers:
Unhandled fault: external abort on non-linefetch (0x1008) at 0xd025840a
...
[<c05481a4>] (musb_default_readb) from [<c0545abc>] (musb_disable_interrupts+0x84/0xa8)
[<c0545abc>] (musb_disable_interrupts) from [<c0546b08>] (musb_suspend+0x38/0xb8)
[<c0546b08>] (musb_suspend) from [<c04a57f8>] (platform_pm_suspend+0x3c/0x64)
This is easily reproduced on a BBB by enabling the peripheral port only
(as the host port may enable the shared clock) and keeping it
disconnected so that the controller is runtime suspended. (Well, you
would also need to the not-yet-merged am33xx-suspend patches by Dave
Gerlach to be able to suspend the BBB.)
This is a regression that was introduced by commit 1c4d0b4e1806 ("usb:
musb: Remove pm_runtime_set_irq_safe") which allowed the parent glue
device to runtime suspend and thereby exposed a couple of older issues:
Register accesses without explicitly making sure the controller is
runtime resumed during suspend was first introduced by commit
c338412b5ded ("usb: musb: unconditionally save and restore the context
on suspend") in 3.14.
Commit a1fc1920aaaa ("usb: musb: core: make sure musb is in RPM_ACTIVE on
resume") later started setting the RPM status to active during resume
without first making sure that the parent was runtime resumed. This was
also implicitly relying on the parent always being active. Since commit
71723f95463d ("PM / runtime: print error when activating a child to
unactive parent") this now also results in following warning:
musb-hdrc musb-hdrc.0: runtime PM trying to activate child device
musb-hdrc.0 but parent (47401400.usb) is not active
This patch has been verified on 4.13-rc2, 4.12 and 4.9 using a BBB
(the dsps glue would always be active also in 4.8).
Fixes: c338412b5ded ("usb: musb: unconditionally save and restore the context on suspend")
Fixes: a1fc1920aaaa ("usb: musb: core: make sure musb is in RPM_ACTIVE on resume")
Fixes: 1c4d0b4e1806 ("usb: musb: Remove pm_runtime_set_irq_safe")
Cc: stable <stable@vger.kernel.org> # 4.8
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Daniel Mack <zonque@gmail.com>
Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/usb/musb/musb_core.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 87cbd56cc761..b67692857daf 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2671,6 +2671,13 @@ static int musb_suspend(struct device *dev)
{
struct musb *musb = dev_to_musb(dev);
unsigned long flags;
+ int ret;
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(dev);
+ return ret;
+ }
musb_platform_disable(musb);
musb_disable_interrupts(musb);
@@ -2721,14 +2728,6 @@ static int musb_resume(struct device *dev)
if ((devctl & mask) != (musb->context.devctl & mask))
musb->port1_status = 0;
- /*
- * The USB HUB code expects the device to be in RPM_ACTIVE once it came
- * out of suspend
- */
- pm_runtime_disable(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
-
musb_start(musb);
spin_lock_irqsave(&musb->lock, flags);
@@ -2738,6 +2737,9 @@ static int musb_resume(struct device *dev)
error);
spin_unlock_irqrestore(&musb->lock, flags);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
return 0;
}
--
2.13.3
next reply other threads:[~2017-07-24 9:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-24 9:49 Johan Hovold [this message]
2017-07-24 14:38 ` [PATCH] USB: musb: fix external abort on suspend Alan Stern
2017-07-24 15:20 ` Johan Hovold
2017-07-24 17:13 ` Alan Stern
2017-07-25 7:09 ` Johan Hovold
2017-07-25 14:38 ` Alan Stern
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=20170724094939.21477-1-johan@kernel.org \
--to=johan@kernel.org \
--cc=b-liu@ti.com \
--cc=bigeasy@linutronix.de \
--cc=d-gerlach@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=stable@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
--cc=tony@atomide.com \
--cc=zonque@gmail.com \
/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).