Re: [PATCH v4 2/5] arm64: dts: qcom: Introduce Shikra SoC base dtsi

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect grouping of heterogeneous cores into separate clusters in `cpu-map`.
- [High] Missing `clock-names` property in the Global Clock Controller (GCC) node.
--

commit a88352d8dd5b4d1d3fc313dace6282e258e6b324
Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>

arm64: dts: qcom: Introduce Shikra SoC base dtsi

This commit adds initial device tree support for the Qualcomm Shikra SoC.
It enables support for CPU nodes, global and RPM-based clock/power
controllers, interrupts, TLMM, UART, eMMC, and system timers.

> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
> new file mode 100644
> index 0000000000000..a4334d99c1f35
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi

[ ... ]

> +		cpu-map {
> +			cluster0 {
> +				core0 {
> +					cpu = <&cpu0>;
> +				};
> +
> +				core1 {
> +					cpu = <&cpu1>;
> +				};
> +
> +				core2 {
> +					cpu = <&cpu2>;
> +				};
> +			};
> +
> +			cluster1 {
> +				core0 {
> +					cpu = <&cpu3>;
> +				};
> +			};
> +		};

[Severity: High]
Does this cpu-map configuration accurately reflect the hardware topology?

The Cortex-A55 cores (cpu0-cpu2) and the Cortex-A78C core (cpu3) physically
share the same Last Level Cache (&l3), and their reg values (0x0 to 0x300)
indicate they belong to the same physical cluster.

Splitting them into cluster0 and cluster1 might cause the scheduler to wrongly
assume the cores do not share an LLC, which can impact cache locality load
balancing. Should these cores be grouped under a single cluster node?

[ ... ]

> +		gcc: clock-controller@1400000 {
> +			compatible = "qcom,shikra-gcc";
> +			reg = <0x0 0x01400000 0x0 0x1f0000>;
> +			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +				 <&sleep_clk>,
> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <0>;
> +			power-domains = <&rpmpd RPMPD_VDDCX>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};

[Severity: High]
Is this node missing a clock-names property?

The gcc node defines an array of 8 clocks, but omits the corresponding
clock-names property. 

Since Qualcomm GCC clock drivers map input parent clocks using string
matching, could the driver fail to resolve its parent clocks without this
array, potentially causing the GCC probe to fail?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260527-shikra-dt-v4-0-b5ca1fa0b392@oss.qualcomm.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy