diff mbox

[11/14] arm64: dts: Add initial device tree support for EXYNOS7

Message ID 1409132660-1898-3-git-send-email-ch.naveen@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Naveen Krishna Chatradhi Aug. 27, 2014, 9:44 a.m. UTC
Add initial device tree nodes for EXYNOS7 SoC.
Also, includes the dt-binding definitions for clock ids.

Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Cc: Thomas Abraham <thomas.ab@samsung.com>
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.dtsi

Comments

Mark Rutland Aug. 27, 2014, 10:42 a.m. UTC | #1
Hi Naveen,

On Wed, Aug 27, 2014 at 10:44:18AM +0100, Naveen Krishna Chatradhi wrote:
> Add initial device tree nodes for EXYNOS7 SoC.
> Also, includes the dt-binding definitions for clock ids.

Fallout from a rebase? That latter part doesn't seem to be relevant.

> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> 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.dtsi
> 
> diff --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>;

Can we guarantee everything going to live within 0x0 - 0xffffffff for
all boards using the SoC?

I suspect that we can't, so the addresses and sizes at the top level
should be two cells. At some point there is bound to be something above
4GB that we'll need to map, so to save us from a painful dts refactoring
we should have the dts organised to support that from the outside.

[...]

> +       cpus {
> +               #address-cells = <2>;
> +               #size-cells = <0>;
> +
> +               cpu@0 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a57", "arm,armv8";
> +                       reg = <0x0 0x0>;
> +               };
> +       };

Only UP?

[...]

> +       gic: interrupt-controller@11001000 {
> +               compatible = "arm,gic-400";
> +               #interrupt-cells = <3>;
> +               #address-cells = <0>;
> +               interrupt-controller;
> +               reg =   <0x11001000 0x1000>,
> +                       <0x11002000 0x1000>,
> +                       <0x11004000 0x2000>,
> +                       <0x11006000 0x2000>;
> +       };

Nice to see GICV and GICH.

[...]

> +       mct@101C0000 {
> +               compatible = "samsung,exynos4210-mct";
> +               reg = <0x101C0000 0x800>;
> +               interrupt-controller;
> +               #interrupt-cells = <1>;
> +               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>;
> +               };
> +       };

I don't see why need the map here. Why can't we describe the GIC
interrupts directly?

[...]

> +       timer {
> +               compatible = "arm,armv8-timer";
> +               interrupts = <1 13 0xff01>,
> +                            <1 14 0xff01>,
> +                            <1 11 0xff01>,
> +                            <1 10 0xff01>;
> +               clock-frequency = <24000000>;

Your firmware/bootloader should configure CNTFRQ, and this shouldn't be
necessary. The clock-frequency property is an incomplete workaround for
broken firmware that in an ideal world we could kill off.

> +               use-clocksource-only;
> +               use-physical-timer;

Neither of these properties were introduced by this series, and no
rationale was given.

What are these properties for, and why do you believe they are
necessary?

Thanks,
Mark.
Tomasz Figa Aug. 27, 2014, 11:30 a.m. UTC | #2
Hi Naveen,

Please see my comments inline.

On 27.08.2014 11:44, Naveen Krishna Chatradhi wrote:
> Add initial device tree nodes for EXYNOS7 SoC.
> Also, includes the dt-binding definitions for clock ids.
> 
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> 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.dtsi
> 
> diff --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
Catalin Marinas Aug. 27, 2014, 2:54 p.m. UTC | #3
On Wed, Aug 27, 2014 at 11:42:31AM +0100, Mark Rutland wrote:
> On Wed, Aug 27, 2014 at 10:44:18AM +0100, Naveen Krishna Chatradhi wrote:
> > +       cpus {
> > +               #address-cells = <2>;
> > +               #size-cells = <0>;
> > +
> > +               cpu@0 {
> > +                       device_type = "cpu";
> > +                       compatible = "arm,cortex-a57", "arm,armv8";
> > +                       reg = <0x0 0x0>;
> > +               };
> > +       };
> 
> Only UP?

I see what you meant. Maybe this is populated by firmware and that's
only an example.
Olof Johansson Aug. 28, 2014, 3:56 a.m. UTC | #4
Hi,

On Wed, Aug 27, 2014 at 03:14:18PM +0530, Naveen Krishna Chatradhi wrote:
> Add initial device tree nodes for EXYNOS7 SoC.
> Also, includes the dt-binding definitions for clock ids.

Uh, no -- it just adds the dtsi.

> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> 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.dtsi
> 
> diff --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

Let's not make the same mistake as on 32-bit, and go with a directory
hierarchy here from day one.

So, please create a exynos subdirectory for this file. You also need
a Makefile when you add a board dts.

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

You should probably use address-cells/size-cells 2/2 on a 64-bit platform.

> +	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;
> +	};
> +
> +	chipid@10000000 {
> +		compatible = "samsung,exynos4210-chipid";
> +		reg = <0x10000000 0x100>;
> +	};
> +
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;

Why size-cells=2? Can you not fit a cpuid in 32 bits?

> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a57", "arm,armv8";
> +			reg = <0x0 0x0>;
> +		};
> +	};
> +
> +	fin_pll: xxti {
> +		compatible = "fixed-clock";
> +		clock-frequency = <24000000>;
> +		clock-output-names = "fin_pll";
> +		#clock-cells = <0>;
> +	};
> +
> +	gic: interrupt-controller@11001000 {
> +		compatible = "arm,gic-400";
> +		#interrupt-cells = <3>;
> +		#address-cells = <0>;
> +		interrupt-controller;
> +		reg =	<0x11001000 0x1000>,
> +			<0x11002000 0x1000>,
> +			<0x11004000 0x2000>,
> +			<0x11006000 0x2000>;
> +	};
> +
> +	hsi2c_0: hsi2c@13640000 {
> +		compatible = "samsung,exynos7-hsi2c";

Is the i2c controller here completely new?

Also, please use 'i2c' for node name on these nodes.

> +		reg = <0x13640000 0x1000>;
> +		interrupts = <0 441 0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hs_i2c0_bus>;
> +		clocks = <&clock_peric0 PCLK_HSI2C0>;
> +		clock-names = "hsi2c";
> +		status = "disabled";
> +	};
> +
> +	hsi2c_1: hsi2c@13650000 {
> +		compatible = "samsung,exynos7-hsi2c";
> +		reg = <0x13650000 0x1000>;
> +		interrupts = <0 442 0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hs_i2c1_bus>;
> +		clocks = <&clock_peric0 PCLK_HSI2C1>;
> +		clock-names = "hsi2c";
> +		status = "disabled";
> +	};
> +
> +	hsi2c_2: hsi2c@14E60000 {

I much prefer lowercase hex in unit addresses (and reg entries) below. I
know 32-bit uses uppercase, but let's switch going forward here.

> +	mct@101C0000 {
> +		compatible = "samsung,exynos4210-mct";

Please just do away with MCT here, and use architected timers going
forward. There really shouldn't be a need to keep supporting MCT any
more -- it's a construct from before arch timers on Cortex-A9.

> +	mmc_0: mmc@15740000 {
> +		compatible = "samsung,exynos7-dw-mshc-smu";

Is this controller backwards compatible with exynos5 ones?

> +	/* The Clock nodes are ordered as per the usermanual. */

"The clock"

"user manual"

> +	timer {
> +	        compatible = "arm,armv8-timer";
> +	        interrupts = <1 13 0xff01>,
> +	                     <1 14 0xff01>,
> +	                     <1 11 0xff01>,
> +	                     <1 10 0xff01>;
> +	        clock-frequency = <24000000>;
> +		use-clocksource-only;
> +		use-physical-timer;

These two properties are not standard, and I would expect any 64-bit
platform to come with PSCI such that you have a way to initialize the
virtual timers.


-Olof
Marc Zyngier Aug. 28, 2014, 8:35 a.m. UTC | #5
On 28/08/14 04:56, Olof Johansson wrote:
> Hi,
> 
> On Wed, Aug 27, 2014 at 03:14:18PM +0530, Naveen Krishna Chatradhi wrote:
>> Add initial device tree nodes for EXYNOS7 SoC.
>> Also, includes the dt-binding definitions for clock ids.
> 
> Uh, no -- it just adds the dtsi.
> 
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> Cc: Thomas Abraham <thomas.ab@samsung.com>
>> 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.dtsi
>>
>> diff --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
> 
> Let's not make the same mistake as on 32-bit, and go with a directory
> hierarchy here from day one.
> 
> So, please create a exynos subdirectory for this file. You also need
> a Makefile when you add a board dts.
> 
>> @@ -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>;
> 
> You should probably use address-cells/size-cells 2/2 on a 64-bit platform.
> 
>> +	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;
>> +	};
>> +
>> +	chipid@10000000 {
>> +		compatible = "samsung,exynos4210-chipid";
>> +		reg = <0x10000000 0x100>;
>> +	};
>> +
>> +	cpus {
>> +		#address-cells = <2>;
>> +		#size-cells = <0>;
> 
> Why size-cells=2? Can you not fit a cpuid in 32 bits?
> 
>> +		cpu@0 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a57", "arm,armv8";
>> +			reg = <0x0 0x0>;
>> +		};
>> +	};
>> +
>> +	fin_pll: xxti {
>> +		compatible = "fixed-clock";
>> +		clock-frequency = <24000000>;
>> +		clock-output-names = "fin_pll";
>> +		#clock-cells = <0>;
>> +	};
>> +
>> +	gic: interrupt-controller@11001000 {
>> +		compatible = "arm,gic-400";
>> +		#interrupt-cells = <3>;
>> +		#address-cells = <0>;
>> +		interrupt-controller;
>> +		reg =	<0x11001000 0x1000>,
>> +			<0x11002000 0x1000>,
>> +			<0x11004000 0x2000>,
>> +			<0x11006000 0x2000>;
>> +	};
>> +
>> +	hsi2c_0: hsi2c@13640000 {
>> +		compatible = "samsung,exynos7-hsi2c";
> 
> Is the i2c controller here completely new?
> 
> Also, please use 'i2c' for node name on these nodes.
> 
>> +		reg = <0x13640000 0x1000>;
>> +		interrupts = <0 441 0>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&hs_i2c0_bus>;
>> +		clocks = <&clock_peric0 PCLK_HSI2C0>;
>> +		clock-names = "hsi2c";
>> +		status = "disabled";
>> +	};
>> +
>> +	hsi2c_1: hsi2c@13650000 {
>> +		compatible = "samsung,exynos7-hsi2c";
>> +		reg = <0x13650000 0x1000>;
>> +		interrupts = <0 442 0>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&hs_i2c1_bus>;
>> +		clocks = <&clock_peric0 PCLK_HSI2C1>;
>> +		clock-names = "hsi2c";
>> +		status = "disabled";
>> +	};
>> +
>> +	hsi2c_2: hsi2c@14E60000 {
> 
> I much prefer lowercase hex in unit addresses (and reg entries) below. I
> know 32-bit uses uppercase, but let's switch going forward here.
> 
>> +	mct@101C0000 {
>> +		compatible = "samsung,exynos4210-mct";
> 
> Please just do away with MCT here, and use architected timers going
> forward. There really shouldn't be a need to keep supporting MCT any
> more -- it's a construct from before arch timers on Cortex-A9.
> 
>> +	mmc_0: mmc@15740000 {
>> +		compatible = "samsung,exynos7-dw-mshc-smu";
> 
> Is this controller backwards compatible with exynos5 ones?
> 
>> +	/* The Clock nodes are ordered as per the usermanual. */
> 
> "The clock"
> 
> "user manual"
> 
>> +	timer {
>> +	        compatible = "arm,armv8-timer";
>> +	        interrupts = <1 13 0xff01>,
>> +	                     <1 14 0xff01>,
>> +	                     <1 11 0xff01>,
>> +	                     <1 10 0xff01>;
>> +	        clock-frequency = <24000000>;
>> +		use-clocksource-only;
>> +		use-physical-timer;
> 
> These two properties are not standard, and I would expect any 64-bit
> platform to come with PSCI such that you have a way to initialize the
> virtual timers.

It really sickens me that this is the n-th iteration of a Samsung SoC
having the generic timer (basically since the 5250 came out), and still
it is littered with stupid firmware bugs:
- Broken CNTFRQ (as outlined by the need of clock-frequency)
- Broken CNTVOFF (as hinted by the reliance on the physical timer)

You would think that after over two years, someone would have a clue and
added the missing 4 instructions to the boot ROM.

Or not.

	M.
Mark Rutland Aug. 28, 2014, 9:48 a.m. UTC | #6
Hi,

> > +	cpus {
> > +		#address-cells = <2>;
> > +		#size-cells = <0>;
> 
> Why size-cells=2? Can you not fit a cpuid in 32 bits?

As of commit 72aea393a2e7 (arm64: smp: honour #address-size when parsing
CPU reg property) Linux can handle single-cell cpu node reg entries
where /cpus/#address-cells = <1>.

I can't make any guarantees about other code (e.g. bootloaders) which
might try to do things with cpu nodes, YMMV.

[...]

> > +	hsi2c_2: hsi2c@14E60000 {
> 
> I much prefer lowercase hex in unit addresses (and reg entries) below. I
> know 32-bit uses uppercase, but let's switch going forward here.

My preference also; I'm happy to enforce that on new dts.

[...]

> > +	timer {
> > +	        compatible = "arm,armv8-timer";
> > +	        interrupts = <1 13 0xff01>,
> > +	                     <1 14 0xff01>,
> > +	                     <1 11 0xff01>,
> > +	                     <1 10 0xff01>;
> > +	        clock-frequency = <24000000>;
> > +		use-clocksource-only;
> > +		use-physical-timer;
> 
> These two properties are not standard, and I would expect any 64-bit
> platform to come with PSCI such that you have a way to initialize the
> virtual timers.

Likewise with clock-frequency. It's not a full workaround, and it's not
hard to initialise CNTFRQ on each CPU.

Mark.
Olof Johansson Aug. 28, 2014, 4:28 p.m. UTC | #7
On Thu, Aug 28, 2014 at 2:48 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
>> > +   cpus {
>> > +           #address-cells = <2>;
>> > +           #size-cells = <0>;
>>
>> Why size-cells=2? Can you not fit a cpuid in 32 bits?
>
> As of commit 72aea393a2e7 (arm64: smp: honour #address-size when parsing
> CPU reg property) Linux can handle single-cell cpu node reg entries
> where /cpus/#address-cells = <1>.
>
> I can't make any guarantees about other code (e.g. bootloaders) which
> might try to do things with cpu nodes, YMMV.

Ok. If address-cells is kept at 2 the unit address needs to be changed
to "0,0". So one or the other has to be changed.

> [...]
>
>> > +   hsi2c_2: hsi2c@14E60000 {
>>
>> I much prefer lowercase hex in unit addresses (and reg entries) below. I
>> know 32-bit uses uppercase, but let's switch going forward here.
>
> My preference also; I'm happy to enforce that on new dts.
>
> [...]
>
>> > +   timer {
>> > +           compatible = "arm,armv8-timer";
>> > +           interrupts = <1 13 0xff01>,
>> > +                        <1 14 0xff01>,
>> > +                        <1 11 0xff01>,
>> > +                        <1 10 0xff01>;
>> > +           clock-frequency = <24000000>;
>> > +           use-clocksource-only;
>> > +           use-physical-timer;
>>
>> These two properties are not standard, and I would expect any 64-bit
>> platform to come with PSCI such that you have a way to initialize the
>> virtual timers.
>
> Likewise with clock-frequency. It's not a full workaround, and it's not
> hard to initialise CNTFRQ on each CPU.

Technically clock-frequency is documented, but not recommended to be
used unless needed for working around firmware that doesn't setup the
register value. :)

In this case it's likely a cargo cult carry over from 5250 where the
CNTFRQ requirement happened around the same time as we were working on
it so that generation firmware lacked support for it -- it should
since then have been fixed properly.


-Olof
Naveen Krishna Ch Sept. 3, 2014, 7:48 a.m. UTC | #8
Hi Mark,

On 27 August 2014 16:12, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Naveen,
>
> On Wed, Aug 27, 2014 at 10:44:18AM +0100, Naveen Krishna Chatradhi wrote:
>> Add initial device tree nodes for EXYNOS7 SoC.
>> Also, includes the dt-binding definitions for clock ids.
>
> Fallout from a rebase? That latter part doesn't seem to be relevant.

Yes, this is fixed now.

>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> Cc: Thomas Abraham <thomas.ab@samsung.com>
>> 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.dtsi
>>
>> diff --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>;
>
> Can we guarantee everything going to live within 0x0 - 0xffffffff for
> all boards using the SoC?
>
> I suspect that we can't, so the addresses and sizes at the top level
> should be two cells. At some point there is bound to be something above
> 4GB that we'll need to map, so to save us from a painful dts refactoring
> we should have the dts organised to support that from the outside.

Ok, this is fixed with #address-cells = 2 and #size-cells = 2.

>
> [...]
>
>> +       cpus {
>> +               #address-cells = <2>;
>> +               #size-cells = <0>;
>> +
>> +               cpu@0 {
>> +                       device_type = "cpu";
>> +                       compatible = "arm,cortex-a57", "arm,armv8";
>> +                       reg = <0x0 0x0>;
>> +               };
>> +       };
>
> Only UP?

This is a quad core SoC and the rest of the CPU nodes have been added
in the second version of this series.

>
> [...]
>
>> +       gic: interrupt-controller@11001000 {
>> +               compatible = "arm,gic-400";
>> +               #interrupt-cells = <3>;
>> +               #address-cells = <0>;
>> +               interrupt-controller;
>> +               reg =   <0x11001000 0x1000>,
>> +                       <0x11002000 0x1000>,
>> +                       <0x11004000 0x2000>,
>> +                       <0x11006000 0x2000>;
>> +       };
>
> Nice to see GICV and GICH.
>
> [...]
>
>> +       mct@101C0000 {
>> +               compatible = "samsung,exynos4210-mct";
>> +               reg = <0x101C0000 0x800>;
>> +               interrupt-controller;
>> +               #interrupt-cells = <1>;
>> +               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>;
>> +               };
>> +       };
>
> I don't see why need the map here. Why can't we describe the GIC
> interrupts directly?

Right, it was not required. Also, we will try and use only arch timer
and not MCT.

>
> [...]
>
>> +       timer {
>> +               compatible = "arm,armv8-timer";
>> +               interrupts = <1 13 0xff01>,
>> +                            <1 14 0xff01>,
>> +                            <1 11 0xff01>,
>> +                            <1 10 0xff01>;
>> +               clock-frequency = <24000000>;
>
> Your firmware/bootloader should configure CNTFRQ, and this shouldn't be
> necessary. The clock-frequency property is an incomplete workaround for
> broken firmware that in an ideal world we could kill off.
>
>> +               use-clocksource-only;
>> +               use-physical-timer;
>
> Neither of these properties were introduced by this series, and no
> rationale was given.
>
> What are these properties for, and why do you believe they are
> necessary?

Sorry, that was a mistake. This has been fixed.

>
> Thanks,
> Mark.

Thanks.
Naveen Krishna Ch Sept. 3, 2014, 7:55 a.m. UTC | #9
Hi Tomasz,

On 27 August 2014 17:00, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Naveen,
>
> Please see my comments inline.
>
> On 27.08.2014 11:44, Naveen Krishna Chatradhi wrote:
>> Add initial device tree nodes for EXYNOS7 SoC.
>> Also, includes the dt-binding definitions for clock ids.
>>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> Cc: Thomas Abraham <thomas.ab@samsung.com>
>> 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.dtsi
>>
>> diff --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.

Ok.

>
>> +     };
>> +
>> +     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.

Ok.

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

Ok, will fix.

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

Ok. Fixed.

>
>> +     };
>> +
>> +     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.

Right. That was a mistake. This is fixed.

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

Thanks for your comments.
Naveen Krishna Ch Sept. 3, 2014, 8:05 a.m. UTC | #10
Hi Olof,

On 28 August 2014 09:26, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
> On Wed, Aug 27, 2014 at 03:14:18PM +0530, Naveen Krishna Chatradhi wrote:
>> Add initial device tree nodes for EXYNOS7 SoC.
>> Also, includes the dt-binding definitions for clock ids.
>
> Uh, no -- it just adds the dtsi.

Ok. Will fix.

>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> Cc: Thomas Abraham <thomas.ab@samsung.com>
>> 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.dtsi
>>
>> diff --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
>
> Let's not make the same mistake as on 32-bit, and go with a directory
> hierarchy here from day one.
>
> So, please create a exynos subdirectory for this file. You also need
> a Makefile when you add a board dts.

Ok. Will fix.

>
>> @@ -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>;
>
> You should probably use address-cells/size-cells 2/2 on a 64-bit platform.

Ok.

>
>> +     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;
>> +     };
>> +
>> +     chipid@10000000 {
>> +             compatible = "samsung,exynos4210-chipid";
>> +             reg = <0x10000000 0x100>;
>> +     };
>> +
>> +     cpus {
>> +             #address-cells = <2>;
>> +             #size-cells = <0>;
>
> Why size-cells=2? Can you not fit a cpuid in 32 bits?
>
>> +             cpu@0 {
>> +                     device_type = "cpu";
>> +                     compatible = "arm,cortex-a57", "arm,armv8";
>> +                     reg = <0x0 0x0>;
>> +             };
>> +     };
>> +
>> +     fin_pll: xxti {
>> +             compatible = "fixed-clock";
>> +             clock-frequency = <24000000>;
>> +             clock-output-names = "fin_pll";
>> +             #clock-cells = <0>;
>> +     };
>> +
>> +     gic: interrupt-controller@11001000 {
>> +             compatible = "arm,gic-400";
>> +             #interrupt-cells = <3>;
>> +             #address-cells = <0>;
>> +             interrupt-controller;
>> +             reg =   <0x11001000 0x1000>,
>> +                     <0x11002000 0x1000>,
>> +                     <0x11004000 0x2000>,
>> +                     <0x11006000 0x2000>;
>> +     };
>> +
>> +     hsi2c_0: hsi2c@13640000 {
>> +             compatible = "samsung,exynos7-hsi2c";
>
> Is the i2c controller here completely new?

It is almost the same as in Exynos5 but few register bits have been
changed. So we have added a new compatible string.

>
> Also, please use 'i2c' for node name on these nodes.

Ok.

>
>> +             reg = <0x13640000 0x1000>;
>> +             interrupts = <0 441 0>;
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&hs_i2c0_bus>;
>> +             clocks = <&clock_peric0 PCLK_HSI2C0>;
>> +             clock-names = "hsi2c";
>> +             status = "disabled";
>> +     };
>> +
>> +     hsi2c_1: hsi2c@13650000 {
>> +             compatible = "samsung,exynos7-hsi2c";
>> +             reg = <0x13650000 0x1000>;
>> +             interrupts = <0 442 0>;
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&hs_i2c1_bus>;
>> +             clocks = <&clock_peric0 PCLK_HSI2C1>;
>> +             clock-names = "hsi2c";
>> +             status = "disabled";
>> +     };
>> +
>> +     hsi2c_2: hsi2c@14E60000 {
>
> I much prefer lowercase hex in unit addresses (and reg entries) below. I
> know 32-bit uses uppercase, but let's switch going forward here.

Ok. Will fix.

>
>> +     mct@101C0000 {
>> +             compatible = "samsung,exynos4210-mct";
>
> Please just do away with MCT here, and use architected timers going
> forward. There really shouldn't be a need to keep supporting MCT any
> more -- it's a construct from before arch timers on Cortex-A9.

Ok.

>
>> +     mmc_0: mmc@15740000 {
>> +             compatible = "samsung,exynos7-dw-mshc-smu";
>
> Is this controller backwards compatible with exynos5 ones?

The dwmmc controller in Exynos7 is not fully backward compatible with
Exynos5. Specifically, it requires 64-bit related changes for IDMAC
and handling the changes in register offsets.

>
>> +     /* The Clock nodes are ordered as per the usermanual. */
>
> "The clock"
>
> "user manual"

Ok.

>
>> +     timer {
>> +             compatible = "arm,armv8-timer";
>> +             interrupts = <1 13 0xff01>,
>> +                          <1 14 0xff01>,
>> +                          <1 11 0xff01>,
>> +                          <1 10 0xff01>;
>> +             clock-frequency = <24000000>;
>> +             use-clocksource-only;
>> +             use-physical-timer;
>
> These two properties are not standard, and I would expect any 64-bit
> platform to come with PSCI such that you have a way to initialize the
> virtual timers.

Ok.

>
>
> -Olof


Thanks for your comments.
diff mbox

Patch

diff --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;
+	};
+
+	chipid@10000000 {
+		compatible = "samsung,exynos4210-chipid";
+		reg = <0x10000000 0x100>;
+	};
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a57", "arm,armv8";
+			reg = <0x0 0x0>;
+		};
+	};
+
+	fin_pll: xxti {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "fin_pll";
+		#clock-cells = <0>;
+	};
+
+	gic: interrupt-controller@11001000 {
+		compatible = "arm,gic-400";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		interrupt-controller;
+		reg =	<0x11001000 0x1000>,
+			<0x11002000 0x1000>,
+			<0x11004000 0x2000>,
+			<0x11006000 0x2000>;
+	};
+
+	hsi2c_0: hsi2c@13640000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x13640000 0x1000>;
+		interrupts = <0 441 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c0_bus>;
+		clocks = <&clock_peric0 PCLK_HSI2C0>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_1: hsi2c@13650000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x13650000 0x1000>;
+		interrupts = <0 442 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c1_bus>;
+		clocks = <&clock_peric0 PCLK_HSI2C1>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_2: hsi2c@14E60000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x14E60000 0x1000>;
+		interrupts = <0 459 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c2_bus>;
+		clocks = <&clock_peric1 PCLK_HSI2C2>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_3: hsi2c@14E70000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x14E70000 0x1000>;
+		interrupts = <0 460 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c3_bus>;
+		clocks = <&clock_peric1 PCLK_HSI2C3>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_4: hsi2c@13660000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x13660000 0x1000>;
+		interrupts = <0 443 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c4_bus>;
+		clocks = <&clock_peric0 PCLK_HSI2C4>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_5: hsi2c@13670000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x13670000 0x1000>;
+		interrupts = <0 444 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c5_bus>;
+		clocks = <&clock_peric0 PCLK_HSI2C5>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_6: hsi2c@14E00000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x14E00000 0x1000>;
+		interrupts = <0 461 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c6_bus>;
+		clocks = <&clock_peric1 PCLK_HSI2C6>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_7: hsi2c@13E10000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x13E10000 0x1000>;
+		interrupts = <0 462 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c7_bus>;
+		clocks = <&clock_peric1 PCLK_HSI2C7>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_8: hsi2c@14E20000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x14E20000 0x1000>;
+		interrupts = <0 463 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c8_bus>;
+		clocks = <&clock_peric1 PCLK_HSI2C8>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_9: hsi2c@13680000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x13680000 0x1000>;
+		interrupts = <0 445 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c9_bus>;
+		clocks = <&clock_peric0 PCLK_HSI2C9>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_10: hsi2c@13690000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x13690000 0x1000>;
+		interrupts = <0 446 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c10_bus>;
+		clocks = <&clock_peric0 PCLK_HSI2C10>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	hsi2c_11: hsi2c@136A0000 {
+		compatible = "samsung,exynos7-hsi2c";
+		reg = <0x136A0000 0x1000>;
+		interrupts = <0 447 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&hs_i2c11_bus>;
+		clocks = <&clock_peric0 PCLK_HSI2C11>;
+		clock-names = "hsi2c";
+		status = "disabled";
+	};
+
+	mct@101C0000 {
+		compatible = "samsung,exynos4210-mct";
+		reg = <0x101C0000 0x800>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		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>;
+		};
+	};
+
+	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>;
+		wakeup-interrupt-controller {
+			compatible = "samsung,exynos4210-wakeup-eint";
+			interrupt-parent = <&gic>;
+			interrupts = <0 16 0>;
+		};
+	};
+
+	pinctrl_1: pinctrl@114B0000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x114B0000 0x1000>;
+		interrupts = <0 92 0>;
+	};
+
+	pinctrl_2: pinctrl@13470000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x13470000 0x1000>;
+		interrupts = <0 383 0>;
+	};
+
+	pinctrl_3: pinctrl@14870000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x14870000 0x1000>;
+		interrupts = <0 384 0>;
+	};
+
+	pinctrl_4: pinctrl@14CD0000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x14CD0000 0x1000>;
+		interrupts = <0 473 0>;
+	};
+
+	pinctrl_5: pinctrl@14CE0000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x14CE0000 0x1000>;
+		interrupts = <0 474 0>;
+	};
+
+	pinctrl_6: pinctrl@14C90000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x14C90000 0x1000>;
+		interrupts = <0 475 0>;
+	};
+
+	pinctrl_7: pinctrl@14CA0000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x14CA0000 0x1000>;
+		interrupts = <0 476 0>;
+	};
+
+	pinctrl_8: pinctrl@10E60000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x10E60000 0x1000>;
+		interrupts = <0 221 0>;
+	};
+
+	pinctrl_9: pinctrl@15690000 {
+		compatible = "samsung,exynos7-pinctrl";
+		reg = <0x15690000 0x1000>;
+		interrupts = <0 203 0>;
+	};
+
+	pwm: pwm@136C0000 {
+		compatible = "samsung,exynos4210-pwm";
+		reg = <0x136C0000 0x100>;
+		samsung,pwm-outputs = <0>, <1>, <2>, <3>;
+		#pwm-cells = <3>;
+		clocks = <&clock_peric0 PCLK_PWM>;
+		clock-names = "timers";
+		status = "disabled";
+	};
+
+	rtc@10590000 {
+		compatible = "samsung,s3c6410-rtc";
+		reg = <0x10590000 0x100>;
+		interrupts = <0 355 0>, <0 356 0>;
+		clocks = <&clock_ccore PCLK_RTC>;
+		clock-names = "rtc";
+	};
+
+	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";
+	};
+
+	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";
+		status = "okay";
+	};
+
+	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";
+		status = "okay";
+	};
+
+	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";
+		status = "okay";
+	};
+
+	/* The Clock nodes are ordered as per the usermanual. */
+	clock_topc: clock-controller@10570000 {
+		compatible = "samsung,exynos7-clock-topc";
+		reg = <0x10570000 0x10000>;
+		#clock-cells = <1>;
+	};
+
+	clock_top0: clock-controller@105D0000 {
+		compatible = "samsung,exynos7-clock-top0";
+		reg = <0x105D0000 0xB000>;
+		#clock-cells = <1>;
+	};
+
+	clock_top1: clock-controller@105E0000 {
+		compatible = "samsung,exynos7-clock-top1";
+		reg = <0x105E0000 0xB000>;
+		#clock-cells = <1>;
+	};
+
+	clock_atlas: clock-controller@11800000 {
+		compatible = "samsung,exynos7-clock-atlas";
+		reg = <0x11800000 0x1100>;
+		#clock-cells = <1>;
+	};
+
+	clock_g3d: clock-controller@14AA0000 {
+		compatible = "samsung,exynos7-clock-g3d";
+		reg = <0x14AA0000 0x1100>;
+		#clock-cells = <1>;
+	};
+
+	clock_mif0: clock-controller@10850000 {
+		compatible = "samsung,exynos7-clock-mif0";
+		reg = <0x10850000 0x1100>;
+		#clock-cells = <1>;
+	};
+
+	clock_mif1: clock-controller@10950000 {
+		compatible = "samsung,exynos7-clock-mif1";
+		reg = <0x10950000 0x1100>;
+		#clock-cells = <1>;
+	};
+
+	clock_mif2: clock-controller@10A50000 {
+		compatible = "samsung,exynos7-clock-mif2";
+		reg = <0x10A50000 0x1100>;
+		#clock-cells = <1>;
+	};
+
+	clock_mif3: clock-controller@10B50000 {
+		compatible = "samsung,exynos7-clock-mif3";
+		reg = <0x10B50000 0x1100>;
+		#clock-cells = <1>;
+	};
+
+	clock_ccore: clock-controller@105B0000 {
+		compatible = "samsung,exynos7-clock-ccore";
+		reg = <0x105B0000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_imem: clock-controller@11060000 {
+		compatible = "samsung,exynos7-clock-imem";
+		reg = <0x11060000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_peric0: clock-controller@13610000 {
+		compatible = "samsung,exynos7-clock-peric0";
+		reg = <0x13610000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_peric1: clock-controller@14C80000 {
+		compatible = "samsung,exynos7-clock-peric1";
+		reg = <0x14C80000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_peris: clock-controller@10040000 {
+		compatible = "samsung,exynos7-clock-peris";
+		reg = <0x10040000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_bus0: clock-controller@13400000 {
+		compatible = "samsung,exynos7-clock-bus0";
+		reg = <0x13400000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_bus1: clock-controller@14800000 {
+		compatible = "samsung,exynos7-clock-bus1";
+		reg = <0x14800000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_disp: clock-controller@13AD0000 {
+		compatible = "samsung,exynos7-clock-disp";
+		reg = <0x13AD0000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_aud: clock-controller@114C0000 {
+		compatible = "samsung,exynos7-clock-aud";
+		reg = <0x114C0000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_fsys0: clock-controller@10E90000 {
+		compatible = "samsung,exynos7-clock-fsys0";
+		reg = <0x10E90000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_fsys1: clock-controller@156E0000 {
+		compatible = "samsung,exynos7-clock-fsys1";
+		reg = <0x156E0000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_mscl: clock-controller@150D0000 {
+		compatible = "samsung,exynos7-clock-mscl";
+		reg = <0x150D0000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	clock_mfc: clock-controller@15280000 {
+		compatible = "samsung,exynos7-clock-mfc";
+		reg = <0x15280000 0xD00>;
+		#clock-cells = <1>;
+	};
+
+	timer {
+	        compatible = "arm,armv8-timer";
+	        interrupts = <1 13 0xff01>,
+	                     <1 14 0xff01>,
+	                     <1 11 0xff01>,
+	                     <1 10 0xff01>;
+	        clock-frequency = <24000000>;
+		use-clocksource-only;
+		use-physical-timer;
+	};
+};
+
+#include "exynos7-pinctrl.dtsi"