drivers/gpio/gpiolib-acpi-core.c | 57 ++++++++++++++------------------ include/linux/acpi.h | 2 ++ include/linux/gpio/driver.h | 2 ++ 3 files changed, 28 insertions(+), 33 deletions(-)
Hi Andy, Mika, thanks for the patience: I was way too excited about my first patch on the linux kernel and let it takes over the reasoning and I'm sorry for that. I've moved the defines on the right places as requested and refactored the patches to be smaller and tested the build with KASAN and kmemleak as requested by @Mika without any visible regression as for now. Marco Scardovi (6): ACPI: Move DEFINE_FREE(acpi_free) to global header gpiolib: Move DEFINE_FREE(free_gpio_desc) to driver header gpio: acpi: ignore out-of-range pins in acpi_gpiochip_alloc_event() gpio: acpi: use guard(mutex) for conn_lock gpio: acpi: use cleanup.h for automated resource deallocation gpio: acpi: sort header inclusion alphabetically drivers/gpio/gpiolib-acpi-core.c | 57 ++++++++++++++------------------ include/linux/acpi.h | 2 ++ include/linux/gpio/driver.h | 2 ++ 3 files changed, 28 insertions(+), 33 deletions(-) -- 2.54.0
On Thu, May 07, 2026 at 05:56:41PM +0200, Marco Scardovi wrote: > Hi Andy, Mika, > thanks for the patience: I was way too excited about my first patch on > the linux kernel and let it takes over the reasoning and I'm sorry for that. > I've moved the defines on the right places as requested and refactored the > patches to be smaller and tested the build with KASAN and kmemleak as requested > by @Mika without any visible regression as for now. You MUST provide: - DCO (Developer certificate of origin) - commit messages Without that, it's no go *at all*. -- With Best Regards, Andy Shevchenko
Hi Andy, as requested I've added the missing Signed-off-by and related commits' messages. Let me know if it's everything alright now. Marco Scardovi (6): ACPI: Move DEFINE_FREE(acpi_free) to global header gpiolib: Move DEFINE_FREE(free_gpio_desc) to driver header gpio: acpi: ignore out-of-range pins in acpi_gpiochip_alloc_event() gpio: acpi: use guard(mutex) for conn_lock gpio: acpi: use cleanup.h for automated resource deallocation gpio: acpi: sort header inclusion alphabetically drivers/gpio/gpiolib-acpi-core.c | 57 ++++++++++++++------------------ include/linux/acpi.h | 2 ++ include/linux/gpio/driver.h | 2 ++ 3 files changed, 28 insertions(+), 33 deletions(-) -- 2.54.0
On Fri, May 08, 2026 at 08:17:23AM +0200, Marco Scardovi wrote: > Hi Andy, > as requested I've added the missing Signed-off-by and related commits' messages. > Let me know if it's everything alright now. Note, cover letter is not just for a simple comment like the above, it's mainly for the overview of the series as a whole, what it doing and why. No need to resend a series, just try to write down one in a reply to this message. -- With Best Regards, Andy Shevchenko
On Fri, May 08, 2026 at 10:21:11AM +0300, Andy Shevchenko wrote: > On Fri, May 08, 2026 at 08:17:23AM +0200, Marco Scardovi wrote: > > Hi Andy, > > as requested I've added the missing Signed-off-by and related commits' messages. > > Let me know if it's everything alright now. > > Note, cover letter is not just for a simple comment like the above, it's mainly > for the overview of the series as a whole, what it doing and why. No need to > resend a series, just try to write down one in a reply to this message. Also this is v4 of the series, you need to provide a changelog for each delta between two sequential versions (here we expect to see a v1->v2, v2->v3, and v3->v4). -- With Best Regards, Andy Shevchenko
Introduce DEFINE_FREE(acpi_free) in include/linux/acpi.h to provide
a standard way to perform automated memory deallocation for ACPI
objects. This enables the use of the __free(acpi_free) attribute
wherever ACPI_FREE() is required.
Signed-off-by: Marco Scardovi <mscardovi95@gmail.com>
---
include/linux/acpi.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 67effb91fa98..f58e704ee850 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -234,6 +234,8 @@ static inline struct acpi_table_header *acpi_get_table_pointer(char *signature,
}
DEFINE_FREE(acpi_put_table, struct acpi_table_header *, if (!IS_ERR_OR_NULL(_T)) acpi_put_table(_T))
+DEFINE_FREE(acpi_free, void *, if (_T) ACPI_FREE(_T))
+
int acpi_table_parse(char *id, acpi_tbl_table_handler handler);
int __init_or_acpilib acpi_table_parse_entries(char *id,
unsigned long table_size, int entry_id,
--
2.54.0On Fri, May 8, 2026 at 8:18 AM Marco Scardovi <mscardovi95@gmail.com> wrote: > > Introduce DEFINE_FREE(acpi_free) in include/linux/acpi.h to provide > a standard way to perform automated memory deallocation for ACPI > objects. This enables the use of the __free(acpi_free) attribute > wherever ACPI_FREE() is required. This goes a bit overboard because the objects needing ACPI_FREE() for freeing are most of the time allocated in a way that precludes using __free(). Note that you are expected to initialize the pointer at declaration time when using __free() and that cannot be done with objects allocated by ACPICA code in many cases. > Signed-off-by: Marco Scardovi <mscardovi95@gmail.com> > --- > include/linux/acpi.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 67effb91fa98..f58e704ee850 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -234,6 +234,8 @@ static inline struct acpi_table_header *acpi_get_table_pointer(char *signature, > } > DEFINE_FREE(acpi_put_table, struct acpi_table_header *, if (!IS_ERR_OR_NULL(_T)) acpi_put_table(_T)) > > +DEFINE_FREE(acpi_free, void *, if (_T) ACPI_FREE(_T)) > + > int acpi_table_parse(char *id, acpi_tbl_table_handler handler); > int __init_or_acpilib acpi_table_parse_entries(char *id, > unsigned long table_size, int entry_id, > -- > 2.54.0 > >
Define the free_gpio_desc cleanup handler in include/linux/gpio/driver.h.
This allows drivers to use the __free(free_gpio_desc) attribute for
automated deallocation of GPIO descriptors obtained via
gpiochip_request_own_desc().
Signed-off-by: Marco Scardovi <mscardovi95@gmail.com>
---
include/linux/gpio/driver.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 17511434ed07..7ee65b49056d 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -807,6 +807,8 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *gc,
enum gpiod_flags dflags);
void gpiochip_free_own_desc(struct gpio_desc *desc);
+DEFINE_FREE(free_gpio_desc, struct gpio_desc *, if (!IS_ERR_OR_NULL(_T)) gpiochip_free_own_desc(_T))
+
struct gpio_desc *
gpio_device_get_desc(struct gpio_device *gdev, unsigned int hwnum);
--
2.54.0Add a check to verify that the pin index retrieved from ACPI is within
the valid range for the GPIO chip (pin < chip->ngpio). If the pin is
out of range, an error is logged and the function returns early to
avoid invalid memory access.
Signed-off-by: Marco Scardovi <mscardovi95@gmail.com>
---
drivers/gpio/gpiolib-acpi-core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index eb8a40cfb7a9..e53d68578024 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -370,6 +370,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",
--
2.54.0Replace manual mutex_lock() and mutex_unlock() calls in
acpi_gpio_adr_space_handler() with the guard(mutex) helper. This
ensures the conn_lock is automatically released when exiting the
scope, preventing potential deadlocks and simplifying the logic.
Signed-off-by: Marco Scardovi <mscardovi95@gmail.com>
---
drivers/gpio/gpiolib-acpi-core.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index e53d68578024..33d6c3b6cdf0 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -1123,7 +1123,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) {
@@ -1155,17 +1155,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;
@@ -1173,8 +1171,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
--
2.54.0Refactor 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@gmail.com>
---
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;
}
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;
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;
}
--
2.54.0On Fri, May 8, 2026 at 8:18 AM Marco Scardovi <mscardovi95@gmail.com> 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@gmail.com>
> ---
> 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;
> }
>
Reorder the #include directives in gpiolib-acpi-core.c to follow
alphabetical order. This improves code readability and ensures
consistency with kernel coding style for header management.
Signed-off-by: Marco Scardovi <mscardovi95@gmail.com>
---
drivers/gpio/gpiolib-acpi-core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpiolib-acpi-core.c b/drivers/gpio/gpiolib-acpi-core.c
index c9b12e24de14..7b324dd4ae67 100644
--- a/drivers/gpio/gpiolib-acpi-core.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -8,12 +8,14 @@
*/
#include <linux/acpi.h>
+#include <linux/cleanup.h>
#include <linux/dmi.h>
#include <linux/errno.h>
#include <linux/export.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/mutex.h>
+#include <linux/slab.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/gpio/consumer.h>
--
2.54.0© 2016 - 2026 Red Hat, Inc.