[PATCH 1/3] acpi: Provide default GPE handler if the firmware doesn't

[PATCH 1/3] acpi: Provide default GPE handler if the firmware doesn't

[Matthew Garrett]

Firmware may support using GPEs for system wakeup without providing any
runtime GPE handlers. This prevents the use runtime power management on
these devices even if the hardware supports it. This patch adds support
for providing default GPE handlers if the firmware doesn't, allowing us
to implement runtime power management on machines where it would be
otherwise impossible.

This implementation adds basic support for Intel chipsets.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/pci/pci-acpi.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 119 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index dae90cc..7fff59c 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -879,6 +879,124 @@ static struct acpi_bus_type acpi_pci_bus = {
 	.find_bridge = acpi_pci_find_root_bridge,
 };
 
+static void acpi_pci_bus_notify(struct work_struct *work)
+{
+	acpi_handle handle = acpi_get_pci_rootbridge_handle(0, 0);
+	struct acpi_device *acpi_dev;
+	struct acpi_pci_root *root;
+
+	kfree(work);
+
+	if (acpi_bus_get_device(handle, &acpi_dev))
+		return;
+
+	root = acpi_driver_data(acpi_dev);
+
+	if (!root)
+		return;
+
+	pci_pme_wakeup_bus(root->bus);
+}
+
+
+static void acpi_pci_uhci_notify(struct work_struct *work)
+{
+	struct pci_dev *pci_dev = NULL;
+
+	kfree(work);
+
+	while ((pci_dev = pci_get_class(PCI_CLASS_SERIAL_USB_UHCI, pci_dev))) {
+		pci_pme_wakeup(pci_dev);
+		pci_dev_put(pci_dev);
+	}
+}
+
+static void acpi_pci_audio_or_uhci_notify(struct work_struct *work)
+{
+	/* FIXME: Need to work out how to check what this actually maps to */
+	return acpi_pci_uhci_notify(work);
+}
+
+static acpi_status acpi_pci_pme_notify(void *context)
+{
+	struct work_struct *work = kzalloc(sizeof(struct work_struct),
+					   GFP_ATOMIC);
+
+	if (work) {
+		INIT_WORK(work, context);
+		schedule_work(work);
+	}
+	return AE_OK;
+}
+
+struct gpe_fixup {
+	int gpe;
+	work_func_t callback;
+};
+
+static struct gpe_fixup acpi_pci_intel_gpes[] = {
+	{ 0x3, &acpi_pci_uhci_notify, },
+	{ 0x4, &acpi_pci_uhci_notify, },
+	{ 0x5, &acpi_pci_audio_or_uhci_notify, },
+	{ 0xb, &acpi_pci_bus_notify, },
+	{ 0xc, &acpi_pci_uhci_notify, },
+	{ 0xd, &acpi_pci_bus_notify, },
+	{ 0xe, &acpi_pci_uhci_notify, },
+	{ 0x20, &acpi_pci_uhci_notify, },
+	{ 0, NULL, },
+};
+
+static void __init acpi_pci_install_gpe(u32 gpe_number,
+					work_func_t callback, int force)
+{
+	acpi_status status;
+	acpi_event_status event_status;
+
+	status = acpi_get_gpe_status(NULL, gpe_number, ACPI_NOT_ISR,
+				     &event_status);
+
+	/* Don't override existing methods by default */
+	if (!force && (event_status & ACPI_EVENT_FLAG_HANDLE))
+		return;
+
+	status = acpi_install_gpe_handler(NULL, gpe_number,
+					  ACPI_GPE_LEVEL_TRIGGERED,
+					  acpi_pci_pme_notify, callback);
+
+	if (ACPI_FAILURE(status) && status != AE_ALREADY_EXISTS) {
+		printk(KERN_INFO "Unable to install GPE fixup for %x\n",
+		       gpe_number);
+		acpi_remove_gpe_handler(NULL, gpe_number, acpi_pci_pme_notify);
+	}
+}
+
+static int __init acpi_pci_gpe_fixups(void)
+{
+	struct pci_dev *lpc;
+	struct gpe_fixup *fixups;
+	int i;
+
+	lpc = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
+
+	if (!lpc)
+		return -ENODEV;
+
+	switch(lpc->vendor) {
+	case PCI_VENDOR_ID_INTEL:
+		fixups = acpi_pci_intel_gpes;
+		break;
+	}
+
+	if (fixups)
+		for (i=0; fixups[i].callback != NULL; i++)
+			acpi_pci_install_gpe(fixups[i].gpe,
+					     fixups[i].callback, false);
+
+	pci_dev_put(lpc);
+
+	return 0;
+}
+
 static int __init acpi_pci_init(void)
 {
 	int ret;
@@ -900,3 +1018,4 @@ static int __init acpi_pci_init(void)
 	return 0;
 }
 arch_initcall(acpi_pci_init);
