[PATCH v3 0/3] arm64: add new support Samsung GH7 SoC and SSDK board

[PATCH v3 0/3] arm64: add new support Samsung GH7 SoC and SSDK board

[Kukjin Kim]

This adds support for Samsung ARMv8 core based GH7 SoC and its evaluation SSDK
board.

Changes since v2:
- add comment about reserved memory
- reduced size of reserved memory
- add 4 more cores
- add specific compatible string for pmu
- change model strin for SSDK-GH7 board
- add DT ml in Cc

Note, still missing clocks for serial and it will be added later.

Changes since v1:
- remove model and compatible in dtsi
- remove inclusion /dts-v1/ in dtsi
- add aliase for serial0
- remove compatible cortex-a9-gic for gic
- remove clock-frequncy for timer
- change number of address-cells and size-cells to 2
- enable ARCH_GH7 in single defconfig for arm64

There were many comments for SoC vendor's Kconfig entry but now I have no
choice except adding ARCH_GH7 and enabling it in current single defconfig
for arm64 by default.

[PATCH v3 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board
[PATCH v3 2/3] arm64: defconfig: Enable ARCH_GH7 by default
[PATCH v3 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board

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

[Kukjin Kim]

Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Thomas Abraham <thomas.ab@samsung.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/boot/dts/samsung-gh7.dtsi     | 134 +++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/samsung-ssdk-gh7.dts |  26 ++++++
 2 files changed, 160 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 0000000..d3ab914
--- /dev/null
+++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
@@ -0,0 +1,134 @@
+/*
+ * 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/ 0xFEC00000 0x1400000;	/* EL3 monitor, secure intepreter */
+
+/ {
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	aliases {
+		serial0 = "/amba/uart@12c00000";
+	};
+
+	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>;
+		};
+		cpu@100 {
+			device_type = "cpu";
+			compatible = "arm,armv8";
+			reg = <0x0 0x100>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x8000fff8>;
+		};
+		cpu@101 {
+			device_type = "cpu";
+			compatible = "arm,armv8";
+			reg = <0x0 0x101>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x8000fff8>;
+		};
+		cpu@102 {
+			device_type = "cpu";
+			compatible = "arm,armv8";
+			reg = <0x0 0x102>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x8000fff8>;
+		};
+		cpu@103 {
+			device_type = "cpu";
+			compatible = "arm,armv8";
+			reg = <0x0 0x103>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x8000fff8>;
+		};
+	};
+
+	gic: interrupt-controller@1C000000 {
+		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 = "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>;
+	};
+
+	amba {
+		compatible = "arm,amba-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		serial@12c00000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0 0x12c00000 0 0x10000>;
+			interrupts = <0 418 0>;
+		};
+
+		serial@12c20000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0 0x12c20000 0 0x10000>;
+			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 0000000..80bd93c
--- /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";
+	compatible = "samsung,ssdk-gh7", "samsung,gh7";
+
+	chosen {
+	};
+
+	memory@80000000 {
+		device_type = "memory";
+		reg = <0x00000000 0x80000000 0 0x80000000>;
+	};
+};
-- 
1.8.2.1.339.g52a3e01

[PATCH v3 2/3] arm64: defconfig: Enable ARCH_GH7 by default

[Kukjin Kim]

This patch adds support for Samsung GH7 SoC in arm64/Kconfig and
enable Samsung GH7 based SSDK_GH7 in single defconfig for arm64.

Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/Kconfig           | 5 +++++
 arch/arm64/boot/dts/Makefile | 1 +
 arch/arm64/configs/defconfig | 1 +
 3 files changed, 7 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index dd4327f..7d71823 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -111,6 +111,11 @@ source "kernel/Kconfig.freezer"
 
 menu "Platform selection"
 
+config ARCH_GH7
+	bool "Samsung ARMv8 GH7 SoC"
+	help
+	  This enables support for Samsung GH7 SoC
+
 config ARCH_VEXPRESS
 	bool "ARMv8 software model (Versatile Express)"
 	select ARCH_REQUIRE_GPIOLIB
diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index c52bdb0..54fb0cf 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_GH7) += samsung-ssdk-gh7.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
 dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
 
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 84139be..d683890 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -23,6 +23,7 @@ CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 # CONFIG_BLK_DEV_BSG is not set
 # CONFIG_IOSCHED_DEADLINE is not set
+CONFIG_ARCH_GH7=y
 CONFIG_ARCH_VEXPRESS=y
 CONFIG_ARCH_XGENE=y
 CONFIG_SMP=y
-- 
1.8.2.1.339.g52a3e01

[PATCH v3 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board

[Kukjin Kim]

Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Thomas Abraham <thomas.ab@samsung.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 Documentation/devicetree/bindings/arm/samsung-boards.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/samsung-boards.txt b/Documentation/devicetree/bindings/arm/samsung-boards.txt
index 2168ed3..d6b5493 100644
--- a/Documentation/devicetree/bindings/arm/samsung-boards.txt
+++ b/Documentation/devicetree/bindings/arm/samsung-boards.txt
@@ -16,3 +16,12 @@ Optional:
 		compatible = "samsung,secure-firmware";
 		reg = <0x0203F000 0x1000>;
 	};
