Skip to contentSkip to conversation

Add DTS for SDM845 SoC and MTP

PATCH Superseded Rajendra Nayak 3 participants 9 messages Jan 31, 2018 – Feb 7, 2018

Subsystems (3)

v2 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v2 cover Add DTS for SDM845 SoC and MTP

Now that we have serial[1], pinctrl[2] and clock[3] drivers for SDM845 SoC out on the lists, heres the basic dts files needed to boot a SDM845 based MTP board to a ramfs based serial console shell.

[1] https://patchwork.ozlabs.org/cover/860251/
[2] https://patchwork.kernel.org/patch/10157143/
[3] https://lkml.org/lkml/2018/1/22/78

changes in v2:
* dropped cpu-map
* dropped GIC_CPU_MASK_SIMPLE()
* Added new cpu compatible for kryo385
* added ITS node, marked as disabled
* other cosmetic fixes

Rajendra Nayak (3):
Documentation: dt-bindings: arm: Document kryo385 cpu
arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP
arm64: dts: sdm845: Add serial console support

 Documentation/devicetree/bindings/arm/cpus.txt |   1 +
 arch/arm64/boot/dts/qcom/Makefile              |   1 +
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts        |  55 +++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi           | 298 +++++++++++++++++++++++++
 4 files changed, 355 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#1 lore raw

PATCH v2 1/3 Documentation: dt-bindings: arm: Document kryo385 cpu

Document the compatible string for the Kryo385 cpus found in qualcomm SoCs.

---
 Documentation/devicetree/bindings/arm/cpus.txt | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/arm/cpus.txt +1 -0
index a0009b72e9be..19611ccb61d9 100644
@@ -184,6 +184,7 @@ described below.
 			    "nvidia,tegra186-denver"
 			    "qcom,krait"
 			    "qcom,kryo"
+			    "qcom,kryo385"
 			    "qcom,scorpion"
 	- enable-method
 		Value type: <stringlist>
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
maintainer Reviewed-by on v2 1/3
#2 lore raw

Re: [PATCH v2 1/3] Documentation: dt-bindings: arm: Document kryo385 cpu

Quoted 7 lines · Rajendra Nayak

Document the compatible string for the Kryo385 cpus found in qualcomm SoCs.

---
 Documentation/devicetree/bindings/arm/cpus.txt | 1 +
 1 file changed, 1 insertion(+)

"dt-bindings: arm: " for the subject prefix. Otherwise,

#3 lore raw

PATCH v2 2/3 arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP

In reply to Rajendra Nayak

Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files

---
 arch/arm64/boot/dts/qcom/Makefile       |   1 +
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  13 ++
 arch/arm64/boot/dts/qcom/sdm845.dtsi    | 277 ++++++++++++++++++++++++++++++++
 3 files changed, 291 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
arch/arm64/boot/dts/qcom/Makefile +1 -0
index 55ec5ee7f7e8..9319e74b8906 100644
@@ -6,3 +6,4 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8992-bullhead-rev-101.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-angler-rev-101.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
arch/arm64/boot/dts/qcom/sdm845-mtp.dts new file +13 -0
new file mode 100644
index 000000000000..617c7bb25fb1
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+/dts-v1/;
+
+#include "sdm845.dtsi"
+
+/ {
+	model = "Qualcomm Technologies, Inc. SDM845 MTP";
+	compatible = "qcom,sdm845-mtp";
+};
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +277 -0

277 lines not on this page · raw message

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#4 lore raw

Re: [PATCH v2 2/3] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP

Hi,

Quoted 6 lines · Rajendra Nayak
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +0 -0
new file mode 100644
index 000000000000..02520f19e4ca

arch/arm64/boot/dts/qcom/sdm845.dtsi line 9

Quoted 9 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +9 -0
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+       model = "Qualcomm Technologies, Inc. SDM845";

I'm fairly certain that "model" doesn't belong in the SoC .dtsi file. Only in the board .dts file.

arch/arm64/boot/dts/qcom/sdm845.dtsi line 137

