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

[Kees Bakker]

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@xxxxxxxxxxxxxxx>
> ---
>   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