Re: [PATCH 3/8] interconnect: qcom: icc-rpm: depopulate SMD RPM child NoCs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] interconnect: qcom: icc-rpm: WARN_ON() in qnoc_remove() for childless NoCs
--

commit e3eed275bb16fc9fb22f00c7b7dd10e369c4c8ab
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

interconnect: qcom: icc-rpm: depopulate SMD RPM child NoCs

This commit replaces of_platform_populate() in qnoc_probe() with
devm_of_platform_populate() and calls devm_of_platform_depopulate() in
qnoc_remove() to clean up child NoC devices before tearing down the parent
provider.

> diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
> index 6f325789a96ea..ddc7df31ed331 100644
> --- a/drivers/interconnect/qcom/icc-rpm.c
> +++ b/drivers/interconnect/qcom/icc-rpm.c
> @@ -677,7 +677,7 @@ int qnoc_probe(struct platform_device *pdev)
>  
>  	/* Populate child NoC devices if any */
>  	if (of_get_child_count(dev->of_node) > 0) {
> -		ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> +		ret = devm_of_platform_populate(dev);
>  		if (ret)
>  			goto err_deregister_provider;
>  	}
> @@ -699,6 +699,7 @@ void qnoc_remove(struct platform_device *pdev)
>  {
>  	struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
>  
> +	devm_of_platform_depopulate(&pdev->dev);
>  	icc_provider_deregister(&qp->provider);
>  	icc_nodes_remove(&qp->provider);
>  	clk_disable_unprepare(qp->bus_clk);

[Severity: Medium]
Will calling devm_of_platform_depopulate() unconditionally in qnoc_remove()
trigger a WARN_ON() when unbinding a NoC device without child nodes (or when
depopulating the child NoC devices themselves)?

In qnoc_probe(), devm_of_platform_populate() is only called when
of_get_child_count(dev->of_node) > 0. For any NoC device with 0 children, no
devm_of_platform_populate_release devres action is registered.

When qnoc_remove() later calls devm_of_platform_depopulate(&pdev->dev) on
such a device, devres_release() returns -ENOENT and triggers WARN_ON(ret):

drivers/of/platform.c:devm_of_platform_depopulate() {
	int ret;

	ret = devres_release(dev, devm_of_platform_populate_release,
			     devm_of_platform_match, dev);

	WARN_ON(ret);
}

Should devm_of_platform_depopulate() also be guarded, or could
devm_of_platform_populate() be called unconditionally (or
of_platform_populate() / of_platform_depopulate() used directly)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-msm8998-icc-v1-0-c2da3934b6c8@oss.qualcomm.com?part=3