* [PATCH 0/2] Add DTS for SDM845 SoC and MTP @ 2018-01-25 16:32 Rajendra Nayak 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak 2018-01-25 16:32 ` [PATCH 2/2] arm64: dts: sdm845: Add serial console support Rajendra Nayak 0 siblings, 2 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-01-25 16:32 UTC (permalink / raw) To: linux-arm-kernel 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 Rajendra Nayak (2): arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP arm64: dts: sdm845: Add serial console support arch/arm64/boot/dts/qcom/Makefile | 1 + arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 13 ++ arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi | 14 ++ arch/arm64/boot/dts/qcom/sdm845-pins.dtsi | 32 +++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 330 ++++++++++++++++++++++++++++++ 5 files changed, 390 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi create mode 100644 arch/arm64/boot/dts/qcom/sdm845-pins.dtsi 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] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-25 16:32 [PATCH 0/2] Add DTS for SDM845 SoC and MTP Rajendra Nayak @ 2018-01-25 16:32 ` Rajendra Nayak 2018-01-26 20:31 ` Evan Green ` (3 more replies) 2018-01-25 16:32 ` [PATCH 2/2] arm64: dts: sdm845: Add serial console support Rajendra Nayak 1 sibling, 4 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-01-25 16:32 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-mtp.dtsi | 11 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 308 +++++++++++++++++++++++++++++++ 4 files changed, 333 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi 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..c57701bed084 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..95e41e42bee1 --- /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-mtp.dtsi" + +/ { + model = "Qualcomm Technologies, Inc. SDM845 MTP"; + compatible = "qcom,sdm845-mtp"; +}; diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi b/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi new file mode 100644 index 000000000000..5b1022c20bad --- /dev/null +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#include "sdm845.dtsi" + +/ { + soc { + }; +}; diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi new file mode 100644 index 000000000000..a21f4912b3e2 --- /dev/null +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -0,0 +1,308 @@ +// 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"; + + interrupt-parent = <&intc>; + + #address-cells = <2>; + #size-cells = <2>; + + chosen { }; + + memory { + device_type = "memory"; + /* We expect the bootloader to fill in the reg */ + reg = <0 0 0 0>; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>; + + CPU0: cpu at 0 { + device_type = "cpu"; + compatible = "qcom,kryo"; + 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,kryo"; + 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,kryo"; + 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,kryo"; + 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,kryo"; + 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,kryo"; + 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,kryo"; + 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,kryo"; + reg = <0x0 0x700>; + enable-method = "psci"; + next-level-cache = <&L2_700>; + L2_700: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + cpu-map { + cluster0 { + core0 { + cpu = <&CPU0>; + }; + + core1 { + cpu = <&CPU1>; + }; + + core2 { + cpu = <&CPU2>; + }; + + core3 { + cpu = <&CPU3>; + }; + }; + + cluster1 { + core0 { + cpu = <&CPU4>; + }; + + core1 { + cpu = <&CPU5>; + }; + + core2 { + cpu = <&CPU6>; + }; + + core3 { + cpu = <&CPU7>; + }; + }; + }; + }; + + timer { + compatible = "arm,armv8-timer"; + interrupts = <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 0 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; + }; + + clocks { + xo_board: xo_board { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <19200000>; + clock-output-names = "xo_board"; + }; + + sleep_clk: sleep_clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32764>; + clock-output-names = "sleep_clk"; + }; + }; + + 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"; + #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>; + }; + + gcc: clock-controller at 100000 { + compatible = "qcom,gcc-sdm845"; + reg = <0x100000 0x1f0000>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + + tlmm: pinctrl at 03400000 { + 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>; + clock-frequency = <19200000>; + + 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: qcom,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] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak @ 2018-01-26 20:31 ` Evan Green 2018-01-30 8:48 ` Rajendra Nayak 2018-01-26 22:15 ` Stephen Boyd ` (2 subsequent siblings) 3 siblings, 1 reply; 25+ messages in thread From: Evan Green @ 2018-01-26 20:31 UTC (permalink / raw) To: linux-arm-kernel Hi Rajendra, On Thu, Jan 25, 2018 at 8:32 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-mtp.dtsi | 11 ++ > arch/arm64/boot/dts/qcom/sdm845.dtsi | 308 +++++++++++++++++++++++++++++++ > 4 files changed, 333 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi > 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..c57701bed084 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 Minor nit: This should be a tab before the += to be consistent. (Unfortunately I don't have the expertise quite yet to comment on the rest of this). -Evan ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-26 20:31 ` Evan Green @ 2018-01-30 8:48 ` Rajendra Nayak 0 siblings, 0 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-01-30 8:48 UTC (permalink / raw) To: linux-arm-kernel On 01/27/2018 02:01 AM, Evan Green wrote: > Hi Rajendra, > > On Thu, Jan 25, 2018 at 8:32 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-mtp.dtsi | 11 ++ >> arch/arm64/boot/dts/qcom/sdm845.dtsi | 308 +++++++++++++++++++++++++++++++ >> 4 files changed, 333 insertions(+) >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi >> 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..c57701bed084 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 > > Minor nit: This should be a tab before the += to be consistent. thanks, will fix when I respin. > > (Unfortunately I don't have the expertise quite yet to comment on the > rest of this). > > -Evan > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak 2018-01-26 20:31 ` Evan Green @ 2018-01-26 22:15 ` Stephen Boyd 2018-01-29 8:13 ` Rajendra Nayak 2018-02-06 20:26 ` Rob Herring 2018-02-06 18:54 ` Bjorn Andersson 2018-02-06 20:31 ` Rob Herring 3 siblings, 2 replies; 25+ messages in thread From: Stephen Boyd @ 2018-01-26 22:15 UTC (permalink / raw) To: linux-arm-kernel On 01/25, Rajendra Nayak wrote: > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi Do we really need two files? Maybe collapse the two? > create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > new file mode 100644 > index 000000000000..a21f4912b3e2 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -0,0 +1,308 @@ > +// 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"; > + > + interrupt-parent = <&intc>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + > + chosen { }; > + > + memory { > + device_type = "memory"; > + /* We expect the bootloader to fill in the reg */ > + reg = <0 0 0 0>; > + }; > + > + cpus { > + #address-cells = <2>; > + #size-cells = <0>; > + > + CPU0: cpu at 0 { > + device_type = "cpu"; > + compatible = "qcom,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + reg = <0x0 0x700>; > + enable-method = "psci"; > + next-level-cache = <&L2_700>; > + L2_700: l2-cache { > + compatible = "cache"; > + next-level-cache = <&L3_0>; > + }; > + }; > + > + cpu-map { > + cluster0 { > + core0 { > + cpu = <&CPU0>; > + }; > + > + core1 { > + cpu = <&CPU1>; > + }; > + > + core2 { > + cpu = <&CPU2>; > + }; > + > + core3 { > + cpu = <&CPU3>; > + }; > + }; > + > + cluster1 { > + core0 { > + cpu = <&CPU4>; > + }; > + > + core1 { > + cpu = <&CPU5>; > + }; > + > + core2 { > + cpu = <&CPU6>; > + }; > + > + core3 { > + cpu = <&CPU7>; > + }; > + }; > + }; >From what I recall, this layout causes the kernel to spew warnings? I mean to say this is the power/performance view, but not the architectural view. > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, Are we supposed to use the GIC_CPU_MASK_SIMPLE macros still? > + <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 0 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; > + }; > + > + clocks { > + xo_board: xo_board { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <19200000>; > + clock-output-names = "xo_board"; We can drop clock-output-names on these. > + }; > + > + sleep_clk: sleep_clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32764>; > + clock-output-names = "sleep_clk"; > + }; > + }; > + > + psci { > + compatible = "arm,psci-1.0"; > + method = "smc"; > + }; > + > + soc: soc { Will anyone use this phandle? > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0 0 0xffffffff>; > + compatible = "simple-bus"; > + > + intc: interrupt-controller at 17a00000 { > + compatible = "arm,gic-v3"; > + #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>; Can you also add the ITS node please and mark it as disabled? I'll send a patch to the list to skip status = "disabled" ones. We may want to support ITS on these SoCs if the firmware is different. > + }; > + > + gcc: clock-controller at 100000 { > + compatible = "qcom,gcc-sdm845"; > + reg = <0x100000 0x1f0000>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + }; > + > + tlmm: pinctrl at 03400000 { Drop leading zeroes please. > + 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 { Lowercase hex please. > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + compatible = "arm,armv7-timer-mem"; > + reg = <0x17C90000 0x1000>; Lowercase hex please. > + clock-frequency = <19200000>; Drop this? Or we can't read it from the hardware so we have to hardcode it? > + > + frame at 17CA0000 { Lowecase again. > + frame-number = <0>; > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>; > + reg = <0x17CA0000 0x1000>, > + <0x17CB0000 0x1000>; > + }; > + -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-26 22:15 ` Stephen Boyd @ 2018-01-29 8:13 ` Rajendra Nayak 2018-01-30 9:48 ` Stephen Boyd 2018-02-06 20:26 ` Rob Herring 1 sibling, 1 reply; 25+ messages in thread From: Rajendra Nayak @ 2018-01-29 8:13 UTC (permalink / raw) To: linux-arm-kernel On 01/27/2018 03:45 AM, Stephen Boyd wrote: > On 01/25, Rajendra Nayak wrote: >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi > > Do we really need two files? Maybe collapse the two? will do. Not sure why, but this is how all other qualcomm boards are structured with an almost empty .dts file. > >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi >> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> new file mode 100644 >> index 000000000000..a21f4912b3e2 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> @@ -0,0 +1,308 @@ >> +// 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"; >> + >> + interrupt-parent = <&intc>; >> + >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + chosen { }; >> + >> + memory { >> + device_type = "memory"; >> + /* We expect the bootloader to fill in the reg */ >> + reg = <0 0 0 0>; >> + }; >> + >> + cpus { >> + #address-cells = <2>; >> + #size-cells = <0>; >> + >> + CPU0: cpu at 0 { >> + device_type = "cpu"; >> + compatible = "qcom,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + reg = <0x0 0x700>; >> + enable-method = "psci"; >> + next-level-cache = <&L2_700>; >> + L2_700: l2-cache { >> + compatible = "cache"; >> + next-level-cache = <&L3_0>; >> + }; >> + }; >> + >> + cpu-map { >> + cluster0 { >> + core0 { >> + cpu = <&CPU0>; >> + }; >> + >> + core1 { >> + cpu = <&CPU1>; >> + }; >> + >> + core2 { >> + cpu = <&CPU2>; >> + }; >> + >> + core3 { >> + cpu = <&CPU3>; >> + }; >> + }; >> + >> + cluster1 { >> + core0 { >> + cpu = <&CPU4>; >> + }; >> + >> + core1 { >> + cpu = <&CPU5>; >> + }; >> + >> + core2 { >> + cpu = <&CPU6>; >> + }; >> + >> + core3 { >> + cpu = <&CPU7>; >> + }; >> + }; >> + }; > > From what I recall, this layout causes the kernel to spew > warnings? I mean to say this is the power/performance view, but > not the architectural view. hmm, I haven't seen any warnings with this when I boot up on my sdm845 MTP. Will recheck. > >> + }; >> + >> + timer { >> + compatible = "arm,armv8-timer"; >> + interrupts = <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > > Are we supposed to use the GIC_CPU_MASK_SIMPLE macros still? Not sure, is there another way? > >> + <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 0 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; >> + }; >> + >> + clocks { >> + xo_board: xo_board { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <19200000>; >> + clock-output-names = "xo_board"; > > We can drop clock-output-names on these. will do. > >> + }; >> + >> + sleep_clk: sleep_clk { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <32764>; >> + clock-output-names = "sleep_clk"; >> + }; >> + }; >> + >> + psci { >> + compatible = "arm,psci-1.0"; >> + method = "smc"; >> + }; >> + >> + soc: soc { > > Will anyone use this phandle? maybe not, will drop. > >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges = <0 0 0 0xffffffff>; >> + compatible = "simple-bus"; >> + >> + intc: interrupt-controller at 17a00000 { >> + compatible = "arm,gic-v3"; >> + #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>; > > Can you also add the ITS node please and mark it as disabled? > I'll send a patch to the list to skip status = "disabled" ones. > We may want to support ITS on these SoCs if the firmware is > different. will add. > >> + }; >> + >> + gcc: clock-controller at 100000 { >> + compatible = "qcom,gcc-sdm845"; >> + reg = <0x100000 0x1f0000>; >> + #clock-cells = <1>; >> + #reset-cells = <1>; >> + }; >> + >> + tlmm: pinctrl at 03400000 { > > Drop leading zeroes please. > >> + 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 { > > Lowercase hex please. > >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + compatible = "arm,armv7-timer-mem"; >> + reg = <0x17C90000 0x1000>; > > Lowercase hex please. > >> + clock-frequency = <19200000>; > > Drop this? Or we can't read it from the hardware so we have to > hardcode it? will drop, shouldn't be needed. > >> + >> + frame at 17CA0000 { > > Lowecase again. > >> + frame-number = <0>; >> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>; >> + reg = <0x17CA0000 0x1000>, >> + <0x17CB0000 0x1000>; >> + }; >> + > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-29 8:13 ` Rajendra Nayak @ 2018-01-30 9:48 ` Stephen Boyd 2018-01-30 10:25 ` Rajendra Nayak 0 siblings, 1 reply; 25+ messages in thread From: Stephen Boyd @ 2018-01-30 9:48 UTC (permalink / raw) To: linux-arm-kernel On 01/29, Rajendra Nayak wrote: > > > On 01/27/2018 03:45 AM, Stephen Boyd wrote: > > On 01/25, Rajendra Nayak wrote: > >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts > >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi > > > > Do we really need two files? Maybe collapse the two? > > will do. Not sure why, but this is how all other qualcomm > boards are structured with an almost empty .dts file. I think it's because we have v1, v2, v3, etc. when we're sorting out the versions of silicon, but then those are all dropped when everything is done. Upstream we probably never care. > >> + > >> + core2 { > >> + cpu = <&CPU6>; > >> + }; > >> + > >> + core3 { > >> + cpu = <&CPU7>; > >> + }; > >> + }; > >> + }; > > > > From what I recall, this layout causes the kernel to spew > > warnings? I mean to say this is the power/performance view, but > > not the architectural view. > > hmm, I haven't seen any warnings with this when I boot up on my sdm845 > MTP. Will recheck. Ok! I will recheck as well. > > > > >> + }; > >> + > >> + timer { > >> + compatible = "arm,armv8-timer"; > >> + interrupts = <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > > > > Are we supposed to use the GIC_CPU_MASK_SIMPLE macros still? > > Not sure, is there another way? Me either. See this thread from Marc[1]. I guess just drop them? [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.0/03522.html -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-30 9:48 ` Stephen Boyd @ 2018-01-30 10:25 ` Rajendra Nayak 0 siblings, 0 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-01-30 10:25 UTC (permalink / raw) To: linux-arm-kernel On 01/30/2018 03:18 PM, Stephen Boyd wrote: >>>> + }; >>>> + >>>> + timer { >>>> + compatible = "arm,armv8-timer"; >>>> + interrupts = <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >>> Are we supposed to use the GIC_CPU_MASK_SIMPLE macros still? >> Not sure, is there another way? > Me either. See this thread from Marc[1]. I guess just drop them? > > [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.0/03522.html thanks, Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt does seem to explain how to specify PPI CPU affinity for GICv3 using a 4th cell if needed which I hadn't read :/ I'll send in a patch to get rid of them on 8996 as well. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-26 22:15 ` Stephen Boyd 2018-01-29 8:13 ` Rajendra Nayak @ 2018-02-06 20:26 ` Rob Herring 2018-02-07 4:14 ` Rajendra Nayak 1 sibling, 1 reply; 25+ messages in thread From: Rob Herring @ 2018-02-06 20:26 UTC (permalink / raw) To: linux-arm-kernel On Fri, Jan 26, 2018 at 4:15 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 01/25, Rajendra Nayak wrote: >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi > > Do we really need two files? Maybe collapse the two? > >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi >> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> new file mode 100644 >> index 000000000000..a21f4912b3e2 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> @@ -0,0 +1,308 @@ >> +// 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"; >> + >> + interrupt-parent = <&intc>; >> + >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + chosen { }; >> + >> + memory { >> + device_type = "memory"; >> + /* We expect the bootloader to fill in the reg */ >> + reg = <0 0 0 0>; >> + }; >> + >> + cpus { >> + #address-cells = <2>; >> + #size-cells = <0>; >> + >> + CPU0: cpu at 0 { >> + device_type = "cpu"; >> + compatible = "qcom,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + 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,kryo"; >> + reg = <0x0 0x700>; >> + enable-method = "psci"; >> + next-level-cache = <&L2_700>; >> + L2_700: l2-cache { >> + compatible = "cache"; >> + next-level-cache = <&L3_0>; >> + }; >> + }; >> + >> + cpu-map { >> + cluster0 { >> + core0 { >> + cpu = <&CPU0>; >> + }; >> + >> + core1 { >> + cpu = <&CPU1>; >> + }; >> + >> + core2 { >> + cpu = <&CPU2>; >> + }; >> + >> + core3 { >> + cpu = <&CPU3>; >> + }; >> + }; >> + >> + cluster1 { >> + core0 { >> + cpu = <&CPU4>; >> + }; >> + >> + core1 { >> + cpu = <&CPU5>; >> + }; >> + >> + core2 { >> + cpu = <&CPU6>; >> + }; >> + >> + core3 { >> + cpu = <&CPU7>; >> + }; >> + }; >> + }; > > From what I recall, this layout causes the kernel to spew > warnings? I mean to say this is the power/performance view, but > not the architectural view. > >> + }; >> + >> + timer { >> + compatible = "arm,armv8-timer"; >> + interrupts = <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > > Are we supposed to use the GIC_CPU_MASK_SIMPLE macros still? > >> + <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 0 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; >> + }; >> + >> + clocks { >> + xo_board: xo_board { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <19200000>; >> + clock-output-names = "xo_board"; > > We can drop clock-output-names on these. > >> + }; >> + >> + sleep_clk: sleep_clk { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <32764>; >> + clock-output-names = "sleep_clk"; >> + }; >> + }; >> + >> + psci { >> + compatible = "arm,psci-1.0"; >> + method = "smc"; >> + }; >> + >> + soc: soc { > > Will anyone use this phandle? > >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges = <0 0 0 0xffffffff>; >> + compatible = "simple-bus"; >> + >> + intc: interrupt-controller at 17a00000 { >> + compatible = "arm,gic-v3"; >> + #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>; > > Can you also add the ITS node please and mark it as disabled? > I'll send a patch to the list to skip status = "disabled" ones. > We may want to support ITS on these SoCs if the firmware is > different. > >> + }; >> + >> + gcc: clock-controller at 100000 { >> + compatible = "qcom,gcc-sdm845"; >> + reg = <0x100000 0x1f0000>; >> + #clock-cells = <1>; >> + #reset-cells = <1>; >> + }; >> + >> + tlmm: pinctrl at 03400000 { > > Drop leading zeroes please. Build dtbs with W=2 and fix the warnings so reviewers don't have to waste their time on these issues. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-02-06 20:26 ` Rob Herring @ 2018-02-07 4:14 ` Rajendra Nayak 0 siblings, 0 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-02-07 4:14 UTC (permalink / raw) To: linux-arm-kernel [].. >>> + }; >>> + >>> + gcc: clock-controller at 100000 { >>> + compatible = "qcom,gcc-sdm845"; >>> + reg = <0x100000 0x1f0000>; >>> + #clock-cells = <1>; >>> + #reset-cells = <1>; >>> + }; >>> + >>> + tlmm: pinctrl at 03400000 { >> >> Drop leading zeroes please. > > Build dtbs with W=2 and fix the warnings so reviewers don't have to > waste their time on these issues. Noted. Thanks Rob. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak 2018-01-26 20:31 ` Evan Green 2018-01-26 22:15 ` Stephen Boyd @ 2018-02-06 18:54 ` Bjorn Andersson 2018-02-07 4:15 ` Rajendra Nayak 2018-02-06 20:31 ` Rob Herring 3 siblings, 1 reply; 25+ messages in thread From: Bjorn Andersson @ 2018-02-06 18:54 UTC (permalink / raw) To: linux-arm-kernel On Thu 25 Jan 08:32 PST 2018, Rajendra Nayak wrote: > + spmi_bus: qcom,spmi at c440000 { [..] > + }; > + While we have the chance, please remove this empty line. > + }; > +}; Regards, Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-02-06 18:54 ` Bjorn Andersson @ 2018-02-07 4:15 ` Rajendra Nayak 0 siblings, 0 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-02-07 4:15 UTC (permalink / raw) To: linux-arm-kernel On 02/07/2018 12:24 AM, Bjorn Andersson wrote: > On Thu 25 Jan 08:32 PST 2018, Rajendra Nayak wrote: >> + spmi_bus: qcom,spmi at c440000 { > [..] >> + }; >> + > > While we have the chance, please remove this empty line. Will do. Thanks for the review. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak ` (2 preceding siblings ...) 2018-02-06 18:54 ` Bjorn Andersson @ 2018-02-06 20:31 ` Rob Herring 2018-02-07 4:47 ` Rajendra Nayak 3 siblings, 1 reply; 25+ messages in thread From: Rob Herring @ 2018-02-06 20:31 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jan 25, 2018 at 10:32 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-mtp.dtsi | 11 ++ > arch/arm64/boot/dts/qcom/sdm845.dtsi | 308 +++++++++++++++++++++++++++++++ > 4 files changed, 333 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts > create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi > 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..c57701bed084 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..95e41e42bee1 > --- /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-mtp.dtsi" > + > +/ { > + model = "Qualcomm Technologies, Inc. SDM845 MTP"; > + compatible = "qcom,sdm845-mtp"; > +}; > diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi b/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi > new file mode 100644 > index 000000000000..5b1022c20bad > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi > @@ -0,0 +1,11 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + */ > + > +#include "sdm845.dtsi" > + > +/ { > + soc { > + }; > +}; > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > new file mode 100644 > index 000000000000..a21f4912b3e2 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -0,0 +1,308 @@ > +// 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"; This should only be in the board level file. > + > + interrupt-parent = <&intc>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + > + chosen { }; > + > + memory { > + device_type = "memory"; > + /* We expect the bootloader to fill in the reg */ The start address is variable? If not you should populate the base and have a unit-address. > + reg = <0 0 0 0>; > + }; > + > + cpus { > + #address-cells = <2>; > + #size-cells = <0>; > + > + CPU0: cpu at 0 { > + device_type = "cpu"; > + compatible = "qcom,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + 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,kryo"; > + reg = <0x0 0x700>; > + enable-method = "psci"; > + next-level-cache = <&L2_700>; > + L2_700: l2-cache { > + compatible = "cache"; > + next-level-cache = <&L3_0>; > + }; > + }; > + > + cpu-map { > + cluster0 { > + core0 { > + cpu = <&CPU0>; > + }; > + > + core1 { > + cpu = <&CPU1>; > + }; > + > + core2 { > + cpu = <&CPU2>; > + }; > + > + core3 { > + cpu = <&CPU3>; > + }; > + }; > + > + cluster1 { > + core0 { > + cpu = <&CPU4>; > + }; > + > + core1 { > + cpu = <&CPU5>; > + }; > + > + core2 { > + cpu = <&CPU6>; > + }; > + > + core3 { > + cpu = <&CPU7>; > + }; > + }; > + }; > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 0 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>; > + }; > + > + clocks { > + xo_board: xo_board { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <19200000>; > + clock-output-names = "xo_board"; > + }; > + > + sleep_clk: sleep_clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32764>; > + clock-output-names = "sleep_clk"; > + }; > + }; > + > + 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"; > + #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>; > + }; > + > + gcc: clock-controller at 100000 { > + compatible = "qcom,gcc-sdm845"; sdm845-gcc is the preferred order. > + reg = <0x100000 0x1f0000>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + }; > + > + tlmm: pinctrl at 03400000 { > + 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>; > + clock-frequency = <19200000>; > + > + 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: qcom,spmi at c440000 { spmi at ... > + 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 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-02-06 20:31 ` Rob Herring @ 2018-02-07 4:47 ` Rajendra Nayak 2018-02-07 17:37 ` Rob Herring 0 siblings, 1 reply; 25+ messages in thread From: Rajendra Nayak @ 2018-02-07 4:47 UTC (permalink / raw) To: linux-arm-kernel [].. >> + >> +#include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> +/ { >> + model = "Qualcomm Technologies, Inc. SDM845"; > > This should only be in the board level file. thanks, will fix. > >> + >> + interrupt-parent = <&intc>; >> + >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + chosen { }; >> + >> + memory { >> + device_type = "memory"; >> + /* We expect the bootloader to fill in the reg */ > > The start address is variable? If not you should populate the base and > have a unit-address. sure, I'll check and update. > >> + reg = <0 0 0 0>; >> + }; >> + [].. >> + >> + 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"; >> + #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>; >> + }; >> + >> + gcc: clock-controller at 100000 { >> + compatible = "qcom,gcc-sdm845"; > > sdm845-gcc is the preferred order. This is still proposed as part of the GCC patch for sdm845 [1] (which looks like has neither you nor the DT list copied :/ ) Also looking at Documentation/devicetree/bindings/clock/qcom,gcc.txt, I see we seem to follow the gcc-<soc> convention for compatible all along :( "qcom,gcc-apq8064" "qcom,gcc-apq8084" "qcom,gcc-ipq8064" "qcom,gcc-ipq4019" "qcom,gcc-ipq8074" "qcom,gcc-msm8660" "qcom,gcc-msm8916" "qcom,gcc-msm8960" "qcom,gcc-msm8974" "qcom,gcc-msm8974pro" "qcom,gcc-msm8974pro-ac" "qcom,gcc-msm8994" "qcom,gcc-msm8996" "qcom,gcc-mdm9615" [1] https://patchwork.kernel.org/patch/10193895/ -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-02-07 4:47 ` Rajendra Nayak @ 2018-02-07 17:37 ` Rob Herring 0 siblings, 0 replies; 25+ messages in thread From: Rob Herring @ 2018-02-07 17:37 UTC (permalink / raw) To: linux-arm-kernel On Tue, Feb 6, 2018 at 10:47 PM, Rajendra Nayak <rnayak@codeaurora.org> wrote: > [].. > >>> + >>> +#include <dt-bindings/interrupt-controller/arm-gic.h> >>> + >>> +/ { >>> + model = "Qualcomm Technologies, Inc. SDM845"; >> >> This should only be in the board level file. > > thanks, will fix. > >> >>> + >>> + interrupt-parent = <&intc>; >>> + >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + chosen { }; >>> + >>> + memory { >>> + device_type = "memory"; >>> + /* We expect the bootloader to fill in the reg */ >> >> The start address is variable? If not you should populate the base and >> have a unit-address. > > sure, I'll check and update. > >> >>> + reg = <0 0 0 0>; >>> + }; >>> + > > [].. >>> + >>> + 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"; >>> + #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>; >>> + }; >>> + >>> + gcc: clock-controller at 100000 { >>> + compatible = "qcom,gcc-sdm845"; >> >> sdm845-gcc is the preferred order. > > This is still proposed as part of the GCC patch for sdm845 [1] > (which looks like has neither you nor the DT list copied :/ ) > Also looking at Documentation/devicetree/bindings/clock/qcom,gcc.txt, > I see we seem to follow the gcc-<soc> convention for compatible all along :( > > "qcom,gcc-apq8064" > "qcom,gcc-apq8084" > "qcom,gcc-ipq8064" > "qcom,gcc-ipq4019" > "qcom,gcc-ipq8074" > "qcom,gcc-msm8660" > "qcom,gcc-msm8916" > "qcom,gcc-msm8960" > "qcom,gcc-msm8974" > "qcom,gcc-msm8974pro" > "qcom,gcc-msm8974pro-ac" > "qcom,gcc-msm8994" > "qcom,gcc-msm8996" > "qcom,gcc-mdm9615" Okay, I guess the pattern for this is pretty much established. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-01-25 16:32 [PATCH 0/2] Add DTS for SDM845 SoC and MTP Rajendra Nayak 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak @ 2018-01-25 16:32 ` Rajendra Nayak 2018-01-26 22:18 ` Stephen Boyd 1 sibling, 1 reply; 25+ messages in thread From: Rajendra Nayak @ 2018-01-25 16:32 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> --- This patch is based on the current proposed DT bindings for the geni based serial driver [1] and also depends on the GCC driver [2] which adds dt-bindings/clock/qcom,gcc-sdm845.h header. This can only be merged once the dependent patches do. [1] https://patchwork.ozlabs.org/cover/860251/ [2] https://lkml.org/lkml/2018/1/22/78 arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi | 3 +++ arch/arm64/boot/dts/qcom/sdm845-pins.dtsi | 32 +++++++++++++++++++++++++++++++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 22 +++++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/sdm845-pins.dtsi diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi b/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi index 5b1022c20bad..640a48cd628b 100644 --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dtsi @@ -7,5 +7,8 @@ / { soc { + serial at a84000 { + status = "okay"; + }; }; }; diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi new file mode 100644 index 000000000000..b97f99e6f4b4 --- /dev/null +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +&tlmm { + 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; + }; + }; +}; diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index a21f4912b3e2..529f4ba3a1db 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> / { model = "Qualcomm Technologies, Inc. SDM845"; @@ -304,5 +305,26 @@ 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"; + }; }; }; +#include "sdm845-pins.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 related [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-01-25 16:32 ` [PATCH 2/2] arm64: dts: sdm845: Add serial console support Rajendra Nayak @ 2018-01-26 22:18 ` Stephen Boyd 2018-01-29 8:18 ` Rajendra Nayak ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Stephen Boyd @ 2018-01-26 22:18 UTC (permalink / raw) To: linux-arm-kernel On 01/25, Rajendra Nayak wrote: > diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > new file mode 100644 > index 000000000000..b97f99e6f4b4 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > @@ -0,0 +1,32 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + */ > + > +&tlmm { I'm not the maintainer, but I find this approach to the pins really annoying. I have to flip to another file to figure out how a board has configured the pins. And we may bring in a bunch of settings that we don't ever use on some board too. Why can't we put the settings in the board file directly? > + 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; > + }; > + }; > +}; > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index a21f4912b3e2..529f4ba3a1db 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> > > / { > model = "Qualcomm Technologies, Inc. SDM845"; I'd expect some sort of serial alias node stuff here too. > @@ -304,5 +305,26 @@ > 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>; Is this binding still being reviewed? Ugly. > + status = "disabled"; > + }; > }; > }; > +#include "sdm845-pins.dtsi" Why at the bottom? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-01-26 22:18 ` Stephen Boyd @ 2018-01-29 8:18 ` Rajendra Nayak 2018-02-06 19:00 ` Bjorn Andersson 2018-02-06 18:37 ` Doug Anderson 2018-02-06 20:36 ` Rob Herring 2 siblings, 1 reply; 25+ messages in thread From: Rajendra Nayak @ 2018-01-29 8:18 UTC (permalink / raw) To: linux-arm-kernel On 01/27/2018 03:48 AM, Stephen Boyd wrote: > On 01/25, Rajendra Nayak wrote: >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> new file mode 100644 >> index 000000000000..b97f99e6f4b4 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> @@ -0,0 +1,32 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> + */ >> + >> +&tlmm { > > I'm not the maintainer, but I find this approach to the pins > really annoying. I have to flip to another file to figure out how > a board has configured the pins. And we may bring in a bunch of > settings that we don't ever use on some board too. Why can't we > put the settings in the board file directly? I was just keeping it consistent with how things are for other qualcomm platforms. I can move this to the board dts if no one else sees any issues. > >> + 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; >> + }; >> + }; >> +}; >> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> index a21f4912b3e2..529f4ba3a1db 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> >> >> / { >> model = "Qualcomm Technologies, Inc. SDM845"; > > I'd expect some sort of serial alias node stuff here too. yes, will add. > >> @@ -304,5 +305,26 @@ >> 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>; > > Is this binding still being reviewed? Ugly. yes, its still being reviewed > >> + status = "disabled"; >> + }; >> }; >> }; >> +#include "sdm845-pins.dtsi" > > Why at the bottom? Again keeping it consistent with how things are in msm8916/msm8992/msm8996 dtsi files. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-01-29 8:18 ` Rajendra Nayak @ 2018-02-06 19:00 ` Bjorn Andersson 0 siblings, 0 replies; 25+ messages in thread From: Bjorn Andersson @ 2018-02-06 19:00 UTC (permalink / raw) To: linux-arm-kernel On Mon 29 Jan 00:18 PST 2018, Rajendra Nayak wrote: > On 01/27/2018 03:48 AM, Stephen Boyd wrote: > > On 01/25, Rajendra Nayak wrote: > >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > >> new file mode 100644 > >> index 000000000000..b97f99e6f4b4 > >> --- /dev/null > >> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > >> @@ -0,0 +1,32 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > >> + */ > >> + > >> +&tlmm { > > > > I'm not the maintainer, but I find this approach to the pins > > really annoying. I have to flip to another file to figure out how > > a board has configured the pins. And we may bring in a bunch of > > settings that we don't ever use on some board too. Why can't we > > put the settings in the board file directly? > > I was just keeping it consistent with how things are for other > qualcomm platforms. I can move this to the board dts if no one else > sees any issues. > What we decided recently for 8916 (at least) was that we define the functional properties in the soc dtsi and add the electrical properties in the board dts(i). I fully agree with Stephen on this one, but as you say we're keeping the pinconf in separate files for the other platforms/boards. I'll prepare and bring some new guidelines to our Thursday meeting and we can decide what to do based on that. Regards, Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-01-26 22:18 ` Stephen Boyd 2018-01-29 8:18 ` Rajendra Nayak @ 2018-02-06 18:37 ` Doug Anderson 2018-02-06 19:06 ` Bjorn Andersson 2018-02-06 20:36 ` Rob Herring 2 siblings, 1 reply; 25+ messages in thread From: Doug Anderson @ 2018-02-06 18:37 UTC (permalink / raw) To: linux-arm-kernel Hi, On Fri, Jan 26, 2018 at 2:18 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 01/25, Rajendra Nayak wrote: >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> new file mode 100644 >> index 000000000000..b97f99e6f4b4 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> @@ -0,0 +1,32 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> + */ >> + >> +&tlmm { > > I'm not the maintainer, but I find this approach to the pins > really annoying. I have to flip to another file to figure out how > a board has configured the pins. And we may bring in a bunch of > settings that we don't ever use on some board too. Why can't we > put the settings in the board file directly? I'm not so familiar with how things work with Qualcomm, but in general I think putting this in the "board" file is a bad idea. I'd be OK with putting this directly in the SoC file (though it might get unwieldy?), but not moving things to the board file as was done with v2 of this patch. Said another way: nearly board that uses SDM845 that uses UART2 will have the same definitions for these pins so we shouldn't be duplicating it across every board, right? I'll also respond to the v2 patch so it's obvious there is feedback there... -Doug ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-02-06 18:37 ` Doug Anderson @ 2018-02-06 19:06 ` Bjorn Andersson 2018-02-06 19:49 ` Doug Anderson 0 siblings, 1 reply; 25+ messages in thread From: Bjorn Andersson @ 2018-02-06 19:06 UTC (permalink / raw) To: linux-arm-kernel On Tue 06 Feb 10:37 PST 2018, Doug Anderson wrote: > Hi, > > On Fri, Jan 26, 2018 at 2:18 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > > On 01/25, Rajendra Nayak wrote: > >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > >> new file mode 100644 > >> index 000000000000..b97f99e6f4b4 > >> --- /dev/null > >> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > >> @@ -0,0 +1,32 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > >> + */ > >> + > >> +&tlmm { > > > > I'm not the maintainer, but I find this approach to the pins > > really annoying. I have to flip to another file to figure out how > > a board has configured the pins. And we may bring in a bunch of > > settings that we don't ever use on some board too. Why can't we > > put the settings in the board file directly? > > I'm not so familiar with how things work with Qualcomm, but in general > I think putting this in the "board" file is a bad idea. I'd be OK > with putting this directly in the SoC file (though it might get > unwieldy?), but not moving things to the board file as was done with > v2 of this patch. > > Said another way: nearly board that uses SDM845 that uses UART2 will > have the same definitions for these pins so we shouldn't be > duplicating it across every board, right? > We've run into several cases where different boards uses the same function but requires board specific electrical configuration. So what we decided was to keep the pinmux in the soc-file (where e.g. the uart definition is) and then extend it with the board specific electrical properties (the pinconf), in the board files. This does come with the complexity of having the pinctrl nodes split in two places, but the responsibilities of the two parts is clear and we remove the need for all board files to ensure the appropriate pinmux is in place. NB. We did discuss adding "sane defaults" for the pinconf in the soc dtsi, but we end up spending considerable time debugging issues stemming from not having the right pinconf; so better make this explicit and say that the board has to specify it's config. Regards, Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-02-06 19:06 ` Bjorn Andersson @ 2018-02-06 19:49 ` Doug Anderson 2018-02-06 20:05 ` Bjorn Andersson 2018-02-07 4:12 ` Rajendra Nayak 0 siblings, 2 replies; 25+ messages in thread From: Doug Anderson @ 2018-02-06 19:49 UTC (permalink / raw) To: linux-arm-kernel Hi, On Tue, Feb 6, 2018 at 11:06 AM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > On Tue 06 Feb 10:37 PST 2018, Doug Anderson wrote: > >> Hi, >> >> On Fri, Jan 26, 2018 at 2:18 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: >> > On 01/25, Rajendra Nayak wrote: >> >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> >> new file mode 100644 >> >> index 000000000000..b97f99e6f4b4 >> >> --- /dev/null >> >> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> >> @@ -0,0 +1,32 @@ >> >> +// SPDX-License-Identifier: GPL-2.0 >> >> +/* >> >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> >> + */ >> >> + >> >> +&tlmm { >> > >> > I'm not the maintainer, but I find this approach to the pins >> > really annoying. I have to flip to another file to figure out how >> > a board has configured the pins. And we may bring in a bunch of >> > settings that we don't ever use on some board too. Why can't we >> > put the settings in the board file directly? >> >> I'm not so familiar with how things work with Qualcomm, but in general >> I think putting this in the "board" file is a bad idea. I'd be OK >> with putting this directly in the SoC file (though it might get >> unwieldy?), but not moving things to the board file as was done with >> v2 of this patch. >> >> Said another way: nearly board that uses SDM845 that uses UART2 will >> have the same definitions for these pins so we shouldn't be >> duplicating it across every board, right? >> > > We've run into several cases where different boards uses the same > function but requires board specific electrical configuration. > > So what we decided was to keep the pinmux in the soc-file (where e.g. > the uart definition is) and then extend it with the board specific > electrical properties (the pinconf), in the board files. > > This does come with the complexity of having the pinctrl nodes split in > two places, but the responsibilities of the two parts is clear and we > remove the need for all board files to ensure the appropriate pinmux is > in place. > > > NB. We did discuss adding "sane defaults" for the pinconf in the soc > dtsi, but we end up spending considerable time debugging issues stemming > from not having the right pinconf; so better make this explicit and say > that the board has to specify it's config. Whoops, saw your responses _after_ I sent my response to v2. In any case this makes sense to me then! On Rockchip boards I've been involved in we often added "sane defaults", but I can see how that could be confusing in different ways. I'm happy with your choice and it seems like a happy medium. The sdm845.dtsi file can have the main definition of the nodes and can thus refer to the nodes. Then you just add the extra bit in the board file. What you propose is not what happened in v2 of the series <https://patchwork.kernel.org/patch/10194201/> though. In v2 _both_ the pinconf and the pinmux moved to the board file. That's wrong. To make it concrete, you'd have something like this (this has the wrong bindings from the UART, but folks get the picture hopefully): In sdm845.dtsi: 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"; }; 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>; qup_uart2_default: qup_uart2_default { pinmux { function = "qup9"; pins = "gpio4", "gpio5"; }; }; qup_uart2_sleep: qup_uart2_sleep { pinmux { function = "gpio"; pins = "gpio4", "gpio5"; }; }; }; In sdm845-mtp.dts: &qup_uart2_default { pinconf { pins = "gpio4", "gpio5"; drive-strength = <2>; bias-disable; }; }; &qup_uart2_sleep { pinconf { pins = "gpio4", "gpio5"; drive-strength = <2>; bias-disable; }; }; ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-02-06 19:49 ` Doug Anderson @ 2018-02-06 20:05 ` Bjorn Andersson 2018-02-07 4:12 ` Rajendra Nayak 1 sibling, 0 replies; 25+ messages in thread From: Bjorn Andersson @ 2018-02-06 20:05 UTC (permalink / raw) To: linux-arm-kernel On Tue 06 Feb 11:49 PST 2018, Doug Anderson wrote: > Hi, > > On Tue, Feb 6, 2018 at 11:06 AM, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > On Tue 06 Feb 10:37 PST 2018, Doug Anderson wrote: > > > >> Hi, > >> > >> On Fri, Jan 26, 2018 at 2:18 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > >> > On 01/25, Rajendra Nayak wrote: > >> >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > >> >> new file mode 100644 > >> >> index 000000000000..b97f99e6f4b4 > >> >> --- /dev/null > >> >> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi > >> >> @@ -0,0 +1,32 @@ > >> >> +// SPDX-License-Identifier: GPL-2.0 > >> >> +/* > >> >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > >> >> + */ > >> >> + > >> >> +&tlmm { > >> > > >> > I'm not the maintainer, but I find this approach to the pins > >> > really annoying. I have to flip to another file to figure out how > >> > a board has configured the pins. And we may bring in a bunch of > >> > settings that we don't ever use on some board too. Why can't we > >> > put the settings in the board file directly? > >> > >> I'm not so familiar with how things work with Qualcomm, but in general > >> I think putting this in the "board" file is a bad idea. I'd be OK > >> with putting this directly in the SoC file (though it might get > >> unwieldy?), but not moving things to the board file as was done with > >> v2 of this patch. > >> > >> Said another way: nearly board that uses SDM845 that uses UART2 will > >> have the same definitions for these pins so we shouldn't be > >> duplicating it across every board, right? > >> > > > > We've run into several cases where different boards uses the same > > function but requires board specific electrical configuration. > > > > So what we decided was to keep the pinmux in the soc-file (where e.g. > > the uart definition is) and then extend it with the board specific > > electrical properties (the pinconf), in the board files. > > > > This does come with the complexity of having the pinctrl nodes split in > > two places, but the responsibilities of the two parts is clear and we > > remove the need for all board files to ensure the appropriate pinmux is > > in place. > > > > > > NB. We did discuss adding "sane defaults" for the pinconf in the soc > > dtsi, but we end up spending considerable time debugging issues stemming > > from not having the right pinconf; so better make this explicit and say > > that the board has to specify it's config. > > Whoops, saw your responses _after_ I sent my response to v2. In any > case this makes sense to me then! On Rockchip boards I've been > involved in we often added "sane defaults", but I can see how that > could be confusing in different ways. I'm happy with your choice and > it seems like a happy medium. The sdm845.dtsi file can have the main > definition of the nodes and can thus refer to the nodes. Then you > just add the extra bit in the board file. > > What you propose is not what happened in v2 of the series > <https://patchwork.kernel.org/patch/10194201/> though. In v2 _both_ > the pinconf and the pinmux moved to the board file. That's wrong. > > > To make it concrete, you'd have something like this (this has the > wrong bindings from the UART, but folks get the picture hopefully): > > > In sdm845.dtsi: > > 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"; > }; > > 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>; > > qup_uart2_default: qup_uart2_default { > pinmux { > function = "qup9"; > pins = "gpio4", "gpio5"; > }; > }; > > qup_uart2_sleep: qup_uart2_sleep { > pinmux { > function = "gpio"; > pins = "gpio4", "gpio5"; > }; > }; > }; > > In sdm845-mtp.dts: > > &qup_uart2_default { > pinconf { > pins = "gpio4", "gpio5"; > drive-strength = <2>; > bias-disable; > }; > }; > > &qup_uart2_sleep { > pinconf { > pins = "gpio4", "gpio5"; > drive-strength = <2>; > bias-disable; > }; > }; Correct. This example does however show another thing that I really do not like; When you have a lot of nodes I find it very useful to maintain some sort of grouping, to know that I can find a node describing properties related to some block close to related blocks - e.g. nodes describing a pmic block is close to other nodes for that pmic. Today we seem to have a mixture of bus-based grouping, arbitrary grouping and no grouping at all in our upstream dtsi files, so I think we should set some guidelines here as well. Regards, Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-02-06 19:49 ` Doug Anderson 2018-02-06 20:05 ` Bjorn Andersson @ 2018-02-07 4:12 ` Rajendra Nayak 1 sibling, 0 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-02-07 4:12 UTC (permalink / raw) To: linux-arm-kernel On 02/07/2018 01:19 AM, Doug Anderson wrote: > Hi, > > On Tue, Feb 6, 2018 at 11:06 AM, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: >> On Tue 06 Feb 10:37 PST 2018, Doug Anderson wrote: >> >>> Hi, >>> >>> On Fri, Jan 26, 2018 at 2:18 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: >>>> On 01/25, Rajendra Nayak wrote: >>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >>>>> new file mode 100644 >>>>> index 000000000000..b97f99e6f4b4 >>>>> --- /dev/null >>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >>>>> @@ -0,0 +1,32 @@ >>>>> +// SPDX-License-Identifier: GPL-2.0 >>>>> +/* >>>>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >>>>> + */ >>>>> + >>>>> +&tlmm { >>>> >>>> I'm not the maintainer, but I find this approach to the pins >>>> really annoying. I have to flip to another file to figure out how >>>> a board has configured the pins. And we may bring in a bunch of >>>> settings that we don't ever use on some board too. Why can't we >>>> put the settings in the board file directly? >>> >>> I'm not so familiar with how things work with Qualcomm, but in general >>> I think putting this in the "board" file is a bad idea. I'd be OK >>> with putting this directly in the SoC file (though it might get >>> unwieldy?), but not moving things to the board file as was done with >>> v2 of this patch. >>> >>> Said another way: nearly board that uses SDM845 that uses UART2 will >>> have the same definitions for these pins so we shouldn't be >>> duplicating it across every board, right? >>> >> >> We've run into several cases where different boards uses the same >> function but requires board specific electrical configuration. >> >> So what we decided was to keep the pinmux in the soc-file (where e.g. >> the uart definition is) and then extend it with the board specific >> electrical properties (the pinconf), in the board files. >> >> This does come with the complexity of having the pinctrl nodes split in >> two places, but the responsibilities of the two parts is clear and we >> remove the need for all board files to ensure the appropriate pinmux is >> in place. >> >> >> NB. We did discuss adding "sane defaults" for the pinconf in the soc >> dtsi, but we end up spending considerable time debugging issues stemming >> from not having the right pinconf; so better make this explicit and say >> that the board has to specify it's config. > > Whoops, saw your responses _after_ I sent my response to v2. In any > case this makes sense to me then! On Rockchip boards I've been > involved in we often added "sane defaults", but I can see how that > could be confusing in different ways. I'm happy with your choice and > it seems like a happy medium. The sdm845.dtsi file can have the main > definition of the nodes and can thus refer to the nodes. Then you > just add the extra bit in the board file. > > What you propose is not what happened in v2 of the series > <https://patchwork.kernel.org/patch/10194201/> though. In v2 _both_ > the pinconf and the pinmux moved to the board file. That's wrong. got it. I'll fix this up in my v3. Thanks for the review. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-01-26 22:18 ` Stephen Boyd 2018-01-29 8:18 ` Rajendra Nayak 2018-02-06 18:37 ` Doug Anderson @ 2018-02-06 20:36 ` Rob Herring 2 siblings, 0 replies; 25+ messages in thread From: Rob Herring @ 2018-02-06 20:36 UTC (permalink / raw) To: linux-arm-kernel On Fri, Jan 26, 2018 at 4:18 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 01/25, Rajendra Nayak wrote: >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> new file mode 100644 >> index 000000000000..b97f99e6f4b4 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/sdm845-pins.dtsi >> @@ -0,0 +1,32 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> + */ >> + >> +&tlmm { > > I'm not the maintainer, but I find this approach to the pins > really annoying. I have to flip to another file to figure out how > a board has configured the pins. And we may bring in a bunch of > settings that we don't ever use on some board too. Why can't we > put the settings in the board file directly? FYI, there's a pending dtc change to allow deleting unreferenced nodes which can solve the bloat problem. Rob ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2018-02-07 17:37 UTC | newest] Thread overview: 25+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-25 16:32 [PATCH 0/2] Add DTS for SDM845 SoC and MTP Rajendra Nayak 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak 2018-01-26 20:31 ` Evan Green 2018-01-30 8:48 ` Rajendra Nayak 2018-01-26 22:15 ` Stephen Boyd 2018-01-29 8:13 ` Rajendra Nayak 2018-01-30 9:48 ` Stephen Boyd 2018-01-30 10:25 ` Rajendra Nayak 2018-02-06 20:26 ` Rob Herring 2018-02-07 4:14 ` Rajendra Nayak 2018-02-06 18:54 ` Bjorn Andersson 2018-02-07 4:15 ` Rajendra Nayak 2018-02-06 20:31 ` Rob Herring 2018-02-07 4:47 ` Rajendra Nayak 2018-02-07 17:37 ` Rob Herring 2018-01-25 16:32 ` [PATCH 2/2] arm64: dts: sdm845: Add serial console support Rajendra Nayak 2018-01-26 22:18 ` Stephen Boyd 2018-01-29 8:18 ` Rajendra Nayak 2018-02-06 19:00 ` Bjorn Andersson 2018-02-06 18:37 ` Doug Anderson 2018-02-06 19:06 ` Bjorn Andersson 2018-02-06 19:49 ` Doug Anderson 2018-02-06 20:05 ` Bjorn Andersson 2018-02-07 4:12 ` Rajendra Nayak 2018-02-06 20:36 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).