* [PATCH v2 0/3] arm64: add new support Samsung GH7 SoC and SSDK board @ 2014-03-10 22:51 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel, linux-samsung-soc Cc: Thomas Abraham, Ilho Lee, Catalin Marinas This adds support for Samsung ARMv8 core based GH7 SoC and its evaluation SSDK board. 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 v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default [PATCH v2 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 0/3] arm64: add new support Samsung GH7 SoC and SSDK board @ 2014-03-10 22:51 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel This adds support for Samsung ARMv8 core based GH7 SoC and its evaluation SSDK board. 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 v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default [PATCH v2 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-10 22:51 ` Kukjin Kim @ 2014-03-10 22:51 ` Kukjin Kim -1 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel, linux-samsung-soc Cc: Thomas Abraham, Ilho Lee, Catalin Marinas, 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 | 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 0000000..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/ 0x80000000 0x0C400000; + +/ { + 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>; + }; + }; + + 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 = "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@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..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 = "samsung,ssdk-gh7", "samsung,gh7"; + + chosen { + }; + + memory@80000000 { + device_type = "memory"; + reg = <0x00000000 0x80000000 0 0x80000000>; + }; +}; -- 1.8.2.1.339.g52a3e01 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-10 22:51 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel 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 | 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 0000000..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/ 0x80000000 0x0C400000; + +/ { + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + aliases { + serial0 = "/amba/uart at 12c00000"; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 000 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x000>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + cpu at 001 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x001>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + cpu at 002 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x002>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + cpu at 003 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x003>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + }; + + gic: interrupt-controller at 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 = "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 at 12c00000 { + compatible = "arm,pl011", "arm,primecell"; + reg = <0 0x12c00000 0 0x10000>; + interrupts = <0 418 0>; + }; + + serial at 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..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 = "samsung,ssdk-gh7", "samsung,gh7"; + + chosen { + }; + + memory at 80000000 { + device_type = "memory"; + reg = <0x00000000 0x80000000 0 0x80000000>; + }; +}; -- 1.8.2.1.339.g52a3e01 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-10 22:51 ` Kukjin Kim @ 2014-03-11 11:59 ` Catalin Marinas -1 siblings, 0 replies; 30+ messages in thread From: Catalin Marinas @ 2014-03-11 11:59 UTC (permalink / raw) To: Kukjin Kim; +Cc: linux-arm-kernel, linux-samsung-soc, Thomas Abraham, Ilho Lee On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com> > Reviewed-by: Thomas Abraham <thomas.ab@samsung.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> This patch should go to the devicetree list as well to get some reviews/acks from the DT maintainers. -- Catalin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-11 11:59 ` Catalin Marinas 0 siblings, 0 replies; 30+ messages in thread From: Catalin Marinas @ 2014-03-11 11:59 UTC (permalink / raw) To: linux-arm-kernel On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com> > Reviewed-by: Thomas Abraham <thomas.ab@samsung.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> This patch should go to the devicetree list as well to get some reviews/acks from the DT maintainers. -- Catalin ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-10 22:51 ` Kukjin Kim @ 2014-03-11 18:29 ` Mark Rutland -1 siblings, 0 replies; 30+ messages in thread From: Mark Rutland @ 2014-03-11 18:29 UTC (permalink / raw) To: Kukjin Kim Cc: linux-arm-kernel, linux-samsung-soc, Catalin Marinas, Thomas Abraham, Ilho Lee On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > 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 | 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 0000000..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/ 0x80000000 0x0C400000; 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@12c00000"; > + }; > + > + 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@1C000000 { > + 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@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>; > + }; These are both missing required clocks. > + }; > +}; > 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..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"; Is the "based on GH7 SoC" part necessary? Does the "SSDK-GH7" not give that away? Thanks, Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-11 18:29 ` Mark Rutland 0 siblings, 0 replies; 30+ messages in thread From: Mark Rutland @ 2014-03-11 18:29 UTC (permalink / raw) To: linux-arm-kernel On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > 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 | 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 0000000..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/ 0x80000000 0x0C400000; 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 at 12c00000"; > + }; > + > + cpus { > + #address-cells = <2>; > + #size-cells = <0>; > + > + cpu at 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 at 001 { > + device_type = "cpu"; > + compatible = "arm,armv8"; > + reg = <0x0 0x001>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0x8000fff8>; > + }; > + cpu at 002 { > + device_type = "cpu"; > + compatible = "arm,armv8"; > + reg = <0x0 0x002>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0x8000fff8>; > + }; > + cpu at 003 { > + device_type = "cpu"; > + compatible = "arm,armv8"; > + reg = <0x0 0x003>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0x8000fff8>; > + }; > + }; > + > + gic: interrupt-controller at 1C000000 { > + 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 at 12c00000 { > + compatible = "arm,pl011", "arm,primecell"; > + reg = <0 0x12c00000 0 0x10000>; > + interrupts = <0 418 0>; > + }; > + > + serial at 12c20000 { > + compatible = "arm,pl011", "arm,primecell"; > + reg = <0 0x12c20000 0 0x10000>; > + interrupts = <0 420 0>; > + }; These are both missing required clocks. > + }; > +}; > 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..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"; Is the "based on GH7 SoC" part necessary? Does the "SSDK-GH7" not give that away? Thanks, Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-11 18:29 ` Mark Rutland @ 2014-03-12 4:31 ` Kukjin Kim -1 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-12 4:31 UTC (permalink / raw) To: 'Mark Rutland' Cc: linux-arm-kernel, linux-samsung-soc, 'Catalin Marinas', 'Thomas Abraham', 'Ilho Lee' Mark Rutland wrote: > Hi Mark, > On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > > 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 | 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 0000000..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/ 0x80000000 0x0C400000; > > 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 0xf8000000 and size is 128MB(0x8000000). > What address does the kernel end up getting loaded at on this board? > We load the kernel image from at 0x80080000 after changing the reserve memory area. > > + > > +/ { > > + 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"; > > 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@1C000000 { > > + 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 here because I thought the 'armv8-pmuv3' is enough. > > + 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. > > + > > + 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>; > > + }; > > These are both missing required clocks. > Yes right. > > + }; > > +}; > > 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..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"; > > 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. Thanks, Kukjin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-12 4:31 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-12 4:31 UTC (permalink / raw) To: linux-arm-kernel Mark Rutland wrote: > Hi Mark, > On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > > 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 | 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 0000000..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/ 0x80000000 0x0C400000; > > 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 0xf8000000 and size is 128MB(0x8000000). > What address does the kernel end up getting loaded at on this board? > We load the kernel image from at 0x80080000 after changing the reserve memory area. > > + > > +/ { > > + interrupt-parent = <&gic>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + aliases { > > + serial0 = "/amba/uart at 12c00000"; > > + }; > > + > > + cpus { > > + #address-cells = <2>; > > + #size-cells = <0>; > > + > > + cpu at 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 at 001 { > > + device_type = "cpu"; > > + compatible = "arm,armv8"; > > + reg = <0x0 0x001>; > > + enable-method = "spin-table"; > > + cpu-release-addr = <0x0 0x8000fff8>; > > + }; > > + cpu at 002 { > > + device_type = "cpu"; > > + compatible = "arm,armv8"; > > + reg = <0x0 0x002>; > > + enable-method = "spin-table"; > > + cpu-release-addr = <0x0 0x8000fff8>; > > + }; > > + cpu at 003 { > > + device_type = "cpu"; > > + compatible = "arm,armv8"; > > + reg = <0x0 0x003>; > > + enable-method = "spin-table"; > > + cpu-release-addr = <0x0 0x8000fff8>; > > + }; > > + }; > > + > > + gic: interrupt-controller at 1C000000 { > > + 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 here because I thought the 'armv8-pmuv3' is enough. > > + 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. > > + > > + amba { > > + compatible = "arm,amba-bus"; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + > > + serial at 12c00000 { > > + compatible = "arm,pl011", "arm,primecell"; > > + reg = <0 0x12c00000 0 0x10000>; > > + interrupts = <0 418 0>; > > + }; > > + > > + serial at 12c20000 { > > + compatible = "arm,pl011", "arm,primecell"; > > + reg = <0 0x12c20000 0 0x10000>; > > + interrupts = <0 420 0>; > > + }; > > These are both missing required clocks. > Yes right. > > + }; > > +}; > > 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..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"; > > 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. Thanks, Kukjin ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-12 4:31 ` Kukjin Kim @ 2014-03-13 10:33 ` Mark Rutland -1 siblings, 0 replies; 30+ messages in thread From: Mark Rutland @ 2014-03-13 10:33 UTC (permalink / raw) To: Kukjin Kim Cc: Catalin Marinas, 'Ilho Lee', linux-samsung-soc, 'Thomas Abraham', linux-arm-kernel On Wed, Mar 12, 2014 at 04:31:56AM +0000, Kukjin Kim wrote: > Mark Rutland wrote: > > > Hi Mark, > > > On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > > > 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 | 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 0000000..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/ 0x80000000 0x0C400000; > > > > 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 > 0xf8000000 and size is 128MB(0x8000000). Ok. > > What address does the kernel end up getting loaded at on this board? > > > We load the kernel image from at 0x80080000 after changing the reserve > memory area. Ok. > > > +/ { > > > + 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"; > > > > 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@1C000000 { > > > + 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). > > > + #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 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". > > > + 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. > > > + 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>; > > > + }; > > > > These are both missing required clocks. > > > Yes right. So you'll add these? > > > 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..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"; > > > > 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. Thanks, Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-13 10:33 ` Mark Rutland 0 siblings, 0 replies; 30+ messages in thread From: Mark Rutland @ 2014-03-13 10:33 UTC (permalink / raw) To: linux-arm-kernel On Wed, Mar 12, 2014 at 04:31:56AM +0000, Kukjin Kim wrote: > Mark Rutland wrote: > > > Hi Mark, > > > On Mon, Mar 10, 2014 at 10:51:17PM +0000, Kukjin Kim wrote: > > > 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 | 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 0000000..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/ 0x80000000 0x0C400000; > > > > 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 > 0xf8000000 and size is 128MB(0x8000000). Ok. > > What address does the kernel end up getting loaded at on this board? > > > We load the kernel image from at 0x80080000 after changing the reserve > memory area. Ok. > > > +/ { > > > + interrupt-parent = <&gic>; > > > + #address-cells = <2>; > > > + #size-cells = <2>; > > > + > > > + aliases { > > > + serial0 = "/amba/uart at 12c00000"; > > > + }; > > > + > > > + cpus { > > > + #address-cells = <2>; > > > + #size-cells = <0>; > > > + > > > + cpu at 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 at 001 { > > > + device_type = "cpu"; > > > + compatible = "arm,armv8"; > > > + reg = <0x0 0x001>; > > > + enable-method = "spin-table"; > > > + cpu-release-addr = <0x0 0x8000fff8>; > > > + }; > > > + cpu at 002 { > > > + device_type = "cpu"; > > > + compatible = "arm,armv8"; > > > + reg = <0x0 0x002>; > > > + enable-method = "spin-table"; > > > + cpu-release-addr = <0x0 0x8000fff8>; > > > + }; > > > + cpu at 003 { > > > + device_type = "cpu"; > > > + compatible = "arm,armv8"; > > > + reg = <0x0 0x003>; > > > + enable-method = "spin-table"; > > > + cpu-release-addr = <0x0 0x8000fff8>; > > > + }; > > > + }; > > > + > > > + gic: interrupt-controller at 1C000000 { > > > + 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). > > > + #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 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". > > > + 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. > > > + amba { > > > + compatible = "arm,amba-bus"; > > > + #address-cells = <2>; > > > + #size-cells = <2>; > > > + ranges; > > > + > > > + serial at 12c00000 { > > > + compatible = "arm,pl011", "arm,primecell"; > > > + reg = <0 0x12c00000 0 0x10000>; > > > + interrupts = <0 418 0>; > > > + }; > > > + > > > + serial at 12c20000 { > > > + compatible = "arm,pl011", "arm,primecell"; > > > + reg = <0 0x12c20000 0 0x10000>; > > > + interrupts = <0 420 0>; > > > + }; > > > > These are both missing required clocks. > > > Yes right. So you'll add these? > > > 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..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"; > > > > 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. Thanks, Mark. ^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-13 10:33 ` Mark Rutland @ 2014-03-14 1:26 ` Kukjin Kim -1 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-14 1:26 UTC (permalink / raw) To: 'Mark Rutland' Cc: linux-arm-kernel, linux-samsung-soc, 'Catalin Marinas', 'Thomas Abraham', 'Ilho Lee' Mark Rutland wrote: > [...] > > > > +/memreserve/ 0x80000000 0x0C400000; > > > > > > 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 > > 0xf8000000 and size is 128MB(0x8000000). > > Ok. > > > > What address does the kernel end up getting loaded at on this board? > > > > > We load the kernel image from at 0x80080000 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@1C000000 { > > > > + 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@12c20000 { > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > + reg = <0 0x12c20000 0 0x10000>; > > > > + 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. [...] > > > > + 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. > OK, agreed. So it should be: + model = "samsung,SSDK-GH7"; Thank, Kukjin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-14 1:26 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-14 1:26 UTC (permalink / raw) To: linux-arm-kernel Mark Rutland wrote: > [...] > > > > +/memreserve/ 0x80000000 0x0C400000; > > > > > > 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 > > 0xf8000000 and size is 128MB(0x8000000). > > Ok. > > > > What address does the kernel end up getting loaded at on this board? > > > > > We load the kernel image from at 0x80080000 after changing the reserve > > memory area. > > Ok. [...] > > > > + cpu at 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 at 1C000000 { > > > > + 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 at 12c20000 { > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > + reg = <0 0x12c20000 0 0x10000>; > > > > + 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. [...] > > > > + 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. > OK, agreed. So it should be: + model = "samsung,SSDK-GH7"; Thank, Kukjin ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-14 1:26 ` Kukjin Kim @ 2014-03-18 18:10 ` Mark Rutland -1 siblings, 0 replies; 30+ messages in thread From: Mark Rutland @ 2014-03-18 18:10 UTC (permalink / raw) To: Kukjin Kim Cc: Catalin Marinas, 'Ilho Lee', linux-samsung-soc, 'Thomas Abraham', linux-arm-kernel On Fri, Mar 14, 2014 at 01:26:31AM +0000, 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@1C000000 { > > > > > + 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@12c20000 { > > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > > + reg = <0 0x12c20000 0 0x10000>; > > > > > + 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-18 18:10 ` Mark Rutland 0 siblings, 0 replies; 30+ messages in thread From: Mark Rutland @ 2014-03-18 18:10 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 14, 2014 at 01:26:31AM +0000, Kukjin Kim wrote: > > > > > + cpu at 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 at 1C000000 { > > > > > + 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 at 12c20000 { > > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > > + reg = <0 0x12c20000 0 0x10000>; > > > > > + 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-18 18:10 ` Mark Rutland @ 2014-03-19 8:33 ` Kukjin Kim -1 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-19 8:33 UTC (permalink / raw) To: 'Mark Rutland' Cc: linux-arm-kernel, linux-samsung-soc, 'Catalin Marinas', 'Thomas Abraham', 'Ilho Lee' Mark Rutland wrote: > > On Fri, Mar 14, 2014 at 01:26:31AM +0000, 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@1C000000 { > > > > > > + 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@12c20000 { > > > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > > > + reg = <0 0x12c20000 0 0x10000>; > > > > > > + 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-19 8:33 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-19 8:33 UTC (permalink / raw) To: linux-arm-kernel Mark Rutland wrote: > > On Fri, Mar 14, 2014 at 01:26:31AM +0000, Kukjin Kim wrote: > > > > > > + cpu at 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 at 1C000000 { > > > > > > + 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 at 12c20000 { > > > > > > + compatible = "arm,pl011", "arm,primecell"; > > > > > > + reg = <0 0x12c20000 0 0x10000>; > > > > > > + 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board 2014-03-13 10:33 ` Mark Rutland @ 2014-03-14 12:48 ` Mark Brown -1 siblings, 0 replies; 30+ messages in thread From: Mark Brown @ 2014-03-14 12:48 UTC (permalink / raw) To: Mark Rutland Cc: Kukjin Kim, linux-arm-kernel, linux-samsung-soc, Catalin Marinas, 'Thomas Abraham', 'Ilho Lee' [-- Attachment #1: Type: text/plain, Size: 1247 bytes --] On Thu, Mar 13, 2014 at 10:33:29AM +0000, Mark Rutland wrote: > On Wed, Mar 12, 2014 at 04:31:56AM +0000, 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. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board @ 2014-03-14 12:48 ` Mark Brown 0 siblings, 0 replies; 30+ messages in thread From: Mark Brown @ 2014-03-14 12:48 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 13, 2014 at 10:33:29AM +0000, Mark Rutland wrote: > On Wed, Mar 12, 2014 at 04:31:56AM +0000, 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140314/14d82e38/attachment.sig> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default 2014-03-10 22:51 ` Kukjin Kim @ 2014-03-10 22:51 ` Kukjin Kim -1 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel, linux-samsung-soc Cc: Thomas Abraham, Ilho Lee, Catalin Marinas, 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 | 3 +++ arch/arm64/boot/dts/Makefile | 1 + arch/arm64/configs/defconfig | 1 + 3 files changed, 5 insertions(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index dd4327f..f0f2bf2 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -111,6 +111,9 @@ source "kernel/Kconfig.freezer" menu "Platform selection" +config ARCH_GH7 + bool + 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default @ 2014-03-10 22:51 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel 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 | 3 +++ arch/arm64/boot/dts/Makefile | 1 + arch/arm64/configs/defconfig | 1 + 3 files changed, 5 insertions(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index dd4327f..f0f2bf2 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -111,6 +111,9 @@ source "kernel/Kconfig.freezer" menu "Platform selection" +config ARCH_GH7 + bool + 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default 2014-03-10 22:51 ` Kukjin Kim @ 2014-03-11 12:01 ` Catalin Marinas -1 siblings, 0 replies; 30+ messages in thread From: Catalin Marinas @ 2014-03-11 12:01 UTC (permalink / raw) To: Kukjin Kim; +Cc: linux-arm-kernel, linux-samsung-soc, Thomas Abraham, Ilho Lee On Mon, Mar 10, 2014 at 10:51:18PM +0000, Kukjin Kim wrote: > 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 | 3 +++ > arch/arm64/boot/dts/Makefile | 1 + > arch/arm64/configs/defconfig | 1 + > 3 files changed, 5 insertions(+) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index dd4327f..f0f2bf2 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -111,6 +111,9 @@ source "kernel/Kconfig.freezer" > > menu "Platform selection" > > +config ARCH_GH7 > + bool You should add some text here so that people can disable it. The aim is single Image by default but I think we should let people disable certain platforms for more targeted deployments. -- Catalin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default @ 2014-03-11 12:01 ` Catalin Marinas 0 siblings, 0 replies; 30+ messages in thread From: Catalin Marinas @ 2014-03-11 12:01 UTC (permalink / raw) To: linux-arm-kernel On Mon, Mar 10, 2014 at 10:51:18PM +0000, Kukjin Kim wrote: > 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 | 3 +++ > arch/arm64/boot/dts/Makefile | 1 + > arch/arm64/configs/defconfig | 1 + > 3 files changed, 5 insertions(+) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index dd4327f..f0f2bf2 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -111,6 +111,9 @@ source "kernel/Kconfig.freezer" > > menu "Platform selection" > > +config ARCH_GH7 > + bool You should add some text here so that people can disable it. The aim is single Image by default but I think we should let people disable certain platforms for more targeted deployments. -- Catalin ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default 2014-03-11 12:01 ` Catalin Marinas @ 2014-03-11 22:45 ` Kukjin Kim -1 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-11 22:45 UTC (permalink / raw) To: Catalin Marinas Cc: Kukjin Kim, Ilho Lee, linux-samsung-soc, Thomas Abraham, linux-arm-kernel On 03/11/14 21:01, Catalin Marinas wrote: > On Mon, Mar 10, 2014 at 10:51:18PM +0000, Kukjin Kim wrote: >> 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 | 3 +++ >> arch/arm64/boot/dts/Makefile | 1 + >> arch/arm64/configs/defconfig | 1 + >> 3 files changed, 5 insertions(+) >> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index dd4327f..f0f2bf2 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -111,6 +111,9 @@ source "kernel/Kconfig.freezer" >> >> menu "Platform selection" >> >> +config ARCH_GH7 >> + bool > > You should add some text here so that people can disable it. The aim is > single Image by default but I think we should let people disable certain > platforms for more targeted deployments. > OK, I'll add and re-submit with Cc'ing dt mailing list. Thanks, Kukjin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default @ 2014-03-11 22:45 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-11 22:45 UTC (permalink / raw) To: linux-arm-kernel On 03/11/14 21:01, Catalin Marinas wrote: > On Mon, Mar 10, 2014 at 10:51:18PM +0000, Kukjin Kim wrote: >> 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 | 3 +++ >> arch/arm64/boot/dts/Makefile | 1 + >> arch/arm64/configs/defconfig | 1 + >> 3 files changed, 5 insertions(+) >> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index dd4327f..f0f2bf2 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -111,6 +111,9 @@ source "kernel/Kconfig.freezer" >> >> menu "Platform selection" >> >> +config ARCH_GH7 >> + bool > > You should add some text here so that people can disable it. The aim is > single Image by default but I think we should let people disable certain > platforms for more targeted deployments. > OK, I'll add and re-submit with Cc'ing dt mailing list. Thanks, Kukjin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board 2014-03-10 22:51 ` Kukjin Kim @ 2014-03-10 22:51 ` Kukjin Kim -1 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel, linux-samsung-soc Cc: Thomas Abraham, Ilho Lee, Catalin Marinas, 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board @ 2014-03-10 22:51 ` Kukjin Kim 0 siblings, 0 replies; 30+ messages in thread From: Kukjin Kim @ 2014-03-10 22:51 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board 2014-03-10 22:51 ` Kukjin Kim @ 2014-03-11 12:02 ` Catalin Marinas -1 siblings, 0 replies; 30+ messages in thread From: Catalin Marinas @ 2014-03-11 12:02 UTC (permalink / raw) To: Kukjin Kim; +Cc: linux-arm-kernel, linux-samsung-soc, Thomas Abraham, Ilho Lee On Mon, Mar 10, 2014 at 10:51:19PM +0000, Kukjin Kim wrote: > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com> > Reviewed-by: Thomas Abraham <thomas.ab@samsung.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> Same here, please cc the devicetree list. -- Catalin ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board @ 2014-03-11 12:02 ` Catalin Marinas 0 siblings, 0 replies; 30+ messages in thread From: Catalin Marinas @ 2014-03-11 12:02 UTC (permalink / raw) To: linux-arm-kernel On Mon, Mar 10, 2014 at 10:51:19PM +0000, Kukjin Kim wrote: > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com> > Reviewed-by: Thomas Abraham <thomas.ab@samsung.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> Same here, please cc the devicetree list. -- Catalin ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2014-03-19 8:33 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-03-10 22:51 [PATCH v2 0/3] arm64: add new support Samsung GH7 SoC and SSDK board Kukjin Kim 2014-03-10 22:51 ` Kukjin Kim 2014-03-10 22:51 ` [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board Kukjin Kim 2014-03-10 22:51 ` Kukjin Kim 2014-03-11 11:59 ` Catalin Marinas 2014-03-11 11:59 ` Catalin Marinas 2014-03-11 18:29 ` Mark Rutland 2014-03-11 18:29 ` Mark Rutland 2014-03-12 4:31 ` Kukjin Kim 2014-03-12 4:31 ` Kukjin Kim 2014-03-13 10:33 ` Mark Rutland 2014-03-13 10:33 ` Mark Rutland 2014-03-14 1:26 ` Kukjin Kim 2014-03-14 1:26 ` Kukjin Kim 2014-03-18 18:10 ` Mark Rutland 2014-03-18 18:10 ` Mark Rutland 2014-03-19 8:33 ` Kukjin Kim 2014-03-19 8:33 ` Kukjin Kim 2014-03-14 12:48 ` Mark Brown 2014-03-14 12:48 ` Mark Brown 2014-03-10 22:51 ` [PATCH v2 2/3] arm64: defconfig: Enable ARCH_GH7 by default Kukjin Kim 2014-03-10 22:51 ` Kukjin Kim 2014-03-11 12:01 ` Catalin Marinas 2014-03-11 12:01 ` Catalin Marinas 2014-03-11 22:45 ` Kukjin Kim 2014-03-11 22:45 ` Kukjin Kim 2014-03-10 22:51 ` [PATCH v2 3/3] Documentation: DT: add new entry for Samsung GH7 SoC and SSDK board Kukjin Kim 2014-03-10 22:51 ` Kukjin Kim 2014-03-11 12:02 ` Catalin Marinas 2014-03-11 12:02 ` Catalin Marinas
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.