linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Add basic support for Mediatek MT8173 SoC
@ 2014-12-10 10:49 Eddie Huang
  2014-12-10 10:49 ` [PATCH 1/4] Document: DT: Add bindings for mediatek MT8173 Soc Platform Eddie Huang
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Eddie Huang @ 2014-12-10 10:49 UTC (permalink / raw)
  To: linux-arm-kernel

MT8173 is a SoC based on 64bit ARMv8 architecture. It contains 
2 CA53 + 2 CA57 cores. MT8173 share many HW IP with MT8135 and other 
MT65xx series. This patchset was tested on MT8173 engineer sample, and 
boot to shell ok.

This series contains document bindings, device tree, defconfig, 
and one sysirq modification for MT8173.

This series is based on 3.18, and Joe's MT8135 basic support patch [1] 
and sysirq patch [2]

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296093.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305808.html

Eddie Huang (3):
  Document: DT: Add bindings for mediatek MT8173 Soc Platform
  arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and
    Makefile
  arm64: mediatek: Add MT8173 SoC Kconfig and defconfig

Yingjoe Chen (1):
  irqchip: mediatek: Add support for mt8173

 Documentation/devicetree/bindings/arm/mediatek.txt |   4 +
 arch/arm64/Kconfig                                 |   6 +
 arch/arm64/boot/dts/Makefile                       |   1 +
 arch/arm64/boot/dts/mt8173-evb.dts                 |  31 ++++
 arch/arm64/boot/dts/mt8173.dtsi                    | 164 +++++++++++++++++++++
 arch/arm64/configs/defconfig                       |   2 +
 drivers/irqchip/irq-mtk-sysirq.c                   |  22 ++-
 7 files changed, 226 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm64/boot/dts/mt8173-evb.dts
 create mode 100644 arch/arm64/boot/dts/mt8173.dtsi

-- 
1.8.1.1.dirty 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/4] Document: DT: Add bindings for mediatek MT8173 Soc Platform
  2014-12-10 10:49 [PATCH 0/4] Add basic support for Mediatek MT8173 SoC Eddie Huang
@ 2014-12-10 10:49 ` Eddie Huang
  2014-12-10 10:50 ` [PATCH 2/4] irqchip: mediatek: Add support for mt8173 Eddie Huang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Eddie Huang @ 2014-12-10 10:49 UTC (permalink / raw)
  To: linux-arm-kernel

Add the devicetree binding document for mediatek MT8173 SoC.
This is a 64-bit four core Soc.
And mt8173-evb is a evaluation board based on mt8173.

Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
---
 Documentation/devicetree/bindings/arm/mediatek.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.txt b/Documentation/devicetree/bindings/arm/mediatek.txt
index 1ca713e..622c5d9 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek.txt
@@ -8,6 +8,7 @@ compatible: Must contain one of
    "mediatek,mt6589"
    "mediatek,mt8127"
    "mediatek,mt8135"
+   "mediatek,mt8173"
 
 
 Supported boards:
@@ -21,3 +22,6 @@ Supported boards:
 - MTK mt8135 tablet EVB:
     Required root node properties:
       - compatible = "mediatek,mt8135-evbp1", "mediatek,mt8135";
+- MTK mt8173 tablet EVB:
+    Required root node properties:
+      - compatible = "mediatek,mt8173-evb", "mediatek,mt8173";
-- 
1.8.1.1.dirty

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/4] irqchip: mediatek: Add support for mt8173
  2014-12-10 10:49 [PATCH 0/4] Add basic support for Mediatek MT8173 SoC Eddie Huang
  2014-12-10 10:49 ` [PATCH 1/4] Document: DT: Add bindings for mediatek MT8173 Soc Platform Eddie Huang
@ 2014-12-10 10:50 ` Eddie Huang
  2014-12-10 11:00   ` Arnd Bergmann
  2014-12-10 10:50 ` [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile Eddie Huang
  2014-12-10 10:50 ` [PATCH 4/4] arm64: mediatek: Add MT8173 SoC Kconfig and defconfig Eddie Huang
  3 siblings, 1 reply; 22+ messages in thread
From: Eddie Huang @ 2014-12-10 10:50 UTC (permalink / raw)
  To: linux-arm-kernel

From: Yingjoe Chen <yingjoe.chen@mediatek.com>

MT8173 intpol have 32 more irq pins, add support to it.

Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
---
 drivers/irqchip/irq-mtk-sysirq.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
index 7e342df..c8c0cdb 100644
--- a/drivers/irqchip/irq-mtk-sysirq.c
+++ b/drivers/irqchip/irq-mtk-sysirq.c
@@ -24,6 +24,7 @@
 #include "irqchip.h"
 
 #define MT6577_SYS_INTPOL_NUM	(224)
+#define MT8173_SYS_INTPOL_NUM	(256)
 
 struct mtk_sysirq_chip_data {
 	spinlock_t lock;
@@ -120,7 +121,8 @@ static struct irq_domain_ops sysirq_domain_ops = {
 };
 
 static int __init mtk_sysirq_of_init(struct device_node *node,
-				     struct device_node *parent)
+				     struct device_node *parent,
+				     int intpol_num)
 {
 	struct irq_domain *domain, *domain_parent;
 	struct mtk_sysirq_chip_data *chip_data;
@@ -143,8 +145,7 @@ static int __init mtk_sysirq_of_init(struct device_node *node,
 		goto out_free;
 	}
 
-	domain = irq_domain_add_hierarchy(domain_parent, 0,
-					  MT6577_SYS_INTPOL_NUM, node,
+	domain = irq_domain_add_hierarchy(domain_parent, 0, intpol_num, node,
 					  &sysirq_domain_ops, chip_data);
 	if (!domain) {
 		ret = -ENOMEM;
@@ -160,4 +161,17 @@ out_free:
 	kfree(chip_data);
 	return ret;
 }
-IRQCHIP_DECLARE(mtk_sysirq, "mediatek,mt6577-sysirq", mtk_sysirq_of_init);
+
+static int __init mt6577_sysirq_of_init(struct device_node *node,
+					struct device_node *parent)
+{
+	return mtk_sysirq_of_init(node, parent, MT6577_SYS_INTPOL_NUM);
+}
+
+static int __init mt8173_sysirq_of_init(struct device_node *node,
+					struct device_node *parent)
+{
+	return mtk_sysirq_of_init(node, parent, MT8173_SYS_INTPOL_NUM);
+}
+IRQCHIP_DECLARE(mt6577_sysirq, "mediatek,mt6577-sysirq", mt6577_sysirq_of_init);
+IRQCHIP_DECLARE(mt8173_sysirq, "mediatek,mt8173-sysirq", mt8173_sysirq_of_init);
-- 
1.8.1.1.dirty

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-10 10:49 [PATCH 0/4] Add basic support for Mediatek MT8173 SoC Eddie Huang
  2014-12-10 10:49 ` [PATCH 1/4] Document: DT: Add bindings for mediatek MT8173 Soc Platform Eddie Huang
  2014-12-10 10:50 ` [PATCH 2/4] irqchip: mediatek: Add support for mt8173 Eddie Huang
@ 2014-12-10 10:50 ` Eddie Huang
  2014-12-10 14:27   ` Yingjoe Chen
  2014-12-11 18:02   ` Mark Rutland
  2014-12-10 10:50 ` [PATCH 4/4] arm64: mediatek: Add MT8173 SoC Kconfig and defconfig Eddie Huang
  3 siblings, 2 replies; 22+ messages in thread
From: Eddie Huang @ 2014-12-10 10:50 UTC (permalink / raw)
  To: linux-arm-kernel

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;
+	};
+
+	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 at 0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x000>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu at 1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			reg = <0x001>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu at 2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a57";
+			reg = <0x100>;
+			enable-method = "psci";
+		};
+
+		cpu3: cpu at 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 at 10200620 {
+			compatible = "mediatek,mt8173-sysirq", "mediatek,mt6577-sysirq";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			reg = <0 0x10200620 0 0x20>;
+		};
+
+		gic: interrupt-controller at 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 at 11002000 {
+			compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart";
+			reg = <0 0x11002000 0 0x400>;
+			interrupts = <0 83 8>;
+			clocks = <&uart_clk>;
+		};
+
+		uart1: serial at 11003000 {
+			compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart";
+			reg = <0 0x11003000 0 0x400>;
+			interrupts = <0 84 8>;
+			clocks = <&uart_clk>;
+		};
+
+		uart2: serial at 11004000 {
+			compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart";
+			reg = <0 0x11004000 0 0x400>;
+			interrupts = <0 85 8>;
+			clocks = <&uart_clk>;
+		};
+
+		uart3: serial at 11005000 {
+			compatible = "mediatek,mt8173-uart","mediatek,mt6577-uart";
+			reg = <0 0x11005000 0 0x400>;
+			interrupts = <0 86 8>;
+			clocks = <&uart_clk>;
+		};
+	};
+
+};
+
-- 
1.8.1.1.dirty

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/4] arm64: mediatek: Add MT8173 SoC Kconfig and defconfig
  2014-12-10 10:49 [PATCH 0/4] Add basic support for Mediatek MT8173 SoC Eddie Huang
                   ` (2 preceding siblings ...)
  2014-12-10 10:50 ` [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile Eddie Huang
@ 2014-12-10 10:50 ` Eddie Huang
  3 siblings, 0 replies; 22+ messages in thread
From: Eddie Huang @ 2014-12-10 10:50 UTC (permalink / raw)
  To: linux-arm-kernel

Add MT8173 arm64 Kconfig and defconfig files

Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
---
 arch/arm64/Kconfig           | 6 ++++++
 arch/arm64/configs/defconfig | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9532f8d..85fd1ca 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -142,6 +142,12 @@ source "kernel/Kconfig.freezer"
 
 menu "Platform selection"
 
+config ARCH_MEDIATEK
+	bool "Mediatek MT65xx & MT81xx ARMv8 SoC"
+	select ARM_GIC
+	help
+	  Support for Mediatek MT65xx & MT81xx ARMv8 SoCs
+
 config ARCH_THUNDER
 	bool "Cavium Inc. Thunder SoC Family"
 	help
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index dd301be..ed84d21 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -32,6 +32,7 @@ CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 # CONFIG_BLK_DEV_BSG is not set
 # CONFIG_IOSCHED_DEADLINE is not set
+CONFIG_ARCH_MEDIATEK=y
 CONFIG_ARCH_THUNDER=y
 CONFIG_ARCH_VEXPRESS=y
 CONFIG_ARCH_XGENE=y
@@ -87,6 +88,7 @@ CONFIG_SERIO_AMBAKMI=y
 CONFIG_LEGACY_PTY_COUNT=16
 CONFIG_SERIAL_8250=y
 CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_8250_MT6577=y
 CONFIG_SERIAL_AMBA_PL011=y
 CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
 CONFIG_SERIAL_OF_PLATFORM=y
-- 
1.8.1.1.dirty

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/4] irqchip: mediatek: Add support for mt8173
  2014-12-10 10:50 ` [PATCH 2/4] irqchip: mediatek: Add support for mt8173 Eddie Huang
@ 2014-12-10 11:00   ` Arnd Bergmann
  2014-12-10 14:37     ` Yingjoe Chen
  0 siblings, 1 reply; 22+ messages in thread
From: Arnd Bergmann @ 2014-12-10 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 10 December 2014 18:50:00 Eddie Huang wrote:
> From: Yingjoe Chen <yingjoe.chen@mediatek.com>
> 
> MT8173 intpol have 32 more irq pins, add support to it.
> 
> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> 

How about adding a property for the number of irq pins and leave the
old compatible string in place? I don't think it would be good if
we have to update this driver for each new SoC that uses this
irqchip just to change one number.

	Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-10 10:50 ` [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile Eddie Huang
@ 2014-12-10 14:27   ` Yingjoe Chen
  2014-12-10 14:50     ` Matthias Brugger
  2014-12-11 18:02   ` Mark Rutland
  1 sibling, 1 reply; 22+ messages in thread
From: Yingjoe Chen @ 2014-12-10 14:27 UTC (permalink / raw)
  To: linux-arm-kernel


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/4] irqchip: mediatek: Add support for mt8173
  2014-12-10 11:00   ` Arnd Bergmann
@ 2014-12-10 14:37     ` Yingjoe Chen
  2014-12-10 14:41       ` Arnd Bergmann
  0 siblings, 1 reply; 22+ messages in thread
From: Yingjoe Chen @ 2014-12-10 14:37 UTC (permalink / raw)
  To: linux-arm-kernel


Hi Arnd,

On Wed, 2014-12-10 at 12:00 +0100, Arnd Bergmann wrote:
> On Wednesday 10 December 2014 18:50:00 Eddie Huang wrote:
> > From: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > 
> > MT8173 intpol have 32 more irq pins, add support to it.
> > 
> > Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> > 
> 
> How about adding a property for the number of irq pins and leave the
> old compatible string in place? I don't think it would be good if
> we have to update this driver for each new SoC that uses this
> irqchip just to change one number.
> 
> 	Arnd

OK, I'll change to something like this in next version:


--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
@@ -17,12 +17,17 @@ Required properties:
   use the same interrupt-cells format as GIC.
 - reg: Physical base address of the intpol registers and length of memory
   mapped region.

+Optional properties:
+- mediatek,intpol-number: The number of interrupts supported by intpol,
+  default 224 if omitted.
+
 Example:
        sysirq: interrupt-controller at 10200100 {
                compatible = "mediatek,mt6589-sysirq", "mediatek,mt6577-sysirq";
                interrupt-controller;
                #interrupt-cells = <3>;
                interrupt-parent = <&gic>;
                reg = <0 0x10200100 0 0x1c>;
+               mediatek,intpol-number = <224>;
        };

Joe.C

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/4] irqchip: mediatek: Add support for mt8173
  2014-12-10 14:37     ` Yingjoe Chen
