From: Kumar Gala <galak@codeaurora.org> To: Mark Rutland <mark.rutland@arm.com> Cc: "linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "arm@kernel.org" <arm@kernel.org>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "heiko@sntech.de" <heiko@sntech.de> Subject: Re: [PATCH v3 2/4] arm64: dts: Add Qualcomm MSM8916 SoC and evaluation board dts Date: Thu, 12 Mar 2015 12:33:22 -0500 [thread overview] Message-ID: <E114AF0E-5DA9-412A-B3E2-DDD95D224C66@codeaurora.org> (raw) In-Reply-To: <20150312170541.GE30145@leverpostej> On Mar 12, 2015, at 12:05 PM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi Kumar, > >> +/ { >> + model = "Qualcomm Technologies, Inc. MSM 8916 MTP"; >> + compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360", >> + "qcom,msm8916", "qcom,mtp"; >> +}; > > No /chosen/stdout-path? Nope ;). > > Does your UART driver support earlycon? It does. > > [...] > >> + cpus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + CPU0: cpu@0 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x0>; >> + }; >> + >> + CPU1: cpu@1 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x1>; >> + }; >> + >> + CPU2: cpu@2 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x2>; >> + }; >> + >> + CPU3: cpu@3 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x3>; >> + }; >> + }; > > The secondary CPUs need an enable-method. Are you using PSCI or > spin-table? This is on purpose. We aren’t using either PSCI or spin-table. Right now the dts is for booting on a single core. I can drop CPU1..CPU3 if that helps. > Which exception level do the CPUs enter the kernel? > >> + timer { >> + compatible = "arm,armv7-timer"; > > This should be "arm,armv8-timer”. will change > >> + interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 4 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; >> + clock-frequency = <19200000>; >> + }; > > NAK. CNTFRQ should be programmed on all CPUs prior to entering the > kernel, per the boot protocol. You should not need clock-frequency here. Will drop clock-frequency. > [...] > >> + intc: interrupt-controller@b000000 { >> + compatible = "qcom,msm-qgic2"; > > This string isn't documented (but seems to be supported by the GIC > driver). There’s a patch posted to add ‘qcom,msm-qgic2’ to the binding doc. > How does this differ from other GIC implementations? Not sure the exact details, just that its qcom’s on implementation of the GIC spec. > >> + interrupt-controller; >> + #interrupt-cells = <3>; >> + reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>; >> + }; > > No GICH, GICV, maintenance interrupt? Nope. > > Minor nit, but I'd prefer if the reg entries were on individual lines as > happens in other dts. > > Thanks, > Mark. -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
WARNING: multiple messages have this Message-ID (diff)
From: galak@codeaurora.org (Kumar Gala) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v3 2/4] arm64: dts: Add Qualcomm MSM8916 SoC and evaluation board dts Date: Thu, 12 Mar 2015 12:33:22 -0500 [thread overview] Message-ID: <E114AF0E-5DA9-412A-B3E2-DDD95D224C66@codeaurora.org> (raw) In-Reply-To: <20150312170541.GE30145@leverpostej> On Mar 12, 2015, at 12:05 PM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi Kumar, > >> +/ { >> + model = "Qualcomm Technologies, Inc. MSM 8916 MTP"; >> + compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360", >> + "qcom,msm8916", "qcom,mtp"; >> +}; > > No /chosen/stdout-path? Nope ;). > > Does your UART driver support earlycon? It does. > > [...] > >> + cpus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + CPU0: cpu at 0 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x0>; >> + }; >> + >> + CPU1: cpu at 1 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x1>; >> + }; >> + >> + CPU2: cpu at 2 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x2>; >> + }; >> + >> + CPU3: cpu at 3 { >> + device_type = "cpu"; >> + compatible = "arm,cortex-a53", "arm,armv8"; >> + reg = <0x3>; >> + }; >> + }; > > The secondary CPUs need an enable-method. Are you using PSCI or > spin-table? This is on purpose. We aren?t using either PSCI or spin-table. Right now the dts is for booting on a single core. I can drop CPU1..CPU3 if that helps. > Which exception level do the CPUs enter the kernel? > >> + timer { >> + compatible = "arm,armv7-timer"; > > This should be "arm,armv8-timer?. will change > >> + interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 3 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 4 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 1 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; >> + clock-frequency = <19200000>; >> + }; > > NAK. CNTFRQ should be programmed on all CPUs prior to entering the > kernel, per the boot protocol. You should not need clock-frequency here. Will drop clock-frequency. > [...] > >> + intc: interrupt-controller at b000000 { >> + compatible = "qcom,msm-qgic2"; > > This string isn't documented (but seems to be supported by the GIC > driver). There?s a patch posted to add ?qcom,msm-qgic2? to the binding doc. > How does this differ from other GIC implementations? Not sure the exact details, just that its qcom?s on implementation of the GIC spec. > >> + interrupt-controller; >> + #interrupt-cells = <3>; >> + reg = <0x0b000000 0x1000>, <0x0b002000 0x1000>; >> + }; > > No GICH, GICV, maintenance interrupt? Nope. > > Minor nit, but I'd prefer if the reg entries were on individual lines as > happens in other dts. > > Thanks, > Mark. -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
next prev parent reply other threads:[~2015-03-12 17:33 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-03-11 20:51 [PATCH v3 1/4] arm64: qcom: Add support for Qualcomm MSM8916 SoC Kumar Gala 2015-03-11 20:51 ` Kumar Gala 2015-03-11 20:51 ` [PATCH v3 2/4] arm64: dts: Add Qualcomm MSM8916 SoC and evaluation board dts Kumar Gala 2015-03-11 20:51 ` Kumar Gala 2015-03-12 17:05 ` Mark Rutland 2015-03-12 17:05 ` Mark Rutland 2015-03-12 17:05 ` Mark Rutland 2015-03-12 17:33 ` Kumar Gala [this message] 2015-03-12 17:33 ` Kumar Gala 2015-03-12 17:33 ` Kumar Gala 2015-03-12 18:25 ` Mark Rutland 2015-03-12 18:25 ` Mark Rutland 2015-03-12 18:25 ` Mark Rutland 2015-03-12 19:54 ` Kumar Gala 2015-03-12 19:54 ` Kumar Gala 2015-03-12 19:54 ` Kumar Gala [not found] ` <F0B0524B-AC0E-4E60-9A21-1208C4A978F5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2015-03-13 10:34 ` Mark Rutland 2015-03-13 10:34 ` Mark Rutland 2015-03-13 10:34 ` Mark Rutland 2015-03-13 12:07 ` Catalin Marinas 2015-03-13 12:07 ` Catalin Marinas 2015-03-13 12:07 ` Catalin Marinas 2015-03-11 20:51 ` [PATCH v3 3/4] devicetree: bindings: Document qcom,msm-id and qcom,board-id Kumar Gala 2015-03-11 20:51 ` [PATCH v3 3/4] devicetree: bindings: Document qcom, msm-id and qcom, board-id Kumar Gala 2015-03-11 20:51 ` [PATCH v3 4/4] arm64: dts: Add Qualcomm MSM8916 & MTP8916 ids Kumar Gala 2015-03-11 20:51 ` Kumar Gala
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=E114AF0E-5DA9-412A-B3E2-DDD95D224C66@codeaurora.org \ --to=galak@codeaurora.org \ --cc=arm@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=heiko@sntech.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.