Style cleanup, plus properly backup partial resource allocation in

AcpiOsInstallInterruptHandler() in the case of failure to initialize.

1 files changed, 51 insertions, 44 deletions

diff --git a/sys/dev/acpica/Osd/OsdInterrupt.c b/sys/dev/acpica/Osd/OsdInterrupt.c
index 42974cc..25fe2ce 100644
--- a/sys/dev/acpica/Osd/OsdInterrupt.c
+++ b/sys/dev/acpica/Osd/OsdInterrupt.c
@@ -31,29 +31,27 @@
  * 6.5 : Interrupt handling
  */
 
-#include "acpi.h"
-
+#include <sys/param.h>
+#include <sys/kernel.h>
 #include <sys/bus.h>
-#include <machine/resource.h>
 #include <machine/bus.h>
+#include <machine/resource.h>
 #include <sys/rman.h>
  
+#include "acpi.h"
 #include <dev/acpica/acpivar.h>
 
 #define _COMPONENT	ACPI_OS_SERVICES
 ACPI_MODULE_NAME("INTERRUPT")
 
 static void		InterruptWrapper(void *arg);
-static OSD_HANDLER	InterruptHandler;
 
-static UINT32 InterruptOverride = 0;
+static OSD_HANDLER	InterruptHandler;
+static UINT32		InterruptOverride = 0;
 
-/*
- * XXX this does not correctly free resources in the case of partically successful
- * attachment.
- */
 ACPI_STATUS
