Re: [PATCH v11 11/12] iio: adc: qcom-spmi-iadc: 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] Unconditional probe failure due to an incorrect argument passed to `spmi_get_parent_spmi_device()`.
--

--- Patch [11]: [PATCH v11 11/12] iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add() ---
commit a65138c323454925e722f4d8782149a45b65f227
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

    iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add()

This commit migrates the Qualcomm SPMI IADC driver to use
devm_spmi_subdevice_alloc_and_add() to register an SPMI sub-device.
Instead of treating the parent PMIC as a syscon, it initializes its own
regmap with the sub-device's specific base address, allowing the regmap
API to handle register base offsets.

> diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
> index 0ec3a0c4b1de0..71049cdc27653 100644
> --- a/drivers/iio/adc/qcom-spmi-iadc.c
> +++ b/drivers/iio/adc/qcom-spmi-iadc.c

[ ... ]

> @@ -502,18 +508,24 @@ static int iadc_probe(struct platform_device *pdev)
>  	iadc = iio_priv(indio_dev);
>  	iadc->dev = dev;
>  
> -	iadc->regmap = dev_get_regmap(dev->parent, NULL);
> -	if (!iadc->regmap)
> +	sparent = spmi_get_parent_spmi_device(dev->parent);

[Severity: High]
Does this code inadvertently pass the wrong argument to
spmi_get_parent_spmi_device()?

Looking at the implementation of spmi_get_parent_spmi_device():

struct spmi_device *spmi_get_parent_spmi_device(struct device *dev)
{
	struct device *parent;

	if (dev && dev->parent) {
		parent = dev->parent;

		if (parent->type == &spmi_dev_type)
			return to_spmi_device(parent);
	}
	return NULL;
}

If we pass dev->parent, the helper will evaluate the grandparent's type
instead of the parent's. Since the grandparent is the SPMI controller
(&spmi_ctrl_type), this condition will fail, causing sparent to be NULL
and leading to an unconditional -ENODEV probe failure.

Should this be passing dev instead?

> +	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=11

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