Re: [PATCH v1 2/2] 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 3 potential issue(s) to consider:
- [High] The newly added `memory-region` for `audio_cma_mem` will be silently ignored because the `qcom,q6apm-dais` driver does not invoke `of_reserved_mem_device_init()`.
- [Medium] The commit message incorrectly references stream IDs `0x3060` and `0x3062`, while the code and the included error logs implement `0x2060` and `0x2062`.
- [Low] The reserved-memory child node name `qcom,audio-ml` incorrectly uses a vendor prefix and comma, violating standard node naming conventions and schema bindings.
--

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

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

This patch adds a dedicated 16 MiB shared-dma-pool CMA region for audio ML
offload and connects it to the q6apm DAIs node. It also updates the EL2
overlay to include the ML-related stream IDs so the SMMU translates rather
than blocks these transactions.

> 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]
This isn't a bug in the code, but there appears to be a typo in the commit
message. The text references stream IDs 0x3060 and 0x3062, while the code
and the included dmesg logs correctly use 0x2060 and 0x2062. Should the
commit message be updated to match the implementation?

> 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

[ ... ]

> 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 standard device tree naming conventions? Using a
vendor prefix and a comma in the node name (qcom,audio-ml) typically causes
dt_binding_check validation failures. 

Standard reserved-memory schemas strictly expect child node names to match
generic patterns like memory, pool, shared-dma-pool, or cma. Should this be
named something generic like audio-ml-cma?

> +			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]
Will this memory-region actually be utilized by the driver?

Looking at the driver probe routine in sound/soc/qcom/qdsp6/q6apm-dai.c,
q6apm_dai_probe() does not appear to call of_reserved_mem_device_init().

Without that initialization, the driver will silently fall back to allocating
from the default CMA pool instead of using this dedicated 16 MiB region.

This seems like it would waste the reserved 16 MiB of RAM and defeat the
patch's goal of isolating allocations to guarantee contiguity, potentially
breaking the ML offload feature when the default CMA pool fragments.

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