@ 2014-12-10 14:41       ` Arnd Bergmann
  0 siblings, 0 replies; 22+ messages in thread
From: Arnd Bergmann @ 2014-12-10 14:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 10 December 2014 22:37:13 Yingjoe Chen wrote:
> OK, I'll change to something like this in next version:

I've just taken another look at the driver and have a different
suggestion:

> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
> @@ -17,12 +17,17 @@ Required properties:
>    use the same interrupt-cells format as GIC.
>  - reg: Physical base address of the intpol registers and length of memory
>    mapped region.
> 
> +Optional properties:
> +- mediatek,intpol-number: The number of interrupts supported by intpol,
> +  default 224 if omitted.
> +
>  Example:
>         sysirq: interrupt-controller at 10200100 {
>                 compatible = "mediatek,mt6589-sysirq", "mediatek,mt6577-sysirq";
>                 interrupt-controller;
>                 #interrupt-cells = >;
>                 interrupt-parent = <&gic>;
>                 reg = <0 0x10200100 0 0x1c>;
> +               mediatek,intpol-number = <224>;
>         };

The number of interrupt pins directly corresponds to the size of the reg
property in bits, so I think you can just use that and don't even need
another property.

	Arnd

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-10 14:27   ` Yingjoe Chen
@ 2014-12-10 14:50     ` Matthias Brugger
  2014-12-11 12:47       ` Eddie Huang
  0 siblings, 1 reply; 22+ messages in thread
From: Matthias Brugger @ 2014-12-10 14:50 UTC (permalink / raw)
  To: linux-arm-kernel

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
>
>



-- 
motzblog.wordpress.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-10 14:50     ` Matthias Brugger
@ 2014-12-11 12:47       ` Eddie Huang
  2014-12-11 13:02         ` Matthias Brugger
  0 siblings, 1 reply; 22+ messages in thread
From: Eddie Huang @ 2014-12-11 12:47 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-11 12:47       ` Eddie Huang
@ 2014-12-11 13:02         ` Matthias Brugger
  2014-12-12  7:45           ` Eddie Huang
  0 siblings, 1 reply; 22+ messages in thread
From: Matthias Brugger @ 2014-12-11 13:02 UTC (permalink / raw)
  To: linux-arm-kernel

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

>



-- 
motzblog.wordpress.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-10 10:50 ` [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile Eddie Huang
  2014-12-10 14:27   ` Yingjoe Chen
@ 2014-12-11 18:02   ` Mark Rutland
  2014-12-12  6:52     ` Sascha Hauer
  2014-12-12  8:08     ` Eddie Huang
  1 sibling, 2 replies; 22+ messages in thread
From: Mark Rutland @ 2014-12-11 18:02 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x000>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu1: cpu at 1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x001>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu2: cpu at 2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a57";
> +			reg = <0x100>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu3: cpu at 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 at 10200620 {
> +			compatible = "mediatek,mt8173-sysirq", "mediatek,mt6577-sysirq";
> +			interrupt-controller;
> +			#interrupt-cells = <3>;
> +			interrupt-parent = <&gic>;
> +			reg = <0 0x10200620 0 0x20>;
> +		};
> +
> +		gic: interrupt-controller at 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-11 18:02   ` Mark Rutland
@ 2014-12-12  6:52     ` Sascha Hauer
  2014-12-15 11:28       ` Mark Rutland
  2014-12-12  8:08     ` Eddie Huang
  1 sibling, 1 reply; 22+ messages in thread
From: Sascha Hauer @ 2014-12-12  6:52 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-11 13:02         ` Matthias Brugger
@ 2014-12-12  7:45           ` Eddie Huang
  0 siblings, 0 replies; 22+ messages in thread
From: Eddie Huang @ 2014-12-12  7:45 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-11 18:02   ` Mark Rutland
  2014-12-12  6:52     ` Sascha Hauer
@ 2014-12-12  8:08     ` Eddie Huang
  2014-12-12 16:42       ` Jason Cooper
  2014-12-15 12:59       ` Mark Rutland
  1 sibling, 2 replies; 22+ messages in thread
