RE: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-19 Thread Kukjin Kim
Mark Rutland wrote:
 
 On Fri, Mar 14, 2014 at 01:26:31AM +, Kukjin Kim wrote:
  +   cpu@000 {
  +   device_type = cpu;
  +   compatible = arm,armv8;

 The arm,armv8 should be a fallback rather than the only entry. The
 precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi 
 for
 an example).

Well, do I really need another name if GH7 has just 'ARMv8 core' 
exactly?
  
   Yes please. You presumably don't have just 'ARMv8 core', but rather a
   specific ARMv8 implementation.
  
  OK, so in my understanding if it is really _just_ 'ARMv8 core' from S/W 
  point
  of view, arm,armv8 should be fine. I will make sure.
 
 It will work, sure. But the compatible list should also list the
 specific CPU. From the S/W point of view there are likely
 implementation-defined registers which mean that no real CPU will be
 just an ARMv8 core.
 
Well, IMHO, SoC is just SoC and we don't think each CPU(core) from SoC and I'm
still wondering why the specific compatible string for CPU is needed in kernel.
And I don't see any custom handling for GH7 SoC. Note sorry, I don't get about
implementation-defined registers.

 We have enough trouble with DTBs for 32-bit SoCs not listing things they
 should. I see no reason to be lax here.
 
Could you please let me know what was the problem?
If it could be on exynos, would be helpful to me.

  +   gic: interrupt-controller@1C00 {
  +   compatible = arm,cortex-a15-gic;

 We should have a more specific compatible string early in the list 
 here
 too.

Well, I think more specific compatible string is not required for gic, 
there
were discussion about using another compatible string for gic-v2. And
cortex-a15-gic should be fine at this moment and if another name is 
required
as per previous discussion, we will then.
  
   While it's probably ok to have arm,cortex-a15-gic in the compatible
   list, it would be good to also have a more specific string, so we can
   identify the GIC implementation precisely in future (in case we need to
   perform any workarounds, or there's some kind of optimisation we could
   perform for some implementations).
  
  Hmm... let's use just it for now then add another specific string later ;-)
 
 Why not do this from the start? Then we can actually rely on the DTB
 rather than it being useless.
 
I mean, we need to use current GIC implementation for GH7 SoC at this moment.

  +   pmu {
  +   compatible = arm,armv8-pmuv3;

 This is missing a specific compatible string.

I don't know why I need another specific compatible string here because 
I
thought the 'armv8-pmuv3' is enough.
  
   Each CPU microarchitecture has different PMU events and quirks. While
   the CPU PMU might be compatible with ARMv8's PMUv3, it will also have
   implementation-specific events, and may have quirks we need to work
   around in future.
  
   As with 32-bit, we'd expect these to be of the form
   ${implementor},${cpu}-pmu.
  
  OK.
 
  +   compatible = samsung,gh7-pmu, armv8-pmuv3;
 
 Is GH7 an SoC or a CPU?
 
SoC. Similar as above. AFAIK, ARM SoC vendors usually didn't name for each CPU
not SoC if there is no change in core hardware implementation, at least Samsung
didn't name.

OK, if you still have strong objection on this, I will remove PMU support here
until agreement about that.

  +   serial@12c2 {
  +   compatible = arm,pl011, arm,primecell;
  +   reg = 0 0x12c2 0 0x1;
  +   interrupts = 0 420 0;
  +   };

 These are both missing required clocks.

Yes right.
  
   So you'll add these?
  
  To be honest, still I'm not sure how it should be handled because regarding
  clocks do not need to handle in the kernel for GH7. And for it, I needed to
  use some HACKs to change clock handling in the kernel. I will provide proper
  solution soon.
 
 Are the clocks always-on, fixed-rate clocks, or does some firmware
 manage them dynamically? We have bindings for the former.
 
Actually, some firmware handles them and basically and kernel doesn't need.

 There's not much point having a DTS upstream that relies on hacks which
 are not.
 
At the end, we are going to use just upstream kernel for GH7 without any HACKs
but as a preliminary, this is a good start point even there is HACK for a while.

Thanks,
Kukjin

--
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 v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-18 Thread Mark Rutland
On Fri, Mar 14, 2014 at 01:26:31AM +, Kukjin Kim wrote:
 + cpu@000 {
 + device_type = cpu;
 + compatible = arm,armv8;
   
The arm,armv8 should be a fallback rather than the only entry. The
precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for
an example).
   
   Well, do I really need another name if GH7 has just 'ARMv8 core' exactly?
  
  Yes please. You presumably don't have just 'ARMv8 core', but rather a
  specific ARMv8 implementation.
  
 OK, so in my understanding if it is really _just_ 'ARMv8 core' from S/W point
 of view, arm,armv8 should be fine. I will make sure.

It will work, sure. But the compatible list should also list the
specific CPU. From the S/W point of view there are likely
implementation-defined registers which mean that no real CPU will be
just an ARMv8 core.

We have enough trouble with DTBs for 32-bit SoCs not listing things they
should. I see no reason to be lax here.

 + gic: interrupt-controller@1C00 {
 + compatible = arm,cortex-a15-gic;
   
We should have a more specific compatible string early in the list here
too.
   
   Well, I think more specific compatible string is not required for gic, 
   there
   were discussion about using another compatible string for gic-v2. And
   cortex-a15-gic should be fine at this moment and if another name is 
   required
   as per previous discussion, we will then.
  
  While it's probably ok to have arm,cortex-a15-gic in the compatible
  list, it would be good to also have a more specific string, so we can
  identify the GIC implementation precisely in future (in case we need to
  perform any workarounds, or there's some kind of optimisation we could
  perform for some implementations).
  
 Hmm... let's use just it for now then add another specific string later ;-)

Why not do this from the start? Then we can actually rely on the DTB
rather than it being useless.

 + pmu {
 + compatible = arm,armv8-pmuv3;
   
This is missing a specific compatible string.
   
   I don't know why I need another specific compatible string here because I
   thought the 'armv8-pmuv3' is enough.
  
  Each CPU microarchitecture has different PMU events and quirks. While
  the CPU PMU might be compatible with ARMv8's PMUv3, it will also have
  implementation-specific events, and may have quirks we need to work
  around in future.
  
  As with 32-bit, we'd expect these to be of the form
  ${implementor},${cpu}-pmu.
  
 OK.
 
 + compatible = samsung,gh7-pmu, armv8-pmuv3;

Is GH7 an SoC or a CPU?

 + serial@12c2 {
 + compatible = arm,pl011, arm,primecell;
 + reg = 0 0x12c2 0 0x1;
 + interrupts = 0 420 0;
 + };
   
These are both missing required clocks.
   
   Yes right.
  
  So you'll add these?
  
 To be honest, still I'm not sure how it should be handled because regarding
 clocks do not need to handle in the kernel for GH7. And for it, I needed to
 use some HACKs to change clock handling in the kernel. I will provide proper
 solution soon.

Are the clocks always-on, fixed-rate clocks, or does some firmware
manage them dynamically? We have bindings for the former.

There's not much point having a DTS upstream that relies on hacks which
are not.

Cheers,
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 v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-14 Thread Mark Brown
On Thu, Mar 13, 2014 at 10:33:29AM +, Mark Rutland wrote:
 On Wed, Mar 12, 2014 at 04:31:56AM +, Kukjin Kim wrote:

+/ {
+   model = SAMSUNG SSDK-GH7 board based on GH7 SoC;

   Is the based on GH7 SoC part necessary? Does the SSDK-GH7 not give
   that away?

  In this case, yes, SSDK-GH7 is enough but I though, in case of different
  board adding what SoC is used on the board in that is useful. Anyway, OK.

 Looking at ePAPR, the recommended format is manufacturer,model, and
 the string is intended to identify a particular implementation. It is
 not intended to give details about the implementation that can be
 derived from the name.

 We seem to have ignored the format (and to some degree purpose) of the
 model property so far, but I don't see any reason to fill it with
 unnecessary information.

Might it be worth defining a property explicitly intended to be used as
a display name for human consumption?  Half the problem with model is
that we don't have a way to use it for quirking so nobody ever really
looks at it (though I guess we will want that at some point now we're
going for fixed ABI stuff), but not having a place to put a pretty name
does encourage this sort of thing.


signature.asc
Description: Digital signature


Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-13 Thread Mark Rutland
On Wed, Mar 12, 2014 at 04:31:56AM +, Kukjin Kim wrote:
 Mark Rutland wrote:
  
 Hi Mark,
 
  On Mon, Mar 10, 2014 at 10:51:17PM +, Kukjin Kim wrote:
   Signed-off-by: Kukjin Kim kgene@samsung.com
   Reviewed-by: Thomas Abraham thomas...@samsung.com
   Cc: Catalin Marinas catalin.mari...@arm.com
   ---
arch/arm64/boot/dts/samsung-gh7.dtsi | 106
  +++
arch/arm64/boot/dts/samsung-ssdk-gh7.dts |  26 
2 files changed, 132 insertions(+)
create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi
create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts
  
   diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi
  b/arch/arm64/boot/dts/samsung-gh7.dtsi
   new file mode 100644
   index 000..b5e8488
   --- /dev/null
   +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
   @@ -0,0 +1,106 @@
   +/*
   + * SAMSUNG GH7 SoC device tree source
   + *
   + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
   + *   http://www.samsung.com
   + *
   + * This program is free software; you can redistribute it and/or modify
   + * it under the terms of the GNU General Public License version 2 as
   + * published by the Free Software Foundation.
   +*/
   +
   +/memreserve/ 0x8000 0x0C40;
  
  Please have a comment as to what this memreserve is intended to protect.
  This is very large, and we don't want pointless memreserves.
  
 Well, you mean I need to comment about each reserved memory area?

Please have a comment about what each /memreserve/ is intended to
protect.

 We need the reserved memory for EL3 monitor, UEFI services, secure
 interpreter, hypervisor and Scan-chain for debug...BTW isn't it depending on
 each hardware platform and usage model?

If it depends on each particular model then it doesn't belong in the
shared dtsi, and each dts should have the /memreserve/ for that
platform.

Why would the hypervisor or secure OS memory ever be accessible to the
kernel such that it would require a memreserve? That sounds
fundamentally broken.

I was under the impression that if using UEFI we can identify and
reserve the UEFI services by walking to UEFI memory map. If we don't use
them, they don't need to be protected. As such, I don't see why they
need a memreserve for them.

 Just note, I will change the reserve memory area and size. From at
 0xf800 and size is 128MB(0x800).

Ok.

  What address does the kernel end up getting loaded at on this board?
  
 We load the kernel image from at 0x8008 after changing the reserve
 memory area.

Ok.

   +/ {
   + interrupt-parent = gic;
   + #address-cells = 2;
   + #size-cells = 2;
   +
   + aliases {
   + serial0 = /amba/uart@12c0;
   + };
   +
   + cpus {
   + #address-cells = 2;
   + #size-cells = 0;
   +
   + cpu@000 {
   + device_type = cpu;
   + compatible = arm,armv8;
  
  The arm,armv8 should be a fallback rather than the only entry. The
  precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for
  an example).
  
 Well, do I really need another name if GH7 has just 'ARMv8 core' exactly?

Yes please. You presumably don't have just 'ARMv8 core', but rather a
specific ARMv8 implementation.

   + reg = 0x0 0x000;
  
  Any reason for padding these to three hexadecimal digits (in both the
  reg and unit-address)?
  
 Yes, I already commented on Olof's question before, other cores will be
 added and it should be separated from this.

Ok.

   + enable-method = spin-table;
   + cpu-release-addr = 0x0 0x8000fff8;
   + };
   + cpu@001 {
   + device_type = cpu;
   + compatible = arm,armv8;
   + reg = 0x0 0x001;
   + enable-method = spin-table;
   + cpu-release-addr = 0x0 0x8000fff8;
   + };
   + cpu@002 {
   + device_type = cpu;
   + compatible = arm,armv8;
   + reg = 0x0 0x002;
   + enable-method = spin-table;
   + cpu-release-addr = 0x0 0x8000fff8;
   + };
   + cpu@003 {
   + device_type = cpu;
   + compatible = arm,armv8;
   + reg = 0x0 0x003;
   + enable-method = spin-table;
   + cpu-release-addr = 0x0 0x8000fff8;
   + };
   + };
   +
   + gic: interrupt-controller@1C00 {
   + compatible = arm,cortex-a15-gic;
  
  We should have a more specific compatible string early in the list here
  too.
  
 Well, I think more specific compatible string is not required for gic, there
 were discussion about using another compatible string for gic-v2. And
 cortex-a15-gic should be fine at this moment and if another name is required
 as per previous discussion, we will then.

While it's probably ok to have arm,cortex-a15-gic in the compatible
list, it would be good to also 

RE: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-13 Thread Kukjin Kim
Mark Rutland wrote:
 

[...]

+/memreserve/ 0x8000 0x0C40;
  
   Please have a comment as to what this memreserve is intended to protect.
   This is very large, and we don't want pointless memreserves.
  
  Well, you mean I need to comment about each reserved memory area?
 
 Please have a comment about what each /memreserve/ is intended to
 protect.
 
OK and I will try to reduce the size of reserved memory.

  We need the reserved memory for EL3 monitor, UEFI services, secure
  interpreter, hypervisor and Scan-chain for debug...BTW isn't it depending on
  each hardware platform and usage model?
 
 If it depends on each particular model then it doesn't belong in the
 shared dtsi, and each dts should have the /memreserve/ for that
 platform.
 
I mean it depends on each SoC not board :-)

 Why would the hypervisor or secure OS memory ever be accessible to the
 kernel such that it would require a memreserve? That sounds
 fundamentally broken.
 
 I was under the impression that if using UEFI we can identify and
 reserve the UEFI services by walking to UEFI memory map. If we don't use
 them, they don't need to be protected. As such, I don't see why they
 need a memreserve for them.
 
I need to talk to regarding engineer internally. Then I will update it.

  Just note, I will change the reserve memory area and size. From at
  0xf800 and size is 128MB(0x800).
 
 Ok.
 
   What address does the kernel end up getting loaded at on this board?
  
  We load the kernel image from at 0x8008 after changing the reserve
  memory area.
 
 Ok.

[...]

+   cpu@000 {
+   device_type = cpu;
+   compatible = arm,armv8;
  
   The arm,armv8 should be a fallback rather than the only entry. The
   precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for
   an example).
  
  Well, do I really need another name if GH7 has just 'ARMv8 core' exactly?
 
 Yes please. You presumably don't have just 'ARMv8 core', but rather a
 specific ARMv8 implementation.
 
OK, so in my understanding if it is really _just_ 'ARMv8 core' from S/W point
of view, arm,armv8 should be fine. I will make sure.

+   reg = 0x0 0x000;
  
   Any reason for padding these to three hexadecimal digits (in both the
   reg and unit-address)?
  
  Yes, I already commented on Olof's question before, other cores will be
  added and it should be separated from this.
 
 Ok.

[...]

+   gic: interrupt-controller@1C00 {
+   compatible = arm,cortex-a15-gic;
  
   We should have a more specific compatible string early in the list here
   too.
  
  Well, I think more specific compatible string is not required for gic, there
  were discussion about using another compatible string for gic-v2. And
  cortex-a15-gic should be fine at this moment and if another name is required
  as per previous discussion, we will then.
 
 While it's probably ok to have arm,cortex-a15-gic in the compatible
 list, it would be good to also have a more specific string, so we can
 identify the GIC implementation precisely in future (in case we need to
 perform any workarounds, or there's some kind of optimisation we could
 perform for some implementations).
 
Hmm... let's use just it for now then add another specific string later ;-)

[...]

+   pmu {
+   compatible = arm,armv8-pmuv3;
  
   This is missing a specific compatible string.
  
  I don't know why I need another specific compatible string here because I
  thought the 'armv8-pmuv3' is enough.
 
 Each CPU microarchitecture has different PMU events and quirks. While
 the CPU PMU might be compatible with ARMv8's PMUv3, it will also have
 implementation-specific events, and may have quirks we need to work
 around in future.
 
 As with 32-bit, we'd expect these to be of the form
 ${implementor},${cpu}-pmu.
 
OK.

+   compatible = samsung,gh7-pmu, armv8-pmuv3;

+   interrupts = 0 294 0,
+0 295 0,
+0 296 0,
+0 297 0,
+0 298 0,
+0 299 0,
+0 300 0,
+0 301 0;
+   };
  
   There are four CPUs described, yet eight interrupts here. There should
   be one per CPU.
  
  Yes right. I added them here by mistake, the other four interrupts will be
  used for other 4 cores will be added later though.
 
 Ok.

[...]

+   serial@12c2 {
+   compatible = arm,pl011, arm,primecell;
+   reg = 0 0x12c2 0 0x1;
+   interrupts = 0 420 0;
+   };
  
   These are both missing required clocks.
  
  Yes right.
 
 So you'll add these?
 
To be honest, still I'm not sure how it should be handled because regarding
clocks do not need to handle in the 

Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-11 Thread Catalin Marinas
On Mon, Mar 10, 2014 at 10:51:17PM +, Kukjin Kim wrote:
 Signed-off-by: Kukjin Kim kgene@samsung.com
 Reviewed-by: Thomas Abraham thomas...@samsung.com
 Cc: Catalin Marinas catalin.mari...@arm.com

This patch should go to the devicetree list as well to get some
reviews/acks from the DT maintainers.

-- 
Catalin
--
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 v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-11 Thread Mark Rutland
On Mon, Mar 10, 2014 at 10:51:17PM +, Kukjin Kim wrote:
 Signed-off-by: Kukjin Kim kgene@samsung.com
 Reviewed-by: Thomas Abraham thomas...@samsung.com
 Cc: Catalin Marinas catalin.mari...@arm.com
 ---
  arch/arm64/boot/dts/samsung-gh7.dtsi | 106 
 +++
  arch/arm64/boot/dts/samsung-ssdk-gh7.dts |  26 
  2 files changed, 132 insertions(+)
  create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi
  create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts
 
 diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi 
 b/arch/arm64/boot/dts/samsung-gh7.dtsi
 new file mode 100644
 index 000..b5e8488
 --- /dev/null
 +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
 @@ -0,0 +1,106 @@
 +/*
 + * SAMSUNG GH7 SoC device tree source
 + *
 + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
 + *   http://www.samsung.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 +*/
 +
 +/memreserve/ 0x8000 0x0C40;

Please have a comment as to what this memreserve is intended to protect.
This is very large, and we don't want pointless memreserves.

What address does the kernel end up getting loaded at on this board?

 +
 +/ {
 + interrupt-parent = gic;
 + #address-cells = 2;
 + #size-cells = 2;
 +
 + aliases {
 + serial0 = /amba/uart@12c0;
 + };
 +
 + cpus {
 + #address-cells = 2;
 + #size-cells = 0;
 +
 + cpu@000 {
 + device_type = cpu;
 + compatible = arm,armv8;

The arm,armv8 should be a fallback rather than the only entry. The
precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for
an example).

 + reg = 0x0 0x000;

Any reason for padding these to three hexadecimal digits (in both the
reg and unit-address)?

 + enable-method = spin-table;
 + cpu-release-addr = 0x0 0x8000fff8;
 + };
 + cpu@001 {
 + device_type = cpu;
 + compatible = arm,armv8;
 + reg = 0x0 0x001;
 + enable-method = spin-table;
 + cpu-release-addr = 0x0 0x8000fff8;
 + };
 + cpu@002 {
 + device_type = cpu;
 + compatible = arm,armv8;
 + reg = 0x0 0x002;
 + enable-method = spin-table;
 + cpu-release-addr = 0x0 0x8000fff8;
 + };
 + cpu@003 {
 + device_type = cpu;
 + compatible = arm,armv8;
 + reg = 0x0 0x003;
 + enable-method = spin-table;
 + cpu-release-addr = 0x0 0x8000fff8;
 + };
 + };
 +
 + gic: interrupt-controller@1C00 {
 + compatible = arm,cortex-a15-gic;

We should have a more specific compatible string early in the list here
too.

 + #interrupt-cells = 3;
 + interrupt-controller;
 + reg = 0x0 0x1C001000 0 0x1000,/* GIC Dist */
 +   0x0 0x1C002000 0 0x1000,/* GIC CPU */
 +   0x0 0x1C004000 0 0x2000,/* GIC VCPU Control */
 +   0x0 0x1C006000 0 0x2000;/* GIC VCPU */
 + interrupts = 1 9 0xf04;   /* GIC Maintenence IRQ */
 + };
 +
 + timer {
 + compatible = arm,armv8-timer;
 + interrupts = 1 13 0xff01, /* Secure Phys IRQ */
 +  1 14 0xff01, /* Non-secure Phys IRQ */
 +  1 11 0xff01, /* Virt IRQ */
 +  1 10 0xff01; /* Hyp IRQ */
 + };
 +
 + pmu {
 + compatible = arm,armv8-pmuv3;

This is missing a specific compatible string.

 + interrupts = 0 294 0,
 +  0 295 0,
 +  0 296 0,
 +  0 297 0,
 +  0 298 0,
 +  0 299 0,
 +  0 300 0,
 +  0 301 0;
 + };

There are four CPUs described, yet eight interrupts here. There should
be one per CPU.

 +
 + amba {
 + compatible = arm,amba-bus;
 + #address-cells = 2;
 + #size-cells = 2;
 + ranges;
 +
 + serial@12c0 {
 + compatible = arm,pl011, arm,primecell;
 + reg = 0 0x12c0 0 0x1;
 + interrupts = 0 418 0;
 + };
 +
 + serial@12c2 {
 + compatible = arm,pl011, arm,primecell;
 + reg = 0 0x12c2 0 0x1;
 + interrupts = 0 420 0;
 + };

These are both missing 

RE: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-11 Thread Kukjin Kim
Mark Rutland wrote:
 
Hi Mark,

 On Mon, Mar 10, 2014 at 10:51:17PM +, Kukjin Kim wrote:
  Signed-off-by: Kukjin Kim kgene@samsung.com
  Reviewed-by: Thomas Abraham thomas...@samsung.com
  Cc: Catalin Marinas catalin.mari...@arm.com
  ---
   arch/arm64/boot/dts/samsung-gh7.dtsi | 106
 +++
   arch/arm64/boot/dts/samsung-ssdk-gh7.dts |  26 
   2 files changed, 132 insertions(+)
   create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi
   create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts
 
  diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi
 b/arch/arm64/boot/dts/samsung-gh7.dtsi
  new file mode 100644
  index 000..b5e8488
  --- /dev/null
  +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
  @@ -0,0 +1,106 @@
  +/*
  + * SAMSUNG GH7 SoC device tree source
  + *
  + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
  + * http://www.samsung.com
  + *
  + * This program is free software; you can redistribute it and/or modify
  + * it under the terms of the GNU General Public License version 2 as
  + * published by the Free Software Foundation.
  +*/
  +
  +/memreserve/ 0x8000 0x0C40;
 
 Please have a comment as to what this memreserve is intended to protect.
 This is very large, and we don't want pointless memreserves.
 
Well, you mean I need to comment about each reserved memory area?

We need the reserved memory for EL3 monitor, UEFI services, secure
interpreter, hypervisor and Scan-chain for debug...BTW isn't it depending on
each hardware platform and usage model?

Just note, I will change the reserve memory area and size. From at
0xf800 and size is 128MB(0x800).

 What address does the kernel end up getting loaded at on this board?
 
We load the kernel image from at 0x8008 after changing the reserve
memory area.

  +
  +/ {
  +   interrupt-parent = gic;
  +   #address-cells = 2;
  +   #size-cells = 2;
  +
  +   aliases {
  +   serial0 = /amba/uart@12c0;
  +   };
  +
  +   cpus {
  +   #address-cells = 2;
  +   #size-cells = 0;
  +
  +   cpu@000 {
  +   device_type = cpu;
  +   compatible = arm,armv8;
 
 The arm,armv8 should be a fallback rather than the only entry. The
 precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for
 an example).
 
Well, do I really need another name if GH7 has just 'ARMv8 core' exactly?

  +   reg = 0x0 0x000;
 
 Any reason for padding these to three hexadecimal digits (in both the
 reg and unit-address)?
 
Yes, I already commented on Olof's question before, other cores will be
added and it should be separated from this.

  +   enable-method = spin-table;
  +   cpu-release-addr = 0x0 0x8000fff8;
  +   };
  +   cpu@001 {
  +   device_type = cpu;
  +   compatible = arm,armv8;
  +   reg = 0x0 0x001;
  +   enable-method = spin-table;
  +   cpu-release-addr = 0x0 0x8000fff8;
  +   };
  +   cpu@002 {
  +   device_type = cpu;
  +   compatible = arm,armv8;
  +   reg = 0x0 0x002;
  +   enable-method = spin-table;
  +   cpu-release-addr = 0x0 0x8000fff8;
  +   };
  +   cpu@003 {
  +   device_type = cpu;
  +   compatible = arm,armv8;
  +   reg = 0x0 0x003;
  +   enable-method = spin-table;
  +   cpu-release-addr = 0x0 0x8000fff8;
  +   };
  +   };
  +
  +   gic: interrupt-controller@1C00 {
  +   compatible = arm,cortex-a15-gic;
 
 We should have a more specific compatible string early in the list here
 too.
 
Well, I think more specific compatible string is not required for gic, there
were discussion about using another compatible string for gic-v2. And
cortex-a15-gic should be fine at this moment and if another name is required
as per previous discussion, we will then.

  +   #interrupt-cells = 3;
  +   interrupt-controller;
  +   reg = 0x0 0x1C001000 0 0x1000,/* GIC Dist */
  + 0x0 0x1C002000 0 0x1000,/* GIC CPU */
  + 0x0 0x1C004000 0 0x2000,/* GIC VCPU Control
*/
  + 0x0 0x1C006000 0 0x2000;/* GIC VCPU */
  +   interrupts = 1 9 0xf04;   /* GIC Maintenence IRQ */
  +   };
  +
  +   timer {
  +   compatible = arm,armv8-timer;
  +   interrupts = 1 13 0xff01, /* Secure Phys IRQ */
  +1 14 0xff01, /* Non-secure Phys IRQ */
  +1 11 0xff01, /* Virt IRQ */
  +1 10 0xff01; /* Hyp IRQ */
  +   };
  +
  +   pmu {
  +   compatible = arm,armv8-pmuv3;
 
 This is missing a specific compatible string.
 
I don't know why I need another specific compatible string 

[PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board

2014-03-10 Thread Kukjin Kim
Signed-off-by: Kukjin Kim kgene@samsung.com
Reviewed-by: Thomas Abraham thomas...@samsung.com
Cc: Catalin Marinas catalin.mari...@arm.com
---
 arch/arm64/boot/dts/samsung-gh7.dtsi | 106 +++
 arch/arm64/boot/dts/samsung-ssdk-gh7.dts |  26 
 2 files changed, 132 insertions(+)
 create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi
 create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts

diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi 
b/arch/arm64/boot/dts/samsung-gh7.dtsi
new file mode 100644
index 000..b5e8488
--- /dev/null
+++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
@@ -0,0 +1,106 @@
+/*
+ * SAMSUNG GH7 SoC device tree source
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+/memreserve/ 0x8000 0x0C40;
+
+/ {
+   interrupt-parent = gic;
+   #address-cells = 2;
+   #size-cells = 2;
+
+   aliases {
+   serial0 = /amba/uart@12c0;
+   };
+
+   cpus {
+   #address-cells = 2;
+   #size-cells = 0;
+
+   cpu@000 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x000;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8000fff8;
+   };
+   cpu@001 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x001;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8000fff8;
+   };
+   cpu@002 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x002;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8000fff8;
+   };
+   cpu@003 {
+   device_type = cpu;
+   compatible = arm,armv8;
+   reg = 0x0 0x003;
+   enable-method = spin-table;
+   cpu-release-addr = 0x0 0x8000fff8;
+   };
+   };
+
+   gic: interrupt-controller@1C00 {
+   compatible = arm,cortex-a15-gic;
+   #interrupt-cells = 3;
+   interrupt-controller;
+   reg = 0x0 0x1C001000 0 0x1000,/* GIC Dist */
+ 0x0 0x1C002000 0 0x1000,/* GIC CPU */
+ 0x0 0x1C004000 0 0x2000,/* GIC VCPU Control */
+ 0x0 0x1C006000 0 0x2000;/* GIC VCPU */
+   interrupts = 1 9 0xf04;   /* GIC Maintenence IRQ */
+   };
+
+   timer {
+   compatible = arm,armv8-timer;
+   interrupts = 1 13 0xff01, /* Secure Phys IRQ */
+1 14 0xff01, /* Non-secure Phys IRQ */
+1 11 0xff01, /* Virt IRQ */
+1 10 0xff01; /* Hyp IRQ */
+   };
+
+   pmu {
+   compatible = arm,armv8-pmuv3;
+   interrupts = 0 294 0,
+0 295 0,
+0 296 0,
+0 297 0,
+0 298 0,
+0 299 0,
+0 300 0,
+0 301 0;
+   };
+
+   amba {
+   compatible = arm,amba-bus;
+   #address-cells = 2;
+   #size-cells = 2;
+   ranges;
+
+   serial@12c0 {
+   compatible = arm,pl011, arm,primecell;
+   reg = 0 0x12c0 0 0x1;
+   interrupts = 0 418 0;
+   };
+
+   serial@12c2 {
+   compatible = arm,pl011, arm,primecell;
+   reg = 0 0x12c2 0 0x1;
+   interrupts = 0 420 0;
+   };
+   };
+};
diff --git a/arch/arm64/boot/dts/samsung-ssdk-gh7.dts 
b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
new file mode 100644
index 000..47afbc4
--- /dev/null
+++ b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
@@ -0,0 +1,26 @@
+/*
+ * SAMSUNG SSDK-GH7 board device tree source
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+/dts-v1/;
+#include samsung-gh7.dtsi
+
+/ {
+   model = SAMSUNG SSDK-GH7 board based on GH7 SoC;
+   compatible =