[PATCH 0/4] [RFC V4] Adding DTB to architecture independent vmlinux

[PATCH 0/4] [RFC V4] Adding DTB to architecture independent vmlinux

[dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w]

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

The following series implements the ability to link device tree
blob(s) into the kernel image.

Changes since V3:
Added kernel command line option to pass a "compatible" string to
select the DTB for the platform.

Added function to find the compatible DTB in the kernel image if any

Added code to dtc to force the DTB to be modulo 32 bytes in size

Changed the alignment of the .dtb section


Dirk Brandewie (4):
  x86/of:  Support building device tree blob(s) into image.
  of: Add support for linking device tree blobs into vmlinux
  of/dtc: force dtb size to modulo 32 bytes
  of/fdt: add kernel command line option for dtb_compat string

 arch/x86/Kconfig                  |    6 +++-
 arch/x86/boot/dts/Kconfig         |    8 +++++
 arch/x86/kernel/Makefile          |   11 ++++++++
 drivers/of/fdt.c                  |   52 +++++++++++++++++++++++++++++++++++++
 include/asm-generic/vmlinux.lds.h |   13 ++++++++-
 include/linux/of_fdt.h            |    9 ++++++
 init/Kconfig                      |    7 +++++
 scripts/Makefile.lib              |   14 ++++++++++
 scripts/dtc/flattree.c            |    6 +++-
 9 files changed, 123 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/boot/dts/Kconfig

-- 
1.7.2.3

[PATCH 1/4] x86/of: Support building device tree blob(s) into image.

[dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w]

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

This patch adds support for linking device tree blobs into vmlinux.
The blobs linked into the image is controlled via kernel config
variables

Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/x86/Kconfig          |    6 +++++-
 arch/x86/boot/dts/Kconfig |    8 ++++++++
 arch/x86/kernel/Makefile  |   11 +++++++++++
 3 files changed, 24 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/boot/dts/Kconfig

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5904f38..62c195d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -299,13 +299,17 @@ config X86_BIGSMP
 	---help---
 	  This option is needed for the systems that have more than 8 CPUs
 
-config X86_OF
+menuconfig X86_OF
 	bool "Support for device tree"
 	select OF
 	select OF_FLATTREE
 	---help---
 	  Device tree support on X86.
 
+if X86_OF
+source "arch/x86/boot/dts/Kconfig
+endif
+
 if X86_32
 config X86_EXTENDED_PLATFORM
 	bool "Support for extended (non-PC) x86 platforms"
diff --git a/arch/x86/boot/dts/Kconfig b/arch/x86/boot/dts/Kconfig
new file mode 100644
index 0000000..5380b6b
--- /dev/null
+++ b/arch/x86/boot/dts/Kconfig
@@ -0,0 +1,8 @@
+config CE4100_DTB
+       bool "Intel CE4100"
+       depends on X86_OF && KERNEL_DTB
+
+config TEST_DTB
+       bool "Test DTS"
+       depends on  X86_OF && KERNEL_DTB
+
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 586df14..cf15e8c 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -113,6 +113,17 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
 obj-$(CONFIG_SWIOTLB)			+= pci-swiotlb.o
 obj-$(CONFIG_X86_OF)			+= prom.o
 
+ifeq ($(CONFIG_KERNEL_DTB),y)
+obj-$(CONFIG_CE4100_DTB) += ce4100.dtb.o
+obj-$(CONFIG_TEST_DTB) += test.dtb.o
+endif
+
+dtstree	:= $(srctree)/arch/x86/boot/dts
+
+$(obj)/%.dtb: $(dtstree)/%.dts
+	$(call if_changed,dtc)
+
+
 ###
 # 64 bit specific files
 ifeq ($(CONFIG_X86_64),y)
-- 
1.7.2.3

Re: [PATCH 1/4] x86/of: Support building device tree blob(s) into image.

[Grant Likely]

On Thu, Nov 11, 2010 at 04:03:47PM -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 blobs linked into the image is controlled via kernel config
> variables
> 
> Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/x86/Kconfig          |    6 +++++-
>  arch/x86/boot/dts/Kconfig |    8 ++++++++
>  arch/x86/kernel/Makefile  |   11 +++++++++++
>  3 files changed, 24 insertions(+), 1 deletions(-)
>  create mode 100644 arch/x86/boot/dts/Kconfig
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5904f38..62c195d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -299,13 +299,17 @@ config X86_BIGSMP
>  	---help---
>  	  This option is needed for the systems that have more than 8 CPUs
>  
> -config X86_OF
> +menuconfig X86_OF
>  	bool "Support for device tree"
>  	select OF
>  	select OF_FLATTREE
>  	---help---
>  	  Device tree support on X86.
>  
> +if X86_OF
> +source "arch/x86/boot/dts/Kconfig
> +endif
> +
>  if X86_32
>  config X86_EXTENDED_PLATFORM
>  	bool "Support for extended (non-PC) x86 platforms"
> diff --git a/arch/x86/boot/dts/Kconfig b/arch/x86/boot/dts/Kconfig
> new file mode 100644
> index 0000000..5380b6b
> --- /dev/null
> +++ b/arch/x86/boot/dts/Kconfig
> @@ -0,0 +1,8 @@
> +config CE4100_DTB
> +       bool "Intel CE4100"
> +       depends on X86_OF && KERNEL_DTB
> +
> +config TEST_DTB
> +       bool "Test DTS"
> +       depends on  X86_OF && KERNEL_DTB
> +

