[PATCH 0/2] Adding DTB's to architecture independent vmlinux

[PATCH 0/2] Adding DTB's to architecture independent vmlinux

[dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w]

From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch set adds support for linking an arbitrary number of device
tree blobs into the kernel image and allow passing the "compatible"
string for the platform on the kernel command line.

Patch 1/2:
modifies asm-generic/vmlinux.lds.h to add linking .dtb.init.rodata
sections into the .init.data section of the vmlinux image. modifies
scripts/Makefile.lib to add a kbuild command to compile DTS files to
device tree blobs and a rule to create objects to wrap the blobs for
linking into vmlinux. 

A DTB is linked in by adding the DTB object to the list of objects to
be linked into vmlinux in the archtecture specific Makefile using
   obj-y += dts/foo.dtb.o
   $(obj)/%.dtb: $(src)/%.dts
       $(call if_changed,dtc)
The location of the device tree DTS files is architecture specific.

Patch 2/2:
Adds a kernel command line option "dtb_compat=<string>" and functions
for architecture/platform specific code to retrieve the command line
string and locate the compatible DTB linked into the kernel
e.g.
    compat_string = of_get_dtb_compatible_string();
    compat_dtb = of_find_compatible_dtb(compat_string);

Changes from V4:
Patch to device tree compiler dropped, alignment done in DTB wrapper
object code.

Name of DTB section changed to .dtb.init.rodata from .dtb

Code cleanup in of_find_compatible_dtb() to remove redundant code and
improve readabiility.

Patch to arch/x86/kernel Makefile dropped from this set.
  

Dirk Brandewie (2):
  of: Add support for linking device tree blobs into vmlinux
  of/fdt: add kernel command line option for dtb_compat string

 Documentation/kernel-parameters.txt |    7 ++++
 drivers/of/fdt.c                    |   55 +++++++++++++++++++++++++++++++++++
 include/asm-generic/vmlinux.lds.h   |   19 +++++++++++-
 include/linux/of_fdt.h              |    5 +++
 scripts/Makefile.lib                |   17 +++++++++++
 5 files changed, 101 insertions(+), 2 deletions(-)

-- 
1.7.2.3

[PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w]

From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch adds support for linking device tree blobs into
vmlinux. The device tree blobs are placed in the init.data
section.

Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
 scripts/Makefile.lib              |   17 +++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index bd69d79..ea671e7 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -67,7 +67,14 @@
  * Align to a 32 byte boundary equal to the
  * alignment gcc 4.5 uses for a struct
  */
-#define STRUCT_ALIGN() . = ALIGN(32)
+#define STRUCT_ALIGNMENT 32
+#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
+
+/* Device tree blobs linked into the kernel need to have proper
+ * structure alignment to be parsed by the flat device tree library
+ * used in early boot
+*/
+#define DTB_ALIGNMENT STRUCT_ALIGNMENT
 
 /* The actual configuration determine if the init/exit sections
  * are handled as text/data or they can be discarded (which
@@ -146,6 +153,13 @@
 #define TRACE_SYSCALLS()
 #endif
 
+
+#define KERNEL_DTB()							\
+	. = ALIGN(DTB_ALIGNMENT);					\
+	VMLINUX_SYMBOL(__dtb_start) = .;				\
+	*(.dtb.init.rodata)						\
+	VMLINUX_SYMBOL(__dtb_end) = .;
+
 /* .data section */
 #define DATA_DATA							\
 	*(.data)							\
@@ -468,7 +482,8 @@
 	MCOUNT_REC()							\
 	DEV_DISCARD(init.rodata)					\
 	CPU_DISCARD(init.rodata)					\
-	MEM_DISCARD(init.rodata)
+	MEM_DISCARD(init.rodata)					\
+	KERNEL_DTB()
 
 #define INIT_TEXT							\
 	*(.init.text)							\
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 4c72c11..a8a4774 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -200,6 +200,23 @@ quiet_cmd_gzip = GZIP    $@
 cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
 	(rm -f $@ ; false)
 
+# DTC
+#  ---------------------------------------------------------------------------
+$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
+	@echo '#include <asm-generic/vmlinux.lds.h>' > $@
+	@echo '.section .dtb.init.rodata,"a"' >> $@
+	@echo '.balign DTB_ALIGNMENT' >> $@
+	@echo '.global __dtb_$(*F)_begin' >> $@
+	@echo '__dtb_$(*F)_begin:' >> $@
+	@echo '.incbin "$<" ' >> $@
+	@echo '__dtb_$(*F)_end:' >> $@
+	@echo '.global __dtb_$(*F)_end' >> $@
+	@echo '.balign DTB_ALIGNMENT' >> $@
+
+DTC = $(objtree)/scripts/dtc/dtc
+
+quiet_cmd_dtc = DTC	$@
+      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(src)/$*.dts
 
 # Bzip2
 # ---------------------------------------------------------------------------
-- 
1.7.2.3

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Grant Likely]

On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This patch adds support for linking device tree blobs into
> vmlinux. The device tree blobs are placed in the init.data
> section.
> 
> Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
>  scripts/Makefile.lib              |   17 +++++++++++++++++
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index bd69d79..ea671e7 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -67,7 +67,14 @@
>   * Align to a 32 byte boundary equal to the
>   * alignment gcc 4.5 uses for a struct
>   */
> -#define STRUCT_ALIGN() . = ALIGN(32)
> +#define STRUCT_ALIGNMENT 32
> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
> +
> +/* Device tree blobs linked into the kernel need to have proper
> + * structure alignment to be parsed by the flat device tree library
> + * used in early boot
> +*/
> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>  
>  /* The actual configuration determine if the init/exit sections
>   * are handled as text/data or they can be discarded (which
> @@ -146,6 +153,13 @@
>  #define TRACE_SYSCALLS()
>  #endif
>  
> +
> +#define KERNEL_DTB()							\
> +	. = ALIGN(DTB_ALIGNMENT);					\
> +	VMLINUX_SYMBOL(__dtb_start) = .;				\
> +	*(.dtb.init.rodata)						\
> +	VMLINUX_SYMBOL(__dtb_end) = .;
> +
>  /* .data section */
>  #define DATA_DATA							\
>  	*(.data)							\
> @@ -468,7 +482,8 @@
>  	MCOUNT_REC()							\
>  	DEV_DISCARD(init.rodata)					\
>  	CPU_DISCARD(init.rodata)					\
> -	MEM_DISCARD(init.rodata)
> +	MEM_DISCARD(init.rodata)					\
> +	KERNEL_DTB()
>  
>  #define INIT_TEXT							\
>  	*(.init.text)							\
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 4c72c11..a8a4774 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -200,6 +200,23 @@ quiet_cmd_gzip = GZIP    $@
>  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
>  	(rm -f $@ ; false)
>  
> +# DTC
> +#  ---------------------------------------------------------------------------
> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
> +	@echo '#include <asm-generic/vmlinux.lds.h>' > $@
> +	@echo '.section .dtb.init.rodata,"a"' >> $@
> +	@echo '.balign DTB_ALIGNMENT' >> $@
> +	@echo '.global __dtb_$(*F)_begin' >> $@
> +	@echo '__dtb_$(*F)_begin:' >> $@
> +	@echo '.incbin "$<" ' >> $@
> +	@echo '__dtb_$(*F)_end:' >> $@
> +	@echo '.global __dtb_$(*F)_end' >> $@
> +	@echo '.balign DTB_ALIGNMENT' >> $@
> +
> +DTC = $(objtree)/scripts/dtc/dtc
> +
> +quiet_cmd_dtc = DTC	$@
> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(src)/$*.dts

Missing the %.dtb: %.dts rule, but otherwise looks okay.  You will
need to make sure this doesn't break powerpc or microblaze when the
dts->dtb rule is added.

g.

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Dirk Brandewie]

