Re: [PATCH 11/14] arm64: dts: Add initial device tree support for EXYNOS7
From: Tomasz Figa <hidden>
Date: 2014-08-27 11:30:29
Also in:
linux-arm-kernel, linux-samsung-soc
Hi Naveen, Please see my comments inline. On 27.08.2014 11:44, Naveen Krishna Chatradhi wrote:
quoted hunk ↗ jump to hunk
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids. Signed-off-by: Naveen Krishna Chatradhi <redacted> Cc: Thomas Abraham <redacted> Cc: Rob Herring <robh@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com> --- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsidiff --git a/arch/arm64/boot/dts/exynos7.dtsi b/arch/arm64/boot/dts/exynos7.dtsi new file mode 100644 index 0000000..6b9eaf4 --- /dev/null +++ b/arch/arm64/boot/dts/exynos7.dtsi@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * 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. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>; + + aliases { + pinctrl0 = &pinctrl_0; + pinctrl1 = &pinctrl_1; + pinctrl2 = &pinctrl_2; + pinctrl3 = &pinctrl_3; + pinctrl4 = &pinctrl_4; + pinctrl5 = &pinctrl_5; + pinctrl6 = &pinctrl_6; + pinctrl7 = &pinctrl_7; + pinctrl8 = &pinctrl_8; + pinctrl9 = &pinctrl_9; + mshc0 = &mmc_0; + mshc2 = &mmc_2;
Please add aliases for serial controllers. Refer to relevant DT binding documentation for more information.
+ };
+
+ chipid@10000000 {
+ compatible = "samsung,exynos4210-chipid";
+ reg = <0x10000000 0x100>;
+ };Please put SoC components (except cpus node) under a simple-bus node called "soc". Please see exynos5260.dtsi as an example.
+
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a57", "arm,armv8";
+ reg = <0x0 0x0>;
+ };Does this SoC really has only one CPU or this is a workaround for things like missing CPU bring-up code?
+ };
[snip]
+ mct@101C0000 {
+ compatible = "samsung,exynos4210-mct";
+ reg = <0x101C0000 0x800>;
+ interrupt-controller;
+ #interrupt-cells = <1>;MCT is not an interrupt controller.
+ interrupt-parent = <&mct_map>;
+ interrupts = <0>, <1>, <2>, <3>,
+ <4>, <5>, <6>, <7>,
+ <8>, <9>, <10>, <11>;
+ clocks = <&fin_pll>, <&clock_peris PCLK_MCT>;
+ clock-names = "fin_pll", "mct";
+
+ mct_map: mct-map {
+ #interrupt-cells = <1>;
+ #address-cells = <0>;
+ #size-cells = <0>;
+ interrupt-map = <0 &gic 0 112 0>,
+ <1 &gic 0 113 0>,
+ <2 &gic 0 114 0>,
+ <3 &gic 0 115 0>,
+ <4 &gic 0 116 0>,
+ <5 &gic 0 117 0>,
+ <6 &gic 0 118 0>,
+ <7 &gic 0 119 0>,
+ <8 &gic 0 120 0>,
+ <9 &gic 0 121 0>,
+ <10 &gic 0 122 0>,
+ <11 &gic 0 123 0>;All inputs of this interrupt map come from the same interrupt controller. What's the point of this map then?
+ };
+ };
+
+ mmc_0: mmc@15740000 {
+ compatible = "samsung,exynos7-dw-mshc-smu";
+ interrupts = <0 201 0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x15740000 0x2000>;
+ clocks = <&clock_fsys1 ACLK_MMC0>,
+ <&clock_top1 CLK_SCLK_MMC0>;
+ clock-names = "biu", "ciu";
+ fifo-depth = <0x40>;
+ status = "disabled";
+ };
+
+ mmc_2: mmc@15560000 {
+ compatible = "samsung,exynos7-dw-mshc-smu";
+ interrupts = <0 216 0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x15560000 0x2000>;
+ clocks = <&clock_fsys0 ACLK_MMC2>,
+ <&clock_top1 CLK_SCLK_MMC2>;
+ clock-names = "biu", "ciu";
+ fifo-depth = <0x40>;
+ status = "disabled";
+ };
+
+ pinctrl_0: pinctrl@10580000 {
+ compatible = "samsung,exynos7-pinctrl";
+ reg = <0x10580000 0x1000>;
+ interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
+ <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
+ <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
+ <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>;According to patch 5/14, this bank supports only wake-up interrupts. Their interrupt specifiers should be specified either in the wake-up interrupt controller node (for muxed wake-up interrupts) or in nodes of respective banks (for direct wake-up interrupts).
+ wakeup-interrupt-controller {
+ compatible = "samsung,exynos4210-wakeup-eint";
+ interrupt-parent = <&gic>;
+ interrupts = <0 16 0>;
+ };
+ };[snip]
+ serial@13630000 {
+ compatible = "samsung,exynos4210-uart";
+ reg = <0x13630000 0x100>;
+ interrupts = <0 440 0>;
+ clocks = <&clock_peric0 PCLK_UART0>, <&clock_peric0 SCLK_UART0>;
+ clock-names = "uart", "clk_uart_baud0";
+ status = "okay";No need to explicitly specify the status as "okay" in top level dtsi file.
+ };
+
+ serial@14C20000 {
+ compatible = "samsung,exynos4210-uart";
+ reg = <0x14C20000 0x100>;
+ interrupts = <0 456 0>;
+ clocks = <&clock_peric1 PCLK_UART1>, <&clock_peric1 SCLK_UART1>;
+ clock-names = "uart", "clk_uart_baud1";The "clk_uart_baud1" clock doesn't seem right. The N in "clk_uart_baudN" stands for the input of internal clock source mux, not index of the IP block in the SoC. Please make sure this is defined correctly.
+ status = "okay";
Ditto.
+ };
+
+ serial@14C30000 {
+ compatible = "samsung,exynos4210-uart";
+ reg = <0x14C30000 0x100>;
+ interrupts = <0 457 0>;
+ clocks = <&clock_peric1 PCLK_UART2>, <&clock_peric1 SCLK_UART2>;
+ clock-names = "uart", "clk_uart_baud2";Ditto.
+ status = "okay";
Ditto.
+ };
+
+ serial@14C40000 {
+ compatible = "samsung,exynos4210-uart";
+ reg = <0x14C40000 0x100>;
+ interrupts = <0 458 0>;
+ clocks = <&clock_peric1 PCLK_UART3>, <&clock_peric1 SCLK_UART3>;
+ clock-names = "uart", "clk_uart_baud3";Ditto.
+ status = "okay";
Ditto. Best regards, Tomasz