Changes to implement 20040514:

* Add calls to AcpiSetGpeType.  We use wake/run as the type for lid and
button switches since wake-only causes Thinkpads to immediately wake on
the second suspend.  Note that with wake/run, some systems return both
wake and device-specific notifies so we don't register for system notifies
for lid and button switches.
* Remove the hw.acpi.osi_method tunable since it is not needed.
* Always print unknown notifies for all types.
* Add more cleanup for the EC if it fails to attach.
* Use the GPE handle now that we parse it.  This allows GPEs to be defined
in AML GPE blocks.
* Always use ACPI_NOT_ISR since it's ok to acquire a mutex in our thread
which processes queued requests.

5 files changed, 67 insertions, 45 deletions

diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index af035f2..53b643a 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -201,17 +201,6 @@ static int acpi_serialize_methods;
 TUNABLE_INT("hw.acpi.serialize_methods", &acpi_serialize_methods);
 
 /*
- * Allow override of whether to support the _OSI method.  This allows us
- * to claim compatibility with various MS OSs without changing the value
- * we report for _OS.  This is enabled by default since it fixes some
- * problems with interrupt routing although it can be disabled if it
- * causes problems.  See the definition of "AcpiGbl_ValidOsiStrings" for
- * a list of systems we claim.
- */
-static int acpi_osi_method = TRUE;
-TUNABLE_INT("hw.acpi.osi_method", &acpi_osi_method);
-
-/*
  * ACPI can only be loaded as a module by the loader; activating it after
  * system bootstrap time is not useful, and can be fatal to the system.
  * It also cannot be unloaded, since the entire system bus heirarchy hangs
@@ -261,11 +250,9 @@ acpi_Startup(void)
 
     /*
      * Set the globals from our tunables.  This is needed because ACPI-CA
-     * uses UINT8 for some values and we have no tunable_uint8.
+     * uses UINT8 for some values and we have no tunable_byte.
      */
-    AcpiGbl_AllMethodsSerialized = acpi_serialize_methods;
-    AcpiGbl_CreateOsiMethod = acpi_osi_method;
-    AcpiGbl_LeaveWakeGpesDisabled = FALSE;
+    AcpiGbl_AllMethodsSerialized = (UINT8)acpi_serialize_methods;
 
     /* Start up the ACPI CA subsystem. */
 #ifdef ACPI_DEBUGGER
@@ -2057,7 +2044,13 @@ acpi_device_enable_wake_event(ACPI_HANDLE h)
 	 * enabled for the wake event.
 	 */
 	gpe_bit = res->Package.Elements[0].Integer.Value;
-	status = AcpiEnableGpe(NULL, gpe_bit, ACPI_EVENT_WAKE_ENABLE);
+	status = AcpiSetGpeType(NULL, gpe_bit, ACPI_GPE_TYPE_WAKE_RUN);
+	if (ACPI_FAILURE(status)) {
+	    printf("wake enable: AcpiSetGpeType failed for %u\n",
+		   gpe_bit);
+	    goto out;
+	}
+	status = AcpiEnableGpe(NULL, gpe_bit, ACPI_NOT_ISR);
 	if (ACPI_FAILURE(status))
 	    printf("wake enable: AcpiEnableGpe failed for %u\n",
 		   gpe_bit);
@@ -2082,7 +2075,13 @@ acpi_device_enable_wake_event(ACPI_HANDLE h)
 	handle = acpi_GetReference(NULL, &res2->Package.Elements[0]);
 	if (handle == NULL || acpi_PkgInt32(res2, 1, &gpe_bit) != 0)
 	    goto out;
