[sprdlinux] Fwd: [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
- From: Lyra Zhang <zhang.lyra@xxxxxxxxx>
- To: "sprdlinux@xxxxxxxxxxxxx" <sprdlinux@xxxxxxxxxxxxx>
- Date: Thu, 25 Dec 2014 10:53:58 +0800
---------- Forwarded message ----------
From: Mark Rutland <mark.rutland@xxxxxxx>
Date: Fri, Dec 12, 2014 at 2:02 AM
Subject: Re: [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and
evaluation board dts and Makefile
To: Eddie Huang <eddie.huang@xxxxxxxxxxxx>
Cc: Matthias Brugger <matthias.bgg@xxxxxxxxx>, Rob Herring
<robh+dt@xxxxxxxxxx>, Jason Cooper <jason@xxxxxxxxxxxxxx>,
"srv_heupstream@xxxxxxxxxxxx" <srv_heupstream@xxxxxxxxxxxx>, Pawel
Moll <Pawel.Moll@xxxxxxx>, Ian Campbell
<ijc+devicetree@xxxxxxxxxxxxxx>, Kumar Gala <galak@xxxxxxxxxxxxxx>,
Catalin Marinas <Catalin.Marinas@xxxxxxx>, Will Deacon
<Will.Deacon@xxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Olof
Johansson <olof@xxxxxxxxx>, "Joe.C" <yingjoe.chen@xxxxxxxxxxxx>,
Robert Richter <rrichter@xxxxxxxxxx>, Mark Brown <broonie@xxxxxxxxxx>,
"devicetree@xxxxxxxxxxxxxxx" <devicetree@xxxxxxxxxxxxxxx>,
"linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>,
"linux-arm-kernel@xxxxxxxxxxxxxxxxxxx"
<linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>, Sascha Hauer
<kernel@xxxxxxxxxxxxxx>, "yh.chen@xxxxxxxxxxxx" <yh.chen@xxxxxxxxxxxx>
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@xxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxx>
> + *
> + * 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@xxxxxxxxxxxx>
> + *
> + * 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.
这个点我们做的比M好 ：）
> + 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.
友商也没有实现PSCI-0.2
> +
> + clocks {
Please remove the clock container node. It serves no purpose whatsoever.
Just put these clocks directly under the root.
这个问题我们也存在，即使多个clk存在也不需要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>;
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Other related posts:
