linux-kernel - Re: [PATCH] greybus: gb-beagleplay: remove unneeded calls to devm_gpiod

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] [day] [month] [year] [list]

Message-ID: <85286090-e8da-4189-ad07-84876e0cf283@gmail.com>
Date: Tue, 24 Jun 2025 00:28:17 +0530
From: Ayush Singh <ayushdevel1325@...il.com>
To: Bartosz Golaszewski <brgl@...ev.pl>, Johan Hovold <johan@...nel.org>,
 Alex Elder <elder@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: greybus-dev@...ts.linaro.org, linux-kernel@...r.kernel.org,
 Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH] greybus: gb-beagleplay: remove unneeded calls to
 devm_gpiod_put()

On 6/18/25 13:50, Bartosz Golaszewski wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>
> gb_fw_init() is only called in this driver's probe() and we abort the
> probing if it fails. This means that calling devm_gpiod_put() in error
> path is not required as devres will already manage the releasing of the
> resources when the device is detached.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> ---
>   drivers/greybus/gb-beagleplay.c | 21 ++++-----------------
>   1 file changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c
> index da31f1131afc..1ea48b71a030 100644
> --- a/drivers/greybus/gb-beagleplay.c
> +++ b/drivers/greybus/gb-beagleplay.c
> @@ -1041,7 +1041,6 @@ static const struct fw_upload_ops cc1352_bootloader_ops = {
>   
>   static int gb_fw_init(struct gb_beagleplay *bg)
>   {
> -	int ret;
>   	struct fw_upload *fwl;
>   	struct gpio_desc *desc;
>   
> @@ -1060,29 +1059,17 @@ static int gb_fw_init(struct gb_beagleplay *bg)
>   	bg->bootloader_backdoor_gpio = desc;
>   
>   	desc = devm_gpiod_get(&bg->sd->dev, "reset", GPIOD_IN);
> -	if (IS_ERR(desc)) {
> -		ret = PTR_ERR(desc);
> -		goto free_boot;
> -	}
> +	if (IS_ERR(desc))
> +		return PTR_ERR(desc);
>   	bg->rst_gpio = desc;
>   
>   	fwl = firmware_upload_register(THIS_MODULE, &bg->sd->dev, "cc1352p7",
>   				       &cc1352_bootloader_ops, bg);
> -	if (IS_ERR(fwl)) {
> -		ret = PTR_ERR(fwl);
> -		goto free_reset;
> -	}
> +	if (IS_ERR(fwl))
> +		return PTR_ERR(fwl);
>   	bg->fwl = fwl;
>   
>   	return 0;
> -
> -free_reset:
> -	devm_gpiod_put(&bg->sd->dev, bg->rst_gpio);
> -	bg->rst_gpio = NULL;
> -free_boot:
> -	devm_gpiod_put(&bg->sd->dev, bg->bootloader_backdoor_gpio);
> -	bg->bootloader_backdoor_gpio = NULL;
> -	return ret;
>   }
>   
>   static void gb_fw_deinit(struct gb_beagleplay *bg)


Can you add a comment above the function that it should only ever be 
called from probing code since it does not free up the resources? I 
might end up forgetting and using it improperly in the future.


Best Regards,

Ayush Singh