Re: [PATCH v4 5/6] gpio: acpi: use cleanup.h for automated resource deallocation

[Rafael J. Wysocki]

On Fri, May 8, 2026 at 8:18 AM Marco Scardovi <mscardovi95@xxxxxxxxx> wrote:
>
> Refactor acpi_gpiochip_alloc_event() and acpi_gpio_adr_space_handler()
> to use the __free() macro for GPIO descriptors and ACPI resources.
> This simplifies error handling by removing manual cleanup calls and
> reducing the need for goto labels.
>
> Signed-off-by: Marco Scardovi <mscardovi95@xxxxxxxxx>
> ---
>  drivers/gpio/gpiolib-acpi-core.c | 40 +++++++++++---------------------
>  1 file changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
> index 33d6c3b6cdf0..c9b12e24de14 100644
> --- a/drivers/gpio/gpiolib-acpi-core.c
> +++ b/drivers/gpio/gpiolib-acpi-core.c
> @@ -397,31 +397,27 @@ 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;
>         }
>
> +       struct gpio_desc *desc_guard __free(free_gpio_desc) = 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;

You are not supposed to mix up cleanup.h stuff with gotos.

>         }
>
>         event = kzalloc_obj(*event);
>         if (!event)
> -               goto fail_unlock_irq;
> +               goto err_unlock;
>
>         event->irqflags = IRQF_ONESHOT;
>         if (agpio->triggering == ACPI_LEVEL_SENSITIVE) {
> @@ -449,17 +445,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;
>  }
>
> @@ -1091,7 +1085,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;

Nope, that's not how you use __free().

>         u16 pin_index = address;
>         acpi_status status;
>         int length;
> @@ -1102,18 +1096,14 @@ 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);
>         for (i = pin_index; i < length; ++i) {
> @@ -1189,8 +1179,6 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
>                 }
>         }
>
> -out:
> -       ACPI_FREE(ares);
>         return status;
>  }
>