diff mbox

[v3,2/4] arm64: dts: Add Qualcomm MSM8916 SoC and evaluation board dts

Message ID 1426107080-29079-2-git-send-email-galak@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar Gala March 11, 2015, 8:51 p.m. UTC
Add initial device tree support for Qualcomm MSM8916 SoC and MTP8916
evaluation board.  At the current time we only boot up a single processor.

Signed-off-by: Kumar Gala <galak@codeaurora.org>
---
v3:
* Removed qcom,msm-id and qcom,board-id
* Added top level compat for "qcom,msm8916-mtp-smb1360"

v2:
* Updated to dropping CONFIG_ARCH_QCOM_MSM8916
* Updated to use qcom-ids.h
 arch/arm64/boot/dts/Makefile              |   1 +
 arch/arm64/boot/dts/qcom/Makefile         |   5 +
 arch/arm64/boot/dts/qcom/msm8916-mtp.dts  |  21 ++++
 arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi |  24 ++++
 arch/arm64/boot/dts/qcom/msm8916.dtsi     | 184 ++++++++++++++++++++++++++++++
 5 files changed, 235 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/Makefile
 create mode 100644 arch/arm64/boot/dts/qcom/msm8916-mtp.dts
 create mode 100644 arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/msm8916.dtsi

Comments

Mark Rutland March 12, 2015, 5:05 p.m. UTC | #1
Hi Kumar,

> +/ {
> +	model = "Qualcomm Technologies, Inc. MSM 8916 MTP";
> +	compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360",
> +			"qcom,msm8916", "qcom,mtp";
> +};

No /chosen/stdout-path?

Does your UART driver support earlycon?

[...]

> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		CPU0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x0>;
> +		};
> +
> +		CPU1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x1>;
> +		};
> +
> +		CPU2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x2>;
> +		};
> +
> +		CPU3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53", "arm,armv8";
> +			reg = <0x3>;
> +		};
> +	};

The secondary CPUs need an enable-method. Are you using PSCI or
spin-table? 

Which exception level do the CPUs enter the kernel?

> +	timer {
> +		compatible = "arm,armv7-timer";

This should be "arm,armv8-timer".

> +		interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 4 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +		clock-frequency = <19200000>;
> +	};

NAK. CNTFRQ should be programmed on all CPUs prior to entering the
kernel, per the boot protocol. You should not need clock-frequency here.

[...]

> +		intc: interrupt-controller@b000000 {
> +			compatible = "qcom,msm-qgic2";

This string isn't documented (but seems to be supported by the GIC
driver).

How does this differ from other GIC implementations?

> +			interrupt-controller;
> +			#interrupt-cells = <3>;
> +			reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>;
> +		};

No GICH, GICV, maintenance interrupt?

Minor nit, but I'd prefer if the reg entries were on individual lines as
happens in other dts.

Thanks,
Mark.
Kumar Gala March 12, 2015, 5:33 p.m. UTC | #2
On Mar 12, 2015, at 12:05 PM, Mark Rutland <mark.rutland@arm.com> wrote:

> Hi Kumar,
> 
>> +/ {
>> +	model = "Qualcomm Technologies, Inc. MSM 8916 MTP";
>> +	compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360",
>> +			"qcom,msm8916", "qcom,mtp";
>> +};
> 
> No /chosen/stdout-path?

Nope ;).

> 
> Does your UART driver support earlycon?

It does.

> 
> [...]
> 
>> +	cpus {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		CPU0: cpu@0 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a53", "arm,armv8";
>> +			reg = <0x0>;
>> +		};
>> +
>> +		CPU1: cpu@1 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a53", "arm,armv8";
>> +			reg = <0x1>;
>> +		};
>> +
>> +		CPU2: cpu@2 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a53", "arm,armv8";
>> +			reg = <0x2>;
>> +		};
>> +
>> +		CPU3: cpu@3 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a53", "arm,armv8";
>> +			reg = <0x3>;
>> +		};
>> +	};
> 
> The secondary CPUs need an enable-method. Are you using PSCI or
> spin-table?

