Re: [PATCH v2 6/8] arm64: dts: qcom: Add device tree for Nord SA8797P SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] SCMI providers are left disabled, leading to probe failures for dependent devices.
- [Medium] Missing GPU SMMU node claimed in the commit message.
- [Medium] Inconsistent and missing SCMI protocol sub-nodes compared to the commit message.
- [Low] Unresolved phandle dependency in shared `scmi-common.dtsi` breaks compilation for SoCs with fewer channels.
--

commit 360295b4c98c35977b904736f9ce01833171cfbb
Author: Deepti Jaggi <deepti.jaggi@oss.qualcomm.com>

arm64: dts: qcom: Add device tree for Nord SA8797P SoC

This commit adds the device tree include for the SA8797P automotive SoC.
It covers SCMI shared memory regions, GENI SE QUP blocks, and UFS
controllers, while introducing scmi-common.dtsi for shared SCMI
channel nodes.

>     - GPU SMMU node with SCMI power domain via scmi15

Is this description accurate? There doesn't appear to be a GPU SMMU
node or any usage of the <&scmi15_pd> power domain in nord-sa8797p.dtsi.

>    Also introduce scmi-common.dtsi providing the firmware-level SCMI
>    channel nodes (scmi0, scmi3, scmi11, scmi15, scmi23) shared across
>    SCMI based SoCs, each carrying clock, power domain, performance,
>    and sensor protocol sub-nodes.

Are these sub-nodes correctly described?

Looking at the implementation in scmi-common.dtsi, the clock protocol
sub-node (protocol@14) is missing from all instances, and the sensor
protocol sub-node (protocol@15) is only present in scmi23.

Additionally, a reset protocol sub-node (protocol@16) is included in most
nodes but isn't mentioned here.

> diff --git a/arch/arm64/boot/dts/qcom/nord-sa8797p.dtsi b/arch/arm64/boot/dts/qcom/nord-sa8797p.dtsi
> index 0000000000000..343de75129282
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/nord-sa8797p.dtsi

[ ... ]

> +&ufs_mem_hc {
> +	compatible = "qcom,sa8797p-ufshc",
> +		     "qcom,sa8255p-ufshc";
> +	reg = <0x0 0x01d44000 0x0 0x3000>;
> +	power-domains = <&scmi3_pd 0>;
> +};

Since scmi3 is defined with status = "disabled" in scmi-common.dtsi and
not overridden to "okay" here, will this prevent the SCMI provider from
probing?

If the SCMI providers remain disabled, won't this cause the power domain
core to perpetually defer probing for dependent devices like ufs_mem_hc,
uart0, and i2c0?

> diff --git a/arch/arm64/boot/dts/qcom/scmi-common.dtsi b/arch/arm64/boot/dts/qcom/scmi-common.dtsi
> index 0000000000000..0c7ffe9e415c4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/scmi-common.dtsi
> @@ -0,0 +1,1918 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +&firmware {
> +	scmi0: scmi-0 {
> +		compatible = "qcom,scmi-smc";
> +		arm,smc-id = <0xc6008012>;
> +		shmem = <&shmem0>;

Will this static reference to <&shmem0> through <&shmem63> break compilation
for other SoCs that include scmi-common.dtsi but have fewer than 64 channels?

Because the Device Tree Compiler validates phandles globally at compile time,
it seems any SoC including this file would be forced to define all 64 shmem
labels even if they are unused.

Should the shmem property assignments be moved to the SoC-specific .dtsi files
instead?

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