Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-12-02 Thread Mark Rutland
On Mon, Dec 01, 2014 at 02:21:46AM +, Chanwoo Choi wrote:
 Dear Mark,
 
 On 11/28/2014 11:00 PM, Mark Rutland wrote:
  On Fri, Nov 28, 2014 at 01:18:25PM +, Chanwoo Choi wrote:
  Dear Mark,
 
  On 11/27/2014 08:18 PM, Mark Rutland wrote:
  On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote:
  This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
  based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).
 
  Cc: Kukjin Kim kgene@samsung.com
  Cc: Mark Rutland mark.rutl...@arm.com
  Cc: Arnd Bergmann a...@arndb.de
  Cc: Olof Johansson o...@lixom.net
  Cc: Catalin Marinas catalin.mari...@arm.com
  Cc: Will Deacon will.dea...@arm.com
  Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
  Acked-by: Inki Dae inki@samsung.com
  Acked-by: Geunsik Lim geunsik@samsung.com
  ---
   arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 
  +
   arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
   2 files changed, 1221 insertions(+)
   create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
   create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi
 
 
  [...]
 
  +   cpus {
  +   #address-cells = 2;
  +   #size-cells = 0;
  +
  +   cpu0: cpu@100 {
  +   device_type = cpu;
  +   compatible = arm,cortex-a53, arm,armv8;
  +   enable-method = psci;
 
  While the CPU nodes have enable-methods, I didn't spot a PSCI node
  anywhere, so this dts cannot possibly have been used to bring up an SMP
  system.
 
  How has this dts been tested?
 
  What PSCI revision have you implemented? Have have you tested it?
 
  My mistake,
  Exynos5433 supports PSCI v0.1. I'll add following PSCI nodes:
  I tested the boot of secondary cpu.
 
  psci {
  compatible = arm,psci;
  method = smc;
  cpu_off = 0x8402;
  cpu_on = 0xC403;
  };
 
  Ok. I take it _any_ CPU may be hotplugged (including CPU0), given that
  you don't have MIGRATE_INFO_TYPE from PSCI 0.2 to tell you that this is
  not possible? If not, attempting to hotplug CPU0 will result in a BUG()
  and the kernel will explode.
 
  Has that been tested?
 
 I just tested secondary CPU on during kernel booting after added 'psci' dt 
 node.
 So, I got the ON state of Octa CPUs.
 
 Maybe I need more time to implement CPU0 and secondary cpu hotplugged 
 dynamically on runtime.

So currently PSCI CPU_OFF is not implemented at all?

  Do all CPUs enter the kernel at EL2?
 
 I didn't consider EL2 for hypervisor mode.
 First role of this job, I'll implement CPU on/off and suspend by using PSCI.

Is there any reason not to enter the kernel at EL2?

PSCI 0.2 mandates entering at EL2 if present (and not under a
hypervisor), and it gives the kernel a lot more flexibility to fix
things up (and there's less for FW to restore) even when a hypervisor is
not in use.

Implementing all that to EL2 is _simpler_ than implementing it to EL1.
The kernel will restore what it needs to.

Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-11-30 Thread Chanwoo Choi
Dear Mark,

On 11/28/2014 11:00 PM, Mark Rutland wrote:
 On Fri, Nov 28, 2014 at 01:18:25PM +, Chanwoo Choi wrote:
 Dear Mark,

 On 11/27/2014 08:18 PM, Mark Rutland wrote:
 On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote:
 This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
 based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).

 Cc: Kukjin Kim kgene@samsung.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Olof Johansson o...@lixom.net
 Cc: Catalin Marinas catalin.mari...@arm.com
 Cc: Will Deacon will.dea...@arm.com
 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Acked-by: Geunsik Lim geunsik@samsung.com
 ---
  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 
 +
  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
  2 files changed, 1221 insertions(+)
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

 
 [...]
 
 +   cpus {
 +   #address-cells = 2;
 +   #size-cells = 0;
 +
 +   cpu0: cpu@100 {
 +   device_type = cpu;
 +   compatible = arm,cortex-a53, arm,armv8;
 +   enable-method = psci;

 While the CPU nodes have enable-methods, I didn't spot a PSCI node
 anywhere, so this dts cannot possibly have been used to bring up an SMP
 system.

 How has this dts been tested?

 What PSCI revision have you implemented? Have have you tested it?

 My mistake,
 Exynos5433 supports PSCI v0.1. I'll add following PSCI nodes:
 I tested the boot of secondary cpu.

 psci {
 compatible = arm,psci;
 method = smc;
 cpu_off = 0x8402;
 cpu_on = 0xC403;
 };
 
 Ok. I take it _any_ CPU may be hotplugged (including CPU0), given that
 you don't have MIGRATE_INFO_TYPE from PSCI 0.2 to tell you that this is
 not possible? If not, attempting to hotplug CPU0 will result in a BUG()
 and the kernel will explode.
 
 Has that been tested? 

I just tested secondary CPU on during kernel booting after added 'psci' dt node.
So, I got the ON state of Octa CPUs.

Maybe I need more time to implement CPU0 and secondary cpu hotplugged 
dynamically on runtime.

 
 Do all CPUs enter the kernel at EL2?

I didn't consider EL2 for hypervisor mode.
First role of this job, I'll implement CPU on/off and suspend by using PSCI.

 
 +   soc: soc {
 +   compatible = simple-bus;
 +   #address-cells = 1;
 +   #size-cells = 1;
 +   ranges;
 +
 +   fixed-rate-clocks {
 +   #address-cells = 1;
 +   #size-cells = 0;
 +
 +   xusbxti: clock@0 {
 +   compatible = fixed-clock;
 +   clock-output-names = xusbxti;
 +   #clock-cells = 0;
 +   };
 +   };

 Get rid of the fixed-rate-clocks container node. It's pointless and
 messy. Given you only have one there's no need for the bogus
 unit-address either.

 OK, I'll remove unneeded code and will add following dt node for fin_pll.

 fin_pll: xxti {
 compatible = fixed-clock;
 clock-output-names = fin_pll;
 #clock-cells = 0;
 };
 
 That looks fine to me.
 
 [...]
 
 +   mct@101c {
 +   compatible = samsung,exynos4210-mct;
 +   reg = 0x101c 0x800;
 +   interrupts = 0 102 0, 0 103 0, 0 104 0, 0 
 105 0,
 +   0 106 0, 0 107 0, 0 108 0, 0 109,
 +   0 110 0, 0 111 0, 0 112 0, 0 113 0;
 +   clocks = cmu_top CLK_FIN_PLL, cmu_peris 
 CLK_PCLK_MCT;
 +   clock-names = fin_pll, mct;
 +   };

 Hase this block had no changes whatsoever since its use in Exynos4210?
 Do we not need a samsung,exynos5433-mct comaptible string too?

 The type of Exynos5433's MCT(Multi-Core Timer) IP is the same with the type 
 of Exynos4210 MCT.
 Just Exynos5433 have eight local timer for Octa cores.
 
 So samsung,exynos4210-mct should appear in the list. I'm just
 wondering if it's worth having:
 
   compatible = samsung,exynos5433-mct, samsung,exynos4210-mct;
 
 Just in case we need to special-case the 5433 MCT for some reason later.

OK, I'll add samsung,exynos5433-mct compatible string in exynos5433.dtsi
according to your comment.

 

CPU0   CPU1   CPU2   CPU3   CPU4   CPU5   
 CPU6   CPU7
 134:  0  0  0  0  0  0   
0  0   GIC 134  mct_comp_irq
 138:   3189  0  0  0  0  0   

Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-11-28 Thread Chanwoo Choi
Dear Mark,

On 11/27/2014 08:18 PM, Mark Rutland wrote:
 On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote:
 This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
 based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).

 Cc: Kukjin Kim kgene@samsung.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Olof Johansson o...@lixom.net
 Cc: Catalin Marinas catalin.mari...@arm.com
 Cc: Will Deacon will.dea...@arm.com
 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Acked-by: Geunsik Lim geunsik@samsung.com
 ---
  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 
 +
  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
  2 files changed, 1221 insertions(+)
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi
 
 [...]
 
 diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi 
 b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
 new file mode 100644
 index 000..3d8b576
 --- /dev/null
 +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
 @@ -0,0 +1,523 @@
 +/*
 + * Samsung's Exynos5433 SoC device tree source
 + *
 + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
 + * http://www.samsung.com
 + *
 + * Samsung's Exynos5433 SoC device nodes are listed in this file. Exynos5433
 + * based board files can include this file and provide values for board 
 specfic
 + * bindings.
 + *
 + * Note: This file does not include device nodes for all the controllers in
 + * Exynos5433 SoC. As device tree coverage for Exynos5433 increases, 
 additional
 + * nodes can be added to this file.
 + *
 + * 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 skeleton.dtsi
 +#include dt-bindings/clock/exynos5433.h
 +
 
 Just to check: no memory reservations required for any reason?
 
 There also don't appear to be any memory nodes. Typically if that's
 filled in by the bootloader/FW we'd have an empty node (or one with a
 zero size entry) and a comment regarding the FW.

I add the memory node to board dtsi file because memory information
is more dependent on on h/w target than SoC.

 
 +/ {
 +   compatible = samsung,exynos5433;
 +   #address-cells = 1;
 +   #size-cells = 1;
 
 Not two, on both counts? The CPUs can address more than 32 bits.

You're right. I'll fix it as two and then retry to test it.

 
 Is there nothing in the physical address space above 0x?
 
 [...]
 
 +   cpus {
 +   #address-cells = 2;
 +   #size-cells = 0;
 +
 +   cpu0: cpu@100 {
 +   device_type = cpu;
 +   compatible = arm,cortex-a53, arm,armv8;
 +   enable-method = psci;
 
 While the CPU nodes have enable-methods, I didn't spot a PSCI node
 anywhere, so this dts cannot possibly have been used to bring up an SMP
 system.
 
 How has this dts been tested?
 
 What PSCI revision have you implemented? Have have you tested it?

My mistake,
Exynos5433 supports PSCI v0.1. I'll add following PSCI nodes: 
I tested the boot of secondary cpu.

psci {
compatible = arm,psci;
method = smc;
cpu_off = 0x8402;
cpu_on = 0xC403;
};

 
 I take it from the presence of GICH/GICV in the gic node that CPUs enter
 the kernel at EL2?
 
 +   reg = 0x0 0x100;
 +   clock-frequency = 105000;
 
 What uses this?

It is un-used. I'll drop it.

 
 +   };
 
 [...]
 
 +   soc: soc {
 +   compatible = simple-bus;
 +   #address-cells = 1;
 +   #size-cells = 1;
 +   ranges;
 +
 +   fixed-rate-clocks {
 +   #address-cells = 1;
 +   #size-cells = 0;
 +
 +   xusbxti: clock@0 {
 +   compatible = fixed-clock;
 +   clock-output-names = xusbxti;
 +   #clock-cells = 0;
 +   };
 +   };
 
 Get rid of the fixed-rate-clocks container node. It's pointless and
 messy. Given you only have one there's no need for the bogus
 unit-address either.

OK, I'll remove unneeded code and will add following dt node for fin_pll.

fin_pll: xxti {
compatible = fixed-clock;
clock-output-names = fin_pll;
#clock-cells = 0;
};

 
 +
 +   cmu_top: clock-controller@0x1003{
 
 s/@0x/@/ -- a unit-address should not have the leading '0x'. Please
 apply that to the rest of the file.

I'll remove '0x'.

 
 +   compatible = 

Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-11-28 Thread Chanwoo Choi
Dear Marc,

On 11/27/2014 07:26 PM, Marc Zyngier wrote:
 On 27/11/14 07:35, Chanwoo Choi wrote:
 This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
 based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).

 Cc: Kukjin Kim kgene@samsung.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Olof Johansson o...@lixom.net
 Cc: Catalin Marinas catalin.mari...@arm.com
 Cc: Will Deacon will.dea...@arm.com
 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Acked-by: Geunsik Lim geunsik@samsung.com
 ---
  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 
 +
  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
  2 files changed, 1221 insertions(+)
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

 
 [...]
 
 diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi 
 b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
 new file mode 100644
 index 000..3d8b576
 --- /dev/null
 +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
 
 [...]
 
 +   timer {
 +   compatible = arm,armv8-timer;
 +   interrupts = 1 13 0xff01,
 +1 14 0xff01,
 +1 11 0xff01,
 +1 10 0xff01;
 
 This is wrong. Timer interrupts for both A53 and A57 are level triggered.

I'll fix it level triggering instead of edge triggering.

If possible, could you give the document url to check the correct type of level 
trigger?
whether irq is high level trigger or low level trigger.

 
 +   clock-frequency = 2400;
 
 Please go and fix your firmware. Really...
 
 +   use-clocksource-only;
 +   use-physical-timer;
 +   };
 
 Well, that's a total NAK. Neither of these properties are part of the
 binding, and we've already established that none of that would never be
 valid on arm64.
 
 I suggest you finally do what we've been asking for years, which is to
 fix your boot ROM by adding the 5 lines of assembly code that are needed
 instead of repeatedly post the same bogus DT files.

I'll remove last three properties.

Best Regards,
Chanwoo Choi


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-11-28 Thread Mark Rutland
On Fri, Nov 28, 2014 at 01:18:25PM +, Chanwoo Choi wrote:
 Dear Mark,
 
 On 11/27/2014 08:18 PM, Mark Rutland wrote:
  On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote:
  This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
  based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).
 
  Cc: Kukjin Kim kgene@samsung.com
  Cc: Mark Rutland mark.rutl...@arm.com
  Cc: Arnd Bergmann a...@arndb.de
  Cc: Olof Johansson o...@lixom.net
  Cc: Catalin Marinas catalin.mari...@arm.com
  Cc: Will Deacon will.dea...@arm.com
  Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
  Acked-by: Inki Dae inki@samsung.com
  Acked-by: Geunsik Lim geunsik@samsung.com
  ---
   arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 
  +
   arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
   2 files changed, 1221 insertions(+)
   create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
   create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi
 

[...]

  +   cpus {
  +   #address-cells = 2;
  +   #size-cells = 0;
  +
  +   cpu0: cpu@100 {
  +   device_type = cpu;
  +   compatible = arm,cortex-a53, arm,armv8;
  +   enable-method = psci;
 
  While the CPU nodes have enable-methods, I didn't spot a PSCI node
  anywhere, so this dts cannot possibly have been used to bring up an SMP
  system.
 
  How has this dts been tested?
 
  What PSCI revision have you implemented? Have have you tested it?
 
 My mistake,
 Exynos5433 supports PSCI v0.1. I'll add following PSCI nodes:
 I tested the boot of secondary cpu.
 
 psci {
 compatible = arm,psci;
 method = smc;
 cpu_off = 0x8402;
 cpu_on = 0xC403;
 };

Ok. I take it _any_ CPU may be hotplugged (including CPU0), given that
you don't have MIGRATE_INFO_TYPE from PSCI 0.2 to tell you that this is
not possible? If not, attempting to hotplug CPU0 will result in a BUG()
and the kernel will explode.

Has that been tested? 

Do all CPUs enter the kernel at EL2?

  +   soc: soc {
  +   compatible = simple-bus;
  +   #address-cells = 1;
  +   #size-cells = 1;
  +   ranges;
  +
  +   fixed-rate-clocks {
  +   #address-cells = 1;
  +   #size-cells = 0;
  +
  +   xusbxti: clock@0 {
  +   compatible = fixed-clock;
  +   clock-output-names = xusbxti;
  +   #clock-cells = 0;
  +   };
  +   };
 
  Get rid of the fixed-rate-clocks container node. It's pointless and
  messy. Given you only have one there's no need for the bogus
  unit-address either.
 
 OK, I'll remove unneeded code and will add following dt node for fin_pll.
 
 fin_pll: xxti {
 compatible = fixed-clock;
 clock-output-names = fin_pll;
 #clock-cells = 0;
 };

That looks fine to me.

[...]

  +   mct@101c {
  +   compatible = samsung,exynos4210-mct;
  +   reg = 0x101c 0x800;
  +   interrupts = 0 102 0, 0 103 0, 0 104 0, 0 
  105 0,
  +   0 106 0, 0 107 0, 0 108 0, 0 109,
  +   0 110 0, 0 111 0, 0 112 0, 0 113 0;
  +   clocks = cmu_top CLK_FIN_PLL, cmu_peris 
  CLK_PCLK_MCT;
  +   clock-names = fin_pll, mct;
  +   };
 
  Hase this block had no changes whatsoever since its use in Exynos4210?
  Do we not need a samsung,exynos5433-mct comaptible string too?
 
 The type of Exynos5433's MCT(Multi-Core Timer) IP is the same with the type 
 of Exynos4210 MCT.
 Just Exynos5433 have eight local timer for Octa cores.

So samsung,exynos4210-mct should appear in the list. I'm just
wondering if it's worth having:

compatible = samsung,exynos5433-mct, samsung,exynos4210-mct;

Just in case we need to special-case the 5433 MCT for some reason later.

 
CPU0   CPU1   CPU2   CPU3   CPU4   CPU5   
 CPU6   CPU7
 134:  0  0  0  0  0  0
   0  0   GIC 134  mct_comp_irq
 138:   3189  0  0  0  0  0
   0  0   GIC 138  mct_tick0
 139:  0   2670  0  0  0  0
   0  0   GIC 139  mct_tick1
 140:  0  0   2763  0  0  0
   0  0   GIC 140  mct_tick2
 141:  0  0  0   2732  0  0
   0  0   GIC 141  mct_tick3
 142:  0  0  

Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-11-27 Thread Marc Zyngier
On 27/11/14 07:35, Chanwoo Choi wrote:
 This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
 based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).
 
 Cc: Kukjin Kim kgene@samsung.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Olof Johansson o...@lixom.net
 Cc: Catalin Marinas catalin.mari...@arm.com
 Cc: Will Deacon will.dea...@arm.com
 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Acked-by: Geunsik Lim geunsik@samsung.com
 ---
  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 
 +
  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
  2 files changed, 1221 insertions(+)
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi
 

[...]

 diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi 
 b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
 new file mode 100644
 index 000..3d8b576
 --- /dev/null
 +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi

[...]

 +   timer {
 +   compatible = arm,armv8-timer;
 +   interrupts = 1 13 0xff01,
 +1 14 0xff01,
 +1 11 0xff01,
 +1 10 0xff01;

This is wrong. Timer interrupts for both A53 and A57 are level triggered.

 +   clock-frequency = 2400;

Please go and fix your firmware. Really...

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

Well, that's a total NAK. Neither of these properties are part of the
binding, and we've already established that none of that would never be
valid on arm64.

I suggest you finally do what we've been asking for years, which is to
fix your boot ROM by adding the 5 lines of assembly code that are needed
instead of repeatedly post the same bogus DT files.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-11-27 Thread Mark Rutland
On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote:
 This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
 based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).

 Cc: Kukjin Kim kgene@samsung.com
 Cc: Mark Rutland mark.rutl...@arm.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Olof Johansson o...@lixom.net
 Cc: Catalin Marinas catalin.mari...@arm.com
 Cc: Will Deacon will.dea...@arm.com
 Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
 Acked-by: Inki Dae inki@samsung.com
 Acked-by: Geunsik Lim geunsik@samsung.com
 ---
  arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 
 +
  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
  2 files changed, 1221 insertions(+)
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

[...]

 diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi 
 b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
 new file mode 100644
 index 000..3d8b576
 --- /dev/null
 +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
 @@ -0,0 +1,523 @@
 +/*
 + * Samsung's Exynos5433 SoC device tree source
 + *
 + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
 + * http://www.samsung.com
 + *
 + * Samsung's Exynos5433 SoC device nodes are listed in this file. Exynos5433
 + * based board files can include this file and provide values for board 
 specfic
 + * bindings.
 + *
 + * Note: This file does not include device nodes for all the controllers in
 + * Exynos5433 SoC. As device tree coverage for Exynos5433 increases, 
 additional
 + * nodes can be added to this file.
 + *
 + * 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 skeleton.dtsi
 +#include dt-bindings/clock/exynos5433.h
 +

Just to check: no memory reservations required for any reason?

There also don't appear to be any memory nodes. Typically if that's
filled in by the bootloader/FW we'd have an empty node (or one with a
zero size entry) and a comment regarding the FW.

 +/ {
 +   compatible = samsung,exynos5433;
 +   #address-cells = 1;
 +   #size-cells = 1;

Not two, on both counts? The CPUs can address more than 32 bits.

Is there nothing in the physical address space above 0x?

[...]

 +   cpus {
 +   #address-cells = 2;
 +   #size-cells = 0;
 +
 +   cpu0: cpu@100 {
 +   device_type = cpu;
 +   compatible = arm,cortex-a53, arm,armv8;
 +   enable-method = psci;

While the CPU nodes have enable-methods, I didn't spot a PSCI node
anywhere, so this dts cannot possibly have been used to bring up an SMP
system.

How has this dts been tested?

What PSCI revision have you implemented? Have have you tested it?

I take it from the presence of GICH/GICV in the gic node that CPUs enter
the kernel at EL2?

 +   reg = 0x0 0x100;
 +   clock-frequency = 105000;

What uses this?

 +   };

[...]

 +   soc: soc {
 +   compatible = simple-bus;
 +   #address-cells = 1;
 +   #size-cells = 1;
 +   ranges;
 +
 +   fixed-rate-clocks {
 +   #address-cells = 1;
 +   #size-cells = 0;
 +
 +   xusbxti: clock@0 {
 +   compatible = fixed-clock;
 +   clock-output-names = xusbxti;
 +   #clock-cells = 0;
 +   };
 +   };

Get rid of the fixed-rate-clocks container node. It's pointless and
messy. Given you only have one there's no need for the bogus
unit-address either.

 +
 +   cmu_top: clock-controller@0x1003{

s/@0x/@/ -- a unit-address should not have the leading '0x'. Please
apply that to the rest of the file.

 +   compatible = samsung,exynos5433-cmu-top;
 +   reg = 0x1003 0x0c04;
 +   #clock-cells = 1;
 +   };

[...]

 +   mct@101c {
 +   compatible = samsung,exynos4210-mct;
 +   reg = 0x101c 0x800;
 +   interrupts = 0 102 0, 0 103 0, 0 104 0, 0 105 
 0,
 +   0 106 0, 0 107 0, 0 108 0, 0 109,
 +   0 110 0, 0 111 0, 0 112 0, 0 113 0;
 +   clocks = cmu_top CLK_FIN_PLL, cmu_peris 
 CLK_PCLK_MCT;
 +   clock-names = fin_pll, mct;
 +   };

Hase this block had no changes whatsoever since its use in Exynos4210?
Do we not need a samsung,exynos5433-mct comaptible string too?

 +
 +   gic:interrupt-controller@11001000 {
 + 

[PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC

2014-11-26 Thread Chanwoo Choi
This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC
based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53).

Cc: Kukjin Kim kgene@samsung.com
Cc: Mark Rutland mark.rutl...@arm.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Olof Johansson o...@lixom.net
Cc: Catalin Marinas catalin.mari...@arm.com
Cc: Will Deacon will.dea...@arm.com
Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
Acked-by: Inki Dae inki@samsung.com
Acked-by: Geunsik Lim geunsik@samsung.com
---
 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 +
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++
 2 files changed, 1221 insertions(+)
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
 create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi 
b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
new file mode 100644
index 000..81fe925
--- /dev/null
+++ b/arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi
@@ -0,0 +1,698 @@
+/*
+ * Samsung's Exynos5433 SoC pin-mux and pin-config device tree source
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Samsung's Exynos5433 SoC pin-mux and pin-config options are listed as device
+ * tree nodes are listed in this file.
+ *
+ * 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.
+*/
+
+pinctrl_alive {
+   gpa0: gpa0 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   interrupt-parent = gic;
+   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;
+   #interrupt-cells = 2;
+   };
+
+   gpa1: gpa1 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   interrupt-parent = gic;
+   interrupts = 0 8 0, 0 9 0, 0 10 0, 0 11 0,
+0 12 0, 0 13 0, 0 14 0, 0 15 0;
+   #interrupt-cells = 2;
+   };
+
+   gpa2: gpa2 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+
+   gpa3: gpa3 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+};
+
+pinctrl_aud {
+   gpz0: gpz0 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+
+   gpz1: gpz1 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+
+   i2s0_bus: i2s0-bus {
+   samsung,pins = gpz0-0, gpz0-1, gpz0-2, gpz0-3,
+   gpz0-4, gpz0-5, gpz0-6;
+   samsung,pin-function = 2;
+   samsung,pin-pud = 1;
+   samsung,pin-drv = 0;
+   };
+
+   pcm0_bus: pcm0-bus {
+   samsung,pins = gpz1-0, gpz1-1, gpz1-2, gpz1-3;
+   samsung,pin-function = 3;
+   samsung,pin-pud = 1;
+   samsung,pin-drv = 0;
+   };
+};
+
+pinctrl_cpif {
+   gpv6: gpv6 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+};
+
+pinctrl_ese {
+   gpj2: gpj2 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+};
+
+pinctrl_finger {
+   gpd5: gpd5 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+
+   spi2_bus: spi2-bus {
+   samsung,pins = gpd5-0, gpd5-2, gpd5-3;
+   samsung,pin-function = 2;
+   samsung,pin-pud = 3;
+   samsung,pin-drv = 0;
+   };
+
+   hs_i2c6_bus: hs-i2c6-bus {
+   samsung,pins = gpd5-3, gpd5-2;
+   samsung,pin-function = 4;
+   samsung,pin-pud = 3;
+   samsung,pin-drv = 0;
+   };
+
+};
+
+pinctrl_fsys {
+   gph1: gph1 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+
+   gpr4: gpr4 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+   interrupt-controller;
+   #interrupt-cells = 2;
+   };
+
+   gpr0: gpr0 {
+   gpio-controller;
+   #gpio-cells = 2;
+
+