acpi_ec: changes in communication with hardware

Short description of the changes:
- attempt to retry some commands for which it is possible (read, query)
- always make a short sleep before checking EC status in polled mode
- periodically poll EC status in interrupt mode
- change logic for detecting broken interrupt delivery and falling back
  to polled mode
- check that EC is ready for input before starting a new command, wait
  if necessary

This commit is based on the original patch by David Naylor.

PR:		kern/150517
Submitted by:	David Naylor <naylor.b.david@gmail.com>
Reviewed by:	jkim
MFC after:	3 weeks

1 files changed, 89 insertions, 83 deletions

diff --git a/sys/dev/acpica/acpi_ec.c b/sys/dev/acpica/acpi_ec.c
index 2c18972..f864983 100644
--- a/sys/dev/acpica/acpi_ec.c
+++ b/sys/dev/acpica/acpi_ec.c
@@ -153,7 +153,7 @@ struct acpi_ec_softc {
     int			ec_glkhandle;
     int			ec_burstactive;
     int			ec_sci_pend;
-    u_int		ec_gencount;
+    volatile u_int	ec_gencount;
     int			ec_suspending;
 };
 
@@ -165,7 +165,7 @@ struct acpi_ec_softc {
 #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	50
 
 /* Total time in ms spent waiting for a response from EC. */
 #define EC_TIMEOUT	750
@@ -599,12 +599,32 @@ acpi_ec_write_method(device_t dev, u_int addr, UINT64 val, int width)
     return (0);
 }
 
+static ACPI_STATUS
+EcCheckStatus(struct acpi_ec_softc *sc, const char *msg, EC_EVENT event)
+{
+    ACPI_STATUS status;
+    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);
+	sc->ec_burstactive = FALSE;
+    }
+    if (EVENT_READY(event, ec_status)) {
+	CTR2(KTR_ACPI, "ec %s wait ready, status %#x", msg, ec_status);
+	status = AE_OK;
+    }
+    return (status);
+}
+
 static void
 EcGpeQueryHandler(void *Context)
 {
     struct acpi_ec_softc	*sc = (struct acpi_ec_softc *)Context;
     UINT8			Data;
     ACPI_STATUS			Status;
+    int				retry;
     char			qxx[5];
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
@@ -625,7 +645,16 @@ EcGpeQueryHandler(void *Context)
      * that may arise from running the query from causing another query
      * to be queued, we clear the pending flag only after running it.
      */
-    Status = EcCommand(sc, EC_COMMAND_QUERY);
+    for (retry = 0; retry < 2; retry++) {
+	Status = EcCommand(sc, EC_COMMAND_QUERY);
+	if (ACPI_SUCCESS(Status))
+	    break;
+	if (EcCheckStatus(sc, "retr_check",
+	    EC_EVENT_INPUT_BUFFER_EMPTY) == AE_OK)
+	    continue;
+	else
+	    break;
+    }
     sc->ec_sci_pend = FALSE;
     if (ACPI_FAILURE(Status)) {
 	EcUnlock(sc);
@@ -678,7 +707,7 @@ EcGpeHandler(void *Context)
      * address and then data values.)
      */
     atomic_add_int(&sc->ec_gencount, 1);
-    wakeup(&sc->ec_gencount);
+    wakeup(&sc);
 
     /*
      * If the EC_SCI bit of the status register is set, queue a query handler.
@@ -789,68 +818,27 @@ EcSpaceHandler(UINT32 Function, ACPI_PHYSICAL_ADDRESS Address, UINT32 Width,
 }
 
 static ACPI_STATUS
-EcCheckStatus(struct acpi_ec_softc *sc, const char *msg, EC_EVENT event)
-{
-    ACPI_STATUS status;
-    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);
-	sc->ec_burstactive = FALSE;
-    }
-    if (EVENT_READY(event, ec_status)) {
-	CTR2(KTR_ACPI, "ec %s wait ready, status %#x", msg, ec_status);
-	status = AE_OK;
-    }
-    return (status);
-}
-
-static ACPI_STATUS
 EcWaitEvent(struct acpi_ec_softc *sc, EC_EVENT Event, u_int gen_count)
 {
+    static int	no_intr = 0;
     ACPI_STATUS	Status;
-    int		count, i, slp_ival;
+    int		count, i, need_poll, slp_ival;
 
     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);
-	}
-    }
+    need_poll = cold || rebooting || ec_polled_mode || sc->ec_suspending;
 
     /* Wait for event by polling or GPE (interrupt). */
     if (need_poll) {
 	count = (ec_timeout * 1000) / EC_POLL_DELAY;
 	if (count == 0)
 	    count = 1;
+	DELAY(10);
 	for (i = 0; i < count; i++) {
 	    Status = EcCheckStatus(sc, "poll", Event);
 	    if (Status == AE_OK)
 		break;
-	    AcpiOsStall(EC_POLL_DELAY);
+	    DELAY(EC_POLL_DELAY);
 	}
     } else {
 	slp_ival = hz / 1000;
@@ -869,34 +857,37 @@ EcWaitEvent(struct acpi_ec_softc *sc, EC_EVENT Event, u_int gen_count)
 	 * 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;
+	    if (gen_count == sc->ec_gencount)
+		tsleep(&sc, 0, "ecgpe", slp_ival);
+	    /*
+	     * 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.
+	     */
+	    Status = EcCheckStatus(sc, "sleep", Event);
+	    if (Status == AE_OK) {
+		if (gen_count == sc->ec_gencount)
+		    no_intr++;
+		else
+		    no_intr = 0;
+		break;
 	    }
-	    tsleep(&sc->ec_gencount, PZERO, "ecgpe", slp_ival);
+	    gen_count = sc->ec_gencount;
 	}
 
 	/*
 	 * 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.
+	 * the best we can do at this point.
 	 */
-	if (Status != AE_OK) {
+	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;
-	}
+    }
+    if (!need_poll && no_intr > 10) {
+	device_printf(sc->ec_dev,
+	    "not getting interrupts, switched to polled mode\n");
+	ec_polled_mode = 1;
     }
     if (Status != AE_OK)
 	    CTR0(KTR_ACPI, "error: ec wait timed out");
