[2/4] ARM: DT: Add a basic dts file for SMDKV310 machine

Message ID 1296998250-21856-3-git-send-email-thomas.abraham@linaro.org
State New
Headers show

Commit Message

thomas.abraham@linaro.org Feb. 6, 2011, 1:17 p.m.
This patch adds a basic dts file for Samsung's SMDKV310 machine.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts

Comments

David Gibson Feb. 7, 2011, 12:04 a.m. | #1
On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
> This patch adds a basic dts file for Samsung's SMDKV310 machine.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>  1 files changed, 38 insertions(+), 0 deletions(-)
>  create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
> 
> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> new file mode 100755
> index 0000000..74d80bf
> --- /dev/null
> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> @@ -0,0 +1,38 @@
> +/dts-v1/;
> +
> +/ {
> +	model = "smdkv310";
> +	compatible = "samsung,smdkv310";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x40000000 0x08000000>;
> +	};

Uh.. where are the cpus?

> +	chosen {
> +		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";

It's generally a good idea to list the specific soc model before "simple-bus".

> +		ranges = <0x00000000 0x00000000 0xFFFFFFFF>;
> +
> +		GIC:gic@0x10500000 {
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0x10500000 0x1000>;
> +			compatible = "arm,gic";
> +		};
> +
> +		watchdog@0x10060000 {
> +			reg = <0x10060000 0x400>;
> +			interrupts = <552>;
> +			interrupt-parent = <&GIC>;
> +			compatible = "samsung,s3c2410-wdt";
> +		};
> +	};
> +};
Grant Likely Feb. 7, 2011, 6:54 a.m. | #2
On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
> This patch adds a basic dts file for Samsung's SMDKV310 machine.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>  1 files changed, 38 insertions(+), 0 deletions(-)
>  create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
> 
> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> new file mode 100755
> index 0000000..74d80bf
> --- /dev/null
> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> @@ -0,0 +1,38 @@
> +/dts-v1/;
> +
> +/ {
> +	model = "smdkv310";
> +	compatible = "samsung,smdkv310";

Should include the soc in this compatible list:

	compatible = "samsung,smdkv310", "samsung,s5pv310";

Also, every 'compatible' value specified in this file should also have
documentation in Documentation/devicetree/bindings.

> +	#address-cells = <1>;
> +	#size-cells = <1>;

It's usually a good idea to have a "interrupt-parent = <&GIC>;" here
so that all child nodes inherit it as the default interrupt
controller.

Also, missing a 'cpus' node.

> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x40000000 0x08000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";

initrd base address will get passed via the linux,initrd-start and
linux,initrd-end properties which are automatically populated by
u-boot.  Drop initrd from the boot args.

> +	};
> +
> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";

Should also specify the soc with something like:

		compatible = "samsung,s5pv310-soc", "simple-bus";

> +		ranges = <0x00000000 0x00000000 0xFFFFFFFF>;

You can specify that the entire ranges is translated simply by
specifying an empty ranges property like so:

		ranges;

> +
> +		GIC:gic@0x10500000 {
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0x10500000 0x1000>;
> +			compatible = "arm,gic";

Be specific about the device on all the device nodes, followed by a
list of the compatible devices.  Should be something like:

	compatible = "samsung,s5pv310-soc", "arm,gic";

> +		};
> +
> +		watchdog@0x10060000 {
> +			reg = <0x10060000 0x400>;
> +			interrupts = <552>;
> +			interrupt-parent = <&GIC>;

By putting the interrupt-parent property in the root node, this
interrupt-parent property can be dropped.

> +			compatible = "samsung,s3c2410-wdt";

compatible needs to first specify the exact device
(samsung,s5pv310-wdt?) followed by the compatible device.  I assume
the wdt in the s5pv310 is compatible with the one in the s3c2410,
correct?  Or am I missing something?

> +		};
> +	};
> +};
> -- 
> 1.6.6.rc2
> 
> 
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
Rob Herring Feb. 7, 2011, 7:24 p.m. | #3
David, Thomas,

