From owner-freebsd-current Thu Aug 29 19: 3: 6 2002 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 53A3037B400 for ; Thu, 29 Aug 2002 19:02:55 -0700 (PDT) Received: from axe-inc.co.jp (axegw.axe-inc.co.jp [61.199.217.66]) by mx1.FreeBSD.org (Postfix) with ESMTP id A8AA943E4A for ; Thu, 29 Aug 2002 19:02:53 -0700 (PDT) (envelope-from takawata@axe-inc.co.jp) Received: from axe-inc.co.jp ([192.47.224.47]) by axe-inc.co.jp (8.9.3+3.2W/3.7W) with ESMTP id LAA23367; Fri, 30 Aug 2002 11:02:51 +0900 (JST) Message-Id: <200208300202.LAA23367@axe-inc.co.jp> To: current@freebsd.org Cc: acpi-jp@jp.FreeBSD.org Subject: Re: [acpi-jp 1770] Re: EC handler doing bad things.. In-reply-to: Your message of "Thu, 29 Aug 2002 16:47:54 -0400." Date: Fri, 30 Aug 2002 11:03:20 +0900 From: Takanori Watanabe Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In message , John Baldwin さんいわく: > >On 29-Aug-2002 Mitsuru IWASAKI wrote: >>> A while back I used to get warnings about temperature events a lot. >>> I don't get those anymore but now I get a lot of errors when >>> embedded controller events trigger like so: >>> >>> ACPI-0433: *** Error: Handler for [EmbeddedControl] returned AE_ERROR >>> >>> Does anyone have any ideas on why and/or where the best place to look? >>> For example, which Ec handler is ACPICA calling that is failing? >> >> I don't know why, but it seems that evregion.c:AcpiEvAddressSpaceDispatch(), >> acpi_ec.c:EcSpaceHandler() and acpi_ec.c:EcTransaction() are good >> cadidate to check out. > >Ok, I've found it, reverting this commit makes it work again: > >takawata 2002/07/01 20:38:07 PDT > > Modified files: > sys/dev/acpica acpi_ec.c > Log: > Make interrupt driven EC transaction optional. > > Revision Changes Path > 1.26 +2 -0 src/sys/dev/acpica/acpi_ec.c > >This commit seems a bit incomplete (it uses an option that isn't setup in >conf/options or defined anywhere). Watanabe-san, can you explain why >you made this change a bit better? It seems to break on at least my >laptop (Dell Inspiron 5000e). The reason of changing itself is just the same reason as you complain. (It did not work for at least 2 people including me.) And I tested a patch so that first the EC try to use interrrupt driven mode then use polling mode if it failed. But my keyboard controller (in many cases, it shares ACPI embedded controller) get wrong by using the patch. OK. If there are any people that is happy with the interrupt driven mode, I'll turn it from the #ifdef to TUNABLE_INT option. The patch are as follows. Index: /src/sys/dev/acpica/acpi_ec.c diff -u /src/sys/dev/acpica/acpi_ec.c:1.1.1.1 /src/sys/dev/acpica/acpi_ec.c:1.2 --- /src/sys/dev/acpica/acpi_ec.c:1.1.1.1 Sat Jul 27 14:00:22 2002 +++ /src/sys/dev/acpica/acpi_ec.c Sat Jul 27 14:37:43 2002 @@ -143,7 +143,7 @@ #include #include #include - +#include #include "acpi.h" #include @@ -242,10 +242,14 @@ int ec_lockhandle; int ec_pendquery; int ec_csrvalue; + int ec_burst; }; #define EC_LOCK_TIMEOUT 1000 /* 1ms */ - +static int ec_readfail = 0; +static int ec_writefail = 0; +SYSCTL_INT(_debug, OID_AUTO, acpi_ec_readfail, CTLFLAG_RD, &ec_readfail, 0, ""); +SYSCTL_INT(_debug, OID_AUTO, acpi_ec_writefail, CTLFLAG_RD, &ec_writefail, 0, ""); static __inline ACPI_STATUS EcLock(struct acpi_ec_softc *sc) { @@ -289,7 +293,8 @@ static ACPI_STATUS EcTransaction(struct acpi_ec_softc *sc, EC_REQUEST *EcRequest); 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); - +static ACPI_STATUS EcBurstEnable(struct acpi_ec_softc *sc); +static ACPI_STATUS EcBurstDisable(struct acpi_ec_softc *sc); static void acpi_ec_identify(driver_t driver, device_t bus); static int acpi_ec_probe(device_t dev); static int acpi_ec_attach(device_t dev); @@ -371,6 +376,7 @@ /* * Attach bus resources */ + sc->ec_burst = 0; sc->ec_data_rid = 0; if ((sc->ec_data_res = bus_alloc_resource(sc->ec_dev, SYS_RES_IOPORT, &sc->ec_data_rid, 0, ~0, 1, RF_ACTIVE)) == NULL) { @@ -571,7 +577,6 @@ 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; @@ -592,6 +597,8 @@ /* * Perform the transaction. */ + if(width > 16) + EcBurstEnable(sc); for (i = 0; i < width; i += 8) { if (Function == ACPI_READ) EcRequest.Data = 0; @@ -603,6 +610,10 @@ if (++EcRequest.Address == 0) return_ACPI_STATUS(AE_BAD_PARAMETER); } + if(sc->ec_burst) + EcBurstDisable(sc); + if(Status != AE_OK) + printf("%x %d\n", (UINT32) Address, width); return_ACPI_STATUS(Status); } @@ -610,7 +621,7 @@ * Wait for an event interrupt for a specific condition. */ static ACPI_STATUS -EcWaitEventIntr(struct acpi_ec_softc *sc, EC_EVENT Event) +EcWaitEventIntr(struct acpi_ec_softc *sc, EC_EVENT Event, int poll) { EC_STATUS EcStatus; int i; @@ -618,9 +629,7 @@ ACPI_FUNCTION_TRACE_U32((char *)(uintptr_t)__func__, (UINT32)Event); /* XXX this should test whether interrupts are available some other way */ -#ifdef ACPI_EC_EVENT_DRIVEN - if(cold) -#endif + if(cold||sc->ec_burst|| poll) return_ACPI_STATUS(EcWaitEvent(sc, Event)); if (!EcIsLocked(sc)) @@ -776,72 +785,121 @@ return(Status); } - static ACPI_STATUS -EcRead(struct acpi_ec_softc *sc, UINT8 Address, UINT8 *Data) +EcBurstEnable(struct acpi_ec_softc *sc) { ACPI_STATUS Status; if (!EcIsLocked(sc)) ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), - "EcRead called without EC lock!\n"); - - /*EcBurstEnable(EmbeddedController);*/ - - EC_SET_CSR(sc, EC_COMMAND_READ); - if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY))) { + "EcBurstEnable called without EC lock!\n"); + EC_SET_CSR(sc, EC_COMMAND_BURST_ENABLE); + if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_OUTPUT_BUFFER_FULL,0))) { + printf("Burst failed\n"); ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), "EcRead: Failed waiting for EC to process read command.\n"); return(Status); } + if(EC_GET_DATA(sc) != 0x90){ + printf("Burst failed\n"); + } + sc->ec_burst=1; + return AE_OK; +} +static ACPI_STATUS +EcBurstDisable(struct acpi_ec_softc *sc) +{ + ACPI_STATUS Status; - EC_SET_DATA(sc, Address); - if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_OUTPUT_BUFFER_FULL))) { + if (!EcIsLocked(sc)|| !sc->ec_burst) + ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), + "EcBurstEnable called without EC lock!\n"); + EC_SET_CSR(sc, EC_COMMAND_BURST_DISABLE); + if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY,0))) { + printf("Burst failed\n"); ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), - "EcRead: Failed waiting for EC to send data.\n"); + "EcRead: Failed waiting for EC to process read command.\n"); return(Status); } + sc->ec_burst=0; + return AE_OK; +} - (*Data) = EC_GET_DATA(sc); +static ACPI_STATUS +EcRead(struct acpi_ec_softc *sc, UINT8 Address, UINT8 *Data) +{ + ACPI_STATUS Status; + int i; - /*EcBurstDisable(EmbeddedController);*/ + if (!EcIsLocked(sc)) + ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), + "EcRead called without EC lock!\n"); - return(AE_OK); + /*EcBurstEnable(EmbeddedController);*/ + /*Retry up to 10*/ + for(i = 0 ; i < 10; i++){ + EC_SET_CSR(sc, EC_COMMAND_READ); + if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY, i))) { + ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), + "EcRead: Failed waiting for EC to process read command.\n"); + ec_readfail++; + continue; + } + + EC_SET_DATA(sc, Address); + if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_OUTPUT_BUFFER_FULL, i))) { + ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), + "EcRead: Failed waiting for EC to send data.\n"); + ec_readfail++; + continue; + } + + (*Data) = EC_GET_DATA(sc); + return (AE_OK); + } + /*EcBurstDisable(EmbeddedController);*/ + return(AE_ERROR); } static ACPI_STATUS EcWrite(struct acpi_ec_softc *sc, UINT8 Address, UINT8 *Data) { ACPI_STATUS Status; - + int i; + if (!EcIsLocked(sc)) ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), "EcWrite called without EC lock!\n"); /*EcBurstEnable(EmbeddedController);*/ - - EC_SET_CSR(sc, EC_COMMAND_WRITE); - if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY))) { + for(i = 0; i < 10; i++){ + EC_SET_CSR(sc, EC_COMMAND_WRITE); + if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY, i))) { ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), - "EcWrite: Failed waiting for EC to process write command.\n"); - return(Status); + "EcWrite: Failed waiting for EC to process write command.\n"); + ec_writefail++; + continue; } - - EC_SET_DATA(sc, Address); - if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY))) { + + EC_SET_DATA(sc, Address); + if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY, i))) { ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), - "EcRead: Failed waiting for EC to process address.\n"); - return(Status); - } - - EC_SET_DATA(sc, *Data); - if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY))) { + "EcRead: Failed waiting for EC to process address.\n"); + ec_writefail++; + continue; + } + + EC_SET_DATA(sc, *Data); + if (ACPI_FAILURE(Status = EcWaitEventIntr(sc, EC_EVENT_INPUT_BUFFER_EMPTY, i))) { ACPI_VPRINT(sc->ec_dev, acpi_device_get_parent_softc(sc->ec_dev), - "EcWrite: Failed waiting for EC to process data.\n"); - return(Status); + "EcWrite: Failed waiting for EC to process data.\n"); + ec_writefail++; + continue; + } + + /*EcBurstDisable(EmbeddedController);*/ + return(AE_OK); } - - /*EcBurstDisable(EmbeddedController);*/ - - return(AE_OK); + + return (AE_ERROR); } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message