Subject: New acpi_ec driver
To: None <port-i386@netbsd.org>
From: Takayoshi Kochi <kochi@netbsd.org>
List: port-i386
Date: 08/16/2003 17:26:22
----Next_Part(Sat_Aug_16_17:26:22_2003_052)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi all,

I'd like to import the new acpi_ec driver which is a port
from Nate Lawson's work for FreeBSD.

The most important change in the new driver is rewrite of
its locking logic, which was sometimes faulty.

Except its stability, EC driver's functionality hasn't changed.
So I'd like to make sure there's no regression before commiting this.
And I hope this new driver resolves kern/19764 (acpi caused
envstat to hang).

Please test, and if there's no bad report in a week, I'll commit this.

Note:
I haven't integrated ECDT table support in it yet.
(If ECDT table support is integrated, you can use EC functionality
before acpi_ec driver is attached on boottime, which makes it possible
to attach some devices that rely on EC before acpi_ec is attached)

---
Takayoshi Kochi

----Next_Part(Sat_Aug_16_17:26:22_2003_052)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="acpi_ec.c.diff2"

--- acpi_ec.c.orig	2003-08-16 16:40:23.000000000 +0900
+++ acpi_ec.c	2003-08-16 17:08:30.000000000 +0900
@@ -36,6 +36,7 @@
  */
 
 /*-
+ * Copyright (c) 2003 Nate Lawson
  * Copyright (c) 2000 Michael Smith
  * Copyright (c) 2000 BSDi
  * All rights reserved.
@@ -207,11 +208,14 @@
 	int		sc_flags;	/* see below */
 
 	uint32_t	sc_csrvalue;	/* saved control register */
-	UINT32		sc_lockhandle;
+
+	UINT32		sc_glkhandle;
+	struct simplelock sc_lock;
+	uint32_t	sc_polldelay;
+	uint32_t	sc_poll_timeout;
 };
 
-#define	EC_F_LOCKED	0x01		/* EC is locked */
-#define	EC_F_PENDQUERY	0x02		/* query is pending */
+#define EC_F_USEGLK	0x01		/* Global Lock is required */
 
 #define	EC_DATA_READ(sc)						\
 	bus_space_read_1((sc)->sc_data_st, (sc)->sc_data_sh, 0)
@@ -223,43 +227,55 @@
 #define	EC_CSR_WRITE(sc, v)						\
 	bus_space_write_1((sc)->sc_csr_st, (sc)->sc_csr_sh, 0, (v))
 
-static __inline int
-EcIsLocked(struct acpi_ec_softc *sc)
-{
- 
-	return (acpi_is_global_locked() && (sc->sc_flags & EC_F_LOCKED));
-}
+/*
+ * XXX
+ * I couldn't find it in the spec but other implementations also use a
+ * value of 1 ms for the time to acquire global lock.
+ */
+#define EC_LOCK_TIMEOUT 1000
+
+/*
+ * Start with an interval of 1 us for status poll loop.  This delay
+ * will be dynamically adjusted based on the actual time waited.
+ */
+#define EC_POLL_DELAY	1
+
+/* Total time in ms spent in the poll loop waiting for a response. */
+#define EC_POLL_TIMEOUT 50
+
+#define EVENT_READY(event, status)			\
+	(((event) == EC_EVENT_OUTPUT_BUFFER_FULL &&	\
+	 ((status) & EC_FLAG_OUTPUT_BUFFER) != 0) ||	\
+	 ((event) == EC_EVENT_INPUT_BUFFER_EMPTY &&	\
+	 ((status) & EC_FLAG_INPUT_BUFFER) == 0))
 
 static __inline ACPI_STATUS
 EcLock(struct acpi_ec_softc *sc)
 {
-	ACPI_STATUS status;
-	UINT32 handle;
+	ACPI_STATUS status = AE_OK;
 
-	status = acpi_acquire_global_lock(&handle);
-	if (ACPI_SUCCESS(status)) {
-		sc->sc_flags |= EC_F_LOCKED;
-		sc->sc_lockhandle = handle;
+	/* Always acquire this EC's mutex */
+	simple_lock(&sc->sc_lock);
+
+	/* if _GLK is non-zero, also acquire the global lock. */
+	if (sc->sc_flags & EC_F_USEGLK) {
+		/* XXXTK EC_LOCK_TIMEOUT */
+		status = acpi_acquire_global_lock(&sc->sc_glkhandle);
+		if (ACPI_FAILURE(status))
+			simple_unlock(&sc->sc_lock);
 	}
 
-	return (status);
+	return status;
 }
 
 static __inline void
 EcUnlock(struct acpi_ec_softc *sc)
 {
-	if (!EcIsLocked(sc))
-		return;
-	sc->sc_flags &= ~EC_F_LOCKED;
-	acpi_release_global_lock(sc->sc_lockhandle);
+	if (sc->sc_flags & EC_F_USEGLK)
+		acpi_release_global_lock(sc->sc_glkhandle);
+	simple_unlock(&sc->sc_lock);
 }
 
-typedef struct {
-	EC_COMMAND	Command;
-	UINT8		Address;
-	UINT8		Data;
-} EC_REQUEST;
-
 static void		EcGpeHandler(void *Context);
 static ACPI_STATUS	EcSpaceSetup(ACPI_HANDLE Region, UINT32 Function,
 			    void *Context, void **return_Context);
@@ -269,13 +285,12 @@
 			    void *RegionContext);
 
 static ACPI_STATUS	EcWaitEvent(struct acpi_ec_softc *sc, EC_EVENT Event);