-	status = AcpiEnableGpe(handle, gpe_bit, ACPI_EVENT_WAKE_ENABLE);
+	status = AcpiSetGpeType(handle, gpe_bit, ACPI_GPE_TYPE_WAKE_RUN);
+	if (ACPI_FAILURE(status)) {
+	    printf("wake enable: AcpiSetGpeType failed for %u\n",
+		   gpe_bit);
+	    goto out;
+	}
+	status = AcpiEnableGpe(handle, gpe_bit, ACPI_NOT_ISR);
 	if (ACPI_FAILURE(status))
 	    printf("wake enable: AcpiEnableGpe (package) failed for %u\n",
 		   gpe_bit);
diff --git a/sys/dev/acpica/acpi_acad.c b/sys/dev/acpica/acpi_acad.c
index af22286..d6b2e20 100644
--- a/sys/dev/acpica/acpi_acad.c
+++ b/sys/dev/acpica/acpi_acad.c
@@ -116,11 +116,9 @@ acpi_acad_get_status(void *context)
 static void
 acpi_acad_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context)
 {
-    device_t dev = context;
-
-    ACPI_VPRINT(dev, acpi_device_get_parent_softc(dev),
-		"Notify 0x%x\n", notify);
+    device_t dev;
 
+    dev = (device_t)context;
     switch (notify) {
     case ACPI_DEVICE_CHECK_PNP:
     case ACPI_DEVICE_CHECK_EXISTENCE:
@@ -129,6 +127,7 @@ acpi_acad_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context)
 	AcpiOsQueueForExecution(OSD_PRIORITY_LO, acpi_acad_get_status, context);
 	break;
     default:
+	device_printf(dev, "unknown notify %#x\n", notify);
 	break;
     }
 }
diff --git a/sys/dev/acpica/acpi_button.c b/sys/dev/acpica/acpi_button.c
index 8071102..121ebec 100644
--- a/sys/dev/acpica/acpi_button.c
+++ b/sys/dev/acpica/acpi_button.c
@@ -141,6 +141,11 @@ acpi_button_attach(device_t dev)
 	status = AcpiInstallFixedEventHandler(event,
 			acpi_button_fixed_handler, sc);
     } else {
+	/*
+	 * If a system does not get lid events, it may make sense to change
+	 * the type to ACPI_ALL_NOTIFY.  Some systems generate both a wake
+	 * and runtime notify in that case though.
+	 */
 	status = AcpiInstallNotifyHandler(sc->button_handle,
 			ACPI_DEVICE_NOTIFY, acpi_button_notify_handler, sc);
     }
@@ -231,10 +236,11 @@ acpi_button_notify_wakeup(void *arg)
 static void 
 acpi_button_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context)
 {
-    struct acpi_button_softc	*sc = (struct acpi_button_softc *)context;
+    struct acpi_button_softc	*sc;
 
     ACPI_FUNCTION_TRACE_U32((char *)(uintptr_t)__func__, notify);
 
+    sc = (struct acpi_button_softc *)context;
     switch (notify) {
     case ACPI_NOTIFY_BUTTON_PRESSED_FOR_SLEEP:
 	AcpiOsQueueForExecution(OSD_PRIORITY_LO,
@@ -245,7 +251,8 @@ acpi_button_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context)
 				acpi_button_notify_wakeup, sc);
 	break;   
     default:
-	break;		/* unknown notification value */
+	device_printf(sc->button_dev, "unknown notify %#x\n", notify);
+	break;
     }
 }
 
diff --git a/sys/dev/acpica/acpi_ec.c b/sys/dev/acpica/acpi_ec.c
index 7d65fc9..474f132 100644
--- a/sys/dev/acpica/acpi_ec.c
+++ b/sys/dev/acpica/acpi_ec.c
@@ -323,7 +323,7 @@ EcUnlock(struct acpi_ec_softc *sc)
     mtx_unlock(&sc->ec_mtx);
 }
 
