Re: [PATCH 8/9] arm64: dts: qcom: Add DTS for MSM8976 and MSM8956 SoCs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 08/11/22 05:55, Bjorn Andersson ha scritto:
On Fri, Nov 04, 2022 at 06:21:21PM +0100, AngeloGioacchino Del Regno wrote:
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>

This commit adds device trees for MSM8956 and MSM8976 SoCs.
They are *almost* identical, with minor differences, such as
MSM8956 having two A72 cores less.

However, there is a bug in Sony Loire bootloader that requires presence
of all 8 cores in the cpu{} node, so these will not be deleted.

Co-developed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
Co-developed-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>

Hello,

Thanks to everyone for the feedback!
I'll send a new version this Friday, according to the received reviews.

In the meanwhile, a few answers will follow, check below.

---
  arch/arm64/boot/dts/qcom/msm8956.dtsi |   18 +
  arch/arm64/boot/dts/qcom/msm8976.dtsi | 1208 +++++++++++++++++++++++++
  2 files changed, 1226 insertions(+)
  create mode 100644 arch/arm64/boot/dts/qcom/msm8956.dtsi
  create mode 100644 arch/arm64/boot/dts/qcom/msm8976.dtsi


..snip..

+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&CPU0>;
+				};
+
+				core1 {
+					cpu = <&CPU1>;
+				};
+
+				core2 {
+					cpu = <&CPU2>;
+				};
+
+				core3 {
+					cpu = <&CPU3>;
+				};
+			};
+
+			cluster1 {

Are you sure that the two clusters should be expressed separately in the
cpu-map?


This SoC has two clusters with split L2 cache, can shutdown one cluster CPU, or
the L2 cache for one cluster, or one entire cluster, hence can also manage idle
states on a per-cluster basis.

Also, as per bindings/cpu/cpu-topology.txt - I am here describing the hierarchy
of CPUs in MSM8976, containing one "little" cluster, composed of four slower CPU
cores and its own L2 cache slice, and one "big" cluster, composed of four (8976)
or two (8956) faster CPU cores and its own L2 cache slice.

That said, I am sure that the two clusters shall be expressed separately.

Am I underestimating and/or ignoring anything?

+				core0 {
+					cpu = <&CPU4>;
+				};
+

..snip..

+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		cont_splash_mem: memory@83000000 {

memory is "reserved", please use specific node names for these regions.


Agreed.

+			reg = <0x0 0x83000000 0x0 0x2800000>;
+		};
[..]
+		apcs: syscon@b011000 {
+			compatible = "syscon";

Why not use qcom,msm8976-apcs-kpss-global here?


There's no reason not to use the suggested compatible. I'm sorry for the miss.

+			reg = <0x0b011000 0x1000>;
+		};
[..]
+
+		imem: imem@8600000 {
+			compatible = "simple-mfd";

sram/qcom,imem.yaml please.


Will do on v2.

Regards,
Angelo



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux