Re: [PATCH v2 3/3] arm64: dts: qcom: monaco: enable audio ML offload memory and SMMU mappings

[sashiko-bot@kernel.org]

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

New issues:
- [High] The ML-related SMMU stream IDs are added to a different device's IOMMU domain (remoteproc_adsp) than the device mapping the CMA buffer (q6apmdai), leading to Stage 1 Translation Faults.
- [Medium] The commit message incorrectly documents the Stream IDs being added as 0x3060 and 0x3062, while the code adds 0x2060 and 0x2062.
- [Low] The newly added reserved-memory node uses a vendor prefix and comma (`qcom,audio-ml`), which violates standard DT node naming guidelines.

Pre-existing issues:
- [High] The `q6apmdai` driver does not initialize the DT `memory-region` property, rendering the devicetree addition inert and causing allocations to fall back to the default allocator.
--

--- Patch [3]: [PATCH v2 3/3] arm64: dts: qcom: monaco: enable audio ML offload memory and SMMU mappings ---
commit 4a8f1567e9c6cd6a5d86dc17c6731564a853e2ff
Author: Pratyush Meduri <pratyush.meduri@oss.qualcomm.com>

arm64: dts: qcom: monaco: enable audio ML offload memory and SMMU mappings

The audio ML (machine-learning) offload use case requires a contiguous,
physically addressable buffer shared with the audio DSP/SPF, and issues
DMA transactions through additional SMMU stream IDs that are not
covered by the existing ADSP mapping.

[ ... ]

> Add the ML-related stream IDs (0x3060 mask 0x9, 0x3062 mask 0x1) to the
> remoteproc_adsp iommus property in the EL2 overlay so these buffers are
> translated by the SMMU instead of being rejected.

[Severity: Medium]
The commit message documents adding stream IDs 0x3060 and 0x3062, but the code
adds 0x2060 and 0x2062. Is this a typo in the commit message?

> diff --git a/arch/arm64/boot/dts/qcom/monaco-el2.dtso b/arch/arm64/boot/dts/qcom/monaco-el2.dtso
> index a7e3270f86090..97bb8287cec33 100644
> --- a/arch/arm64/boot/dts/qcom/monaco-el2.dtso
> +++ b/arch/arm64/boot/dts/qcom/monaco-el2.dtso
> @@ -17,7 +17,9 @@ &iris {
>  };
>  
>  &remoteproc_adsp {
> -	iommus = <&apps_smmu 0x2000 0x0>;
> +	iommus = <&apps_smmu 0x2000 0x0>,
> +		<&apps_smmu 0x2060 0x9>,
> +		<&apps_smmu 0x2062 0x1>;
>  };

[Severity: High]
Does adding the ML-related stream IDs to the remoteproc_adsp node result in
Stage 1 Translation Faults?

The memory-region is assigned to the q6apmdai device (which uses stream ID
0x2001) in monaco.dtsi. When q6apmdai allocates and DMA-maps the memory, the
mapping is created in the IOMMU domain associated with 0x2001. 

By adding 0x2060 and 0x2062 to remoteproc_adsp (which uses 0x2000), the Linux
arm-smmu driver places these stream IDs into a different IOMMU domain than the
one holding the ML memory mapping. Should these stream IDs be added to the
q6apmdai node instead so they share the same IOMMU domain?

> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
> index 64fc0d592282c..d971c0ea1b302 100644
> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
> @@ -729,6 +729,14 @@ reserved-memory {
>  		#size-cells = <2>;
>  		ranges;
>  
> +		audio_cma_mem: qcom,audio-ml {

[Severity: Low]
Does this node name violate Devicetree naming conventions? Node names should
typically be generic and avoid vendor prefixes and commas, which are reserved
for compatible strings.

> +			compatible = "shared-dma-pool";
> +			alloc-ranges = <0x0 0x00000000 0x0 0xffffffff>;
> +			reusable;
> +			alignment = <0x0 0x400000>;
> +			size = <0x0 0x1000000>;
> +		};
> +
[ ... ]
> @@ -2844,6 +2852,7 @@ q6apmbedai: bedais {
>  						q6apmdai: dais {
>  							compatible = "qcom,q6apm-dais";
>  							iommus = <&apps_smmu 0x2001 0x0>;
> +							memory-region = <&audio_cma_mem>;
>  						};

[Severity: High]
This isn't a bug introduced by this patch, but will this memory-region
assignment be ignored by the q6apmdai driver?

Looking at q6apm_dai_probe() in sound/soc/qcom/qdsp6/q6apm-dai.c:

	rc = of_parse_phandle_with_fixed_args(node, "iommus", 1, 0, &args);
	if (rc < 0)
		pdata->sid = -1;
	else
		pdata->sid = args.args[0] & SID_MASK_DEFAULT;

	dev_set_drvdata(dev, pdata);

	return devm_snd_soc_register_component(dev, &q6apm_fe_dai_component, NULL, 0);

The driver probe function does not call of_reserved_mem_device_init(). For a
shared-dma-pool, the driver must explicitly initialize it. Without this call,
the driver dev->cma_area remains unconfigured and memory allocations will
silently fall back to the default system CMA pool, defeating the purpose of
adding this dedicated 16MiB region.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717095825.3962666-1-pratyush.meduri@oss.qualcomm.com?part=3