This is on purpose.  We aren’t using either PSCI or spin-table.  Right now the dts is for booting on a single core.  I can drop CPU1..CPU3 if that helps.

> Which exception level do the CPUs enter the kernel?
> 
>> +	timer {
>> +		compatible = "arm,armv7-timer";
> 
> This should be "arm,armv8-timer”.

will change

> 
>> +		interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +			     <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +			     <GIC_PPI 4 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> +			     <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> +		clock-frequency = <19200000>;
>> +	};
> 
> NAK. CNTFRQ should be programmed on all CPUs prior to entering the
> kernel, per the boot protocol. You should not need clock-frequency here.

Will drop clock-frequency.

> [...]
> 
>> +		intc: interrupt-controller@b000000 {
>> +			compatible = "qcom,msm-qgic2";
> 
> This string isn't documented (but seems to be supported by the GIC
> driver).

There’s a patch posted to add ‘qcom,msm-qgic2’ to the binding doc.

> How does this differ from other GIC implementations?

Not sure the exact details, just that its qcom’s on implementation of the GIC spec.

> 
>> +			interrupt-controller;
>> +			#interrupt-cells = <3>;
>> +			reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>;
>> +		};
> 
> No GICH, GICV, maintenance interrupt?

Nope.

> 
> Minor nit, but I'd prefer if the reg entries were on individual lines as
> happens in other dts.
> 
> Thanks,
> Mark.
Mark Rutland March 12, 2015, 6:25 p.m. UTC | #3
> >> +	cpus {
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +
> >> +		CPU0: cpu@0 {
> >> +			device_type = "cpu";
> >> +			compatible = "arm,cortex-a53", "arm,armv8";
> >> +			reg = <0x0>;
> >> +		};
> >> +
> >> +		CPU1: cpu@1 {
> >> +			device_type = "cpu";
> >> +			compatible = "arm,cortex-a53", "arm,armv8";
> >> +			reg = <0x1>;
> >> +		};
> >> +
> >> +		CPU2: cpu@2 {
> >> +			device_type = "cpu";
> >> +			compatible = "arm,cortex-a53", "arm,armv8";
> >> +			reg = <0x2>;
> >> +		};
> >> +
> >> +		CPU3: cpu@3 {
> >> +			device_type = "cpu";
> >> +			compatible = "arm,cortex-a53", "arm,armv8";
> >> +			reg = <0x3>;
> >> +		};
> >> +	};
> > 
> > The secondary CPUs need an enable-method. Are you using PSCI or
> > spin-table?
> 
> This is on purpose.  We aren’t using either PSCI or spin-table.  Right
> now the dts is for booting on a single core.  I can drop CPU1..CPU3 if
> that helps.

We won't poke the CPUs without an enable-method, so personally I'm not
too worried either way about having the CPUs listed.

Which of spin-table/psci are you planning on using for SMP support, and
when would that be likely to appear?

Which exception level do CPUs enter the kernel? Even without a
virt-capable GIC booting at EL2 is less work for the FW and gives the
kernel a better chance of fixing things up (e.g. CNTVOFF).

Thanks,
Mark.
Kumar Gala March 12, 2015, 7:54 p.m. UTC | #4
On Mar 12, 2015, at 1:25 PM, Mark Rutland <mark.rutland@arm.com> wrote:

