Re: [PATCH v2] arm64: dts: qcom: sc8280xp: Mark FastRPC context banks as dma-coherent

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch modifies the SDSP (Sensor DSP) context banks instead of the ADSP (Audio DSP) context banks, contradicting the commit message and leaving the ADSP context banks without the intended fix.
--

commit 38a25d40245dd18744b644c71b8689bfd5e22715
Author: Xilin Wu <sophon@radxa.com>

arm64: dts: qcom: sc8280xp: Mark FastRPC context banks as dma-coherent

The FastRPC context banks are DMA-coherent on SC8280XP. Describe this in
the device tree so that the DMA API uses the appropriate coherency
attributes for their mappings.

Link: https://patch.msgid.link/20260507-sc8280xp-fastrpc-dma-coherent-v1-1-e92aed797865@radxa.com

> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index fbcfd3883bbb4..dfe7dbaebe62d 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -2910,18 +2910,21 @@ compute-cb@1 {
>  						compatible = "qcom,fastrpc-compute-cb";
>  						reg = <1>;
>  						iommus = <&apps_smmu 0x0521 0x0>;
> +						dma-coherent;
>  					};
>  
>  					compute-cb@2 {
>  						compatible = "qcom,fastrpc-compute-cb";
>  						reg = <2>;
>  						iommus = <&apps_smmu 0x0522 0x0>;
> +						dma-coherent;
>  					};
>  
>  					compute-cb@3 {
>  						compatible = "qcom,fastrpc-compute-cb";
>  						reg = <3>;
>  						iommus = <&apps_smmu 0x0523 0x0>;
> +						dma-coherent;
>  					};
>  				};
>  			};

[Severity: Medium]
Are these changes modifying the SDSP context banks instead of the ADSP banks?

The commit message states:
"Mark the ADSP and CDSP FastRPC context banks as dma-coherent to fix these
failures and allow QAIRT applications to run on the CDSP."

However, this block of changes in arch/arm64/boot/dts/qcom/sc8280xp.dtsi is
under the slpi remoteproc node, which corresponds to the SDSP
(label = "sdsp").

The actual ADSP context banks under remoteproc_adsp appear to have been
missed by this patch:

arch/arm64/boot/dts/qcom/sc8280xp.dtsi:
remoteproc_adsp: remoteproc@3000000 {
	...
	fastrpc {
		label = "adsp";
		...
		compute-cb@3 {
			compatible = "qcom,fastrpc-compute-cb";
			reg = <3>;
			iommus = <&apps_smmu 0x0c03 0x0>;
		};

Could it be that the dma-coherent property was mistakenly added to the SDSP
banks instead of the ADSP banks? Or is it intended for the SDSP banks to also
receive this property, while the ADSP banks were inadvertently left out?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-sc8280xp-fastrpc-dma-coherent-v2-1-a112abfd41b4@radxa.com?part=1