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

Groups > linux.kernel > #1660587 > unrolled thread

[PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 02:20 +0200
    [PATCH 1/6] ACPI / PM: Run wakeup notify handlers synchronously "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 02:20 +0200
    [PATCH 5/6] PM / sleep: Print timing information if debug is enabled "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 02:20 +0200
    [PATCH 3/6] ACPI / PM: Change log level of wakeup-related message "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 02:20 +0200
    [PATCH 6/6] ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 02:20 +0200
    [PATCH 2/6] USB / PCI / PM: Allow the PCI core to do the resume cleanup "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 02:20 +0200
      Re: [PATCH 2/6] USB / PCI / PM: Allow the PCI core to do the resume  cleanup Alan Stern <stern@rowland.harvard.edu> - 2017-06-08 17:30 +0200
        Re: [PATCH 2/6] USB / PCI / PM: Allow the PCI core to do the resume cleanup "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-09 01:10 +0200
    [PATCH 4/6] ACPI / PM: Clean up device wakeup enable/disable code "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 02:20 +0200
    Re: [PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with  spurious ACPI wakeups Hans de Goede <hdegoede@redhat.com> - 2017-06-08 09:30 +0200
    Re: [PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with  spurious ACPI wakeups Dominik Brodowski <linux@dominikbrodowski.net> - 2017-06-08 10:50 +0200
      Re: [PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-08 14:10 +0200
    [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
      [PATCH v2 6/8] PCI / PM: Restore PME Enable if skipping wakeup setup "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
        Re: [PATCH v2 6/8] PCI / PM: Restore PME Enable if skipping wakeup  setup Bjorn Helgaas <helgaas@kernel.org> - 2017-06-14 20:20 +0200
      [PATCH v2 4/8] ACPI / PM: Clean up device wakeup enable/disable code "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
      [PATCH v2 5/8] PM / sleep: Print timing information if debug is enabled "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
        Re: [PATCH v2 5/8] PM / sleep: Print timing information if debug is  enabled Joe Perches <joe@perches.com> - 2017-06-13 00:20 +0200
      [PATCH v2 8/8] ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
      [PATCH v2 3/8] ACPI / PM: Change log level of wakeup-related message "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
      [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
        Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the  resume cleanup Greg KH <greg@kroah.com> - 2017-06-13 11:00 +0200
          Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-13 13:30 +0200
      [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
        Re: [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers  synchronously Bjorn Helgaas <helgaas@kernel.org> - 2017-06-14 20:20 +0200
          Re: [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously "Rafael J. Wysocki" <rafael@kernel.org> - 2017-06-15 00:30 +0200
      [PATCH v2 7/8] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-12 23:10 +0200
      Re: [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with  spurious ACPI wakeups Dominik Brodowski <linux@dominikbrodowski.net> - 2017-06-13 08:00 +0200
        Re: [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-06-13 13:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1664231 — [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

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

hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
PCI USB controllers calls pci_back_from_sleep() which is unnecessary
and may become problematic.

It is unnecessary, because the PCI bus type carries out post-suspend
cleanup of all PCI devices during resume and that covers all things
done by the pci_back_from_sleep().  There is no reason why USB cannot
follow all of the other PCI devices in that respect.

It will become problematic after subsequent changes that make it
possible to go back to sleep again after executing dpm_resume_noirq()
if no valid system wakeup events have been detected at that point.
Namely, calling pci_back_from_sleep() at the _resume_noirq stage
will cause the wakeup status of the devices in question to be cleared
and if any of them has triggered system wakeup, that event may be
missed then.

For the above reasons, drop the pci_back_from_sleep() invocation
from hcd_pci_resume_noirq().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/hcd-pci.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Index: linux-pm/drivers/usb/core/hcd-pci.c
===================================================================
--- linux-pm.orig/drivers/usb/core/hcd-pci.c
+++ linux-pm/drivers/usb/core/hcd-pci.c
@@ -584,12 +584,7 @@ static int hcd_pci_suspend_noirq(struct
 
 static int hcd_pci_resume_noirq(struct device *dev)
 {
-	struct pci_dev		*pci_dev = to_pci_dev(dev);
-
-	powermac_set_asic(pci_dev, 1);
-
-	/* Go back to D0 and disable remote wakeup */
-	pci_back_from_sleep(pci_dev);
+	powermac_set_asic(to_pci_dev(dev), 1);
 	return 0;
 }
 

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

#1664610 — Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

On Mon, Jun 12, 2017 at 10:49:40PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> and may become problematic.
> 
> It is unnecessary, because the PCI bus type carries out post-suspend
> cleanup of all PCI devices during resume and that covers all things
> done by the pci_back_from_sleep().  There is no reason why USB cannot
> follow all of the other PCI devices in that respect.
> 
> It will become problematic after subsequent changes that make it
> possible to go back to sleep again after executing dpm_resume_noirq()
> if no valid system wakeup events have been detected at that point.
> Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> will cause the wakeup status of the devices in question to be cleared
> and if any of them has triggered system wakeup, that event may be
> missed then.
> 
> For the above reasons, drop the pci_back_from_sleep() invocation
> from hcd_pci_resume_noirq().
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

#1664762 — Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

On Tuesday, June 13, 2017 10:52:52 AM Greg KH wrote:
> On Mon, Jun 12, 2017 at 10:49:40PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> > PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> > and may become problematic.
> > 
> > It is unnecessary, because the PCI bus type carries out post-suspend
> > cleanup of all PCI devices during resume and that covers all things
> > done by the pci_back_from_sleep().  There is no reason why USB cannot
> > follow all of the other PCI devices in that respect.
> > 
> > It will become problematic after subsequent changes that make it
> > possible to go back to sleep again after executing dpm_resume_noirq()
> > if no valid system wakeup events have been detected at that point.
> > Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> > will cause the wakeup status of the devices in question to be cleared
> > and if any of them has triggered system wakeup, that event may be
> > missed then.
> > 
> > For the above reasons, drop the pci_back_from_sleep() invocation
> > from hcd_pci_resume_noirq().
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks!

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

#1664233 — [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously

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

The work functions provided by the users of acpi_add_pm_notifier()
should be run synchronously before re-enabling the wakeup GPE in
case they are used to clear the status and/or disable the wakeup
signaling at the source.  Otherwise, which is the case currently in
the PCI bus type code, the same wakeup event may be signaled for
multiple times while the execution of the work function in response
to it has already been queued up.

Fortunately, acpi_add_pm_notifier() is only used by PCI and by
ACPI device PM code internally, so the change is relatively
straightforward to make.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   27 +++++++++++----------------
 drivers/pci/pci-acpi.c   |   17 +++++++----------
 include/acpi/acpi_bus.h  |    4 ++--
 3 files changed, 20 insertions(+), 28 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -400,8 +400,8 @@ static void acpi_pm_notify_handler(acpi_
 
 	if (adev->wakeup.flags.notifier_present) {
 		__pm_wakeup_event(adev->wakeup.ws, 0);
-		if (adev->wakeup.context.work.func)
-			queue_pm_work(&adev->wakeup.context.work);
+		if (adev->wakeup.context.func)
+			adev->wakeup.context.func(&adev->wakeup.context);
 	}
 
 	mutex_unlock(&acpi_pm_notifier_lock);
@@ -413,7 +413,7 @@ static void acpi_pm_notify_handler(acpi_
  * acpi_add_pm_notifier - Register PM notify handler for given ACPI device.
  * @adev: ACPI device to add the notify handler for.
  * @dev: Device to generate a wakeup event for while handling the notification.
- * @work_func: Work function to execute when handling the notification.
+ * @func: Work function to execute when handling the notification.
  *
  * NOTE: @adev need not be a run-wake or wakeup device to be a valid source of
  * PM wakeup events.  For example, wakeup events may be generated for bridges
@@ -421,11 +421,11 @@ static void acpi_pm_notify_handler(acpi_
  * bridge itself doesn't have a wakeup GPE associated with it.
  */
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
-				 void (*work_func)(struct work_struct *work))
+			void (*func)(struct acpi_device_wakeup_context *context))
 {
 	acpi_status status = AE_ALREADY_EXISTS;
 
-	if (!dev && !work_func)
+	if (!dev && !func)
 		return AE_BAD_PARAMETER;
 
 	mutex_lock(&acpi_pm_notifier_lock);
@@ -435,8 +435,7 @@ acpi_status acpi_add_pm_notifier(struct
 
 	adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
 	adev->wakeup.context.dev = dev;
-	if (work_func)
-		INIT_WORK(&adev->wakeup.context.work, work_func);
+	adev->wakeup.context.func = func;
 
 	status = acpi_install_notify_handler(adev->handle, ACPI_SYSTEM_NOTIFY,
 					     acpi_pm_notify_handler, NULL);
@@ -469,10 +468,7 @@ acpi_status acpi_remove_pm_notifier(stru
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	if (adev->wakeup.context.work.func) {
-		cancel_work_sync(&adev->wakeup.context.work);
-		adev->wakeup.context.work.func = NULL;
-	}
+	adev->wakeup.context.func = NULL;
 	adev->wakeup.context.dev = NULL;
 	wakeup_source_unregister(adev->wakeup.ws);
 
@@ -658,16 +654,15 @@ EXPORT_SYMBOL(acpi_pm_device_sleep_state
 
 /**
  * acpi_pm_notify_work_func - ACPI devices wakeup notification work function.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void acpi_pm_notify_work_func(struct work_struct *work)
+static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context)
 {
-	struct device *dev;
+	struct device *dev = context->dev;
 
-	dev = container_of(work, struct acpi_device_wakeup_context, work)->dev;
 	if (dev) {
 		pm_wakeup_event(dev, 0);
-		pm_runtime_resume(dev);
+		pm_request_resume(dev);
 	}
 }
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -319,7 +319,7 @@ struct acpi_device_wakeup_flags {
 };
 
 struct acpi_device_wakeup_context {
-	struct work_struct work;
+	void (*func)(struct acpi_device_wakeup_context *context);
 	struct device *dev;
 };
 
@@ -599,7 +599,7 @@ static inline bool acpi_device_always_pr
 
 #ifdef CONFIG_PM
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
-				 void (*work_func)(struct work_struct *work));
+			void (*func)(struct acpi_device_wakeup_context *context));
 acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
 int acpi_pm_device_sleep_state(struct device *, int *, int);
 int acpi_pm_device_run_wake(struct device *, bool);
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -395,29 +395,26 @@ bool pciehp_is_native(struct pci_dev *pd
 
 /**
  * pci_acpi_wake_bus - Root bus wakeup notification fork function.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void pci_acpi_wake_bus(struct work_struct *work)
+static void pci_acpi_wake_bus(struct acpi_device_wakeup_context *context)
 {
 	struct acpi_device *adev;
 	struct acpi_pci_root *root;
 
-	adev = container_of(work, struct acpi_device, wakeup.context.work);
+	adev = container_of(context, struct acpi_device, wakeup.context);
 	root = acpi_driver_data(adev);
 	pci_pme_wakeup_bus(root->bus);
 }
 
 /**
  * pci_acpi_wake_dev - PCI device wakeup notification work function.
- * @handle: ACPI handle of a device the notification is for.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void pci_acpi_wake_dev(struct work_struct *work)
+static void pci_acpi_wake_dev(struct acpi_device_wakeup_context *context)
 {
-	struct acpi_device_wakeup_context *context;
 	struct pci_dev *pci_dev;
 
-	context = container_of(work, struct acpi_device_wakeup_context, work);
 	pci_dev = to_pci_dev(context->dev);
 
 	if (pci_dev->pme_poll)
@@ -425,7 +422,7 @@ static void pci_acpi_wake_dev(struct wor
 
 	if (pci_dev->current_state == PCI_D3cold) {
 		pci_wakeup_event(pci_dev);
-		pm_runtime_resume(&pci_dev->dev);
+		pm_request_resume(&pci_dev->dev);
 		return;
 	}
 
@@ -434,7 +431,7 @@ static void pci_acpi_wake_dev(struct wor
 		pci_check_pme_status(pci_dev);
 
 	pci_wakeup_event(pci_dev);
-	pm_runtime_resume(&pci_dev->dev);
+	pm_request_resume(&pci_dev->dev);
 
 	pci_pme_wakeup_bus(pci_dev->subordinate);
 }

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

#1666124 — Re: [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously

On Mon, Jun 12, 2017 at 10:48:41PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The work functions provided by the users of acpi_add_pm_notifier()
> should be run synchronously before re-enabling the wakeup GPE in
> case they are used to clear the status and/or disable the wakeup
> signaling at the source.  Otherwise, which is the case currently in
> the PCI bus type code, the same wakeup event may be signaled for
> multiple times while the execution of the work function in response
> to it has already been queued up.
> 
> Fortunately, acpi_add_pm_notifier() is only used by PCI and by
> ACPI device PM code internally, so the change is relatively
> straightforward to make.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/acpi/device_pm.c |   27 +++++++++++----------------
>  drivers/pci/pci-acpi.c   |   17 +++++++----------
>  include/acpi/acpi_bus.h  |    4 ++--
>  3 files changed, 20 insertions(+), 28 deletions(-)
> 
> Index: linux-pm/drivers/acpi/device_pm.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -400,8 +400,8 @@ static void acpi_pm_notify_handler(acpi_
>  
>  	if (adev->wakeup.flags.notifier_present) {
>  		__pm_wakeup_event(adev->wakeup.ws, 0);
> -		if (adev->wakeup.context.work.func)
> -			queue_pm_work(&adev->wakeup.context.work);
> +		if (adev->wakeup.context.func)
> +			adev->wakeup.context.func(&adev->wakeup.context);
>  	}
>  
>  	mutex_unlock(&acpi_pm_notifier_lock);
> @@ -413,7 +413,7 @@ static void acpi_pm_notify_handler(acpi_
>   * acpi_add_pm_notifier - Register PM notify handler for given ACPI device.
>   * @adev: ACPI device to add the notify handler for.
>   * @dev: Device to generate a wakeup event for while handling the notification.
> - * @work_func: Work function to execute when handling the notification.
> + * @func: Work function to execute when handling the notification.
>   *
>   * NOTE: @adev need not be a run-wake or wakeup device to be a valid source of
>   * PM wakeup events.  For example, wakeup events may be generated for bridges
> @@ -421,11 +421,11 @@ static void acpi_pm_notify_handler(acpi_
>   * bridge itself doesn't have a wakeup GPE associated with it.
>   */
>  acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
> -				 void (*work_func)(struct work_struct *work))
> +			void (*func)(struct acpi_device_wakeup_context *context))
>  {
>  	acpi_status status = AE_ALREADY_EXISTS;
>  
> -	if (!dev && !work_func)
> +	if (!dev && !func)
>  		return AE_BAD_PARAMETER;
>  
>  	mutex_lock(&acpi_pm_notifier_lock);
> @@ -435,8 +435,7 @@ acpi_status acpi_add_pm_notifier(struct
>  
>  	adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
>  	adev->wakeup.context.dev = dev;
> -	if (work_func)
> -		INIT_WORK(&adev->wakeup.context.work, work_func);
> +	adev->wakeup.context.func = func;
>  
>  	status = acpi_install_notify_handler(adev->handle, ACPI_SYSTEM_NOTIFY,
>  					     acpi_pm_notify_handler, NULL);
> @@ -469,10 +468,7 @@ acpi_status acpi_remove_pm_notifier(stru
>  	if (ACPI_FAILURE(status))
>  		goto out;
>  
> -	if (adev->wakeup.context.work.func) {
> -		cancel_work_sync(&adev->wakeup.context.work);
> -		adev->wakeup.context.work.func = NULL;
> -	}
> +	adev->wakeup.context.func = NULL;
>  	adev->wakeup.context.dev = NULL;
>  	wakeup_source_unregister(adev->wakeup.ws);
>  
> @@ -658,16 +654,15 @@ EXPORT_SYMBOL(acpi_pm_device_sleep_state
>  
>  /**
>   * acpi_pm_notify_work_func - ACPI devices wakeup notification work function.
> - * @work: Work item to handle.
> + * @context: Device wakeup context.
>   */
> -static void acpi_pm_notify_work_func(struct work_struct *work)
> +static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context)
>  {
> -	struct device *dev;
> +	struct device *dev = context->dev;
>  
> -	dev = container_of(work, struct acpi_device_wakeup_context, work)->dev;
>  	if (dev) {
>  		pm_wakeup_event(dev, 0);
> -		pm_runtime_resume(dev);
> +		pm_request_resume(dev);
>  	}
>  }
>  
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -319,7 +319,7 @@ struct acpi_device_wakeup_flags {
>  };
>  
>  struct acpi_device_wakeup_context {
> -	struct work_struct work;
> +	void (*func)(struct acpi_device_wakeup_context *context);
>  	struct device *dev;
>  };
>  
> @@ -599,7 +599,7 @@ static inline bool acpi_device_always_pr
>  
>  #ifdef CONFIG_PM
>  acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
> -				 void (*work_func)(struct work_struct *work));
> +			void (*func)(struct acpi_device_wakeup_context *context));
>  acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
>  int acpi_pm_device_sleep_state(struct device *, int *, int);
>  int acpi_pm_device_run_wake(struct device *, bool);
> Index: linux-pm/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-acpi.c
> +++ linux-pm/drivers/pci/pci-acpi.c
> @@ -395,29 +395,26 @@ bool pciehp_is_native(struct pci_dev *pd
>  
>  /**
>   * pci_acpi_wake_bus - Root bus wakeup notification fork function.
> - * @work: Work item to handle.
> + * @context: Device wakeup context.
>   */
> -static void pci_acpi_wake_bus(struct work_struct *work)
> +static void pci_acpi_wake_bus(struct acpi_device_wakeup_context *context)
>  {
>  	struct acpi_device *adev;
>  	struct acpi_pci_root *root;
>  
> -	adev = container_of(work, struct acpi_device, wakeup.context.work);
> +	adev = container_of(context, struct acpi_device, wakeup.context);
>  	root = acpi_driver_data(adev);
>  	pci_pme_wakeup_bus(root->bus);
>  }
>  
>  /**
>   * pci_acpi_wake_dev - PCI device wakeup notification work function.
> - * @handle: ACPI handle of a device the notification is for.
> - * @work: Work item to handle.
> + * @context: Device wakeup context.
>   */
> -static void pci_acpi_wake_dev(struct work_struct *work)
> +static void pci_acpi_wake_dev(struct acpi_device_wakeup_context *context)
>  {
> -	struct acpi_device_wakeup_context *context;
>  	struct pci_dev *pci_dev;
>  
> -	context = container_of(work, struct acpi_device_wakeup_context, work);
>  	pci_dev = to_pci_dev(context->dev);
>  
>  	if (pci_dev->pme_poll)
> @@ -425,7 +422,7 @@ static void pci_acpi_wake_dev(struct wor
>  
>  	if (pci_dev->current_state == PCI_D3cold) {
>  		pci_wakeup_event(pci_dev);
> -		pm_runtime_resume(&pci_dev->dev);
> +		pm_request_resume(&pci_dev->dev);
>  		return;
>  	}
>  
> @@ -434,7 +431,7 @@ static void pci_acpi_wake_dev(struct wor
>  		pci_check_pme_status(pci_dev);
>  
>  	pci_wakeup_event(pci_dev);
> -	pm_runtime_resume(&pci_dev->dev);
> +	pm_request_resume(&pci_dev->dev);
>  
>  	pci_pme_wakeup_bus(pci_dev->subordinate);
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

#1666288 — Re: [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously

On Wed, Jun 14, 2017 at 8:09 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Jun 12, 2017 at 10:48:41PM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> The work functions provided by the users of acpi_add_pm_notifier()
>> should be run synchronously before re-enabling the wakeup GPE in
>> case they are used to clear the status and/or disable the wakeup
>> signaling at the source.  Otherwise, which is the case currently in
>> the PCI bus type code, the same wakeup event may be signaled for
>> multiple times while the execution of the work function in response
>> to it has already been queued up.
>>
>> Fortunately, acpi_add_pm_notifier() is only used by PCI and by
>> ACPI device PM code internally, so the change is relatively
>> straightforward to make.
>>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Thanks!

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

#1664234 — [PATCH v2 7/8] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

From: Hans de Goede <hdegoede@redhat.com>

Some peripherals on Bay Trail and Cherry Trail platforms signal a
Power Management Event (PME) to the Power Management Controller (PMC)
to wakeup the system. When this happens software needs to explicitly
clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
IRQ storm on IRQ 9.

This is modelled in ACPI through the INT0002 ACPI device, which is
called a "Virtual GPIO controller" in ACPI because it defines the
event handler to call when the PME triggers through _AEI and _L02
methods as would be done for a real GPIO interrupt in ACPI.

This commit adds a driver which registers the Virtual GPIOs expected
by the DSDT on these devices, letting gpiolib-acpi claim the
virtual GPIO and install a GPIO-interrupt handler which call the _L02
handler as it would for a real GPIO controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/Kconfig               |  19 +++
 drivers/platform/x86/Makefile              |   1 +
 drivers/platform/x86/intel_int0002_vgpio.c | 219 +++++++++++++++++++++++++++++
 3 files changed, 239 insertions(+)
 create mode 100644 drivers/platform/x86/intel_int0002_vgpio.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 8489020ecf44..a3ccc3c795a5 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -794,6 +794,25 @@ config INTEL_CHT_INT33FE
 	  This driver instantiates i2c-clients for these, so that standard
 	  i2c drivers for these chips can bind to the them.
 
+config INTEL_INT0002_VGPIO
+	tristate "Intel ACPI INT0002 Virtual GPIO driver"
+	depends on GPIOLIB && ACPI
+	select GPIOLIB_IRQCHIP
+	---help---
+	  Some peripherals on Bay Trail and Cherry Trail platforms signal a
+	  Power Management Event (PME) to the Power Management Controller (PMC)
+	  to wakeup the system. When this happens software needs to explicitly
+	  clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
+	  IRQ storm on IRQ 9.
+
+	  This is modelled in ACPI through the INT0002 ACPI device, which is
+	  called a "Virtual GPIO controller" in ACPI because it defines the
+	  event handler to call when the PME triggers through _AEI and _L02
+	  methods as would be done for a real GPIO interrupt in ACPI.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called intel_int0002_vgpio.
+
 config INTEL_HID_EVENT
 	tristate "INTEL HID Event"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 182a3ed6605a..ab22ce77fb66 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_TOSHIBA_BT_RFKILL)	+= toshiba_bluetooth.o
 obj-$(CONFIG_TOSHIBA_HAPS)	+= toshiba_haps.o
 obj-$(CONFIG_TOSHIBA_WMI)	+= toshiba-wmi.o
 obj-$(CONFIG_INTEL_CHT_INT33FE)	+= intel_cht_int33fe.o
+obj-$(CONFIG_INTEL_INT0002_VGPIO) += intel_int0002_vgpio.o
 obj-$(CONFIG_INTEL_HID_EVENT)	+= intel-hid.o
 obj-$(CONFIG_INTEL_VBTN)	+= intel-vbtn.o
 obj-$(CONFIG_INTEL_SCU_IPC)	+= intel_scu_ipc.o
diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c
new file mode 100644
index 000000000000..92dc230ef5b2
--- /dev/null
+++ b/drivers/platform/x86/intel_int0002_vgpio.c
@@ -0,0 +1,219 @@
+/*
+ * Intel INT0002 "Virtual GPIO" driver
+ *
+ * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
+ *
+ * Loosely based on android x86 kernel code which is:
+ *
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * Author: Dyut Kumar Sil <dyut.k.sil@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Some peripherals on Bay Trail and Cherry Trail platforms signal a Power
+ * Management Event (PME) to the Power Management Controller (PMC) to wakeup
+ * the system. When this happens software needs to clear the PME bus 0 status
+ * bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
+ *
+ * This is modelled in ACPI through the INT0002 ACPI device, which is
+ * called a "Virtual GPIO controller" in ACPI because it defines the event
+ * handler to call when the PME triggers through _AEI and _L02 / _E02
+ * methods as would be done for a real GPIO interrupt in ACPI. Note this
+ * is a hack to define an AML event handler for the PME while using existing
+ * ACPI mechanisms, this is not a real GPIO at all.
+ *
+ * This driver will bind to the INT0002 device, and register as a GPIO
+ * controller, letting gpiolib-acpi.c call the _L02 handler as it would
+ * for a real GPIO controller.
+ */
+
+#include <linux/acpi.h>
+#include <linux/bitmap.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
+#define DRV_NAME			"INT0002 Virtual GPIO"
+
+/* For some reason the virtual GPIO pin tied to the GPE is numbered pin 2 */
+#define GPE0A_PME_B0_VIRT_GPIO_PIN	2
+
+#define GPE0A_PME_B0_STS_BIT		BIT(13)
+#define GPE0A_PME_B0_EN_BIT		BIT(13)
+#define GPE0A_STS_PORT			0x420
+#define GPE0A_EN_PORT			0x428
+
+#define ICPU(model)	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+static const struct x86_cpu_id int0002_cpu_ids[] = {
+/*
+ * Limit ourselves to Cherry Trail for now, until testing shows we
+ * need to handle the INT0002 device on Baytrail too.
+ *	ICPU(INTEL_FAM6_ATOM_SILVERMONT1),	 * Valleyview, Bay Trail *
+ */
+	ICPU(INTEL_FAM6_ATOM_AIRMONT),		/* Braswell, Cherry Trail */
+	{}
+};
+
+/*
+ * As this is not a real GPIO at all, but just a hack to model an event in
+ * ACPI the get / set functions are dummy functions.
+ */
+
+static int int0002_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	return 0;
+}
+
+static void int0002_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+}
+
+static int int0002_gpio_direction_output(struct gpio_chip *chip,
+					 unsigned int offset, int value)
+{
+	return 0;
+}
+
+static void int0002_irq_ack(struct irq_data *data)
+{
+	outl(GPE0A_PME_B0_STS_BIT, GPE0A_STS_PORT);
+}
+
+static void int0002_irq_unmask(struct irq_data *data)
+{
+	u32 gpe_en_reg;
+
+	gpe_en_reg = inl(GPE0A_EN_PORT);
+	gpe_en_reg |= GPE0A_PME_B0_EN_BIT;
+	outl(gpe_en_reg, GPE0A_EN_PORT);
+}
+
+static void int0002_irq_mask(struct irq_data *data)
+{
+	u32 gpe_en_reg;
+
+	gpe_en_reg = inl(GPE0A_EN_PORT);
+	gpe_en_reg &= ~GPE0A_PME_B0_EN_BIT;
+	outl(gpe_en_reg, GPE0A_EN_PORT);
+}
+
+static irqreturn_t int0002_irq(int irq, void *data)
+{
+	struct gpio_chip *chip = data;
+	u32 gpe_sts_reg;
+
+	gpe_sts_reg = inl(GPE0A_STS_PORT);
+	if (!(gpe_sts_reg & GPE0A_PME_B0_STS_BIT))
+		return IRQ_NONE;
+
+	generic_handle_irq(irq_find_mapping(chip->irqdomain,
+					    GPE0A_PME_B0_VIRT_GPIO_PIN));
+
+	pm_system_wakeup();
+
+	return IRQ_HANDLED;
+}
+
+static struct irq_chip int0002_irqchip = {
+	.name			= DRV_NAME,
+	.irq_ack		= int0002_irq_ack,
+	.irq_mask		= int0002_irq_mask,
+	.irq_unmask		= int0002_irq_unmask,
+};
+
+static int int0002_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct x86_cpu_id *cpu_id;
+	struct gpio_chip *chip;
+	int irq, ret;
+
+	/* Menlow has a different INT0002 device? <sigh> */
+	cpu_id = x86_match_cpu(int0002_cpu_ids);
+	if (!cpu_id)
+		return -ENODEV;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "Error getting IRQ: %d\n", irq);
+		return irq;
+	}
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->label = DRV_NAME;
+	chip->parent = dev;
+	chip->owner = THIS_MODULE;
+	chip->get = int0002_gpio_get;
+	chip->set = int0002_gpio_set;
+	chip->direction_input = int0002_gpio_get;
+	chip->direction_output = int0002_gpio_direction_output;
+	chip->base = -1;
+	chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1;
+	chip->irq_need_valid_mask = true;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL);
+	if (ret) {
+		dev_err(dev, "Error adding gpio chip: %d\n", ret);
+		return ret;
+	}
+
+	bitmap_clear(chip->irq_valid_mask, 0, GPE0A_PME_B0_VIRT_GPIO_PIN);
+
+	/*
+	 * We manually request the irq here instead of passing a flow-handler
+	 * to gpiochip_set_chained_irqchip, because the irq is shared.
+	 */
+	ret = devm_request_irq(dev, irq, int0002_irq,
+			       IRQF_SHARED | IRQF_NO_THREAD, "INT0002", chip);
+	if (ret) {
+		dev_err(dev, "Error requesting IRQ %d: %d\n", irq, ret);
+		return ret;
+	}
+
+	ret = gpiochip_irqchip_add(chip, &int0002_irqchip, 0, handle_edge_irq,
+				   IRQ_TYPE_NONE);
+	if (ret) {
+		dev_err(dev, "Error adding irqchip: %d\n", ret);
+		return ret;
+	}
+
+	gpiochip_set_chained_irqchip(chip, &int0002_irqchip, irq, NULL);
+
+	return 0;
+}
+
+static const struct acpi_device_id int0002_acpi_ids[] = {
+	{ "INT0002", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, int0002_acpi_ids);
+
+static struct platform_driver int0002_driver = {
+	.driver = {
+		.name			= DRV_NAME,
+		.acpi_match_table	= int0002_acpi_ids,
+	},
+	.probe	= int0002_probe,
+};
+
+module_platform_driver(int0002_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Intel INT0002 Virtual GPIO driver");
+MODULE_LICENSE("GPL");

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

#1664460 — Re: [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

Rafael,

On Mon, Jun 12, 2017 at 10:46:47PM +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> On Thursday, June 08, 2017 02:00:40 AM Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> > spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> > will be reverted shortly, because it triggered issues on quite a few systems.
> > 
> > The last patch in the series is a counterpart of the above commit with a few
> > modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> > printed to kernel logs to make them somewhat less confusing.
> 
> Here's a v2 which is very similar to the previous one, except for 3 things:
> - There are few build fixes in the last patch.
> - The patch from Hans mentioned previously is included now (as [7/8]).
> - There is an additional PCI change related to config space saving/restoring
>   and PME.
> 
> If anyone has any objections or concerns regarding this, please speak up now,
> as I'm going to put it into linux-next shortly to allow it to receive some more
> testing than commit eed4d47efe95 had had before it went it.

suspend-to-mem works as expected, also with this v2 applied. Thanks!

	Dominik

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

#1664761 — Re: [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

On Tuesday, June 13, 2017 07:54:22 AM Dominik Brodowski wrote:
> Rafael,
> 
> On Mon, Jun 12, 2017 at 10:46:47PM +0200, Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > On Thursday, June 08, 2017 02:00:40 AM Rafael J. Wysocki wrote:
> > > Hi All,
> > > 
> > > This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> > > spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> > > will be reverted shortly, because it triggered issues on quite a few systems.
> > > 
> > > The last patch in the series is a counterpart of the above commit with a few
> > > modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> > > printed to kernel logs to make them somewhat less confusing.
> > 
> > Here's a v2 which is very similar to the previous one, except for 3 things:
> > - There are few build fixes in the last patch.
> > - The patch from Hans mentioned previously is included now (as [7/8]).
> > - There is an additional PCI change related to config space saving/restoring
> >   and PME.
> > 
> > If anyone has any objections or concerns regarding this, please speak up now,
> > as I'm going to put it into linux-next shortly to allow it to receive some more
> > testing than commit eed4d47efe95 had had before it went it.
> 
> suspend-to-mem works as expected, also with this v2 applied. Thanks!

Thanks for the confirmation!

Rafael

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web