linux-kernel - Re: [PATCH v2 6/6] gpiolib: acpi: Deduplicate some code in __acpi_find

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9715c8dd-38df-48fd-a9d1-7a78163dc989@ijzerbout.nl>
Date: Tue, 8 Apr 2025 22:09:25 +0200
From: Kees Bakker <kees@...erbout.nl>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
 linux-gpio@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: Mika Westerberg <westeri@...nel.org>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH v2 6/6] gpiolib: acpi: Deduplicate some code in
 __acpi_find_gpio()

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@...ux.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