-static void		EcGpeHandler(void *Context);
+static uint32_t		EcGpeHandler(void *Context);
 static ACPI_STATUS	EcSpaceSetup(ACPI_HANDLE Region, UINT32 Function, 
 				void *Context, void **return_Context);
 static ACPI_STATUS	EcSpaceHandler(UINT32 Function,
@@ -540,7 +540,6 @@ acpi_ec_attach(device_t dev)
     struct acpi_ec_softc	*sc;
     struct acpi_ec_params	*params;
     ACPI_STATUS			Status;
-    int				errval = 0;
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
@@ -565,8 +564,7 @@ acpi_ec_attach(device_t dev)
 			&sc->ec_data_rid, RF_ACTIVE);
     if (sc->ec_data_res == NULL) {
 	device_printf(dev, "can't allocate data port\n");
-	errval = ENXIO;
-	goto out;
+	goto error;
     }
     sc->ec_data_tag = rman_get_bustag(sc->ec_data_res);
     sc->ec_data_handle = rman_get_bushandle(sc->ec_data_res);
@@ -576,8 +574,7 @@ acpi_ec_attach(device_t dev)
 			&sc->ec_csr_rid, RF_ACTIVE);
     if (sc->ec_csr_res == NULL) {
 	device_printf(dev, "can't allocate command/status port\n");
-	errval = ENXIO;
-	goto out;
+	goto error;
     }
     sc->ec_csr_tag = rman_get_bustag(sc->ec_csr_res);
     sc->ec_csr_handle = rman_get_bushandle(sc->ec_csr_res);
@@ -592,8 +589,7 @@ acpi_ec_attach(device_t dev)
     if (ACPI_FAILURE(Status)) {
 	device_printf(dev, "can't install GPE handler for %s - %s\n",
 		      acpi_name(sc->ec_handle), AcpiFormatException(Status));
-	errval = ENXIO;
-	goto out;
+	goto error;
     }
 
     /* 
@@ -605,18 +601,31 @@ acpi_ec_attach(device_t dev)
     if (ACPI_FAILURE(Status)) {
 	device_printf(dev, "can't install address space handler for %s - %s\n",
 		      acpi_name(sc->ec_handle), AcpiFormatException(Status));
-	Status = AcpiRemoveGpeHandler(sc->ec_gpehandle, sc->ec_gpebit,
-				      &EcGpeHandler);
-	if (ACPI_FAILURE(Status))
-	    panic("Added GPE handler but can't remove it");
-	errval = ENXIO;
-	goto out;
+	goto error;
+    }
+
+    /* Enable runtime GPEs for the handler. */
+    Status = AcpiSetGpeType(sc->ec_gpehandle, sc->ec_gpebit,
+			    ACPI_GPE_TYPE_RUNTIME);
+    if (ACPI_FAILURE(Status)) {
+	device_printf(dev, "AcpiSetGpeType failed: %s\n",
+		      AcpiFormatException(Status));
+	goto error;
+    }
+    Status = AcpiEnableGpe(sc->ec_gpehandle, sc->ec_gpebit, ACPI_NOT_ISR);
+    if (ACPI_FAILURE(Status)) {
+	device_printf(dev, "AcpiEnableGpe failed: %s\n",
+		      AcpiFormatException(Status));
+	goto error;
     }
 
     ACPI_DEBUG_PRINT((ACPI_DB_RESOURCES, "acpi_ec_attach complete\n"));
     return (0);
 
- out:
+error:
+    AcpiRemoveGpeHandler(sc->ec_gpehandle, sc->ec_gpebit, &EcGpeHandler);
+    AcpiRemoveAddressSpaceHandler(sc->ec_handle, ACPI_ADR_SPACE_EC,
+	EcSpaceHandler);
     if (sc->ec_csr_res)
 	bus_release_resource(sc->ec_dev, SYS_RES_IOPORT, sc->ec_csr_rid, 
 			     sc->ec_csr_res);
@@ -624,7 +633,7 @@ acpi_ec_attach(device_t dev)
 	bus_release_resource(sc->ec_dev, SYS_RES_IOPORT, sc->ec_data_rid,
 			     sc->ec_data_res);
     mtx_destroy(&sc->ec_mtx);