On 11/15/2010 08:41 PM, Grant Likely wrote:
> On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> From: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> This patch adds support for linking device tree blobs into
>> vmlinux. The device tree blobs are placed in the init.data
>> section.
>>
>> Signed-off-by: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
>>   scripts/Makefile.lib              |   17 +++++++++++++++++
>>   2 files changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> index bd69d79..ea671e7 100644
>> --- a/include/asm-generic/vmlinux.lds.h
>> +++ b/include/asm-generic/vmlinux.lds.h
>> @@ -67,7 +67,14 @@
>>    * Align to a 32 byte boundary equal to the
>>    * alignment gcc 4.5 uses for a struct
>>    */
>> -#define STRUCT_ALIGN() . = ALIGN(32)
>> +#define STRUCT_ALIGNMENT 32
>> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
>> +
>> +/* Device tree blobs linked into the kernel need to have proper
>> + * structure alignment to be parsed by the flat device tree library
>> + * used in early boot
>> +*/
>> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>>
>>   /* The actual configuration determine if the init/exit sections
>>    * are handled as text/data or they can be discarded (which
>> @@ -146,6 +153,13 @@
>>   #define TRACE_SYSCALLS()
>>   #endif
>>
>> +
>> +#define KERNEL_DTB()							\
>> +	. = ALIGN(DTB_ALIGNMENT);					\
>> +	VMLINUX_SYMBOL(__dtb_start) = .;				\
>> +	*(.dtb.init.rodata)						\
>> +	VMLINUX_SYMBOL(__dtb_end) = .;
>> +
>>   /* .data section */
>>   #define DATA_DATA							\
>>   	*(.data)							\
>> @@ -468,7 +482,8 @@
>>   	MCOUNT_REC()							\
>>   	DEV_DISCARD(init.rodata)					\
>>   	CPU_DISCARD(init.rodata)					\
>> -	MEM_DISCARD(init.rodata)
>> +	MEM_DISCARD(init.rodata)					\
>> +	KERNEL_DTB()
>>
>>   #define INIT_TEXT							\
>>   	*(.init.text)							\
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 4c72c11..a8a4774 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -200,6 +200,23 @@ quiet_cmd_gzip = GZIP    $@
>>   cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9>  $@) || \
>>   	(rm -f $@ ; false)
>>
>> +# DTC
>> +#  ---------------------------------------------------------------------------
>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>> +	@echo '#include<asm-generic/vmlinux.lds.h>'>  $@
>> +	@echo '.section .dtb.init.rodata,"a"'>>  $@
>> +	@echo '.balign DTB_ALIGNMENT'>>  $@
>> +	@echo '.global __dtb_$(*F)_begin'>>  $@
>> +	@echo '__dtb_$(*F)_begin:'>>  $@
>> +	@echo '.incbin "$<" '>>  $@
>> +	@echo '__dtb_$(*F)_end:'>>  $@
>> +	@echo '.global __dtb_$(*F)_end'>>  $@
>> +	@echo '.balign DTB_ALIGNMENT'>>  $@
>> +
>> +DTC = $(objtree)/scripts/dtc/dtc
>> +
>> +quiet_cmd_dtc = DTC	$@
>> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(src)/$*.dts
>
> Missing the %.dtb: %.dts rule, but otherwise looks okay.  You will
> need to make sure this doesn't break powerpc or microblaze when the
> dts->dtb rule is added.
>
I have the rule
   $(obj)/%.dtb: $(src)/%.dts
	  $(call if_changed,dtc)
in the arch/x86/kernel/Makefile to prevent this sneaking into other other 
architectures.

I need some more skilled in kbuild to help craft the more generic rule so we can 
have the dts files anywhere in the arch/<*>/ directory structure and be able to 
find the correct dts files.

--Dirk

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Grant Likely]

On Mon, Nov 15, 2010 at 10:06 PM, Dirk Brandewie
<dirk.brandewie@gmail.com> wrote:
> On 11/15/2010 08:41 PM, Grant Likely wrote:
>>
>> On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie@gmail.com wrote=
:
>>>
>>> From: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>
>>> This patch adds support for linking device tree blobs into
>>> vmlinux. The device tree blobs are placed in the init.data
>>> section.
>>>
>>> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com>
>>> ---
>>> =A0include/asm-generic/vmlinux.lds.h | =A0 19 +++++++++++++++++--
>>> =A0scripts/Makefile.lib =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 17 +++++++++++=
++++++
>>> =A02 files changed, 34 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/asm-generic/vmlinux.lds.h
>>> b/include/asm-generic/vmlinux.lds.h
>>> index bd69d79..ea671e7 100644
>>> --- a/include/asm-generic/vmlinux.lds.h
>>> +++ b/include/asm-generic/vmlinux.lds.h
>>> @@ -67,7 +67,14 @@
>>> =A0 * Align to a 32 byte boundary equal to the
>>> =A0 * alignment gcc 4.5 uses for a struct
>>> =A0 */
>>> -#define STRUCT_ALIGN() . =3D ALIGN(32)
>>> +#define STRUCT_ALIGNMENT 32
>>> +#define STRUCT_ALIGN() . =3D ALIGN(STRUCT_ALIGNMENT)
>>> +
>>> +/* Device tree blobs linked into the kernel need to have proper
>>> + * structure alignment to be parsed by the flat device tree library
>>> + * used in early boot
>>> +*/
>>> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>>>
>>> =A0/* The actual configuration determine if the init/exit sections
>>> =A0 * are handled as text/data or they can be discarded (which
>>> @@ -146,6 +153,13 @@
>>> =A0#define TRACE_SYSCALLS()
>>> =A0#endif
>>>
>>> +
>>> +#define KERNEL_DTB() =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>>> + =A0 =A0 =A0 . =3D ALIGN(DTB_ALIGNMENT); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>>> + =A0 =A0 =A0 VMLINUX_SYMBOL(__dtb_start) =3D .; =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>>> + =A0 =A0 =A0 *(.dtb.init.rodata) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>>> + =A0 =A0 =A0 VMLINUX_SYMBOL(__dtb_end) =3D .;
>>> +
>>> =A0/* .data section */
>>> =A0#define DATA_DATA =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>>> =A0 =A0 =A0 =A0*(.data) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>>> @@ -468,7 +482,8 @@
>>> =A0 =A0 =A0 =A0MCOUNT_REC() =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>>> =A0 =A0 =A0 =A0DEV_DISCARD(init.rodata) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>>> =A0 =A0 =A0 =A0CPU_DISCARD(init.rodata) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>>> - =A0 =A0 =A0 MEM_DISCARD(init.rodata)
>>> + =A0 =A0 =A0 MEM_DISCARD(init.rodata) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>>> + =A0 =A0 =A0 KERNEL_DTB()
>>>
>>> =A0#define INIT_TEXT =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>>> =A0 =A0 =A0 =A0*(.init.text) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>> index 4c72c11..a8a4774 100644
>>> --- a/scripts/Makefile.lib
>>> +++ b/scripts/Makefile.lib
>>> @@ -200,6 +200,23 @@ quiet_cmd_gzip =3D GZIP =A0 =A0$@
>>> =A0cmd_gzip =3D (cat $(filter-out FORCE,$^) | gzip -f -9> =A0$@) || \
>>> =A0 =A0 =A0 =A0(rm -f $@ ; false)
>>>
>>> +# DTC
>>> +#
>>> =A0--------------------------------------------------------------------=
-------
>>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>>> + =A0 =A0 =A0 @echo '#include<asm-generic/vmlinux.lds.h>'> =A0$@
>>> + =A0 =A0 =A0 @echo '.section .dtb.init.rodata,"a"'>> =A0$@
>>> + =A0 =A0 =A0 @echo '.balign DTB_ALIGNMENT'>> =A0$@
>>> + =A0 =A0 =A0 @echo '.global __dtb_$(*F)_begin'>> =A0$@
>>> + =A0 =A0 =A0 @echo '__dtb_$(*F)_begin:'>> =A0$@
>>> + =A0 =A0 =A0 @echo '.incbin "$<" '>> =A0$@
>>> + =A0 =A0 =A0 @echo '__dtb_$(*F)_end:'>> =A0$@
>>> + =A0 =A0 =A0 @echo '.global __dtb_$(*F)_end'>> =A0$@
>>> + =A0 =A0 =A0 @echo '.balign DTB_ALIGNMENT'>> =A0$@
>>> +
>>> +DTC =3D $(objtree)/scripts/dtc/dtc
>>> +
>>> +quiet_cmd_dtc =3D DTC =A0 =A0$@
>>> + =A0 =A0 =A0cmd_dtc =3D $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 =A0$(src)/=
$*.dts
>>
>> Missing the %.dtb: %.dts rule, but otherwise looks okay. =A0You will
>> need to make sure this doesn't break powerpc or microblaze when the
>> dts->dtb rule is added.
>>
> I have the rule
> =A0$(obj)/%.dtb: $(src)/%.dts
> =A0 =A0 =A0 =A0 =A0$(call if_changed,dtc)
> in the arch/x86/kernel/Makefile to prevent this sneaking into other other
> architectures.

This rule looks correct.  PowerPC and Microblaze need to be modified
to use it.  It should not be hard to do, give it a try.  Worst case,
your first attempt is wrong and the rest of us fix it up.  :-)

Hint: PowerPC currently puts the .dtb file in a different directory
from the source .dts file.  It doesn't need to do it that way.
arch/powerpc/boot/Makefile will need to be modified.

g.

>
> I need some more skilled in kbuild to help craft the more generic rule so=
 we
> can have the dts files anywhere in the arch/<*>/ directory structure and =
be
> able to find the correct dts files.
>
> --Dirk
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Grant Likely]