-static ACPI_STATUS	EcQuery(struct acpi_ec_softc *sc, UINT8 *Data);
-static ACPI_STATUS	EcTransaction(struct acpi_ec_softc *sc,
-			    EC_REQUEST *EcRequest);
+static ACPI_STATUS	EcCommand(struct acpi_ec_softc *sc,
+			    EC_COMMAND Command);
 static ACPI_STATUS	EcRead(struct acpi_ec_softc *sc, UINT8 Address,
 			    UINT8 *Data);
 static ACPI_STATUS	EcWrite(struct acpi_ec_softc *sc, UINT8 Address,
-			    UINT8 *Data);
+			    UINT8 Data);
 
 int	acpiec_match(struct device *, struct cfdata *, void *);
 void	acpiec_attach(struct device *, struct device *, void *);
@@ -294,12 +309,12 @@
 	struct acpi_attach_args *aa = aux;
 
 	if (aa->aa_node->ad_type != ACPI_TYPE_DEVICE)
-		return (0);
+		return 0;
 
 	if (strcmp(aa->aa_node->ad_devinfo.HardwareId, "PNP0C09") == 0)
-		return (1);
+		return 1;
 
-	return (0);
+	return 0;
 }
 
 /*
@@ -313,6 +328,7 @@
 	struct acpi_ec_softc *sc = (void *) self;
 	struct acpi_attach_args *aa = aux;
 	struct acpi_io *io0, *io1;
+	UINT32 tmp;
 	ACPI_STATUS rv;
 
 	ACPI_FUNCTION_TRACE(__FUNCTION__);
@@ -320,15 +336,18 @@
 	printf(": ACPI Embedded Controller\n");
 
 	sc->sc_node = aa->aa_node;
+	simple_lock_init(&sc->sc_lock);
+	sc->sc_polldelay = EC_POLL_DELAY;
+	sc->sc_poll_timeout = EC_POLL_TIMEOUT;
 
 	/* Parse our resources. */
 	ACPI_DEBUG_PRINT((ACPI_DB_RESOURCES, "parsing EC resources\n"));
 	rv = acpi_resource_parse(&sc->sc_dev, sc->sc_node, &sc->sc_res,
 	    &acpi_resource_parse_ops_default);
