From patchwork Tue Mar 8 23:03:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 8538271 Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E3F4E9F2B4 for ; Tue, 8 Mar 2016 23:03:31 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F2F70201C8 for ; Tue, 8 Mar 2016 23:03:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 018A7201BC for ; Tue, 8 Mar 2016 23:03:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751525AbcCHXD2 (ORCPT ); Tue, 8 Mar 2016 18:03:28 -0500 Received: from mail-pf0-f176.google.com ([209.85.192.176]:35034 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751149AbcCHXD1 (ORCPT ); Tue, 8 Mar 2016 18:03:27 -0500 Received: by mail-pf0-f176.google.com with SMTP id x188so23374907pfb.2; Tue, 08 Mar 2016 15:03:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:mime-version:content-disposition :user-agent; bh=9fIzmuaNhoIzWFpzjzAHQYdC+NJ+V8lVak8uJ8wS5RU=; b=jTYFCW+CQgDtwvtNPDlZXhWlWx/QMd+bgP4konTduT1+nIfcXncgZW15i0/pq3BpZz J0fx5lOkeS7W3Kyo6x7yVm/NhZrg+yuLEvu1Zzj8ttP3YeJBaaqW6hF+o2eAiJ9B/TJP 0D8d4Tb2LUz/tLuDKlFJ953OG5FZ39Z2RDVIAuS8jCAPw8hXYogItssJFyli0KcQa9/r Qp/I6CDN9ZRxlXr70sls6UPTUg1kdBAeFxGWXijRZBQrlids1mlXFWIK/QYWtiRsRX6z +G0OAZryyxn+g96Hk6g4ephccIJXVuyTuFtxPdBFZkBBCvYsDCfzT09CFx1aPr7kgrzU i8kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=9fIzmuaNhoIzWFpzjzAHQYdC+NJ+V8lVak8uJ8wS5RU=; b=leJQtDmPf4nkod6yYx61LCLBy26XGAwJHD8Y4Q3S51lIf0NXYUV5aNP5kW24OgaXlk R4vjWZSPYFTlCN5DeLM1IWU2S2Cbjfh09YDibB8YdsecDefMzY7ktXTt8cGYkostw355 LVe1nlM2KhYC/GmOOXw0pDSEOV3l2AdaLWeJPA964/9Ij7tur9d9a7D0Evs4uCliiEKC KrXzLmsbZ0XROsiLsBA5bJaiNpXR5iXlk9V+D4n63hQxilnBKlt91kFCRO7T5cB/u8Ap O/TkQ8JNgL2y7OA77KbcNWwBHnV/OpGbm7ednIDCbf+SmvgNPtieB/DQTLjLuF1CuukA 6nyw== X-Gm-Message-State: AD7BkJIrpZaOCpsXaBDQ1UTGwTwS5gqLl+XM+LA/EAQXBCjInmExb7SxodJkR9K3wI4FbA== X-Received: by 10.98.13.132 with SMTP id 4mr13021008pfn.122.1457478207014; Tue, 08 Mar 2016 15:03:27 -0800 (PST) Received: from dtor-ws ([2620:0:1000:1301:f8e0:5402:9139:7572]) by smtp.gmail.com with ESMTPSA id cq9sm7332167pac.26.2016.03.08.15.03.25 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 08 Mar 2016 15:03:26 -0800 (PST) Date: Tue, 8 Mar 2016 15:03:23 -0800 From: Dmitry Torokhov To: Jiri Kosina Cc: Benjamin Tissoires , Mika Westerberg , Andrew Duggan , Benson Leung , Dan Carpenter , Gabriele Mazzotta , Doug Anderson , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, "Rafael J. Wysocki" Subject: [PATCH v3] HID: i2c-hid: Fix suspend/resume when already runtime suspended Message-ID: <20160308230323.GA4382@dtor-ws> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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). Signed-off-by: Doug Anderson Signed-off-by: Dmitry Torokhov Reviewed-by: Benson Leung Tested-by: Mika Westerberg Acked-by: Benjamin Tissoires --- This is an uprev of a patch that Doug sent a year ago (see https://patchwork.kernel.org/patch/5970731/), adjusted to the latest mainline. The change from v2 is that we runtime-resume the device before calling into HID driver's suspend callback. drivers/hid/i2c-hid/i2c-hid.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index b921693..1f62751 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) + 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;