On Mon, Nov 15, 2010 at 10:06 PM, Dirk Brandewie
<dirk.brandewie@gmail.com> wrote:
> On 11/15/2010 08:41 PM, Grant Likely wrote:
>>
>> On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie@gmail.com wrote:
>>>
>>> From: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>
>>> This patch adds support for linking device tree blobs into
>>> vmlinux. The device tree blobs are placed in the init.data
>>> section.
>>>
>>> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com>
>>> ---
>>>  include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
>>>  scripts/Makefile.lib              |   17 +++++++++++++++++
>>>  2 files changed, 34 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/asm-generic/vmlinux.lds.h
>>> b/include/asm-generic/vmlinux.lds.h
>>> index bd69d79..ea671e7 100644
>>> --- a/include/asm-generic/vmlinux.lds.h
>>> +++ b/include/asm-generic/vmlinux.lds.h
>>> @@ -67,7 +67,14 @@
>>>   * Align to a 32 byte boundary equal to the
>>>   * alignment gcc 4.5 uses for a struct
>>>   */
>>> -#define STRUCT_ALIGN() . = ALIGN(32)
>>> +#define STRUCT_ALIGNMENT 32
>>> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
>>> +
>>> +/* Device tree blobs linked into the kernel need to have proper
>>> + * structure alignment to be parsed by the flat device tree library
>>> + * used in early boot
>>> +*/
>>> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>>>
>>>  /* The actual configuration determine if the init/exit sections
>>>   * are handled as text/data or they can be discarded (which
>>> @@ -146,6 +153,13 @@
>>>  #define TRACE_SYSCALLS()
>>>  #endif
>>>
>>> +
>>> +#define KERNEL_DTB()                                                   \
>>> +       . = ALIGN(DTB_ALIGNMENT);                                       \
>>> +       VMLINUX_SYMBOL(__dtb_start) = .;                                \
>>> +       *(.dtb.init.rodata)                                             \
>>> +       VMLINUX_SYMBOL(__dtb_end) = .;
>>> +
>>>  /* .data section */
>>>  #define DATA_DATA                                                     \
>>>        *(.data)                                                        \
>>> @@ -468,7 +482,8 @@
>>>        MCOUNT_REC()                                                    \
>>>        DEV_DISCARD(init.rodata)                                        \
>>>        CPU_DISCARD(init.rodata)                                        \
>>> -       MEM_DISCARD(init.rodata)
>>> +       MEM_DISCARD(init.rodata)                                        \
>>> +       KERNEL_DTB()
>>>
>>>  #define INIT_TEXT                                                     \
>>>        *(.init.text)                                                   \
>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>> index 4c72c11..a8a4774 100644
>>> --- a/scripts/Makefile.lib
>>> +++ b/scripts/Makefile.lib
>>> @@ -200,6 +200,23 @@ quiet_cmd_gzip = GZIP    $@
>>>  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9>  $@) || \
>>>        (rm -f $@ ; false)
>>>
>>> +# DTC
>>> +#
>>>  ---------------------------------------------------------------------------
>>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>>> +       @echo '#include<asm-generic/vmlinux.lds.h>'>  $@
>>> +       @echo '.section .dtb.init.rodata,"a"'>>  $@
>>> +       @echo '.balign DTB_ALIGNMENT'>>  $@
>>> +       @echo '.global __dtb_$(*F)_begin'>>  $@
>>> +       @echo '__dtb_$(*F)_begin:'>>  $@
>>> +       @echo '.incbin "$<" '>>  $@
>>> +       @echo '__dtb_$(*F)_end:'>>  $@
>>> +       @echo '.global __dtb_$(*F)_end'>>  $@
>>> +       @echo '.balign DTB_ALIGNMENT'>>  $@
>>> +
>>> +DTC = $(objtree)/scripts/dtc/dtc
>>> +
>>> +quiet_cmd_dtc = DTC    $@
>>> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(src)/$*.dts
>>
>> Missing the %.dtb: %.dts rule, but otherwise looks okay.  You will
>> need to make sure this doesn't break powerpc or microblaze when the
>> dts->dtb rule is added.
>>
> I have the rule
>  $(obj)/%.dtb: $(src)/%.dts
>          $(call if_changed,dtc)
> in the arch/x86/kernel/Makefile to prevent this sneaking into other other
> architectures.

This rule looks correct.  PowerPC and Microblaze need to be modified
to use it.  It should not be hard to do, give it a try.  Worst case,
your first attempt is wrong and the rest of us fix it up.  :-)

Hint: PowerPC currently puts the .dtb file in a different directory
from the source .dts file.  It doesn't need to do it that way.
arch/powerpc/boot/Makefile will need to be modified.

g.

>
> I need some more skilled in kbuild to help craft the more generic rule so we
> can have the dts files anywhere in the arch/<*>/ directory structure and be
> able to find the correct dts files.
>
> --Dirk
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Dirk Brandewie]

On 11/15/2010 09:17 PM, Grant Likely wrote:
> On Mon, Nov 15, 2010 at 10:06 PM, Dirk Brandewie
> <dirk.brandewie@gmail.com>  wrote:
>> On 11/15/2010 08:41 PM, Grant Likely wrote:
>>>
>>> On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie@gmail.com wrote:
>>>>
>>>> From: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>>
>>>> This patch adds support for linking device tree blobs into
>>>> vmlinux. The device tree blobs are placed in the init.data
>>>> section.
>>>>
>>>> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>> ---
>>>>   include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
>>>>   scripts/Makefile.lib              |   17 +++++++++++++++++
>>>>   2 files changed, 34 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/asm-generic/vmlinux.lds.h
>>>> b/include/asm-generic/vmlinux.lds.h
>>>> index bd69d79..ea671e7 100644
>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>> @@ -67,7 +67,14 @@
>>>>    * Align to a 32 byte boundary equal to the
>>>>    * alignment gcc 4.5 uses for a struct
>>>>    */
>>>> -#define STRUCT_ALIGN() . = ALIGN(32)
>>>> +#define STRUCT_ALIGNMENT 32
>>>> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
>>>> +
>>>> +/* Device tree blobs linked into the kernel need to have proper
>>>> + * structure alignment to be parsed by the flat device tree library
>>>> + * used in early boot
>>>> +*/
>>>> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>>>>
>>>>   /* The actual configuration determine if the init/exit sections
>>>>    * are handled as text/data or they can be discarded (which
>>>> @@ -146,6 +153,13 @@
>>>>   #define TRACE_SYSCALLS()
>>>>   #endif
>>>>
>>>> +
>>>> +#define KERNEL_DTB()                                                   \
>>>> +       . = ALIGN(DTB_ALIGNMENT);                                       \
>>>> +       VMLINUX_SYMBOL(__dtb_start) = .;                                \
>>>> +       *(.dtb.init.rodata)                                             \
>>>> +       VMLINUX_SYMBOL(__dtb_end) = .;
>>>> +
>>>>   /* .data section */
>>>>   #define DATA_DATA                                                     \
>>>>         *(.data)                                                        \
>>>> @@ -468,7 +482,8 @@
>>>>         MCOUNT_REC()                                                    \
>>>>         DEV_DISCARD(init.rodata)                                        \
>>>>         CPU_DISCARD(init.rodata)                                        \
>>>> -       MEM_DISCARD(init.rodata)
>>>> +       MEM_DISCARD(init.rodata)                                        \
>>>> +       KERNEL_DTB()
>>>>
>>>>   #define INIT_TEXT                                                     \
>>>>         *(.init.text)                                                   \
>>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>>> index 4c72c11..a8a4774 100644
>>>> --- a/scripts/Makefile.lib
>>>> +++ b/scripts/Makefile.lib
>>>> @@ -200,6 +200,23 @@ quiet_cmd_gzip = GZIP    $@
>>>>   cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9>    $@) || \
>>>>         (rm -f $@ ; false)
>>>>
>>>> +# DTC
>>>> +#
>>>>   ---------------------------------------------------------------------------
>>>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>>>> +       @echo '#include<asm-generic/vmlinux.lds.h>'>    $@
>>>> +       @echo '.section .dtb.init.rodata,"a"'>>    $@
>>>> +       @echo '.balign DTB_ALIGNMENT'>>    $@
>>>> +       @echo '.global __dtb_$(*F)_begin'>>    $@
>>>> +       @echo '__dtb_$(*F)_begin:'>>    $@
>>>> +       @echo '.incbin "$<" '>>    $@
>>>> +       @echo '__dtb_$(*F)_end:'>>    $@
>>>> +       @echo '.global __dtb_$(*F)_end'>>    $@
>>>> +       @echo '.balign DTB_ALIGNMENT'>>    $@
>>>> +
>>>> +DTC = $(objtree)/scripts/dtc/dtc
>>>> +
>>>> +quiet_cmd_dtc = DTC    $@
>>>> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(src)/$*.dts
>>>
>>> Missing the %.dtb: %.dts rule, but otherwise looks okay.  You will
>>> need to make sure this doesn't break powerpc or microblaze when the
>>> dts->dtb rule is added.
>>>
>> I have the rule
>>   $(obj)/%.dtb: $(src)/%.dts
>>           $(call if_changed,dtc)
>> in the arch/x86/kernel/Makefile to prevent this sneaking into other other
>> architectures.
>
> This rule looks correct.  PowerPC and Microblaze need to be modified
> to use it.  It should not be hard to do, give it a try.  Worst case,
> your first attempt is wrong and the rest of us fix it up.  :-)
>
> Hint: PowerPC currently puts the .dtb file in a different directory
> from the source .dts file.  It doesn't need to do it that way.
> arch/powerpc/boot/Makefile will need to be modified.
>

