Re: [PATCH v11 10/12] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The probe function incorrectly passes `dev->parent` to `spmi_get_parent_spmi_device()`, causing an unconditional probe failure.
--

--- Patch [10]: [PATCH v11 10/12] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add() ---
commit eab936776e34776b8e8c7c97d0effbd869a38db0
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 a new SPMI
sub-device instead of using the parent PMIC as a syscon. It initializes
a separate regmap with the sub-device's specific base address retrieved
from the devicetree.

> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
> --- a/drivers/misc/qcom-coincell.c
> +++ b/drivers/misc/qcom-coincell.c

[ ... ]

> @@ -92,16 +101,25 @@ static int qcom_coincell_probe(struct platform_device *pdev)
>  
>  	chgr.dev = &pdev->dev;
>  
> -	chgr.regmap = dev_get_regmap(pdev->dev.parent, NULL);
> -	if (!chgr.regmap) {
> -		dev_err(chgr.dev, "Unable to get regmap\n");
> -		return -EINVAL;
> -	}
> -
> -	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_get_parent_spmi_device(dev->parent);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this cause an unconditional probe failure?

If dev is the platform device, dev->parent is the SPMI device (the PMIC).
The spmi_get_parent_spmi_device() helper already inspects the parent of the
device passed to it. By passing dev->parent here, it evaluates
dev->parent->parent (the SPMI bus controller).

Since the bus controller's device type is &spmi_ctrl_type rather than
&spmi_dev_type, the helper's internal type check will fail and return NULL,
causing qcom_coincell_probe() to always return -ENODEV.

Should this pass dev instead of dev->parent?

> +	if (!sparent)
> +		return -ENODEV;
> +
> +	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/20260707101821.173319-1-angelogioacchino.delregno@collabora.com?part=10

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