run resume code only for S1-S4 states
Andriy Gapon
avg at freebsd.org
Tue Apr 14 09:11:23 PDT 2009
Guys,
could you please review the attached patch?
Its main idea is to make control flow of acpi_EnterSleepState similar
to that of acpi_ReqSleepState: reject invalid state parameter immediately and
handle special S5 as early as possible. Primary purpose is to avoid running resume
code when it is not necessary - e.g. shutdown_nice() typically returns immediately
after initiating a graceful shutdown by sending a signal to init.
As such, S5 is handled right after checking/disabling re-entry.
switch becomes unneeded, because all remaining possibilities are grouped
into a single case. I decided to use do-while(0) statement in the place of the
switch for the following reasons:
1. minimize diff by preserving indentation
2. minimize diff by preserving control flow that depends on break statement
But I am not sure how this while(0) corresponds with style(9), I couldn't find any
reference in the manual page.
There is also a concern about calling shutdown_nice() outside of the Giant lock
and binding to CPU 0. I am not sure about the pre-requisites for this function.
John, maybe you could help me here?
--
Andriy Gapon
-------------- next part --------------
diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index 50b84a5..ac654d2 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -2482,6 +2482,9 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state)
ACPI_FUNCTION_TRACE_U32((char *)(uintptr_t)__func__, state);
+ if (state < ACPI_STATE_S1 || state > ACPI_STATE_S5)
+ return_ACPI_STATUS (AE_BAD_PARAMETER);
+
/* Re-entry once we're suspending is not allowed. */
status = acpi_sleep_disable(sc);
if (ACPI_FAILURE(status)) {
@@ -2489,6 +2492,15 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state)
return (status);
}
+ if (state == ACPI_STATE_S5) {
+ /*
+ * Shut down cleanly and power off. This will call us back through the
+ * shutdown handlers.
+ */
+ shutdown_nice(RB_POWEROFF);
+ return_ACPI_STATUS (AE_OK);
+ }
+
#ifdef SMP
thread_lock(curthread);
sched_bind(curthread, 0);
@@ -2502,11 +2514,7 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state)
mtx_lock(&Giant);
slp_state = ACPI_SS_NONE;
- switch (state) {
- case ACPI_STATE_S1:
- case ACPI_STATE_S2:
- case ACPI_STATE_S3:
- case ACPI_STATE_S4:
+ do {
status = AcpiGetSleepTypeData(state, &TypeA, &TypeB);
if (status == AE_NOT_FOUND) {
device_printf(sc->acpi_dev,
@@ -2569,20 +2577,7 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state)
}
}
slp_state = ACPI_SS_SLEPT;
- break;
- case ACPI_STATE_S5:
- /*
- * Shut down cleanly and power off. This will call us back through the
- * shutdown handlers.
- */
- shutdown_nice(RB_POWEROFF);
- status = AE_OK;
- break;
- case ACPI_STATE_S0:
- default:
- status = AE_BAD_PARAMETER;
- break;
- }
+ } while (0);
/*
* Back out state according to how far along we got in the suspend
@@ -2609,8 +2604,7 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state)
#endif
/* Allow another sleep request after a while. */
- if (state != ACPI_STATE_S5)
- timeout(acpi_sleep_enable, sc, hz * ACPI_MINIMUM_AWAKETIME);
+ timeout(acpi_sleep_enable, sc, hz * ACPI_MINIMUM_AWAKETIME);
/* Run /etc/rc.resume after we are back. */
if (devctl_process_running())
More information about the freebsd-acpi
mailing list