| Message ID | 1418208602-35584-4-git-send-email-eddie.huang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, 2014-12-10 at 18:50 +0800, Eddie Huang wrote: <...> > diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts > new file mode 100644 > index 0000000..adf26dd > --- /dev/null > +++ b/arch/arm64/boot/dts/mt8173-evb.dts <...> > + timer { > + compatible = "arm,armv8-timer"; > + interrupt-parent = <&gic>; > + interrupts = <1 13 0x8>, > + <1 14 0x8>, > + <1 11 0x8>, > + <1 10 0x8>; > + clock-frequency = <13000000>; I believe our firmware doesn't need this line. Please remove it. Joe.C
2014-12-10 15:27 GMT+01:00 Yingjoe Chen <yingjoe.chen@mediatek.com>: > > Hi, > > On Wed, 2014-12-10 at 18:50 +0800, Eddie Huang wrote: > <...> >> diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts >> new file mode 100644 >> index 0000000..adf26dd >> --- /dev/null >> +++ b/arch/arm64/boot/dts/mt8173-evb.dts > <...> >> + timer { >> + compatible = "arm,armv8-timer"; >> + interrupt-parent = <&gic>; >> + interrupts = <1 13 0x8>, >> + <1 14 0x8>, >> + <1 11 0x8>, >> + <1 10 0x8>; >> + clock-frequency = <13000000>; > > I believe our firmware doesn't need this line. Please remove it. The point here would be to know if you need to enable a special timer from the mtk-timer block to get the arch timer working. In any case, you will need some sort of timer. This dts does not describe the mtk-timer (may in the mt8173 it does not exist) but defines the clocks clk26m and clk32k. So if you don't use the mtk-timer, please remove the clocks as there isn't a block using them. Thanks, Matthias > > Joe.C > >
On Wed, 2014-12-10 at 15:50 +0100, Matthias Brugger wrote: > 2014-12-10 15:27 GMT+01:00 Yingjoe Chen <yingjoe.chen@mediatek.com>: > > > > Hi, > > > > On Wed, 2014-12-10 at 18:50 +0800, Eddie Huang wrote: > > <...> > >> diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts > >> new file mode 100644 > >> index 0000000..adf26dd > >> --- /dev/null > >> +++ b/arch/arm64/boot/dts/mt8173-evb.dts > > <...> > >> + timer { > >> + compatible = "arm,armv8-timer"; > >> + interrupt-parent = <&gic>; > >> + interrupts = <1 13 0x8>, > >> + <1 14 0x8>, > >> + <1 11 0x8>, > >> + <1 10 0x8>; > >> + clock-frequency = <13000000>; > > > > I believe our firmware doesn't need this line. Please remove it. > > The point here would be to know if you need to enable a special timer > from the mtk-timer block to get the arch timer working. > In any case, you will need some sort of timer. This dts does not > describe the mtk-timer (may in the mt8173 it does not exist) but > defines the clocks clk26m and clk32k. So if you don't use the > mtk-timer, please remove the clocks as there isn't a block using them. > MT8173 has two timer set: CPUGPT and APBGPT, and use CPUGPT to enable arch_timer. Previous series only have APBGPT. MT8173 still need enable CPUGPT to get arch timer working, we put this in loader, and transparent to kernel. So I will remove clk26m and clk32k in next version.
Hi Eddie, 2014-12-11 13:47 GMT+01:00 Eddie Huang <eddie.huang@mediatek.com>: > On Wed, 2014-12-10 at 15:50 +0100, Matthias Brugger wrote: >> 2014-12-10 15:27 GMT+01:00 Yingjoe Chen <yingjoe.chen@mediatek.com>: >> > >> > Hi, >> > >> > On Wed, 2014-12-10 at 18:50 +0800, Eddie Huang wrote: >> > <...> >> >> diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts >> >> new file mode 100644 >> >> index 0000000..adf26dd >> >> --- /dev/null >> >> +++ b/arch/arm64/boot/dts/mt8173-evb.dts >> > <...> >> >> + timer { >> >> + compatible = "arm,armv8-timer"; >> >> + interrupt-parent = <&gic>; >> >> + interrupts = <1 13 0x8>, >> >> + <1 14 0x8>, >> >> + <1 11 0x8>, >> >> + <1 10 0x8>; >> >> + clock-frequency = <13000000>; >> > >> > I believe our firmware doesn't need this line. Please remove it. >> >> The point here would be to know if you need to enable a special timer >> from the mtk-timer block to get the arch timer working. >> In any case, you will need some sort of timer. This dts does not >> describe the mtk-timer (may in the mt8173 it does not exist) but >> defines the clocks clk26m and clk32k. So if you don't use the >> mtk-timer, please remove the clocks as there isn't a block using them. >> > > MT8173 has two timer set: CPUGPT and APBGPT, and use CPUGPT to enable > arch_timer. Previous series only have APBGPT. MT8173 still need enable > CPUGPT to get arch timer working, we put this in loader, and transparent > to kernel. So I will remove clk26m and clk32k in next version. Ok, so if this is done in the bootloader, there shouldn't be a problem then. Perfect. Just one more comment on the dts nodes. Can you use the makros defined for the interrupt type e.g "GIC_PPI 14 IRQ_TYPE_LEVEL_LOW" instead of "1 14 0x8". This makes the dts easier to read. Thanks, Matthias >
Hi, On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > Add device tree support for MT8173 SoC and evalutaion board based on it. > > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com> > --- > arch/arm64/boot/dts/Makefile | 1 + > arch/arm64/boot/dts/mt8173-evb.dts | 31 +++++++ > arch/arm64/boot/dts/mt8173.dtsi | 164 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 196 insertions(+) > create mode 100644 arch/arm64/boot/dts/mt8173-evb.dts > create mode 100644 arch/arm64/boot/dts/mt8173.dtsi > > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile > index f8001a6..db7661e 100644 > --- a/arch/arm64/boot/dts/Makefile > +++ b/arch/arm64/boot/dts/Makefile > @@ -1,3 +1,4 @@ > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb > dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb > dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb > dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb > diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts > new file mode 100644 > index 0000000..adf26dd > --- /dev/null > +++ b/arch/arm64/boot/dts/mt8173-evb.dts > @@ -0,0 +1,31 @@ > +/* > + * Copyright (c) 2014 MediaTek Inc. > + * Author: Eddie Huang <eddie.huang@mediatek.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +/dts-v1/; > +#include "mt8173.dtsi" > + > +/ { > + model = "mediatek,mt8173-evb"; > + > + aliases { > + serial0 = &uart0; > + serial1 = &uart1; > + serial2 = &uart2; > + serial3 = &uart3; Do any of these support earlycon? > + }; > + > + memory { Nit: should be memory@40000000 (and you'll need to add device_type = "memory"). > + reg = <0 0x40000000 0 0x40000000>; > + }; > +}; > diff --git a/arch/arm64/boot/dts/mt8173.dtsi b/arch/arm64/boot/dts/mt8173.dtsi > new file mode 100644 > index 0000000..1286801 > --- /dev/null > +++ b/arch/arm64/boot/dts/mt8173.dtsi > @@ -0,0 +1,164 @@ > +/* > + * Copyright (c) 2014 MediaTek Inc. > + * Author: Eddie Huang <eddie.huang@mediatek.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include "skeleton.dtsi" > + > +/ { > + compatible = "mediatek,mt8173"; > + interrupt-parent = <&sysirq>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + cpu-map { This should live under /cpus, as documented in Documentation/devicetree/bindings/arm/topology.txt. > + cluster0 { > + core0 { > + cpu = <&cpu0>; > + }; > + core1 { > + cpu = <&cpu1>; > + }; > + }; > + > + cluster1 { > + core0 { > + cpu = <&cpu2>; > + }; > + core1 { > + cpu = <&cpu3>; > + }; > + }; > + }; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu0: cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x000>; > + enable-method = "psci"; > + }; > + > + cpu1: cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a53"; > + reg = <0x001>; > + enable-method = "psci"; > + }; > + > + cpu2: cpu@2 { > + device_type = "cpu"; > + compatible = "arm,cortex-a57"; > + reg = <0x100>; > + enable-method = "psci"; > + }; > + > + cpu3: cpu@3 { > + device_type = "cpu"; > + compatible = "arm,cortex-a57"; > + reg = <0x101>; > + enable-method = "psci"; > + }; > + }; > + > + psci { > + compatible = "arm,psci-0.2"; > + method = "smc"; > + }; What are you using as your PSCI 0.2 implementation? Is it fully compliant? (e.g. are the reset and power off functions implemented, may CPU0 be hotplugged)? Given only portions of the GIC seem to be described below, what exception level is your kernel entered at? Per the spec it should be EL2, but given the brokenness below with the GIC I'm suspicious. > + > + clocks { Please remove the clock container node. It serves no purpose whatsoever. Just put these clocks directly under the root. > + clk26m: clk26m { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <26000000>; > + }; > + > + clk32k: clk32k { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32000>; > + }; > + > + uart_clk: dummy26m { > + compatible = "fixed-clock"; > + clock-frequency = <26000000>; > + #clock-cells = <0>; > + }; > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupt-parent = <&gic>; > + interrupts = <1 13 0x8>, > + <1 14 0x8>, > + <1 11 0x8>, > + <1 10 0x8>; Shouldn't these have a non-zero cpu mask? > + clock-frequency = <13000000>; Your firmware should be programming CNTFREQ_EL0, so you shouldn't need this (PSCI 0.2 requires CNTFREQ_EL0 to be programmed correctly on all CPUs). > + }; > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + compatible = "simple-bus"; > + ranges; > + > + sysirq: intpol-controller@10200620 { > + compatible = "mediatek,mt8173-sysirq", "mediatek,mt6577-sysirq"; > + interrupt-controller; > + #interrupt-cells = <3>; > + interrupt-parent = <&gic>; > + reg = <0 0x10200620 0 0x20>; > + }; > + > + gic: interrupt-controller@10220000 { > + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; Surely this should be "arm,gic-400"? > + #interrupt-cells = <3>; > + interrupt-parent = <&gic>; > + interrupt-controller; > + reg = <0 0x10221000 0 0x1000>, > + <0 0x10222000 0 0x1000>, > + <0 0x10200620 0 0x1000>; You're missing GICV here, and that GICH address is fundamentally wrong (it _must_ be page aligned). The CPU interface (and virtual CPU interface) should be 0x2000 long. The GIC maintenance interrupt also seems to be missing. Thanks, Mark.
On Thu, Dec 11, 2014 at 06:02:46PM +0000, Mark Rutland wrote: > Hi, > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > > Add device tree support for MT8173 SoC and evalutaion board based on it. > > > > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com> > > --- > > arch/arm64/boot/dts/Makefile | 1 + > > arch/arm64/boot/dts/mt8173-evb.dts | 31 +++++++ > > arch/arm64/boot/dts/mt8173.dtsi | 164 +++++++++++++++++++++++++++++++++++++ > > 3 files changed, 196 insertions(+) > > create mode 100644 arch/arm64/boot/dts/mt8173-evb.dts > > create mode 100644 arch/arm64/boot/dts/mt8173.dtsi > > > > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile > > index f8001a6..db7661e 100644 > > --- a/arch/arm64/boot/dts/Makefile > > +++ b/arch/arm64/boot/dts/Makefile > > @@ -1,3 +1,4 @@ > > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb > > dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb > > dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb > > dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb > > diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts > > new file mode 100644 > > index 0000000..adf26dd > > --- /dev/null > > +++ b/arch/arm64/boot/dts/mt8173-evb.dts > > @@ -0,0 +1,31 @@ > > +/* > > + * Copyright (c) 2014 MediaTek Inc. > > + * Author: Eddie Huang <eddie.huang@mediatek.com> > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +/dts-v1/; > > +#include "mt8173.dtsi" > > + > > +/ { > > + model = "mediatek,mt8173-evb"; > > + > > + aliases { > > + serial0 = &uart0; > > + serial1 = &uart1; > > + serial2 = &uart2; > > + serial3 = &uart3; > > Do any of these support earlycon? > > > + }; > > + > > + memory { > > Nit: should be memory@40000000 (and you'll need to add device_type = > "memory"). skeleton.dtsi already has a /memory node with device_type = "memory". Shouldn't that be used? Sascha
Hi Matthias, On Thu, 2014-12-11 at 14:02 +0100, Matthias Brugger wrote: > Hi Eddie, > > 2014-12-11 13:47 GMT+01:00 Eddie Huang <eddie.huang@mediatek.com>: > > On Wed, 2014-12-10 at 15:50 +0100, Matthias Brugger wrote: > >> 2014-12-10 15:27 GMT+01:00 Yingjoe Chen <yingjoe.chen@mediatek.com>: > >> > > >> > Hi, > >> > > >> > On Wed, 2014-12-10 at 18:50 +0800, Eddie Huang wrote: > >> > <...> > >> >> diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts > >> >> new file mode 100644 > >> >> index 0000000..adf26dd > >> >> --- /dev/null > >> >> +++ b/arch/arm64/boot/dts/mt8173-evb.dts > >> > <...> > >> >> + timer { > >> >> + compatible = "arm,armv8-timer"; > >> >> + interrupt-parent = <&gic>; > >> >> + interrupts = <1 13 0x8>, > >> >> + <1 14 0x8>, > >> >> + <1 11 0x8>, > >> >> + <1 10 0x8>; > >> >> + clock-frequency = <13000000>; > >> > > >> > I believe our firmware doesn't need this line. Please remove it. > >> > >> The point here would be to know if you need to enable a special timer > >> from the mtk-timer block to get the arch timer working. > >> In any case, you will need some sort of timer. This dts does not > >> describe the mtk-timer (may in the mt8173 it does not exist) but > >> defines the clocks clk26m and clk32k. So if you don't use the > >> mtk-timer, please remove the clocks as there isn't a block using them. > >> > > > > MT8173 has two timer set: CPUGPT and APBGPT, and use CPUGPT to enable > > arch_timer. Previous series only have APBGPT. MT8173 still need enable > > CPUGPT to get arch timer working, we put this in loader, and transparent > > to kernel. So I will remove clk26m and clk32k in next version. > > Ok, so if this is done in the bootloader, there shouldn't be a problem > then. Perfect. > > Just one more comment on the dts nodes. Can you use the makros defined > for the interrupt type > e.g "GIC_PPI 14 IRQ_TYPE_LEVEL_LOW" instead of "1 14 0x8". This makes > the dts easier to read. > > Thanks, > Matthias > 3.18 still don't have following patch http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/301425.html So I can't use interrupt macro in mt8173.dtsi
Hi Mark, On Thu, 2014-12-11 at 18:02 +0000, Mark Rutland wrote: > Hi, > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > > Add device tree support for MT8173 SoC and evalutaion board based on it. > > > > +/ { > > + model = "mediatek,mt8173-evb"; > > + > > + aliases { > > + serial0 = &uart0; > > + serial1 = &uart1; > > + serial2 = &uart2; > > + serial3 = &uart3; > > Do any of these support earlycon? Not yet > > > + }; > > + > > + memory { > > Nit: should be memory@40000000 (and you'll need to add device_type = > "memory"). > > > + reg = <0 0x40000000 0 0x40000000>; > > + }; skeleton.dtsi already has /memory node with address-cells=2, size-cells=1, which will cause build warning if I change to use memory@40000000, because we use size-cells=2. I will not include skeleton.dtsi and follow your suggestion in next version. > > + > > +#include "skeleton.dtsi" > > + > > +/ { > > + compatible = "mediatek,mt8173"; > > + interrupt-parent = <&sysirq>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + cpu-map { > > This should live under /cpus, as documented in > Documentation/devicetree/bindings/arm/topology.txt. Got it, fix next version > > + > > + psci { > > + compatible = "arm,psci-0.2"; > > + method = "smc"; > > + }; > > What are you using as your PSCI 0.2 implementation? > > Is it fully compliant? (e.g. are the reset and power off functions > implemented, may CPU0 be hotplugged)? > > Given only portions of the GIC seem to be described below, what > exception level is your kernel entered at? Per the spec it should be > EL2, but given the brokenness below with the GIC I'm suspicious. > Currently we only implement CPU boot, no power off, no CPU0 hotplug either. And enter kernel at EL2. Actually, we run ATF in EL3, then switch to EL2 to run lk and kernel. > > + > > + clocks { > > Please remove the clock container node. It serves no purpose whatsoever. > > Just put these clocks directly under the root. Got it, fix next version > > + > > + uart_clk: dummy26m { > > + compatible = "fixed-clock"; > > + clock-frequency = <26000000>; > > + #clock-cells = <0>; > > + }; > > + }; > > + > > + timer { > > + compatible = "arm,armv8-timer"; > > + interrupt-parent = <&gic>; > > + interrupts = <1 13 0x8>, > > + <1 14 0x8>, > > + <1 11 0x8>, > > + <1 10 0x8>; > > Shouldn't these have a non-zero cpu mask? Yes, should have non-zero cpu mask > > > + clock-frequency = <13000000>; > > Your firmware should be programming CNTFREQ_EL0, so you shouldn't need > this (PSCI 0.2 requires CNTFREQ_EL0 to be programmed correctly on all > CPUs). Yes, I remove and test ok in my platform > > > + }; > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + compatible = "simple-bus"; > > + ranges; > > + > > + sysirq: intpol-controller@10200620 { > > + compatible = "mediatek,mt8173-sysirq", "mediatek,mt6577-sysirq"; > > + interrupt-controller; > > + #interrupt-cells = <3>; > > + interrupt-parent = <&gic>; > > + reg = <0 0x10200620 0 0x20>; > > + }; > > + > > + gic: interrupt-controller@10220000 { > > + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; > > Surely this should be "arm,gic-400"? Yes, it should be "arm,gic-400", sorry for my mistake. > > > + #interrupt-cells = <3>; > > + interrupt-parent = <&gic>; > > + interrupt-controller; > > + reg = <0 0x10221000 0 0x1000>, > > + <0 0x10222000 0 0x1000>, > > + <0 0x10200620 0 0x1000>; > > You're missing GICV here, and that GICH address is fundamentally wrong > (it _must_ be page aligned). > > The CPU interface (and virtual CPU interface) should be 0x2000 long. > > The GIC maintenance interrupt also seems to be missing. I will fix in next version
On Fri, Dec 12, 2014 at 04:08:25PM +0800, Eddie Huang wrote: > On Thu, 2014-12-11 at 18:02 +0000, Mark Rutland wrote: > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: ... > > > + memory { > > > > Nit: should be memory@40000000 (and you'll need to add device_type = > > "memory"). > > > > > + reg = <0 0x40000000 0 0x40000000>; > > > + }; > > skeleton.dtsi already has /memory node with address-cells=2, > size-cells=1, which will cause build warning if I change to use > memory@40000000, because we use size-cells=2. I will not include > skeleton.dtsi and follow your suggestion in next version. There's skeleton64.dtsi in arch/arm/boot/dts (arm 32bit has LPAE-enabled systems). Perhaps we should come up with a way to share both across the arches? hth, Jason.
On Fri, Dec 12, 2014 at 06:52:35AM +0000, Sascha Hauer wrote: > On Thu, Dec 11, 2014 at 06:02:46PM +0000, Mark Rutland wrote: > > Hi, > > > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > > > Add device tree support for MT8173 SoC and evalutaion board based on it. > > > > > > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com> > > > --- > > > arch/arm64/boot/dts/Makefile | 1 + > > > arch/arm64/boot/dts/mt8173-evb.dts | 31 +++++++ > > > arch/arm64/boot/dts/mt8173.dtsi | 164 +++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 196 insertions(+) > > > create mode 100644 arch/arm64/boot/dts/mt8173-evb.dts > > > create mode 100644 arch/arm64/boot/dts/mt8173.dtsi > > > > > > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile > > > index f8001a6..db7661e 100644 > > > --- a/arch/arm64/boot/dts/Makefile > > > +++ b/arch/arm64/boot/dts/Makefile > > > @@ -1,3 +1,4 @@ > > > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb > > > dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb > > > dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb > > > dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb > > > diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts > > > new file mode 100644 > > > index 0000000..adf26dd > > > --- /dev/null > > > +++ b/arch/arm64/boot/dts/mt8173-evb.dts > > > @@ -0,0 +1,31 @@ > > > +/* > > > + * Copyright (c) 2014 MediaTek Inc. > > > + * Author: Eddie Huang <eddie.huang@mediatek.com> > > > + * > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License version 2 as > > > + * published by the Free Software Foundation. > > > + * > > > + * This program is distributed in the hope that it will be useful, > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > > + * GNU General Public License for more details. > > > + */ > > > + > > > +/dts-v1/; > > > +#include "mt8173.dtsi" > > > + > > > +/ { > > > + model = "mediatek,mt8173-evb"; > > > + > > > + aliases { > > > + serial0 = &uart0; > > > + serial1 = &uart1; > > > + serial2 = &uart2; > > > + serial3 = &uart3; > > > > Do any of these support earlycon? > > > > > + }; > > > + > > > + memory { > > > > Nit: should be memory@40000000 (and you'll need to add device_type = > > "memory"). > > skeleton.dtsi already has a /memory node with device_type = "memory". > Shouldn't that be used? To be honest I don't think that skeleton.dtsi is all that helpful. Almost all dts files define the root #address-cells and #size-cells for clarity anyway (which means the memory node isn't necessarily adequately sized), and the memory node(s) should have unit-addresses (so they won't match the memory node it provides). Where a dts is written with the assumption that the bootloader will fill things there should be a comment in the dts to that effect, with adequately sized memory nodes. The only potentially useful items are the empty /chosen and /aliases nodes. Both of which don't seem to be strictly required. If anything, I'd be tempted to get rid of skeleton.dtsi entirely. Thanks, Mark.
On Fri, Dec 12, 2014 at 08:08:25AM +0000, Eddie Huang wrote: > Hi Mark, > > On Thu, 2014-12-11 at 18:02 +0000, Mark Rutland wrote: > > Hi, > > > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > > > Add device tree support for MT8173 SoC and evalutaion board based on it. > > > > > > +/ { > > > + model = "mediatek,mt8173-evb"; > > > + > > > + aliases { > > > + serial0 = &uart0; > > > + serial1 = &uart1; > > > + serial2 = &uart2; > > > + serial3 = &uart3; > > > > Do any of these support earlycon? > > Not yet > > > > > > + }; > > > + > > > + memory { > > > > Nit: should be memory@40000000 (and you'll need to add device_type = > > "memory"). > > > > > + reg = <0 0x40000000 0 0x40000000>; > > > + }; > > skeleton.dtsi already has /memory node with address-cells=2, > size-cells=1, which will cause build warning if I change to use > memory@40000000, because we use size-cells=2. I will not include > skeleton.dtsi and follow your suggestion in next version. That sounds fine to me. > > > > + > > > +#include "skeleton.dtsi" > > > + > > > +/ { > > > + compatible = "mediatek,mt8173"; > > > + interrupt-parent = <&sysirq>; > > > + #address-cells = <2>; > > > + #size-cells = <2>; > > > + > > > + cpu-map { > > > > This should live under /cpus, as documented in > > Documentation/devicetree/bindings/arm/topology.txt. > > Got it, fix next version > > > > + > > > + psci { > > > + compatible = "arm,psci-0.2"; > > > + method = "smc"; > > > + }; > > > > What are you using as your PSCI 0.2 implementation? > > > > Is it fully compliant? (e.g. are the reset and power off functions > > implemented, may CPU0 be hotplugged)? > > > > Given only portions of the GIC seem to be described below, what > > exception level is your kernel entered at? Per the spec it should be > > EL2, but given the brokenness below with the GIC I'm suspicious. > > > > Currently we only implement CPU boot, no power off, no CPU0 hotplug > either. And enter kernel at EL2. Actually, we run ATF in EL3, then > switch to EL2 to run lk and kernel. Ok. In the absence of CPU_OFF, this is not yet a conforming PSCI 0.2 implementation, so I'm wary of marking this as PSCI 0.2 until that is the case. Any attempt to power of CPUs will hit a BUG() in cpu_die(), and we don't want that. Is CPU0 hotplug planned? If not, does your PSCI implementation report CPU0 as non-hotpluggable via MIGRATE_INFO_TYPE reporting a UP not migratable trusted OS (and MIGRATE_INFO_UP_CPU reporting CPU0 as the resident CPU)? Are SYSTEM_OFF and SYSTEM_RESET available? Thanks, Mark.
On Fri, Dec 12, 2014 at 04:42:54PM +0000, Jason Cooper wrote: > On Fri, Dec 12, 2014 at 04:08:25PM +0800, Eddie Huang wrote: > > On Thu, 2014-12-11 at 18:02 +0000, Mark Rutland wrote: > > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > ... > > > > + memory { > > > > > > Nit: should be memory@40000000 (and you'll need to add device_type = > > > "memory"). > > > > > > > + reg = <0 0x40000000 0 0x40000000>; > > > > + }; > > > > skeleton.dtsi already has /memory node with address-cells=2, > > size-cells=1, which will cause build warning if I change to use > > memory@40000000, because we use size-cells=2. I will not include > > skeleton.dtsi and follow your suggestion in next version. > > There's skeleton64.dtsi in arch/arm/boot/dts (arm 32bit has LPAE-enabled > systems). Perhaps we should come up with a way to share both across the > arches? As I mentioned in my other reply [1], I think if anything it would be better to get rid of the skeleton dtsi entirely. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310617.html Thanks, Mark.
Hi, On Mon, 2014-12-15 at 12:59 +0000, Mark Rutland wrote: > On Fri, Dec 12, 2014 at 08:08:25AM +0000, Eddie Huang wrote: > > Hi Mark, > > > > On Thu, 2014-12-11 at 18:02 +0000, Mark Rutland wrote: > > > Hi, > > > > > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > > > > Add device tree support for MT8173 SoC and evalutaion board based on it. > > > > > > > > > > + > > > > + psci { > > > > + compatible = "arm,psci-0.2"; > > > > + method = "smc"; > > > > + }; > > > > > > What are you using as your PSCI 0.2 implementation? > > > > > > Is it fully compliant? (e.g. are the reset and power off functions > > > implemented, may CPU0 be hotplugged)? > > > > > > Given only portions of the GIC seem to be described below, what > > > exception level is your kernel entered at? Per the spec it should be > > > EL2, but given the brokenness below with the GIC I'm suspicious. > > > > > > > Currently we only implement CPU boot, no power off, no CPU0 hotplug > > either. And enter kernel at EL2. Actually, we run ATF in EL3, then > > switch to EL2 to run lk and kernel. > > Ok. In the absence of CPU_OFF, this is not yet a conforming PSCI 0.2 > implementation, so I'm wary of marking this as PSCI 0.2 until that is > the case. Any attempt to power of CPUs will hit a BUG() in cpu_die(), > and we don't want that. We are still developing PSCI related functions, CPU_ON, CPU_OFF, CPU_SUSPEND are ready, others are going. PSCI 0.2 is our target although lacks some implements > > Is CPU0 hotplug planned? No > > If not, does your PSCI implementation report CPU0 as > non-hotpluggable via MIGRATE_INFO_TYPE reporting a UP not migratable > trusted OS (and MIGRATE_INFO_UP_CPU reporting CPU0 as the resident CPU)? > Will check whether add this > Are SYSTEM_OFF and SYSTEM_RESET available? No yet > > Thanks, > Mark. Since we miss some PSCI-0.2 implements, I don't know whether I should remove PSCI stuff in this patch.
On Tue, Dec 16, 2014 at 08:46:55AM +0000, Eddie Huang wrote: > Hi, > > On Mon, 2014-12-15 at 12:59 +0000, Mark Rutland wrote: > > On Fri, Dec 12, 2014 at 08:08:25AM +0000, Eddie Huang wrote: > > > Hi Mark, > > > > > > On Thu, 2014-12-11 at 18:02 +0000, Mark Rutland wrote: > > > > Hi, > > > > > > > > On Wed, Dec 10, 2014 at 10:50:01AM +0000, Eddie Huang wrote: > > > > > Add device tree support for MT8173 SoC and evalutaion board based on it. > > > > > > > > > > > > > + > > > > > + psci { > > > > > + compatible = "arm,psci-0.2"; > > > > > + method = "smc"; > > > > > + }; > > > > > > > > What are you using as your PSCI 0.2 implementation? > > > > > > > > Is it fully compliant? (e.g. are the reset and power off functions > > > > implemented, may CPU0 be hotplugged)? > > > > > > > > Given only portions of the GIC seem to be described below, what > > > > exception level is your kernel entered at? Per the spec it should be > > > > EL2, but given the brokenness below with the GIC I'm suspicious. > > > > > > > > > > Currently we only implement CPU boot, no power off, no CPU0 hotplug > > > either. And enter kernel at EL2. Actually, we run ATF in EL3, then > > > switch to EL2 to run lk and kernel. > > > > Ok. In the absence of CPU_OFF, this is not yet a conforming PSCI 0.2 > > implementation, so I'm wary of marking this as PSCI 0.2 until that is > > the case. Any attempt to power of CPUs will hit a BUG() in cpu_die(), > > and we don't want that. > > We are still developing PSCI related functions, CPU_ON, CPU_OFF, > CPU_SUSPEND are ready, others are going. PSCI 0.2 is our target although > lacks some implements Ok. It sounds like for now you can describe this with PSCI 0.1 with those functions implemented. > > Is CPU0 hotplug planned? > > No In that case, for PSCI 0.1 you won't be able to have an enable-method of "psci" for CPU0 (and hence won't get CPU_SUSPEND for the moment). > > If not, does your PSCI implementation report CPU0 as > > non-hotpluggable via MIGRATE_INFO_TYPE reporting a UP not migratable > > trusted OS (and MIGRATE_INFO_UP_CPU reporting CPU0 as the resident CPU)? > > > Will check whether add this For PSCI 0.2 you will need to implement these if Cthere is no CPU0 hotplug. Luckily they will only need to return constant values so this should be easy. We still need to implement the relevant logic in the Linux PSCI client code for this, but that should be small and self-contained. > > Are SYSTEM_OFF and SYSTEM_RESET available? > > No yet Ok. > > Thanks, > > Mark. > > Since we miss some PSCI-0.2 implements, I don't know whether I should > remove PSCI stuff in this patch. For the moment you will have to remove the PSCI 0.2 parts, as you are not compliant and it's trivial to break the kernel on such a configuration. You might get away with claiming PSCI 0.1, without an enable method on CPU0, but this will come with reduced functionality. Thanks, Mark.
diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile index f8001a6..db7661e 100644 --- a/arch/arm64/boot/dts/Makefile +++ b/arch/arm64/boot/dts/Makefile @@ -1,3 +1,4 @@ +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-evb.dtb dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb diff --git a/arch/arm64/boot/dts/mt8173-evb.dts b/arch/arm64/boot/dts/mt8173-evb.dts new file mode 100644 index 0000000..adf26dd --- /dev/null +++ b/arch/arm64/boot/dts/mt8173-evb.dts @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2014 MediaTek Inc. + * Author: Eddie Huang <eddie.huang@mediatek.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +/dts-v1/; +#include "mt8173.dtsi" + +/ { + model = "mediatek,mt8173-evb"; + + aliases { + serial0 = &uart0; + serial1 = &uart1; + serial2 = &uart2; + serial3 = &uart3; + }; + + memory { + reg = <0 0x40000000 0 0x40000000>; + }; +}; diff --git a/arch/arm64/boot/dts/mt8173.dtsi b/arch/arm64/boot/dts/mt8173.dtsi new file mode 100644 index 0000000..1286801 --- /dev/null +++ b/arch/arm64/boot/dts/mt8173.dtsi @@ -0,0 +1,164 @@ +/* + * Copyright (c) 2014 MediaTek Inc. + * Author: Eddie Huang <eddie.huang@mediatek.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "skeleton.dtsi" + +/ { + compatible = "mediatek,mt8173"; + interrupt-parent = <&sysirq>; + #address-cells = <2>; + #size-cells = <2>; + + cpu-map { + cluster0 { + core0 { + cpu = <&cpu0>; + }; + core1 { + cpu = <&cpu1>; + }; + }; + + cluster1 { + core0 { + cpu = <&cpu2>; + }; + core1 { + cpu = <&cpu3>; + }; + }; + }; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a53"; + reg = <0x000>; + enable-method = "psci"; + }; + + cpu1: cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a53"; + reg = <0x001>; + enable-method = "psci"; + }; + + cpu2: cpu@2 { + device_type = "cpu"; + compatible = "arm,cortex-a57"; + reg = <0x100>; + enable-method = "psci"; + }; + + cpu3: cpu@3 { + device_type = "cpu"; + compatible = "arm,cortex-a57"; + reg = <0x101>; + enable-method = "psci"; + }; + }; + + psci { + compatible = "arm,psci-0.2"; + method = "smc"; + }; + + clocks { + clk26m: clk26m { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <26000000>; + }; + + clk32k: clk32k { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32000>; + }; + + uart_clk: dummy26m { + compatible = "fixed-clock"; + clock-frequency = <26000000>; + #clock-cells = <0>; + }; + }; + + timer { + compatible = "arm,armv8-timer"; + interrupt-parent = <&gic>; + interrupts = <1 13 0x8>, + <1 14 0x8>, + <1 11 0x8>, + <1 10 0x8>; + clock-frequency = <13000000>; + }; + + soc { + #address-cells = <2>; + #size-cells = <2>; + compatible = "simple-bus"; + ranges; + + sysirq: intpol-controller@10200620 { + compatible = "mediatek,mt8173-sysirq", "mediatek,mt6577-sysirq"; + interrupt-controller; + #interrupt-cells = <3>; + interrupt-parent = <&gic>; + reg = <0 0x10200620 0 0x20>; + }; + + gic: interrupt-controller@10220000 { + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; + #interrupt-cells = <3>; + interrupt-parent = <&gic>; + interrupt-controller; + reg = <0 0x10221000 0 0x1000>, + <0 0x10222000 0 0x1000>, + <0 0x10200620 0 0x1000>; + }; + + uart0: serial@11002000 { + compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart"; + reg = <0 0x11002000 0 0x400>; + interrupts = <0 83 8>; + clocks = <&uart_clk>; + }; + + uart1: serial@11003000 { + compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart"; + reg = <0 0x11003000 0 0x400>; + interrupts = <0 84 8>; + clocks = <&uart_clk>; + }; + + uart2: serial@11004000 { + compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart"; + reg = <0 0x11004000 0 0x400>; + interrupts = <0 85 8>; + clocks = <&uart_clk>; + }; + + uart3: serial@11005000 { + compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart"; + reg = <0 0x11005000 0 0x400>; + interrupts = <0 86 8>; + clocks = <&uart_clk>; + }; + }; + +}; +
Add device tree support for MT8173 SoC and evalutaion board based on it. Signed-off-by: Eddie Huang <eddie.huang@mediatek.com> --- arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/mt8173-evb.dts | 31 +++++++ arch/arm64/boot/dts/mt8173.dtsi | 164 +++++++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+) create mode 100644 arch/arm64/boot/dts/mt8173-evb.dts create mode 100644 arch/arm64/boot/dts/mt8173.dtsi