Re: [PATCH v5 3/7] arm64: dts: qcom: nord-rrd: Enable ADSP and CDSPs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: qcom: identical firmware and DTB paths for all CDSP instances
--

commit df6b28010f1754564cc47d083b73f91275527a95
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

arm64: dts: qcom: nord-rrd: Enable ADSP and CDSPs

This commit enables the ADSP (HPASS DSP) and CDSPs remoteproc instances on
the Nord RRD board by specifying their firmware and DTB paths.

> diff --git a/arch/arm64/boot/dts/qcom/nord-rrd.dts b/arch/arm64/boot/dts/qcom/nord-rrd.dts
> index 3673003c8b213..0b0acd33a2e19 100644
> --- a/arch/arm64/boot/dts/qcom/nord-rrd.dts
> +++ b/arch/arm64/boot/dts/qcom/nord-rrd.dts

[ ... ]

> +&remoteproc_cdsp0 {
> +	firmware-name = "qcom/nord/cdsp.mbn",
> +			"qcom/nord/cdsp_dtb.mbn";
> +
> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp1 {
> +	firmware-name = "qcom/nord/cdsp.mbn",
> +			"qcom/nord/cdsp_dtb.mbn";

[Severity: Medium]
Will this cause resource conflicts by sharing the exact same Device Tree Blob
(cdsp_dtb.mbn) across multiple CDSP instances?

The DTB typically contains DSP-specific hardware configurations like PAS IDs,
minidump allocations, and IPC settings. While sharing the base firmware
binary might be possible, assigning the same DTB to CDSP1 overrides the
unique default configurations.

For example, looking at nord_cdsp1_resource in qcom_q6v5_pas.c, the driver
explicitly expects unique names:

drivers/remoteproc/qcom_q6v5_pas.c:
	.firmware_name = "cdsp1.mbn",
	.dtb_firmware_name = "cdsp1_dtb.mbn",

Should CDSP1, CDSP2, and CDSP3 use their respective unique DTB names instead
of CDSP0's dtb?

> +
> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp2 {
> +	firmware-name = "qcom/nord/cdsp.mbn",
> +			"qcom/nord/cdsp_dtb.mbn";

[Severity: Medium]
Does this have the same issue of overriding the default cdsp2_dtb.mbn with
CDSP0's DTB?

> +
> +	status = "okay";
> +};
> +
> +&remoteproc_cdsp3 {
> +	firmware-name = "qcom/nord/cdsp.mbn",
> +			"qcom/nord/cdsp_dtb.mbn";

[Severity: Medium]
Does this similarly misconfigure CDSP3 by assigning it CDSP0's DTB?

> +
> +	status = "okay";
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014063953.549379-1-shengchao.guo@oss.qualcomm.com?part=3