@@ -933,6 +924,14 @@ EcCommand(struct acpi_ec_softc *sc, EC_COMMAND cmd)
 	return (AE_BAD_PARAMETER);
     }
 
+    /*
+     * Ensure empty input buffer before issuing command.
+     * Use generation count of zero to force a quick check.
+     */
+    status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY, 0);
+    if (ACPI_FAILURE(status))
+	return (status);
+
     /* Run the command and wait for the chosen event. */
     CTR1(KTR_ACPI, "ec running command %#x", cmd);
     gen_count = sc->ec_gencount;
@@ -955,24 +954,31 @@ EcRead(struct acpi_ec_softc *sc, UINT8 Address, UINT8 *Data)
 {
     ACPI_STATUS	status;
     u_int gen_count;
+    int retry;
 
     ACPI_SERIAL_ASSERT(ec);
     CTR1(KTR_ACPI, "ec read from %#x", Address);
 
-    status = EcCommand(sc, EC_COMMAND_READ);
-    if (ACPI_FAILURE(status))
-	return (status);
+    for (retry = 0; retry < 2; retry++) {
+	status = EcCommand(sc, EC_COMMAND_READ);
+	if (ACPI_FAILURE(status))
+	    return (status);
 
-    gen_count = sc->ec_gencount;
-    EC_SET_DATA(sc, Address);
-    status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL, gen_count);
-    if (ACPI_FAILURE(status)) {
-	device_printf(sc->ec_dev, "EcRead: failed waiting to get data\n");
-	return (status);
+	gen_count = sc->ec_gencount;
+	EC_SET_DATA(sc, Address);
+	status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL, gen_count);
+	if (ACPI_FAILURE(status)) {
+	    if (EcCheckStatus(sc, "retr_check",
+		EC_EVENT_INPUT_BUFFER_EMPTY) == AE_OK)
+		continue;
+	    else
+		break;
+	}
+	*Data = EC_GET_DATA(sc);
+	return (AE_OK);
     }
-    *Data = EC_GET_DATA(sc);
-
-    return (AE_OK);
+    device_printf(sc->ec_dev, "EcRead: failed waiting to get data\n");
+    return (status);
 }
 
 static ACPI_STATUS
@@ -992,7 +998,7 @@ EcWrite(struct acpi_ec_softc *sc, UINT8 Address, UINT8 Data)
     EC_SET_DATA(sc, Address);
     status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY, gen_count);
     if (ACPI_FAILURE(status)) {
-	device_printf(sc->ec_dev, "EcRead: failed waiting for sent address\n");
+	device_printf(sc->ec_dev, "EcWrite: failed waiting for sent address\n");
 	return (status);
     }