-    return (errval);
+    return (ENXIO);
 }
 
 static void
@@ -689,7 +698,7 @@ EcGpeQueryHandler(void *Context)
 
 re_enable:
     /* Re-enable the GPE event so we'll get future requests. */
-    Status = AcpiEnableGpe(NULL, sc->ec_gpebit, ACPI_NOT_ISR);
+    Status = AcpiEnableGpe(sc->ec_gpehandle, sc->ec_gpebit, ACPI_NOT_ISR);
     if (ACPI_FAILURE(Status))
 	printf("EcGpeQueryHandler: AcpiEnableEvent failed\n");
 }
@@ -699,7 +708,7 @@ re_enable:
  * be handled by polling in EcWaitEvent().  This is because some ECs
  * treat events as level when they should be edge-triggered.
  */
-static void
+static uint32_t
 EcGpeHandler(void *Context)
 {
     struct acpi_ec_softc *sc = Context;
@@ -708,17 +717,19 @@ EcGpeHandler(void *Context)
     KASSERT(Context != NULL, ("EcGpeHandler called with NULL"));
 
     /* Disable further GPEs while we handle this one. */
-    AcpiDisableGpe(NULL, sc->ec_gpebit, ACPI_ISR);
+    AcpiDisableGpe(sc->ec_gpehandle, sc->ec_gpebit, ACPI_NOT_ISR);
 
     /* Schedule the GPE query handler. */
     Status = AcpiOsQueueForExecution(OSD_PRIORITY_GPE, EcGpeQueryHandler,
 		Context);
     if (ACPI_FAILURE(Status)) {
 	printf("Queuing GPE query handler failed.\n");
-	Status = AcpiEnableGpe(NULL, sc->ec_gpebit, ACPI_ISR);
+	Status = AcpiEnableGpe(sc->ec_gpehandle, sc->ec_gpebit, ACPI_NOT_ISR);
 	if (ACPI_FAILURE(Status))
 	    printf("EcGpeHandler: AcpiEnableEvent failed\n");
     }
+
+    return (0);
 }
 
 static ACPI_STATUS
diff --git a/sys/dev/acpica/acpi_lid.c b/sys/dev/acpica/acpi_lid.c
index 1758e17..5f64a62 100644
--- a/sys/dev/acpica/acpi_lid.c
+++ b/sys/dev/acpica/acpi_lid.c
@@ -100,7 +100,11 @@ acpi_lid_attach(device_t dev)
     sc->lid_dev = dev;
     sc->lid_handle = acpi_get_handle(dev);
 
-    /* Install notification handler */
+    /*
+     * If a system does not get lid events, it may make sense to change
+     * the type to ACPI_ALL_NOTIFY.  Some systems generate both a wake and
+     * runtime notify in that case though.
+     */
     AcpiInstallNotifyHandler(sc->lid_handle, ACPI_DEVICE_NOTIFY,
 			     acpi_lid_notify_handler, sc);
     acpi_device_enable_wake_capability(sc->lid_handle, 1);
@@ -167,16 +171,18 @@ acpi_lid_notify_status_changed(void *arg)
 static void 
 acpi_lid_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context)
 {
-    struct acpi_lid_softc	*sc = (struct acpi_lid_softc *)context;
+    struct acpi_lid_softc	*sc;
 
     ACPI_FUNCTION_TRACE_U32((char *)(uintptr_t)__func__, notify);
 
+    sc = (struct acpi_lid_softc *)context;
     switch (notify) {
     case ACPI_NOTIFY_STATUS_CHANGED:
 	AcpiOsQueueForExecution(OSD_PRIORITY_LO,
 				acpi_lid_notify_status_changed, sc);
 	break;
     default:
+	device_printf(sc->lid_dev, "unknown notify %#x\n", notify);
 	break;
     }