-	if (rv != AE_OK) {
+	if (ACPI_FAILURE(rv)) {
 		printf("%s: unable to parse resources: %d\n",
 		    sc->sc_dev.dv_xname, rv);
-		return;
+		return_VOID;
 	}
 
 	sc->sc_data_st = aa->aa_iot;
@@ -336,13 +355,13 @@
 	if (io0 == NULL) {
 		printf("%s: unable to find data register resource\n",
 		    sc->sc_dev.dv_xname);
-		return;
+		return_VOID;
 	}
 	if (bus_space_map(sc->sc_data_st, io0->ar_base, io0->ar_length,
 	    0, &sc->sc_data_sh) != 0) {
 		printf("%s: unable to map data register\n",
 		    sc->sc_dev.dv_xname);
-		return;
+		return_VOID;
 	}
 
 	sc->sc_csr_st = aa->aa_iot;
@@ -350,50 +369,53 @@
 	if (io1 == NULL) {
 		printf("%s: unable to find csr register resource\n",
 		    sc->sc_dev.dv_xname);
-		return;
+		return_VOID;
 	}
 	if (bus_space_map(sc->sc_csr_st, io1->ar_base, io1->ar_length,
 	    0, &sc->sc_csr_sh) != 0) {
 		printf("%s: unable to map csr register\n",
 		    sc->sc_dev.dv_xname);
-		return;
+		return_VOID;
 	}
 
+	/* Check if global lock should be used.  If not, leave flag as 0. */
+	rv = acpi_eval_integer(sc->sc_node->ad_handle, "_GLK", &tmp);
+	if (ACPI_SUCCESS(rv) && tmp == 1)
+		sc->sc_flags |= EC_F_USEGLK;
+
 	/*
 	 * Install GPE handler.
 	 *
 	 * We evaluate the _GPE method to find the GPE bit used by the
 	 * Embedded Controller to signal status (SCI).
 	 */
-	if ((rv = acpi_eval_integer(sc->sc_node->ad_handle, "_GPE",
-	     &sc->sc_gpebit)) != AE_OK) {
-		printf("%s: unable to evaluate _GPE: %d\n",
-		    sc->sc_dev.dv_xname, rv);
-		return;
+	rv = acpi_eval_integer(sc->sc_node->ad_handle, "_GPE", &sc->sc_gpebit);
+	if (ACPI_FAILURE(rv)) {
+		printf("%s: unable to evaluate _GPE: %s\n",
+		    sc->sc_dev.dv_xname, AcpiFormatException(rv));
+		return_VOID;
 	}
 
 	/*
-	 * Install a handler for this EC's GPE bit.  Note that EC SCIs are 
-	 * treated as both edge- and level-triggered interrupts; in other words
-	 * we clear the status bit immediately after getting an EC-SCI, then
-	 * again after we're done processing the event.  This guarantees that
-	 * events we cause while performing a transaction (e.g. IBE/OBF) get 
-	 * cleared before re-enabling the GPE.
+	 * Install a handler for this EC's GPE bit.  We want edge-triggered
+	 * behavior
 	 */
-	if ((rv = AcpiInstallGpeHandler(sc->sc_gpebit,
-	     ACPI_EVENT_LEVEL_TRIGGERED | ACPI_EVENT_EDGE_TRIGGERED,
-	     EcGpeHandler, sc)) != AE_OK) {
-		printf("%s: unable to install GPE handler: %d\n",
-		    sc->sc_dev.dv_xname, rv);
-		return;
+	rv = AcpiInstallGpeHandler(sc->sc_gpebit,
+		ACPI_EVENT_EDGE_TRIGGERED,
+		EcGpeHandler, sc);
+	if (ACPI_FAILURE(rv)) {
+		printf("%s: unable to install GPE handler: %s\n",
+		    sc->sc_dev.dv_xname, AcpiFormatException(rv));
+		return_VOID;
 	}
 
 	/* Install address space handler. */
-	if ((rv = AcpiInstallAddressSpaceHandler(sc->sc_node->ad_handle,
-	     ACPI_ADR_SPACE_EC, EcSpaceHandler, EcSpaceSetup, sc)) != AE_OK) {
-		printf("%s: unable to install address space handler: %d\n",
-		    sc->sc_dev.dv_xname, rv);
-		return;
+	rv = AcpiInstallAddressSpaceHandler(sc->sc_node->ad_handle,
+		ACPI_ADR_SPACE_EC, EcSpaceHandler, EcSpaceSetup, sc);
+	if (ACPI_FAILURE(rv)) {
+		printf("%s: unable to install address space handler: %s\n",
+		    sc->sc_dev.dv_xname, AcpiFormatException(rv));
+		return_VOID;
 	}
 
 	return_VOID;
@@ -405,62 +427,72 @@
 	struct acpi_ec_softc *sc = Context;
 	UINT8 Data;
 	ACPI_STATUS Status;
+	EC_STATUS EcStatus;
 	char qxx[5];
 
 	ACPI_FUNCTION_TRACE(__FUNCTION__);
 
-	for (;;) {
-		/*
-		 * Check EC_SCI.
-		 * 
-		 * Bail out if the EC_SCI bit of the status register is not
-		 * set. Note that this function should only be called when
-		 * this bit is set (polling is used to detect IBE/OBF events).
-		 *
-		 * It is safe to do this without locking the controller, as
-		 * it's OK to call EcQuery when there's no data ready; in the
-		 * worst case we should just find nothing waiting for us and
-		 * bail.
-		 */
-		if ((EC_CSR_READ(sc) & EC_EVENT_SCI) == 0)
-			break;
+	Status = EcLock(sc);
+	if (ACPI_FAILURE(Status)) {
+		printf("%s: EcGpeQueryHandler lock error: %s\n",
+		       sc->sc_dev.dv_xname, AcpiFormatException(Status));
+		return_VOID;
+	}
 
-		/*
-		 * Find out why the EC is signalling us
-		 */
-		Status = EcQuery(sc, &Data);
-	    
-		/*
-		 * If we failed to get anything from the EC, give up.
-		 */
-		if (Status != AE_OK) {
-			printf("%s: GPE query failed: %d\n",
-			    sc->sc_dev.dv_xname, Status);
-			break;
-		}
+	/*
+	 * Check status for EC_SCI.
+	 * 
+	 * Bail out if the EC_SCI bit of the status register is not set.
+	 * Note that this function should only be called when
+	 * this bit is set (polling is used to detect IBE/OBF events).
+	 *
+	 * We don't acquire the global lock here but do protect against other
+	 * orunning commands (read/write/query) by grabbing sc_lock.
+	 */
+	EcStatus = EC_CSR_READ(sc);
+	if ((EcStatus & EC_EVENT_SCI) == 0) {
+		/* If it's not an SCI, wakeup the EcWaitEvent sleep. */
+		sc->sc_csrvalue = EcStatus;
+		wakeup(&sc->sc_csrvalue);
+		EcUnlock(sc);
+		goto re_enable;
+	}
+
+	/*
+	 * Send a query command to the EC to find out which _Qxx call it
+	 * wants to make.  This command clears the SCi bit and also the
+	 * interrupt source since we are edge-triggered.
+	 */
+	Status = EcCommand(sc, EC_COMMAND_QUERY);
+	if (ACPI_FAILURE(Status)) {
+		EcUnlock(sc);
+		printf("%s: GPE query failed: %s\n",
+		       sc->sc_dev.dv_xname,
+		       AcpiFormatException(Status));
+		goto re_enable;
+	}
+	Data = EC_DATA_READ(sc);
+	EcUnlock(sc);
 
-		/*
-		 * Evaluate _Qxx to respond to the controller.
-		 */
-		sprintf(qxx, "_Q%02x", Data);
-		strupr(qxx);
-		Status = AcpiEvaluateObject(sc->sc_node->ad_handle, qxx,
+	/* Ignore the value for "no outstanding event". (13.3.5) */
+	if (Data == 0)
+		goto re_enable;
+
+	/* Evaluate _Qxx to respond to the controller. */
+	sprintf(qxx, "_Q%02x", Data);
+	strupr(qxx);
+	Status = AcpiEvaluateObject(sc->sc_node->ad_handle, qxx,
 		    NULL, NULL);
 
-		/*
-		 * Ignore spurious query requests.
-		 */
-		if (Status != AE_OK &&
-		    (Data != 0 || Status != AE_NOT_FOUND)) {
-			printf("%s: evaluation of GPE query method %s "
-			    "failed: %d\n", sc->sc_dev.dv_xname, qxx, Status);
-		}
+	if (ACPI_FAILURE(Status) && Status != AE_NOT_FOUND) {
+		printf("%s: evaluation of GPE query method %s "
+		    "failed: %d\n", sc->sc_dev.dv_xname, qxx, Status);
 	}
 
-	/* I know I request Level trigger cleanup */
-	if (AcpiClearEvent(sc->sc_gpebit, ACPI_EVENT_GPE) != AE_OK)
-		printf("%s: AcpiClearEvent failed\n", sc->sc_dev.dv_xname);
-	if (AcpiEnableEvent(sc->sc_gpebit, ACPI_EVENT_GPE, 0) != AE_OK)
+ re_enable:
+	/* Re-enable the GPE event so we'll get future requests. */
+	Status = AcpiEnableEvent(sc->sc_gpebit, ACPI_EVENT_GPE, 0);
+	if (ACPI_FAILURE(Status))
 		printf("%s: AcpiEnableEvent failed\n", sc->sc_dev.dv_xname);
 
 	return_VOID;
@@ -470,29 +502,19 @@
 EcGpeHandler(void *Context)
 {
 	struct acpi_ec_softc *sc = Context;
-	uint32_t csrvalue;
+	ACPI_STATUS Status;
 
-	/* 
-	 * If EC is locked, the intr must process EcRead/Write wait only.
-	 * Query request must be pending.
-	 */
-	if (EcIsLocked(sc)) {
-		csrvalue = EC_CSR_READ(sc);
-		if (csrvalue & EC_EVENT_SCI)
-			sc->sc_flags |= EC_F_PENDQUERY;
-
-		if ((csrvalue & EC_FLAG_OUTPUT_BUFFER) != 0 ||
-		    (csrvalue & EC_FLAG_INPUT_BUFFER) == 0) {
-			sc->sc_csrvalue = csrvalue;
-			wakeup(&sc->sc_csrvalue);
-		}
-	} else {
-		/* Enqueue GpeQuery handler. */
-		if (AcpiOsQueueForExecution(OSD_PRIORITY_HIGH,
-		    EcGpeQueryHandler, Context) != AE_OK) {
-			printf("%s: failed to enqueue query handler\n",
-			    sc->sc_dev.dv_xname);
-		}
+	/* Disable further GPEs while we handle this one. */
+	AcpiDisableEvent(sc->sc_gpebit, ACPI_EVENT_GPE, 0);
+
+	/* Schedule the GPE query handler. */
+	Status = AcpiOsQueueForExecution(OSD_PRIORITY_GPE, EcGpeQueryHandler,
+		    Context);
+	if (ACPI_FAILURE(Status)) {
+		printf("Queuing GPE query handler failed.\n");
+		Status = AcpiEnableEvent(sc->sc_gpebit, ACPI_EVENT_GPE, 0);
+		if (ACPI_FAILURE(Status))
+			printf("EcGpeHandler: AcpiEnableGpe failed.\n");
 	}
 }
 
@@ -503,9 +525,7 @@
 
 	ACPI_FUNCTION_TRACE(__FUNCTION__);
 
-	/*
-	 * Just pass the context through, there's nothing to do here.
-	 */
+	/* Just pass the context through, there's nothing to do here. */
 	*RegionContext = Context;
 
 	return_ACPI_STATUS(AE_OK);
@@ -517,211 +537,166 @@
 {
 	struct acpi_ec_softc *sc = Context;
 	ACPI_STATUS Status = AE_OK;
-	EC_REQUEST EcRequest;
+	UINT8 EcAddr, EcData;
 	int i;
 
 	ACPI_FUNCTION_TRACE_U32(__FUNCTION__, (UINT32)Address);
 
-	if ((Address > 0xFF) || (width % 8 != 0) || (Value == NULL) ||
-	    (Context == NULL))
-		return_ACPI_STATUS(AE_BAD_PARAMETER);
-
-	switch (Function) {
-	case ACPI_READ:
-		EcRequest.Command = EC_COMMAND_READ;
-		EcRequest.Address = Address;
-		(*Value) = 0;
-		break;
-
-	case ACPI_WRITE:
-		EcRequest.Command = EC_COMMAND_WRITE;
-		EcRequest.Address = Address;
-		break;
-
-	default:
-		printf("%s: invalid Address Space function: %d\n",
-		    sc->sc_dev.dv_xname, Function);
+	if (width % 8 != 0 || Value == NULL || Context == NULL ||
+	    Address + width / 8 > 0x100)
 		return_ACPI_STATUS(AE_BAD_PARAMETER);
-	}
-
-	/*
-	 * Perform the transaction.
-	 */
-	for (i = 0; i < width; i += 8) {
-		if (Function == ACPI_READ)
-			EcRequest.Data = 0;
-		else
-			EcRequest.Data = (UINT8)((*Value) >> i);
 
-		if ((Status = EcTransaction(sc, &EcRequest)) != AE_OK)
+	/* Perform the transaction. */
+	EcAddr = Address;
+	if (Function == ACPI_READ)
+		*Value = (ACPI_INTEGER)0;
+	for (i = 0; i < width / 8; i++) {
+		Status = EcLock(sc);
+		if (ACPI_FAILURE(Status))
+			return Status;
+
+		switch (Function) {
+		case ACPI_READ:
+			EcData = 0;
+			Status = EcRead(sc, EcAddr, &EcData);
 			break;
+		case ACPI_WRITE:
+			EcData = (UINT8)((*Value) >> i);
+			Status = EcWrite(sc, EcAddr, EcData);
+			break;
+		default:
+			printf("%s: invalid Address Space function: %d\n",
+			       sc->sc_dev.dv_xname, Function);
+			Status = AE_BAD_PARAMETER;
+			break;
+		}
 
-		(*Value) |= (UINT32)EcRequest.Data << i;
-		if (++EcRequest.Address == 0)
-			return_ACPI_STATUS(AE_BAD_PARAMETER);
-	}
-
-	return_ACPI_STATUS(Status);
-}
-
-static ACPI_STATUS
-EcWaitEventIntr(struct acpi_ec_softc *sc, EC_EVENT Event)
-{
-	EC_STATUS EcStatus;
-	int i;
-
-	ACPI_FUNCTION_TRACE_U32(__FUNCTION__, (UINT32)Event);
-
-	/* XXX Need better test for "yes, you have interrupts". */
-	if (cold)
-		return_ACPI_STATUS(EcWaitEvent(sc, Event));
-
-	if (EcIsLocked(sc) == 0)
-		printf("%s: EcWaitEventIntr called without EC lock!\n",
-		    sc->sc_dev.dv_xname);
+		EcUnlock(sc);
+		if (ACPI_FAILURE(Status))
+			return_ACPI_STATUS(Status);
 
-	EcStatus = EC_CSR_READ(sc);
-
-	/* Too long? */
-	for (i = 0; i < 10; i++) {
-		/* Check EC status against the desired event. */
-		if ((Event == EC_EVENT_OUTPUT_BUFFER_FULL) &&
-		    (EcStatus & EC_FLAG_OUTPUT_BUFFER) != 0)
-			return_ACPI_STATUS(AE_OK);
-      
-		if ((Event == EC_EVENT_INPUT_BUFFER_EMPTY) &&
-		    (EcStatus & EC_FLAG_INPUT_BUFFER) == 0)
-			return_ACPI_STATUS(AE_OK);
-
-		sc->sc_csrvalue = 0;
-		/* XXXJRT Sleeping with a lock held? */
-		if (tsleep(&sc->sc_csrvalue, 0, "EcWait", 1) != EWOULDBLOCK)
-			EcStatus = sc->sc_csrvalue;
-		else
-			EcStatus = EC_CSR_READ(sc);
+		*Value |= (ACPI_INTEGER)EcData << i;
+		++EcAddr;
 	}
-	return_ACPI_STATUS(AE_ERROR);
+	return_ACPI_STATUS(Status);
 }
 
 static ACPI_STATUS
 EcWaitEvent(struct acpi_ec_softc *sc, EC_EVENT Event)
 {
 	EC_STATUS EcStatus;
-	UINT32 i = 0;
+	ACPI_STATUS Status;
+	int i, period, retval;
+#ifdef ACPI_DEBUG
+	static int EcDbgMaxDelay;
+#endif
 
-	if (EcIsLocked(sc) == 0)
-		printf("%s: EcWaitEvent called without EC lock!\n",
-		    sc->sc_dev.dv_xname);
+	KASSERT(simple_lock_try(&sc->sc_lock));
+
+	Status = AE_NO_HARDWARE_RESPONSE;
 
 	/*
-	 * Stall 1us:
-	 * ----------
-	 * Stall for 1 microsecond before reading the status register
-	 * for the first time.  This allows the EC to set the IBF/OBF
-	 * bit to its proper state.
-	 *
-	 * XXX it is not clear why we read the CSR twice.
+	 * Wait for 1 us before checking the CSR.  Testing shows about
+	 * 50% of requests complete in 1 us and 90% of them complete
+	 * in 5 us or less.
 	 */
 	AcpiOsStall(1);
-	EcStatus = EC_CSR_READ(sc);
 
 	/*
-	 * Wait For Event:
-	 * ---------------
 	 * Poll the EC status register to detect completion of the last
-	 * command.  Wait up to 10ms (in 100us chunks) for this to occur.
+	 * command.  First, wait up to 1 ms in chunks of sc->sc_polldelay
+	 * microseconds.
 	 */
-	for (i = 0; i < 100; i++) {
+	for (i = 0; i < 1000 / sc->sc_polldelay; i++) {
 		EcStatus = EC_CSR_READ(sc);
-
-		if ((Event == EC_EVENT_OUTPUT_BUFFER_FULL) &&
-		    (EcStatus & EC_FLAG_OUTPUT_BUFFER) != 0)
-			return (AE_OK);
-
-		if ((Event == EC_EVENT_INPUT_BUFFER_EMPTY) &&
-		    (EcStatus & EC_FLAG_INPUT_BUFFER) == 0)
-			return (AE_OK);
-
-		AcpiOsStall(10);
+		if (EVENT_READY(Event, EcStatus)) {
+			Status = AE_OK;
+			break;
+		}
+		AcpiOsStall(sc->sc_polldelay);
 	}
 
-	return (AE_ERROR);
-}    
+	/* Scale poll delay by the amount of time actually waited. */
+	period = i * sc->sc_polldelay;
+	if (period <= 5)
+		sc->sc_polldelay = 1;
+	else if (period <= 20)
+		sc->sc_polldelay = 5;
+	else if (period <= 100)
+		sc->sc_polldelay = 10;
+	else
+		sc->sc_polldelay = 100;
 
-static ACPI_STATUS
-EcQuery(struct acpi_ec_softc *sc, UINT8 *Data)
-{
-	ACPI_STATUS Status;
-
-	if ((Status = EcLock(sc)) != AE_OK)
-		return (Status);
-
-	EC_CSR_WRITE(sc, EC_COMMAND_QUERY);
-	Status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL);
-	if (Status == AE_OK)
-		*Data = EC_DATA_READ(sc);
-
-	EcUnlock(sc);
+	/*
+	 * If we still don't have a response, wait up to sc_poll_timeout ms
+	 * for completion, sleeping for chunks of 10 ms.
+	 */
+	if (Status != AE_OK) {
+		retval = -1;
+		for (i = 0; i < sc->sc_poll_timeout / 10; i++) {
+			if (retval != 0)
+				EcStatus = EC_CSR_READ(sc);
+			else
+				EcStatus = sc->sc_csrvalue;
+			if (EVENT_READY(Event, EcStatus)) {
+				Status = AE_OK;
+				break;
+			}
+			retval = ltsleep(&sc->sc_csrvalue, 0, "EcWait",
+					 hz / 100 /* 10ms */,
+					 &sc->sc_lock);
+		}
+	}
 
-	if (Status != AE_OK)
-		printf("%s: timed out waiting for EC to respond to "
-		    "EC_COMMAND_QUERY\n", sc->sc_dev.dv_xname);
+	/* Calculate new delay */
+	if (period == 1000)
+		period += i * 10000;
+#ifdef ACPI_DEBUG
+	/* print the new delay if it exceeds the max. */
+	if (period > EcDbgMaxDelay) {
+		EcDbgMaxDelay = period;
+		printf("%s: info: new max delay is %d us\n",
+		       sc->sc_dev.dv_xname, period);
+	}
+#endif
 
-	return (Status);
-}    
+	return Status;
+}
 
 static ACPI_STATUS
-EcTransaction(struct acpi_ec_softc *sc, EC_REQUEST *EcRequest)
+EcCommand(struct acpi_ec_softc *sc, EC_COMMAND Command)
 {
 	ACPI_STATUS Status;
+	EC_EVENT Event;
 
-	if ((Status = EcLock(sc)) != AE_OK)
-		return (Status);
+	KASSERT(simple_lock_try(&sc->sc_lock));
 
-	/*
-	 * Perform the transaction.
-	 */
-	switch (EcRequest->Command) {
+	switch (Command) {
 	case EC_COMMAND_READ:
-		Status = EcRead(sc, EcRequest->Address, &(EcRequest->Data));
-		break;
-
 	case EC_COMMAND_WRITE:
-		Status = EcWrite(sc, EcRequest->Address, &(EcRequest->Data));
+	case EC_COMMAND_BURST_DISABLE:
+		Event = EC_EVENT_INPUT_BUFFER_EMPTY;
 		break;
-
-	default:
-		Status = AE_SUPPORT;
+	case EC_COMMAND_QUERY:
+	case EC_COMMAND_BURST_ENABLE:
+		Event = EC_EVENT_OUTPUT_BUFFER_FULL;
 		break;
+	default:
+		printf("%s: EcCommand: Invalid Command  %#x\n",
+		       sc->sc_dev.dv_xname, Command);
+		return AE_BAD_PARAMETER;
 	}
 
-	/*
-	 * Clear & Re-Enable the EC GPE:
-	 * -----------------------------
-	 * 'Consume' any EC GPE events that we generated while performing
-	 * the transaction (e.g. IBF/OBF). Clearing the GPE here shouldn't
-	 * have an adverse affect on outstanding EC-SCI's, as the source
-	 * (EC-SCI) will still be high and thus should trigger the GPE
-	 * immediately after we re-enabling it.
-	 */
-	if (sc->sc_flags & EC_F_PENDQUERY) {
-		if (AcpiOsQueueForExecution(OSD_PRIORITY_HIGH,
-		    EcGpeQueryHandler, sc) != AE_OK)
-			printf("%s: unable to queue pending query\n",
-			    sc->sc_dev.dv_xname);
-		sc->sc_flags &= ~EC_F_PENDQUERY;
-	}
-
-	if (AcpiClearEvent(sc->sc_gpebit, ACPI_EVENT_GPE) != AE_OK)
-		printf("%s: EcRequest: unable to clear EC GPE\n",
-		    sc->sc_dev.dv_xname);
-	if (AcpiEnableEvent(sc->sc_gpebit, ACPI_EVENT_GPE, 0) != AE_OK)
-		printf("%s: EcRequest: unable to reenable EC GPE\n",
-		    sc->sc_dev.dv_xname);
+	/* Run the command and wait for the chosen event. */
+	EC_CSR_WRITE(sc, Command);
+	Status = EcWaitEvent(sc, Event);
 
-	EcUnlock(sc);
+	if (ACPI_FAILURE(Status)) {
+		printf("%s: EcCommand: no response to %#x\n",
+		       sc->sc_dev.dv_xname, Command);
+	}
 
-	return(Status);
+	return Status;
 }
 
 static ACPI_STATUS
@@ -729,71 +704,77 @@
 {
 	ACPI_STATUS Status;
 
-	if (EcIsLocked(sc) == 0)
-		printf("%s: EcRead called without EC lock!\n",
-		    sc->sc_dev.dv_xname);
-
-	/* EcBurstEnable(EmbeddedController); */
+	KASSERT(simple_lock_try(&sc->sc_lock));
 
-	EC_CSR_WRITE(sc, EC_COMMAND_READ);
-	if ((Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY)) !=
-	    AE_OK) {
-		printf("%s: EcRead: timeout waiting for EC to process "
-		    "read command\n", sc->sc_dev.dv_xname);
-		return (Status);
-	}
+#ifdef notyet
+	/* If we can't start burst mode, continue anyway. */
+	EcCommand(sc, EC_COMMAND_BURST_ENABLE);
+#endif
+
+	Status = EcCommand(sc, EC_COMMAND_READ);
+	if (ACPI_FAILURE(Status))
+		return Status;
 
 	EC_DATA_WRITE(sc, Address);
-	if ((Status = EcWaitEventIntr(sc, EC_EVENT_OUTPUT_BUFFER_FULL)) !=
-	    AE_OK) {
+	Status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL);
+	if (ACPI_FAILURE(Status)) {
 		printf("%s: EcRead: timeout waiting for EC to send data\n",
 		    sc->sc_dev.dv_xname);
-		return (Status);
+		return Status;
 	}
 
-	(*Data) = EC_DATA_READ(sc);
+	*Data = EC_DATA_READ(sc);
 
-	/* EcBurstDisable(EmbeddedController); */
+#ifdef notyet
+	if (sc->sc_burstactive) {
+		Status = EcCommand(sc, EC_COMMAND_BURST_DISABLE);
+		if (ACPI_FAILURE(Status))
+			return Status;
+	}
+#endif
 
-	return (AE_OK);
-}    
+	return AE_OK;
+}
 
 static ACPI_STATUS
-EcWrite(struct acpi_ec_softc *sc, UINT8 Address, UINT8 *Data)
+EcWrite(struct acpi_ec_softc *sc, UINT8 Address, UINT8 Data)
 {
 	ACPI_STATUS Status;
 
-	if (EcIsLocked(sc) == 0)
-		printf("%s: EcWrite called without EC lock!\n",
-		    sc->sc_dev.dv_xname);
+	KASSERT(simple_lock_try(&sc->sc_lock));
 
-	/* EcBurstEnable(EmbeddedController); */
-
-	EC_CSR_WRITE(sc, EC_COMMAND_WRITE);
-	if ((Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY)) !=
-	    AE_OK) {
-		printf("%s: EcWrite: timeout waiting for EC to process "
-		    "write command\n", sc->sc_dev.dv_xname);
-		return (Status);
-	}
+#ifdef notyet
+	/* If we can't start burst mode, continue anyway. */
+	EcCommand(sc, EC_COMMAND_BURST_ENABLE);
+#endif
+
+	Status = EcCommand(sc, EC_COMMAND_WRITE);
+	if (ACPI_FAILURE(Status))
+		return Status;
 
 	EC_DATA_WRITE(sc, Address);
-	if ((Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY)) !=
-	    AE_OK) {
+	Status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY);
+	if (ACPI_FAILURE(Status)) {
 		printf("%s: EcWrite: timeout waiting for EC to process "
 		    "address\n", sc->sc_dev.dv_xname);
-		return (Status);
+		return Status;
 	}
 
-	EC_DATA_WRITE(sc, *Data);
-	if ((Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY)) !=
-	    AE_OK) {
+	EC_DATA_WRITE(sc, Data);
+	Status = EcWaitEvent(sc, EC_EVENT_INPUT_BUFFER_EMPTY);
+	if (ACPI_FAILURE(Status)) {
 		printf("%s: EcWrite: timeout waiting for EC to process "
 		    "data\n", sc->sc_dev.dv_xname);
-		return (Status);
+		return Status;
 	}
 
-	/* EcBurstDisable(EmbeddedController); */
+#ifdef notyet
+	if (sc->ec_burstactive) {
+		Status = EcCommand(sc, EC_COMMAND_BURST_DISABLE);
+		if (ACPI_FAILURE(Status))
+			return Status;
+	}
+#endif
 
-	return (AE_OK);
+	return AE_OK;
 }

----Next_Part(Sat_Aug_16_17:26:22_2003_052)----