[PATCH 11/14] arm64: dts: Add initial device tree support for EXYNOS7
Naveen Krishna Ch
naveenkrishna.ch at gmail.com
Wed Sep 3 01:05:46 PDT 2014
Hi Olof,
On 28 August 2014 09:26, Olof Johansson <olof at 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 at samsung.com>
>> Cc: Thomas Abraham <thomas.ab at samsung.com>
>> Cc: Rob Herring <robh at kernel.org>
>> Cc: Catalin Marinas <catalin.marinas at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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.
--
Shine bright,
(: Nav :)
More information about the linux-arm-kernel
mailing list