+
+* Samsung's GH7 based SSDK-GH7 evaluation board
+
+SSDK-GH7 evaluation board is based on Samsung's GH7 SoC which has ARMv8 cores.
+
+Required root node properties:
+    - compatible = should be one or more of the following.
+        (a) "samsung,ssdk-gh7" - for Samsung's SSDK-GH7 eval board.
+        (b) "samsung,gh7"  - for boards based on GH7 SoC.
-- 
1.8.2.1.339.g52a3e01

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

[Mark Rutland]

Hi,

On Mon, Mar 17, 2014 at 10:14:35PM +0000, Kukjin Kim wrote:
> Signed-off-by: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Thomas Abraham <thomas.ab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm64/boot/dts/samsung-gh7.dtsi     | 134 +++++++++++++++++++++++++++++++
>  arch/arm64/boot/dts/samsung-ssdk-gh7.dts |  26 ++++++
>  2 files changed, 160 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 0000000..d3ab914
> --- /dev/null
> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
> @@ -0,0 +1,134 @@
> +/*
> + * 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/ 0xFEC00000 0x1400000;	/* EL3 monitor, secure intepreter */

As I've mentioned, I'm concerned that this is even in the non-secure
address space that the kernel can access. Why is this not hidden from
the kernel entirely? Why is it expected to be mapped in and reserved?

Additionally, the memory the used by the spin-table (0x0 0x8000fff8) has
not been reserved, and thus the kernel is free to clobber it.

[...]

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

For targeting any future workarounds I would very much prefer a more
specific string.

[...]

> +	pmu {
> +		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>;
> +	};

These are all missing a trigger type (thus making them unusable), and as
"GH7" is the SoC name rather than the CPU name, the compatible string is
somewhat bad.

> +
> +	amba {
> +		compatible = "arm,amba-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		serial@12c00000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0 0x12c00000 0 0x10000>;
> +			interrupts = <0 418 0>;
> +		};
> +
> +		serial@12c20000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0 0x12c20000 0 0x10000>;
> +			interrupts = <0 420 0>;
> +		};

While the primecell bindings and PL011 bindings state that clocks are
optional, the primecell bus code requires a clock named apb_pclk, and
the pl011 driver requires a clock (which it expects to be UARTCLK) to
acquire the frequency from. As neither are provided I do not see how
this DT could possibly be used to boot a usable system.

Additionally the interrupt trigger types are missing.

Given that these are the only IO devices described in the dtsi/dts
combination, and they do not appear to be usable, what is the point in
merging this?

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Kukjin Kim]

Mark Rutland wrote:
> 
> Hi,
> 
Hi Mark,

[...]

> > +/memreserve/ 0xFEC00000 0x1400000;	/* EL3 monitor, secure intepreter */
> 
> As I've mentioned, I'm concerned that this is even in the non-secure
> address space that the kernel can access. Why is this not hidden from
> the kernel entirely? Why is it expected to be mapped in and reserved?
> 
OK, I will make kernel cannot access the memory area with hiding.

> Additionally, the memory the used by the spin-table (0x0 0x8000fff8) has
> not been reserved, and thus the kernel is free to clobber it.
> 
Oops, I missed. OK I will add following instead of above.

+/memreserve/ 0x80000000 0x00010000;

> [...]
> 
> > +	gic: interrupt-controller@1C000000 {
> > +		compatible = "arm,cortex-a15-gic";
> 
> For targeting any future workarounds I would very much prefer a more
> specific string.
> 
If any workarounds are required later, will add specific string then.

> [...]
> 
> > +	pmu {
> > +		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>;
> > +	};
> 
> These are all missing a trigger type (thus making them unusable), and as
> "GH7" is the SoC name rather than the CPU name, the compatible string is
> somewhat bad.
> 
Oops, it should be 8.

And yes, as I've mentioned "GH7" is SoC name not CPU name.

I'm still thinking _really_ I need to use CPU specific name for GH7 SoC because
we don't need to handle for the specific CPU implementation in kernel even we
didn't name it.

> > +
> > +	amba {
> > +		compatible = "arm,amba-bus";
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> > +
> > +		serial@12c00000 {
> > +			compatible = "arm,pl011", "arm,primecell";
> > +			reg = <0 0x12c00000 0 0x10000>;
> > +			interrupts = <0 418 0>;
> > +		};
> > +
> > +		serial@12c20000 {
> > +			compatible = "arm,pl011", "arm,primecell";
> > +			reg = <0 0x12c20000 0 0x10000>;
> > +			interrupts = <0 420 0>;
> > +		};
> 
> While the primecell bindings and PL011 bindings state that clocks are
> optional, the primecell bus code requires a clock named apb_pclk, and
> the pl011 driver requires a clock (which it expects to be UARTCLK) to
> acquire the frequency from. As neither are provided I do not see how
> this DT could possibly be used to boot a usable system.
> 
> Additionally the interrupt trigger types are missing.
> 
> Given that these are the only IO devices described in the dtsi/dts
> combination, and they do not appear to be usable, what is the point in
> merging this?
> 
Definitely, it is meaningful because we can enhance everything more based on
this for the mass product.

Thanks for your time.

- Kukjin