-----Original Message-----
From: Sudeep Holla <redacted>
Sent: Tuesday, August 21, 2018 3:47 PM
To: Vabhav Sharma <redacted>
Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
robh+dt@kernel.org; mark.rutland@arm.com; linuxppc-dev@lists.ozlabs.org;
linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
sboyd@kernel.org; rjw@rjwysocki.net; viresh.kumar@linaro.org; linux-
clk@vger.kernel.org; linux-pm@vger.kernel.org; linux-kernel-
owner@vger.kernel.org; catalin.marinas@arm.com; will.deacon@arm.com;
gregkh@linuxfoundation.org; arnd@arndb.de;
kstewart@linuxfoundation.org; yamada.masahiro@socionext.com;
linux@armlinux.org.uk; Varun Sethi [off-list ref]; Udit Kumar
[off-list ref]; Ramneek Mehresh [off-list ref];
Ying Zhang [off-list ref]; Nipun Gupta
[off-list ref]; Priyanka Jain [off-list ref]; Yogesh
Narayan Gaur [off-list ref]; Sriram Dash
[off-list ref]; Sudeep Holla [off-list ref]
Subject: Re: [PATCH 4/5] arm64: dts: add QorIQ LX2160A SoC support

On Mon, Aug 20, 2018 at 12:17:15PM +0530, Vabhav Sharma wrote:

quoted

LX2160A SoC is based on Layerscape Chassis Generation 3.2 Architecture.

LX2160A features an advanced 16 64-bit ARM v8 CortexA72 processor
cores in 8 cluster, CCN508, GICv3,two 64-bit DDR4 memory controller, 8
I2C controllers, 3 dspi, 2 esdhc,2 USB 3.0, mmu 500, 3 SATA, 4 PL011
SBSA UARTs etc.

Signed-off-by: Ramneek Mehresh <redacted>
Signed-off-by: Zhang Ying-22455 <redacted>
Signed-off-by: Nipun Gupta <redacted>
Signed-off-by: Priyanka Jain <redacted>
Signed-off-by: Yogesh Gaur <redacted>
Signed-off-by: Sriram Dash <redacted>
Signed-off-by: Vabhav Sharma <redacted>
---
 arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 572
+++++++++++++++++++++++++
 1 file changed, 572 insertions(+)
 create mode 100644 arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi

b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
new file mode 100644
index 0000000..e35e494

--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi

@@ -0,0 +1,572 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT) // // Device Tree
+Include file for Layerscape-LX2160A family SoC.
+//
+// Copyright 2018 NXP
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/memreserve/ 0x80000000 0x00010000;
+
+/ {
+	compatible = "fsl,lx2160a";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		// 8 clusters having 2 Cortex-A72 cores each
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0x0>;
+			clocks = <&clockgen 1 0>;
+			next-level-cache = <&cluster0_l2>;

If you expect to get cache properties in sysfs entries, you need to populate
them here and for each L2 cache.

Rather sysfs, If Entry is not present then print  "cacheinfo: Unable to detect cache hierarchy for CPU 0" appears in boot log which is bad saying something is not present.
Either this print is require change to debug instead of warning.

[...]

quoted

+
+	rstcr: syscon@1e60000 {
+		compatible = "syscon";
+		reg = <0x0 0x1e60000 0x0 0x4>;
+	};
+
+	reboot {
+		compatible ="syscon-reboot";
+		regmap = <&rstcr>;
+		offset = <0x0>;
+		mask = <0x2>;

Is this disabled in bootloader ? With PSCI, it's preferred to use
SYSTEM_RESET/OFF. EL3 f/w may need to do some housekeeping on
poweroff.

No, PSCIv0.2 is used and control passes to EL3 fw via smc call, psci node is present in the file.
This node is not required and keeping it in case PSCI is not used.

quoted

+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <1 13 4>, // Physical Secure PPI, active-low

The comment says active low but the value 4 indicates it's HIGH from
"include/dt-bindings/interrupt-controller/irq.h"

Thanks, I will change the entries to existing definition IRQ_TYPE_LEVEL_LOW,GIC_PPI which is self-explanatory and not require comments

quoted

+			     <1 14 4>, // Physical Non-Secure PPI, active-low
+			     <1 11 4>, // Virtual PPI, active-low
+			     <1 10 4>; // Hypervisor PPI, active-low
+	};
+
+	pmu {
+		compatible = "arm,armv8-pmuv3";

More specific compatible preferably "arm,cortex-a72-pmu" ?

Sure.

--
Regards,
Sudeep