* [PATCH v3 0/3] Add DTS for SDM845 SoC and MTP
@ 2018-02-12 6:28 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: andy.gross-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, evgreen-F7+t8E8rja9g9hUCZPvPmw,
bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
dianders-F7+t8E8rja9g9hUCZPvPmw, Rajendra Nayak
These are basic device tree files needed to boot a SDM845 MTP
board to a ramfs based serial console shell
Bindings are based on whats proposed for pinctrl/serial/clock
drivers for SDM845 SoC
pinctrl: https://patchwork.kernel.org/patch/10157143/ (This is now pulled
in by Linus Walleij for 4.17)
clocks: https://lkml.org/lkml/2018/1/31/209 (under review)
serial: https://patchwork.ozlabs.org/cover/860251/ (under review)
'PATCH 3/3' is based on v2 of serial patches, will need an update if
v3 (still in the works) has further binding updates
Since 'PATCH 2/3' also adds an ITS node and keeps it disabled, we also depend
on https://lkml.org/lkml/2018/1/29/383
changes in v3:
* split the pinmux/pinconf nodes across SoC/Board files
* Fixes for issues reported with 'make dtbs W=2'
* other minor fixes based on review
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):
dt-bindings: arm: Document kryo385 cpu
arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/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 | 47 ++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 314 +++++++++++++++++++++++++
4 files changed, 363 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 0/3] Add DTS for SDM845 SoC and MTP
@ 2018-02-12 6:28 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: andy.gross
Cc: devicetree, linux-arm-msm, linux-kernel, linux-arm-kernel, sboyd,
evgreen, bjorn.andersson, dianders, Rajendra Nayak
These are basic device tree files needed to boot a SDM845 MTP
board to a ramfs based serial console shell
Bindings are based on whats proposed for pinctrl/serial/clock
drivers for SDM845 SoC
pinctrl: https://patchwork.kernel.org/patch/10157143/ (This is now pulled
in by Linus Walleij for 4.17)
clocks: https://lkml.org/lkml/2018/1/31/209 (under review)
serial: https://patchwork.ozlabs.org/cover/860251/ (under review)
'PATCH 3/3' is based on v2 of serial patches, will need an update if
v3 (still in the works) has further binding updates
Since 'PATCH 2/3' also adds an ITS node and keeps it disabled, we also depend
on https://lkml.org/lkml/2018/1/29/383
changes in v3:
* split the pinmux/pinconf nodes across SoC/Board files
* Fixes for issues reported with 'make dtbs W=2'
* other minor fixes based on review
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):
dt-bindings: arm: Document kryo385 cpu
arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/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 | 47 ++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 314 +++++++++++++++++++++++++
4 files changed, 363 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 0/3] Add DTS for SDM845 SoC and MTP
@ 2018-02-12 6:28 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: linux-arm-kernel
These are basic device tree files needed to boot a SDM845 MTP
board to a ramfs based serial console shell
Bindings are based on whats proposed for pinctrl/serial/clock
drivers for SDM845 SoC
pinctrl: https://patchwork.kernel.org/patch/10157143/ (This is now pulled
in by Linus Walleij for 4.17)
clocks: https://lkml.org/lkml/2018/1/31/209 (under review)
serial: https://patchwork.ozlabs.org/cover/860251/ (under review)
'PATCH 3/3' is based on v2 of serial patches, will need an update if
v3 (still in the works) has further binding updates
Since 'PATCH 2/3' also adds an ITS node and keeps it disabled, we also depend
on https://lkml.org/lkml/2018/1/29/383
changes in v3:
* split the pinmux/pinconf nodes across SoC/Board files
* Fixes for issues reported with 'make dtbs W=2'
* other minor fixes based on review
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):
dt-bindings: arm: Document kryo385 cpu
arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/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 | 47 ++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 314 +++++++++++++++++++++++++
4 files changed, 363 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 1/3] dt-bindings: arm: Document kryo385 cpu
2018-02-12 6:28 ` Rajendra Nayak
@ 2018-02-12 6:28 ` Rajendra Nayak
-1 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: andy.gross
Cc: devicetree, linux-arm-msm, linux-kernel, linux-arm-kernel, sboyd,
evgreen, bjorn.andersson, dianders, Rajendra Nayak
Document the compatible string for the Kryo385 cpus found in qualcomm
SoCs.
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/arm/cpus.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index a0009b72e9be..19611ccb61d9 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -184,6 +184,7 @@ described below.
"nvidia,tegra186-denver"
"qcom,krait"
"qcom,kryo"
+ "qcom,kryo385"
"qcom,scorpion"
- enable-method
Value type: <stringlist>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 1/3] dt-bindings: arm: Document kryo385 cpu
@ 2018-02-12 6:28 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: linux-arm-kernel
Document the compatible string for the Kryo385 cpus found in qualcomm
SoCs.
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/arm/cpus.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index a0009b72e9be..19611ccb61d9 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -184,6 +184,7 @@ described below.
"nvidia,tegra186-denver"
"qcom,krait"
"qcom,kryo"
+ "qcom,kryo385"
"qcom,scorpion"
- enable-method
Value type: <stringlist>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
2018-02-12 6:28 ` Rajendra Nayak
@ 2018-02-12 6:28 ` Rajendra Nayak
-1 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: andy.gross
Cc: devicetree, linux-arm-msm, linux-kernel, linux-arm-kernel, sboyd,
evgreen, bjorn.andersson, dianders, Rajendra Nayak
Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 13 ++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 275 ++++++++++++++++++++++++++++++++
3 files changed, 289 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 55ec5ee7f7e8..9319e74b8906 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -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
diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
new file mode 100644
index 000000000000..617c7bb25fb1
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -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";
+};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
new file mode 100644
index 000000000000..55a7e0b454e1
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+ interrupt-parent = <&intc>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ chosen { };
+
+ memory@80000000 {
+ device_type = "memory";
+ /* We expect the bootloader to fill in the size */
+ reg = <0 0x80000000 0 0>;
+ };
+
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ CPU0: cpu@0 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x0>;
+ enable-method = "psci";
+ next-level-cache = <&L2_0>;
+ L2_0: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ L3_0: l3-cache {
+ compatible = "cache";
+ };
+ };
+ };
+
+ CPU1: cpu@100 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x100>;
+ enable-method = "psci";
+ next-level-cache = <&L2_100>;
+ L2_100: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU2: cpu@200 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x200>;
+ enable-method = "psci";
+ next-level-cache = <&L2_200>;
+ L2_200: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU3: cpu@300 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x300>;
+ enable-method = "psci";
+ next-level-cache = <&L2_300>;
+ L2_300: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU4: cpu@400 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x400>;
+ enable-method = "psci";
+ next-level-cache = <&L2_400>;
+ L2_400: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU5: cpu@500 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x500>;
+ enable-method = "psci";
+ next-level-cache = <&L2_500>;
+ L2_500: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU6: cpu@600 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x600>;
+ enable-method = "psci";
+ next-level-cache = <&L2_600>;
+ L2_600: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU7: cpu@700 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x700>;
+ enable-method = "psci";
+ next-level-cache = <&L2_700>;
+ L2_700: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 2 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 3 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ clocks {
+ xo_board: xo-board {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <19200000>;
+ };
+
+ sleep_clk: sleep-clk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <32764>;
+ };
+ };
+
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+ };
+
+ soc: soc {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0 0 0xffffffff>;
+ compatible = "simple-bus";
+
+ intc: interrupt-controller@17a00000 {
+ compatible = "arm,gic-v3";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ #interrupt-cells = <3>;
+ interrupt-controller;
+ #redistributor-regions = <1>;
+ redistributor-stride = <0x0 0x20000>;
+ reg = <0x17a00000 0x10000>, /* GICD */
+ <0x17a60000 0x100000>; /* GICR * 8 */
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+
+ gic-its@17a40000 {
+ compatible = "arm,gic-v3-its";
+ msi-controller;
+ #msi-cells = <1>;
+ reg = <0x17a40000 0x20000>;
+ status = "disabled";
+ };
+ };
+
+ gcc: clock-controller@100000 {
+ compatible = "qcom,gcc-sdm845";
+ reg = <0x100000 0x1f0000>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
+
+ tlmm: pinctrl@3400000 {
+ compatible = "qcom,sdm845-pinctrl";
+ reg = <0x03400000 0xc00000>;
+ interrupts = <GIC_SPI 208 IRQ_TYPE_NONE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ timer@17c90000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ compatible = "arm,armv7-timer-mem";
+ reg = <0x17c90000 0x1000>;
+
+ frame@17ca0000 {
+ frame-number = <0>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17ca0000 0x1000>,
+ <0x17cb0000 0x1000>;
+ };
+
+ frame@17cc0000 {
+ frame-number = <1>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17cc0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame@17cd0000 {
+ frame-number = <2>;
+ interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17cd0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame@17ce0000 {
+ frame-number = <3>;
+ interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17ce0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame@17cf0000 {
+ frame-number = <4>;
+ interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17cf0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame@17d00000 {
+ frame-number = <5>;
+ interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17d00000 0x1000>;
+ status = "disabled";
+ };
+
+ frame@17d10000 {
+ frame-number = <6>;
+ interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17d10000 0x1000>;
+ status = "disabled";
+ };
+ };
+
+ spmi_bus: spmi@c440000 {
+ compatible = "qcom,spmi-pmic-arb";
+ reg = <0xc440000 0x1100>,
+ <0xc600000 0x2000000>,
+ <0xe600000 0x100000>,
+ <0xe700000 0xa0000>,
+ <0xc40a000 0x26000>;
+ reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
+ interrupt-names = "periph_irq";
+ interrupts = <GIC_SPI 481 IRQ_TYPE_NONE>;
+ qcom,ee = <0>;
+ qcom,channel = <0>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+ interrupt-controller;
+ #interrupt-cells = <4>;
+ cell-index = <0>;
+ };
+ };
+};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
@ 2018-02-12 6:28 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: linux-arm-kernel
Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 13 ++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 275 ++++++++++++++++++++++++++++++++
3 files changed, 289 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 55ec5ee7f7e8..9319e74b8906 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -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
diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
new file mode 100644
index 000000000000..617c7bb25fb1
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -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";
+};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
new file mode 100644
index 000000000000..55a7e0b454e1
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+ interrupt-parent = <&intc>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ chosen { };
+
+ memory at 80000000 {
+ device_type = "memory";
+ /* We expect the bootloader to fill in the size */
+ reg = <0 0x80000000 0 0>;
+ };
+
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ CPU0: cpu at 0 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x0>;
+ enable-method = "psci";
+ next-level-cache = <&L2_0>;
+ L2_0: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ L3_0: l3-cache {
+ compatible = "cache";
+ };
+ };
+ };
+
+ CPU1: cpu at 100 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x100>;
+ enable-method = "psci";
+ next-level-cache = <&L2_100>;
+ L2_100: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU2: cpu at 200 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x200>;
+ enable-method = "psci";
+ next-level-cache = <&L2_200>;
+ L2_200: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU3: cpu at 300 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x300>;
+ enable-method = "psci";
+ next-level-cache = <&L2_300>;
+ L2_300: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU4: cpu at 400 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x400>;
+ enable-method = "psci";
+ next-level-cache = <&L2_400>;
+ L2_400: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU5: cpu at 500 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x500>;
+ enable-method = "psci";
+ next-level-cache = <&L2_500>;
+ L2_500: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU6: cpu at 600 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x600>;
+ enable-method = "psci";
+ next-level-cache = <&L2_600>;
+ L2_600: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU7: cpu at 700 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x700>;
+ enable-method = "psci";
+ next-level-cache = <&L2_700>;
+ L2_700: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 2 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 3 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ clocks {
+ xo_board: xo-board {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <19200000>;
+ };
+
+ sleep_clk: sleep-clk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <32764>;
+ };
+ };
+
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+ };
+
+ soc: soc {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0 0 0xffffffff>;
+ compatible = "simple-bus";
+
+ intc: interrupt-controller at 17a00000 {
+ compatible = "arm,gic-v3";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ #interrupt-cells = <3>;
+ interrupt-controller;
+ #redistributor-regions = <1>;
+ redistributor-stride = <0x0 0x20000>;
+ reg = <0x17a00000 0x10000>, /* GICD */
+ <0x17a60000 0x100000>; /* GICR * 8 */
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+
+ gic-its at 17a40000 {
+ compatible = "arm,gic-v3-its";
+ msi-controller;
+ #msi-cells = <1>;
+ reg = <0x17a40000 0x20000>;
+ status = "disabled";
+ };
+ };
+
+ gcc: clock-controller at 100000 {
+ compatible = "qcom,gcc-sdm845";
+ reg = <0x100000 0x1f0000>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
+
+ tlmm: pinctrl at 3400000 {
+ compatible = "qcom,sdm845-pinctrl";
+ reg = <0x03400000 0xc00000>;
+ interrupts = <GIC_SPI 208 IRQ_TYPE_NONE>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ timer at 17c90000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ compatible = "arm,armv7-timer-mem";
+ reg = <0x17c90000 0x1000>;
+
+ frame at 17ca0000 {
+ frame-number = <0>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17ca0000 0x1000>,
+ <0x17cb0000 0x1000>;
+ };
+
+ frame at 17cc0000 {
+ frame-number = <1>;
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17cc0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame at 17cd0000 {
+ frame-number = <2>;
+ interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17cd0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame at 17ce0000 {
+ frame-number = <3>;
+ interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17ce0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame at 17cf0000 {
+ frame-number = <4>;
+ interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17cf0000 0x1000>;
+ status = "disabled";
+ };
+
+ frame at 17d00000 {
+ frame-number = <5>;
+ interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17d00000 0x1000>;
+ status = "disabled";
+ };
+
+ frame at 17d10000 {
+ frame-number = <6>;
+ interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x17d10000 0x1000>;
+ status = "disabled";
+ };
+ };
+
+ spmi_bus: spmi at c440000 {
+ compatible = "qcom,spmi-pmic-arb";
+ reg = <0xc440000 0x1100>,
+ <0xc600000 0x2000000>,
+ <0xe600000 0x100000>,
+ <0xe700000 0xa0000>,
+ <0xc40a000 0x26000>;
+ reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
+ interrupt-names = "periph_irq";
+ interrupts = <GIC_SPI 481 IRQ_TYPE_NONE>;
+ qcom,ee = <0>;
+ qcom,channel = <0>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+ interrupt-controller;
+ #interrupt-cells = <4>;
+ cell-index = <0>;
+ };
+ };
+};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
2018-02-12 6:28 ` Rajendra Nayak
@ 2018-02-12 6:28 ` Rajendra Nayak
-1 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: andy.gross
Cc: devicetree, linux-arm-msm, linux-kernel, linux-arm-kernel, sboyd,
evgreen, bjorn.andersson, dianders, Rajendra Nayak
Add the qup uart node and geni se instance needed to
support the serial console on the MTP.
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 34 ++++++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 39 +++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index 617c7bb25fb1..9eab2b815e0d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -10,4 +10,38 @@
/ {
model = "Qualcomm Technologies, Inc. SDM845 MTP";
compatible = "qcom,sdm845-mtp";
+
+ aliases {
+ serial0 = &qup_uart2;
+ };
+
+ chosen {
+ stdout-path = "serial0";
+ };
+};
+
+&soc {
+ geni-se@ac0000 {
+ serial@a84000 {
+ status = "okay";
+ };
+ };
+
+ pinctrl@3400000 {
+ qup-uart2-default {
+ pinconf {
+ pins = "gpio4", "gpio5";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
+ qup-uart2-sleep {
+ pinconf {
+ pins = "gpio4", "gpio5";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+ };
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 55a7e0b454e1..8cf8df25b06d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4,6 +4,7 @@
*/
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,gcc-sdm845.h>
/ {
interrupt-parent = <&intc>;
@@ -193,6 +194,20 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+
+ qup_uart2_default: qup-uart2-default {
+ pinmux {
+ function = "qup9";
+ pins = "gpio4", "gpio5";
+ };
+ };
+
+ qup_uart2_sleep: qup-uart2-sleep {
+ pinmux {
+ function = "gpio";
+ pins = "gpio4", "gpio5";
+ };
+ };
};
timer@17c90000 {
@@ -271,5 +286,29 @@
#interrupt-cells = <4>;
cell-index = <0>;
};
+
+ qup_1: geni-se@ac0000 {
+ compatible = "qcom,geni-se-qup";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ reg = <0xac0000 0x6000>;
+ clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
+ <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
+ clock-names = "m-ahb", "s-ahb";
+
+ qup_uart2: serial@a84000 {
+ compatible = "qcom,geni-debug-uart";
+ reg = <0xa84000 0x4000>;
+ reg-names = "se-phys";
+ clock-names = "se-clk";
+ clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>;
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&qup_uart2_default>;
+ pinctrl-1 = <&qup_uart2_sleep>;
+ interrupts = <GIC_SPI 354 IRQ_TYPE_NONE>;
+ status = "disabled";
+ };
+ };
};
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
@ 2018-02-12 6:28 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-12 6:28 UTC (permalink / raw)
To: linux-arm-kernel
Add the qup uart node and geni se instance needed to
support the serial console on the MTP.
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 34 ++++++++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 39 +++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index 617c7bb25fb1..9eab2b815e0d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -10,4 +10,38 @@
/ {
model = "Qualcomm Technologies, Inc. SDM845 MTP";
compatible = "qcom,sdm845-mtp";
+
+ aliases {
+ serial0 = &qup_uart2;
+ };
+
+ chosen {
+ stdout-path = "serial0";
+ };
+};
+
+&soc {
+ geni-se at ac0000 {
+ serial at a84000 {
+ status = "okay";
+ };
+ };
+
+ pinctrl at 3400000 {
+ qup-uart2-default {
+ pinconf {
+ pins = "gpio4", "gpio5";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
+ qup-uart2-sleep {
+ pinconf {
+ pins = "gpio4", "gpio5";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+ };
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 55a7e0b454e1..8cf8df25b06d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4,6 +4,7 @@
*/
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,gcc-sdm845.h>
/ {
interrupt-parent = <&intc>;
@@ -193,6 +194,20 @@
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+
+ qup_uart2_default: qup-uart2-default {
+ pinmux {
+ function = "qup9";
+ pins = "gpio4", "gpio5";
+ };
+ };
+
+ qup_uart2_sleep: qup-uart2-sleep {
+ pinmux {
+ function = "gpio";
+ pins = "gpio4", "gpio5";
+ };
+ };
};
timer at 17c90000 {
@@ -271,5 +286,29 @@
#interrupt-cells = <4>;
cell-index = <0>;
};
+
+ qup_1: geni-se at ac0000 {
+ compatible = "qcom,geni-se-qup";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ reg = <0xac0000 0x6000>;
+ clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
+ <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
+ clock-names = "m-ahb", "s-ahb";
+
+ qup_uart2: serial at a84000 {
+ compatible = "qcom,geni-debug-uart";
+ reg = <0xa84000 0x4000>;
+ reg-names = "se-phys";
+ clock-names = "se-clk";
+ clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>;
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&qup_uart2_default>;
+ pinctrl-1 = <&qup_uart2_sleep>;
+ interrupts = <GIC_SPI 354 IRQ_TYPE_NONE>;
+ status = "disabled";
+ };
+ };
};
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
2018-02-12 6:28 ` Rajendra Nayak
@ 2018-02-12 9:39 ` Philippe Ombredanne
-1 siblings, 0 replies; 24+ messages in thread
From: Philippe Ombredanne @ 2018-02-12 9:39 UTC (permalink / raw)
To: Rajendra Nayak
Cc: Andy Gross,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-arm-msm, LKML,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Stephen Boyd, evgreen, Bjorn Andersson, Douglas Anderson
On Mon, Feb 12, 2018 at 7:28 AM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
> Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 13 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 275 ++++++++++++++++++++++++++++++++
> 3 files changed, 289 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 55ec5ee7f7e8..9319e74b8906 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -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
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> new file mode 100644
> index 000000000000..617c7bb25fb1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
This is more cosmetic, but since there is only a single line of
copyright statement and no other comments, it would make sense to use
C++ style // for that line too IMHO (and other similar cases)
--
Cordially
Philippe Ombredanne
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
@ 2018-02-12 9:39 ` Philippe Ombredanne
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Ombredanne @ 2018-02-12 9:39 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 12, 2018 at 7:28 AM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
> Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 13 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 275 ++++++++++++++++++++++++++++++++
> 3 files changed, 289 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 55ec5ee7f7e8..9319e74b8906 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -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
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> new file mode 100644
> index 000000000000..617c7bb25fb1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
This is more cosmetic, but since there is only a single line of
copyright statement and no other comments, it would make sense to use
C++ style // for that line too IMHO (and other similar cases)
--
Cordially
Philippe Ombredanne
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
2018-02-12 6:28 ` Rajendra Nayak
@ 2018-02-13 0:51 ` Doug Anderson
-1 siblings, 0 replies; 24+ messages in thread
From: Doug Anderson @ 2018-02-13 0:51 UTC (permalink / raw)
To: Rajendra Nayak
Cc: Andy Gross, devicetree, linux-arm-msm, LKML, Linux ARM,
Stephen Boyd, evgreen, Bjorn Andersson
Hi,
On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
> Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 13 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 275 ++++++++++++++++++++++++++++++++
> 3 files changed, 289 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 55ec5ee7f7e8..9319e74b8906 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -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
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> new file mode 100644
> index 000000000000..617c7bb25fb1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
It would, of course, be up to Qualcomm. ...but might I suggest instead:
// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
The device tree files really don't have any special secret sauce in
them and IIRC allowing them to have a more permissive MIT license _or_
a GPL allowed people to run other operating systems on these boards.
This kind of thing is better to fix now so we don't have to go and get
everyone's permission later on.
In the past we went through this with Rockchip SoCs in commit
b1772506206f ("ARM: dts: rockchip: relicense rk3288.dtsi under
GPLv2/X11").
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
IMHO add an extra line to this comment with a description to avoid the
bike shedding of how we're supposed to do 1-line comments in device
tree files. AKA:
/*
* SDM845 MTP board device tree source
*
* Copyright (c) 2018, The Linux Foundation. All rights reserved.
*/
> +
> +/dts-v1/;
> +
> +#include "sdm845.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. SDM845 MTP";
> + compatible = "qcom,sdm845-mtp";
For me checkpatch complains about this. It looks like the file
"Documentation/devicetree/bindings/arm/qcom.txt" needs to be updated
with "sdm845". I don't think that will make checkpatch be quiet
(since that file doesn't have a full list of every board), but it
still should be the correct thing to do.
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> new file mode 100644
> index 000000000000..55a7e0b454e1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0
As per above, suggest dual licensed?
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
As per above, suggest adding an extra line to avoid the bikeshed.
SDM845 SoC device tree source
Besides those things, everything looks good as far as I can see. I'm
not an expert on every one of the devices used in this file, but
reading through bindings docs and looking at other users of them, it
looks sane enough. Thus, with the above nits fixed you can feel free
to add my Reviewed-by.
-Doug
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
@ 2018-02-13 0:51 ` Doug Anderson
0 siblings, 0 replies; 24+ messages in thread
From: Doug Anderson @ 2018-02-13 0:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
> Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 13 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 275 ++++++++++++++++++++++++++++++++
> 3 files changed, 289 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 55ec5ee7f7e8..9319e74b8906 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -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
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> new file mode 100644
> index 000000000000..617c7bb25fb1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
It would, of course, be up to Qualcomm. ...but might I suggest instead:
// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
The device tree files really don't have any special secret sauce in
them and IIRC allowing them to have a more permissive MIT license _or_
a GPL allowed people to run other operating systems on these boards.
This kind of thing is better to fix now so we don't have to go and get
everyone's permission later on.
In the past we went through this with Rockchip SoCs in commit
b1772506206f ("ARM: dts: rockchip: relicense rk3288.dtsi under
GPLv2/X11").
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
IMHO add an extra line to this comment with a description to avoid the
bike shedding of how we're supposed to do 1-line comments in device
tree files. AKA:
/*
* SDM845 MTP board device tree source
*
* Copyright (c) 2018, The Linux Foundation. All rights reserved.
*/
> +
> +/dts-v1/;
> +
> +#include "sdm845.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. SDM845 MTP";
> + compatible = "qcom,sdm845-mtp";
For me checkpatch complains about this. It looks like the file
"Documentation/devicetree/bindings/arm/qcom.txt" needs to be updated
with "sdm845". I don't think that will make checkpatch be quiet
(since that file doesn't have a full list of every board), but it
still should be the correct thing to do.
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> new file mode 100644
> index 000000000000..55a7e0b454e1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0
As per above, suggest dual licensed?
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
As per above, suggest adding an extra line to avoid the bikeshed.
SDM845 SoC device tree source
Besides those things, everything looks good as far as I can see. I'm
not an expert on every one of the devices used in this file, but
reading through bindings docs and looking at other users of them, it
looks sane enough. Thus, with the above nits fixed you can feel free
to add my Reviewed-by.
-Doug
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
2018-02-12 6:28 ` Rajendra Nayak
@ 2018-02-14 0:32 ` Doug Anderson
-1 siblings, 0 replies; 24+ messages in thread
From: Doug Anderson @ 2018-02-14 0:32 UTC (permalink / raw)
To: Rajendra Nayak
Cc: Andy Gross, devicetree, linux-arm-msm, LKML, Linux ARM,
Stephen Boyd, evgreen, Bjorn Andersson
Hi,
On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
> Add the qup uart node and geni se instance needed to
> support the serial console on the MTP.
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 34 ++++++++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 39 +++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 617c7bb25fb1..9eab2b815e0d 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -10,4 +10,38 @@
> / {
> model = "Qualcomm Technologies, Inc. SDM845 MTP";
> compatible = "qcom,sdm845-mtp";
> +
> + aliases {
> + serial0 = &qup_uart2;
> + };
> +
> + chosen {
> + stdout-path = "serial0";
> + };
> +};
> +
> +&soc {
> + geni-se@ac0000 {
> + serial@a84000 {
> + status = "okay";
> + };
> + };
If others at QC have already decided that they like the style above
then it's OK with me, but I'd prefer instead (at the top level):
&qup_uart2 {
status = "okay";
};
...then you don't need to replicate all the hierarchy.
> + pinctrl@3400000 {
Similar here. This could be:
&qup_uart2_default {
pinconf {
...
}
};
If you're upset about things being in a "random" order at the top
level, you can still create commented sections in the "dts" file to
organize things, but the above means that you don't end up tabbed in
several levels of indentation for no reason.
> + qup-uart2-default {
> + pinconf {
> + pins = "gpio4", "gpio5";
> + drive-strength = <2>;
> + bias-disable;
Possibly you'd want some sort of pull on the "receive" pin unless
you're guaranteed that on this board that the other side will always
be driving the pin. As far as I can tell this UART goes to a debug
connector. If that debug connector is not populated this pin will be
floating, no?
> + };
> + };
> +
> + qup-uart2-sleep {
> + pinconf {
> + pins = "gpio4", "gpio5";
> + drive-strength = <2>;
Does specifying the "drive-strength" in this case actually do anything
useful? If not can we leave it out?
> + bias-disable;
Feel free to ignore if I'm being ignorant, but I would have expected a
"pull" of some sort to be turned on for the "transmit" pin when you're
in sleep mode. Otherwise the line will be left floating which could
generate noise to the other side, no? ...or is there some sort of
external pull present on this board?
Depending on the board you might also want a pull on the "receive" pin
(assuming we wanted one in the "default" state--see above). With my
extremely limited EE understanding I have the impression that
transitions on a line can still cause power draw even if software
isn't paying attention to them, so it's best to prevent them by adding
a pull.
> + };
> + };
> + };
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 55a7e0b454e1..8cf8df25b06d 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -4,6 +4,7 @@
> */
>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/qcom,gcc-sdm845.h>
>
> / {
> interrupt-parent = <&intc>;
> @@ -193,6 +194,20 @@
> #gpio-cells = <2>;
> interrupt-controller;
> #interrupt-cells = <2>;
> +
> + qup_uart2_default: qup-uart2-default {
> + pinmux {
> + function = "qup9";
> + pins = "gpio4", "gpio5";
> + };
> + };
> +
> + qup_uart2_sleep: qup-uart2-sleep {
> + pinmux {
> + function = "gpio";
> + pins = "gpio4", "gpio5";
> + };
> + };
> };
>
> timer@17c90000 {
> @@ -271,5 +286,29 @@
> #interrupt-cells = <4>;
> cell-index = <0>;
> };
> +
> + qup_1: geni-se@ac0000 {
Color me confused. So you're saying here that this is "qup_1".
...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
"qup9", right? So UART2 is on "qup 1" and "qup 9"?
...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
Maybe there are 3 "QUP v3 modules" each of which handles up to 8
"QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
understands this already and it's just me that's confused then I guess
you can just ignore this comment. However, if you can think of any
better alias than "qup_1" that makes this less confusing then that
would make me extra happy. Like maybe "qupv3_id_1" to match the
manual?
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
@ 2018-02-14 0:32 ` Doug Anderson
0 siblings, 0 replies; 24+ messages in thread
From: Doug Anderson @ 2018-02-14 0:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
> Add the qup uart node and geni se instance needed to
> support the serial console on the MTP.
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 34 ++++++++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 39 +++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 617c7bb25fb1..9eab2b815e0d 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -10,4 +10,38 @@
> / {
> model = "Qualcomm Technologies, Inc. SDM845 MTP";
> compatible = "qcom,sdm845-mtp";
> +
> + aliases {
> + serial0 = &qup_uart2;
> + };
> +
> + chosen {
> + stdout-path = "serial0";
> + };
> +};
> +
> +&soc {
> + geni-se at ac0000 {
> + serial at a84000 {
> + status = "okay";
> + };
> + };
If others at QC have already decided that they like the style above
then it's OK with me, but I'd prefer instead (at the top level):
&qup_uart2 {
status = "okay";
};
...then you don't need to replicate all the hierarchy.
> + pinctrl at 3400000 {
Similar here. This could be:
&qup_uart2_default {
pinconf {
...
}
};
If you're upset about things being in a "random" order at the top
level, you can still create commented sections in the "dts" file to
organize things, but the above means that you don't end up tabbed in
several levels of indentation for no reason.
> + qup-uart2-default {
> + pinconf {
> + pins = "gpio4", "gpio5";
> + drive-strength = <2>;
> + bias-disable;
Possibly you'd want some sort of pull on the "receive" pin unless
you're guaranteed that on this board that the other side will always
be driving the pin. As far as I can tell this UART goes to a debug
connector. If that debug connector is not populated this pin will be
floating, no?
> + };
> + };
> +
> + qup-uart2-sleep {
> + pinconf {
> + pins = "gpio4", "gpio5";
> + drive-strength = <2>;
Does specifying the "drive-strength" in this case actually do anything
useful? If not can we leave it out?
> + bias-disable;
Feel free to ignore if I'm being ignorant, but I would have expected a
"pull" of some sort to be turned on for the "transmit" pin when you're
in sleep mode. Otherwise the line will be left floating which could
generate noise to the other side, no? ...or is there some sort of
external pull present on this board?
Depending on the board you might also want a pull on the "receive" pin
(assuming we wanted one in the "default" state--see above). With my
extremely limited EE understanding I have the impression that
transitions on a line can still cause power draw even if software
isn't paying attention to them, so it's best to prevent them by adding
a pull.
> + };
> + };
> + };
> };
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 55a7e0b454e1..8cf8df25b06d 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -4,6 +4,7 @@
> */
>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/qcom,gcc-sdm845.h>
>
> / {
> interrupt-parent = <&intc>;
> @@ -193,6 +194,20 @@
> #gpio-cells = <2>;
> interrupt-controller;
> #interrupt-cells = <2>;
> +
> + qup_uart2_default: qup-uart2-default {
> + pinmux {
> + function = "qup9";
> + pins = "gpio4", "gpio5";
> + };
> + };
> +
> + qup_uart2_sleep: qup-uart2-sleep {
> + pinmux {
> + function = "gpio";
> + pins = "gpio4", "gpio5";
> + };
> + };
> };
>
> timer at 17c90000 {
> @@ -271,5 +286,29 @@
> #interrupt-cells = <4>;
> cell-index = <0>;
> };
> +
> + qup_1: geni-se at ac0000 {
Color me confused. So you're saying here that this is "qup_1".
...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
"qup9", right? So UART2 is on "qup 1" and "qup 9"?
...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
Maybe there are 3 "QUP v3 modules" each of which handles up to 8
"QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
understands this already and it's just me that's confused then I guess
you can just ignore this comment. However, if you can think of any
better alias than "qup_1" that makes this less confusing then that
would make me extra happy. Like maybe "qupv3_id_1" to match the
manual?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
2018-02-13 0:51 ` Doug Anderson
@ 2018-02-14 8:09 ` Rajendra Nayak
-1 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-14 8:09 UTC (permalink / raw)
To: Doug Anderson
Cc: Andy Gross, devicetree, linux-arm-msm, LKML, Linux ARM,
Stephen Boyd, evgreen, Bjorn Andersson
[]..
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> new file mode 100644
>> index 000000000000..617c7bb25fb1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -0,0 +1,13 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> It would, of course, be up to Qualcomm. ...but might I suggest instead:
>
> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>
> The device tree files really don't have any special secret sauce in
> them and IIRC allowing them to have a more permissive MIT license _or_
> a GPL allowed people to run other operating systems on these boards.
> This kind of thing is better to fix now so we don't have to go and get
> everyone's permission later on.
sure, sounds reasonable, but I am told I need to get this reviewed once
by qualcomm legal before we go ahead and use the dual licensing copyright.
I will update based on what I hear.
[]..
>
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + */
>
> IMHO add an extra line to this comment with a description to avoid the
> bike shedding of how we're supposed to do 1-line comments in device
> tree files. AKA:
>
> /*
> * SDM845 MTP board device tree source
> *
> * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> */
sure will update.
>
>
>> +
>> +/dts-v1/;
>> +
>> +#include "sdm845.dtsi"
>> +
>> +/ {
>> + model = "Qualcomm Technologies, Inc. SDM845 MTP";
>> + compatible = "qcom,sdm845-mtp";
>
> For me checkpatch complains about this. It looks like the file
> "Documentation/devicetree/bindings/arm/qcom.txt" needs to be updated
> with "sdm845". I don't think that will make checkpatch be quiet
> (since that file doesn't have a full list of every board), but it
> still should be the correct thing to do.
sure, I missed updating it, will fix.
>
>
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> new file mode 100644
>> index 000000000000..55a7e0b454e1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -0,0 +1,275 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> As per above, suggest dual licensed?
>
>
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>
> As per above, suggest adding an extra line to avoid the bikeshed.
>
> SDM845 SoC device tree source
>
>
> Besides those things, everything looks good as far as I can see. I'm
> not an expert on every one of the devices used in this file, but
> reading through bindings docs and looking at other users of them, it
> looks sane enough. Thus, with the above nits fixed you can feel free
> to add my Reviewed-by.
Thanks, the only thing I need to wait for before I respin this is to
get a go ahead from legal for the dual licensing copyright header.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP
@ 2018-02-14 8:09 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-14 8:09 UTC (permalink / raw)
To: linux-arm-kernel
[]..
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> new file mode 100644
>> index 000000000000..617c7bb25fb1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -0,0 +1,13 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> It would, of course, be up to Qualcomm. ...but might I suggest instead:
>
> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>
> The device tree files really don't have any special secret sauce in
> them and IIRC allowing them to have a more permissive MIT license _or_
> a GPL allowed people to run other operating systems on these boards.
> This kind of thing is better to fix now so we don't have to go and get
> everyone's permission later on.
sure, sounds reasonable, but I am told I need to get this reviewed once
by qualcomm legal before we go ahead and use the dual licensing copyright.
I will update based on what I hear.
[]..
>
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + */
>
> IMHO add an extra line to this comment with a description to avoid the
> bike shedding of how we're supposed to do 1-line comments in device
> tree files. AKA:
>
> /*
> * SDM845 MTP board device tree source
> *
> * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> */
sure will update.
>
>
>> +
>> +/dts-v1/;
>> +
>> +#include "sdm845.dtsi"
>> +
>> +/ {
>> + model = "Qualcomm Technologies, Inc. SDM845 MTP";
>> + compatible = "qcom,sdm845-mtp";
>
> For me checkpatch complains about this. It looks like the file
> "Documentation/devicetree/bindings/arm/qcom.txt" needs to be updated
> with "sdm845". I don't think that will make checkpatch be quiet
> (since that file doesn't have a full list of every board), but it
> still should be the correct thing to do.
sure, I missed updating it, will fix.
>
>
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> new file mode 100644
>> index 000000000000..55a7e0b454e1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -0,0 +1,275 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> As per above, suggest dual licensed?
>
>
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>
> As per above, suggest adding an extra line to avoid the bikeshed.
>
> SDM845 SoC device tree source
>
>
> Besides those things, everything looks good as far as I can see. I'm
> not an expert on every one of the devices used in this file, but
> reading through bindings docs and looking at other users of them, it
> looks sane enough. Thus, with the above nits fixed you can feel free
> to add my Reviewed-by.
Thanks, the only thing I need to wait for before I respin this is to
get a go ahead from legal for the dual licensing copyright header.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
2018-02-14 0:32 ` Doug Anderson
@ 2018-02-14 9:22 ` Rajendra Nayak
-1 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-14 9:22 UTC (permalink / raw)
To: Doug Anderson
Cc: Andy Gross, devicetree, linux-arm-msm, LKML, Linux ARM,
Stephen Boyd, evgreen, Bjorn Andersson, kramasub,
Girish Mahadevan
On 02/14/2018 06:02 AM, Doug Anderson wrote:
> Hi,
>
> On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>> Add the qup uart node and geni se instance needed to
>> support the serial console on the MTP.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 34 ++++++++++++++++++++++++++++
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 39 +++++++++++++++++++++++++++++++++
>> 2 files changed, 73 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> index 617c7bb25fb1..9eab2b815e0d 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -10,4 +10,38 @@
>> / {
>> model = "Qualcomm Technologies, Inc. SDM845 MTP";
>> compatible = "qcom,sdm845-mtp";
>> +
>> + aliases {
>> + serial0 = &qup_uart2;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0";
>> + };
>> +};
>> +
>> +&soc {
>> + geni-se@ac0000 {
>> + serial@a84000 {
>> + status = "okay";
>> + };
>> + };
>
> If others at QC have already decided that they like the style above
> then it's OK with me, but I'd prefer instead (at the top level):
>
> &qup_uart2 {
> status = "okay";
> };
>
> ...then you don't need to replicate all the hierarchy.
>
>> + pinctrl@3400000 {
>
> Similar here. This could be:
>
> &qup_uart2_default {
> pinconf {
> ...
> }
> };
>
> If you're upset about things being in a "random" order at the top
> level, you can still create commented sections in the "dts" file to
> organize things, but the above means that you don't end up tabbed in
> several levels of indentation for no reason.
Yes, I kept it this way mainly because of Bjorn's concerns about things
being in random order.
Bjorn/Andy, any thoughts?
>
>
>> + qup-uart2-default {
>> + pinconf {
>> + pins = "gpio4", "gpio5";
>> + drive-strength = <2>;
>> + bias-disable;
>
> Possibly you'd want some sort of pull on the "receive" pin unless
> you're guaranteed that on this board that the other side will always
> be driving the pin. As far as I can tell this UART goes to a debug
> connector. If that debug connector is not populated this pin will be
> floating, no?
>
>
>> + };
>> + };
>> +
>> + qup-uart2-sleep {
>> + pinconf {
>> + pins = "gpio4", "gpio5";
>> + drive-strength = <2>;
>
> Does specifying the "drive-strength" in this case actually do anything
> useful? If not can we leave it out?
>
>
>> + bias-disable;
>
> Feel free to ignore if I'm being ignorant, but I would have expected a
> "pull" of some sort to be turned on for the "transmit" pin when you're
> in sleep mode. Otherwise the line will be left floating which could
> generate noise to the other side, no? ...or is there some sort of
> external pull present on this board?
>
> Depending on the board you might also want a pull on the "receive" pin
> (assuming we wanted one in the "default" state--see above). With my
> extremely limited EE understanding I have the impression that
> transitions on a line can still cause power draw even if software
> isn't paying attention to them, so it's best to prevent them by adding
> a pull.
What you are suggesting seems to make sense, however, given I blindly
pulled these in from the internal kernels, I am looping in Karthik/Girish
if they have anything to chime in.
>
>
>> + };
>> + };
>> + };
>> };
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> index 55a7e0b454e1..8cf8df25b06d 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -4,6 +4,7 @@
>> */
>>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/qcom,gcc-sdm845.h>
>>
>> / {
>> interrupt-parent = <&intc>;
>> @@ -193,6 +194,20 @@
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> +
>> + qup_uart2_default: qup-uart2-default {
>> + pinmux {
>> + function = "qup9";
>> + pins = "gpio4", "gpio5";
>> + };
>> + };
>> +
>> + qup_uart2_sleep: qup-uart2-sleep {
>> + pinmux {
>> + function = "gpio";
>> + pins = "gpio4", "gpio5";
>> + };
>> + };
>> };
>>
>> timer@17c90000 {
>> @@ -271,5 +286,29 @@
>> #interrupt-cells = <4>;
>> cell-index = <0>;
>> };
>> +
>> + qup_1: geni-se@ac0000 {
>
> Color me confused. So you're saying here that this is "qup_1".
> ...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
> "qup9", right? So UART2 is on "qup 1" and "qup 9"?
>
> ...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
> Maybe there are 3 "QUP v3 modules" each of which handles up to 8
> "QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
> understands this already and it's just me that's confused then I guess
> you can just ignore this comment. However, if you can think of any
> better alias than "qup_1" that makes this less confusing then that
> would make me extra happy. Like maybe "qupv3_id_1" to match the
> manual?
So FWIK, its one QUP with 8 SE instances, and we have 2 such QUP instances
in SDM845. So yes, I should probably name it qupv3_id_1 to avoid confusion.
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
@ 2018-02-14 9:22 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-14 9:22 UTC (permalink / raw)
To: linux-arm-kernel
On 02/14/2018 06:02 AM, Doug Anderson wrote:
> Hi,
>
> On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>> Add the qup uart node and geni se instance needed to
>> support the serial console on the MTP.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 34 ++++++++++++++++++++++++++++
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 39 +++++++++++++++++++++++++++++++++
>> 2 files changed, 73 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> index 617c7bb25fb1..9eab2b815e0d 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -10,4 +10,38 @@
>> / {
>> model = "Qualcomm Technologies, Inc. SDM845 MTP";
>> compatible = "qcom,sdm845-mtp";
>> +
>> + aliases {
>> + serial0 = &qup_uart2;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0";
>> + };
>> +};
>> +
>> +&soc {
>> + geni-se at ac0000 {
>> + serial at a84000 {
>> + status = "okay";
>> + };
>> + };
>
> If others at QC have already decided that they like the style above
> then it's OK with me, but I'd prefer instead (at the top level):
>
> &qup_uart2 {
> status = "okay";
> };
>
> ...then you don't need to replicate all the hierarchy.
>
>> + pinctrl at 3400000 {
>
> Similar here. This could be:
>
> &qup_uart2_default {
> pinconf {
> ...
> }
> };
>
> If you're upset about things being in a "random" order at the top
> level, you can still create commented sections in the "dts" file to
> organize things, but the above means that you don't end up tabbed in
> several levels of indentation for no reason.
Yes, I kept it this way mainly because of Bjorn's concerns about things
being in random order.
Bjorn/Andy, any thoughts?
>
>
>> + qup-uart2-default {
>> + pinconf {
>> + pins = "gpio4", "gpio5";
>> + drive-strength = <2>;
>> + bias-disable;
>
> Possibly you'd want some sort of pull on the "receive" pin unless
> you're guaranteed that on this board that the other side will always
> be driving the pin. As far as I can tell this UART goes to a debug
> connector. If that debug connector is not populated this pin will be
> floating, no?
>
>
>> + };
>> + };
>> +
>> + qup-uart2-sleep {
>> + pinconf {
>> + pins = "gpio4", "gpio5";
>> + drive-strength = <2>;
>
> Does specifying the "drive-strength" in this case actually do anything
> useful? If not can we leave it out?
>
>
>> + bias-disable;
>
> Feel free to ignore if I'm being ignorant, but I would have expected a
> "pull" of some sort to be turned on for the "transmit" pin when you're
> in sleep mode. Otherwise the line will be left floating which could
> generate noise to the other side, no? ...or is there some sort of
> external pull present on this board?
>
> Depending on the board you might also want a pull on the "receive" pin
> (assuming we wanted one in the "default" state--see above). With my
> extremely limited EE understanding I have the impression that
> transitions on a line can still cause power draw even if software
> isn't paying attention to them, so it's best to prevent them by adding
> a pull.
What you are suggesting seems to make sense, however, given I blindly
pulled these in from the internal kernels, I am looping in Karthik/Girish
if they have anything to chime in.
>
>
>> + };
>> + };
>> + };
>> };
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> index 55a7e0b454e1..8cf8df25b06d 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -4,6 +4,7 @@
>> */
>>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/qcom,gcc-sdm845.h>
>>
>> / {
>> interrupt-parent = <&intc>;
>> @@ -193,6 +194,20 @@
>> #gpio-cells = <2>;
>> interrupt-controller;
>> #interrupt-cells = <2>;
>> +
>> + qup_uart2_default: qup-uart2-default {
>> + pinmux {
>> + function = "qup9";
>> + pins = "gpio4", "gpio5";
>> + };
>> + };
>> +
>> + qup_uart2_sleep: qup-uart2-sleep {
>> + pinmux {
>> + function = "gpio";
>> + pins = "gpio4", "gpio5";
>> + };
>> + };
>> };
>>
>> timer at 17c90000 {
>> @@ -271,5 +286,29 @@
>> #interrupt-cells = <4>;
>> cell-index = <0>;
>> };
>> +
>> + qup_1: geni-se at ac0000 {
>
> Color me confused. So you're saying here that this is "qup_1".
> ...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
> "qup9", right? So UART2 is on "qup 1" and "qup 9"?
>
> ...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
> Maybe there are 3 "QUP v3 modules" each of which handles up to 8
> "QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
> understands this already and it's just me that's confused then I guess
> you can just ignore this comment. However, if you can think of any
> better alias than "qup_1" that makes this less confusing then that
> would make me extra happy. Like maybe "qupv3_id_1" to match the
> manual?
So FWIK, its one QUP with 8 SE instances, and we have 2 such QUP instances
in SDM845. So yes, I should probably name it qupv3_id_1 to avoid confusion.
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
2018-02-14 0:32 ` Doug Anderson
(?)
@ 2018-02-14 19:11 ` Bjorn Andersson
-1 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2018-02-14 19:11 UTC (permalink / raw)
To: Doug Anderson
Cc: Rajendra Nayak, Andy Gross, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, LKML, Linux ARM,
Stephen Boyd, evgreen-F7+t8E8rja9g9hUCZPvPmw
On Tue 13 Feb 16:32 PST 2018, Doug Anderson wrote:
> On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
[..]
> > +&soc {
> > + geni-se@ac0000 {
> > + serial@a84000 {
> > + status = "okay";
> > + };
> > + };
>
> If others at QC have already decided that they like the style above
> then it's OK with me, but I'd prefer instead (at the top level):
>
> &qup_uart2 {
> status = "okay";
> };
>
> ...then you don't need to replicate all the hierarchy.
>
> > + pinctrl@3400000 {
>
> Similar here. This could be:
>
> &qup_uart2_default {
> pinconf {
> ...
> }
> };
>
> If you're upset about things being in a "random" order at the top
> level, you can still create commented sections in the "dts" file to
> organize things, but the above means that you don't end up tabbed in
> several levels of indentation for no reason.
>
I prefer using the hierarchy to describe the relationship between
sibling nodes, in favour of using comments and code review to keep
things in order.
This also mean that nodes that are not references by other parts of the
tree does not need a label.
That said, I've promised to write some patches to convert the prior
platforms to this structure, so let's see how that turns out in
practice - although it's still just an indication of what a fully
described board would look like.
>
> > + qup-uart2-default {
> > + pinconf {
> > + pins = "gpio4", "gpio5";
> > + drive-strength = <2>;
> > + bias-disable;
>
> Possibly you'd want some sort of pull on the "receive" pin unless
> you're guaranteed that on this board that the other side will always
> be driving the pin. As far as I can tell this UART goes to a debug
> connector. If that debug connector is not populated this pin will be
> floating, no?
>
The rx pin is typically bias-pull-up and tx bias-disable, so I would
expect the same.
[..]
> > +
> > + qup_1: geni-se@ac0000 {
>
> Color me confused. So you're saying here that this is "qup_1".
> ...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
> "qup9", right? So UART2 is on "qup 1" and "qup 9"?
>
> ...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
> Maybe there are 3 "QUP v3 modules" each of which handles up to 8
> "QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
> understands this already and it's just me that's confused then I guess
> you can just ignore this comment. However, if you can think of any
> better alias than "qup_1" that makes this less confusing then that
> would make me extra happy. Like maybe "qupv3_id_1" to match the
> manual?
This is indeed a source of confusion, in particular since there tend to
be different numbering depending on what part of the puzzle you look at.
But you're right that each QUP has a number of engines, each one being a
UART/I2C/SPI controller.
I don't see a reason for labeling this particular node though, aliases
references individual engines, not the wrapper.
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
@ 2018-02-14 19:11 ` Bjorn Andersson
0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2018-02-14 19:11 UTC (permalink / raw)
To: Doug Anderson
Cc: Rajendra Nayak, Andy Gross, devicetree, linux-arm-msm, LKML,
Linux ARM, Stephen Boyd, evgreen
On Tue 13 Feb 16:32 PST 2018, Doug Anderson wrote:
> On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
[..]
> > +&soc {
> > + geni-se@ac0000 {
> > + serial@a84000 {
> > + status = "okay";
> > + };
> > + };
>
> If others at QC have already decided that they like the style above
> then it's OK with me, but I'd prefer instead (at the top level):
>
> &qup_uart2 {
> status = "okay";
> };
>
> ...then you don't need to replicate all the hierarchy.
>
> > + pinctrl@3400000 {
>
> Similar here. This could be:
>
> &qup_uart2_default {
> pinconf {
> ...
> }
> };
>
> If you're upset about things being in a "random" order at the top
> level, you can still create commented sections in the "dts" file to
> organize things, but the above means that you don't end up tabbed in
> several levels of indentation for no reason.
>
I prefer using the hierarchy to describe the relationship between
sibling nodes, in favour of using comments and code review to keep
things in order.
This also mean that nodes that are not references by other parts of the
tree does not need a label.
That said, I've promised to write some patches to convert the prior
platforms to this structure, so let's see how that turns out in
practice - although it's still just an indication of what a fully
described board would look like.
>
> > + qup-uart2-default {
> > + pinconf {
> > + pins = "gpio4", "gpio5";
> > + drive-strength = <2>;
> > + bias-disable;
>
> Possibly you'd want some sort of pull on the "receive" pin unless
> you're guaranteed that on this board that the other side will always
> be driving the pin. As far as I can tell this UART goes to a debug
> connector. If that debug connector is not populated this pin will be
> floating, no?
>
The rx pin is typically bias-pull-up and tx bias-disable, so I would
expect the same.
[..]
> > +
> > + qup_1: geni-se@ac0000 {
>
> Color me confused. So you're saying here that this is "qup_1".
> ...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
> "qup9", right? So UART2 is on "qup 1" and "qup 9"?
>
> ...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
> Maybe there are 3 "QUP v3 modules" each of which handles up to 8
> "QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
> understands this already and it's just me that's confused then I guess
> you can just ignore this comment. However, if you can think of any
> better alias than "qup_1" that makes this less confusing then that
> would make me extra happy. Like maybe "qupv3_id_1" to match the
> manual?
This is indeed a source of confusion, in particular since there tend to
be different numbering depending on what part of the puzzle you look at.
But you're right that each QUP has a number of engines, each one being a
UART/I2C/SPI controller.
I don't see a reason for labeling this particular node though, aliases
references individual engines, not the wrapper.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
@ 2018-02-14 19:11 ` Bjorn Andersson
0 siblings, 0 replies; 24+ messages in thread
From: Bjorn Andersson @ 2018-02-14 19:11 UTC (permalink / raw)
To: linux-arm-kernel
On Tue 13 Feb 16:32 PST 2018, Doug Anderson wrote:
> On Sun, Feb 11, 2018 at 10:28 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote:
[..]
> > +&soc {
> > + geni-se at ac0000 {
> > + serial at a84000 {
> > + status = "okay";
> > + };
> > + };
>
> If others at QC have already decided that they like the style above
> then it's OK with me, but I'd prefer instead (at the top level):
>
> &qup_uart2 {
> status = "okay";
> };
>
> ...then you don't need to replicate all the hierarchy.
>
> > + pinctrl at 3400000 {
>
> Similar here. This could be:
>
> &qup_uart2_default {
> pinconf {
> ...
> }
> };
>
> If you're upset about things being in a "random" order at the top
> level, you can still create commented sections in the "dts" file to
> organize things, but the above means that you don't end up tabbed in
> several levels of indentation for no reason.
>
I prefer using the hierarchy to describe the relationship between
sibling nodes, in favour of using comments and code review to keep
things in order.
This also mean that nodes that are not references by other parts of the
tree does not need a label.
That said, I've promised to write some patches to convert the prior
platforms to this structure, so let's see how that turns out in
practice - although it's still just an indication of what a fully
described board would look like.
>
> > + qup-uart2-default {
> > + pinconf {
> > + pins = "gpio4", "gpio5";
> > + drive-strength = <2>;
> > + bias-disable;
>
> Possibly you'd want some sort of pull on the "receive" pin unless
> you're guaranteed that on this board that the other side will always
> be driving the pin. As far as I can tell this UART goes to a debug
> connector. If that debug connector is not populated this pin will be
> floating, no?
>
The rx pin is typically bias-pull-up and tx bias-disable, so I would
expect the same.
[..]
> > +
> > + qup_1: geni-se at ac0000 {
>
> Color me confused. So you're saying here that this is "qup_1".
> ...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
> "qup9", right? So UART2 is on "qup 1" and "qup 9"?
>
> ...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
> Maybe there are 3 "QUP v3 modules" each of which handles up to 8
> "QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
> understands this already and it's just me that's confused then I guess
> you can just ignore this comment. However, if you can think of any
> better alias than "qup_1" that makes this less confusing then that
> would make me extra happy. Like maybe "qupv3_id_1" to match the
> manual?
This is indeed a source of confusion, in particular since there tend to
be different numbering depending on what part of the puzzle you look at.
But you're right that each QUP has a number of engines, each one being a
UART/I2C/SPI controller.
I don't see a reason for labeling this particular node though, aliases
references individual engines, not the wrapper.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
2018-02-14 19:11 ` Bjorn Andersson
@ 2018-02-15 5:13 ` Rajendra Nayak
-1 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-15 5:13 UTC (permalink / raw)
To: Bjorn Andersson, Doug Anderson
Cc: Andy Gross, devicetree, linux-arm-msm, LKML, Linux ARM,
Stephen Boyd, evgreen
On 02/15/2018 12:41 AM, Bjorn Andersson wrote:
> [..]
>>> +
>>> + qup_1: geni-se@ac0000 {
>> Color me confused. So you're saying here that this is "qup_1".
>> ...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
>> "qup9", right? So UART2 is on "qup 1" and "qup 9"?
>>
>> ...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
>> Maybe there are 3 "QUP v3 modules" each of which handles up to 8
>> "QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
>> understands this already and it's just me that's confused then I guess
>> you can just ignore this comment. However, if you can think of any
>> better alias than "qup_1" that makes this less confusing then that
>> would make me extra happy. Like maybe "qupv3_id_1" to match the
>> manual?
> This is indeed a source of confusion, in particular since there tend to
> be different numbering depending on what part of the puzzle you look at.
> But you're right that each QUP has a number of engines, each one being a
> UART/I2C/SPI controller.
>
> I don't see a reason for labeling this particular node though, aliases
> references individual engines, not the wrapper.
makes sense, I'll just drop the label.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support
@ 2018-02-15 5:13 ` Rajendra Nayak
0 siblings, 0 replies; 24+ messages in thread
From: Rajendra Nayak @ 2018-02-15 5:13 UTC (permalink / raw)
To: linux-arm-kernel
On 02/15/2018 12:41 AM, Bjorn Andersson wrote:
> [..]
>>> +
>>> + qup_1: geni-se at ac0000 {
>> Color me confused. So you're saying here that this is "qup_1".
>> ...but above you turn the pinmux for pins "GPIO4" and "GPIO5" to
>> "qup9", right? So UART2 is on "qup 1" and "qup 9"?
>>
>> ...OK, so I stared at manuals a bunch more, and _maybe_ I get it.
>> Maybe there are 3 "QUP v3 modules" each of which handles up to 8
>> "QUP"s. So QUP 9 is on "QUP module 1", is that right? If everyone
>> understands this already and it's just me that's confused then I guess
>> you can just ignore this comment. However, if you can think of any
>> better alias than "qup_1" that makes this less confusing then that
>> would make me extra happy. Like maybe "qupv3_id_1" to match the
>> manual?
> This is indeed a source of confusion, in particular since there tend to
> be different numbering depending on what part of the puzzle you look at.
> But you're right that each QUP has a number of engines, each one being a
> UART/I2C/SPI controller.
>
> I don't see a reason for labeling this particular node though, aliases
> references individual engines, not the wrapper.
makes sense, I'll just drop the label.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-02-15 5:13 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-12 6:28 [PATCH v3 0/3] Add DTS for SDM845 SoC and MTP Rajendra Nayak
2018-02-12 6:28 ` Rajendra Nayak
2018-02-12 6:28 ` Rajendra Nayak
2018-02-12 6:28 ` [PATCH v3 1/3] dt-bindings: arm: Document kryo385 cpu Rajendra Nayak
2018-02-12 6:28 ` Rajendra Nayak
2018-02-12 6:28 ` [PATCH v3 2/3] arm64: dts: sdm845: Add minimal dts files for sdm845 SoC/MTP Rajendra Nayak
2018-02-12 6:28 ` Rajendra Nayak
2018-02-12 9:39 ` Philippe Ombredanne
2018-02-12 9:39 ` Philippe Ombredanne
2018-02-13 0:51 ` Doug Anderson
2018-02-13 0:51 ` Doug Anderson
2018-02-14 8:09 ` Rajendra Nayak
2018-02-14 8:09 ` Rajendra Nayak
2018-02-12 6:28 ` [PATCH v3 3/3] arm64: dts: sdm845: Add serial console support Rajendra Nayak
2018-02-12 6:28 ` Rajendra Nayak
2018-02-14 0:32 ` Doug Anderson
2018-02-14 0:32 ` Doug Anderson
2018-02-14 9:22 ` Rajendra Nayak
2018-02-14 9:22 ` Rajendra Nayak
[not found] ` <CAD=FV=UXrmpsK3k+8yZCqVi6S+c_3icPUViDST+shnrQW1A6EA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 19:11 ` Bjorn Andersson
2018-02-14 19:11 ` Bjorn Andersson
2018-02-14 19:11 ` Bjorn Andersson
2018-02-15 5:13 ` Rajendra Nayak
2018-02-15 5:13 ` Rajendra Nayak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.