I will give it a shot.  The only real difference except for the directory 
structures is powerpc and microblaze add padding to the dtb with the -p 1024
command line argument to dtc.  Is the padding needed when the blob are linked 
into vmlinux proper?

--Dirk

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Dirk Brandewie]

On 11/15/2010 09:17 PM, Grant Likely wrote:
> On Mon, Nov 15, 2010 at 10:06 PM, Dirk Brandewie
> <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>  wrote:
>> On 11/15/2010 08:41 PM, Grant Likely wrote:
>>>
>>> On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>>
>>>> From: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> This patch adds support for linking device tree blobs into
>>>> vmlinux. The device tree blobs are placed in the init.data
>>>> section.
>>>>
>>>> Signed-off-by: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>   include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
>>>>   scripts/Makefile.lib              |   17 +++++++++++++++++
>>>>   2 files changed, 34 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/asm-generic/vmlinux.lds.h
>>>> b/include/asm-generic/vmlinux.lds.h
>>>> index bd69d79..ea671e7 100644
>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>> @@ -67,7 +67,14 @@
>>>>    * Align to a 32 byte boundary equal to the
>>>>    * alignment gcc 4.5 uses for a struct
>>>>    */
>>>> -#define STRUCT_ALIGN() . = ALIGN(32)
>>>> +#define STRUCT_ALIGNMENT 32
>>>> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
>>>> +
>>>> +/* Device tree blobs linked into the kernel need to have proper
>>>> + * structure alignment to be parsed by the flat device tree library
>>>> + * used in early boot
>>>> +*/
>>>> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>>>>
>>>>   /* The actual configuration determine if the init/exit sections
>>>>    * are handled as text/data or they can be discarded (which
>>>> @@ -146,6 +153,13 @@
>>>>   #define TRACE_SYSCALLS()
>>>>   #endif
>>>>
>>>> +
>>>> +#define KERNEL_DTB()                                                   \
>>>> +       . = ALIGN(DTB_ALIGNMENT);                                       \
>>>> +       VMLINUX_SYMBOL(__dtb_start) = .;                                \
>>>> +       *(.dtb.init.rodata)                                             \
>>>> +       VMLINUX_SYMBOL(__dtb_end) = .;
>>>> +
>>>>   /* .data section */
>>>>   #define DATA_DATA                                                     \
>>>>         *(.data)                                                        \
>>>> @@ -468,7 +482,8 @@
>>>>         MCOUNT_REC()                                                    \
>>>>         DEV_DISCARD(init.rodata)                                        \
>>>>         CPU_DISCARD(init.rodata)                                        \
>>>> -       MEM_DISCARD(init.rodata)
>>>> +       MEM_DISCARD(init.rodata)                                        \
>>>> +       KERNEL_DTB()
>>>>
>>>>   #define INIT_TEXT                                                     \
>>>>         *(.init.text)                                                   \
>>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>>> index 4c72c11..a8a4774 100644
>>>> --- a/scripts/Makefile.lib
>>>> +++ b/scripts/Makefile.lib
>>>> @@ -200,6 +200,23 @@ quiet_cmd_gzip = GZIP    $@
>>>>   cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9>    $@) || \
>>>>         (rm -f $@ ; false)
>>>>
>>>> +# DTC
>>>> +#
>>>>   ---------------------------------------------------------------------------
>>>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>>>> +       @echo '#include<asm-generic/vmlinux.lds.h>'>    $@
>>>> +       @echo '.section .dtb.init.rodata,"a"'>>    $@
>>>> +       @echo '.balign DTB_ALIGNMENT'>>    $@
>>>> +       @echo '.global __dtb_$(*F)_begin'>>    $@
>>>> +       @echo '__dtb_$(*F)_begin:'>>    $@
>>>> +       @echo '.incbin "$<" '>>    $@
>>>> +       @echo '__dtb_$(*F)_end:'>>    $@
>>>> +       @echo '.global __dtb_$(*F)_end'>>    $@
>>>> +       @echo '.balign DTB_ALIGNMENT'>>    $@
>>>> +
>>>> +DTC = $(objtree)/scripts/dtc/dtc
>>>> +
>>>> +quiet_cmd_dtc = DTC    $@
>>>> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(src)/$*.dts
>>>
>>> Missing the %.dtb: %.dts rule, but otherwise looks okay.  You will
>>> need to make sure this doesn't break powerpc or microblaze when the
>>> dts->dtb rule is added.
>>>
>> I have the rule
>>   $(obj)/%.dtb: $(src)/%.dts
>>           $(call if_changed,dtc)
>> in the arch/x86/kernel/Makefile to prevent this sneaking into other other
>> architectures.
>
> This rule looks correct.  PowerPC and Microblaze need to be modified
> to use it.  It should not be hard to do, give it a try.  Worst case,
> your first attempt is wrong and the rest of us fix it up.  :-)
>
> Hint: PowerPC currently puts the .dtb file in a different directory
> from the source .dts file.  It doesn't need to do it that way.
> arch/powerpc/boot/Makefile will need to be modified.
>

I will give it a shot.  The only real difference except for the directory 
structures is powerpc and microblaze add padding to the dtb with the -p 1024
command line argument to dtc.  Is the padding needed when the blob are linked 
into vmlinux proper?

--Dirk

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Grant Likely]

