Re: [PATCH v2 1/5] arm64: dts: qcom: Add SM7250 base dts file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The `cpu-map` cluster definitions use non-zero starting indices (`core6`, `core7`), breaking Linux topology parsing.
- [High] Mismatched QUPv3 wrapper addresses and clocks.
- [Low] Missing devicetree bindings header for the clock controller.
- [Low] Invalid GIC_CPU_MASK_SIMPLE usage in GICv3 interrupt specifier.
--

--- Patch [1]: [PATCH v2 1/5] arm64: dts: qcom: Add SM7250 base dts file ---
commit f9be33ec0cc19ce8ffedcebd3293f880830f4c83
Author: Sreeshankar K <sreeshankar0910@gmail.com>

arm64: dts: qcom: Add SM7250 base dts file

Add devicetree file for SM7250 SoC and SM7250 MTP platform.
This file adds the basic nodes like cpu, psci and other required
configuration for booting up to the serial console.

> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm7250.dtsi
> @@ -0,0 +1,499 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2026, Sreeshankar K <sreeshankar0910@gmail.com>
> + */
> +
> +#include <dt-bindings/clock/qcom,rpmh.h>
> +#include <dt-bindings/clock/qcom,gcc-sm7250.h>

[Severity: Low]
Does this file exist in the source tree? A missing devicetree bindings
header will cause a preprocessor error during compilation and break the
build.

[ ... ]

> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cpu0>;
> +				};
[ ... ]
> +			};
> +
> +			cluster1 {
> +				core6 {
> +					cpu = <&cpu6>;
> +				};
> +			};
> +
> +			cluster2 {
> +				core7 {
> +					cpu = <&cpu7>;
> +				};
> +			};
> +		};

[Severity: High]
Will this topology be parsed correctly by the kernel? 

Looking at parse_cluster() in drivers/base/arch_topology.c, the kernel
iterates over cluster cores looking for child nodes named core%d
starting strictly at index 0. Because cluster1 and cluster2 start at
core6 and core7, it seems the parser will fail to find core0, break
out of the loop instantly, and completely ignore the remaining cores
which will degrade scheduling behavior and performance.

[ ... ]

> +		qupv3_id_1: geniqup@9c0000 {
> +			compatible = "qcom,geni-se-qup";
> +			reg = <0x0 0x009c0000 0x0 0x6000>;
> +			clock-names = "m-ahb", "s-ahb";
> +			clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
> +				 <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;

[Severity: High]
Are these clocks correct for this QUP base address? 

The base address 0x9c0000 physically corresponds to QUP0, but the clocks
requested here belong to QUP1 (GCC_QUPV3_WRAP_1_M_AHB_CLK). During
driver initialization, won't the driver enable QUP1 clocks and then
attempt to read or write registers belonging to QUP0, triggering a
synchronous external abort and kernel panic due to accessing an
unclocked AMBA peripheral?

[ ... ]

> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 1
> +				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 2
> +				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,

[Severity: Low]
Does this comply with the GICv3 devicetree bindings?

The schema for arm,gic-v3.yaml requires the third cell of a GICv3
interrupt specifier to only contain trigger type and level flags in
bits [3:0]. GIC_CPU_MASK_SIMPLE is a GICv2 legacy macro that sets
bits [15:8], which causes dtbs_check validation failures.

> +			     <GIC_PPI 3
> +				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 0
> +				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260816161808.12419-1-sreeshankar0910@gmail.com?part=1