From: Eddie Huang @ 2014-12-12  8:08 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 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 at 10200620 {
> > +			compatible = "mediatek,mt8173-sysirq", "mediatek,mt6577-sysirq";
> > +			interrupt-controller;
> > +			#interrupt-cells = <3>;
> > +			interrupt-parent = <&gic>;
> > +			reg = <0 0x10200620 0 0x20>;
> > +		};
> > +
> > +		gic: interrupt-controller at 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-12  8:08     ` Eddie Huang
@ 2014-12-12 16:42       ` Jason Cooper
  2014-12-15 13:32         ` Mark Rutland
  2014-12-15 12:59       ` Mark Rutland
  1 sibling, 1 reply; 22+ messages in thread
From: Jason Cooper @ 2014-12-12 16:42 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-12  6:52     ` Sascha Hauer
@ 2014-12-15 11:28       ` Mark Rutland
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2014-12-15 11:28 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-12  8:08     ` Eddie Huang
  2014-12-12 16:42       ` Jason Cooper
@ 2014-12-15 12:59       ` Mark Rutland
  2014-12-16  8:46         ` Eddie Huang
  1 sibling, 1 reply; 22+ messages in thread
From: Mark Rutland @ 2014-12-15 12:59 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-12 16:42       ` Jason Cooper
@ 2014-12-15 13:32         ` Mark Rutland
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2014-12-15 13:32 UTC (permalink / raw)
  To: linux-arm-kernel

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 at 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 at 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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-15 12:59       ` Mark Rutland
@ 2014-12-16  8:46         ` Eddie Huang
  2014-12-16 10:17           ` Mark Rutland
  0 siblings, 1 reply; 22+ messages in thread
From: Eddie Huang @ 2014-12-16  8:46 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile
  2014-12-16  8:46         ` Eddie Huang
@ 2014-12-16 10:17           ` Mark Rutland
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2014-12-16 10:17 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2014-12-16 10:17 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-10 10:49 [PATCH 0/4] Add basic support for Mediatek MT8173 SoC Eddie Huang
2014-12-10 10:49 ` [PATCH 1/4] Document: DT: Add bindings for mediatek MT8173 Soc Platform Eddie Huang
2014-12-10 10:50 ` [PATCH 2/4] irqchip: mediatek: Add support for mt8173 Eddie Huang
2014-12-10 11:00   ` Arnd Bergmann
2014-12-10 14:37     ` Yingjoe Chen
2014-12-10 14:41       ` Arnd Bergmann
2014-12-10 10:50 ` [PATCH 3/4] arm64: dts: Add mediatek MT8173 SoC and evaluation board dts and Makefile Eddie Huang
2014-12-10 14:27   ` Yingjoe Chen
2014-12-10 14:50     ` Matthias Brugger
2014-12-11 12:47       ` Eddie Huang
2014-12-11 13:02         ` Matthias Brugger
2014-12-12  7:45           ` Eddie Huang
2014-12-11 18:02   ` Mark Rutland
2014-12-12  6:52     ` Sascha Hauer
2014-12-15 11:28       ` Mark Rutland
2014-12-12  8:08     ` Eddie Huang
2014-12-12 16:42       ` Jason Cooper
2014-12-15 13:32         ` Mark Rutland
2014-12-15 12:59       ` Mark Rutland
2014-12-16  8:46         ` Eddie Huang
2014-12-16 10:17           ` Mark Rutland
2014-12-10 10:50 ` [PATCH 4/4] arm64: mediatek: Add MT8173 SoC Kconfig and defconfig Eddie Huang

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).