Rather than explicitly adding a Kconfig entry for each and every .dts
file that you'll ever want to embed; I suggest a single Kconfig that
allows the users to input a list of .dtb filenames.  PowerPC already
does this (sort of) for specifying a list of additional dtbimage.%
targets.

See config EXTRA_TARGETS in arch/powerpc/Kconfig and in
arch/powerp/boot/Makefile.

You can still /also/ link in specific .dtb files when other config
symbols are chosen (ie, if some specific board support is chosen that
is known to require it), but it doesn't seem to make a lot of sense to
me to have entirely separate Kconfig symbols for each and every .dtb
file.

> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 586df14..cf15e8c 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -113,6 +113,17 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o
>  obj-$(CONFIG_SWIOTLB)			+= pci-swiotlb.o
>  obj-$(CONFIG_X86_OF)			+= prom.o
>  
> +ifeq ($(CONFIG_KERNEL_DTB),y)
> +obj-$(CONFIG_CE4100_DTB) += ce4100.dtb.o
> +obj-$(CONFIG_TEST_DTB) += test.dtb.o
> +endif

The rule that actually generates the .dtb.o files seems to be in patch
#2, which makes this series non-bisectable (it will break if patch 1
is applied without patch 2).  The changes need to be ordered so that
the kernel still builds at any point in the series.

... hmmm, maybe I should dig deeper....

Okay, I see now that it requires CONFIG_KERNEL_DTB to be set which is
also defined in patch #2, so the series does appear to be bisectable,
but it isn't entirely logical.  I would expect to see all the required
rules and linker script additions to be applied before adding the dts
files to the kernel and adding the Kconfig/Makefile changes that use
them.

heh, and I've also just realized that it still isn't bisectable
because none of these patches add ce4100.dts or test.dts to the
kernel.  :-)

I imagine this series should be something like:

1) add %.dtb:%.dts rule  (ideally to scripts/Makefile.lib; see below)
2) add %.dtb.S:%.dtb rule  (ideally to scripts/Makefile.lib; see below)
3+) add x86 Kconfig and Makefile changes to enable dtb linking and to
    add the .dts files.

> +
> +dtstree	:= $(srctree)/arch/x86/boot/dts
> +
> +$(obj)/%.dtb: $(dtstree)/%.dts
> +	$(call if_changed,dtc)

I know this is what we've done in powerpc, but I suspect it would make
a lot more sense to make this rule generic and arch independent so
that .dts files can live in any subdirectory.

Perhaps this works (completely untested):

$(obj)/%.dtb: $(src)/%.dts
	$(call if_changed,dtc)

And I think that would belong in scripts/Makefile.lib.  I don't see
any reason why PowerPC and Microblaze couldn't use a common definition
for this rule.

g.

[PATCH 2/4] 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 |   13 ++++++++++++-
 init/Kconfig                      |    7 +++++++
 scripts/Makefile.lib              |   14 ++++++++++++++
 3 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index bd69d79..c8f600e 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -146,6 +146,16 @@
 #define TRACE_SYSCALLS()
 #endif
 
+#ifdef CONFIG_KERNEL_DTB
+#define KERNEL_DTB							\
+	. = ALIGN(32);							\
+	VMLINUX_SYMBOL(__dtb_start) = .;				\
+	*(.dtb)			       					\
+	VMLINUX_SYMBOL(__dtb_end) = .;
+#else
+#define KERNEL_DTB
+#endif
+
 /* .data section */
 #define DATA_DATA							\
 	*(.data)							\
@@ -468,7 +478,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/init/Kconfig b/init/Kconfig
index 88c1046..fddfc0f 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1083,6 +1083,13 @@ config PCI_QUIRKS
           bugs/quirks. Disable this only if your target machine is
           unaffected by PCI quirks.
 
+config KERNEL_DTB
+       bool "Support linking a device tree blob into vmlinux"
+       default n
+       help
+         This option provides support for adding a device tree blob(s)
+	 directly to vmlinux
+
 config SLUB_DEBUG
 	default y
 	bool "Enable SLUB debugging support" if EMBEDDED
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 4c72c11..c4487b2 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -200,6 +200,20 @@ quiet_cmd_gzip = GZIP    $@
 cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
 	(rm -f $@ ; false)
 
+# DTC
+#  ---------------------------------------------------------------------------
+$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
+	@echo '.section .dtb,"a"' > $@
+	@echo '.global __dtb_$(*F)_begin' >> $@
+	@echo '__dtb_$(*F)_begin:' >> $@
+	@echo '.incbin "$<" ' >> $@
+	@echo '__dtb_$(*F)_end:' >> $@
+	@echo '.global __dtb_$(*F)_end' >> $@
+
+DTC = $(objtree)/scripts/dtc/dtc
+
+quiet_cmd_dtc = DTC	$@
+      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(dtstree)/$*.dts
 
 # Bzip2
 # ---------------------------------------------------------------------------
-- 
1.7.2.3

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

[Grant Likely]

On Thu, Nov 11, 2010 at 04:03:48PM -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 |   13 ++++++++++++-
>  init/Kconfig                      |    7 +++++++
>  scripts/Makefile.lib              |   14 ++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index bd69d79..c8f600e 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -146,6 +146,16 @@
>  #define TRACE_SYSCALLS()
>  #endif
>  
> +#ifdef CONFIG_KERNEL_DTB
> +#define KERNEL_DTB							\

To match other definitions in this file, this should be defined with
parentheses: #define KERNEL_DTB()

> +	. = ALIGN(32);							\
> +	VMLINUX_SYMBOL(__dtb_start) = .;				\
> +	*(.dtb)			       					\

I wonder if .meminit.rodata.dtb be a better section name.  Could use
some input from more experienced kernel hackers here.  hpa, what say
you?

Also, inconsistent indentation (mixed tabs and spaces).

> +	VMLINUX_SYMBOL(__dtb_end) = .;
> +#else
> +#define KERNEL_DTB
> +#endif
> +

Does this need to be wrapped with CONFIG_KERNEL_DTB?  Is there any
downside to including these sections unconditionally?

>  /* .data section */
>  #define DATA_DATA							\
>  	*(.data)							\
> @@ -468,7 +478,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/init/Kconfig b/init/Kconfig
> index 88c1046..fddfc0f 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1083,6 +1083,13 @@ config PCI_QUIRKS
>            bugs/quirks. Disable this only if your target machine is
>            unaffected by PCI quirks.
>  
> +config KERNEL_DTB
> +       bool "Support linking a device tree blob into vmlinux"
> +       default n
> +       help
> +         This option provides support for adding a device tree blob(s)
> +	 directly to vmlinux

Inconsistent indentation.  I'm also not convinced that this Kconfig
symbol is really needed (as commented on above).

> +
>  config SLUB_DEBUG
>  	default y
>  	bool "Enable SLUB debugging support" if EMBEDDED
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 4c72c11..c4487b2 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -200,6 +200,20 @@ quiet_cmd_gzip = GZIP    $@
>  cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
>  	(rm -f $@ ; false)
>  
> +# DTC
> +#  ---------------------------------------------------------------------------
> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
> +	@echo '.section .dtb,"a"' > $@
> +	@echo '.global __dtb_$(*F)_begin' >> $@
> +	@echo '__dtb_$(*F)_begin:' >> $@
> +	@echo '.incbin "$<" ' >> $@
> +	@echo '__dtb_$(*F)_end:' >> $@
> +	@echo '.global __dtb_$(*F)_end' >> $@
> +
> +DTC = $(objtree)/scripts/dtc/dtc
> +
> +quiet_cmd_dtc = DTC	$@
> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(dtstree)/$*.dts

As already mentioned; need to rationalized these rule additions with
the changes made in patch #1.

g.

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

[Dirk Brandewie]

On 11/13/2010 09:25 PM, Grant Likely wrote:
> On Thu, Nov 11, 2010 at 04:03:48PM -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 |   13 ++++++++++++-
>>   init/Kconfig                      |    7 +++++++
>>   scripts/Makefile.lib              |   14 ++++++++++++++
>>   3 files changed, 33 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> index bd69d79..c8f600e 100644
>> --- a/include/asm-generic/vmlinux.lds.h
>> +++ b/include/asm-generic/vmlinux.lds.h
>> @@ -146,6 +146,16 @@
>>   #define TRACE_SYSCALLS()
>>   #endif
>>
>> +#ifdef CONFIG_KERNEL_DTB
>> +#define KERNEL_DTB							\
>
> To match other definitions in this file, this should be defined with
> parentheses: #define KERNEL_DTB()
>
>> +	. = ALIGN(32);							\
>> +	VMLINUX_SYMBOL(__dtb_start) = .;				\
>> +	*(.dtb)			       					\
>
> I wonder if .meminit.rodata.dtb be a better section name.  Could use
> some input from more experienced kernel hackers here.  hpa, what say
> you?

