diff mbox

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

Message ID 1392100183-30930-2-git-send-email-kgene.kim@samsung.com
State New
Headers show

Commit Message

Kukjin Kim Feb. 11, 2014, 6:29 a.m. UTC
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     |  108 ++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/samsung-ssdk-gh7.dts |   26 +++++++
 2 files changed, 134 insertions(+)
 create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi
 create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts

Comments

Marc Zyngier Feb. 12, 2014, 11:13 a.m. UTC | #1
On 11/02/14 23:36, Olof Johansson wrote:
> Hi,
> 
> Besides what Mark Rutland already commented on:
> 
> On Mon, Feb 10, 2014 at 10:29 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>> +/ {
>> +       model = "SAMSUNG GH7";
>> +       compatible = "samsung,gh7";
> 
> Model and compatible in the dtsi should probably always be overridden
> by a dts that includes it, so there's little use in having it here.
> 
>> +       interrupt-parent = <&gic>;
>> +       #address-cells = <2>;
>> +       #size-cells = <2>;
>> +
>> +       cpus {
>> +               #address-cells = <2>;
>> +               #size-cells = <0>;
>> +
>> +               cpu@000 {
>> +                       device_type = "cpu";
>> +                       compatible = "arm,armv8";
>> +                       reg = <0x0 0x000>;
> 
> No need to zero-pad cpu numbers in unit address or reg.
> 
>> +                       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", "arm,cortex-a9-gic";
> 
> This looks incorrect -- you should at the very least have a more
> specific one than a15-gic? Marc?

"arm,cortex-a9-gic" is definitely wrong (the A9 GIC doesn't have the
virt extensions). This binding matches what the A15 GIC has, so
"arm,cortex-a15-gic" is probably fine. Main issue here is that the GICv2
driver has no compatible string for anything else.

Should we define something more generic (like "arm,gic-v2")? Or carry on
adding more compatible strings?

	M.

>> +               #interrupt-cells = <3>;
>> +               #address-cells = <0>;
>> +               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 */
>> +       };
Kukjin Kim Feb. 24, 2014, 11:56 p.m. UTC | #2
On 02/18/14 19:30, Mark Rutland wrote:
> On Tue, Feb 11, 2014 at 03:16:26AM +0000, Kukjin Kim wrote:
>> On 02/12/14 03:15, Mark Rutland wrote:
>>> On Tue, Feb 11, 2014 at 06:29:41AM +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     |  108 ++++++++++++++++++++++++++++++
>>>>    arch/arm64/boot/dts/samsung-ssdk-gh7.dts |   26 +++++++
>>>>    2 files changed, 134 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..5b8785c
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
>>>> @@ -0,0 +1,108 @@
>>>> +/*
>>>> + * 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.
>>>> +*/
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +/memreserve/ 0x80000000 0x0C400000;
>>>
>>> That looks _very_ large. What is this for?
>>>
>> Yes, I know but we need to reserve that for EL3 monitor, UEFI services,
>> secure, hypervisor and scan chanin...
>
> OK. How much of that memory does the kernel need to know about then?
>
> Surely the kernel shouldn't be able to map the EL3 monitor or hypervisor
> at all?
>
> What address is the kernel getting loaded at if everything up to
> 0x8C400000 isn't usable?
>
> [...]
>
>>
>>> [...]
>>>
>>>> +	amba {
>>>> +		compatible = "arm,amba-bus";
>>>> +		#address-cells =<1>;
>>>> +		#size-cells =<1>;
>>>> +		ranges;
>>>> +
>>>> +		serial@12c00000 {
>>>> +			compatible = "arm,pl011", "arm,primecell";
>>>> +			reg =<0x12c00000 0x10000>;
>>>> +			interrupts =<418>;
>>>> +		};
>>>> +
>>>> +		serial@12c20000 {
>>>> +			compatible = "arm,pl011", "arm,primecell";
>>>> +			reg =<0x12c20000 0x10000>;
>>>> +			interrupts =<420>;
>>>> +		};
>>>
>>> Don't these need clocks?
>>>
>> We don't need and the clocks will be handled by bootloader...
>
> While that might be sufficient for the device to function, Linux doesn't
> know that from this DT, and as far as I can see the device can't
> possibly probe, as no clocks are provided through platform data:
>
> of_platform_bus_create will call of_amba_device_create for anyting
> compatible with "arm,primecell". This in turn will call amba_device_add,
> which will call amba_get_enable_pclk. Then clk_get(&pcdev->dev,
> "apb_pclk") should fail, amba_device_add should fail, and
> of_platform_bus_create will stop trying to probe the node.
> of_platform_populate will carry on probing other nodes.
>
> Surely the pl011 nodes at least need "apb_pclk"?
>
You're right. We should make a dummy clock for pl011, proper clocks will 
be added in next version.

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

Patch

diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi
new file mode 100644
index 0000000..5b8785c
--- /dev/null
+++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
@@ -0,0 +1,108 @@ 
+/*
+ * 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.
+*/
+
+/dts-v1/;
+
+/memreserve/ 0x80000000 0x0C400000;
+
+/ {
+	model = "SAMSUNG GH7";
+	compatible = "samsung,gh7";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	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", "arm,cortex-a9-gic";
+		#interrupt-cells = <3>;
+		#address-cells = <0>;
+		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 */
+		clock-frequency = <100000000>;
+	};
+
+	pmu {
+		compatible = "arm,armv8-pmuv3";
+		interrupts = <0 294 8>,
+			     <0 295 8>,
+			     <0 296 8>,
+			     <0 297 8>,
+			     <0 298 8>,
+			     <0 299 8>,
+			     <0 300 8>,
+			     <0 301 8>;
+	};
+
+	amba {
+		compatible = "arm,amba-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		serial@12c00000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x12c00000 0x10000>;
+			interrupts = <418>;
+		};
+
+		serial@12c20000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x12c20000 0x10000>;
+			interrupts = <420>;
+		};
+	};
+};
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>;
+	};
+};