Re: [PATCH v2 6/6] gpiolib: acpi: Deduplicate some code in __acpi_find_gpio()

Kees Bakker
2025-04-08T20:19:25Z

Op 03-04-2025 om 17:59 schreef Andy Shevchenko:
> __acpi_find_gpio() calls two functions depending on the supplied con_id
> and possibility to fallback to _CRS lookup. Those functions have the same
> pieces of code that can be done only in one place. Do it so.
>
> This gives an impressive shrink of the generated code for x86_64:
>
> add/remove: 0/2 grow/shrink: 0/4 up/down: 0/-1204 (-1204)
> Function old new delta
> acpi_find_gpio.__UNIQUE_ID_ddebug478 56 - -56
> acpi_dev_gpio_irq_wake_get_by.__UNIQUE_ID_ddebug480 56 - -56
> acpi_find_gpio 354 216 -138
> acpi_get_gpiod_by_index 456 307 -149
> __acpi_find_gpio 877 638 -239
> acpi_dev_gpio_irq_wake_get_by 695 129 -566
> Total: Before=15358, After=14154, chg -7.84%
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/gpio/gpiolib-acpi.c | 101 +++++++++++++++++-------------------
> 1 file changed, 48 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index f44d25df15cb..b3fcb9d5a39f 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> [...]
> @@ -983,17 +966,24 @@ __acpi_find_gpio(struct fwnode_handle *fwnode, const char *con_id, unsigned int
> bool can_fallback, struct acpi_gpio_info *info)
> {
> struct acpi_device *adev = to_acpi_device_node(fwnode);
> + struct acpi_gpio_lookup lookup;
> struct gpio_desc *desc;
> char propname[32];
> + int ret;
> +
> + memset(&lookup, 0, sizeof(lookup));
> + lookup.params.crs_entry_index = idx;
>
> /* Try first from _DSD */
> for_each_gpio_property_name(propname, con_id) {
> if (adev)
> - desc = acpi_get_gpiod_by_index(adev,
> - propname, idx, info);
> + ret = acpi_get_gpiod_by_index(adev, propname, &lookup);
> else
> - desc = acpi_get_gpiod_from_data(fwnode,
> - propname, idx, info);
> + ret = acpi_get_gpiod_from_data(fwnode, propname, &lookup);
> + if (ret)
> + continue;
> +
> + desc = lookup.desc;
> if (PTR_ERR(desc) == -EPROBE_DEFER)
> return desc;
>
> @@ -1002,8 +992,13 @@ __acpi_find_gpio(struct fwnode_handle *fwnode, const char *con_id, unsigned int
> }
>
> /* Then from plain _CRS GPIOs */
> - if (can_fallback)
> - return acpi_get_gpiod_by_index(adev, NULL, idx, info);
> + if (can_fallback) {
> + ret = acpi_get_gpiod_by_index(adev, NULL, &lookup);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return lookup.desc;
> + }
>
> return ERR_PTR(-ENOENT);
> }
Please, check the changes in this function again.
`__acpi_find_gpio` doesn't fill `info` anymore. The callers of this
function will continue with
an uninitialized struct.
--
Kees