Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 27 Apr 2009 15:54:08 +0300
From:      Andriy Gapon <avg@freebsd.org>
To:        Fabian Keil <freebsd-listen@fabiankeil.de>
Cc:        freebsd-acpi@freebsd.org
Subject:   Re: run resume code only for S1-S4 states
Message-ID:  <49F5AAF0.9080607@freebsd.org>
In-Reply-To: <20090425102109.0520ce59@fabiankeil.de>
References:  <49DB639A.4090504@icyb.net.ua>	<49DCF5C2.60805@root.org>	<49DDF906.8090400@icyb.net.ua>	<49DF3CA4.1090309@freebsd.org>	<49E4B2A7.3020302@freebsd.org>	<49E61986.7040709@root.org>	<49E8AED0.1090008@freebsd.org>	<20090418125806.2a48b0a8@fabiankeil.de>	<49E9FFB0.6090707@root.org>	<49EC60C6.7000702@freebsd.org>	<49EC9D2F.8080701@root.org>	<49EDFBBA.1080504@freebsd.org>	<20090422183214.1e3372c6@fabiankeil.de>	<49F09A23.9080802@freebsd.org> <20090425102109.0520ce59@fabiankeil.de>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------050607000503030306050307
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

on 25/04/2009 11:21 Fabian Keil said the following:
> Sure. It turns out that the problem is unrelated to your patch.
> I can reproduce it with an unpatched kernel too, by once pressing
> the power button before the second core is started.
> 
> I probably did the same a few days ago, and forgot about it. Sorry.

Fabian,

thank you very much for the testing and the insight, this is very useful and
interesting.
I think that it might be that 'init' process in pre-natal state loses a signal
sent to it.

I decided to follow Nate's advice and exempt S5 from timeout policy (after all it
is possible to execute shutdown(8) multiple times and concurrently with any other
sleep request). With previous version of the patch once shutdown_nice() failed
once it was impossible to enter any sleep state ever. shutdown_nice failure is
quite exotic event, but as you have proven it is not impossible.

So the new patch is attached.


-- 
Andriy Gapon

--------------050607000503030306050307
Content-Type: text/plain;
 name="acpi_s5.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="acpi_s5.diff"

diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index 50b84a5..d8626b4 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -2482,6 +2482,18 @@ 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);
+
+    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);
+    }
+
     /* Re-entry once we're suspending is not allowed. */
     status = acpi_sleep_disable(sc);
     if (ACPI_FAILURE(status)) {
@@ -2502,92 +2514,74 @@ 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:
-	status = AcpiGetSleepTypeData(state, &TypeA, &TypeB);
-	if (status == AE_NOT_FOUND) {
-	    device_printf(sc->acpi_dev,
-			  "Sleep state S%d not supported by BIOS\n", state);
-	    break;
-	} else if (ACPI_FAILURE(status)) {
-	    device_printf(sc->acpi_dev, "AcpiGetSleepTypeData failed - %s\n",
-			  AcpiFormatException(status));
-	    break;
-	}
+    status = AcpiGetSleepTypeData(state, &TypeA, &TypeB);
+    if (status == AE_NOT_FOUND) {
+	device_printf(sc->acpi_dev,
+		      "Sleep state S%d not supported by BIOS\n", state);
+	goto backout;
+    } else if (ACPI_FAILURE(status)) {
+	device_printf(sc->acpi_dev, "AcpiGetSleepTypeData failed - %s\n",
+		      AcpiFormatException(status));
+	goto backout;
+    }
 
-	sc->acpi_sstate = state;
+    sc->acpi_sstate = state;
 
-	/* Enable any GPEs as appropriate and requested by the user. */
-	acpi_wake_prep_walk(state);
-	slp_state = ACPI_SS_GPE_SET;
+    /* Enable any GPEs as appropriate and requested by the user. */
+    acpi_wake_prep_walk(state);
+    slp_state = ACPI_SS_GPE_SET;
 
-	/*
-	 * Inform all devices that we are going to sleep.  If at least one
-	 * device fails, DEVICE_SUSPEND() automatically resumes the tree.
-	 *
-	 * XXX Note that a better two-pass approach with a 'veto' pass
-	 * followed by a "real thing" pass would be better, but the current
-	 * bus interface does not provide for this.
-	 */
-	if (DEVICE_SUSPEND(root_bus) != 0) {
-	    device_printf(sc->acpi_dev, "device_suspend failed\n");
-	    break;
-	}
-	slp_state = ACPI_SS_DEV_SUSPEND;
+    /*
+     * Inform all devices that we are going to sleep.  If at least one
+     * device fails, DEVICE_SUSPEND() automatically resumes the tree.
+     *
+     * XXX Note that a better two-pass approach with a 'veto' pass
+     * followed by a "real thing" pass would be better, but the current
+     * bus interface does not provide for this.
+     */
+    if (DEVICE_SUSPEND(root_bus) != 0) {
+	device_printf(sc->acpi_dev, "device_suspend failed\n");
+	goto backout;
+    }
+    slp_state = ACPI_SS_DEV_SUSPEND;
 
-	/* If testing device suspend only, back out of everything here. */
-	if (acpi_susp_bounce)
-	    break;
+    /* If testing device suspend only, back out of everything here. */
+    if (acpi_susp_bounce)
+	goto backout;
 
-	status = AcpiEnterSleepStatePrep(state);
-	if (ACPI_FAILURE(status)) {
-	    device_printf(sc->acpi_dev, "AcpiEnterSleepStatePrep failed - %s\n",
-			  AcpiFormatException(status));
-	    break;
-	}
-	slp_state = ACPI_SS_SLP_PREP;
+    status = AcpiEnterSleepStatePrep(state);
+    if (ACPI_FAILURE(status)) {
+	device_printf(sc->acpi_dev, "AcpiEnterSleepStatePrep failed - %s\n",
+		      AcpiFormatException(status));
+	goto backout;
+    }
+    slp_state = ACPI_SS_SLP_PREP;
 
-	if (sc->acpi_sleep_delay > 0)
-	    DELAY(sc->acpi_sleep_delay * 1000000);
+    if (sc->acpi_sleep_delay > 0)
+	DELAY(sc->acpi_sleep_delay * 1000000);
 
-	if (state != ACPI_STATE_S1) {
-	    acpi_sleep_machdep(sc, state);
+    if (state != ACPI_STATE_S1) {
+	acpi_sleep_machdep(sc, state);
 
-	    /* Re-enable ACPI hardware on wakeup from sleep state 4. */
-	    if (state == ACPI_STATE_S4)
-		AcpiEnable();
-	} else {
-	    ACPI_DISABLE_IRQS();
-	    status = AcpiEnterSleepState(state);
-	    if (ACPI_FAILURE(status)) {
-		device_printf(sc->acpi_dev, "AcpiEnterSleepState failed - %s\n",
-			      AcpiFormatException(status));
-		break;
-	    }
+	/* Re-enable ACPI hardware on wakeup from sleep state 4. */
+	if (state == ACPI_STATE_S4)
+	    AcpiEnable();
+    } else {
+	ACPI_DISABLE_IRQS();
+	status = AcpiEnterSleepState(state);
+	if (ACPI_FAILURE(status)) {
+	    device_printf(sc->acpi_dev, "AcpiEnterSleepState failed - %s\n",
+			  AcpiFormatException(status));
+	    goto backout;
 	}
-	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;
     }
+    slp_state = ACPI_SS_SLEPT;
 
     /*
      * Back out state according to how far along we got in the suspend
      * process.  This handles both the error and success cases.
      */
+backout:
     sc->acpi_next_sstate = 0;
     if (slp_state >= ACPI_SS_GPE_SET) {
 	acpi_wake_prep_walk(state);
@@ -2609,8 +2603,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())

--------------050607000503030306050307--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?49F5AAF0.9080607>