From: "Andreas Färber" <afaerber@suse.de>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, mp-cs@actions-semi.com,
info@ucrobotics.com, linux-kernel@vger.kernel.org,
Rob Herring <robh+dt@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
devicetree@vger.kernel.org, nd@arm.com
Subject: Re: [PATCH 11/11] ARM64: dts: Prepare Actions Semi S900 and Bubblegum-96
Date: Wed, 15 Feb 2017 19:14:14 +0100 [thread overview]
Message-ID: <267b586e-06ce-f123-b7cc-dc0feb880ca9@suse.de> (raw)
In-Reply-To: <20170215171232.GI31733@leverpostej>
Hi,
Am 15.02.2017 um 18:12 schrieb Mark Rutland:
> On Wed, Feb 15, 2017 at 05:55:28PM +0100, Andreas Färber wrote:
>> +#include "s900.dtsi"
>> +
>> +/ {
>> + compatible = "ucrobotics,bubblegum-96", "acts,s900";
>> + model = "Bubblegum-96";
>> +
>> + aliases {
>> + serial5 = &uart5;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial5:115200n8";
>> + };
>> +};
>
> I didn't spot a memory node here or in the dtsi. Does the FW/bootloader
> create one?
Heh, apparently... For S500, skeleton.dtsi added that - not entirely
clean either. I'll add one here in the .dts and think about adding one
in s500-guitar.dtsi as well.
--- a/arch/arm64/boot/dts/actions/s900-bubblegum96.dts
+++ b/arch/arm64/boot/dts/actions/s900-bubblegum96.dts
@@ -55,6 +55,11 @@
chosen {
stdout-path = "serial5:115200n8";
};
+
+ memory {
+ device_type = "memory";
+ reg = <0x0 0x0 0x0 0x80000000>;
+ };
};
&uart5 {
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> + };
>
> Why have this empty node?
Forgotten about... I see these:
reserving fdt memory region: addr=19000000 size=3000000
reserving fdt memory region: addr=20000000 size=20000000
reserving fdt memory region: addr=18fff000 size=1000
reserving fdt memory region: addr=1f000000 size=1000000
Vendor tree says 0x1f000000 is for secure monitor; 0x19000000 for fb,
0x20000000 for ion (ignoring), and 0x20000000 for afinfo - whatever that
may be (not me! :)).
So as minimum:
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -86,6 +86,11 @@
#address-cells = <2>;
#size-cells = <2>;
ranges;
+
+ secmon@1f000000 {
+ reg = <0x0 0x1f000000 0x0 0x1000000>;
+ no-map;
+ };
};
psci {
>> +
>> + psci {
>> + compatible = "arm,psci-0.2";
>> + method = "smc";
>> + };
>> +
>> + arm-pmu {
>> + compatible = "arm,cortex-a53-pmu";
>> + interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>
> Please ad an interrupt-affinity property, as described in
> Documentation/devicetree/bindings/arm/pmu.txt.
I had done my best to check my properties matched the bindings, but you
caught me not checking for any missing properties.
Again, my guess is:
@@ -99,6 +104,7 @@
<GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
};
timer {
>> +
>> + timer {
>> + compatible = "arm,armv8-timer";
>> + interrupts = <GIC_PPI 13
>> + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 14
>> + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 11
>> + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 10
>> + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>> + };
>> +
>> + soc {
>> + compatible = "simple-bus";
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> +
>> + gic: interrupt-controller@e00f1000 {
>> + compatible = "arm,gic-400";
>> + reg = <0x0 0xe00f1000 0x0 0x1000>,
>> + <0x0 0xe00f2000 0x0 0x1000>,
>> + <0x0 0xe00f4000 0x0 0x2000>,
>> + <0x0 0xe00f6000 0x0 0x2000>;
>
> I believe that the second entry should be 0x2000 in length.
The vendor tree does have 0x1000, but that might be a mistake of course.
I vaguely recall having such discussions elsewhere in the past.
@@ -122,7 +128,7 @@
gic: interrupt-controller@e00f1000 {
compatible = "arm,gic-400";
reg = <0x0 0xe00f1000 0x0 0x1000>,
- <0x0 0xe00f2000 0x0 0x1000>,
+ <0x0 0xe00f2000 0x0 0x2000>,
<0x0 0xe00f4000 0x0 0x2000>,
<0x0 0xe00f6000 0x0 0x2000>;
interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4)
| IRQ_TYPE_LEVEL_LOW)>;
Thanks for the quick review,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
next prev parent reply other threads:[~2017-02-15 18:14 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-15 16:55 [PATCH 00/11] ARM: Initial Actions Semi S500 and S900 enablement Andreas Färber
2017-02-15 16:55 ` [PATCH 01/11] Documentation: devicetree: Add vendor prefix for Actions Semi Andreas Färber
2017-02-15 16:55 ` [PATCH 02/11] Documentation: devicetree: arm: Document Actions Semi S500 Andreas Färber
2017-02-15 16:55 ` [PATCH 03/11] ARM: Prepare " Andreas Färber
2017-02-15 16:55 ` [PATCH 04/11] ARM64: Prepare Actions Semi S900 Andreas Färber
2017-02-16 13:43 ` Arnd Bergmann
2017-02-17 0:34 ` Andreas Färber
2017-02-17 11:32 ` Arnd Bergmann
2017-02-15 16:55 ` [PATCH 05/11] Documentation: devicetree: serial: Document Actions Semi Owl UARTs Andreas Färber
2017-02-15 16:55 ` [PATCH 06/11] tty: serial: Add Actions Semi Owl UART earlycon Andreas Färber
2017-02-16 13:41 ` Arnd Bergmann
2017-02-20 13:40 ` Andreas Färber
2017-02-20 15:17 ` Arnd Bergmann
2017-02-15 16:55 ` [PATCH 07/11] Documentation: kernel-parameters: Document owl earlycon Andreas Färber
2017-02-15 16:55 ` [PATCH 08/11] ARM: dts: Prepare Actions Semi S500 and LeMaker Guitar Andreas Färber
2017-02-15 17:07 ` Mark Rutland
2017-02-15 17:28 ` Andreas Färber
2017-02-15 17:36 ` Mark Rutland
2017-02-24 0:59 ` Andreas Färber
2017-02-15 16:55 ` [PATCH 09/11] Documentation: devicetree: Add vendor prefix for uCRobotics Andreas Färber
2017-02-15 16:55 ` [PATCH 10/11] Documentation: devicetree: arm: Document Actions Semi S900 Andreas Färber
2017-02-27 14:26 ` Rob Herring
2017-02-15 16:55 ` [PATCH 11/11] ARM64: dts: Prepare Actions Semi S900 and Bubblegum-96 Andreas Färber
2017-02-15 17:12 ` Mark Rutland
2017-02-15 18:14 ` Andreas Färber [this message]
2017-02-15 18:23 ` Mark Rutland
2017-02-16 13:46 ` [PATCH 00/11] ARM: Initial Actions Semi S500 and S900 enablement Arnd Bergmann
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=267b586e-06ce-f123-b7cc-dc0feb880ca9@suse.de \
--to=afaerber@suse.de \
--cc=catalin.marinas@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=info@ucrobotics.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mp-cs@actions-semi.com \
--cc=nd@arm.com \
--cc=robh+dt@kernel.org \
--cc=will.deacon@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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).