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