On 02/06/2011 06:04 PM, David Gibson wrote:
> On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
>> This patch adds a basic dts file for Samsung's SMDKV310 machine.
>>
>> Signed-off-by: Thomas Abraham<thomas.abraham@linaro.org>
>> ---
>>   arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>>   1 files changed, 38 insertions(+), 0 deletions(-)
>>   create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
>>
>> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>> new file mode 100755
>> index 0000000..74d80bf
>> --- /dev/null
>> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>> @@ -0,0 +1,38 @@
>> +/dts-v1/;
>> +
>> +/ {
>> +	model = "smdkv310";
>> +	compatible = "samsung,smdkv310";
>> +	#address-cells =<1>;
>> +	#size-cells =<1>;
>> +
>> +	memory {
>> +		device_type = "memory";
>> +		reg =<0x40000000 0x08000000>;
>> +	};
>
> Uh.. where are the cpus?
>

But for ARM, all the details of the cpu are probe-able. So what would we 
gain by putting cpu info in the DTS?

>> +	chosen {
>> +		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
>> +	};
>> +
>> +	soc {
>> +		#address-cells =<1>;
>> +		#size-cells =<1>;
>> +		compatible = "simple-bus";
>
> It's generally a good idea to list the specific soc model before "simple-bus".
>
>> +		ranges =<0x00000000 0x00000000 0xFFFFFFFF>;

For no translation, you can do just:

ranges;

Rob
Meador Inge Feb. 7, 2011, 7:34 p.m. | #4
On 02/07/2011 01:24 PM, Rob Herring wrote:
> David, Thomas,
>
> On 02/06/2011 06:04 PM, David Gibson wrote:
>> On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
>>> This patch adds a basic dts file for Samsung's SMDKV310 machine.
>>>
>>> Signed-off-by: Thomas Abraham<thomas.abraham@linaro.org>
>>> ---
>>>    arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
>>>    1 files changed, 38 insertions(+), 0 deletions(-)
>>>    create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
>>>
>>> diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>>> new file mode 100755
>>> index 0000000..74d80bf
>>> --- /dev/null
>>> +++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
>>> @@ -0,0 +1,38 @@
>>> +/dts-v1/;
>>> +
>>> +/ {
>>> +	model = "smdkv310";
>>> +	compatible = "samsung,smdkv310";
>>> +	#address-cells =<1>;
>>> +	#size-cells =<1>;
>>> +
>>> +	memory {
>>> +		device_type = "memory";
>>> +		reg =<0x40000000 0x08000000>;
>>> +	};
>>
>> Uh.. where are the cpus?
>>
>
> But for ARM, all the details of the cpu are probe-able. So what would we
> gain by putting cpu info in the DTS?

Perhaps there are use cases where the system designer wants to lock in a 
certain configuration.  Hardware partitioning is one such case.  For 
example, say you have a multicore ARM platform and you only want your 
kernel to know about certain cpus.  One way to do that is by pulling the 
cpus you don't want the kernel to know about out of the device tree.
Grant Likely Feb. 13, 2011, 6:11 a.m. | #5
On Mon, Feb 07, 2011 at 01:24:12PM -0600, Rob Herring wrote:
> David, Thomas,
> 
> On 02/06/2011 06:04 PM, David Gibson wrote:
> >On Sun, Feb 06, 2011 at 06:47:28PM +0530, Thomas Abraham wrote:
> >>This patch adds a basic dts file for Samsung's SMDKV310 machine.
> >>
> >>Signed-off-by: Thomas Abraham<thomas.abraham@linaro.org>
> >>---
> >>  arch/arm/mach-s5pv310/mach-smdkv310.dts |   38 +++++++++++++++++++++++++++++++
> >>  1 files changed, 38 insertions(+), 0 deletions(-)
> >>  create mode 100755 arch/arm/mach-s5pv310/mach-smdkv310.dts
> >>
> >>diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> >>new file mode 100755
> >>index 0000000..74d80bf
> >>--- /dev/null
> >>+++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
> >>@@ -0,0 +1,38 @@
> >>+/dts-v1/;
> >>+
> >>+/ {
> >>+	model = "smdkv310";
> >>+	compatible = "samsung,smdkv310";
> >>+	#address-cells =<1>;
> >>+	#size-cells =<1>;
> >>+
> >>+	memory {
> >>+		device_type = "memory";
> >>+		reg =<0x40000000 0x08000000>;
> >>+	};
> >
> >Uh.. where are the cpus?
> >
> 
> But for ARM, all the details of the cpu are probe-able. So what
> would we gain by putting cpu info in the DTS?

You can detect what cpus are present, but you cannot necessarily
detect if all cpus are available (for instance in a multicore AMP
system where some cores run a different operating environment.  It is
also always good practice to have the structure of the device tree
fully populated even if the OS doesn't currently use it because it
provides points that (for instance) cpu affinity or non-probeable
configuration can be hung off of.

Patch

diff --git a/arch/arm/mach-s5pv310/mach-smdkv310.dts b/arch/arm/mach-s5pv310/mach-smdkv310.dts
new file mode 100755
index 0000000..74d80bf
--- /dev/null
+++ b/arch/arm/mach-s5pv310/mach-smdkv310.dts
@@ -0,0 +1,38 @@ 
+/dts-v1/;
+
+/ {
+	model = "smdkv310";
+	compatible = "samsung,smdkv310";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	memory {
+		device_type = "memory";
+		reg = <0x40000000 0x08000000>;
+	};
+
+	chosen {
+		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		ranges = <0x00000000 0x00000000 0xFFFFFFFF>;
+
+		GIC:gic@0x10500000 {
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0x10500000 0x1000>;
+			compatible = "arm,gic";
+		};
+
+		watchdog@0x10060000 {
+			reg = <0x10060000 0x400>;
+			interrupts = <552>;
+			interrupt-parent = <&GIC>;
+			compatible = "samsung,s3c2410-wdt";
+		};
+	};
+};