linux-kernel - [PATCH 3.16 021/133] ARM: OMAP2+: omap_device: drop broken RPM status update from suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1511315892.540315279@decadent.org.uk>
Date:   Wed, 22 Nov 2017 01:58:12 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, "Nishanth Menon" <nm@...com>,
        "Alan Stern" <stern@...land.harvard.edu>,
        "Ulf Hansson" <ulf.hansson@...aro.org>,
        "Dave Gerlach" <d-gerlach@...com>,
        "Tony Lindgren" <tony@...mide.com>,
        "Johan Hovold" <johan@...nel.org>,
        "Kevin Hilman" <khilman@...libre.com>,
        "Grygorii Strashko" <grygorii.strashko@...com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>
Subject: [PATCH 3.16 021/133] ARM: OMAP2+: omap_device: drop broken RPM
 status update from suspend_noirq

3.16.51-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Johan Hovold <johan@...nel.org>

commit 90de9634a5d57b92d8af4ec23aa2c9b297ec8168 upstream.

Since commit a8636c89648a ("PM / Runtime: Don't allow to suspend a
device with an active child"), which went into 4.10, it is no longer
permitted to set RPM_SUSPENDED state for a device with active children
(unless power.ignore_children is set).

This specifically means that the attempts to do just that from the omap
pm-domain suspend_noirq callback have since been failing whenever a
child is active, for example:

  am335x-usb-childs 47400000.usb: runtime PM trying to suspend
    device but active child

Silence this warning by dropping the broken pm_runtime_set_suspended()
call from the omap suspend_noirq callback along with the redundant
pm_runtime_set_active() in resume_noirq.

This effectively reverts commit 3522bf7bfa24 ("ARM: OMAP2+: omap_device:
maintain sane runtime pm status around suspend/resume"), which started
updating the RPM state after the runtime_suspend callback (!) for active
omap devices had been called during system suspend. The rationale was
that a later pm_runtime_get_sync() would then fail (even after runtime
pm had been disabled) and that this in turn would avoid any external
aborts when accessing registers with clocks disabled. (See also commit
6f3c77b040fc ("PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE,
even when disabled, v2").

But during the suspend_noirq phase all children would already have been
suspended and their drivers would specifically not attempt any further
register accesses. And if this was all just a workaround for random
device drivers doing cross-tree calls during system suspend, those
drivers should be fixed and updated to explicitly model such
dependencies using device-links instead (and either way, any such calls
have been causing crashes since 4.10).

Fixes: 3522bf7bfa24 ("ARM: OMAP2+: omap_device: maintain sane runtime pm status around suspend/resume")
Fixes: a8636c89648a ("PM / Runtime: Don't allow to suspend a device with an active child")
Cc: Alan Stern <stern@...land.harvard.edu>
Cc: Dave Gerlach <d-gerlach@...com>
Cc: Kevin Hilman <khilman@...libre.com>
Cc: Nishanth Menon <nm@...com>
Cc: Rafael J. Wysocki <rjw@...ysocki.net>
Cc: Tony Lindgren <tony@...mide.com>
Cc: Ulf Hansson <ulf.hansson@...aro.org>
Signed-off-by: Johan Hovold <johan@...nel.org>
Tested-by: Grygorii Strashko <grygorii.strashko@...com>
Signed-off-by: Tony Lindgren <tony@...mide.com>
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 arch/arm/mach-omap2/omap_device.c | 10 ----------
 1 file changed, 10 deletions(-)

--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -640,7 +640,6 @@ static int _od_suspend_noirq(struct devi

 	if (!ret && !pm_runtime_status_suspended(dev)) {
 		if (pm_generic_runtime_suspend(dev) == 0) {
-			pm_runtime_set_suspended(dev);
 			omap_device_idle(pdev);
 			od->flags |= OMAP_DEVICE_SUSPENDED;
 		}
@@ -657,15 +656,6 @@ static int _od_resume_noirq(struct devic
 	if (od->flags & OMAP_DEVICE_SUSPENDED) {
 		od->flags &= ~OMAP_DEVICE_SUSPENDED;
 		omap_device_enable(pdev);
-		/*
-		 * XXX: we run before core runtime pm has resumed itself. At
-		 * this point in time, we just restore the runtime pm state and
-		 * considering symmetric operations in resume, we donot expect
-		 * to fail. If we failed, something changed in core runtime_pm
-		 * framework OR some device driver messed things up, hence, WARN
-		 */
-		WARN(pm_runtime_set_active(dev),
-		     "Could not set %s runtime state active\n", dev_name(dev));
 		pm_generic_runtime_resume(dev);
 	}