From 01714a6f5fa59a313d8f44dcf017911dfcb25831 Mon Sep 17 00:00:00 2001 From: Doug Anderson Date: Tue, 8 Mar 2016 15:03:23 -0800 Subject: [PATCH] HID: i2c-hid: Fix suspend/resume when already runtime suspended On ACPI-based systems ACPI power domain code runtime resumes device before calling suspend method, which ensures that i2c-hid suspend code starts with device not in low-power state and with interrupts enabled. On other systems, especially if device is not a part of any power domain, we may end up calling driver's system-level suspend routine while the device is runtime-suspended (with controller in presumably low power state and interrupts disabled). This will result in interrupts being essentially disabled twice, and we will only re-enable them after both system resume and runtime resume methods complete. Unfortunately i2c_hid_resume() calls i2c_hid_hwreset() and that only works properly if interrupts are enabled. Also if device is runtime-suspended driver's suspend code may fail if it tries to issue I/O requests. Let's fix it by runtime-resuming the device if we need to run HID driver's suspend code and also disabling interrupts only if device is not already runtime-suspended. Also on resume we mark the device as running at full power (since that is what resetting will do to it). Reviewed-by: Benson Leung Tested-by: Mika Westerberg Acked-by: Benjamin Tissoires Signed-off-by: Doug Anderson Signed-off-by: Dmitry Torokhov Signed-off-by: Jiri Kosina --- drivers/hid/i2c-hid/i2c-hid.c | 45 +++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index a2c6c985b673..5b10a5d6e2d1 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -1108,13 +1108,30 @@ static int i2c_hid_suspend(struct device *dev) struct i2c_client *client = to_i2c_client(dev); struct i2c_hid *ihid = i2c_get_clientdata(client); struct hid_device *hid = ihid->hid; - int ret = 0; + int ret; int wake_status; - if (hid->driver && hid->driver->suspend) - ret = hid->driver->suspend(hid, PMSG_SUSPEND); + if (hid->driver && hid->driver->suspend) { + /* + * Wake up the device so that IO issues in + * HID driver's suspend code can succeed. + */ + ret = pm_runtime_resume(dev); + if (ret < 0) + return ret; + + ret = hid->driver->suspend(hid, PMSG_SUSPEND); + if (ret < 0) + return ret; + } + + if (!pm_runtime_suspended(dev)) { + /* Save some power */ + i2c_hid_set_power(client, I2C_HID_PWR_SLEEP); + + disable_irq(ihid->irq); + } - disable_irq(ihid->irq); if (device_may_wakeup(&client->dev)) { wake_status = enable_irq_wake(ihid->irq); if (!wake_status) @@ -1124,10 +1141,7 @@ static int i2c_hid_suspend(struct device *dev) wake_status); } - /* Save some power */ - i2c_hid_set_power(client, I2C_HID_PWR_SLEEP); - - return ret; + return 0; } static int i2c_hid_resume(struct device *dev) @@ -1138,11 +1152,6 @@ static int i2c_hid_resume(struct device *dev) struct hid_device *hid = ihid->hid; int wake_status; - enable_irq(ihid->irq); - ret = i2c_hid_hwreset(client); - if (ret) - return ret; - if (device_may_wakeup(&client->dev) && ihid->irq_wake_enabled) { wake_status = disable_irq_wake(ihid->irq); if (!wake_status) @@ -1152,6 +1161,16 @@ static int i2c_hid_resume(struct device *dev) wake_status); } + /* We'll resume to full power */ + pm_runtime_disable(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + + enable_irq(ihid->irq); + ret = i2c_hid_hwreset(client); + if (ret) + return ret; + if (hid->driver && hid->driver->reset_resume) { ret = hid->driver->reset_resume(hid); return ret;