>>>> +	cpus {
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <0>;
>>>> +
>>>> +		CPU0: cpu@0 {
>>>> +			device_type = "cpu";
>>>> +			compatible = "arm,cortex-a53", "arm,armv8";
>>>> +			reg = <0x0>;
>>>> +		};
>>>> +
>>>> +		CPU1: cpu@1 {
>>>> +			device_type = "cpu";
>>>> +			compatible = "arm,cortex-a53", "arm,armv8";
>>>> +			reg = <0x1>;
>>>> +		};
>>>> +
>>>> +		CPU2: cpu@2 {
>>>> +			device_type = "cpu";
>>>> +			compatible = "arm,cortex-a53", "arm,armv8";
>>>> +			reg = <0x2>;
>>>> +		};
>>>> +
>>>> +		CPU3: cpu@3 {
>>>> +			device_type = "cpu";
>>>> +			compatible = "arm,cortex-a53", "arm,armv8";
>>>> +			reg = <0x3>;
>>>> +		};
>>>> +	};
>>> 
>>> The secondary CPUs need an enable-method. Are you using PSCI or
>>> spin-table?
>> 
>> This is on purpose.  We aren’t using either PSCI or spin-table.  Right
>> now the dts is for booting on a single core.  I can drop CPU1..CPU3 if
>> that helps.
> 
> We won't poke the CPUs without an enable-method, so personally I'm not
> too worried either way about having the CPUs listed.

That was my thinking, so left them in.

> Which of spin-table/psci are you planning on using for SMP support, and
> when would that be likely to appear?

We have a qcom specific SMP enablement method for this device.  This was one of our first devices so it utilized as much from arm 32-bit as possible.

> Which exception level do CPUs enter the kernel? Even without a
> virt-capable GIC booting at EL2 is less work for the FW and gives the
> kernel a better chance of fixing things up (e.g. CNTVOFF).

I think the enter in EL1.

- k
Mark Rutland March 13, 2015, 10:34 a.m. UTC | #5
> > Which of spin-table/psci are you planning on using for SMP support, and
> > when would that be likely to appear?
> 
> We have a qcom specific SMP enablement method for this device.  This
> was one of our first devices so it utilized as much from arm 32-bit as
> possible.

Implementation specific enable methods are something we really don't
want to see for arm64. If PSCI is out of the question then a spin-table
shim in your bootloader shouldn't be too hard to implement.

> > Which exception level do CPUs enter the kernel? Even without a
> > virt-capable GIC booting at EL2 is less work for the FW and gives the
> > kernel a better chance of fixing things up (e.g. CNTVOFF).
> 
> I think the enter in EL1.

That's unfortunate, but so long as they are consistent, it's not the end
of the world.

Mark.
Catalin Marinas March 13, 2015, 12:07 p.m. UTC | #6
On Fri, Mar 13, 2015 at 10:34:54AM +0000, Mark Rutland wrote:
> > > Which of spin-table/psci are you planning on using for SMP support, and
> > > when would that be likely to appear?
> > 
> > We have a qcom specific SMP enablement method for this device.  This
> > was one of our first devices so it utilized as much from arm 32-bit as
> > possible.
> 
> Implementation specific enable methods are something we really don't
> want to see for arm64.

