Re: [PATCH v3 3/3] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Asynchronous Runtime PM put during device removal violates PM guidelines by racing with device detachment.
--

commit d6d74f48a42727f6a3e8aba532064a1c4577b574
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP

The proxy power domain enable path requests INT_MAX performance state
for every proxy PD. This commit introduces a proxy_pd_performance_states
array in qcom_pas_data to allow per-PD RPMH levels to be declared
explicitly, and adds Hawi CDSP remoteproc support.

> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 42bcbe72f95bd..6b23e711f03dc 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c

[ ... ]

> @@ -912,6 +922,14 @@ static int qcom_pas_probe(struct platform_device *pdev)
>  		goto unassign_mem;
>  	pas->proxy_pd_count = ret;
>  
> +	if (WARN(desc->proxy_pd_performance_states &&
> +		 desc->num_proxy_pd_performance_states != pas->proxy_pd_count,
> +		 "proxy_pd_performance_states count %u != pd count %d\n",
> +		 desc->num_proxy_pd_performance_states, pas->proxy_pd_count)) {
> +		ret = -EINVAL;
> +		goto detach_proxy_pds;
> +	}
> +
>  	ret = qcom_q6v5_init(&pas->q6v5, pdev, rproc, desc->crash_reason_smem,
>  			     desc->load_state, qcom_pas_handover);

[Severity: Medium]
This isn't a bug introduced by this patch, but looking at the driver removal
path:

drivers/remoteproc/qcom_q6v5_pas.c:qcom_pas_pds_disable() {
    ...
    pm_runtime_put(pds[i]);
    ...
}

This asynchronous put queues suspend work that will be cancelled by the
subsequent detach during driver removal:

drivers/remoteproc/qcom_q6v5_pas.c:qcom_pas_remove() {
    ...
    rproc_del(...);
    qcom_pas_pds_detach(pas, pas->proxy_pds, pas->proxy_pd_count);
    ...
}

This detaches and deletes the virtual devices, triggering pm_runtime_disable()
before the asynchronous suspend is guaranteed to complete.

Does this violate the PM guideline which mandates using pm_runtime_put_sync()
instead of pm_runtime_put() when device removal follows immediately?

While GenPD happens to gracefully handle the domain cleanup during detach by
decrementing the device count and queueing power-off work, relying on this
internal fallback appears to violate the API contract and leaves the device's
Runtime PM state machine in an incorrect intermediate state.

Can this be triggered locally by unbinding the driver?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909105049.1317985-1-mukesh.ojha@oss.qualcomm.com?part=3