linux-kernel - [PATCH] RFT: HID: i2c-hid: Fix suspend/resume when already runtime suspended

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>] [day] [month] [year] [list]

Message-Id: <1425685260-30128-1-git-send-email-dianders@chromium.org>
Date:	Fri,  6 Mar 2015 15:41:00 -0800
From:	Doug Anderson <dianders@...omium.org>
To:	Jiri Kosina <jkosina@...e.cz>,
	Andrew Duggan <aduggan@...aptics.com>,
	Vincent Huang <vincent.huang@...synaptics.com>
Cc:	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Doug Anderson <dianders@...omium.org>,
	mika.westerberg@...ux.intel.com, borneo.antonio@...il.com,
	jmaneyrol@...ensense.com, archana.patni@...ux.intel.com,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] RFT: HID: i2c-hid: Fix suspend/resume when already runtime suspended

If the i2c-hid device was runtime suspended and then the system
suspended itself we'd end up disabling interrupts twice (in
i2c_hid_runtime_suspend and i2c_hid_suspend) and not reenabling them
until later when the i2c-hid device was runtime resumed.
Unfortunately the i2c_hid_resume() calls i2c_hid_hwreset() and that
only works properly if interrupts are enabled.

We can fix this by taking the advice from "runtime_pm.txt".
Specifically we'll change i2c-hid to always resume to full power.
This only works if our parents are also resumed to full power, but
given the suggestion in "runtime_pm.txt" this seems a reasonable
assumption.

Signed-off-by: Doug Anderson <dianders@...omium.org>
---
NOTE: this patch was developed and tested on a 3.14 kernel (with some
backports) where the driver is a bit different.  I unfortunately
don't have a good way to test this on an upstream kernel, so hopefully
someone out there can confirm that the problem still exists and that
this patch looks reasonable.

This patch does change the order slightly: namely
hid->driver->suspend() is called later in the suspend sequence.
Hopefully that's OK.

 drivers/hid/i2c-hid/i2c-hid.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index ab4dd95..8943ac0 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1090,15 +1090,18 @@ static int i2c_hid_suspend(struct device *dev)
 	struct hid_device *hid = ihid->hid;
 	int ret = 0;
 
-	disable_irq(ihid->irq);
-	if (device_may_wakeup(&client->dev))
-		enable_irq_wake(ihid->irq);
+	if (!pm_runtime_suspended(dev)) {
+		/* Save some power */
+		i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
+
+		disable_irq(ihid->irq);
+	}
 
 	if (hid->driver && hid->driver->suspend)
 		ret = hid->driver->suspend(hid, PMSG_SUSPEND);
 
-	/* Save some power */
-	i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
+	if (device_may_wakeup(&client->dev))
+		enable_irq_wake(ihid->irq);
 
 	return ret;
 }
@@ -1110,6 +1113,10 @@ static int i2c_hid_resume(struct device *dev)
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
 	struct hid_device *hid = ihid->hid;
 
+	/* We'll resume to full power */
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+
 	enable_irq(ihid->irq);
 	ret = i2c_hid_hwreset(client);
 	if (ret)
@@ -1123,6 +1130,8 @@ static int i2c_hid_resume(struct device *dev)
 		return ret;
 	}
 
+	pm_runtime_enable(dev);
+
 	return 0;
 }
 #endif
-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/