I will use what ever name seems reasonable,  It turns out that the sections that 
are named *.init.rodata are really in a R/W section :-( so that name is really a 
misnomer.  I will check with HPA on name again.

>
> Also, inconsistent indentation (mixed tabs and spaces).
>

will fix

>> +	VMLINUX_SYMBOL(__dtb_end) = .;
>> +#else
>> +#define KERNEL_DTB
>> +#endif
>> +
>
> Does this need to be wrapped with CONFIG_KERNEL_DTB?  Is there any
> downside to including these sections unconditionally?
>

Not sure I was just following the convention set by TRACE_SYSCALLS(), 
BRANCH_PROFILE(), ...


>>   /* .data section */
>>   #define DATA_DATA							\
>>   	*(.data)							\
>> @@ -468,7 +478,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/init/Kconfig b/init/Kconfig
>> index 88c1046..fddfc0f 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1083,6 +1083,13 @@ config PCI_QUIRKS
>>             bugs/quirks. Disable this only if your target machine is
>>             unaffected by PCI quirks.
>>
>> +config KERNEL_DTB
>> +       bool "Support linking a device tree blob into vmlinux"
>> +       default n
>> +       help
>> +         This option provides support for adding a device tree blob(s)
>> +	 directly to vmlinux
>
> Inconsistent indentation.  I'm also not convinced that this Kconfig
> symbol is really needed (as commented on above).
>
>> +
>>   config SLUB_DEBUG
>>   	default y
>>   	bool "Enable SLUB debugging support" if EMBEDDED
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 4c72c11..c4487b2 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -200,6 +200,20 @@ quiet_cmd_gzip = GZIP    $@
>>   cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9>  $@) || \
>>   	(rm -f $@ ; false)
>>
>> +# DTC
>> +#  ---------------------------------------------------------------------------
>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>> +	@echo '.section .dtb,"a"'>  $@
>> +	@echo '.global __dtb_$(*F)_begin'>>  $@
>> +	@echo '__dtb_$(*F)_begin:'>>  $@
>> +	@echo '.incbin "$<" '>>  $@
>> +	@echo '__dtb_$(*F)_end:'>>  $@
>> +	@echo '.global __dtb_$(*F)_end'>>  $@
>> +
>> +DTC = $(objtree)/scripts/dtc/dtc
>> +
>> +quiet_cmd_dtc = DTC	$@
>> +      cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0  $(dtstree)/$*.dts
>
> As already mentioned; need to rationalized these rule additions with
> the changes made in patch #1.
>
> g.
>

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

[H. Peter Anvin]

On 11/15/2010 08:37 AM, Dirk Brandewie wrote:
>>
>> To match other definitions in this file, this should be defined with
>> parentheses: #define KERNEL_DTB()
>>
>>> +	. = ALIGN(32);							\
>>> +	VMLINUX_SYMBOL(__dtb_start) = .;				\
>>> +	*(.dtb)			       					\
>>
>> I wonder if .meminit.rodata.dtb be a better section name.  Could use
>> some input from more experienced kernel hackers here.  hpa, what say
>> you?
> 
> I will use what ever name seems reasonable,  It turns out that the sections that 
> are named *.init.rodata are really in a R/W section :-( so that name is really a 
> misnomer.  I will check with HPA on name again.
> 

I think they should be named *rodata* just to indicate that they should
eventually be moved to a readonly section.

	-hpa

[PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w]

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

This patch forces the size of the DTB to be modulo 32 bytes. This is
needed to support linking multiple DTB's into a single section in the
image. GCC wants structures to be 32 byte aligned without this change
DTB's after the first in the section may not be properly aligned so
the flat tree parsing code will fall over.

Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 scripts/dtc/flattree.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/scripts/dtc/flattree.c b/scripts/dtc/flattree.c
index 76acd28..ccca797 100644
--- a/scripts/dtc/flattree.c
+++ b/scripts/dtc/flattree.c
@@ -358,6 +358,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 	struct data strbuf     = empty_data;
 	struct fdt_header fdt;
 	int padlen = 0;
+	int align_size;
 
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
@@ -395,6 +396,9 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 		fdt.totalsize = cpu_to_fdt32(tsize);
 	}
 
+	align_size = ALIGN(fdt32_to_cpu(fdt.totalsize), 32);
+	fdt.totalsize = cpu_to_fdt32(align_size);
+
 	/*
 	 * Assemble the blob: start with the header, add with alignment
 	 * the reserve buffer, add the reserve map terminating zeroes,
@@ -412,7 +416,7 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version)
 	 */
 	if (padlen > 0)
 		blob = data_append_zeroes(blob, padlen);
-
+	blob = data_append_align(blob, 32);
 	fwrite(blob.val, blob.len, 1, f);
 
 	if (ferror(f))
-- 
1.7.2.3

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[H. Peter Anvin]

