Re: [PATCH v10 09/11] misc: qcom-coincell: 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] Leaked device reference count for the parent SPMI device acquired via spmi_find_device_by_of_node().
- [Low] Returning a pointer (`ERR_PTR(-ENODEV)`) from a function with an `int` return type.
--

--- Patch [9]: [PATCH v10 09/11] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add() ---
commit d4a4d2ba5d9e18044bfae04039df8731e13ea2f2
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()

This commit migrates the coincell charger driver to use the new
devm_spmi_subdevice_alloc_and_add() API. It registers a new SPMI
sub-device and initializes its own regmap.

> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
> index 3c57f74291476..46340f0199cdc 100644
> --- a/drivers/misc/qcom-coincell.c
> +++ b/drivers/misc/qcom-coincell.c
[ ... ]
> @@ -71,37 +71,58 @@ static int qcom_coincell_probe(struct platform_device *pdev)
>  {
[ ... ]
> -	rc = of_property_read_u32(node, "reg", &chgr.base_addr);
> +	rc = device_property_read_u32(dev, "reg", &qcom_coincell_regmap_config.reg_base);
>  	if (rc)
>  		return rc;
>  
> +	sparent = spmi_find_device_by_of_node(dev->parent->of_node);
> +	if (!sparent)
> +		return ERR_PTR(-ENODEV);

[Severity: Low]
The qcom_coincell_probe() function returns an int, but ERR_PTR(-ENODEV)
constructs a pointer. Will this cause a compiler error for implicit
pointer-to-integer conversion?

[Severity: Medium]
The spmi_find_device_by_of_node() function acquires a reference to the parent
SPMI device. Since devm_spmi_subdevice_alloc_and_add() does not consume this
reference, does this code leak the device reference by never calling
put_device() to release it?

These same return type mismatches and reference leaks also appear to be
present in the probe functions for qcom-spmi-sdam.c, qcom-pon.c, and
phy-qcom-eusb2-repeater.c introduced by this patchset.

> +
> +	sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
> +	if (IS_ERR(sub_sdev))
> +		return PTR_ERR(sub_sdev);

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

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