Re: [PATCH v3 09/20] iommu/fsl: use platform_device_set_of_node()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Mixing `goto`-based error handling with `__free()` cleanup in the same function.
--

commit 59e996ccb5b78974f4d3d6c2abcfbc8e49cb4d29
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

iommu/fsl: use platform_device_set_of_node()

Ahead of reworking the reference counting logic for platform devices,
encapsulate the assignment of the OF node for dynamically allocated
platform devices with the provided helper.

> diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
> index 25aa477a95a95..c83bbc3faad56 100644
> --- a/drivers/iommu/fsl_pamu.c
> +++ b/drivers/iommu/fsl_pamu.c
[ ... ]
> @@ -955,7 +955,8 @@ static __init int fsl_pamu_init(void)
>  	 * PAMU node would require significant changes to a lot of code.
>  	 */
>  
> -	np = of_find_compatible_node(NULL, NULL, "fsl,pamu");
> +	struct device_node *np __free(device_node) =
> +			of_find_compatible_node(NULL, NULL, "fsl,pamu");
>  	if (!np) {
>  		pr_err("could not find a PAMU node\n");
>  		return -ENODEV;
> @@ -964,7 +965,7 @@ static __init int fsl_pamu_init(void)
>  	ret = platform_driver_register(&fsl_of_pamu_driver);
>  	if (ret) {
>  		pr_err("could not register driver (err=%i)\n", ret);
> -		goto error_driver_register;
> +		return ret;
>  	}
>  
>  	pdev = platform_device_alloc("fsl-of-pamu", 0);
> @@ -973,7 +974,8 @@ static __init int fsl_pamu_init(void)
>  		ret = -ENOMEM;
>  		goto error_device_alloc;

[Severity: Medium]
Is it intentional to mix goto-based error handling with scope-based
cleanup in fsl_pamu_init()?

The cleanup helper documentation in include/linux/cleanup.h specifies that
goto and cleanup helpers should never be mixed in the same function to
prevent confusing ownership semantics.

Since pdev allocation and other initialization steps in this function still
use goto labels (like error_device_alloc and error_device_add), should they
all be converted to scope-based cleanup, or should np remain manually
managed to keep the error handling consistent?

> -	pdev->dev.of_node = of_node_get(np);
> +
> +	platform_device_set_of_node(pdev, np);
>  
>  	ret = pamu_domain_init();
>  	if (ret)
> @@ -988,17 +990,11 @@ static __init int fsl_pamu_init(void)
>  	return 0;
>  
>  error_device_add:
> -	of_node_put(pdev->dev.of_node);
> -	pdev->dev.of_node = NULL;
> -
>  	platform_device_put(pdev);
>  
>  error_device_alloc:
>  	platform_driver_unregister(&fsl_of_pamu_driver);
>  
> -error_driver_register:
> -	of_node_put(np);
> -
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-pdev-fwnode-ref-v3-0-1ff028e33779@oss.qualcomm.com?part=9