Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/acpi - fix deadlocks due to using lock_status() from...
details: https://anonhg.NetBSD.org/src/rev/72ce95b7b1e9
branches: trunk
changeset: 555244:72ce95b7b1e9
user: yamt <yamt%NetBSD.org@localhost>
date: Wed Nov 12 13:59:23 2003 +0000
description:
- fix deadlocks due to using lock_status() from interrupt context.
- process pending queries in EcUnlock() to close a race window.
now there's no need to do polling for EcQuery().
- reorder inline functions and other prototypes so that
the formers can get needed prototypes.
- add missing prototypes.
diffstat:
sys/dev/acpi/acpi_ec.c | 162 ++++++++++++++++++++++++++++--------------------
1 files changed, 95 insertions(+), 67 deletions(-)
diffs (263 lines):
diff -r 6fc8017dcac0 -r 72ce95b7b1e9 sys/dev/acpi/acpi_ec.c
--- a/sys/dev/acpi/acpi_ec.c Wed Nov 12 13:40:16 2003 +0000
+++ b/sys/dev/acpi/acpi_ec.c Wed Nov 12 13:59:23 2003 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: acpi_ec.c,v 1.19 2003/11/12 13:18:24 yamt Exp $ */
+/* $NetBSD: acpi_ec.c,v 1.20 2003/11/12 13:59:23 yamt Exp $ */
/*
* Copyright 2001 Wasabi Systems, Inc.
@@ -172,7 +172,7 @@
*****************************************************************************/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.19 2003/11/12 13:18:24 yamt Exp $");
+__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.20 2003/11/12 13:59:23 yamt Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -209,6 +209,7 @@
uint32_t sc_csrvalue; /* saved control register */
struct lock sc_lock; /* serialize operations to this EC */
+ struct simplelock sc_slock; /* protect against interrupts */
UINT32 sc_glkhandle; /* global lock handle */
UINT32 sc_glk; /* need global lock? */
} *acpiec_ecdt_softc;
@@ -218,7 +219,8 @@
NULL
};
-#define EC_F_PENDQUERY 0x02 /* query is pending */
+#define EC_F_TRANSACTION 0x01 /* doing transaction */
+#define EC_F_PENDQUERY 0x02 /* query is pending */
/*
* how long to wait to acquire global lock.
@@ -236,45 +238,6 @@
#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 (lockstatus(&sc->sc_lock) == LK_EXCLUSIVE);
-}
-
-static __inline void
-EcLock(struct acpi_ec_softc *sc)
-{
- ACPI_STATUS status;
-
- lockmgr(&sc->sc_lock, LK_EXCLUSIVE, NULL);
- if (sc->sc_glk) {
- status = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT,
- &sc->sc_glkhandle);
- if (ACPI_FAILURE(status)) {
- printf("%s: failed to acquire global lock\n",
- sc->sc_dev.dv_xname);
- lockmgr(&sc->sc_lock, LK_RELEASE, NULL);
- return;
- }
- }
-}
-
-static __inline void
-EcUnlock(struct acpi_ec_softc *sc)
-{
- ACPI_STATUS status;
-
- if (sc->sc_glk) {
- status = AcpiReleaseGlobalLock(sc->sc_glkhandle);
- if (ACPI_FAILURE(status))
- printf("%s: failed to release global lock\n",
- sc->sc_dev.dv_xname);
- }
- lockmgr(&sc->sc_lock, LK_RELEASE, NULL);
-}
-
typedef struct {
EC_COMMAND Command;
UINT8 Address;
@@ -297,6 +260,11 @@
UINT8 *Data);
static ACPI_STATUS EcWrite(struct acpi_ec_softc *sc, UINT8 Address,
UINT8 *Data);
+static void EcGpeQueryHandler(void *);
+static __inline int EcIsLocked(struct acpi_ec_softc *);
+static __inline void EcLock(struct acpi_ec_softc *);
+static __inline void EcUnlock(struct acpi_ec_softc *);
+
int acpiec_match(struct device *, struct cfdata *, void *);
void acpiec_attach(struct device *, struct device *, void *);
@@ -304,6 +272,78 @@
CFATTACH_DECL(acpiec, sizeof(struct acpi_ec_softc),
acpiec_match, acpiec_attach, NULL, NULL);
+static __inline int
+EcIsLocked(struct acpi_ec_softc *sc)
+{
+
+ return (lockstatus(&sc->sc_lock) == LK_EXCLUSIVE);
+}
+
+static __inline void
+EcLock(struct acpi_ec_softc *sc)
+{
+ ACPI_STATUS status;
+ int s;
+
+ lockmgr(&sc->sc_lock, LK_EXCLUSIVE, NULL);
+ if (sc->sc_glk) {
+ status = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT,
+ &sc->sc_glkhandle);
+ if (ACPI_FAILURE(status)) {
+ printf("%s: failed to acquire global lock\n",
+ sc->sc_dev.dv_xname);
+ lockmgr(&sc->sc_lock, LK_RELEASE, NULL);
+ return;
+ }
+ }
+
+ s = splvm(); /* XXX */
+ simple_lock(&sc->sc_slock);
+ sc->sc_flags |= EC_F_TRANSACTION;
+ simple_unlock(&sc->sc_slock);
+ splx(s);
+}
+
+static __inline void
+EcUnlock(struct acpi_ec_softc *sc)
+{
+ ACPI_STATUS status;
+ int s;
+
+ /*
+ * 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.
+ */
+ s = splvm(); /* XXX */
+ simple_lock(&sc->sc_slock);
+ if (sc->sc_flags & EC_F_PENDQUERY) {
+ ACPI_STATUS Status2;
+
+ Status2 = AcpiOsQueueForExecution(OSD_PRIORITY_HIGH,
+ EcGpeQueryHandler, sc);
+ if (ACPI_FAILURE(Status2))
+ printf("%s: unable to queue pending query: %s\n",
+ sc->sc_dev.dv_xname, AcpiFormatException(Status2));
+ sc->sc_flags &= ~EC_F_PENDQUERY;
+ }
+ sc->sc_flags &= ~EC_F_TRANSACTION;
+ simple_unlock(&sc->sc_slock);
+ splx(s);
+
+ if (sc->sc_glk) {
+ status = AcpiReleaseGlobalLock(sc->sc_glkhandle);
+ if (ACPI_FAILURE(status))
+ printf("%s: failed to release global lock\n",
+ sc->sc_dev.dv_xname);
+ }
+ lockmgr(&sc->sc_lock, LK_RELEASE, NULL);
+}
+
/*
* acpiec_match:
*
@@ -366,6 +406,7 @@
printf(": ACPI Embedded Controller\n");
lockinit(&sc->sc_lock, PWAIT, "eclock", 0, 0);
+ simple_lock_init(&sc->sc_slock);
sc->sc_node = aa->aa_node;
@@ -483,10 +524,14 @@
if ((EC_CSR_READ(sc) & EC_EVENT_SCI) == 0)
break;
+ EcLock(sc);
+
/*
* Find out why the EC is signalling us
*/
Status = EcQuery(sc, &Data);
+
+ EcUnlock(sc);
/*
* If we failed to get anything from the EC, give up.
@@ -535,7 +580,8 @@
* If EC is locked, the intr must process EcRead/Write wait only.
* Query request must be pending.
*/
- if (EcIsLocked(sc)) {
+ simple_lock(&sc->sc_slock);
+ if (sc->sc_flags & EC_F_TRANSACTION) {
csrvalue = EC_CSR_READ(sc);
if (csrvalue & EC_EVENT_SCI)
sc->sc_flags |= EC_F_PENDQUERY;
@@ -545,7 +591,9 @@
sc->sc_csrvalue = csrvalue;
wakeup(&sc->sc_csrvalue);
}
+ simple_unlock(&sc->sc_slock);
} else {
+ simple_unlock(&sc->sc_slock);
/* Enqueue GpeQuery handler. */
Status = AcpiOsQueueForExecution(OSD_PRIORITY_HIGH,
EcGpeQueryHandler, Context);
@@ -713,15 +761,15 @@
{
ACPI_STATUS Status;
- EcLock(sc);
+ if (EcIsLocked(sc) == 0)
+ printf("%s: EcQuery called without EC lock!\n",
+ sc->sc_dev.dv_xname);
EC_CSR_WRITE(sc, EC_COMMAND_QUERY);
- Status = EcWaitEvent(sc, EC_EVENT_OUTPUT_BUFFER_FULL);
+ Status = EcWaitEventIntr(sc, EC_EVENT_OUTPUT_BUFFER_FULL);
if (ACPI_SUCCESS(Status))
*Data = EC_DATA_READ(sc);
- EcUnlock(sc);
-
if (ACPI_FAILURE(Status))
printf("%s: timed out waiting for EC to respond to "
"EC_COMMAND_QUERY\n", sc->sc_dev.dv_xname);
@@ -753,26 +801,6 @@
break;
}
- /*
- * 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) {
- ACPI_STATUS Status2;
-
- Status2 = AcpiOsQueueForExecution(OSD_PRIORITY_HIGH,
- EcGpeQueryHandler, sc);
- if (ACPI_FAILURE(Status2))
- printf("%s: unable to queue pending query: %s\n",
- sc->sc_dev.dv_xname, AcpiFormatException(Status2));
- sc->sc_flags &= ~EC_F_PENDQUERY;
- }
-
EcUnlock(sc);
return(Status);
Home |
Main Index |
Thread Index |
Old Index