Re: [PATCH v10 07/11] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The SPMI parent device reference acquired by `spmi_find_device_by_of_node()` is permanently leaked on both success and error paths.
- [Low] Returning a pointer (`ERR_PTR(-ENODEV)`) from a function with an `int` return type.
--

--- Patch [7]: [PATCH v10 07/11] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add() ---
commit 7acdc04479f1ecdcac3a3f4d2ef16b8d339d843d
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

This commit migrates the Qualcomm PON driver to register an SPMI sub-device
and initialize a specific regmap. This allows the driver to avoid manually
adding the base address to every read/write call.

> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> index 7e108982a582e..f94aaecd6ed67 100644
> --- a/drivers/power/reset/qcom-pon.c
> +++ b/drivers/power/reset/qcom-pon.c

[ ... ]

> @@ -46,27 +46,44 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  
>  static int qcom_pon_probe(struct platform_device *pdev)
>  {

[ ... ]

> +	if (!dev->parent)
> +		return -ENODEV;
>  
>  	pon = devm_kzalloc(&pdev->dev, sizeof(*pon), GFP_KERNEL);
>  	if (!pon)
>  		return -ENOMEM;
>  
>  	pon->dev = &pdev->dev;
>  
> -	pon->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> -	if (!pon->regmap) {
> -		dev_err(&pdev->dev, "failed to locate regmap\n");
> -		return -ENODEV;
> -	}
> +	sparent = spmi_find_device_by_of_node(dev->parent->of_node);
> +	if (!sparent)
> +		return ERR_PTR(-ENODEV);

[Severity: Low]
Does this code return a pointer from a function that returns an integer?

qcom_pon_probe() has an int return type, so returning ERR_PTR(-ENODEV)
causes an implicit pointer-to-integer conversion and a build warning.
Should this just return -ENODEV?

(Note: A similar type conversion appears to exist in sdam_probe() in
drivers/nvmem/qcom-spmi-sdam.c)

[Severity: Medium]
Does this leak the device reference acquired by spmi_find_device_by_of_node()?

spmi_find_device_by_of_node() increments the reference count of the SPMI
parent device, but it appears put_device(&sparent->dev) is never called
on either the success or error paths to release it.

(Note: This same reference leak pattern also appears to be duplicated in
sdam_probe() in drivers/nvmem/qcom-spmi-sdam.c)

>  
> -	error = of_property_read_u32(pdev->dev.of_node, "reg",
> -				     &pon->baseaddr);
> +	sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707083730.33977-1-angelogioacchino.delregno@collabora.com?part=7

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy