From patchwork Tue Oct 6 23:31:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 7340551 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 766A9BEEA4 for ; Tue, 6 Oct 2015 23:03:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 74786206AD for ; Tue, 6 Oct 2015 23:03:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5E4292051D for ; Tue, 6 Oct 2015 23:03:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753223AbbJFXDP (ORCPT ); Tue, 6 Oct 2015 19:03:15 -0400 Received: from v094114.home.net.pl ([79.96.170.134]:43734 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753219AbbJFXDO (ORCPT ); Tue, 6 Oct 2015 19:03:14 -0400 Received: from afbh101.neoplus.adsl.tpnet.pl (95.49.33.101) (HELO vostro.rjw.lan) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer v0.80) id 158dc285021b1df7; Wed, 7 Oct 2015 01:03:12 +0200 From: "Rafael J. Wysocki" To: Dmitry Torokhov Cc: Linux PM list , Linux Kernel Mailing List , ACPI Devel Maling List , Alan Stern , Daniel Vetter , Bjorn Helgaas , Linux PCI Subject: Re: [RFC][PATCH v3 3/3] input: i8042: Avoid resetting controller on system suspend/resume Date: Wed, 07 Oct 2015 01:31:44 +0200 Message-ID: <1769930.Ws5UzSBdLV@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/4.1.0-rc5+; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20151006224308.GD31850@dtor-ws> References: <10641052.oOBhM6BU9G@vostro.rjw.lan> <1927174.T9TA7zgbS1@vostro.rjw.lan> <20151006224308.GD31850@dtor-ws> MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, 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 Tuesday, October 06, 2015 03:43:08 PM Dmitry Torokhov wrote: > On Wed, Oct 07, 2015 at 01:08:30AM +0200, Rafael J. Wysocki wrote: > > On Tuesday, October 06, 2015 03:34:42 PM Dmitry Torokhov wrote: > > > On Wed, Oct 07, 2015 at 12:53:49AM +0200, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki > > > > > > > > If the upcoming system suspend is not going to be handled by the > > > > platform firmware, like in the suspend-to-idle case, it is not > > > > necessary to reset the controller in i8042_pm_suspend(), so avoid > > > > doing that. > > > > > > > > Moreover, if the system resume currently in progress has not been > > > > started by the platform firmware, like in the suspend-to-idle case, > > > > i8042_controller_resume() need not be called by i8042_pm_resume(), > > > > so avoid doing that too in that case. > > > > > > > > Additionally, try to catch the event that woke up the system by > > > > calling the interrupt handler early during system resume if it has > > > > not been started by the platform firmware. > > > > > > > > Signed-off-by: Rafael J. Wysocki > > > > --- > > > > drivers/input/serio/i8042.c | 15 +++++++++++++-- > > > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > > > > > Index: linux-pm/drivers/input/serio/i8042.c > > > > =================================================================== > > > > --- linux-pm.orig/drivers/input/serio/i8042.c > > > > +++ linux-pm/drivers/input/serio/i8042.c > > > > @@ -24,6 +24,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > #include > > > > > > > > @@ -1170,7 +1171,8 @@ static int i8042_pm_suspend(struct devic > > > > { > > > > int i; > > > > > > > > - i8042_controller_reset(true); > > > > + if (pm_suspend_via_firmware()) > > > > + i8042_controller_reset(true); > > > > > > > > /* Set up serio interrupts for system wakeup. */ > > > > for (i = 0; i < I8042_NUM_PORTS; i++) { > > > > @@ -1183,6 +1185,14 @@ static int i8042_pm_suspend(struct devic > > > > return 0; > > > > } > > > > > > > > +static int i8042_pm_resume_noirq(struct device *dev) > > > > +{ > > > > + if (!pm_resume_via_firmware()) > > > > + i8042_interrupt(0, NULL); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > static int i8042_pm_resume(struct device *dev) > > > > { > > > > int i; > > > > @@ -1199,7 +1209,7 @@ static int i8042_pm_resume(struct device > > > > * to bring it in a sane state. (In case of S2D we expect > > > > * BIOS to reset the controller for us.) > > > > */ > > > > - return i8042_controller_resume(true); > > > > + return pm_resume_via_firmware() ? i8042_controller_resume(true) : 0; > > > > > > What happens if we were going to suspend via firmware so we reset the > > > controller but then we got wakeup condition and we actually did not > > > suspend. What pm_resume_via_firmware() will return in this case? > > > > It will return 'false'. Do we need to resume the controller then? > > Yes. > > > But I guess > > 'false' should be passed to i8042_controller_resume() in that case? > > Yes, we do not need to reset the controller in this case, just > reactivate multiplexing mode, etc. So something like this I suppose: --- drivers/input/serio/i8042.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: linux-pm/drivers/input/serio/i8042.c =================================================================== --- linux-pm.orig/drivers/input/serio/i8042.c +++ linux-pm/drivers/input/serio/i8042.c @@ -24,6 +24,7 @@ #include #include #include +#include #include @@ -1170,7 +1171,8 @@ static int i8042_pm_suspend(struct devic { int i; - i8042_controller_reset(true); + if (pm_suspend_via_firmware()) + i8042_controller_reset(true); /* Set up serio interrupts for system wakeup. */ for (i = 0; i < I8042_NUM_PORTS; i++) { @@ -1183,6 +1185,14 @@ static int i8042_pm_suspend(struct devic return 0; } +static int i8042_pm_resume_noirq(struct device *dev) +{ + if (!pm_resume_via_firmware()) + i8042_interrupt(0, NULL); + + return 0; +} + static int i8042_pm_resume(struct device *dev) { int i; @@ -1199,7 +1209,8 @@ static int i8042_pm_resume(struct device * to bring it in a sane state. (In case of S2D we expect * BIOS to reset the controller for us.) */ - return i8042_controller_resume(true); + return pm_suspend_via_firmware() ? + i8042_controller_resume(pm_resume_via_firmware()) : 0; } static int i8042_pm_thaw(struct device *dev) @@ -1223,6 +1234,7 @@ static int i8042_pm_restore(struct devic static const struct dev_pm_ops i8042_pm_ops = { .suspend = i8042_pm_suspend, + .resume_noirq = i8042_pm_resume_noirq, .resume = i8042_pm_resume, .thaw = i8042_pm_thaw, .poweroff = i8042_pm_reset,