git: ed60e694f4a9 - main - gpio: rework gpioaei

From: Ahmad Khalifa <vexeduxr_at_FreeBSD.org>
Date: Wed, 20 Aug 2025 06:59:49 UTC
The branch main has been updated by vexeduxr:

URL: https://cgit.FreeBSD.org/src/commit/?id=ed60e694f4a975eaf269ae3784aa6b4fd4aa582d

commit ed60e694f4a975eaf269ae3784aa6b4fd4aa582d
Author:     Ahmad Khalifa <vexeduxr@FreeBSD.org>
AuthorDate: 2025-08-20 06:04:12 +0000
Commit:     Ahmad Khalifa <vexeduxr@FreeBSD.org>
CommitDate: 2025-08-20 06:52:11 +0000

    gpio: rework gpioaei
    
    Rework gpioaei to make it support more than one pin per GPIO resource.
    Also allow one instance of gpioaei to handle multiple resources.
    
    Reviewed by:    imp, jhb
    Approved by:    imp (mentor)
    Differential Revision:  https://reviews.freebsd.org/D51584
---
 sys/dev/gpio/acpi_gpiobus.c    | 148 ++++++++++++++++--------------
 sys/dev/gpio/acpi_gpiobusvar.h |   6 +-
 sys/dev/gpio/gpioaei.c         | 204 ++++++++++++++++++++++++++++++-----------
 3 files changed, 235 insertions(+), 123 deletions(-)

diff --git a/sys/dev/gpio/acpi_gpiobus.c b/sys/dev/gpio/acpi_gpiobus.c
index 170f23615416..a9a2238695f7 100644
--- a/sys/dev/gpio/acpi_gpiobus.c
+++ b/sys/dev/gpio/acpi_gpiobus.c
@@ -52,12 +52,11 @@ struct acpi_gpiobus_ctx {
 
 struct acpi_gpiobus_ivar
 {
-	struct gpiobus_ivar	gpiobus;	/* Must come first */
-	ACPI_HANDLE		dev_handle;	/* ACPI handle for bus */
-	uint32_t		flags;
+	struct gpiobus_ivar	gpiobus;
+	ACPI_HANDLE		handle;
 };
 
-static uint32_t
+uint32_t
 acpi_gpiobus_convflags(ACPI_RESOURCE_GPIO *gpio_res)
 {
 	uint32_t flags = 0;
@@ -150,70 +149,24 @@ acpi_gpiobus_enumerate_res(ACPI_RESOURCE *res, void *context)
 	return (AE_OK);
 }
 
-static struct acpi_gpiobus_ivar *
-acpi_gpiobus_setup_devinfo(device_t bus, device_t child,
-    ACPI_RESOURCE_GPIO *gpio_res)
-{
-	struct acpi_gpiobus_ivar *devi;
-
-	devi = malloc(sizeof(*devi), M_DEVBUF, M_NOWAIT | M_ZERO);
-	if (devi == NULL)
-		return (NULL);
-	resource_list_init(&devi->gpiobus.rl);
-
-	devi->flags = acpi_gpiobus_convflags(gpio_res);
-	if (acpi_quirks & ACPI_Q_AEI_NOPULL)
-		devi->flags &= ~GPIO_PIN_PULLUP;
-
-	devi->gpiobus.npins = 1;
-	if (gpiobus_alloc_ivars(&devi->gpiobus) != 0) {
-		free(devi, M_DEVBUF);
-		return (NULL);
-	}
-
-	for (int i = 0; i < devi->gpiobus.npins; i++)
-		devi->gpiobus.pins[i] = gpio_res->PinTable[i];
-
-	return (devi);
-}
-
 static ACPI_STATUS
 acpi_gpiobus_enumerate_aei(ACPI_RESOURCE *res, void *context)
 {
 	ACPI_RESOURCE_GPIO *gpio_res = &res->Data.Gpio;
-	struct acpi_gpiobus_ctx *ctx = context;
-	device_t bus = ctx->sc->sc_busdev;
-	device_t child;
-	struct acpi_gpiobus_ivar *devi;
+	uint32_t *npins = context, *pins = npins + 1;
 
-	/* Check that we have a GpioInt object. */
+	/*
+	 * Check that we have a GpioInt object.
+	 * Note that according to the spec this
+	 * should always be the case.
+	 */
 	if (res->Type != ACPI_RESOURCE_TYPE_GPIO)
 		return (AE_OK);
 	if (gpio_res->ConnectionType != ACPI_RESOURCE_GPIO_TYPE_INT)
 		return (AE_OK);
 
-	/* Add a child. */
-	child = device_add_child_ordered(bus, 0, "gpio_aei", DEVICE_UNIT_ANY);
-	if (child == NULL)
-		return (AE_OK);
-	devi = acpi_gpiobus_setup_devinfo(bus, child, gpio_res);
-	if (devi == NULL) {
-		device_delete_child(bus, child);
-		return (AE_OK);
-	}
-	device_set_ivars(child, devi);
-
-	for (int i = 0; i < devi->gpiobus.npins; i++) {
-		if (GPIOBUS_PIN_SETFLAGS(bus, child, 0, devi->flags &
-		    ~GPIO_INTR_MASK)) {
-			device_delete_child(bus, child);
-			return (AE_OK);
-		}
-	}
-
-	/* Pass ACPI information to children. */
-	devi->dev_handle = ctx->dev_handle;
-
+	for (int i = 0; i < gpio_res->PinTableLength; i++)
+		pins[(*npins)++] = gpio_res->PinTable[i];
 	return (AE_OK);
 }
 
@@ -296,6 +249,63 @@ err:
 	return (AE_BAD_PARAMETER);
 }
 