On 11/11/2010 04:03 PM, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This patch forces the size of the DTB to be modulo 32 bytes. This is
> needed to support linking multiple DTB's into a single section in the
> image. GCC wants structures to be 32 byte aligned without this change
> DTB's after the first in the section may not be properly aligned so
> the flat tree parsing code will fall over.
> 
> Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

I don't think 32 is a universal number; it should depend on the ABI.  On
x86, for one thing, I'm pretty sure that there is no particular
alignment requirements beyond the natural alignment of the data items,
for example.

Although 32 is probably conservative on any platform, please flag the
origin of this with a comment, or make it a constant defined in a header
file... otherwise, if this breaks for whatever reason it'll be near
impossible to find.

	-hpa

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[Dirk Brandewie]

On 11/11/2010 04:47 PM, H. Peter Anvin wrote:
> On 11/11/2010 04:03 PM, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> From: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> This patch forces the size of the DTB to be modulo 32 bytes. This is
>> needed to support linking multiple DTB's into a single section in the
>> image. GCC wants structures to be 32 byte aligned without this change
>> DTB's after the first in the section may not be properly aligned so
>> the flat tree parsing code will fall over.
>>
>> Signed-off-by: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> I don't think 32 is a universal number; it should depend on the ABI.  On
> x86, for one thing, I'm pretty sure that there is no particular
> alignment requirements beyond the natural alignment of the data items,
> for example.
>
> Although 32 is probably conservative on any platform, please flag the
> origin of this with a comment, or make it a constant defined in a header
> file... otherwise, if this breaks for whatever reason it'll be near
> impossible to find.
>

I will change . = ALIGN(32); to STRUCT_ALIGN();

from vmlinux.lds.h:
/*
  * Align to a 32 byte boundary equal to the
  * alignment gcc 4.5 uses for a struct
  */
#define STRUCT_ALIGN() . = ALIGN(32)

--Dirk

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[David Gibson]

On Thu, Nov 11, 2010 at 05:01:16PM -0800, Dirk Brandewie wrote:
> On 11/11/2010 04:47 PM, H. Peter Anvin wrote:
> >On 11/11/2010 04:03 PM, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> >>From: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>
> >>This patch forces the size of the DTB to be modulo 32 bytes. This is
> >>needed to support linking multiple DTB's into a single section in the
> >>image. GCC wants structures to be 32 byte aligned without this change
> >>DTB's after the first in the section may not be properly aligned so
> >>the flat tree parsing code will fall over.
> >>
> >>Signed-off-by: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >
> >I don't think 32 is a universal number; it should depend on the ABI.  On
> >x86, for one thing, I'm pretty sure that there is no particular
> >alignment requirements beyond the natural alignment of the data items,
> >for example.
> >
> >Although 32 is probably conservative on any platform, please flag the
> >origin of this with a comment, or make it a constant defined in a header
> >file... otherwise, if this breaks for whatever reason it'll be near
> >impossible to find.
> >
> 
> I will change . = ALIGN(32); to STRUCT_ALIGN();
> 
> from vmlinux.lds.h:
> /*
>  * Align to a 32 byte boundary equal to the
>  * alignment gcc 4.5 uses for a struct
>  */
> #define STRUCT_ALIGN() . = ALIGN(32)

I also think this is the wrong place to do this.  scripts/dtc is a
mirror of copy of upstream dtc, designed for making dtb images for
general purposes.  I think the alignment should instead go into the
linker script fragments you generate to incbin the dtbs.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[Dirk Brandewie]

Hi David,
On 11/11/2010 05:16 PM, David Gibson wrote:
> On Thu, Nov 11, 2010 at 05:01:16PM -0800, Dirk Brandewie wrote:
>> On 11/11/2010 04:47 PM, H. Peter Anvin wrote:
>>> On 11/11/2010 04:03 PM, dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>> From: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> This patch forces the size of the DTB to be modulo 32 bytes. This is
>>>> needed to support linking multiple DTB's into a single section in the
>>>> image. GCC wants structures to be 32 byte aligned without this change
>>>> DTB's after the first in the section may not be properly aligned so
>>>> the flat tree parsing code will fall over.
>>>>
>>>> Signed-off-by: Dirk Brandewie<dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>
>>> I don't think 32 is a universal number; it should depend on the ABI.  On
>>> x86, for one thing, I'm pretty sure that there is no particular
>>> alignment requirements beyond the natural alignment of the data items,
>>> for example.
>>>
>>> Although 32 is probably conservative on any platform, please flag the
>>> origin of this with a comment, or make it a constant defined in a header
>>> file... otherwise, if this breaks for whatever reason it'll be near
>>> impossible to find.
>>>
>>
>> I will change . = ALIGN(32); to STRUCT_ALIGN();
>>
>> from vmlinux.lds.h:
>> /*
>>   * Align to a 32 byte boundary equal to the
>>   * alignment gcc 4.5 uses for a struct
>>   */
>> #define STRUCT_ALIGN() . = ALIGN(32)
>
> I also think this is the wrong place to do this.  scripts/dtc is a
> mirror of copy of upstream dtc, designed for making dtb images for
> general purposes.  I think the alignment should instead go into the
> linker script fragments you generate to incbin the dtbs.
>

I was sleep deprived when I responded to HPA I thought he had replied to the 
patch modifying vmlinux.lds.h since we had talked about that file a fair amount. 
So my first response had little bearing on patch in question.

I am trying to solve two issues with this change.

The structure needs to be 32 byte aligned for the code in fdt.c to be able parse 
the blob.  You are right I can force this alignment in the assembly fragment.

The second issue is being able to parse the section in the kernel image to find 
each of the blobs that have been concatenated together.  If the DTB size is 
modulo 32 bytes I can use  blob = blob+be32_to_cpu(blob->totalsize) to find the 
next blob in the section and use that address directly to have fdt.c parse the 
blob.  Otherwise I would need to search for the signature of the next blob 
somewhere past the end of the current blob which is knid of messy IMHO.

I have two questions,
Do you think this acceptable if I made forcing the alignment a command line 
argument?

Where is the DTC git tree I should be delivering patches against?  is 
git://www.jdl.com/software/dtc.git the correct tree?

Thanks
--Dirk

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[H. Peter Anvin]

On 11/12/2010 08:24 AM, Dirk Brandewie wrote:
> 
> The second issue is being able to parse the section in the kernel image to find 
> each of the blobs that have been concatenated together.  If the DTB size is 
> modulo 32 bytes I can use  blob = blob+be32_to_cpu(blob->totalsize) to find the 
> next blob in the section and use that address directly to have fdt.c parse the 
> blob.  Otherwise I would need to search for the signature of the next blob 
> somewhere past the end of the current blob which is knid of messy IMHO.
> 

Not if you know it is aligned mod <whatever> bytes, then you just do:

blob = ALIGN_UP(blob + be32_to_cpu(blob->totalsize), DTB_ALIGNMENT);

This, of course, makes it even more important that DTB_ALIGNMENT is a
common constant across the kernel build.  The nice part is that you can
just add:

	.balign DTB_ALIGNMENT

... into your generated .S files and all is good.

	-hpa

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[Jon Loeliger]

> 
> Where is the DTC git tree I should be delivering patches against?  is 
> git://www.jdl.com/software/dtc.git the correct tree?

Yep.  On this list.

Thanks!
jdl

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[David Gibson]

On Fri, Nov 12, 2010 at 08:24:53AM -0800, Dirk Brandewie wrote:
> Hi David,
> On 11/11/2010 05:16 PM, David Gibson wrote:
> >On Thu, Nov 11, 2010 at 05:01:16PM -0800, Dirk Brandewie wrote:
[snip]
> >I also think this is the wrong place to do this.  scripts/dtc is a
> >mirror of copy of upstream dtc, designed for making dtb images for
> >general purposes.  I think the alignment should instead go into the
> >linker script fragments you generate to incbin the dtbs.
> 
> I was sleep deprived when I responded to HPA I thought he had
> replied to the patch modifying vmlinux.lds.h since we had talked
> about that file a fair amount. So my first response had little
> bearing on patch in question.
> 
> I am trying to solve two issues with this change.
> 
> The structure needs to be 32 byte aligned for the code in fdt.c to
> be able parse the blob.  You are right I can force this alignment in
> the assembly fragment.
> 
> The second issue is being able to parse the section in the kernel
> image to find each of the blobs that have been concatenated
> together.  If the DTB size is modulo 32 bytes I can use  blob =
> blob+be32_to_cpu(blob->totalsize) to find the next blob in the
> section and use that address directly to have fdt.c parse the blob.
> Otherwise I would need to search for the signature of the next blob
> somewhere past the end of the current blob which is knid of messy
> IMHO.

As hpa says you can just align your increment too, to find the next
blob.

> I have two questions,
> Do you think this acceptable if I made forcing the alignment a
> command line argument?

Um, sure.  It's not actually necessary for what you're doing here, but
it might have some use, I guess, and it's pretty non-invasive.  Your
posted patch for this, however, is broken, see my comments in reply,

> Where is the DTC git tree I should be delivering patches against?
> is git://www.jdl.com/software/dtc.git the correct tree?

That's right.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [sodaville] [PATCH 3/4] of/dtc: force dtb size to modulo 32 bytes

[Grant Likely]

On Sun, Nov 14, 2010 at 11:44:09AM +1100, David Gibson wrote:
> On Fri, Nov 12, 2010 at 08:24:53AM -0800, Dirk Brandewie wrote:
[snip]
> > I am trying to solve two issues with this change.
> > 
> > The structure needs to be 32 byte aligned for the code in fdt.c to
> > be able parse the blob.  You are right I can force this alignment in
> > the assembly fragment.
> > 
> > The second issue is being able to parse the section in the kernel
> > image to find each of the blobs that have been concatenated
> > together.  If the DTB size is modulo 32 bytes I can use  blob =
> > blob+be32_to_cpu(blob->totalsize) to find the next blob in the
> > section and use that address directly to have fdt.c parse the blob.
> > Otherwise I would need to search for the signature of the next blob
> > somewhere past the end of the current blob which is knid of messy
> > IMHO.
> 
> As hpa says you can just align your increment too, to find the next
> blob.

+1

I agree, handle the alignment issues entirely within the kernel.  You
don't need to modify dtc to get the behaviour you want.

> 
> > I have two questions,
> > Do you think this acceptable if I made forcing the alignment a
> > command line argument?
> 
> Um, sure.  It's not actually necessary for what you're doing here, but
> it might have some use, I guess, and it's pretty non-invasive.  Your
> posted patch for this, however, is broken, see my comments in reply,

I disagree here.  True it is non-invasive, but I'd rather not be
adding yet another command line option unless there was a real
use-case for doing so.

g.

[PATCH 4/4] 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>
---
 drivers/of/fdt.c       |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_fdt.h |    9 ++++++++
 2 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index c1360e0..07fe4c6 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -604,3 +604,55 @@ void __init unflatten_device_tree(void)
 
 	pr_debug(" <- unflatten_device_tree()\n");
 }
+
+#ifndef MODULE
+#ifdef CONFIG_OF_FLATTREE
+static char dtb_compat_name[MAX_DTB_COMPAT_STR] = "";
+
+char __init *of_get_dtb_compatible_string(void)
+{
+	return dtb_compat_name;
+}
+
+#ifdef CONFIG_KERNEL_DTB
+extern char __dtb_start[];
+extern char __dtb_end[];
+
+void *of_find_compatible_dtb(char *name)
+{
+	void *rc = NULL;
+	char *hdr;
+	unsigned long node, root;
+	struct boot_param_header *blob, *orig_initial_boot_params;
+	
+	orig_initial_boot_params = initial_boot_params;
+	
+	if (__dtb_start != __dtb_end){
+		blob = (struct boot_param_header *)__dtb_start;
+		do{
+			node = ((unsigned long)blob) +  
+				be32_to_cpu(blob->off_dt_struct);
+			initial_boot_params = blob;
+			root = of_get_flat_dt_root();
+			if (of_flat_dt_is_compatible(root, name) > 0) {
+				rc = (void*) blob;
+				break;
+			}
+			blob = (unsigned long)blob+be32_to_cpu(blob->totalsize);
+		}while (blob < (struct boot_param_header *)__dtb_end);
+	}
+	if (rc == NULL)
+		initial_boot_params = orig_initial_boot_params;	
+	return rc;
+}
+#endif
+
+static int __init dtb_compat_setup(char *line)
+{
+	strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
+	return 1;
+}
+
+__setup("dtb_compat=", dtb_compat_setup);
+#endif
+#endif
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 7bbf5b3..181e413 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,13 @@ 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);
+#ifdef CONFIG_KERNEL_DTB
+extern void *of_find_compatible_dtb(char *name);
+#else
+static inline void *of_find_compatible_dtb(char *name) {return 0;}
+
+#endif 
 /*
  * 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 4/4] of/fdt: add kernel command line option for dtb_compat string

[Grant Likely]

On Thu, Nov 11, 2010 at 04:03:50PM -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>
> ---
>  drivers/of/fdt.c       |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_fdt.h |    9 ++++++++
>  2 files changed, 61 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c1360e0..07fe4c6 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -604,3 +604,55 @@ void __init unflatten_device_tree(void)
>  
>  	pr_debug(" <- unflatten_device_tree()\n");
>  }
> +
> +#ifndef MODULE
> +#ifdef CONFIG_OF_FLATTREE
> +static char dtb_compat_name[MAX_DTB_COMPAT_STR] = "";
> +
> +char __init *of_get_dtb_compatible_string(void)
> +{
> +	return dtb_compat_name;
> +}
> +
> +#ifdef CONFIG_KERNEL_DTB
> +extern char __dtb_start[];
> +extern char __dtb_end[];

'char' is completely bogus in this context.  Either void or struct
boot_param_header would be more accurate and useful.  I believe you
can do:

	extern void __dtb_start;
	extern_void __dtb_end;

And then reference the addresses with &__dtb_start and &__dtb_end.
That would eliminate a lot of the casting below.

> +
> +void *of_find_compatible_dtb(char *name)
> +{
> +	void *rc = NULL;
> +	char *hdr;
> +	unsigned long node, root;
> +	struct boot_param_header *blob, *orig_initial_boot_params;
> +	
> +	orig_initial_boot_params = initial_boot_params;
> +	
> +	if (__dtb_start != __dtb_end){

Don't bother with this if(); the empty condition can be handled in the
while (blob < __dtb_end){...} block.  That way a level of indentation
can be removed.

> +		blob = (struct boot_param_header *)__dtb_start;
> +		do{
> +			node = ((unsigned long)blob) +  
> +				be32_to_cpu(blob->off_dt_struct);

Ugly casting.  I would work with everything as pointers (make node and
blob void*) and only cast when necessary....

wait....

'node' isn't actually used anywhere.  Why is this here?

> +			initial_boot_params = blob;

Blech!  I really need to get Stephen (cc'd) to respin his "of/fdt: Add
unflatten_partial_device_tree" patch that decouples the flattree
functions from initial_boot_params.  Having to do it this way is just
plain ugly.

Stephen: nudge, nudge.

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

Unnecessary cast.  Any regular pointer can always be assigned to a void*
variable without a cast..

> +				break;
> +			}
> +			blob = (unsigned long)blob+be32_to_cpu(blob->totalsize);
> +		}while (blob < (struct boot_param_header *)__dtb_end);

Minor coding convention violations.  It is worth running it through
checkpatch.pl before submitting.  checkpatch may not proclaim the
gospel, but it is a useful tool.

> +	}
> +	if (rc == NULL)
> +		initial_boot_params = orig_initial_boot_params;	
> +	return rc;
> +}
> +#endif
> +
> +static int __init dtb_compat_setup(char *line)
> +{
> +	strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
> +	return 1;
> +}
> +
> +__setup("dtb_compat=", dtb_compat_setup);

Could the searching of the linked-in dtbs be handled here at __setup
time?  That would eliminate the need for a fixed size dtb_compat_name
variable.

Also need to add documentation for dtb_compat parameter to
Documentation/kernel-parameters.txt

> +#endif
> +#endif
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 7bbf5b3..181e413 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
> +

Nothing outside fdt.c uses MAC_DTB_COMPAT_STR, so this define should
also be in fdt.c

>  /* 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,13 @@ 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);
> +#ifdef CONFIG_KERNEL_DTB
> +extern void *of_find_compatible_dtb(char *name);
> +#else
> +static inline void *of_find_compatible_dtb(char *name) {return 0;}
> +
> +#endif 
>  /*
>   * 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
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

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

[Stephen Neuendorffer]

[-- Attachment #1.1: Type: text/plain, Size: 809 bytes --]



> > +			initial_boot_params = blob;

> Blech!  I really need to get Stephen (cc'd) to respin his "of/fdt: Add
> unflatten_partial_device_tree" patch that decouples the flattree
> functions from initial_boot_params.  Having to do it this way is just
> plain ugly.

> Stephen: nudge, nudge.

If things have settled down, I'll dust them off again.
Don't know if it will happen this week: I'm called for jury duty.

Steve


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.


[-- Attachment #1.2: Type: text/html, Size: 1431 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

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

[Grant Likely]

On Sat, Nov 13, 2010 at 11:26 PM, Stephen Neuendorffer
<stephen.neuendorffer-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>
>
>> > +                   initial_boot_params = blob;
>
>> Blech!  I really need to get Stephen (cc'd) to respin his "of/fdt: Add
>> unflatten_partial_device_tree" patch that decouples the flattree
>> functions from initial_boot_params.  Having to do it this way is just
>> plain ugly.
>
>> Stephen: nudge, nudge.
>
> If things have settled down, I'll dust them off again.
> Don't know if it will happen this week: I'm called for jury duty.

Ouch!  Have fun.

g.

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

[H. Peter Anvin]

On 11/13/2010 10:22 PM, Grant Likely wrote:
> 
> 'char' is completely bogus in this context.  Either void or struct
> boot_param_header would be more accurate and useful.  I believe you
> can do:
> 
> 	extern void __dtb_start;
> 	extern_void __dtb_end;
> 
> And then reference the addresses with &__dtb_start and &__dtb_end.
> That would eliminate a lot of the casting below.
> 

Except that doesn't work with gcc 4.5+; you really need to use an array
type.

	-hpa

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

[Grant Likely]

On Mon, Nov 15, 2010 at 04:27:06PM -0800, H. Peter Anvin wrote:
> On 11/13/2010 10:22 PM, Grant Likely wrote:
> > 
> > 'char' is completely bogus in this context.  Either void or struct
> > boot_param_header would be more accurate and useful.  I believe you
> > can do:
> > 
> > 	extern void __dtb_start;
> > 	extern_void __dtb_end;
> > 
> > And then reference the addresses with &__dtb_start and &__dtb_end.
> > That would eliminate a lot of the casting below.
> > 
> 
> Except that doesn't work with gcc 4.5+; you really need to use an array
> type.

heh.  Oh well, an array it must be then.

g.