Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1241049 > unrolled thread

[PATCH v3 0/3] PM / sleep: Make it possible to check if suspend/resume goes via firmware

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 0/3] PM / sleep: Make it possible to check if suspend/resume goes via firmware "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-07 00:40 +0200
    [PATCH v3 2/3] PM / PCI / ACPI: Kick devices that might have been reset by firmware "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-07 00:40 +0200
    [RFC][PATCH v3 3/3] input: i8042: Avoid resetting controller on system suspend/resume "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-07 00:40 +0200
      [Update][PATCH 3/3] input: i8042: Avoid resetting controller on system suspend/resume "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-07 02:40 +0200
        Re: [Update][PATCH 3/3] input: i8042: Avoid resetting controller on system suspend/resume "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-12 21:50 +0200
          Re: [Update][PATCH 3/3] input: i8042: Avoid resetting controller on system suspend/resume "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-12 22:20 +0200
          Re: [Update][PATCH 3/3] input: i8042: Avoid resetting controller on  system suspend/resume Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-10-12 22:20 +0200

#1241049 — [PATCH v3 0/3] PM / sleep: Make it possible to check if suspend/resume goes via firmware

On Friday, October 02, 2015 03:50:42 AM Rafael J. Wysocki wrote:
> On Wednesday, September 30, 2015 02:46:10 AM Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > Patch [1/2] introduces a new mechanism that can be used to check whether or not
> > platform firmware is going to be involved in system suspend or was involved
> > in system resume.
> 
> That's still the case, but patch [2/2] modifies the PCI PM layer and the general
> ACPI PM domain only to use the mechanism added by the [1/2].

