| From 082df8be455ade361748f0385aa6c9c8d07be167 Mon Sep 17 00:00:00 2001 |
| From: Johan Hovold <johan@kernel.org> |
| Date: Thu, 24 Aug 2017 11:38:36 -0500 |
| Subject: USB: musb: fix external abort on suspend |
| |
| From: Johan Hovold <johan@kernel.org> |
| |
| commit 082df8be455ade361748f0385aa6c9c8d07be167 upstream. |
| |
| 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, |
| and 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 the 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: 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> |
| Signed-off-by: Bin Liu <b-liu@ti.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/usb/musb/musb_core.c | 18 ++++++++++-------- |
| 1 file changed, 10 insertions(+), 8 deletions(-) |
| |
| --- a/drivers/usb/musb/musb_core.c |
| +++ b/drivers/usb/musb/musb_core.c |
| @@ -2655,6 +2655,13 @@ static int musb_suspend(struct device *d |
| { |
| 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_generic_disable(musb); |
| @@ -2703,14 +2710,6 @@ static int musb_resume(struct device *de |
| 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); |
| @@ -2720,6 +2719,9 @@ static int musb_resume(struct device *de |
| error); |
| spin_unlock_irqrestore(&musb->lock, flags); |
| |
| + pm_runtime_mark_last_busy(dev); |
| + pm_runtime_put_autosuspend(dev); |
| + |
| return 0; |
| } |
| |