Re: [PATCH v3 2/2] remoteproc: qcom: pas: Add Shikra remoteproc support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `ssctl_id` initialization for `shikra_lpaicp_resource` causing graceful shutdown failure.
--

commit de542ab4b48a2146f6973ba793bbbd6328de204c
Author: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>

remoteproc: qcom: pas: Add Shikra remoteproc support
    
This commit adds the CDSP, LPAICP and MPSS Peripheral Authentication Service
support for the Qualcomm Shikra SoC.

> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index da27d1d3c9da6..0101f1db6458e 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
[ ... ]
> @@ -1457,6 +1457,51 @@ static const struct qcom_pas_data sc7280_wpss_resource = {
[ ... ]
> +static const struct qcom_pas_data shikra_lpaicp_resource = {
> +	.crash_reason_smem = 682,
> +	.firmware_name = "lpaicp.mbn",
> +	.dtb_firmware_name = "lpaicp_dtb.mbn",
> +	.pas_id = 0x56,
> +	.dtb_pas_id = 0x57,
> +	.minidump_id = 0,
> +	.auto_boot = true,
> +	.ssr_name = "lpaicp",
> +	.sysmon_name = "lpaicp",
> +};

[Severity: High]
Does this resource definition need an ssctl_id initialized?

The sysmon subdevice is enabled by setting .sysmon_name = "lpaicp", but it
fails to define .ssctl_id. Because .ssctl_id is uninitialized, it defaults
to 0, which means qcom_add_sysmon_subdev() will be passed 0 as the QMI
SSCTL instance ID.

Looking at ssctl_new_server() in drivers/remoteproc/qcom_sysmon.c, the
version 2 QMI service binding requires svc->instance ==
sysmon->ssctl_instance. Since modern Qualcomm PAS QMI SSCTL services
advertise non-zero instance IDs, could this cause the binding to fail?

If the remoteproc framework never establishes an SSCTL connection to the
LPAICP DSP, sysmon_stop() will explicitly skip waiting for the SSCTL service
connection since ssctl_instance == 0.

When the DSP needs to be stopped, will sysmon_stop() bypass
ssctl_request_shutdown() and forcefully power off the DSP instead of
gracefully shutting down?

Forceful shutdowns of active DSPs can lead to hardware state corruption
or SMMU faults, which often cascade into kernel panics on these platforms.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-shikra-rproc-v3-0-2fca0bbe1ad7@oss.qualcomm.com?part=2