The first two patches are unchanged (in the face of the continuing lack of
comments I'll add them to my v4.4 queue at one point).

The last patch is new and actually is an RFC, but it shows how the new
mechanism introduced by [1/3] can be used.

Thanks,
Rafael

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

[toc] | [next] | [standalone]

#1241055 — [PATCH v3 2/3] PM / PCI / ACPI: Kick devices that might have been reset by firmware

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There is a concern that if the platform firmware was involved in
the system resume that's being completed,  some devices might have
been reset by it and if those devices had the power.direct_complete
flag set during the preceding suspend transition, they may stay
in a reset-power-on state indefinitely (until they are runtime-resumed
and then suspended again).  That may not be a big deal from the
individual device's perspective, but if the system is an SoC, it may
be prevented from entering deep SoC-wide low-power states on idle
because of that.

The devices that are most likely to be affected by this issue are
PCI devices and ACPI-enumerated devices using the general ACPI PM
domain, so to prevent it from happening for those devices, force a
runtime resume for them if they have their power.direct_complete
flags set and the platform firmware was involved in the resume
transition currently in progress.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_lpss.c         |    2 +-
 drivers/acpi/device_pm.c         |   19 +------------------
 drivers/base/power/generic_ops.c |   23 +++++++++++++++++++++++
 drivers/pci/pci-driver.c         |    2 +-
 include/linux/pm.h               |    2 +-
 5 files changed, 27 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -963,23 +963,6 @@ int acpi_subsys_prepare(struct device *d
 EXPORT_SYMBOL_GPL(acpi_subsys_prepare);
 
 /**
- * acpi_subsys_complete - Finalize device's resume during system resume.
- * @dev: Device to handle.
- */
-void acpi_subsys_complete(struct device *dev)
-{
-	pm_generic_complete(dev);
-	/*
-	 * If the device had been runtime-suspended before the system went into
-	 * the sleep state it is going out of and it has never been resumed till
-	 * now, resume it in case the firmware powered it up.
-	 */
-	if (dev->power.direct_complete)
-		pm_request_resume(dev);
-}
-EXPORT_SYMBOL_GPL(acpi_subsys_complete);
-
-/**
  * acpi_subsys_suspend - Run the device driver's suspend callback.
  * @dev: Device to handle.
  *
@@ -1047,7 +1030,7 @@ static struct dev_pm_domain acpi_general
 		.runtime_resume = acpi_subsys_runtime_resume,
 #ifdef CONFIG_PM_SLEEP
 		.prepare = acpi_subsys_prepare,
-		.complete = acpi_subsys_complete,
+		.complete = pm_complete_with_resume_check,
 		.suspend = acpi_subsys_suspend,
 		.suspend_late = acpi_subsys_suspend_late,
 		.resume_early = acpi_subsys_resume_early,
Index: linux-pm/drivers/acpi/acpi_lpss.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -664,7 +664,7 @@ static struct dev_pm_domain acpi_lpss_pm
 #ifdef CONFIG_PM
 #ifdef CONFIG_PM_SLEEP
 		.prepare = acpi_subsys_prepare,
-		.complete = acpi_subsys_complete,
+		.complete = pm_complete_with_resume_check,
 		.suspend = acpi_subsys_suspend,
 		.suspend_late = acpi_lpss_suspend_late,
 		.resume_early = acpi_lpss_resume_early,
Index: linux-pm/drivers/base/power/generic_ops.c
===================================================================
--- linux-pm.orig/drivers/base/power/generic_ops.c
+++ linux-pm/drivers/base/power/generic_ops.c
@@ -9,6 +9,7 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/export.h>
+#include <linux/suspend.h>
 
 #ifdef CONFIG_PM
 /**
@@ -297,4 +298,26 @@ void pm_generic_complete(struct device *
 	if (drv && drv->pm && drv->pm->complete)
 		drv->pm->complete(dev);
 }
+
+/**
+ * pm_complete_with_resume_check - Complete a device power transition.
+ * @dev: Device to handle.
+ *
+ * Complete a device power transition during a system-wide power transition and
+ * optionally schedule a runtime resume of the device if the system resume in
+ * progress has been initated by the platform firmware and the device had its
+ * power.direct_complete flag set.
+ */
+void pm_complete_with_resume_check(struct device *dev)
+{
+	pm_generic_complete(dev);
+	/*
+	 * If the device had been runtime-suspended before the system went into
+	 * the sleep state it is going out of and it has never been resumed till
+	 * now, resume it in case the firmware powered it up.
+	 */
+	if (dev->power.direct_complete && pm_resume_via_firmware())
+		pm_request_resume(dev);
+}
+EXPORT_SYMBOL_GPL(pm_complete_with_resume_check);
 #endif /* CONFIG_PM_SLEEP */
Index: linux-pm/drivers/pci/pci-driver.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-driver.c
+++ linux-pm/drivers/pci/pci-driver.c
@@ -687,7 +687,7 @@ static int pci_pm_prepare(struct device
 static void pci_pm_complete(struct device *dev)
 {
 	pci_dev_complete_resume(to_pci_dev(dev));
-	pm_generic_complete(dev);
+	pm_complete_with_resume_check(dev);
 }
 
 #else /* !CONFIG_PM_SLEEP */
Index: linux-pm/include/linux/pm.h
===================================================================
--- linux-pm.orig/include/linux/pm.h
+++ linux-pm/include/linux/pm.h
@@ -731,7 +731,7 @@ extern int pm_generic_restore(struct dev
 extern int pm_generic_poweroff_noirq(struct device *dev);
 extern int pm_generic_poweroff_late(struct device *dev);
 extern int pm_generic_poweroff(struct device *dev);
-extern void pm_generic_complete(struct device *dev);
+extern void pm_complete_with_resume_check(struct device *dev);
 
 #else /* !CONFIG_PM_SLEEP */
 

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

[toc] | [prev] | [next] | [standalone]

#1241056 — [RFC][PATCH v3 3/3] input: i8042: Avoid resetting controller on system suspend/resume

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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 <rafael.j.wysocki@intel.com>
---
 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 <linux/platform_device.h>
 #include <linux/i8042.h>
 #include <linux/slab.h>
+#include <linux/suspend.h>
 
 #include <asm/io.h>
 
@@ -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;
 }
 
 static int i8042_pm_thaw(struct device *dev)
@@ -1223,6 +1233,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,

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

[toc] | [prev] | [next] | [standalone]

#1241099 — [Update][PATCH 3/3] input: i8042: Avoid resetting controller on system suspend/resume

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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 <rafael.j.wysocki@intel.com>
---
 drivers/input/serio/i8042.c |   32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 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 <linux/platform_device.h>
 #include <linux/i8042.h>
 #include <linux/slab.h>
+#include <linux/suspend.h>
 
 #include <asm/io.h>
 
@@ -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,8 +1185,17 @@ 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)
 {
+	bool force_reset;
 	int i;
 
 	for (i = 0; i < I8042_NUM_PORTS; i++) {
@@ -1195,11 +1206,21 @@ static int i8042_pm_resume(struct device
 	}
 
 	/*
-	 * On resume from S2R we always try to reset the controller
-	 * to bring it in a sane state. (In case of S2D we expect
-	 * BIOS to reset the controller for us.)
+	 * If platform firmware was not going to be involved in suspend, we did
+	 * not restore the controller state to whatever it had been at boot
+	 * time, so we do not need to do anything.
 	 */
-	return i8042_controller_resume(true);
+	if (!pm_suspend_via_firmware())
+		return 0;
+
+	/*
+	 * We only need to reset the controller if we are resuming after handing
+	 * off control to the platform firmware, otherwise we can simply restore
+	 * the mode.
+	 */
+	force_reset = pm_resume_via_firmware();
+
+	return i8042_controller_resume(force_reset);
 }
 
 static int i8042_pm_thaw(struct device *dev)
@@ -1223,6 +1244,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,

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

[toc] | [prev] | [next] | [standalone]

#1245065 — Re: [Update][PATCH 3/3] input: i8042: Avoid resetting controller on system suspend/resume

On Wednesday, October 07, 2015 03:03:57 AM Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <rafael.j.wysocki@intel.com>

If there are no more comments here, I'll tentatively queue it up for v4.4
along with the [1-2/3] from this series.

Please let me know if there are any problems with that.

Thanks,
Rafael


> ---
>  drivers/input/serio/i8042.c |   32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 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 <linux/platform_device.h>
>  #include <linux/i8042.h>
>  #include <linux/slab.h>
> +#include <linux/suspend.h>
>  
>  #include <asm/io.h>
>  
> @@ -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,8 +1185,17 @@ 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)
>  {
> +	bool force_reset;
>  	int i;
>  
>  	for (i = 0; i < I8042_NUM_PORTS; i++) {
> @@ -1195,11 +1206,21 @@ static int i8042_pm_resume(struct device
>  	}
>  
>  	/*
> -	 * On resume from S2R we always try to reset the controller
> -	 * to bring it in a sane state. (In case of S2D we expect
> -	 * BIOS to reset the controller for us.)
> +	 * If platform firmware was not going to be involved in suspend, we did
> +	 * not restore the controller state to whatever it had been at boot
> +	 * time, so we do not need to do anything.
>  	 */
> -	return i8042_controller_resume(true);
> +	if (!pm_suspend_via_firmware())
> +		return 0;
> +
> +	/*
> +	 * We only need to reset the controller if we are resuming after handing
> +	 * off control to the platform firmware, otherwise we can simply restore
> +	 * the mode.
> +	 */
> +	force_reset = pm_resume_via_firmware();
> +
> +	return i8042_controller_resume(force_reset);
>  }
>  
>  static int i8042_pm_thaw(struct device *dev)
> @@ -1223,6 +1244,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,
> 
> --

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

[toc] | [prev] | [next] | [standalone]

#1245097 — Re: [Update][PATCH 3/3] input: i8042: Avoid resetting controller on system suspend/resume

On Monday, October 12, 2015 01:11:44 PM Dmitry Torokhov wrote:
> On Mon, Oct 12, 2015 at 10:17:05PM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, October 07, 2015 03:03:57 AM Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > 
> > > 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 <rafael.j.wysocki@intel.com>
> > 
> > If there are no more comments here, I'll tentatively queue it up for v4.4
> > along with the [1-2/3] from this series.
> > 
> > Please let me know if there are any problems with that.
> 
> No problems from my POV.
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks!

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

[toc] | [prev] | [next] | [standalone]

#1245102 — Re: [Update][PATCH 3/3] input: i8042: Avoid resetting controller on system suspend/resume

On Mon, Oct 12, 2015 at 10:17:05PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, October 07, 2015 03:03:57 AM Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > 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 <rafael.j.wysocki@intel.com>
> 
> If there are no more comments here, I'll tentatively queue it up for v4.4
> along with the [1-2/3] from this series.
> 
> Please let me know if there are any problems with that.

No problems from my POV.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web