I fully agree (and we've been stating this for over two years).

> If PSCI is out of the question then a spin-table shim in your
> bootloader shouldn't be too hard to implement.

And I guess only WFI cpuidle supported in Linux.
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index e0350ca..8517f15 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -5,5 +5,6 @@  dts-dirs += cavium
 dts-dirs += exynos
 dts-dirs += freescale
 dts-dirs += mediatek
+dts-dirs += qcom
 
 subdir-y	:= $(dts-dirs)
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
new file mode 100644
index 0000000..360ec4c
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -0,0 +1,5 @@ 
+dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-mtp.dtb
+
+always		:= $(dtb-y)
+subdir-y	:= $(dts-dirs)
+clean-files	:= *.dtb
diff --git a/arch/arm64/boot/dts/qcom/msm8916-mtp.dts b/arch/arm64/boot/dts/qcom/msm8916-mtp.dts
new file mode 100644
index 0000000..784ad92
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-mtp.dts
@@ -0,0 +1,21 @@ 
+/* Copyright (c) 2014-2015, The Linux Foundation. All rights reserved.
+*
+* This program is free software; you can redistribute it and/or modify
+* it under the terms of the GNU General Public License version 2 and
+* only version 2 as published by the Free Software Foundation.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*/
+
+/dts-v1/;
+
+#include "msm8916-mtp.dtsi"
+
+/ {
+	model = "Qualcomm Technologies, Inc. MSM 8916 MTP";
+	compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360",
+			"qcom,msm8916", "qcom,mtp";
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi
new file mode 100644
index 0000000..4d2f073
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi
@@ -0,0 +1,24 @@ 
+/* Copyright (c) 2014-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "msm8916.dtsi"
+
+/ {
+	soc {
+		serial@78b0000 {
+			status = "okay";
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&blsp1_uart2_default>;
+			pinctrl-1 = <&blsp1_uart2_sleep>;
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
new file mode 100644
index 0000000..957486f
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -0,0 +1,184 @@ 
+/* Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "skeleton.dtsi"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,gcc-msm8916.h>
+#include <dt-bindings/reset/qcom,gcc-msm8916.h>
+
+/ {
+	model = "Qualcomm Technologies, Inc. MSM8916";
+	compatible = "qcom,msm8916";
+
+	interrupt-parent = <&intc>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		CPU0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x0>;
+		};
+
+		CPU1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x1>;
+		};
+
+		CPU2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x2>;
+		};
+
+		CPU3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53", "arm,armv8";
+			reg = <0x3>;
+		};
+	};
+
+	timer {
+		compatible = "arm,armv7-timer";
+		interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 4 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+		clock-frequency = <19200000>;
+	};
+
+	soc: soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0 0 0xffffffff>;
+		compatible = "simple-bus";
+
+		pinctrl@1000000 {
+			compatible = "qcom,msm8916-pinctrl";
+			reg = <0x1000000 0x300000>;
+			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+
+			blsp1_uart2_default: blsp1_uart2_default {
+				pinmux {
+					function = "blsp_uart2";
+					pins = "gpio4", "gpio5";
+				};
+				pinconf {
+					pins = "gpio4", "gpio5";
+					drive-strength = <16>;
+					bias-disable;
+				};
+			};
+
+			blsp1_uart2_sleep: blsp1_uart2_sleep {
+				pinmux {
+					function = "blsp_uart2";
+					pins = "gpio4", "gpio5";
+				};
+				pinconf {
+					pins = "gpio4", "gpio5";
+					drive-strength = <2>;
+					bias-pull-down;
+				};
+			};
+		};
+
+		gcc: qcom,gcc@1800000 {
+			compatible = "qcom,gcc-msm8916";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			reg = <0x1800000 0x80000>;
+		};
+
+		blsp1_uart2: serial@78b0000 {
+			compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
+			reg = <0x78b0000 0x200>;
+			interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
+			clock-names = "core", "iface";
+			status = "disabled";
+		};
+
+		intc: interrupt-controller@b000000 {
+			compatible = "qcom,msm-qgic2";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>;
+		};
+
+		timer@b020000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+			compatible = "arm,armv7-timer-mem";
+			reg = <0xb020000 0x1000>;
+			clock-frequency = <19200000>;
+
+			frame@b021000 {
+				frame-number = <0>;
+				interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xb021000 0x1000>,
+				      <0xb022000 0x1000>;
+			};
+
+			frame@b023000 {
+				frame-number = <1>;
+				interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xb023000 0x1000>;
+				status = "disabled";
+			};
+
+			frame@b024000 {
+				frame-number = <2>;
+				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xb024000 0x1000>;
+				status = "disabled";
+			};
+
+			frame@b025000 {
+				frame-number = <3>;
+				interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xb025000 0x1000>;
+				status = "disabled";
+			};
+
+			frame@b026000 {
+				frame-number = <4>;
+				interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xb026000 0x1000>;
+				status = "disabled";
+			};
+
+			frame@b027000 {
+				frame-number = <5>;
+				interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xb027000 0x1000>;
+				status = "disabled";
+			};
+
+			frame@b028000 {
+				frame-number = <6>;
+				interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xb028000 0x1000>;
+				status = "disabled";
+			};
+		};
+	};
+};