Skip to contentSkip to conversation

ACPICA/ACPI: clean up power button event management during resume

PATCH In review Bjorn Helgaas 2 participants 5 messages Jun 19, 2009 – Jun 20, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover ACPICA/ACPI: clean up power button event management during resume

This mini-series moves a little resume-path code from the host OS (Linux in this case) into the ACPI CA.

When resuming, OSPM must clear GPEs and power button events to prevent taking spurious events when interrupts are enabled. Linux currently clears these events itself, but this series adds code to do it in the ACPICA (in acpi_leave_sleep_state_prep(), which is already supposed to be called before enabling interrupts). This should be safe without any host OS changes.

The last patch takes out the event clearing from Linux and obviously depends on the previous ACPICA patches. It's safe to drop this one (no harm in clearing the events twice). I can easily post it again later.

---

Bjorn Helgaas (3):
ACPICA: Use fixed event wrappers to enable/disable/clear
ACPICA: Clear GPEs and power button events during wakeup
ACPI: remove clearing of events on resume, now that ACPI CA does it

 drivers/acpi/acpica/evevent.c |   13 +++----------
 drivers/acpi/acpica/hwsleep.c |   24 +++++++-----------------
 drivers/acpi/sleep.c          |   14 --------------
 3 files changed, 10 insertions(+), 41 deletions(-)
Signature · 1 line
Bjorn
#1 lore raw

PATCH 1/3 ACPICA: Use fixed event wrappers to enable/disable/clear

Use acpi_enable_event(), acpi_disable_event(), and acpi_clear_event() instead of writing the register directly.

This patch may be used under either the GPL v2 or the BSD-style license used for the Intel ACPICA.

---
 drivers/acpi/acpica/evevent.c |   13 +++----------
 drivers/acpi/acpica/hwsleep.c |   11 ++---------
 2 files changed, 5 insertions(+), 19 deletions(-)
drivers/acpi/acpica/evevent.c +3 -10
index cd55c77..5df6af7 100644
@@ -203,10 +203,7 @@ static acpi_status acpi_ev_fixed_event_initialize(void)
 		/* Disable the fixed event */
 
 		if (acpi_gbl_fixed_event_info[i].enable_register_id != 0xFF) {
-			status =
-			    acpi_write_bit_register(acpi_gbl_fixed_event_info
-						    [i].enable_register_id,
-						    ACPI_DISABLE_EVENT);
+			status = acpi_disable_event(i, 0);
 			if (ACPI_FAILURE(status)) {
 				return (status);
 			}
@@ -288,18 +285,14 @@ static u32 acpi_ev_fixed_event_dispatch(u32 event)
 	ACPI_FUNCTION_ENTRY();
 
 	/* Clear the status bit */
-
-	(void)acpi_write_bit_register(acpi_gbl_fixed_event_info[event].
-				      status_register_id, ACPI_CLEAR_STATUS);
+	acpi_clear_event(event);
 
 	/*
 	 * Make sure we've got a handler. If not, report an error. The event is
 	 * disabled to prevent further interrupts.
 	 */
 	if (NULL == acpi_gbl_fixed_event_handlers[event].handler) {
-		(void)acpi_write_bit_register(acpi_gbl_fixed_event_info[event].
-					      enable_register_id,
-					      ACPI_DISABLE_EVENT);
+		acpi_disable_event(event, 0);
 
 		ACPI_ERROR((AE_INFO,
 			    "No installed handler for fixed event [%08X]",
drivers/acpi/acpica/hwsleep.c +2 -9
index db307a3..ea51ee6 100644
@@ -610,15 +610,8 @@ acpi_status acpi_leave_sleep_state(u8 sleep_state)
 
 	/* Enable power button */
 
-	(void)
-	    acpi_write_bit_register(acpi_gbl_fixed_event_info
-			      [ACPI_EVENT_POWER_BUTTON].
-			      enable_register_id, ACPI_ENABLE_EVENT);
-
-	(void)
-	    acpi_write_bit_register(acpi_gbl_fixed_event_info
-			      [ACPI_EVENT_POWER_BUTTON].
-			      status_register_id, ACPI_CLEAR_STATUS);
+	acpi_enable_event(ACPI_EVENT_POWER_BUTTON, 0);
+	acpi_clear_event(ACPI_EVENT_POWER_BUTTON);
 
 	arg.integer.value = ACPI_SST_WORKING;
 	status = acpi_evaluate_object(NULL, METHOD_NAME__SST, &arg_list, NULL);
#2 lore raw

PATCH 2/3 ACPICA: Clear GPEs and power button events during wakeup

In reply to Bjorn Helgaas

Per spec section 4.7.2.2.1.1, OSPM should clear power button status when waking the system. This must be done before enabling interrupts to prevent spurious power button events.

Linux currently clears these events in acpi_suspend_enter() just after calling acpi_leave_sleep_state_prep(). Other OSes should be doing something similar. But it seems more robust to do this in the CA.

The clear in acpi_leave_sleep_state() should be unnecessary.

Thanks to Zhao Yakui for patiently educating me about this.

This patch may be used under either the GPL v2 or the BSD-style license used for the Intel ACPICA.

---
 drivers/acpi/acpica/hwsleep.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)
drivers/acpi/acpica/hwsleep.c +6 -9
index ea51ee6..283e872 100644
@@ -534,6 +534,12 @@ acpi_status acpi_leave_sleep_state_prep(u8 sleep_state)
 			ACPI_EXCEPTION((AE_INFO, status, "During Method _BFS"));
 		}
 	}
+
+	/* Clear any pending events before enabling interrupts */
+
+	acpi_hw_disable_all_gpes();
+	acpi_clear_event(ACPI_EVENT_POWER_BUTTON);
+
 	return_ACPI_STATUS(status);
 }
 
