Re: [PATCH RFC 20/23] arm64: dts: qcom: sm8650: move EL1 Gunyah nodes to sm8650-el1.dtsi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] arm64: dts: qcom: sm8650: lpass_vamacro left enabled with
  zero clocks
- [Low] arm64: dts: qcom: sm8650: leftover video_mem reference on iris
  in sm8650.dtsi
--

commit 6cf38f344aa0427c5afd87fe6828f76c27873b93
Author: Neil Armstrong <neil.armstrong@linaro.org>

arm64: dts: qcom: sm8650: move EL1 Gunyah nodes to sm8650-el1.dtsi

This commit moves the Gunyah EL1-specific reserved memory, remoteproc,
SMP2P, IPA, GPU zap shader, and q6prm LPASS clock nodes out of sm8650.dtsi
into sm8650-el1.dtsi, and removes the corresponding node deletions from
sm8650-valve-deckard.dts.

> diff --git a/arch/arm64/boot/dts/qcom/sm8650-el1.dtsi b/arch/arm64/boot/dts/qcom/sm8650-el1.dtsi
> new file mode 100644
> index 0000000000000..18b7c18ae3120
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8650-el1.dtsi

[ ... ]

> +&iris {
> +	memory-region = <&video_mem>;
> +};

[Severity: Low]
Was memory-region = <&video_mem>; meant to be removed from the iris node in
sm8650.dtsi?

Now that video_mem lives in sm8650-el1.dtsi, leaving memory-region =
<&video_mem>; in sm8650.dtsi duplicates the property on EL1 boards and
causes an unresolved phandle build error on any EL2 board that does not
define its own video_mem label.

> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 86fbb0fd6472d..d88acdb8c7759 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi

[ ... ]

> @@ -4854,12 +4302,9 @@ swr2: soundwire@6d30000 {
>  		lpass_vamacro: codec@6d44000 {
>  			compatible = "qcom,sm8650-lpass-va-macro", "qcom,sm8550-lpass-va-macro";
>  			reg = <0 0x06d44000 0 0x1000>;
> -			clocks = <&q6prmcc LPASS_CLK_ID_TX_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
> -				 <&q6prmcc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
> -				 <&q6prmcc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
> -			clock-names = "mclk",
> -				      "macro",
> -				      "dcodec";
> +
> +			clocks = <0>, <0>, <0>;
> +			clock-names = "mclk", "macro", "dcodec";
>  
>  			#clock-cells = <0>;
>  			clock-output-names = "fsgen";

[Severity: Medium]
Should lpass_vamacro also have status = "disabled" here, like the other
five LPASS macro and pinctrl nodes?

Because /delete-node/ &lpass_vamacro; is dropped from
sm8650-valve-deckard.dts and sm8650-el1.dtsi explicitly sets status =
"okay" on &lpass_vamacro, leaving it enabled in sm8650.dtsi with
placeholder clocks = <0>, <0>, <0> causes va_macro_probe() ->
devm_clk_get(dev, "mclk") to fail with -ENOENT at boot on EL2 boards that
do not override the clocks, and breaks dtbs_check schema validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-topic-sm8650-el2-v1-0-5183f6aab9fc@linaro.org?part=20