On Mon, Nov 15, 2010 at 10:28 PM, Dirk Brandewie
<dirk.brandewie@gmail.com> wrote:
> On 11/15/2010 09:17 PM, Grant Likely wrote:
>>
>> On Mon, Nov 15, 2010 at 10:06 PM, Dirk Brandewie
>> <dirk.brandewie@gmail.com> =A0wrote:
>>>
>>> On 11/15/2010 08:41 PM, Grant Likely wrote:
>>>>
>>>> On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie@gmail.com
>>>> wrote:
>>>>>
>>>>> From: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>>>
>>>>> This patch adds support for linking device tree blobs into
>>>>> vmlinux. The device tree blobs are placed in the init.data
>>>>> section.
>>>>>
>>>>> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>>> ---
>>>>> =A0include/asm-generic/vmlinux.lds.h | =A0 19 +++++++++++++++++--
>>>>> =A0scripts/Makefile.lib =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 17 +++++++++=
++++++++
>>>>> =A02 files changed, 34 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/asm-generic/vmlinux.lds.h
>>>>> b/include/asm-generic/vmlinux.lds.h
>>>>> index bd69d79..ea671e7 100644
>>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>>> @@ -67,7 +67,14 @@
>>>>> =A0 * Align to a 32 byte boundary equal to the
>>>>> =A0 * alignment gcc 4.5 uses for a struct
>>>>> =A0 */
>>>>> -#define STRUCT_ALIGN() . =3D ALIGN(32)
>>>>> +#define STRUCT_ALIGNMENT 32
>>>>> +#define STRUCT_ALIGN() . =3D ALIGN(STRUCT_ALIGNMENT)
>>>>> +
>>>>> +/* Device tree blobs linked into the kernel need to have proper
>>>>> + * structure alignment to be parsed by the flat device tree library
>>>>> + * used in early boot
>>>>> +*/
>>>>> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>>>>>
>>>>> =A0/* The actual configuration determine if the init/exit sections
>>>>> =A0 * are handled as text/data or they can be discarded (which
>>>>> @@ -146,6 +153,13 @@
>>>>> =A0#define TRACE_SYSCALLS()
>>>>> =A0#endif
>>>>>
>>>>> +
>>>>> +#define KERNEL_DTB()
>>>>> \
>>>>> + =A0 =A0 =A0 . =3D ALIGN(DTB_ALIGNMENT);
>>>>> \
>>>>> + =A0 =A0 =A0 VMLINUX_SYMBOL(__dtb_start) =3D .;
>>>>> =A0\
>>>>> + =A0 =A0 =A0 *(.dtb.init.rodata)
>>>>> \
>>>>> + =A0 =A0 =A0 VMLINUX_SYMBOL(__dtb_end) =3D .;
>>>>> +
>>>>> =A0/* .data section */
>>>>> =A0#define DATA_DATA
>>>>> \
>>>>> =A0 =A0 =A0 =A0*(.data)
>>>>> =A0\
>>>>> @@ -468,7 +482,8 @@
>>>>> =A0 =A0 =A0 =A0MCOUNT_REC()
>>>>> =A0\
>>>>> =A0 =A0 =A0 =A0DEV_DISCARD(init.rodata)
>>>>> =A0\
>>>>> =A0 =A0 =A0 =A0CPU_DISCARD(init.rodata)
>>>>> =A0\
>>>>> - =A0 =A0 =A0 MEM_DISCARD(init.rodata)
>>>>> + =A0 =A0 =A0 MEM_DISCARD(init.rodata)
>>>>> =A0\
>>>>> + =A0 =A0 =A0 KERNEL_DTB()
>>>>>
>>>>> =A0#define INIT_TEXT
>>>>> \
>>>>> =A0 =A0 =A0 =A0*(.init.text)
>>>>> \
>>>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>>>> index 4c72c11..a8a4774 100644
>>>>> --- a/scripts/Makefile.lib
>>>>> +++ b/scripts/Makefile.lib
>>>>> @@ -200,6 +200,23 @@ quiet_cmd_gzip =3D GZIP =A0 =A0$@
>>>>> =A0cmd_gzip =3D (cat $(filter-out FORCE,$^) | gzip -f -9> =A0 =A0$@) =
|| \
>>>>> =A0 =A0 =A0 =A0(rm -f $@ ; false)
>>>>>
>>>>> +# DTC
>>>>> +#
>>>>>
>>>>> =A0------------------------------------------------------------------=
---------
>>>>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>>>>> + =A0 =A0 =A0 @echo '#include<asm-generic/vmlinux.lds.h>'> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '.section .dtb.init.rodata,"a"'>> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '.balign DTB_ALIGNMENT'>> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '.global __dtb_$(*F)_begin'>> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '__dtb_$(*F)_begin:'>> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '.incbin "$<" '>> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '__dtb_$(*F)_end:'>> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '.global __dtb_$(*F)_end'>> =A0 =A0$@
>>>>> + =A0 =A0 =A0 @echo '.balign DTB_ALIGNMENT'>> =A0 =A0$@
>>>>> +
>>>>> +DTC =3D $(objtree)/scripts/dtc/dtc
>>>>> +
>>>>> +quiet_cmd_dtc =3D DTC =A0 =A0$@
>>>>> + =A0 =A0 =A0cmd_dtc =3D $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 =A0$(src=
)/$*.dts
>>>>
>>>> Missing the %.dtb: %.dts rule, but otherwise looks okay. =A0You will
>>>> need to make sure this doesn't break powerpc or microblaze when the
>>>> dts->dtb rule is added.
>>>>
>>> I have the rule
>>> =A0$(obj)/%.dtb: $(src)/%.dts
>>> =A0 =A0 =A0 =A0 =A0$(call if_changed,dtc)
>>> in the arch/x86/kernel/Makefile to prevent this sneaking into other oth=
er
>>> architectures.
>>
>> This rule looks correct. =A0PowerPC and Microblaze need to be modified
>> to use it. =A0It should not be hard to do, give it a try. =A0Worst case,
>> your first attempt is wrong and the rest of us fix it up. =A0:-)
>>
>> Hint: PowerPC currently puts the .dtb file in a different directory
>> from the source .dts file. =A0It doesn't need to do it that way.
>> arch/powerpc/boot/Makefile will need to be modified.
>>
>
> I will give it a shot. =A0The only real difference except for the directo=
ry
> structures is powerpc and microblaze add padding to the dtb with the -p 1=
024
> command line argument to dtc.

You could use a $(DTCFLAGS) variable to pass in arch specific flags.

> =A0Is the padding needed when the blob are
> linked into vmlinux proper?

Maybe, maybe not.  Padding is required to be able to modify the .dtb
data in-place, which is important when adapting data from other
sources into the device tree structure.  However, when it is linked
into the kernel, the adding of additional data /possibly/ can be
deferred until after the tree is either copied or unflattened.

g.

Re: [PATCH 1/2] of: Add support for linking device tree blobs into vmlinux

[Grant Likely]

On Mon, Nov 15, 2010 at 10:28 PM, Dirk Brandewie
<dirk.brandewie@gmail.com> wrote:
> On 11/15/2010 09:17 PM, Grant Likely wrote:
>>
>> On Mon, Nov 15, 2010 at 10:06 PM, Dirk Brandewie
>> <dirk.brandewie@gmail.com>  wrote:
>>>
>>> On 11/15/2010 08:41 PM, Grant Likely wrote:
>>>>
>>>> On Mon, Nov 15, 2010 at 08:01:20PM -0800, dirk.brandewie@gmail.com
>>>> wrote:
>>>>>
>>>>> From: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>>>
>>>>> This patch adds support for linking device tree blobs into
>>>>> vmlinux. The device tree blobs are placed in the init.data
>>>>> section.
>>>>>
>>>>> Signed-off-by: Dirk Brandewie<dirk.brandewie@gmail.com>
>>>>> ---
>>>>>  include/asm-generic/vmlinux.lds.h |   19 +++++++++++++++++--
>>>>>  scripts/Makefile.lib              |   17 +++++++++++++++++
>>>>>  2 files changed, 34 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/asm-generic/vmlinux.lds.h
>>>>> b/include/asm-generic/vmlinux.lds.h
>>>>> index bd69d79..ea671e7 100644
>>>>> --- a/include/asm-generic/vmlinux.lds.h
>>>>> +++ b/include/asm-generic/vmlinux.lds.h
>>>>> @@ -67,7 +67,14 @@
>>>>>   * Align to a 32 byte boundary equal to the
>>>>>   * alignment gcc 4.5 uses for a struct
>>>>>   */
>>>>> -#define STRUCT_ALIGN() . = ALIGN(32)
>>>>> +#define STRUCT_ALIGNMENT 32
>>>>> +#define STRUCT_ALIGN() . = ALIGN(STRUCT_ALIGNMENT)
>>>>> +
>>>>> +/* Device tree blobs linked into the kernel need to have proper
>>>>> + * structure alignment to be parsed by the flat device tree library
>>>>> + * used in early boot
>>>>> +*/
>>>>> +#define DTB_ALIGNMENT STRUCT_ALIGNMENT
>>>>>
>>>>>  /* The actual configuration determine if the init/exit sections
>>>>>   * are handled as text/data or they can be discarded (which
>>>>> @@ -146,6 +153,13 @@
>>>>>  #define TRACE_SYSCALLS()
>>>>>  #endif
>>>>>
>>>>> +
>>>>> +#define KERNEL_DTB()
>>>>> \
>>>>> +       . = ALIGN(DTB_ALIGNMENT);
>>>>> \
>>>>> +       VMLINUX_SYMBOL(__dtb_start) = .;
>>>>>  \
>>>>> +       *(.dtb.init.rodata)
>>>>> \
>>>>> +       VMLINUX_SYMBOL(__dtb_end) = .;
>>>>> +
>>>>>  /* .data section */
>>>>>  #define DATA_DATA
>>>>> \
>>>>>        *(.data)
>>>>>  \
>>>>> @@ -468,7 +482,8 @@
>>>>>        MCOUNT_REC()
>>>>>  \
>>>>>        DEV_DISCARD(init.rodata)
>>>>>  \
>>>>>        CPU_DISCARD(init.rodata)
>>>>>  \
>>>>> -       MEM_DISCARD(init.rodata)
>>>>> +       MEM_DISCARD(init.rodata)
>>>>>  \
>>>>> +       KERNEL_DTB()
>>>>>
>>>>>  #define INIT_TEXT
>>>>> \
>>>>>        *(.init.text)
>>>>> \
>>>>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>>>>> index 4c72c11..a8a4774 100644
>>>>> --- a/scripts/Makefile.lib
>>>>> +++ b/scripts/Makefile.lib
>>>>> @@ -200,6 +200,23 @@ quiet_cmd_gzip = GZIP    $@
>>>>>  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9>    $@) || \
>>>>>        (rm -f $@ ; false)
>>>>>
>>>>> +# DTC
>>>>> +#
>>>>>
>>>>>  ---------------------------------------------------------------------------
>>>>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>>>>> +       @echo '#include<asm-generic/vmlinux.lds.h>'>    $@
>>>>> +       @echo '.section .dtb.init.rodata,"a"'>>    $@
>>>>> +       @echo '.balign DTB_ALIGNMENT'>>    $@
>>>>> +       @echo '.global __dtb_$(*F)_begin'>>    $@
>>>>> +       @echo '__dtb_$(*F)_begin:'>>    $@
>>>>> +       @echo '.incbin "$<" '>>    $@
>>>>> +       @echo '__dtb_$(*F)_end:'>>    $@
>>>>> +       @echo '.global __dtb_$(*F)_end'>>    $@
>>>>> +       @echo '.balign DTB_ALIGNMENT'>>    $@
>>>>> +
>>>>> +DTC = $(objtree)/scripts/dtc/dtc
>>>>> +
>>>>> +quiet_cmd_dtc = DTC    $@
>>>>> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(src)/$*.dts
>>>>
>>>> Missing the %.dtb: %.dts rule, but otherwise looks okay.  You will
>>>> need to make sure this doesn't break powerpc or microblaze when the
>>>> dts->dtb rule is added.
>>>>
>>> I have the rule
>>>  $(obj)/%.dtb: $(src)/%.dts
>>>          $(call if_changed,dtc)
>>> in the arch/x86/kernel/Makefile to prevent this sneaking into other other
>>> architectures.
>>
>> This rule looks correct.  PowerPC and Microblaze need to be modified
>> to use it.  It should not be hard to do, give it a try.  Worst case,
>> your first attempt is wrong and the rest of us fix it up.  :-)
>>
>> Hint: PowerPC currently puts the .dtb file in a different directory
>> from the source .dts file.  It doesn't need to do it that way.
>> arch/powerpc/boot/Makefile will need to be modified.
>>
>
> I will give it a shot.  The only real difference except for the directory
> structures is powerpc and microblaze add padding to the dtb with the -p 1024
> command line argument to dtc.

You could use a $(DTCFLAGS) variable to pass in arch specific flags.

>  Is the padding needed when the blob are
> linked into vmlinux proper?

Maybe, maybe not.  Padding is required to be able to modify the .dtb
data in-place, which is important when adapting data from other
sources into the device tree structure.  However, when it is linked
into the kernel, the adding of additional data /possibly/ can be
deferred until after the tree is either copied or unflattened.

g.

[PATCH 2/2] of/fdt: add kernel command line option for dtb_compat string

[dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w]

From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Add support for specifying a "compatible" string from the kernel
command line and functions for the platform to find the compatible
blob present in the kernel image if any.

Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/kernel-parameters.txt |    7 ++++
 drivers/of/fdt.c                    |   55 +++++++++++++++++++++++++++++++++++
 include/linux/of_fdt.h              |    5 +++
 3 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index ed45e98..f9b77fa 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -655,6 +655,13 @@ and is between 256 and 4096 characters. It is defined in the file
 
 	dscc4.setup=	[NET]
 
+	dtb_compat=	[KNL]
+			Specify the "compatible" string for the device
+			tree blob present in the kernel image.  This
+			string will be used to select the first device
+			tree blob whose compatible property matches
+			the string passed on the kernel commandline.
+
 	dynamic_printk	Enables pr_debug()/dev_dbg() calls if
 			CONFIG_DYNAMIC_PRINTK_DEBUG has been enabled.
 			These can also be switched on/off via
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index c1360e0..77ab091 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -15,6 +15,8 @@
 #include <linux/of_fdt.h>
 #include <linux/string.h>
 #include <linux/errno.h>
+#include <asm-generic/vmlinux.lds.h>
+
 
 #ifdef CONFIG_PPC
 #include <asm/machdep.h>
@@ -604,3 +606,56 @@ void __init unflatten_device_tree(void)
 
 	pr_debug(" <- unflatten_device_tree()\n");
 }
+
+#ifndef MODULE
+#ifdef CONFIG_OF_FLATTREE
+char dtb_compat_name[MAX_DTB_COMPAT_STR] = "";
+
+char __init *of_get_dtb_compatible_string(void)
+{
+	return dtb_compat_name;
+}
+
+
+extern uint8_t __dtb_start[];
+extern uint8_t __dtb_end[];
+void *of_find_compatible_dtb(char *name)
+{
+	void *rc = NULL;
+	unsigned long root, size;
+	struct boot_param_header  *orig_initial_boot_params;
+	uint8_t *blob;
+
+	orig_initial_boot_params = initial_boot_params;
+	blob = __dtb_start;
+	initial_boot_params = (struct boot_param_header *)blob;
+
+	do {
+		root = of_get_flat_dt_root();
+		if (of_flat_dt_is_compatible(root, name) > 0) {
+			rc = (void *) blob;
+			break;
+		}
+
+		size =  be32_to_cpu(initial_boot_params->totalsize);
+		blob = (uint8_t *) ALIGN(((unsigned long)blob + size),
+					DTB_ALIGNMENT);
+		initial_boot_params = (struct boot_param_header *)blob;
+	} while (blob < __dtb_end);
+
+	if (rc == NULL)
+		initial_boot_params = orig_initial_boot_params;
+	return rc;
+}
+
+
+static int __init dtb_compat_setup(char *line)
+{
+	strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
+	return 1;
+}
+
+early_param("dtb_compat", dtb_compat_setup);
+#endif
+#endif
+
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 7bbf5b3..c696da3c 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -58,6 +58,8 @@ struct boot_param_header {
 };
 
 #if defined(CONFIG_OF_FLATTREE)
+#define MAX_DTB_COMPAT_STR 64
+
 /* TBD: Temporary export of fdt globals - remove when code fully merged */
 extern int __initdata dt_root_addr_cells;
 extern int __initdata dt_root_size_cells;
@@ -82,6 +84,9 @@ extern void early_init_dt_add_memory_arch(u64 base, u64 size);
 extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
 extern u64 dt_mem_next_cell(int s, __be32 **cellp);
 
+extern char *of_get_dtb_compatible_string(void);
+extern void *of_find_compatible_dtb(char *name);
+
 /*
  * If BLK_DEV_INITRD, the fdt early init code will call this function,
  * to be provided by the arch code. start and end are specified as
-- 
1.7.2.3

Re: [PATCH 2/2] of/fdt: add kernel command line option for dtb_compat string

[Grant Likely]

On Mon, Nov 15, 2010 at 08:01:21PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Add support for specifying a "compatible" string from the kernel
> command line and functions for the platform to find the compatible
> blob present in the kernel image if any.
> 
> Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Dirk,

a couple of general comments:

A lot of the information in the cover letter should really be here in
the actual patch description.  Use the cover letter to provide context
for the entire series, but put the specifics with each patch.  Version
changelog history should also appear in patch description, not the
cover letter.

For the next version, please cc: linux-kernel-u79uwXL29TaiAVqoAR/hOA@public.gmane.org
This series should have a wider review audience, particularly because
it touches the linker sections and the kernel parameters.  You should
also cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org and
microblaze-uclinux-rVRm/Wmeqae7NGdpmJTKYQ@public.gmane.org for the patch that adds the common
rules because it will affect both of those architectures directly.

More comments below.

> ---
>  Documentation/kernel-parameters.txt |    7 ++++
>  drivers/of/fdt.c                    |   55 +++++++++++++++++++++++++++++++++++
>  include/linux/of_fdt.h              |    5 +++
>  3 files changed, 67 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index ed45e98..f9b77fa 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -655,6 +655,13 @@ and is between 256 and 4096 characters. It is defined in the file
>  
>  	dscc4.setup=	[NET]
>  
> +	dtb_compat=	[KNL]
> +			Specify the "compatible" string for the device
> +			tree blob present in the kernel image.  This
> +			string will be used to select the first device
> +			tree blob whose compatible property matches
> +			the string passed on the kernel commandline.
> +
>  	dynamic_printk	Enables pr_debug()/dev_dbg() calls if
>  			CONFIG_DYNAMIC_PRINTK_DEBUG has been enabled.
>  			These can also be switched on/off via
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c1360e0..77ab091 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -15,6 +15,8 @@
>  #include <linux/of_fdt.h>
>  #include <linux/string.h>
>  #include <linux/errno.h>
> +#include <asm-generic/vmlinux.lds.h>
> +
>  
>  #ifdef CONFIG_PPC
>  #include <asm/machdep.h>
> @@ -604,3 +606,56 @@ void __init unflatten_device_tree(void)
>  
>  	pr_debug(" <- unflatten_device_tree()\n");
>  }
> +
> +#ifndef MODULE

fdt.c cannot be compiled as a module.  Is this necessary?

> +#ifdef CONFIG_OF_FLATTREE

Unnecessary #ifdef.  fdt.c is only compiled when CONFIG_OF_FLATTREE is
set.

> +char dtb_compat_name[MAX_DTB_COMPAT_STR] = "";
> +
> +char __init *of_get_dtb_compatible_string(void)
> +{
> +	return dtb_compat_name;
> +}
> +
> +
> +extern uint8_t __dtb_start[];
> +extern uint8_t __dtb_end[];
> +void *of_find_compatible_dtb(char *name)

Please rename of_flat_dt_find_compatible_dtb() to match the naming
conventions that I'm working to establish in this file.

Also, it should have an __init annotation.

> +{
> +	void *rc = NULL;
> +	unsigned long root, size;
> +	struct boot_param_header  *orig_initial_boot_params;
> +	uint8_t *blob;

blob should be a void* here.  It is never used as a uint8.

> +
> +	orig_initial_boot_params = initial_boot_params;
> +	blob = __dtb_start;
> +	initial_boot_params = (struct boot_param_header *)blob;
> +
> +	do {

Start the loop with "while (blob < __dtb_end) {" to protect against
the possibility that __dtb_start == __dtb_end (no dtbs linked in).

> +		root = of_get_flat_dt_root();

Just to be defencive, should check the value of
initial_boot_params->magic before dereferencing the rest of the
structure or trying to parse nodes.

> +		if (of_flat_dt_is_compatible(root, name) > 0) {
> +			rc = (void *) blob;

Unnecessary cast (regardless of whether blob is a void* or a uint8_t*).

> +			break;
> +		}
> +
> +		size =  be32_to_cpu(initial_boot_params->totalsize);
> +		blob = (uint8_t *) ALIGN(((unsigned long)blob + size),
> +					DTB_ALIGNMENT);

I believe PTR_ALIGN() can be used and the casting removed.

> +		initial_boot_params = (struct boot_param_header *)blob;
> +	} while (blob < __dtb_end);
> +
> +	if (rc == NULL)
> +		initial_boot_params = orig_initial_boot_params;
> +	return rc;
> +}
> +
> +
> +static int __init dtb_compat_setup(char *line)

Please rename of_flat_dt_compat_setup()

> +{
> +	strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
> +	return 1;
> +}

I don't remember getting a response about whether or not
of_find_compatible_dtb can be called directly from dtb_compat_setup.
Doing so would eliminate the arbitrary MAX_DTB_COMPAT_STR because
there would be no need to keep around a copy of dtb_compat_name.  It
also means that of_find_compatible_dtb() can be restricted to the file
scope.

Is there a use-case for calling of_find_compatible_dtb() anywhere
other than in dtb_compat_setup?

> +
> +early_param("dtb_compat", dtb_compat_setup);
> +#endif
> +#endif
> +
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 7bbf5b3..c696da3c 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -58,6 +58,8 @@ struct boot_param_header {
>  };
>  
>  #if defined(CONFIG_OF_FLATTREE)
> +#define MAX_DTB_COMPAT_STR 64
> +

Only used by drivers/of/fdt.c, so should not be in a header file.

>  /* TBD: Temporary export of fdt globals - remove when code fully merged */
>  extern int __initdata dt_root_addr_cells;
>  extern int __initdata dt_root_size_cells;
> @@ -82,6 +84,9 @@ extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>  extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
>  extern u64 dt_mem_next_cell(int s, __be32 **cellp);
>  
> +extern char *of_get_dtb_compatible_string(void);
> +extern void *of_find_compatible_dtb(char *name);
> +

As commented above, I don't think either of these need to be visible
outside of fdt.c

g.

Re: [PATCH 2/2] of/fdt: add kernel command line option for dtb_compat string

[Dirk Brandewie]

On 11/15/2010 10:32 PM, Grant Likely wrote:
> On Mon, Nov 15, 2010 at 08:01:21PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> From: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Add support for specifying a "compatible" string from the kernel
>> command line and functions for the platform to find the compatible
>> blob present in the kernel image if any.
>>
>> Signed-off-by: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Hi Dirk,
>
> a couple of general comments:
>
> A lot of the information in the cover letter should really be here in
> the actual patch description.  Use the cover letter to provide context
> for the entire series, but put the specifics with each patch.  Version
> changelog history should also appear in patch description, not the
> cover letter.
>

OK

> For the next version, please cc: linux-kernel-u79uwXL29TaiAVqoAR/hOA@public.gmane.org
> This series should have a wider review audience, particularly because
> it touches the linker sections and the kernel parameters.  You should
> also cc: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org and
> microblaze-uclinux-rVRm/Wmeqae7NGdpmJTKYQ@public.gmane.org for the patch that adds the common
> rules because it will affect both of those architectures directly.
>

Will do
> More comments below.
>

>> +#ifndef MODULE
>
> fdt.c cannot be compiled as a module.  Is this necessary?
>
>> +#ifdef CONFIG_OF_FLATTREE
>
> Unnecessary #ifdef.  fdt.c is only compiled when CONFIG_OF_FLATTREE is
> set.
>

Both #ifdef's removed
>> +char dtb_compat_name[MAX_DTB_COMPAT_STR] = "";
>> +
>> +char __init *of_get_dtb_compatible_string(void)
>> +{
>> +	return dtb_compat_name;
>> +}
>> +
>> +
>> +extern uint8_t __dtb_start[];
>> +extern uint8_t __dtb_end[];
>> +void *of_find_compatible_dtb(char *name)
>
> Please rename of_flat_dt_find_compatible_dtb() to match the naming
> conventions that I'm working to establish in this file.
>
> Also, it should have an __init annotation.
>

done

>> +{
>> +	void *rc = NULL;
>> +	unsigned long root, size;
>> +	struct boot_param_header  *orig_initial_boot_params;
>> +	uint8_t *blob;
>
> blob should be a void* here.  It is never used as a uint8.
>

I used uint8_t prevent compiler warning when blob is compared to __dtb_end

>> +
>> +	orig_initial_boot_params = initial_boot_params;
>> +	blob = __dtb_start;
>> +	initial_boot_params = (struct boot_param_header *)blob;
>> +
>> +	do {
>
> Start the loop with "while (blob<  __dtb_end) {" to protect against
> the possibility that __dtb_start == __dtb_end (no dtbs linked in).
>

Done

>> +		root = of_get_flat_dt_root();
>
> Just to be defencive, should check the value of
> initial_boot_params->magic before dereferencing the rest of the
> structure or trying to parse nodes.
>

Added as a condition to while loop

>> +		if (of_flat_dt_is_compatible(root, name)>  0) {
>> +			rc = (void *) blob;
>
> Unnecessary cast (regardless of whether blob is a void* or a uint8_t*).
>

done

>> +			break;
>> +		}
>> +
>> +		size =  be32_to_cpu(initial_boot_params->totalsize);
>> +		blob = (uint8_t *) ALIGN(((unsigned long)blob + size),
>> +					DTB_ALIGNMENT);
>
> I believe PTR_ALIGN() can be used and the casting removed.
>

done

>> +		initial_boot_params = (struct boot_param_header *)blob;
>> +	} while (blob<  __dtb_end);
>> +
>> +	if (rc == NULL)
>> +		initial_boot_params = orig_initial_boot_params;
>> +	return rc;
>> +}
>> +
>> +
>> +static int __init dtb_compat_setup(char *line)
>
> Please rename of_flat_dt_compat_setup()
>

done

>> +{
>> +	strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
>> +	return 1;
>> +}
>
> I don't remember getting a response about whether or not
> of_find_compatible_dtb can be called directly from dtb_compat_setup.
> Doing so would eliminate the arbitrary MAX_DTB_COMPAT_STR because
> there would be no need to keep around a copy of dtb_compat_name.  It
> also means that of_find_compatible_dtb() can be restricted to the file
> scope.
>
> Is there a use-case for calling of_find_compatible_dtb() anywhere
> other than in dtb_compat_setup?
>

The use case for having these functions exposed is allowing the platfrom code 
decide the policy for which dtb will be used in the case where a dtb might have 
been passed in by the boot loader. These functions can be used as a fallback if 
a dtb was not passed in or to override of the passed in dtb by the platform code.
>> +
>> +early_param("dtb_compat", dtb_compat_setup);
>> +#endif
>> +#endif
>> +
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 7bbf5b3..c696da3c 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -58,6 +58,8 @@ struct boot_param_header {
>>   };
>>
>>   #if defined(CONFIG_OF_FLATTREE)
>> +#define MAX_DTB_COMPAT_STR 64
>> +
>
> Only used by drivers/of/fdt.c, so should not be in a header file.
>

removed

>>   /* TBD: Temporary export of fdt globals - remove when code fully merged */
>>   extern int __initdata dt_root_addr_cells;
>>   extern int __initdata dt_root_size_cells;
>> @@ -82,6 +84,9 @@ extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>   extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
>>   extern u64 dt_mem_next_cell(int s, __be32 **cellp);
>>
>> +extern char *of_get_dtb_compatible_string(void);
>> +extern void *of_find_compatible_dtb(char *name);
>> +
>
> As commented above, I don't think either of these need to be visible
> outside of fdt.c
>
> g.

Re: [PATCH 2/2] of/fdt: add kernel command line option for dtb_compat string

[Grant Likely]

On Tue, Nov 16, 2010 at 6:50 AM, Dirk Brandewie
<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 11/15/2010 10:32 PM, Grant Likely wrote:
>>
>> On Mon, Nov 15, 2010 at 08:01:21PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>> +{
>>> +       void *rc = NULL;
>>> +       unsigned long root, size;
>>> +       struct boot_param_header  *orig_initial_boot_params;
>>> +       uint8_t *blob;
>>
>> blob should be a void* here.  It is never used as a uint8.
>>
>
> I used uint8_t prevent compiler warning when blob is compared to __dtb_end
>

okay

>>> +{
>>> +       strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
>>> +       return 1;
>>> +}
>>
>> I don't remember getting a response about whether or not
>> of_find_compatible_dtb can be called directly from dtb_compat_setup.
>> Doing so would eliminate the arbitrary MAX_DTB_COMPAT_STR because
>> there would be no need to keep around a copy of dtb_compat_name.  It
>> also means that of_find_compatible_dtb() can be restricted to the file
>> scope.
>>
>> Is there a use-case for calling of_find_compatible_dtb() anywhere
>> other than in dtb_compat_setup?
>>
>
> The use case for having these functions exposed is allowing the platfrom
> code decide the policy for which dtb will be used in the case where a dtb
> might have been passed in by the boot loader. These functions can be used as
> a fallback if a dtb was not passed in or to override of the passed in dtb by
> the platform code.

Still this all looks very generic, even across architectures.  I say
don't export the functions for now, and change it later if a use case
shows up that requires it.  I'd really rather not let platform code
muck about with it directly if there isn't a use case for it.

However, thinking about it further.  You also need to make sure this
code is excluded for the powerpc and microblaze architectures because
the kernel command line is passed via the device tree blob in those
cases.  By that time it gets called, it is too late to switch the
device tree pointer.

g.

Re: [PATCH 2/2] of/fdt: add kernel command line option for dtb_compat string

[Dirk Brandewie]

On 11/16/2010 04:12 PM, Grant Likely wrote:
> On Tue, Nov 16, 2010 at 6:50 AM, Dirk Brandewie
> <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>  wrote:
>> On 11/15/2010 10:32 PM, Grant Likely wrote:
>>>
>>> On Mon, Nov 15, 2010 at 08:01:21PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>> +{
>>>> +       void *rc = NULL;
>>>> +       unsigned long root, size;
>>>> +       struct boot_param_header  *orig_initial_boot_params;
>>>> +       uint8_t *blob;
>>>
>>> blob should be a void* here.  It is never used as a uint8.
>>>
>>
>> I used uint8_t prevent compiler warning when blob is compared to __dtb_end
>>
>
> okay
>
>>>> +{
>>>> +       strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
>>>> +       return 1;
>>>> +}
>>>
>>> I don't remember getting a response about whether or not
>>> of_find_compatible_dtb can be called directly from dtb_compat_setup.
>>> Doing so would eliminate the arbitrary MAX_DTB_COMPAT_STR because
>>> there would be no need to keep around a copy of dtb_compat_name.  It
>>> also means that of_find_compatible_dtb() can be restricted to the file
>>> scope.
>>>
>>> Is there a use-case for calling of_find_compatible_dtb() anywhere
>>> other than in dtb_compat_setup?
>>>
>>
>> The use case for having these functions exposed is allowing the platfrom
>> code decide the policy for which dtb will be used in the case where a dtb
>> might have been passed in by the boot loader. These functions can be used as
>> a fallback if a dtb was not passed in or to override of the passed in dtb by
>> the platform code.
>
> Still this all looks very generic, even across architectures.  I say
> don't export the functions for now, and change it later if a use case
> shows up that requires it.  I'd really rather not let platform code
> muck about with it directly if there isn't a use case for it.

I have the use case for it in my platform.  There will two ways that the kernel 
can get the DTB.
1. The blob is linked in and I go find the one for my platform. This is the path 
that uses the two new functions.

2. The a pointer blob passed in (physical address) from the bootloader. The blob 
is relocated/copied to keep the bootloader from having to know the kernel memory 
layout.

One method will be able to override the other.  I am not sure what the 
precedence between the methods will be ATM.

>
> However, thinking about it further.  You also need to make sure this
> code is excluded for the powerpc and microblaze architectures because
> the kernel command line is passed via the device tree blob in those
> cases.  By that time it gets called, it is too late to switch the
> device tree pointer.
>
if a "wrapped" kernel is booted doesn't it know exactly which dtb it should be 
using?  These platforms would never go looking for the command line value even 
it it was passed in.

> g.

Re: [PATCH 2/2] of/fdt: add kernel command line option for dtb_compat string

[Grant Likely]

On Tue, Nov 16, 2010 at 6:48 PM, Dirk Brandewie
<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 11/16/2010 04:12 PM, Grant Likely wrote:
>>
>> On Tue, Nov 16, 2010 at 6:50 AM, Dirk Brandewie
>> <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>  wrote:
>>>
>>> On 11/15/2010 10:32 PM, Grant Likely wrote:
>>>>
>>>> On Mon, Nov 15, 2010 at 08:01:21PM -0800, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>>>> wrote:
>>>>>
>>>>> +{
>>>>> +       void *rc = NULL;
>>>>> +       unsigned long root, size;
>>>>> +       struct boot_param_header  *orig_initial_boot_params;
>>>>> +       uint8_t *blob;
>>>>
>>>> blob should be a void* here.  It is never used as a uint8.
>>>>
>>>
>>> I used uint8_t prevent compiler warning when blob is compared to
>>> __dtb_end
>>>
>>
>> okay
>>
>>>>> +{
>>>>> +       strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
>>>>> +       return 1;
>>>>> +}
>>>>
>>>> I don't remember getting a response about whether or not
>>>> of_find_compatible_dtb can be called directly from dtb_compat_setup.
>>>> Doing so would eliminate the arbitrary MAX_DTB_COMPAT_STR because
>>>> there would be no need to keep around a copy of dtb_compat_name.  It
>>>> also means that of_find_compatible_dtb() can be restricted to the file
>>>> scope.
>>>>
>>>> Is there a use-case for calling of_find_compatible_dtb() anywhere
>>>> other than in dtb_compat_setup?
>>>>
>>>
>>> The use case for having these functions exposed is allowing the platfrom
>>> code decide the policy for which dtb will be used in the case where a dtb
>>> might have been passed in by the boot loader. These functions can be used
>>> as
>>> a fallback if a dtb was not passed in or to override of the passed in dtb
>>> by
>>> the platform code.
>>
>> Still this all looks very generic, even across architectures.  I say
>> don't export the functions for now, and change it later if a use case
>> shows up that requires it.  I'd really rather not let platform code
>> muck about with it directly if there isn't a use case for it.
>
> I have the use case for it in my platform.  There will two ways that the
> kernel can get the DTB.
> 1. The blob is linked in and I go find the one for my platform. This is the
> path that uses the two new functions.
>
> 2. The a pointer blob passed in (physical address) from the bootloader. The
> blob is relocated/copied to keep the bootloader from having to know the
> kernel memory layout.
>
> One method will be able to override the other.  I am not sure what the
> precedence between the methods will be ATM.

ATM, let's assume that all platforms will use the same precedence
rules.  I really don't want to see arbitrary differences between
platforms in this regard.  Encode the behaviour fully into fdt.c.  If
the precedence rules end up being wrong, then it can be changed.  If
it turns out that different platforms really do need to have different
rules, then it can be exported at that time.

g.

>> However, thinking about it further.  You also need to make sure this
>> code is excluded for the powerpc and microblaze architectures because
>> the kernel command line is passed via the device tree blob in those
>> cases.  By that time it gets called, it is too late to switch the
>> device tree pointer.
>>
> if a "wrapped" kernel is booted doesn't it know exactly which dtb it should
> be using?

Yes.  All of the current powerpc dt-wrappers only carry 1 dtb image
per wrapper, so the question of which dtb is moot.  However, powerpc
wrappers are firmware-interface specific, and their entire purpose in
life is to translate data from the firmware-specific interface into
the dtb representation.  Powerpc history has a number of oddball
firmware interfaces with no way to detect which one is being used.
Therefore, the wrapper has to be hand chosen for each legacy platform.

> These platforms would never go looking for the command line value
> even it it was passed in.

Correct.  On PowerPC the vmlinux entry point *requires* passing in a
device tree, so the command line value isn't useful.

g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.