| Message ID | 1440948270-1991-3-git-send-email-j@jannau.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2015-08-30 17:24:30 +0200, Janne Grunau wrote: > Creates apm-storm-883408.dtsi which should be shareable with the HP > Moonshot m400 cartridge. > > Signed-off-by: Janne Grunau <j@jannau.net> > --- > arch/arm64/boot/dts/apm/Makefile | 1 + > arch/arm64/boot/dts/apm/apm-storm-883408.dtsi | 98 +++++++++++++++++++++++++++ > arch/arm64/boot/dts/apm/mp30ar0.dts | 85 +++++++++++++++++++++++ > 3 files changed, 184 insertions(+) > create mode 100644 arch/arm64/boot/dts/apm/apm-storm-883408.dtsi > create mode 100644 arch/arm64/boot/dts/apm/mp30ar0.dts > > diff --git a/arch/arm64/boot/dts/apm/mp30ar0.dts b/arch/arm64/boot/dts/apm/mp30ar0.dts > new file mode 100644 > index 0000000..f7a9dae5 > --- /dev/null > +++ b/arch/arm64/boot/dts/apm/mp30ar0.dts > @@ -0,0 +1,85 @@ > +/* > + * dts file for Gigabyte MP30-AR0 board > + * > + * Copyright (C) 2013, Applied Micro Circuits Corporation > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + */ > + > +/dts-v1/; > + > +/include/ "apm-storm-883408.dtsi" > +/include/ "apm-storm-soc.dtsi" /include/ "apm-storm.dtsi" I split the soc node first into its own file but then decided that keeping the larger part in the same file introduces less merge conflicts. > + > +/ { > + pmu { > + compatible = "arm,armv8-pmuv3"; > + interrupts = <0x1 0xc 0xff04>; > + }; > + > + memory { > + #address-cells = <0x2>; > + #size-cells = <0x2>; > + device_type = "memory"; > + reg = <0x0 0x0 0x0 0x2000000>; > + }; > + > + poweroff_mbox: poweroff_mbox@10548000 { > + compatible = "syscon"; > + reg = <0x0 0x10548000 0x0 0x100>; > + }; > + > + poweroff@10548010 { > + compatible = "syscon-poweroff"; > + regmap = <&poweroff_mbox>; > + offset = <0x10>; > + mask = <0x1>; > + }; This doesn't seem to work, even with the poweroff/reset patches for mustang. Janne
On Sun, Aug 30, 2015 at 04:24:30PM +0100, Janne Grunau wrote: > Creates apm-storm-883408.dtsi which should be shareable with the HP > Moonshot m400 cartridge. > > Signed-off-by: Janne Grunau <j@jannau.net> > --- > arch/arm64/boot/dts/apm/Makefile | 1 + > arch/arm64/boot/dts/apm/apm-storm-883408.dtsi | 98 +++++++++++++++++++++++++++ > arch/arm64/boot/dts/apm/mp30ar0.dts | 85 +++++++++++++++++++++++ > 3 files changed, 184 insertions(+) > create mode 100644 arch/arm64/boot/dts/apm/apm-storm-883408.dtsi > create mode 100644 arch/arm64/boot/dts/apm/mp30ar0.dts > > diff --git a/arch/arm64/boot/dts/apm/Makefile b/arch/arm64/boot/dts/apm/Makefile > index a2afabb..b0ec2b6 100644 > --- a/arch/arm64/boot/dts/apm/Makefile > +++ b/arch/arm64/boot/dts/apm/Makefile > @@ -1,4 +1,5 @@ > dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb > +dtb-$(CONFIG_ARCH_XGENE) += mp30ar0.dtb > > always := $(dtb-y) > subdir-y := $(dts-dirs) > diff --git a/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi b/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi > new file mode 100644 > index 0000000..83116df > --- /dev/null > +++ b/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi > @@ -0,0 +1,98 @@ > +/* > + * dts file for AppliedMicro (APM) X-Gene Storm SOC APM883408 > + * > + * Copyright (C) 2013, Applied Micro Circuits Corporation > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + */ > + > +/ { > + compatible = "apm,xgene-storm"; > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + cpus { > + #address-cells = <2>; > + #size-cells = <0>; > + > + cpu@000 { > + device_type = "cpu"; > + compatible = "apm,potenza", "arm,armv8"; > + reg = <0x0 0x000>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x40 0x0000fff8>; > + }; This release address doesn't fall in the region described by the memory node, and I don't see any /memreserve/. That worries me. Does the bootlaoder override the memory node? Does it add the requisite memreserve? Does it override the cpu-release-addr? [...] > + gic: interrupt-controller@78090000 { > + compatible = "arm,cortex-a15-gic"; > + #interrupt-cells = <3>; > + interrupt-controller; > + reg = <0x0 0x78090000 0x0 0x10000>, /* GIC Dist */ > + <0x0 0x780a0000 0x0 0x10000>, /* GIC CPU */ > + <0x0 0x780c0000 0x0 0x20000>, /* GIC VCPU Control */ > + <0x0 0x780e0000 0x0 0x20000>; /* GIC VCPU */ > + interrupts = <1 9 0xf04>; > + }; Nit: add a blank line here. > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <1 0 0xff04>, /* Secure Phys IRQ */ > + <1 13 0xff04>, /* Non-secure Phys IRQ */ > + <1 14 0xff04>, /* Virt IRQ */ > + <1 15 0xff04>; /* Hyp IRQ */ > + clock-frequency = <50000000>; > + }; Surely the firmware is configuring CNTFRQ, and it's not necessary to have a clock-frequency property? > +}; > diff --git a/arch/arm64/boot/dts/apm/mp30ar0.dts b/arch/arm64/boot/dts/apm/mp30ar0.dts > new file mode 100644 > index 0000000..f7a9dae5 > --- /dev/null > +++ b/arch/arm64/boot/dts/apm/mp30ar0.dts > @@ -0,0 +1,85 @@ > +/* > + * dts file for Gigabyte MP30-AR0 board > + * > + * Copyright (C) 2013, Applied Micro Circuits Corporation > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + */ > + > +/dts-v1/; > + > +/include/ "apm-storm-883408.dtsi" > +/include/ "apm-storm-soc.dtsi" > + > +/ { > + pmu { > + compatible = "arm,armv8-pmuv3"; > + interrupts = <0x1 0xc 0xff04>; > + }; > + > + memory { > + #address-cells = <0x2>; > + #size-cells = <0x2>; I don't see why these properties should be here. > + device_type = "memory"; > + reg = <0x0 0x0 0x0 0x2000000>; Is this overriden by the bootloader? It seems a tad small... > + }; > + > + poweroff_mbox: poweroff_mbox@10548000 { > + compatible = "syscon"; > + reg = <0x0 0x10548000 0x0 0x100>; > + }; > + > + poweroff@10548010 { > + compatible = "syscon-poweroff"; > + regmap = <&poweroff_mbox>; > + offset = <0x10>; > + mask = <0x1>; > + }; > + > + chosen { > + linux,stdout-path = "/soc/serial@1c020000"; Please use stdout-path rather than linux,stdout-path. No rate configuration? This would look nicer with an alias. Thanks, Mark.
On 2015-09-01 11:55:11 +0100, Mark Rutland wrote: > On Sun, Aug 30, 2015 at 04:24:30PM +0100, Janne Grunau wrote: > > Creates apm-storm-883408.dtsi which should be shareable with the HP > > Moonshot m400 cartridge. > > > > Signed-off-by: Janne Grunau <j@jannau.net> > > --- > > arch/arm64/boot/dts/apm/Makefile | 1 + > > arch/arm64/boot/dts/apm/apm-storm-883408.dtsi | 98 +++++++++++++++++++++++++++ > > arch/arm64/boot/dts/apm/mp30ar0.dts | 85 +++++++++++++++++++++++ > > 3 files changed, 184 insertions(+) > > create mode 100644 arch/arm64/boot/dts/apm/apm-storm-883408.dtsi > > create mode 100644 arch/arm64/boot/dts/apm/mp30ar0.dts > > > > diff --git a/arch/arm64/boot/dts/apm/Makefile b/arch/arm64/boot/dts/apm/Makefile > > index a2afabb..b0ec2b6 100644 > > --- a/arch/arm64/boot/dts/apm/Makefile > > +++ b/arch/arm64/boot/dts/apm/Makefile > > @@ -1,4 +1,5 @@ > > dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb > > +dtb-$(CONFIG_ARCH_XGENE) += mp30ar0.dtb > > > > always := $(dtb-y) > > subdir-y := $(dts-dirs) > > diff --git a/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi b/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi > > new file mode 100644 > > index 0000000..83116df > > --- /dev/null > > +++ b/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi > > @@ -0,0 +1,98 @@ > > +/* > > + * dts file for AppliedMicro (APM) X-Gene Storm SOC APM883408 > > + * > > + * Copyright (C) 2013, Applied Micro Circuits Corporation > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation; either version 2 of > > + * the License, or (at your option) any later version. > > + */ > > + > > +/ { > > + compatible = "apm,xgene-storm"; > > + interrupt-parent = <&gic>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + cpus { > > + #address-cells = <2>; > > + #size-cells = <0>; > > + > > + cpu@000 { > > + device_type = "cpu"; > > + compatible = "apm,potenza", "arm,armv8"; > > + reg = <0x0 0x000>; > > + enable-method = "spin-table"; > > + cpu-release-addr = <0x40 0x0000fff8>; > > + }; > > This release address doesn't fall in the region described by the memory > node, and I don't see any /memreserve/. That worries me. > > Does the bootlaoder override the memory node? The bootloader overrides the memory node and memory starts at 0x40 0x0 > Does it add the requisite memreserve? > > Does it override the cpu-release-addr? neither of that > > + timer { > > + compatible = "arm,armv8-timer"; > > + interrupts = <1 0 0xff04>, /* Secure Phys IRQ */ > > + <1 13 0xff04>, /* Non-secure Phys IRQ */ > > + <1 14 0xff04>, /* Virt IRQ */ > > + <1 15 0xff04>; /* Hyp IRQ */ > > + clock-frequency = <50000000>; > > + }; > > Surely the firmware is configuring CNTFRQ, and it's not necessary to > have a clock-frequency property? it does, clock-frequency removed > > +}; > > diff --git a/arch/arm64/boot/dts/apm/mp30ar0.dts b/arch/arm64/boot/dts/apm/mp30ar0.dts > > new file mode 100644 > > index 0000000..f7a9dae5 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/apm/mp30ar0.dts [...] > > + memory { > > + #address-cells = <0x2>; > > + #size-cells = <0x2>; > > I don't see why these properties should be here. removed > > + device_type = "memory"; > > + reg = <0x0 0x0 0x0 0x2000000>; > > Is this overriden by the bootloader? It seems a tad small... yes, it is overriden by the bootloader. It requires a memory node in the dts but will overwrite the contents. I'll set size to zero and add a comment that it is updated by the bootloader. I could set start to 0x40 0x00 or leave it at zero. > > + }; > > + > > + poweroff_mbox: poweroff_mbox@10548000 { > > + compatible = "syscon"; > > + reg = <0x0 0x10548000 0x0 0x100>; > > + }; > > + > > + poweroff@10548010 { > > + compatible = "syscon-poweroff"; > > + regmap = <&poweroff_mbox>; > > + offset = <0x10>; > > + mask = <0x1>; > > + }; > > + > > + chosen { > > + linux,stdout-path = "/soc/serial@1c020000"; > > Please use stdout-path rather than linux,stdout-path. > > No rate configuration? > > This would look nicer with an alias. I can change that but the bootloader will add a 'linux,stdout-path' and the stdout-path will override it. OTOH the bootloader seems to always add this exact string so it will be fine to "override" it from the dts. The alternative would to remove it. Thanks, Janne > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > > +}; > > > diff --git a/arch/arm64/boot/dts/apm/mp30ar0.dts b/arch/arm64/boot/dts/apm/mp30ar0.dts > > > new file mode 100644 > > > index 0000000..f7a9dae5 > > > --- /dev/null > > > +++ b/arch/arm64/boot/dts/apm/mp30ar0.dts > > [...] > > > > + memory { > > > + #address-cells = <0x2>; > > > + #size-cells = <0x2>; > > > > I don't see why these properties should be here. > > removed > > > > + device_type = "memory"; > > > + reg = <0x0 0x0 0x0 0x2000000>; > > > > Is this overriden by the bootloader? It seems a tad small... > > yes, it is overriden by the bootloader. It requires a memory node in the > dts but will overwrite the contents. I'll set size to zero and add a > comment that it is updated by the bootloader. I could set start to 0x40 > 0x00 or leave it at zero. I think all zeroes is preferable. The comment is definitely worthwhile. > > > + }; > > > + > > > + poweroff_mbox: poweroff_mbox@10548000 { > > > + compatible = "syscon"; > > > + reg = <0x0 0x10548000 0x0 0x100>; > > > + }; > > > + > > > + poweroff@10548010 { > > > + compatible = "syscon-poweroff"; > > > + regmap = <&poweroff_mbox>; > > > + offset = <0x10>; > > > + mask = <0x1>; > > > + }; > > > + > > > + chosen { > > > + linux,stdout-path = "/soc/serial@1c020000"; > > > > Please use stdout-path rather than linux,stdout-path. > > > > No rate configuration? > > > > This would look nicer with an alias. > > I can change that but the bootloader will add a 'linux,stdout-path' and > the stdout-path will override it. OTOH the bootloader seems to always > add this exact string so it will be fine to "override" it from the dts. > The alternative would to remove it. I guess it doesn't fill in the rate, which is somewhat unfortunate. Does it override an existing linux,stdout-path? If not, we could add the rate. I think we should have a comment about the bootlaoder filling in linux,stdout-path, but other than that I'm not really sure what the best option is. Thanks, Mark.
On 2015-09-02 14:57:53 +0100, Mark Rutland wrote: > > > > +}; > > > > diff --git a/arch/arm64/boot/dts/apm/mp30ar0.dts b/arch/arm64/boot/dts/apm/mp30ar0.dts > > > > new file mode 100644 > > > > index 0000000..f7a9dae5 > > > > --- /dev/null > > > > +++ b/arch/arm64/boot/dts/apm/mp30ar0.dts [...] > > > > + }; > > > > + > > > > + poweroff_mbox: poweroff_mbox@10548000 { > > > > + compatible = "syscon"; > > > > + reg = <0x0 0x10548000 0x0 0x100>; > > > > + }; > > > > + > > > > + poweroff@10548010 { > > > > + compatible = "syscon-poweroff"; > > > > + regmap = <&poweroff_mbox>; > > > > + offset = <0x10>; > > > > + mask = <0x1>; > > > > + }; > > > > + > > > > + chosen { > > > > + linux,stdout-path = "/soc/serial@1c020000"; > > > > > > Please use stdout-path rather than linux,stdout-path. > > > > > > No rate configuration? > > > > > > This would look nicer with an alias. > > > > I can change that but the bootloader will add a 'linux,stdout-path' and > > the stdout-path will override it. OTOH the bootloader seems to always > > add this exact string so it will be fine to "override" it from the dts. > > The alternative would to remove it. > > I guess it doesn't fill in the rate, which is somewhat unfortunate. > > Does it override an existing linux,stdout-path? If not, we could add the > rate. yes, it overrides an existing linux,stdout-path and it looks like an hardcoded string. > I think we should have a comment about the bootlaoder filling in > linux,stdout-path, but other than that I'm not really sure what the best > option is. I'll add the comment and a stdout-path with alias and rate. There is at least some hope that Gigabyte or APM will realease a newer/fixed bootloader. The installed bootloader doesn't clear x2/x3. Thanks Janne
diff --git a/arch/arm64/boot/dts/apm/Makefile b/arch/arm64/boot/dts/apm/Makefile index a2afabb..b0ec2b6 100644 --- a/arch/arm64/boot/dts/apm/Makefile +++ b/arch/arm64/boot/dts/apm/Makefile @@ -1,4 +1,5 @@ dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb +dtb-$(CONFIG_ARCH_XGENE) += mp30ar0.dtb always := $(dtb-y) subdir-y := $(dts-dirs) diff --git a/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi b/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi new file mode 100644 index 0000000..83116df --- /dev/null +++ b/arch/arm64/boot/dts/apm/apm-storm-883408.dtsi @@ -0,0 +1,98 @@ +/* + * dts file for AppliedMicro (APM) X-Gene Storm SOC APM883408 + * + * Copyright (C) 2013, Applied Micro Circuits Corporation + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +/ { + compatible = "apm,xgene-storm"; + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu@000 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x000>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + cpu@001 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x001>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + cpu@100 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x100>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + cpu@101 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x101>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + cpu@200 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x200>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + cpu@201 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x201>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + cpu@300 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x300>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + cpu@301 { + device_type = "cpu"; + compatible = "apm,potenza", "arm,armv8"; + reg = <0x0 0x301>; + enable-method = "spin-table"; + cpu-release-addr = <0x40 0x0000fff8>; + }; + }; + + gic: interrupt-controller@78090000 { + compatible = "arm,cortex-a15-gic"; + #interrupt-cells = <3>; + interrupt-controller; + reg = <0x0 0x78090000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x780a0000 0x0 0x10000>, /* GIC CPU */ + <0x0 0x780c0000 0x0 0x20000>, /* GIC VCPU Control */ + <0x0 0x780e0000 0x0 0x20000>; /* GIC VCPU */ + interrupts = <1 9 0xf04>; + }; + timer { + compatible = "arm,armv8-timer"; + interrupts = <1 0 0xff04>, /* Secure Phys IRQ */ + <1 13 0xff04>, /* Non-secure Phys IRQ */ + <1 14 0xff04>, /* Virt IRQ */ + <1 15 0xff04>; /* Hyp IRQ */ + clock-frequency = <50000000>; + }; +}; diff --git a/arch/arm64/boot/dts/apm/mp30ar0.dts b/arch/arm64/boot/dts/apm/mp30ar0.dts new file mode 100644 index 0000000..f7a9dae5 --- /dev/null +++ b/arch/arm64/boot/dts/apm/mp30ar0.dts @@ -0,0 +1,85 @@ +/* + * dts file for Gigabyte MP30-AR0 board + * + * Copyright (C) 2013, Applied Micro Circuits Corporation + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +/dts-v1/; + +/include/ "apm-storm-883408.dtsi" +/include/ "apm-storm-soc.dtsi" + +/ { + pmu { + compatible = "arm,armv8-pmuv3"; + interrupts = <0x1 0xc 0xff04>; + }; + + memory { + #address-cells = <0x2>; + #size-cells = <0x2>; + device_type = "memory"; + reg = <0x0 0x0 0x0 0x2000000>; + }; + + poweroff_mbox: poweroff_mbox@10548000 { + compatible = "syscon"; + reg = <0x0 0x10548000 0x0 0x100>; + }; + + poweroff@10548010 { + compatible = "syscon-poweroff"; + regmap = <&poweroff_mbox>; + offset = <0x10>; + mask = <0x1>; + }; + + chosen { + linux,stdout-path = "/soc/serial@1c020000"; + }; +}; + +&pcie0clk { + status = "ok"; +}; + +&pcie2clk { + status = "ok"; +}; + +&pcie3clk { + status = "ok"; +}; + +&pcie0 { + status = "ok"; +}; + +&pcie2 { + status = "ok"; +}; + +&pcie3 { + status = "ok"; +}; + +&serial0 { + status = "ok"; +}; + +&sgenet0 { + status = "ok"; +}; + +&sgenet1 { + status = "ok"; +}; + +&xgenet { + status = "ok"; +};
Creates apm-storm-883408.dtsi which should be shareable with the HP Moonshot m400 cartridge. Signed-off-by: Janne Grunau <j@jannau.net> --- arch/arm64/boot/dts/apm/Makefile | 1 + arch/arm64/boot/dts/apm/apm-storm-883408.dtsi | 98 +++++++++++++++++++++++++++ arch/arm64/boot/dts/apm/mp30ar0.dts | 85 +++++++++++++++++++++++ 3 files changed, 184 insertions(+) create mode 100644 arch/arm64/boot/dts/apm/apm-storm-883408.dtsi create mode 100644 arch/arm64/boot/dts/apm/mp30ar0.dts