@@ -578,15 +584,7 @@ acpi_status acpi_leave_sleep_state(u8 sleep_state)
 	/*
 	 * GPEs must be enabled before _WAK is called as GPEs
 	 * might get fired there
-	 *
-	 * Restore the GPEs:
-	 * 1) Disable/Clear all GPEs
-	 * 2) Enable all runtime GPEs
 	 */
-	status = acpi_hw_disable_all_gpes();
-	if (ACPI_FAILURE(status)) {
-		return_ACPI_STATUS(status);
-	}
 	status = acpi_hw_enable_all_runtime_gpes();
 	if (ACPI_FAILURE(status)) {
 		return_ACPI_STATUS(status);
@@ -611,7 +609,6 @@ acpi_status acpi_leave_sleep_state(u8 sleep_state)
 	/* Enable power button */
 
 	acpi_enable_event(ACPI_EVENT_POWER_BUTTON, 0);
-	acpi_clear_event(ACPI_EVENT_POWER_BUTTON);
 
 	arg.integer.value = ACPI_SST_WORKING;
 	status = acpi_evaluate_object(NULL, METHOD_NAME__SST, &arg_list, NULL);
#3 lore raw

PATCH 3/3 ACPI: remove clearing of events on resume, now that ACPI CA does it

In reply to Bjorn Helgaas

The ACPI CA now clears GPEs and power button events in acpi_leave_sleep_state_prep(), so there's no need for us to do it again.

---
 drivers/acpi/sleep.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)
drivers/acpi/sleep.c +0 -14
index 01574a0..bf2e2e1 100644
@@ -257,20 +257,6 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
 	/* Reprogram control registers and execute _BFS */
 	acpi_leave_sleep_state_prep(acpi_state);
 
-	/* ACPI 3.0 specs (P62) says that it's the responsibility
-	 * of the OSPM to clear the status bit [ implying that the
-	 * POWER_BUTTON event should not reach userspace ]
-	 */
-	if (ACPI_SUCCESS(status) && (acpi_state == ACPI_STATE_S3))
-		acpi_clear_event(ACPI_EVENT_POWER_BUTTON);
-
-	/*
-	 * Disable and clear GPE status before interrupt is enabled. Some GPEs
-	 * (like wakeup GPE) haven't handler, this can avoid such GPE misfire.
-	 * acpi_leave_sleep_state will reenable specific GPEs later
-	 */
-	acpi_disable_all_gpes();
-
 	local_irq_restore(flags);
 	printk(KERN_DEBUG "Back to C!\n");
 
reviewer
#4 lore raw
In reply to Bjorn Helgaas

series applied to acpi-test

however, i will not push this series upstream as it is... will wait for the patches to flow from acpica.

thanks,
Len Brown, Intel Open Source Technology Center

Quoted 35 lines · Bjorn Helgaas

This mini-series moves a little resume-path code from the host OS (Linux in this case) into the ACPI CA.

When resuming, OSPM must clear GPEs and power button events to prevent taking spurious events when interrupts are enabled. Linux currently clears these events itself, but this series adds code to do it in the ACPICA (in acpi_leave_sleep_state_prep(), which is already supposed to be called before enabling interrupts). This should be safe without any host OS changes.

The last patch takes out the event clearing from Linux and obviously depends on the previous ACPICA patches. It's safe to drop this one (no harm in clearing the events twice). I can easily post it again later.

---

Bjorn Helgaas (3):
ACPICA: Use fixed event wrappers to enable/disable/clear
ACPICA: Clear GPEs and power button events during wakeup
ACPI: remove clearing of events on resume, now that ACPI CA does it

 drivers/acpi/acpica/evevent.c |   13 +++----------
 drivers/acpi/acpica/hwsleep.c |   24 +++++++-----------------
 drivers/acpi/sleep.c          |   14 --------------
 3 files changed, 10 insertions(+), 41 deletions(-)
Signature · 5 lines
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html