-AcpiOsInstallInterruptHandler(UINT32 InterruptNumber, OSD_HANDLER ServiceRoutine, void *Context)
+AcpiOsInstallInterruptHandler(UINT32 InterruptNumber,
+    OSD_HANDLER ServiceRoutine, void *Context)
 {
     struct acpi_softc	*sc;
 
@@ -64,71 +62,83 @@ AcpiOsInstallInterruptHandler(UINT32 InterruptNumber, OSD_HANDLER ServiceRoutine
     if (sc->acpi_dev == NULL)
 	panic("acpi softc has invalid device");
 
-    if ((InterruptNumber < 0) || (InterruptNumber > 255))
-	return_ACPI_STATUS(AE_BAD_PARAMETER);
+    if (InterruptNumber < 0 || InterruptNumber > 255)
+	return_ACPI_STATUS (AE_BAD_PARAMETER);
     if (ServiceRoutine == NULL)
-	return_ACPI_STATUS(AE_BAD_PARAMETER);
-    if (InterruptHandler != NULL && InterruptHandler != ServiceRoutine) {
-	device_printf(sc->acpi_dev, "can't register more than one ACPI interrupt\n");
-	return_ACPI_STATUS(AE_BAD_PARAMETER);
+	return_ACPI_STATUS (AE_BAD_PARAMETER);
+    if (InterruptHandler != NULL) {
+	device_printf(sc->acpi_dev, "interrupt handler already installed\n");
+	return_ACPI_STATUS (AE_ALREADY_EXISTS);
     }
     InterruptHandler = ServiceRoutine;
 
     /*
-     * This isn't strictly true, as we ought to be able to handle > 1 interrupt.  The ACPI
-     * spec doesn't call for this though.
+     * If the MADT contained an interrupt override directive for the SCI,
+     * we use that value instead of the one from the FADT.
      */
-    if (sc->acpi_irq != NULL) {
-	device_printf(sc->acpi_dev, "attempt to register more than one interrupt handler\n");
-	return_ACPI_STATUS(AE_ALREADY_EXISTS);
-    }
-    sc->acpi_irq_rid = 0;
     if (InterruptOverride != 0) {
 	    device_printf(sc->acpi_dev,
 		"Overriding SCI Interrupt from IRQ %u to IRQ %u\n",
 		InterruptNumber, InterruptOverride);
 	    InterruptNumber = InterruptOverride;
     }
+
+    /* Set up the interrupt resource. */
+    sc->acpi_irq_rid = 0;
     bus_set_resource(sc->acpi_dev, SYS_RES_IRQ, 0, InterruptNumber, 1);
-    if ((sc->acpi_irq = bus_alloc_resource_any(sc->acpi_dev, SYS_RES_IRQ,
-	&sc->acpi_irq_rid, RF_SHAREABLE | RF_ACTIVE)) == NULL) {
-	device_printf(sc->acpi_dev, "could not allocate SCI interrupt\n");
-	return_ACPI_STATUS(AE_ALREADY_EXISTS);
+    sc->acpi_irq = bus_alloc_resource_any(sc->acpi_dev, SYS_RES_IRQ,
+	&sc->acpi_irq_rid, RF_SHAREABLE | RF_ACTIVE);
+    if (sc->acpi_irq == NULL) {
+	device_printf(sc->acpi_dev, "could not allocate interrupt\n");
+	goto error;
     }
-    if (bus_setup_intr(sc->acpi_dev, sc->acpi_irq, INTR_TYPE_MISC, (driver_intr_t *)InterruptWrapper,
-		       Context, &sc->acpi_irq_handle)) {
-	device_printf(sc->acpi_dev, "could not set up SCI interrupt\n");
-	return_ACPI_STATUS(AE_ALREADY_EXISTS);
+    if (bus_setup_intr(sc->acpi_dev, sc->acpi_irq, INTR_TYPE_MISC,
+	InterruptWrapper, Context, &sc->acpi_irq_handle)) {
+	device_printf(sc->acpi_dev, "could not set up interrupt\n");
+	goto error;
     }
-	
-    return_ACPI_STATUS(AE_OK);
+
+    return_ACPI_STATUS (AE_OK);
+
+error:
+    if (sc->acpi_irq_handle)
+	bus_teardown_intr(sc->acpi_dev, sc->acpi_irq, sc->acpi_irq_handle);
+    sc->acpi_irq_handle = NULL;
+    if (sc->acpi_irq)
+	bus_release_resource(sc->acpi_dev, SYS_RES_IRQ, 0, sc->acpi_irq);
+    sc->acpi_irq = NULL;
+    bus_delete_resource(sc->acpi_dev, SYS_RES_IRQ, 0);
+    InterruptHandler = NULL;
+
+    return_ACPI_STATUS (AE_ALREADY_EXISTS);
 }
 
 ACPI_STATUS
-AcpiOsRemoveInterruptHandler (UINT32 InterruptNumber, OSD_HANDLER ServiceRoutine)
+AcpiOsRemoveInterruptHandler(UINT32 InterruptNumber, OSD_HANDLER ServiceRoutine)
 {
     struct acpi_softc	*sc;
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    if ((InterruptNumber < 0) || (InterruptNumber > 255))
-	return_ACPI_STATUS(AE_BAD_PARAMETER);
+    if (InterruptNumber < 0 || InterruptNumber > 255)
+	return_ACPI_STATUS (AE_BAD_PARAMETER);
     if (ServiceRoutine == NULL)
-	return_ACPI_STATUS(AE_BAD_PARAMETER);
+	return_ACPI_STATUS (AE_BAD_PARAMETER);
 
     if ((sc = devclass_get_softc(devclass_find("acpi"), 0)) == NULL)
 	panic("can't find ACPI device to deregister interrupt");
 
     if (sc->acpi_irq == NULL)
-	return_ACPI_STATUS(AE_NOT_EXIST);
+	return_ACPI_STATUS (AE_NOT_EXIST);
 
     bus_teardown_intr(sc->acpi_dev, sc->acpi_irq, sc->acpi_irq_handle);
     bus_release_resource(sc->acpi_dev, SYS_RES_IRQ, 0, sc->acpi_irq);
     bus_delete_resource(sc->acpi_dev, SYS_RES_IRQ, 0);
 
     sc->acpi_irq = NULL;
+    InterruptHandler = NULL;
 
-    return_ACPI_STATUS(AE_OK);
+    return_ACPI_STATUS (AE_OK);
 }
 
 ACPI_STATUS
@@ -138,14 +148,11 @@ acpi_OverrideInterruptLevel(UINT32 InterruptNumber)
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
     if (InterruptOverride != 0)
-	return_ACPI_STATUS(AE_ALREADY_EXISTS);
+	return_ACPI_STATUS (AE_ALREADY_EXISTS);
     InterruptOverride = InterruptNumber;
-    return_ACPI_STATUS(AE_OK);
+    return_ACPI_STATUS (AE_OK);
 }
 
-/*
- * Interrupt handler wrapper.
- */
 static void
 InterruptWrapper(void *arg)
 {