[PATCH] gpio: acpi: modernize resource management using cleanup.h
From: Marco Scardovi
Date: Wed May 06 2026 - 05:32:23 EST
Hi everyone,
I was looking for ways to switch to modern cleanup guards and auto-freeing
pointers to simplify error paths and synchronization in gpiolib-acpi-core.c
so I came up with the patch you can find below.
Here you can see the main points I've worked on:
- Use DEFINE_FREE() for gpio_desc and ACPI resources.
- Use guard(mutex)() within the OpRegion handler loop for automatic locking.
- Use __free() for automatic descriptor and memory cleanup.
- Fix off-by-one error in GPIO pin bounds check.
- Return AE_OK on out-of-range pins to allow processing other resources
even if one is misconfigured in firmware.
- Use break instead of goto in OpRegion handler for cleaner control flow
leveraging auto-cleanup.
I've tested it (both build and functionality) against linux-next-20260430.
Signed-off-by: Marco Scardovi <mscardovi95@xxxxxxxxx>
---
drivers/gpio/gpiolib-acpi-core.c | 94 +++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 51 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index eb8a40cfb7a9..19a18222b7b2 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -7,6 +7,9 @@
* Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
*/
+#include <linux/cleanup.h>
+#include <linux/slab.h>
+
#include <linux/acpi.h>
#include <linux/dmi.h>
#include <linux/errno.h>
@@ -23,6 +26,16 @@
#include "gpiolib.h"
#include "gpiolib-acpi.h"
+DEFINE_FREE(free_gpio_desc, struct gpio_desc *, {
+ if (_T)
+ gpiochip_free_own_desc(_T);
+})
+
+DEFINE_FREE(acpi_free, void *, {
+ if (_T)
+ ACPI_FREE(_T);
+})
+
/**
* struct acpi_gpio_event - ACPI GPIO event handler data
*
@@ -361,6 +374,7 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
struct acpi_gpio_event *event;
irq_handler_t handler = NULL;
struct gpio_desc *desc;
+ struct gpio_desc *desc_guard __free(free_gpio_desc) = NULL;
unsigned int pin;
int ret, irq;
@@ -370,6 +384,11 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
handle = ACPI_HANDLE(chip->parent);
pin = agpio->pin_table[0];
+ if (pin >= chip->ngpio) {
+ dev_err(chip->parent, "Failed to request GPIO for pin 0x%04X, out of range\n", pin);
+ return AE_OK;
+ }
+
if (pin <= 255) {
char ev_name[8];
sprintf(ev_name, "_%c%02X",
@@ -392,31 +411,26 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
desc = acpi_request_own_gpiod(chip, agpio, 0, "ACPI:Event");
if (IS_ERR(desc)) {
- dev_err(chip->parent,
- "Failed to request GPIO for pin 0x%04X, err %pe\n",
- pin, desc);
+ dev_err(chip->parent, "Failed to request GPIO for pin 0x%04X, err %pe\n", pin, desc);
return AE_OK;
}
+ desc_guard = desc;
ret = gpiochip_lock_as_irq(chip, pin);
if (ret) {
- dev_err(chip->parent,
- "Failed to lock GPIO pin 0x%04X as interrupt, err %d\n",
- pin, ret);
- goto fail_free_desc;
+ dev_err(chip->parent, "Failed to lock GPIO pin 0x%04X as interrupt, err %d\n", pin, ret);
+ return AE_OK;
}
irq = gpiod_to_irq(desc);
if (irq < 0) {
- dev_err(chip->parent,
- "Failed to translate GPIO pin 0x%04X to IRQ, err %d\n",
- pin, irq);
- goto fail_unlock_irq;
+ dev_err(chip->parent, "Failed to translate GPIO pin 0x%04X to IRQ, err %d\n", pin, irq);
+ goto err_unlock;
}
event = kzalloc_obj(*event);
if (!event)
- goto fail_unlock_irq;
+ goto err_unlock;
event->irqflags = IRQF_ONESHOT;
if (agpio->triggering == ACPI_LEVEL_SENSITIVE) {
@@ -444,17 +458,15 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
event->irq = irq;
event->irq_is_wake = acpi_gpio_irq_is_wake(chip->parent, agpio);
event->pin = pin;
- event->desc = desc;
+ /* Transfer ownership to event, prevent auto-free */
+ event->desc = no_free_ptr(desc_guard);
list_add_tail(&event->node, &acpi_gpio->events);
return AE_OK;
-fail_unlock_irq:
+err_unlock:
gpiochip_unlock_as_irq(chip, pin);
-fail_free_desc:
- gpiochip_free_own_desc(desc);
-
return AE_OK;
}
@@ -1086,7 +1098,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
struct acpi_gpio_chip *achip = region_context;
struct gpio_chip *chip = achip->chip;
struct acpi_resource_gpio *agpio;
- struct acpi_resource *ares;
+ struct acpi_resource *ares __free(acpi_free) = NULL;
u16 pin_index = address;
acpi_status status;
int length;
@@ -1097,20 +1109,17 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
if (ACPI_FAILURE(status))
return status;
- if (WARN_ON(ares->type != ACPI_RESOURCE_TYPE_GPIO)) {
- ACPI_FREE(ares);
+ if (WARN_ON(ares->type != ACPI_RESOURCE_TYPE_GPIO))
return AE_BAD_PARAMETER;
- }
agpio = &ares->data.gpio;
if (WARN_ON(agpio->io_restriction == ACPI_IO_RESTRICT_INPUT &&
- function == ACPI_WRITE)) {
- ACPI_FREE(ares);
+ function == ACPI_WRITE))
return AE_BAD_PARAMETER;
- }
length = min(agpio->pin_table_length, pin_index + bits);
+ status = AE_OK;
for (i = pin_index; i < length; ++i) {
unsigned int pin = agpio->pin_table[i];
struct acpi_gpio_connection *conn;
@@ -1118,7 +1127,7 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
u16 word, shift;
bool found;
- mutex_lock(&achip->conn_lock);
+ guard(mutex)(&achip->conn_lock);
found = false;
list_for_each_entry(conn, &achip->conns, node) {
@@ -1150,17 +1159,15 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
if (!found) {
desc = acpi_request_own_gpiod(chip, agpio, i, "ACPI:OpRegion");
if (IS_ERR(desc)) {
- mutex_unlock(&achip->conn_lock);
status = AE_ERROR;
- goto out;
+ break;
}
conn = kzalloc_obj(*conn);
if (!conn) {
gpiochip_free_own_desc(desc);
- mutex_unlock(&achip->conn_lock);
status = AE_NO_MEMORY;
- goto out;
+ break;
}
conn->pin = pin;
@@ -1168,8 +1175,6 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
list_add_tail(&conn->node, &achip->conns);
}
- mutex_unlock(&achip->conn_lock);
-
/*
* For the cases when OperationRegion() consists of more than
* 64 bits calculate the word and bit shift to use that one to
@@ -1188,8 +1193,6 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
}
}
-out:
- ACPI_FREE(ares);
return status;
}