+late_initcall(acpi_pci_gpe_fixups);
-- 
1.6.5.2

[PATCH 2/3] acpica: Add support for unregistering ACPI GPE methods

[Matthew Garrett]

Some vendors ship firmware with untested GPE handlers. The presence of
these handlers may be sufficient to cause the OS to assume that runtime
power management of the associated devices is possible, potentially
resulting in devices being powered down and no wakeup events ever being
generated.

The simplest solution is to add support for the OS to unhook these GPE
methods when it knows that they are broken. Add a function to the ACPICA
code to allow this.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/acpica/evxface.c |   69 +++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpixf.h         |    3 ++
 2 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/acpica/evxface.c b/drivers/acpi/acpica/evxface.c
index 90cdeae..33b1dc0 100644
--- a/drivers/acpi/acpica/evxface.c
+++ b/drivers/acpi/acpica/evxface.c
@@ -736,6 +736,75 @@ ACPI_EXPORT_SYMBOL(acpi_remove_gpe_handler)
 
 /*******************************************************************************
  *
+ * FUNCTION:    acpi_remove_gpe_method
+ *
+ * PARAMETERS:  gpe_device      - Namespace node for the GPE (NULL for FADT
+ *                                defined GPEs)
+ *              gpe_number      - The event to remove a handler
+ *
+ * RETURN:      Status
+ *
+ * DESCRIPTION: Remove the method for a General Purpose acpi_event.
+ *
+ ******************************************************************************/
+acpi_status
+acpi_remove_gpe_method(acpi_handle gpe_device, u32 gpe_number)
+{
+	struct acpi_gpe_event_info *gpe_event_info;
+	acpi_status status;
+	acpi_cpu_flags flags;
+
+	ACPI_FUNCTION_TRACE(acpi_remove_gpe_method);
+
+	/* Parameter validation */
+
+	status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS);
+	if (ACPI_FAILURE(status)) {
+		return_ACPI_STATUS(status);
+	}
+
+	/* Ensure that we have a valid GPE number */
+
+	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
+	if (!gpe_event_info) {
+		status = AE_BAD_PARAMETER;
+		goto unlock_and_exit;
+	}
+
+	/* Make sure that a method is indeed installed */
+
+	if ((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) !=
+	    ACPI_GPE_DISPATCH_METHOD) {
+		status = AE_NOT_EXIST;
+		goto unlock_and_exit;
+	}
+
+	/* Make sure all deferred tasks are completed */
+
+	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
+	acpi_os_wait_events_complete(NULL);
+	status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS);
+	if (ACPI_FAILURE(status)) {
+		return_ACPI_STATUS(status);
+	}
+
+	/* Remove the method */
+
+	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	gpe_event_info->dispatch.method_node = NULL;
+	gpe_event_info->flags &= ~ACPI_GPE_DISPATCH_METHOD;
+
+	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+
+      unlock_and_exit:
+	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
+	return_ACPI_STATUS(status);
+}
+
+ACPI_EXPORT_SYMBOL(acpi_remove_gpe_method)
+
+/*******************************************************************************
+ *
  * FUNCTION:    acpi_acquire_global_lock
  *
  * PARAMETERS:  Timeout         - How long the caller is willing to wait
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index bb3c975..f89aa7b 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -258,6 +258,9 @@ acpi_status
 acpi_remove_gpe_handler(acpi_handle gpe_device,
 			u32 gpe_number, acpi_event_handler address);
 
+acpi_status
+acpi_remove_gpe_method(acpi_handle gpe_device, u32 gpe_number);
+
 #ifdef ACPI_FUTURE_USAGE
 acpi_status acpi_install_exception_handler(acpi_exception_handler handler);
 #endif
-- 
1.6.5.2

[PATCH 3/3] pci: Override gpe 0xd on Dell machines

[Matthew Garrett]

Dell's mobile BIOS implements a platform-level check for the bus device
that triggered the PME# GPE. The GPE triggers an SMI, which in turn
causes notifications to be sent to only the devices that require it.

Unfortunately, this SMI is incorrect and fails to generate any notifications.
The wakeup device's PME line thus remains asserted, triggering another GPE
and executing the SMI again. As no notification will ever be sent, the
device is never powered up and the PME bit never cleared. This is less than
optimal.

This issue was supposed to be fixed in systems with an OEM table ID of M09
or higher - unfortunately, it seems that that isn't the case. So, for now,
disable the GPE and allow it to be replaced with an OS-level handler (on
Intel) or ignored (on other chipsets)

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/pci/pci-acpi.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 7fff59c..1606d06 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -14,6 +14,7 @@
 #include <linux/pci-aspm.h>
 #include <acpi/acpi.h>
 #include <acpi/acpi_bus.h>
+#include <linux/dmi.h>
 
 #include <linux/pci-acpi.h>
 #include "pci.h"
@@ -970,12 +971,35 @@ static void __init acpi_pci_install_gpe(u32 gpe_number,
 	}
 }
 
+static int __init acpi_gpe_remove_0xd_method(const struct dmi_system_id *d)
+{
+	acpi_remove_gpe_method(NULL, 0xd);
+	return 0;
+}
+
+static struct dmi_system_id gpe_dmi_table[] __initdata = {
+	/*
+	 * Dells have a broken handler for GPE 0xd. Remove it rather than
+	 * trusting it
+	 */
+	{
+		.callback = acpi_gpe_remove_0xd_method,
+		.ident = "Dell",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Dell Inc."),
+		},
+	},
+	{}
+};
+
 static int __init acpi_pci_gpe_fixups(void)
 {
 	struct pci_dev *lpc;
 	struct gpe_fixup *fixups;
 	int i;
 
+	dmi_check_system(gpe_dmi_table);
+
 	lpc = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
 
 	if (!lpc)
-- 
1.6.5.2

Re: [PATCH 2/3] acpica: Add support for unregistering ACPI GPE methods

[Rafael J. Wysocki]

On Monday 09 November 2009, Matthew Garrett wrote:
> Some vendors ship firmware with untested GPE handlers. The presence of
> these handlers may be sufficient to cause the OS to assume that runtime
> power management of the associated devices is possible, potentially
> resulting in devices being powered down and no wakeup events ever being
> generated.
> 
> The simplest solution is to add support for the OS to unhook these GPE
> methods when it knows that they are broken. Add a function to the ACPICA
> code to allow this.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
>  drivers/acpi/acpica/evxface.c |   69 +++++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpixf.h         |    3 ++
>  2 files changed, 72 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/evxface.c b/drivers/acpi/acpica/evxface.c
> index 90cdeae..33b1dc0 100644
> --- a/drivers/acpi/acpica/evxface.c
> +++ b/drivers/acpi/acpica/evxface.c
> @@ -736,6 +736,75 @@ ACPI_EXPORT_SYMBOL(acpi_remove_gpe_handler)
>  
>  /*******************************************************************************
>   *
> + * FUNCTION:    acpi_remove_gpe_method
> + *
> + * PARAMETERS:  gpe_device      - Namespace node for the GPE (NULL for FADT
> + *                                defined GPEs)
> + *              gpe_number      - The event to remove a handler
> + *
> + * RETURN:      Status
> + *
> + * DESCRIPTION: Remove the method for a General Purpose acpi_event.
> + *
> + ******************************************************************************/
> +acpi_status
> +acpi_remove_gpe_method(acpi_handle gpe_device, u32 gpe_number)
> +{
> +	struct acpi_gpe_event_info *gpe_event_info;
> +	acpi_status status;
> +	acpi_cpu_flags flags;
> +
> +	ACPI_FUNCTION_TRACE(acpi_remove_gpe_method);
> +
> +	/* Parameter validation */
> +
> +	status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS);
> +	if (ACPI_FAILURE(status)) {
> +		return_ACPI_STATUS(status);
> +	}
> +
> +	/* Ensure that we have a valid GPE number */
> +
> +	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
> +	if (!gpe_event_info) {
> +		status = AE_BAD_PARAMETER;
> +		goto unlock_and_exit;
> +	}
> +
> +	/* Make sure that a method is indeed installed */
> +
> +	if ((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) !=
> +	    ACPI_GPE_DISPATCH_METHOD) {
> +		status = AE_NOT_EXIST;
> +		goto unlock_and_exit;
> +	}
> +
> +	/* Make sure all deferred tasks are completed */
> +
> +	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
> +	acpi_os_wait_events_complete(NULL);
> +	status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS);
> +	if (ACPI_FAILURE(status)) {
> +		return_ACPI_STATUS(status);
> +	}
> +
> +	/* Remove the method */
> +
> +	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
> +	gpe_event_info->dispatch.method_node = NULL;
> +	gpe_event_info->flags &= ~ACPI_GPE_DISPATCH_METHOD;
> +
> +	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
> +
> +      unlock_and_exit:
> +	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
> +	return_ACPI_STATUS(status);
> +}
> +
> +ACPI_EXPORT_SYMBOL(acpi_remove_gpe_method)
> +
> +/*******************************************************************************
> + *
>   * FUNCTION:    acpi_acquire_global_lock
>   *
>   * PARAMETERS:  Timeout         - How long the caller is willing to wait
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index bb3c975..f89aa7b 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -258,6 +258,9 @@ acpi_status
>  acpi_remove_gpe_handler(acpi_handle gpe_device,
>  			u32 gpe_number, acpi_event_handler address);
>  
> +acpi_status
> +acpi_remove_gpe_method(acpi_handle gpe_device, u32 gpe_number);
> +
>  #ifdef ACPI_FUTURE_USAGE
>  acpi_status acpi_install_exception_handler(acpi_exception_handler handler);
>  #endif
> 

Re: [PATCH 1/3] acpi: Provide default GPE handler if the firmware doesn't

[Rafael J. Wysocki]

On Monday 09 November 2009, Matthew Garrett wrote:
> Firmware may support using GPEs for system wakeup without providing any
> runtime GPE handlers. This prevents the use runtime power management on
> these devices even if the hardware supports it. This patch adds support
> for providing default GPE handlers if the firmware doesn't, allowing us
> to implement runtime power management on machines where it would be
> otherwise impossible.
> 
> This implementation adds basic support for Intel chipsets.

Do I think correctly it's based on top of the run-time PM patches?

> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  drivers/pci/pci-acpi.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 119 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index dae90cc..7fff59c 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -879,6 +879,124 @@ static struct acpi_bus_type acpi_pci_bus = {
>  	.find_bridge = acpi_pci_find_root_bridge,
>  };
>  
...
> +
> +static acpi_status acpi_pci_pme_notify(void *context)
> +{
> +	struct work_struct *work = kzalloc(sizeof(struct work_struct),
> +					   GFP_ATOMIC);

kzalloc(sizeof(*work), ...) would save you a few characters. :-)

> +
> +	if (work) {
> +		INIT_WORK(work, context);
> +		schedule_work(work);

Hmm.  Not sure if putting that into pm_wq wouldn't be better.

Thanks,
Rafael

Re: [PATCH 1/3] acpi: Provide default GPE handler if the firmware doesn't

[Matthew Garrett]

On Wed, Nov 11, 2009 at 10:31:57PM +0100, Rafael J. Wysocki wrote:

> Do I think correctly it's based on top of the run-time PM patches?

It's logically distinct, but doesn't really do anything useful without 
them.

> > +static acpi_status acpi_pci_pme_notify(void *context)
> > +{
> > +	struct work_struct *work = kzalloc(sizeof(struct work_struct),
> > +					   GFP_ATOMIC);
> 
> kzalloc(sizeof(*work), ...) would save you a few characters. :-)

Heh. True.

> > +
> > +	if (work) {
> > +		INIT_WORK(work, context);
> > +		schedule_work(work);
> 
> Hmm.  Not sure if putting that into pm_wq wouldn't be better.

Would probably work.

-- 
Matthew Garrett | mjg59@srcf.ucam.org