* [PATCH 0/2] Add DTS for SDM845 SoC and MTP @ 2018-01-25 16:32 Rajendra Nayak [not found] ` <20180125163216.29018-1-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 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: andy.gross Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree, Rajendra Nayak 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
[parent not found: <20180125163216.29018-1-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP [not found] ` <20180125163216.29018-1-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2018-01-25 16:32 ` Rajendra Nayak [not found] ` <20180125163216.29018-2-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Rajendra Nayak @ 2018-01-25 16:32 UTC (permalink / raw) To: andy.gross-QSEj5FYQhm4dnm+yROfE0A Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, Rajendra Nayak Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files Signed-off-by: Rajendra Nayak <rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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@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@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@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@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@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@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@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@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@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@100000 { + compatible = "qcom,gcc-sdm845"; + reg = <0x100000 0x1f0000>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + + tlmm: pinctrl@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@17C90000 { + #address-cells = <1>; + #size-cells = <1>; + ranges; + compatible = "arm,armv7-timer-mem"; + reg = <0x17C90000 0x1000>; + clock-frequency = <19200000>; + + 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: qcom,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 -- 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 related [flat|nested] 25+ messages in thread
[parent not found: <20180125163216.29018-2-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP [not found] ` <20180125163216.29018-2-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2018-01-26 20:31 ` Evan Green 2018-01-30 8:48 ` Rajendra Nayak 2018-01-26 22:15 ` Stephen Boyd 1 sibling, 1 reply; 25+ messages in thread From: Evan Green @ 2018-01-26 20:31 UTC (permalink / raw) To: Rajendra Nayak Cc: Andy Gross, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA Hi Rajendra, On Thu, Jan 25, 2018 at 8:32 AM, Rajendra Nayak <rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote: > Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files > > Signed-off-by: Rajendra Nayak <rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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 -- 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] 25+ messages in thread
* Re: [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: Evan Green Cc: Andy Gross, linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree 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
* Re: [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP [not found] ` <20180125163216.29018-2-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2018-01-26 20:31 ` Evan Green @ 2018-01-26 22:15 ` Stephen Boyd [not found] ` <20180126221501.GD28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2018-02-06 20:26 ` Rob Herring 1 sibling, 2 replies; 25+ messages in thread From: Stephen Boyd @ 2018-01-26 22:15 UTC (permalink / raw) To: Rajendra Nayak Cc: andy.gross-QSEj5FYQhm4dnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA 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@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@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@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@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@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@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@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@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@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@100000 { > + compatible = "qcom,gcc-sdm845"; > + reg = <0x100000 0x1f0000>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + }; > + > + tlmm: pinctrl@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@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@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 -- 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] 25+ messages in thread
[parent not found: <20180126221501.GD28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP [not found] ` <20180126221501.GD28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2018-01-29 8:13 ` Rajendra Nayak 2018-01-30 9:48 ` Stephen Boyd 0 siblings, 1 reply; 25+ messages in thread From: Rajendra Nayak @ 2018-01-29 8:13 UTC (permalink / raw) To: Stephen Boyd Cc: andy.gross-QSEj5FYQhm4dnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA 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@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@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@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@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@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@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@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@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@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@100000 { >> + compatible = "qcom,gcc-sdm845"; >> + reg = <0x100000 0x1f0000>; >> + #clock-cells = <1>; >> + #reset-cells = <1>; >> + }; >> + >> + tlmm: pinctrl@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@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@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 -- 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] 25+ messages in thread
* Re: [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: Rajendra Nayak Cc: andy.gross, linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree 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
* Re: [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: Stephen Boyd Cc: andy.gross, linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree 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
* Re: [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP 2018-01-26 22:15 ` Stephen Boyd [not found] ` <20180126221501.GD28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2018-02-06 20:26 ` Rob Herring [not found] ` <CAL_JsqJ1xjQ5ZP-KXeZQ0s=ib8GTfvfYjFqyy+Zcub-akCs7Bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 25+ messages in thread From: Rob Herring @ 2018-02-06 20:26 UTC (permalink / raw) To: Stephen Boyd, Rajendra Nayak Cc: Andy Gross, linux-kernel, linux-arm-msm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, devicetree 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@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@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@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@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@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@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@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@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@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@100000 { >> + compatible = "qcom,gcc-sdm845"; >> + reg = <0x100000 0x1f0000>; >> + #clock-cells = <1>; >> + #reset-cells = <1>; >> + }; >> + >> + tlmm: pinctrl@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
[parent not found: <CAL_JsqJ1xjQ5ZP-KXeZQ0s=ib8GTfvfYjFqyy+Zcub-akCs7Bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP [not found] ` <CAL_JsqJ1xjQ5ZP-KXeZQ0s=ib8GTfvfYjFqyy+Zcub-akCs7Bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 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: Rob Herring, Stephen Boyd Cc: Andy Gross, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, devicetree-u79uwXL29TY76Z2rM5mHXA [].. >>> + }; >>> + >>> + gcc: clock-controller@100000 { >>> + compatible = "qcom,gcc-sdm845"; >>> + reg = <0x100000 0x1f0000>; >>> + #clock-cells = <1>; >>> + #reset-cells = <1>; >>> + }; >>> + >>> + tlmm: pinctrl@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 -- 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] 25+ messages in thread
* Re: [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 [not found] ` <20180125163216.29018-2-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2018-02-06 18:54 ` Bjorn Andersson 2018-02-07 4:15 ` Rajendra Nayak 2018-02-06 20:31 ` Rob Herring 2 siblings, 1 reply; 25+ messages in thread From: Bjorn Andersson @ 2018-02-06 18:54 UTC (permalink / raw) To: Rajendra Nayak Cc: andy.gross, linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree On Thu 25 Jan 08:32 PST 2018, Rajendra Nayak wrote: > + spmi_bus: qcom,spmi@c440000 { [..] > + }; > + While we have the chance, please remove this empty line. > + }; > +}; Regards, Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [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: Bjorn Andersson Cc: andy.gross-QSEj5FYQhm4dnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA 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@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 -- 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] 25+ messages in thread
* Re: [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 [not found] ` <20180125163216.29018-2-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2018-02-06 18:54 ` Bjorn Andersson @ 2018-02-06 20:31 ` Rob Herring 2018-02-07 4:47 ` Rajendra Nayak 2 siblings, 1 reply; 25+ messages in thread From: Rob Herring @ 2018-02-06 20:31 UTC (permalink / raw) To: Rajendra Nayak Cc: Andy Gross, linux-kernel, linux-arm-msm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, devicetree 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@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@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@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@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@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@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@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@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@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@100000 { > + compatible = "qcom,gcc-sdm845"; sdm845-gcc is the preferred order. > + reg = <0x100000 0x1f0000>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + }; > + > + tlmm: pinctrl@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@17C90000 { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + compatible = "arm,armv7-timer-mem"; > + reg = <0x17C90000 0x1000>; > + clock-frequency = <19200000>; > + > + 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: qcom,spmi@c440000 { spmi@... > + 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [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: Rob Herring Cc: Andy Gross, linux-arm-msm, linux-kernel, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, devicetree [].. >> + >> +#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@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@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
* Re: [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: Rajendra Nayak Cc: Andy Gross, linux-kernel, linux-arm-msm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, devicetree 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@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@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 [not found] ` <20180125163216.29018-1-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 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: andy.gross Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree, 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> --- 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@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@ac0000 { + compatible = "qcom,geni-se-qup"; + reg = <0xac0000 0x6000>; + }; + + qup_uart2: serial@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
* Re: [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: Rajendra Nayak Cc: andy.gross, linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree 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@ac0000 { > + compatible = "qcom,geni-se-qup"; > + reg = <0xac0000 0x6000>; > + }; > + > + qup_uart2: serial@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
* Re: [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 [not found] ` <20180126221808.GE28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2 siblings, 1 reply; 25+ messages in thread From: Rajendra Nayak @ 2018-01-29 8:18 UTC (permalink / raw) To: Stephen Boyd Cc: andy.gross, linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree 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@ac0000 { >> + compatible = "qcom,geni-se-qup"; >> + reg = <0xac0000 0x6000>; >> + }; >> + >> + qup_uart2: serial@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
* Re: [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: Rajendra Nayak Cc: Stephen Boyd, andy.gross, linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree 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
* Re: [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 [not found] ` <CAD=FV=WcCnQJc25=sKWtOi=ZWi=ium6DVsexuQnsLDL=aJE6-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> [not found] ` <20180126221808.GE28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2 siblings, 1 reply; 25+ messages in thread From: Doug Anderson @ 2018-02-06 18:37 UTC (permalink / raw) To: Stephen Boyd Cc: Rajendra Nayak, Andy Gross, LKML, linux-arm-msm, Linux ARM, devicetree 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
[parent not found: <CAD=FV=WcCnQJc25=sKWtOi=ZWi=ium6DVsexuQnsLDL=aJE6-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/2] arm64: dts: sdm845: Add serial console support [not found] ` <CAD=FV=WcCnQJc25=sKWtOi=ZWi=ium6DVsexuQnsLDL=aJE6-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 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: Doug Anderson Cc: Stephen Boyd, Rajendra Nayak, Andy Gross, LKML, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Linux ARM, devicetree-u79uwXL29TY76Z2rM5mHXA On Tue 06 Feb 10:37 PST 2018, Doug Anderson wrote: > Hi, > > On Fri, Jan 26, 2018 at 2:18 PM, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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 -- 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] 25+ messages in thread
* Re: [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-02-06 19:06 ` Bjorn Andersson @ 2018-02-06 19:49 ` Doug Anderson [not found] ` <CAD=FV=Whxf1VRJ6qx9mioEEkTeS+uJfJKPbOMcGy-Wigh_NORw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 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: Bjorn Andersson Cc: Stephen Boyd, Rajendra Nayak, Andy Gross, LKML, linux-arm-msm, Linux ARM, devicetree 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@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@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
[parent not found: <CAD=FV=Whxf1VRJ6qx9mioEEkTeS+uJfJKPbOMcGy-Wigh_NORw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/2] arm64: dts: sdm845: Add serial console support [not found] ` <CAD=FV=Whxf1VRJ6qx9mioEEkTeS+uJfJKPbOMcGy-Wigh_NORw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2018-02-06 20:05 ` Bjorn Andersson 0 siblings, 0 replies; 25+ messages in thread From: Bjorn Andersson @ 2018-02-06 20:05 UTC (permalink / raw) To: Doug Anderson Cc: Stephen Boyd, Rajendra Nayak, Andy Gross, LKML, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Linux ARM, devicetree-u79uwXL29TY76Z2rM5mHXA 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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@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@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 -- 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] 25+ messages in thread
* Re: [PATCH 2/2] arm64: dts: sdm845: Add serial console support 2018-02-06 19:49 ` Doug Anderson [not found] ` <CAD=FV=Whxf1VRJ6qx9mioEEkTeS+uJfJKPbOMcGy-Wigh_NORw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 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: Doug Anderson, Bjorn Andersson Cc: Stephen Boyd, Andy Gross, LKML, linux-arm-msm, Linux ARM, devicetree 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
[parent not found: <20180126221808.GE28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH 2/2] arm64: dts: sdm845: Add serial console support [not found] ` <20180126221808.GE28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2018-02-06 20:36 ` Rob Herring 0 siblings, 0 replies; 25+ messages in thread From: Rob Herring @ 2018-02-06 20:36 UTC (permalink / raw) To: Stephen Boyd Cc: Rajendra Nayak, Andy Gross, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-msm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, devicetree-u79uwXL29TY76Z2rM5mHXA On Fri, Jan 26, 2018 at 4:18 PM, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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 -- 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] 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 [not found] ` <20180125163216.29018-1-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2018-01-25 16:32 ` [PATCH 1/2] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 " Rajendra Nayak [not found] ` <20180125163216.29018-2-rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2018-01-26 20:31 ` Evan Green 2018-01-30 8:48 ` Rajendra Nayak 2018-01-26 22:15 ` Stephen Boyd [not found] ` <20180126221501.GD28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 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 [not found] ` <CAL_JsqJ1xjQ5ZP-KXeZQ0s=ib8GTfvfYjFqyy+Zcub-akCs7Bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 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 [not found] ` <CAD=FV=WcCnQJc25=sKWtOi=ZWi=ium6DVsexuQnsLDL=aJE6-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-06 19:06 ` Bjorn Andersson 2018-02-06 19:49 ` Doug Anderson [not found] ` <CAD=FV=Whxf1VRJ6qx9mioEEkTeS+uJfJKPbOMcGy-Wigh_NORw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2018-02-06 20:05 ` Bjorn Andersson 2018-02-07 4:12 ` Rajendra Nayak [not found] ` <20180126221808.GE28313-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 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).