[PATCH] gpio: acpi: modernize resource management using cleanup.h

[Marco Scardovi <mscardovi95@gmail.com>]

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@gmail.com>
---
  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@linux.intel.com>
   */

+#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;
  }