Re: [PATCH] soc: qcom: rpmh-rsc: unwind notifiers on populate failure

[Konrad Dybcio]

On 6/16/26 2:56 AM, Pengpeng Hou wrote:
> rpmh_rsc_probe() registers either a genpd notifier or a CPU PM notifier
> before populating child devices.  The genpd path is unwound if child
> population fails, but the CPU PM notifier registration return value is
> ignored and the notifier remains registered on the same failure path.
> 
> Check the CPU PM notifier registration result and unregister it if child
> population fails.  Also depopulate any children that were created before
> devm_of_platform_populate() returned an error, because the devres cleanup
> action is only installed after successful population.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
> ---
>  drivers/soc/qcom/rpmh-rsc.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index c6f7d5c9c493..a75ff97423e8 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -1033,6 +1033,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>  	struct device_node *dn = pdev->dev.of_node;
>  	struct rsc_drv *drv;
>  	char drv_id[10] = {0};
> +	bool cpu_pm_registered = false;
>  	int ret, irq;
>  	u32 solver_config;
>  	u32 rsc_id;
> @@ -1107,7 +1108,10 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>  				return ret;
>  		} else {
>  			drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
> -			cpu_pm_register_notifier(&drv->rsc_pm);
> +			ret = cpu_pm_register_notifier(&drv->rsc_pm);
> +			if (ret)
> +				return ret;
> +			cpu_pm_registered = true;
>  		}
>  	}
>  
> @@ -1123,9 +1127,14 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>  	drv->dev = &pdev->dev;
>  
>  	ret = devm_of_platform_populate(&pdev->dev);
> -	if (ret && pdev->dev.pm_domain) {
> -		dev_pm_genpd_remove_notifier(&pdev->dev);
> -		pm_runtime_disable(&pdev->dev);
> +	if (ret) {
> +		of_platform_depopulate(&pdev->dev);

devm_ functions clean up after themselves already

> +		if (pdev->dev.pm_domain) {
> +			dev_pm_genpd_remove_notifier(&pdev->dev);
> +			pm_runtime_disable(&pdev->dev);

We can probably likewise turn pm_runtime_enable() into devm_pm_runtime_enable()

> +		} else if (cpu_pm_registered) {
> +			cpu_pm_unregister_notifier(&drv->rsc_pm);
> +		}

This is a valid change, deserving a Fixes tag, see:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

Konrad