+static void
+acpi_gpiobus_attach_aei(struct acpi_gpiobus_softc *sc, ACPI_HANDLE handle)
+{
+	struct acpi_gpiobus_ivar *devi;
+	ACPI_HANDLE aei_handle;
+	device_t child;
+	uint32_t *pins;
+	ACPI_STATUS status;
+	int err;
+
+	status = AcpiGetHandle(handle, "_AEI", &aei_handle);
+	if (ACPI_FAILURE(status))
+		return;
+
+	/* pins[0] specifies the length of the array. */
+	pins = mallocarray(sc->super_sc.sc_npins + 1,
+	    sizeof(uint32_t), M_DEVBUF, M_WAITOK);
+	pins[0] = 0;
+
+	status = AcpiWalkResources(handle, "_AEI",
+	    acpi_gpiobus_enumerate_aei, pins);
+	if (ACPI_FAILURE(status)) {
+		device_printf(sc->super_sc.sc_busdev,
+		    "Failed to enumerate AEI resources\n");
+		free(pins, M_DEVBUF);
+		return;
+	}
+
+	child = BUS_ADD_CHILD(sc->super_sc.sc_busdev, 0, "gpio_aei",
+	    DEVICE_UNIT_ANY);
+	if (child == NULL) {
+		device_printf(sc->super_sc.sc_busdev,
+		    "Failed to add gpio_aei child\n");
+		free(pins, M_DEVBUF);
+		return;
+	}
+
+	devi = device_get_ivars(child);
+	devi->gpiobus.npins = pins[0];
+	devi->handle = aei_handle;
+
+	err = gpiobus_alloc_ivars(&devi->gpiobus);
+	if (err != 0) {
+		device_printf(sc->super_sc.sc_busdev,
+		    "Failed to allocate gpio_aei ivars\n");
+		device_delete_child(sc->super_sc.sc_busdev, child);
+		free(pins, M_DEVBUF);
+		return;
+	}
+
+	for (int i = 0; i < pins[0]; i++)
+		devi->gpiobus.pins[i] = pins[i + 1];
+	free(pins, M_DEVBUF);
+
+	bus_attach_children(sc->super_sc.sc_busdev);
+}
+
 static int
 acpi_gpiobus_probe(device_t dev)
 {
@@ -353,13 +363,8 @@ acpi_gpiobus_attach(device_t dev)
 	if (ACPI_FAILURE(status))
 		device_printf(dev, "Failed to enumerate GPIO resources\n");
 
-	/* Look for AEI children */
-	status = AcpiWalkResources(handle, "_AEI", acpi_gpiobus_enumerate_aei,
-	    &ctx);
-
-	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
-		device_printf(dev, "Failed to enumerate AEI resources\n");
-
+	/* Look for AEI child */
+	acpi_gpiobus_attach_aei(sc, handle);
 	return (0);
 }
 
@@ -390,10 +395,7 @@ acpi_gpiobus_read_ivar(device_t dev, device_t child, int which,
 
 	switch (which) {
 	case ACPI_GPIOBUS_IVAR_HANDLE:
-		*result = (uintptr_t)devi->dev_handle;
-		break;
-	case ACPI_GPIOBUS_IVAR_FLAGS:
-		*result = (uintptr_t)devi->flags;
+		*result = (uintptr_t)devi->handle;
 		break;
 	default:
 		return (gpiobus_read_ivar(dev, child, which, result));
@@ -402,6 +404,13 @@ acpi_gpiobus_read_ivar(device_t dev, device_t child, int which,
 	return (0);
 }
 
+static device_t
+acpi_gpiobus_add_child(device_t dev, u_int order, const char *name, int unit)
+{
+	return (gpiobus_add_child_common(dev, order, name, unit,
+	    sizeof(struct acpi_gpiobus_ivar)));
+}
+
 static device_method_t acpi_gpiobus_methods[] = {
 	/* Device interface */
 	DEVMETHOD(device_probe,		acpi_gpiobus_probe),
@@ -410,6 +419,7 @@ static device_method_t acpi_gpiobus_methods[] = {
 
 	/* Bus interface */
 	DEVMETHOD(bus_read_ivar,	acpi_gpiobus_read_ivar),
+	DEVMETHOD(bus_add_child,	acpi_gpiobus_add_child),
 
 	DEVMETHOD_END
 };
diff --git a/sys/dev/gpio/acpi_gpiobusvar.h b/sys/dev/gpio/acpi_gpiobusvar.h
index f8d502eab9d1..288e8bd0f2af 100644
--- a/sys/dev/gpio/acpi_gpiobusvar.h
+++ b/sys/dev/gpio/acpi_gpiobusvar.h
@@ -33,16 +33,16 @@
 #include <contrib/dev/acpica/include/acpi.h>
 
 enum acpi_gpiobus_ivars {
-	ACPI_GPIOBUS_IVAR_HANDLE	= 10600,
-	ACPI_GPIOBUS_IVAR_FLAGS,
+	ACPI_GPIOBUS_IVAR_HANDLE	= 10600
 };
 
 #define ACPI_GPIOBUS_ACCESSOR(var, ivar, type)			\
 	__BUS_ACCESSOR(acpi_gpiobus, var, ACPI_GPIOBUS, ivar, type)
 
 ACPI_GPIOBUS_ACCESSOR(handle,	HANDLE,		ACPI_HANDLE)
-ACPI_GPIOBUS_ACCESSOR(flags,	FLAGS,		uint32_t)
 
 #undef ACPI_GPIOBUS_ACCESSOR
 
+uint32_t acpi_gpiobus_convflags(ACPI_RESOURCE_GPIO *);
+
 #endif	/* __ACPI_GPIOBUS_H__ */
diff --git a/sys/dev/gpio/gpioaei.c b/sys/dev/gpio/gpioaei.c
index ecae8ccaf2fa..7b97277aaf61 100644
--- a/sys/dev/gpio/gpioaei.c
+++ b/sys/dev/gpio/gpioaei.c
@@ -45,13 +45,21 @@ enum gpio_aei_type {
 	ACPI_AEI_TYPE_EVT
 };
 
-struct gpio_aei_softc {
-	ACPI_HANDLE handle;
-	enum gpio_aei_type type;
-	int pin;
+struct gpio_aei_ctx {
+	SLIST_ENTRY(gpio_aei_ctx) next;
 	struct resource * intr_res;
-	int intr_rid;
 	void * intr_cookie;
+	ACPI_HANDLE handle;
+	gpio_pin_t gpio;
+	uint32_t pin;
+	int intr_rid;
+	enum gpio_aei_type type;
+};
+
+struct gpio_aei_softc {
+	SLIST_HEAD(, gpio_aei_ctx) aei_ctx;
+	ACPI_HANDLE dev_handle;
+	device_t dev;
 };
 
 static int
@@ -65,69 +73,157 @@ gpio_aei_probe(device_t dev)
 static void
 gpio_aei_intr(void * arg)
 {
-	struct gpio_aei_softc * sc = arg;
+	struct gpio_aei_ctx * ctx = arg;
 
 	/* Ask ACPI to run the appropriate _EVT, _Exx or _Lxx method. */
-	if (sc->type == ACPI_AEI_TYPE_EVT)
-		acpi_SetInteger(sc->handle, NULL, sc->pin);
+	if (ctx->type == ACPI_AEI_TYPE_EVT)
+		acpi_SetInteger(ctx->handle, NULL, ctx->pin);
 	else
-		AcpiEvaluateObject(sc->handle, NULL, NULL, NULL);
+		AcpiEvaluateObject(ctx->handle, NULL, NULL, NULL);
+}
+
+static ACPI_STATUS
+gpio_aei_enumerate(ACPI_RESOURCE * res, void * context)
+{
+	ACPI_RESOURCE_GPIO * gpio_res = &res->Data.Gpio;
+	struct gpio_aei_softc * sc = context;
+	uint32_t flags, maxpin;
+	device_t busdev;
+	int err;
+
+	/*
+	 * Check that we have a GpioInt object.
+	 * Note that according to the spec this
+	 * should always be the case.
+	 */
+	if (res->Type != ACPI_RESOURCE_TYPE_GPIO)
+		return (AE_OK);
+	if (gpio_res->ConnectionType != ACPI_RESOURCE_GPIO_TYPE_INT)
+		return (AE_OK);
+
+	flags = acpi_gpiobus_convflags(gpio_res);
+	if (acpi_quirks & ACPI_Q_AEI_NOPULL)
+		flags &= ~GPIO_PIN_PULLUP;
+
+	err = GPIO_PIN_MAX(acpi_get_device(sc->dev_handle), &maxpin);
+	if (err != 0)
+		return (AE_ERROR);
+
+	busdev = GPIO_GET_BUS(acpi_get_device(sc->dev_handle));
+	for (int i = 0; i < gpio_res->PinTableLength; i++) {
+		struct gpio_aei_ctx * ctx;
+		uint32_t pin = gpio_res->PinTable[i];
+
+		if (__predict_false(pin > maxpin)) {
+			device_printf(sc->dev,
+			    "Invalid pin 0x%x, max: 0x%x (bad ACPI tables?)\n",
+			    pin, maxpin);
+			continue;
+		}
+
+		ctx = malloc(sizeof(struct gpio_aei_ctx), M_DEVBUF, M_WAITOK);
+		ctx->type = ACPI_AEI_TYPE_UNKNOWN;
+		if (pin <= 255) {
+			char objname[5];	/* "_EXX" or "_LXX" */
+			sprintf(objname, "_%c%02X",
+			    (flags & GPIO_INTR_EDGE_MASK) ? 'E' : 'L', pin);
+			if (ACPI_SUCCESS(AcpiGetHandle(sc->dev_handle, objname,
+			    &ctx->handle)))
+				ctx->type = ACPI_AEI_TYPE_ELX;
+		}
+
+		if (ctx->type == ACPI_AEI_TYPE_UNKNOWN) {
+			if (ACPI_SUCCESS(AcpiGetHandle(sc->dev_handle, "_EVT",
+			    &ctx->handle)))
+				ctx->type = ACPI_AEI_TYPE_EVT;
+			else {
+				device_printf(sc->dev,
+				    "AEI Device type is unknown for pin 0x%x\n",
+				    pin);
+
+				free(ctx, M_DEVBUF);
+				continue;
+			}
+		}
+
+		err = gpio_pin_get_by_bus_pinnum(busdev, pin, &ctx->gpio);
+		if (err != 0) {
+			device_printf(sc->dev, "Cannot acquire pin 0x%x\n",
+			    pin);
+
+			free(ctx, M_DEVBUF);
+			continue;
+		}
+
+		err = gpio_pin_setflags(ctx->gpio, flags & ~GPIO_INTR_MASK);
+		if (err != 0) {
+			device_printf(sc->dev,
+			    "Cannot set pin flags for pin 0x%x\n", pin);
+
+			gpio_pin_release(ctx->gpio);
+			free(ctx, M_DEVBUF);
+			continue;
+		}
+
+		ctx->intr_rid = 0;
+		ctx->intr_res = gpio_alloc_intr_resource(sc->dev,
+		    &ctx->intr_rid, RF_ACTIVE, ctx->gpio,
+		    flags & GPIO_INTR_MASK);
+		if (ctx->intr_res == NULL) {
+			device_printf(sc->dev,
+			    "Cannot allocate an IRQ for pin 0x%x\n", pin);
+
+			gpio_pin_release(ctx->gpio);
+			free(ctx, M_DEVBUF);
+			continue;
+		}
+
+		err = bus_setup_intr(sc->dev, ctx->intr_res, INTR_TYPE_MISC |
+		    INTR_MPSAFE | INTR_EXCL | INTR_SLEEPABLE, NULL,
+		    gpio_aei_intr, ctx, &ctx->intr_cookie);
+		if (err != 0) {
+			device_printf(sc->dev,
+			    "Cannot set up an IRQ for pin 0x%x\n", pin);
+
+			bus_release_resource(sc->dev, ctx->intr_res);
+			gpio_pin_release(ctx->gpio);
+			free(ctx, M_DEVBUF);
+			continue;
+		}
+
+		ctx->pin = pin;
+		SLIST_INSERT_HEAD(&sc->aei_ctx, ctx, next);
+	}
+
+	return (AE_OK);
 }
 
 static int
 gpio_aei_attach(device_t dev)
 {
 	struct gpio_aei_softc * sc = device_get_softc(dev);
-	gpio_pin_t pin;
-	uint32_t flags;
 	ACPI_HANDLE handle;
-	int err;
+	ACPI_STATUS status;
 
 	/* This is us. */
 	device_set_desc(dev, "ACPI Event Information Device");
 
-	/* Store parameters needed by gpio_aei_intr. */
 	handle = acpi_gpiobus_get_handle(dev);
-	if (gpio_pin_get_by_child_index(dev, 0, &pin) != 0) {
-		device_printf(dev, "Unable to get the input pin\n");
+	status = AcpiGetParent(handle, &sc->dev_handle);
+	if (ACPI_FAILURE(status)) {
+		device_printf(dev, "Cannot get parent of %s\n",
+		    acpi_name(handle));
 		return (ENXIO);
 	}
 
-	sc->type = ACPI_AEI_TYPE_UNKNOWN;
-	sc->pin = pin->pin;
-
-	flags = acpi_gpiobus_get_flags(dev);
-	if (pin->pin <= 255) {
-		char objname[5];	/* "_EXX" or "_LXX" */
-		sprintf(objname, "_%c%02X",
-		    (flags & GPIO_INTR_EDGE_MASK) ? 'E' : 'L', pin->pin);
-		if (ACPI_SUCCESS(AcpiGetHandle(handle, objname, &sc->handle)))
-			sc->type = ACPI_AEI_TYPE_ELX;
-	}
-	if (sc->type == ACPI_AEI_TYPE_UNKNOWN) {
-		if (ACPI_SUCCESS(AcpiGetHandle(handle, "_EVT", &sc->handle)))
-			sc->type = ACPI_AEI_TYPE_EVT;
-	}
-
-	if (sc->type == ACPI_AEI_TYPE_UNKNOWN) {
-		device_printf(dev, "ACPI Event Information Device type is unknown");
-		return (ENOTSUP);
-	}
+	SLIST_INIT(&sc->aei_ctx);
+	sc->dev = dev;
 
-	/* Set up the interrupt. */
-	if ((sc->intr_res = gpio_alloc_intr_resource(dev, &sc->intr_rid,
-	    RF_ACTIVE, pin, flags & GPIO_INTR_MASK)) == NULL) {
-		device_printf(dev, "Cannot allocate an IRQ\n");
-		return (ENOTSUP);
-	}
-	err = bus_setup_intr(dev, sc->intr_res, INTR_TYPE_MISC | INTR_MPSAFE |
-	    INTR_EXCL | INTR_SLEEPABLE, NULL, gpio_aei_intr, sc,
-	    &sc->intr_cookie);
-	if (err != 0) {
-		device_printf(dev, "Cannot set up IRQ\n");
-		bus_release_resource(dev, SYS_RES_IRQ, sc->intr_rid,
-		    sc->intr_res);
-		return (err);
+	status = AcpiWalkResources(sc->dev_handle, "_AEI",
+	    gpio_aei_enumerate, sc);
+	if (ACPI_FAILURE(status)) {
+		device_printf(dev, "Failed to enumerate AEI resources\n");
+		return (ENXIO);
 	}
 
 	return (0);
@@ -137,9 +233,15 @@ static int
 gpio_aei_detach(device_t dev)
 {
 	struct gpio_aei_softc * sc = device_get_softc(dev);
+	struct gpio_aei_ctx * ctx, * tctx;
+
+	SLIST_FOREACH_SAFE(ctx, &sc->aei_ctx, next, tctx) {
+		bus_teardown_intr(dev, ctx->intr_res, ctx->intr_cookie);
+		bus_release_resource(dev, ctx->intr_res);
+		gpio_pin_release(ctx->gpio);
+		free(ctx, M_DEVBUF);
+	}
 
-	bus_teardown_intr(dev, sc->intr_res, sc->intr_cookie);
-	bus_release_resource(dev, SYS_RES_IRQ, sc->intr_rid, sc->intr_res);
 	return (0);
 }