Quoted 2 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +2 -0
+       clocks {
+               xo_board: xo_board {

Just to make it explicit: see my comments in patch 3/3 in this series about using "_" in node names. I believe this should be:

  xo_board: xo-board {

arch/arm64/boot/dts/qcom/sdm845.dtsi line 257

Quoted 1 line
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +1 -0
+               spmi_bus: qcom,spmi at c440000 {

Drop the qcom in the node name. AKA, I believe this should be:

spmi_bus: spmi at c440000 {

Specifically the node name is supposed to be a generic component name then with an address. I see that Rob Herring said the same thing when he reviewed v1 of this patch just now (it seems like people are still commenting there, so make sure you collect the latest feedback from there when re-spinning).

-Doug

#5 lore raw

Re: [PATCH v2 2/3] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP

Quoted 36 lines · Doug Anderson

Hi,

Quoted 6 lines · Rajendra Nayak
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +0 -0
new file mode 100644
index 000000000000..02520f19e4ca

arch/arm64/boot/dts/qcom/sdm845.dtsi line 9

Quoted 9 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +9 -0
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+       model = "Qualcomm Technologies, Inc. SDM845";

I'm fairly certain that "model" doesn't belong in the SoC .dtsi file. Only in the board .dts file.

arch/arm64/boot/dts/qcom/sdm845.dtsi line 137

Quoted 2 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +2 -0
+       clocks {
+               xo_board: xo_board {

Just to make it explicit: see my comments in patch 3/3 in this series about using "_" in node names. I believe this should be:

  xo_board: xo-board {

arch/arm64/boot/dts/qcom/sdm845.dtsi line 257

Quoted 1 line
arch/arm64/boot/dts/qcom/sdm845.dtsi new file +1 -0
+               spmi_bus: qcom,spmi at c440000 {

Drop the qcom in the node name. AKA, I believe this should be:

Quoted 7 lines

spmi_bus: spmi at c440000 {

Specifically the node name is supposed to be a generic component name then with an address. I see that Rob Herring said the same thing when he reviewed v1 of this patch just now (it seems like people are still commenting there, so make sure you collect the latest feedback from there when re-spinning).

yes, I'll make sure I fix up based on Robs' review of the v1.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#6 lore raw

PATCH v2 3/3 arm64: dts: sdm845: Add serial console support

In reply to Rajendra Nayak

Add the qup uart node and geni se instance needed to support the serial console on the MTP.

---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 42 +++++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi    | 21 +++++++++++++++++
 2 files changed, 63 insertions(+)
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +42 -0
index 617c7bb25fb1..42fbf2ab9a2d 100644
@@ -10,4 +10,46 @@
 / {
 	model = "Qualcomm Technologies, Inc. SDM845 MTP";
 	compatible = "qcom,sdm845-mtp";
+
+	aliases {
+		serial0 = &qup_uart2;
+	};
+
+	chosen {
+		stdout-path = "serial0";
+	};
+
+	soc {
+		serial at a84000 {
+			status = "okay";
+		};
+
+		pinctrl at 3400000 {
+			qup_uart2_default: qup_uart2_default {
+				pinmux {
+					function = "qup9";
+					pins = "gpio4", "gpio5";
+				};
+
+				pinconf {
+					pins = "gpio4", "gpio5";
+					drive-strength = <2>;
+					bias-disable;
+				};
+			};
+
+			qup_uart2_sleep: qup_uart2_sleep {
+				pinmux {
+					function = "gpio";
+					pins = "gpio4", "gpio5";
+				};
+
+				pinconf {
+					pins = "gpio4", "gpio5";
+					drive-strength = <2>;
+					bias-disable;
+				};
+			};
+		};
+	};
 };
arch/arm64/boot/dts/qcom/sdm845.dtsi +21 -0
index 02520f19e4ca..c4ce70840acf 100644
@@ -4,6 +4,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,gcc-sdm845.h>
 
 / {
 	model = "Qualcomm Technologies, Inc. SDM845";
@@ -273,5 +274,25 @@
 			cell-index = <0>;
 		};
 
+		qup_1: qcom,geni_se at ac0000 {
+			compatible = "qcom,geni-se-qup";
+			reg = <0xac0000 0x6000>;
+		};
+
+		qup_uart2: serial at a84000 {
+			compatible = "qcom,geni-console", "qcom,geni-uart";
+			reg = <0xa84000 0x4000>;
+			reg-names = "se_phys";
+			clock-names = "se-clk", "m-ahb", "s-ahb";
+			clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>,
+				 <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
+				 <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
+			pinctrl-names = "default", "sleep";
+			pinctrl-0 = <&qup_uart2_default>;
+			pinctrl-1 = <&qup_uart2_sleep>;
+			interrupts = <GIC_SPI 354 IRQ_TYPE_NONE>;
+			qcom,wrapper-core = <&qup_1>;
+			status = "disabled";
+		};
 	};
 };
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#7 lore raw

Re: [PATCH v2 3/3] arm64: dts: sdm845: Add serial console support

Hi,

Quoted 17 lines · Rajendra Nayak

Add the qup uart node and geni se instance needed to support the serial console on the MTP.

---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 42 +++++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi    | 21 +++++++++++++++++
 2 files changed, 63 insertions(+)
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +0 -0
index 617c7bb25fb1..42fbf2ab9a2d 100644
@@ -10,4 +10,46 @@
 / {
        model = "Qualcomm Technologies, Inc. SDM845 MTP";
        compatible = "qcom,sdm845-mtp";

arch/arm64/boot/dts/qcom/sdm845-mtp.dts line 22

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +10 -0
+
+       aliases {
+               serial0 = &qup_uart2;
+       };
+
+       chosen {
+               stdout-path = "serial0";
+       };
+
+       soc {

I don't know if there's an official position, but in general I'm seen people use the actual "soc" alias here. AKA at the top level of this dts, just do:

&soc {
...
};

Normally doing stuff like that is useful so you don't need to replicate the whole hierarchy. In this case that's not a huge savings, but it can be nice to be consistent. In the very least it saves you a level of indentation.

arch/arm64/boot/dts/qcom/sdm845-mtp.dts line 25

Quoted 3 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +3 -0
+               serial at a84000 {
+                       status = "okay";
+               };

Similarly here you can use the alias from the sdm845.dtsi file to avoid replicating the hierarchy. AKA at the top level do:

&qup_uart2 {
status = "okay";
};

In this case it saves you 2 levels of indentation.

arch/arm64/boot/dts/qcom/sdm845-mtp.dts line 28

Quoted 3 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +3 -0
+
+               pinctrl at 3400000 {
+                       qup_uart2_default: qup_uart2_default {

I'm not sure how persnickety I should be, but according to
<https://elinux.org/Device_Tree_Linux>:

node names use dash "-" instead of underscore "_"

...but, of course, labels can't use dashes (and the same page says to use underscore for labels). This is why, in rk3288 for instance, you see:

i2c2_xfer: i2c2-xfer {
rockchip,pins = <6 9 RK_FUNC_1 &pcfg_pull_none>, <6 10 RK_FUNC_1 &pcfg_pull_none>;
};

AKA the label and the node name are the same but the label uses "_" and the node names use "-".

Quoted 15 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +15 -0
+                               pinmux {
+                                       function = "qup9";
+                                       pins = "gpio4", "gpio5";
+                               };
+
+                               pinconf {
+                                       pins = "gpio4", "gpio5";
+                                       drive-strength = <2>;
+                                       bias-disable;
+                               };
+                       };
+
+                       qup_uart2_sleep: qup_uart2_sleep {
+                               pinmux {
+                                       function = "gpio";

arch/arm64/boot/dts/qcom/sdm845-mtp.dts line 53

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +10 -0
+                                       pins = "gpio4", "gpio5";
+                               };
+
+                               pinconf {
+                                       pins = "gpio4", "gpio5";
+                                       drive-strength = <2>;
+                                       bias-disable;
+                               };
+                       };
+               };

As per my (admittedly tardy) response to v1, these shouldn't be in the board file because every board that happens to need uart2 will need to duplicate these definitions.

Quoted 15 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +1 -0
+       };
 };
arch/arm64/boot/dts/qcom/sdm845.dtsi +1 -0
index 02520f19e4ca..c4ce70840acf 100644
@@ -4,6 +4,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,gcc-sdm845.h>
 
 / {
        model = "Qualcomm Technologies, Inc. SDM845";

arch/arm64/boot/dts/qcom/sdm845.dtsi line 279

Quoted 6 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +3 -0
@@ -273,5 +274,25 @@
                        cell-index = <0>;
                };
 
+               qup_1: qcom,geni_se at ac0000 {
+                       compatible = "qcom,geni-se-qup";
+                       reg = <0xac0000 0x6000>;

I think you may have mentioned this in another context, but this doesn't match the current bindings. Some clocks should be here. ...and it looks like the uart should be a subnode.

arch/arm64/boot/dts/qcom/sdm845.dtsi line 292

Quoted 13 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +13 -0
+               };
+
+               qup_uart2: serial at a84000 {
+                       compatible = "qcom,geni-console", "qcom,geni-uart";
+                       reg = <0xa84000 0x4000>;
+                       reg-names = "se_phys";
+                       clock-names = "se-clk", "m-ahb", "s-ahb";
+                       clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>,
+                                <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
+                                <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
+                       pinctrl-names = "default", "sleep";
+                       pinctrl-0 = <&qup_uart2_default>;
+                       pinctrl-1 = <&qup_uart2_sleep>;

In general the "SoC" dtsi file shouldn't be relying on definitions that are made in the board "dts" file.

...if everyone disagrees with me and we really decide to duplicate all the pinctrl in all board files, we'll need to move the "pinctrl-names", "pinctrl-0", and "pinctrl-1" to the board files too. ;)

-Doug

#8 lore raw

Re: [PATCH v2 3/3] arm64: dts: sdm845: Add serial console support

[]..

Quoted 33 lines
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +0 -0
@@ -10,4 +10,46 @@
 / {
        model = "Qualcomm Technologies, Inc. SDM845 MTP";
        compatible = "qcom,sdm845-mtp";

arch/arm64/boot/dts/qcom/sdm845-mtp.dts line 22

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +10 -0
+
+       aliases {
+               serial0 = &qup_uart2;
+       };
+
+       chosen {
+               stdout-path = "serial0";
+       };
+
+       soc {

I don't know if there's an official position, but in general I'm seen people use the actual "soc" alias here. AKA at the top level of this dts, just do:

&soc {
...
};

Normally doing stuff like that is useful so you don't need to replicate the whole hierarchy. In this case that's not a huge savings, but it can be nice to be consistent. In the very least it saves you a level of indentation.

arch/arm64/boot/dts/qcom/sdm845-mtp.dts line 25

Quoted 3 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +3 -0
+               serial at a84000 {
+                       status = "okay";
+               };
Quoted 8 lines

Similarly here you can use the alias from the sdm845.dtsi file to avoid replicating the hierarchy. AKA at the top level do:

&qup_uart2 {
status = "okay";
};

In this case it saves you 2 levels of indentation.

Right. Andy/Bjorn, are there any preferences here?
I see we don't do this for the other board files, and I not sure theres a specific reasoning for how its currently done and if we need to stick to it.

Quoted 14 lines

arch/arm64/boot/dts/qcom/sdm845-mtp.dts line 28

Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +3 -0
+
+               pinctrl at 3400000 {
+                       qup_uart2_default: qup_uart2_default {

I'm not sure how persnickety I should be, but according to
<https://elinux.org/Device_Tree_Linux>:

node names use dash "-" instead of underscore "_"

...but, of course, labels can't use dashes (and the same page says to use underscore for labels). This is why, in rk3288 for instance, you see:

Quoted 7 lines

i2c2_xfer: i2c2-xfer {
rockchip,pins = <6 9 RK_FUNC_1 &pcfg_pull_none>, <6 10 RK_FUNC_1 &pcfg_pull_none>;
};

AKA the label and the node name are the same but the label uses "_" and the node names use "-".

Sure, I'll fix these up.

[]

Quoted 16 lines
Quoted 16 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +1 -0
index 02520f19e4ca..c4ce70840acf 100644
@@ -4,6 +4,7 @@
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,gcc-sdm845.h>
 
 / {
        model = "Qualcomm Technologies, Inc. SDM845";
@@ -273,5 +274,25 @@
                        cell-index = <0>;
                };
 
Quoted 7 lines

arch/arm64/boot/dts/qcom/sdm845.dtsi line 279

Quoted 3 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +3 -0
+               qup_1: qcom,geni_se at ac0000 {
+                       compatible = "qcom,geni-se-qup";
+                       reg = <0xac0000 0x6000>;

I think you may have mentioned this in another context, but this doesn't match the current bindings. Some clocks should be here. ...and it looks like the uart should be a subnode.

right, these were tested with the v1 set for serial. I will update them.

regards
Rajendra

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation