kern/150517: [acpi] acpi_ec does not work properly on Lenovo S10[e] (due to dynamic switching to polled mode)

David Naylor naylor.b.david at gmail.com
Mon Sep 13 07:20:07 UTC 2010


>Number:         150517
>Category:       kern
>Synopsis:       [acpi] acpi_ec does not work properly on Lenovo S10[e] (due to dynamic switching to polled mode)
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Sep 13 07:20:06 UTC 2010
>Closed-Date:
>Last-Modified:
>Originator:     David Naylor
>Release:        FreeBSD-current
>Organization:
Private
>Environment:
FreeBSD dragonpico.dg 9.0-CURRENT FreeBSD 9.0-CURRENT #0: Thu Aug  5 16:21:50 SAST 2010     root at dragon.dg:/tmp/home/freebsd9/src/sys/LENOVOS10  i386
>Description:
The Lenovo S10's EC is a bit faulty and regularly resets itself.  This causes FreeBSD's handler to switch to polling mode which does not work the the EC.  

The problem appears to be triggered when commands are given to the EC in rapid succession.  This is too much for the EC to handle and it resets itself.  

This results in battery level to no longer work, and `shutdown -p`.  The console also gets spammed with error messages from ACPI.  

(A work around [and possible solution] is given below)
(This is an issue with all BIOS versions published by Lenovo)
>How-To-Repeat:
Run FreeBSD (generic) on a Lenovo S10[e]
>Fix:
WORK-A-ROUND:
Attached is a patch that works around the problem.  It disables switching to polling mode and forces a time to be waited before issuing more commands to the EC.  In conjunction to the patch I need in /boot/loader.conf:

# ACPI Debug options
debug.acpi.ec.delay="200"
debug.acpi.ec.gpe="1"
debug.acpi.ec.timeout="200"

Although this works for me others have reported failure.  Also the laptop fails to shutdown properly on occasion (actual power off sometimes does not happen).  

SOLUTION:
Looking at the Linux code it appears it detects when the EC reset itself and will reissue the command.  FreeBSD's code does not appear to do that.  

Patch attached with submission follows:

--- acpi_ec.c~	2009-06-17 21:14:48.000000000 +0200
+++ acpi_ec.c	2009-06-19 14:40:02.000000000 +0200
@@ -168,7 +168,7 @@
 #define EC_LOCK_TIMEOUT	1000
 
 /* Default delay in microseconds between each run of the status polling loop. */
-#define EC_POLL_DELAY	5
+#define EC_POLL_DELAY	100
 
 /* Total time in ms spent waiting for a response from EC. */
 #define EC_TIMEOUT	750
@@ -184,13 +184,21 @@
 SYSCTL_DECL(_debug_acpi);
 SYSCTL_NODE(_debug_acpi, OID_AUTO, ec, CTLFLAG_RD, NULL, "EC debugging");
 
-static int	ec_burst_mode;
+static int	ec_burst_mode = FALSE;
 TUNABLE_INT("debug.acpi.ec.burst", &ec_burst_mode);
-SYSCTL_INT(_debug_acpi_ec, OID_AUTO, burst, CTLFLAG_RW, &ec_burst_mode, 0,
+SYSCTL_INT(_debug_acpi_ec, OID_AUTO, burst, CTLFLAG_RW, &ec_burst_mode, FALSE,
     "Enable use of burst mode (faster for nearly all systems)");
-static int	ec_polled_mode;
+static int	ec_delay = 0;
+TUNABLE_INT("debug.acpi.ec.delay", &ec_delay);
+SYSCTL_INT(_debug_acpi_ec, OID_AUTO, delay, CTLFLAG_RW, &ec_delay, 0,
+    "Delay after waiting for responce (GPE and polled mode)");
+static int	ec_gpe_mode = FALSE;
+TUNABLE_INT("debug.acpi.ec.gpe", &ec_gpe_mode);
+SYSCTL_INT(_debug_acpi_ec, OID_AUTO, gpe, CTLFLAG_RW, &ec_gpe_mode, FALSE,
+    "Disable adaptive GPE switching (to polled mode)");
+static int	ec_polled_mode = FALSE;
 TUNABLE_INT("debug.acpi.ec.polled", &ec_polled_mode);
-SYSCTL_INT(_debug_acpi_ec, OID_AUTO, polled, CTLFLAG_RW, &ec_polled_mode, 0,
+SYSCTL_INT(_debug_acpi_ec, OID_AUTO, polled, CTLFLAG_RW, &ec_polled_mode, FALSE,
     "Force use of polled mode (only if interrupt mode doesn't work)");
 static int	ec_timeout = EC_TIMEOUT;
 TUNABLE_INT("debug.acpi.ec.timeout", &ec_timeout);
@@ -794,6 +802,7 @@
     EC_STATUS ec_status;
 
     status = AE_NO_HARDWARE_RESPONSE;
+
     ec_status = EC_GET_CSR(sc);
     if (sc->ec_burstactive && !(ec_status & EC_FLAG_BURST_MODE)) {
 	CTR1(KTR_ACPI, "ec burst disabled in waitevent (%s)", msg);
@@ -810,56 +819,36 @@
 EcWaitEvent(struct acpi_ec_softc *sc, EC_EVENT Event, u_int gen_count)
 {
     ACPI_STATUS	Status;
-    int		count, i, slp_ival;
+    int		count, i, req_ticks, cur_ticks;
 
     ACPI_SERIAL_ASSERT(ec);
     Status = AE_NO_HARDWARE_RESPONSE;
     int need_poll = cold || rebooting || ec_polled_mode || sc->ec_suspending;
-    /*
-     * The main CPU should be much faster than the EC.  So the status should
-     * be "not ready" when we start waiting.  But if the main CPU is really
-     * slow, it's possible we see the current "ready" response.  Since that
-     * can't be distinguished from the previous response in polled mode,
-     * this is a potential issue.  We really should have interrupts enabled
-     * during boot so there is no ambiguity in polled mode.
-     *
-     * If this occurs, we add an additional delay before actually entering
-     * the status checking loop, hopefully to allow the EC to go to work
-     * and produce a non-stale status.
-     */
-    if (need_poll) {
-	static int	once;
-
-	if (EcCheckStatus(sc, "pre-check", Event) == AE_OK) {
-	    if (!once) {
-		device_printf(sc->ec_dev,
-		    "warning: EC done before starting event wait\n");
-		once = 1;
-	    }
-	    AcpiOsStall(10);
-	}
-    }
 
     /* Wait for event by polling or GPE (interrupt). */
     if (need_poll) {
 	count = (ec_timeout * 1000) / EC_POLL_DELAY;
 	if (count == 0)
 	    count = 1;
+
+	/* The EC is slow, give it some time to catch up to us */
+	AcpiOsStall(100);
 	for (i = 0; i < count; i++) {
 	    Status = EcCheckStatus(sc, "poll", Event);
 	    if (Status == AE_OK)
 		break;
 	    AcpiOsStall(EC_POLL_DELAY);
 	}
+
+	if (Status != AE_OK)
+	    device_printf(sc->ec_dev, "wait timed out [polling mode]\n");
     } else {
-	slp_ival = hz / 1000;
-	if (slp_ival != 0) {
-	    count = ec_timeout;
-	} else {
-	    /* hz has less than 1 ms resolution so scale timeout. */
-	    slp_ival = 1;
-	    count = ec_timeout / (1000 / hz);
-	}
+	/* How many ticks should we sleep for (max) */
+	req_ticks = hz < 1000 ? (ec_timeout * hz) / 1000
+	    : (ec_timeout * 1000) / hz;
+	/* Make sure we sleep for at least one tick, from now */
+	cur_ticks = (volatile int)ticks;
+	req_ticks = cur_ticks + (req_ticks ? req_ticks : 1) + 1;
 
 	/*
 	 * Wait for the GPE to signal the status changed, checking the
@@ -867,38 +856,42 @@
 	 * GPE for an event we're not interested in here (i.e., SCI for
 	 * EC query).
 	 */
-	for (i = 0; i < count; i++) {
-	    if (gen_count != sc->ec_gencount) {
-		/*
-		 * Record new generation count.  It's possible the GPE was
-		 * just to notify us that a query is needed and we need to
-		 * wait for a second GPE to signal the completion of the
-		 * event we are actually waiting for.
-		 */
-		gen_count = sc->ec_gencount;
-		Status = EcCheckStatus(sc, "sleep", Event);
-		if (Status == AE_OK)
-		    break;
-	    }
-	    tsleep(&sc->ec_gencount, PZERO, "ecgpe", slp_ival);
+	while ((int)(req_ticks - cur_ticks) > 0) {
+	    /* If we have not received a signal then wait for one */
+	    if (gen_count == sc->ec_gencount)
+		tsleep(&sc->ec_gencount, PZERO, "ecgpe", req_ticks - cur_ticks);
+
+	    /*
+	     * Record new generation count.  It's possible the GPE was
+	     * just to notify us that a query is needed and we need to
+	     * wait for a second GPE to signal the completion of the
+	     * event we are actually waiting for.
+	     */
+	    gen_count = sc->ec_gencount;
+	    Status = EcCheckStatus(sc, "sleep", Event);
+	    if (Status == AE_OK)
+	        break;
+
+	    /* Update current tick (so we always have a consistant value */
+	    cur_ticks = (volatile int)ticks;
 	}
 
 	/*
-	 * We finished waiting for the GPE and it never arrived.  Try to
-	 * read the register once and trust whatever value we got.  This is
-	 * the best we can do at this point.  Then, force polled mode on
-	 * since this system doesn't appear to generate GPEs.
+	 * We finished waiting for the GPE and it never arrived.  The register
+	 * has been read on a timeout so no need to re-read it.  Force polled
+	 * mode on since this system doesn't appear to generate GPEs.
 	 */
 	if (Status != AE_OK) {
-	    Status = EcCheckStatus(sc, "sleep_end", Event);
-	    device_printf(sc->ec_dev,
-		"wait timed out (%sresponse), forcing polled mode\n",
-		Status == AE_OK ? "" : "no ");
-	    ec_polled_mode = TRUE;
+	    device_printf(sc->ec_dev, "wait timed out [GPE mode]%s\n",
+		ec_gpe_mode ? "" : ", forcing polled mode");
+	    ec_polled_mode = TRUE && !ec_gpe_mode;
 	}
     }
     if (Status != AE_OK)
 	    CTR0(KTR_ACPI, "error: ec wait timed out");
+    else if (ec_delay);
+	/* Give the EC a chance to recover from all its hard work!!! */
+	AcpiOsStall(ec_delay);
     return (Status);
 }
 


>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list