Re: [PATCH v11 5/5] arm64: dts: qcom: Add SM7250 devicetree file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: incorrect cpu-map topology for DynamIQ
- [Low] dts: unused qcom-rpmpd.h header inclusion
- [Low] dts: empty ranges property with mismatched cell counts triggers dtc warning
- [Low] dts: cpu-map core nodes incorrectly numbered across clusters
--

commit 44af3970816d0373bee16d99b2e8e2afd4954505
Author: Sreeshankar K <sreeshankar0910@gmail.com>

arm64: dts: qcom: Add SM7250 devicetree file

This commit adds the base devicetree file for the Qualcomm SM7250 SoC.
It includes basic nodes like cpu, psci, and other required configuration
for booting up to the serial console.

> diff --git a/arch/arm64/boot/dts/qcom/sm7250.dtsi b/arch/arm64/boot/dts/qcom/sm7250.dtsi
> new file mode 100644
> index 0000000000000..48480e6855f0c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm7250.dtsi
> @@ -0,0 +1,771 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2026, Sreeshankar K <sreeshankar0910@gmail.com>
> + */
> +
> +#include <dt-bindings/clock/qcom,rpmh.h>
> +#include <dt-bindings/clock/qcom,sm7250-gcc.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/power/qcom,rpmhpd.h>
> +#include <dt-bindings/power/qcom-rpmpd.h>

[Severity: Low]
Is it necessary to include qcom-rpmpd.h here? It appears to be unused.

The SM7250 SoC relies on the newer RPMh framework, so only the definitions from
<dt-bindings/power/qcom,rpmhpd.h> are used (e.g., RPMHPD_CX). Does this code
rely on any definitions from qcom-rpmpd.h, or can it be removed as dead code?

[ ... ]

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

[Severity: Medium]
Does splitting the cores across cluster0, cluster1, and cluster2 accurately
reflect the SM7250 topology?

The SM7250 uses a single DynamIQ cluster where all 8 cores share the L3 cache.
By placing the cores into three separate clusters in the cpu-map, the Linux
scheduler is told that migrating tasks between them crosses a socket/package
boundary, which artificially penalizes cross-core migration and prevents
optimal load-balancing.

Should all 8 cores be placed under cluster0 to correctly represent the single
shared DynamIQ cluster, similar to other SoCs like SC7180 or SM8250?


[Severity: Low]
Additionally, for the secondary clusters defined here, should the core nodes
restart their index at core0?

Devicetree schema (cpus.yaml) requires that core nodes within a cluster be
named coreN where N is the local index within that cluster. Continuing the
global index (core6 and core7) instead of starting at core0 will trigger
schema validation errors when running make dtbs_check.

[ ... ]

> +		timer@17c20000 {
> +			compatible = "arm,armv7-timer-mem";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;

[Severity: Low]
Will this empty ranges property trigger a DeviceTree compiler warning?

The parent soc@0 node uses #address-cells = <2> and #size-cells = <2>, while
timer@17c20000 overrides them to <1> and <1>. The DeviceTree compiler (dtc)
enforces that an empty ranges property can only be used when the child node's
cell counts strictly match the parent's.

While Linux's __of_translate_address handles this gracefully at runtime by
zero-extending, this mismatch violates the Devicetree specification and
triggers a ranges_format warning during make dtbs. Could an explicit ranges
translation be provided here (e.g., ranges = <0x0 0x0 0x0 0xffffffff>;) to
fix the warning?

> +			reg = <0x0 0x17c20000 0x0 0x1000>;
> +
> +			frame@17c21000 {

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