2010-11-16 22:42:16

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 0/5] Add the ability to link device blobs into vmlinux

From: Dirk Brandewie <[email protected]>

This patch set adds the ability to link device tree blob(s)
directly into the vmlinux image and specify the blob to be used via a
kernel command line option.

Patch 1 implements the changes to include/asm-generic/vmlinux.lds.h,
add generic rules for building and linking the DTB's into vmlinux.

Patch 2 adds the command line option, the functions for the
platform code to retrieve the value passed in and locate the matching
blob in the image. This patch has been tested on x86.

Patch 3-5 show using the generic dts->dtb rule in x86, microblaze and
powerpc. The microblaze and powerpc patches have only been compile
tested.

Dirk Brandewie (5):
of: Add support for linking device tree blobs into vmlinux
of/fdt: add kernel command line option for dtb_compat string
x86/of: Add building device tree blob(s) into image.
of/powerpc: Move build to use generic dts->dtb rule
of/microblaze: Move build to use generic dts->dtb rule

Documentation/kernel-parameters.txt | 7 +++++
arch/microblaze/boot/Makefile | 13 +--------
arch/powerpc/boot/Makefile | 7 -----
arch/x86/Kconfig | 6 +++-
arch/x86/kernel/Makefile | 6 ++++
arch/x86/kernel/dts/Kconfig | 7 +++++
drivers/of/fdt.c | 52 +++++++++++++++++++++++++++++++++++
include/asm-generic/vmlinux.lds.h | 19 +++++++++++-
include/linux/of_fdt.h | 4 +++
scripts/Makefile.lib | 20 +++++++++++++
10 files changed, 119 insertions(+), 22 deletions(-)
create mode 100644 arch/x86/kernel/dts/Kconfig

--
1.7.2.3


2010-11-16 22:42:18

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

From: Dirk Brandewie <[email protected]>

This patch adds support for linking device tree blobs into
vmlinux. 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.

The DTB's are placed on 32 byte boundries to allow parsing the blob
with driver/of/fdt.c during early boot without having to copy the blob
to get the structure alignment GCC expects.

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 += foo.dtb.o

Signed-off-by: Dirk Brandewie <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 19 +++++++++++++++++--
scripts/Makefile.lib | 20 ++++++++++++++++++++
2 files changed, 37 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..29db062 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -200,6 +200,26 @@ 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 $(DTS_FLAGS) $(src)/dts/$*.dts
+
+$(obj)/%.dtb: $(src)/dts/%.dts
+ $(call if_changed,dtc)

# Bzip2
# ---------------------------------------------------------------------------
--
1.7.2.3

2010-11-16 22:42:21

by Dirk Brandewie

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

From: Dirk Brandewie <[email protected]>

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

of_flat_dt_get_dtb_compatible_string() returns a pointer string passed
from the command line or a pointer to an empty string.

of_flat_dt_find_compatible_dtb() returns a pointer to a DTB that is
compatible with the "compatible" string or a NULL pointer if no
matching DTB is present.

Signed-off-by: Dirk Brandewie <[email protected]>
---
Documentation/kernel-parameters.txt | 7 +++++
drivers/of/fdt.c | 52 +++++++++++++++++++++++++++++++++++
include/linux/of_fdt.h | 4 +++
3 files changed, 63 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..c0858ce 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,53 @@ void __init unflatten_device_tree(void)

pr_debug(" <- unflatten_device_tree()\n");
}
+
+#define MAX_DTB_COMPAT_STR 64
+char dtb_compat_name[MAX_DTB_COMPAT_STR] = "";
+
+char __init *of_flat_dt_get_dtb_compatible_string(void)
+{
+ return dtb_compat_name;
+}
+
+
+extern uint8_t __dtb_start[];
+extern uint8_t __dtb_end[];
+void __init *of_flat_dt_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;
+
+ while (blob < __dtb_end &&
+ (be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER)) {
+ root = of_get_flat_dt_root();
+ if (of_flat_dt_is_compatible(root, name) > 0) {
+ rc = blob;
+ break;
+ }
+
+ size = be32_to_cpu(initial_boot_params->totalsize);
+ blob = PTR_ALIGN(blob + size, DTB_ALIGNMENT);
+ initial_boot_params = (struct boot_param_header *)blob;
+ }
+
+ if (rc == NULL)
+ initial_boot_params = orig_initial_boot_params;
+ return rc;
+}
+
+
+static int __init of_flat_dtb_compat_setup(char *line)
+{
+ strncpy(dtb_compat_name, line, MAX_DTB_COMPAT_STR);
+ return 1;
+}
+
+early_param("dtb_compat", of_flat_dtb_compat_setup);
+
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 7bbf5b3..29561f4 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -58,6 +58,7 @@ struct boot_param_header {
};

#if defined(CONFIG_OF_FLATTREE)
+
/* 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 +83,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_flat_dt_get_dtb_compatible_string(void);
+extern void *of_flat_dt_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

2010-11-16 22:42:24

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 4/5] of/powerpc: Move build to use generic dts->dtb rule

From: Dirk Brandewie <[email protected]>

This patch changes arch/powerpc/boot/Makefile to use the generic
rule build the device tree blobs in scripts/Makefile.lib

Signed-off-by: Dirk Brandewie <[email protected]>
---
arch/powerpc/boot/Makefile | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index fae8192..d90c674 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -147,8 +147,6 @@ targets += $(patsubst $(obj)/%,%,$(obj-boot) wrapper.a)
extra-y := $(obj)/wrapper.a $(obj-plat) $(obj)/empty.o \
$(obj)/zImage.lds $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds

-dtstree := $(srctree)/$(src)/dts
-
wrapper :=$(srctree)/$(src)/wrapper
wrapperbits := $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
$(wrapper) FORCE
@@ -331,11 +329,6 @@ $(obj)/treeImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits)
$(obj)/treeImage.%: vmlinux $(obj)/%.dtb $(wrapperbits)
$(call if_changed,wrap,treeboot-$*,,$(obj)/$*.dtb)

-# Rule to build device tree blobs
-DTC = $(objtree)/scripts/dtc/dtc
-
-$(obj)/%.dtb: $(dtstree)/%.dts
- $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(dtstree)/$*.dts

# If there isn't a platform selected then just strip the vmlinux.
ifeq (,$(image-y))
--
1.7.2.3

2010-11-16 22:42:46

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 5/5] of/microblaze: Move build to use generic dts->dtb rule

From: Dirk Brandewie <[email protected]>

This patch changes arch/microblaze/boot/Makefile to use the generic
rule build the device tree blobs in scripts/Makefile.lib

Signed-off-by: Dirk Brandewie <[email protected]>
---
arch/microblaze/boot/Makefile | 13 +------------
1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
index be01d78..4fda765 100644
--- a/arch/microblaze/boot/Makefile
+++ b/arch/microblaze/boot/Makefile
@@ -10,9 +10,6 @@ targets := linux.bin linux.bin.gz simpleImage.%

OBJCOPYFLAGS := -O binary

-# Where the DTS files live
-dtstree := $(srctree)/$(src)/dts
-
# Ensure system.dtb exists
$(obj)/linked_dtb.o: $(obj)/system.dtb

@@ -51,14 +48,6 @@ $(obj)/simpleImage.%: vmlinux FORCE
$(call if_changed,strip)
@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'

-# Rule to build device tree blobs
-DTC = $(objtree)/scripts/dtc/dtc
-
-# Rule to build device tree blobs
-quiet_cmd_dtc = DTC $@
- cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 -p 1024 $(dtstree)/$*.dts
-
-$(obj)/%.dtb: $(dtstree)/%.dts FORCE
- $(call if_changed,dtc)
+DTS_FLAGS := -p 1024

clean-files += *.dtb simpleImage.*.unstrip linux.bin.ub
--
1.7.2.3

2010-11-16 22:43:07

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 3/5] x86/of: Add building device tree blob(s) into image.

From: Dirk Brandewie <[email protected]>

This patch adds linking device tree blobs into vmlinux. DTB's are
added by adding the blob object name to list of objects to be linked
into the image.
e.g:
obj-$(CONFIG_TEST_DTB) += test.dtb.o

The set of DTB linked into the image is controlled the Kconfig file
in arch/x86/kernel/dts/Kconfig

Signed-off-by: Dirk Brandewie <[email protected]>
---
arch/x86/Kconfig | 6 +++++-
arch/x86/kernel/Makefile | 6 ++++++
arch/x86/kernel/dts/Kconfig | 7 +++++++
3 files changed, 18 insertions(+), 1 deletions(-)
create mode 100644 arch/x86/kernel/dts/Kconfig

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5904f38..f2f516a 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/kernel/dts/Kconfig
+endif
+
if X86_32
config X86_EXTENDED_PLATFORM
bool "Support for extended (non-PC) x86 platforms"
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 586df14..49e017d 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -114,6 +114,12 @@ obj-$(CONFIG_SWIOTLB) += pci-swiotlb.o
obj-$(CONFIG_X86_OF) += prom.o

###
+# device tree blobs
+obj-$(CONFIG_CE4100_DTB) += ce4100.dtb.o
+obj-$(CONFIG_TEST_DTB) += test.dtb.o
+
+
+###
# 64 bit specific files
ifeq ($(CONFIG_X86_64),y)
obj-$(CONFIG_AUDIT) += audit_64.o
diff --git a/arch/x86/kernel/dts/Kconfig b/arch/x86/kernel/dts/Kconfig
new file mode 100644
index 0000000..d3e5cd4
--- /dev/null
+++ b/arch/x86/kernel/dts/Kconfig
@@ -0,0 +1,7 @@
+config CE4100_DTB
+ bool "Intel CE4100"
+
+config TEST_DTB
+ bool "Test DTS"
+
+
--
1.7.2.3

2010-11-17 00:16:37

by Grant Likely

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

On Tue, Nov 16, 2010 at 3:41 PM, <[email protected]> wrote:
> From: Dirk Brandewie <[email protected]>
>
> 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
>
> of_flat_dt_get_dtb_compatible_string() returns a pointer string passed
> from the command line or a pointer to an empty string.
>
> of_flat_dt_find_compatible_dtb() returns a pointer to a DTB that is
> compatible with the "compatible" string or a NULL pointer if no
> matching DTB is present.
>
> Signed-off-by: Dirk Brandewie <[email protected]>
> ---
[...]
> +void __init *of_flat_dt_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;
> +
> + ? ? ? while (blob < __dtb_end &&
> + ? ? ? ? ? ? ? (be32_to_cpu(initial_boot_params->magic) == OF_DT_HEADER)) {

The kernel needs to complain *loudly* if this occurs because it
represents a bug. I'm tempted to say use BUG, but that would halt the
kernel and prevent any possibility of kernel log output. WARN_ON() is
probably best.

g.

2010-11-17 00:39:49

by David Daney

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

Thanks for doing this. However I have a few comments...

On 11/16/2010 02:41 PM, [email protected] wrote:
> From: Dirk Brandewie<[email protected]>
>
> This patch adds support for linking device tree blobs into
> vmlinux. 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.
>
> The DTB's are placed on 32 byte boundries to allow parsing the blob
> with driver/of/fdt.c during early boot without having to copy the blob
> to get the structure alignment GCC expects.
>
> 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 += foo.dtb.o
>
> Signed-off-by: Dirk Brandewie<[email protected]>
> ---
> include/asm-generic/vmlinux.lds.h | 19 +++++++++++++++++--
> scripts/Makefile.lib | 20 ++++++++++++++++++++
> 2 files changed, 37 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()
>

I thought the init.rodata was only for data used by __init things.
Although the current linker scripts do not put it in the section that
gets recycled as usable memory.

IIRC the unflattened version of the device tree has pointers to the
flattened data. Since the device tree nodes are live for the entire
kernel lifecycle, shouldn't the device tree blobs be in non-init memory?


> #define INIT_TEXT \
> *(.init.text) \
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 4c72c11..29db062 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -200,6 +200,26 @@ quiet_cmd_gzip = GZIP $@
> cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9> $@) || \
> (rm -f $@ ; false)
>
> +# DTC
> +# ---------------------------------------------------------------------------
> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE

Why 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 $(DTS_FLAGS) $(src)/dts/$*.dts
> +
> +$(obj)/%.dtb: $(src)/dts/%.dts
> + $(call if_changed,dtc)
>

Do all the generated files get cleaned up?

I will try it tomorrow to see for sure.


Thanks,
David Daney

2010-11-17 02:21:14

by Dirk Brandewie

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux


On 11/16/2010 04:39 PM, David Daney wrote:
> Thanks for doing this. However I have a few comments...
>
> On 11/16/2010 02:41 PM, [email protected] wrote:
>> From: Dirk Brandewie<[email protected]>
>>
>> /* .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()
>>
>
> I thought the init.rodata was only for data used by __init things. Although the
> current linker scripts do not put it in the section that gets recycled as usable
> memory.
>
> IIRC the unflattened version of the device tree has pointers to the flattened
> data. Since the device tree nodes are live for the entire kernel lifecycle,
> shouldn't the device tree blobs be in non-init memory?
>

The contents of the blob get copied to allocated memory during
unflatten_device_tree() so the blob that is linked in is no longer needed after
init.

>
>> #define INIT_TEXT \
>> *(.init.text) \
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 4c72c11..29db062 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -200,6 +200,26 @@ quiet_cmd_gzip = GZIP $@
>> cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9> $@) || \
>> (rm -f $@ ; false)
>>
>> +# DTC
>> +# ---------------------------------------------------------------------------
>> +$(obj)/%.dtb.S: $(obj)/%.dtb FORCE
>
> Why FORCE?
Crud left over from debugging I will remove.
>
>
>> + @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 $(DTS_FLAGS) $(src)/dts/$*.dts
>> +
>> +$(obj)/%.dtb: $(src)/dts/%.dts
>> + $(call if_changed,dtc)
>>
>
> Do all the generated files get cleaned up?
>
> I will try it tomorrow to see for sure.
>
Looks like I need to add the generated .S files to clean-files

>
> Thanks,
> David Daney

2010-11-17 02:59:18

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

On Tue, Nov 16, 2010 at 7:21 PM, Dirk Brandewie
<[email protected]> wrote:
>
> On 11/16/2010 04:39 PM, David Daney wrote:
>>
>> Thanks for doing this. However I have a few comments...
>>
>> On 11/16/2010 02:41 PM, [email protected] wrote:
>>>
>>> From: Dirk Brandewie<[email protected]>
>>>
>>> /* .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()
>>>
>>
>> I thought the init.rodata was only for data used by __init things.
>> Although the
>> current linker scripts do not put it in the section that gets recycled as
>> usable
>> memory.
>>
>> IIRC the unflattened version of the device tree has pointers to the
>> flattened
>> data. Since the device tree nodes are live for the entire kernel
>> lifecycle,
>> shouldn't the device tree blobs be in non-init memory?
>>
>
> The contents of the blob get copied to allocated memory during
> unflatten_device_tree() so the blob that is linked in is no longer needed
> after init.

Have you written a patch to add this behaviour? The current code doesn't. :-)

g.

2010-11-17 06:02:34

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 3/5] x86/of: Add building device tree blob(s) into image.

On Tue, Nov 16, 2010 at 02:41:38PM -0800, [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> This patch adds linking device tree blobs into vmlinux. DTB's are
> added by adding the blob object name to list of objects to be linked
> into the image.
> e.g:
> obj-$(CONFIG_TEST_DTB) += test.dtb.o
>
> The set of DTB linked into the image is controlled the Kconfig file
> in arch/x86/kernel/dts/Kconfig
>
> Signed-off-by: Dirk Brandewie <[email protected]>
> ---
> arch/x86/Kconfig | 6 +++++-
> arch/x86/kernel/Makefile | 6 ++++++
> arch/x86/kernel/dts/Kconfig | 7 +++++++
> 3 files changed, 18 insertions(+), 1 deletions(-)
> create mode 100644 arch/x86/kernel/dts/Kconfig
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5904f38..f2f516a 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/kernel/dts/Kconfig
> +endif
> +
> if X86_32
> config X86_EXTENDED_PLATFORM
> bool "Support for extended (non-PC) x86 platforms"
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 586df14..49e017d 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -114,6 +114,12 @@ obj-$(CONFIG_SWIOTLB) += pci-swiotlb.o
> obj-$(CONFIG_X86_OF) += prom.o
>
> ###
> +# device tree blobs
> +obj-$(CONFIG_CE4100_DTB) += ce4100.dtb.o
> +obj-$(CONFIG_TEST_DTB) += test.dtb.o
> +
> +
> +###
> # 64 bit specific files
> ifeq ($(CONFIG_X86_64),y)
> obj-$(CONFIG_AUDIT) += audit_64.o
> diff --git a/arch/x86/kernel/dts/Kconfig b/arch/x86/kernel/dts/Kconfig
> new file mode 100644
> index 0000000..d3e5cd4
> --- /dev/null
> +++ b/arch/x86/kernel/dts/Kconfig
> @@ -0,0 +1,7 @@
> +config CE4100_DTB
> + bool "Intel CE4100"
> +
> +config TEST_DTB
> + bool "Test DTS"
> +
> +

As previously mentioned, this isn't going to scale. Need to look at
allowing the user to specify a list of .dtbs that will be linked in.

g.

> --
> 1.7.2.3
>
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss

2010-11-17 06:07:00

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 4/5] of/powerpc: Move build to use generic dts->dtb rule

On Tue, Nov 16, 2010 at 02:41:39PM -0800, [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> This patch changes arch/powerpc/boot/Makefile to use the generic
> rule build the device tree blobs in scripts/Makefile.lib
>
> Signed-off-by: Dirk Brandewie <[email protected]>
> ---
> arch/powerpc/boot/Makefile | 7 -------
> 1 files changed, 0 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index fae8192..d90c674 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -147,8 +147,6 @@ targets += $(patsubst $(obj)/%,%,$(obj-boot) wrapper.a)
> extra-y := $(obj)/wrapper.a $(obj-plat) $(obj)/empty.o \
> $(obj)/zImage.lds $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds
>
> -dtstree := $(srctree)/$(src)/dts
> -
> wrapper :=$(srctree)/$(src)/wrapper
> wrapperbits := $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
> $(wrapper) FORCE
> @@ -331,11 +329,6 @@ $(obj)/treeImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits)
> $(obj)/treeImage.%: vmlinux $(obj)/%.dtb $(wrapperbits)
> $(call if_changed,wrap,treeboot-$*,,$(obj)/$*.dtb)
>
> -# Rule to build device tree blobs
> -DTC = $(objtree)/scripts/dtc/dtc
> -
> -$(obj)/%.dtb: $(dtstree)/%.dts
> - $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(dtstree)/$*.dts

This patch needs to modify the rules that have %.dtb dependencies to
go looking in arch/powerpc/boot/dts instead of arch/powerpc/boot
because the rule change will change where .dtb files get generated.

Also, this patch and patch 5 need to be merged with patch 1 so that
the series remains bisectable.

g.

>
> # If there isn't a platform selected then just strip the vmlinux.
> ifeq (,$(image-y))
> --
> 1.7.2.3
>
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss

2010-11-17 06:14:07

by Dirk Brandewie

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

On 11/16/2010 06:58 PM, Grant Likely wrote:
> On Tue, Nov 16, 2010 at 7:21 PM, Dirk Brandewie
> <[email protected]> wrote:
>>
>> On 11/16/2010 04:39 PM, David Daney wrote:
>>>
>>> Thanks for doing this. However I have a few comments...
>>>
>>> On 11/16/2010 02:41 PM, [email protected] wrote:
>>>>
>>>> From: Dirk Brandewie<[email protected]>
>>>>
>>>> /* .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()
>>>>
>>>
>>> I thought the init.rodata was only for data used by __init things.
>>> Although the
>>> current linker scripts do not put it in the section that gets recycled as
>>> usable
>>> memory.
>>>
>>> IIRC the unflattened version of the device tree has pointers to the
>>> flattened
>>> data. Since the device tree nodes are live for the entire kernel
>>> lifecycle,
>>> shouldn't the device tree blobs be in non-init memory?
>>>
>>
>> The contents of the blob get copied to allocated memory during
>> unflatten_device_tree() so the blob that is linked in is no longer needed
>> after init.
>
> Have you written a patch to add this behaviour? The current code doesn't. :-)
>

I misspoke, my blob gets copied to allocated memory during unflatten_device_tree.
my early_init_dt_alloc_memory_arch() returns the physical address of a kmalloc'd
buffer.

You would want copy the dtb that your platform is going to use to non-init memory.

--Dirk

2010-11-17 06:32:51

by Dirk Brandewie

[permalink] [raw]
Subject: Re: [PATCH 4/5] of/powerpc: Move build to use generic dts->dtb rule

On 11/16/2010 10:06 PM, Grant Likely wrote:
> On Tue, Nov 16, 2010 at 02:41:39PM -0800, [email protected] wrote:
>> From: Dirk Brandewie<[email protected]>
>>
>> This patch changes arch/powerpc/boot/Makefile to use the generic
>> rule build the device tree blobs in scripts/Makefile.lib
>>
>> Signed-off-by: Dirk Brandewie<[email protected]>
>> ---
>> arch/powerpc/boot/Makefile | 7 -------
>> 1 files changed, 0 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
>> index fae8192..d90c674 100644
>> --- a/arch/powerpc/boot/Makefile
>> +++ b/arch/powerpc/boot/Makefile
>> @@ -147,8 +147,6 @@ targets += $(patsubst $(obj)/%,%,$(obj-boot) wrapper.a)
>> extra-y := $(obj)/wrapper.a $(obj-plat) $(obj)/empty.o \
>> $(obj)/zImage.lds $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds
>>
>> -dtstree := $(srctree)/$(src)/dts
>> -
>> wrapper :=$(srctree)/$(src)/wrapper
>> wrapperbits := $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
>> $(wrapper) FORCE
>> @@ -331,11 +329,6 @@ $(obj)/treeImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits)
>> $(obj)/treeImage.%: vmlinux $(obj)/%.dtb $(wrapperbits)
>> $(call if_changed,wrap,treeboot-$*,,$(obj)/$*.dtb)
>>
>> -# Rule to build device tree blobs
>> -DTC = $(objtree)/scripts/dtc/dtc
>> -
>> -$(obj)/%.dtb: $(dtstree)/%.dts
>> - $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(dtstree)/$*.dts
>
> This patch needs to modify the rules that have %.dtb dependencies to
> go looking in arch/powerpc/boot/dts instead of arch/powerpc/boot
> because the rule change will change where .dtb files get generated.

The rule in patch 1 takes of this. The dts directory is relative to the
arch/powerpc/boot/Makefile
+quiet_cmd_dtc = DTC $@
+ cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(src)/dts/$*.dts
+

>
> Also, this patch and patch 5 need to be merged with patch 1 so that
> the series remains bisectable.

I tested building a powerpc image with patch 1 applied and patch 4 unapplied and
the image built without errors and the dtb was present in the wrapped image.
unfortunately I don't have a system to do runtime testing.
I could not build the uboot image type I am missing mkimage. I did build
the dtbImage, zImage and cuImage types before and after this patch was applied.

--Dirk
>
> g.
>
>>
>> # If there isn't a platform selected then just strip the vmlinux.
>> ifeq (,$(image-y))
>> --
>> 1.7.2.3
>>
>> _______________________________________________
>> devicetree-discuss mailing list
>> [email protected]
>> https://lists.ozlabs.org/listinfo/devicetree-discuss

2010-11-17 06:43:53

by Dirk Brandewie

[permalink] [raw]
Subject: Re: [PATCH 3/5] x86/of: Add building device tree blob(s) into image.

On 11/16/2010 10:02 PM, Grant Likely wrote:
> On Tue, Nov 16, 2010 at 02:41:38PM -0800, [email protected] wrote:
>> From: Dirk Brandewie<[email protected]>
>>
>> This patch adds linking device tree blobs into vmlinux. DTB's are
>> added by adding the blob object name to list of objects to be linked
>> into the image.
>> e.g:
>> obj-$(CONFIG_TEST_DTB) += test.dtb.o
>>
>> The set of DTB linked into the image is controlled the Kconfig file
>> in arch/x86/kernel/dts/Kconfig
>>
>> Signed-off-by: Dirk Brandewie<[email protected]>
>> ---
>> arch/x86/Kconfig | 6 +++++-
>> arch/x86/kernel/Makefile | 6 ++++++
>> arch/x86/kernel/dts/Kconfig | 7 +++++++
>> 3 files changed, 18 insertions(+), 1 deletions(-)
>> create mode 100644 arch/x86/kernel/dts/Kconfig
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 5904f38..f2f516a 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/kernel/dts/Kconfig
>> +endif
>> +
>> if X86_32
>> config X86_EXTENDED_PLATFORM
>> bool "Support for extended (non-PC) x86 platforms"
>> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>> index 586df14..49e017d 100644
>> --- a/arch/x86/kernel/Makefile
>> +++ b/arch/x86/kernel/Makefile
>> @@ -114,6 +114,12 @@ obj-$(CONFIG_SWIOTLB) += pci-swiotlb.o
>> obj-$(CONFIG_X86_OF) += prom.o
>>
>> ###
>> +# device tree blobs
>> +obj-$(CONFIG_CE4100_DTB) += ce4100.dtb.o
>> +obj-$(CONFIG_TEST_DTB) += test.dtb.o
>> +
>> +
>> +###
>> # 64 bit specific files
>> ifeq ($(CONFIG_X86_64),y)
>> obj-$(CONFIG_AUDIT) += audit_64.o
>> diff --git a/arch/x86/kernel/dts/Kconfig b/arch/x86/kernel/dts/Kconfig
>> new file mode 100644
>> index 0000000..d3e5cd4
>> --- /dev/null
>> +++ b/arch/x86/kernel/dts/Kconfig
>> @@ -0,0 +1,7 @@
>> +config CE4100_DTB
>> + bool "Intel CE4100"
>> +
>> +config TEST_DTB
>> + bool "Test DTS"
>> +
>> +
>
> As previously mentioned, this isn't going to scale. Need to look at
> allowing the user to specify a list of .dtbs that will be linked in.
>

These config variables will likely get pushed into being set when the
platform configuration is selected. I still need to talk to the x86 maintainers
and my distribution team to figure out what is going to happen here.

> g.
>
>> --
>> 1.7.2.3
>>
>> _______________________________________________
>> devicetree-discuss mailing list
>> [email protected]
>> https://lists.ozlabs.org/listinfo/devicetree-discuss

2010-11-17 09:27:57

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

On Tue, Nov 16, 2010 at 02:41:36PM -0800, [email protected] wrote:
> From: Dirk Brandewie <[email protected]>
>
> This patch adds support for linking device tree blobs into
> vmlinux. 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.
>
> The DTB's are placed on 32 byte boundries to allow parsing the blob
> with driver/of/fdt.c during early boot without having to copy the blob
> to get the structure alignment GCC expects.
>
> 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 += foo.dtb.o
>
> Signed-off-by: Dirk Brandewie <[email protected]>
> ---
> include/asm-generic/vmlinux.lds.h | 19 +++++++++++++++++--
> scripts/Makefile.lib | 20 ++++++++++++++++++++
> 2 files changed, 37 insertions(+), 2 deletions(-)

When you touch Makefiles in scripts/* it is always a good idea to cc:
kbuild maintainer on the patch - I have added Michal.

Support functionality in Makefile.lib is documented in
Documentation/kbuild/* - please add documentation there.

>
> 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

It has been discussed in another thread some time ago to move
to a general 32 byte alignment for everything in vmlinux.lds.h
So there is not much need for the specific DTB alignment.

> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 4c72c11..29db062 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -200,6 +200,26 @@ 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' >> $@


This will be noisy during build. Please use proper macors to supress output.


> +
> +DTC = $(objtree)/scripts/dtc/dtc
> +
> +quiet_cmd_dtc = DTC $@
Please avoid tabs in the output - all other uses spaces. (There is a tab between DTC and $@)

> + cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(src)/dts/$*.dts

Looks strange. How about:
cmd_dtc = $(DTC) -O dtb -o $@ -b 0 $(DTS_FLAGS) $<

Then you avoid the hardcoded path in the rule too.


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

This snippet belong in the file that uses this.
This is how we do for other rules like bzip etc.

Sam

2010-11-17 17:54:16

by David Daney

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

On 11/16/2010 10:14 PM, Dirk Brandewie wrote:
> On 11/16/2010 06:58 PM, Grant Likely wrote:
>> On Tue, Nov 16, 2010 at 7:21 PM, Dirk Brandewie
>> <[email protected]> wrote:
>>>
>>> On 11/16/2010 04:39 PM, David Daney wrote:
>>>>
>>>> Thanks for doing this. However I have a few comments...
>>>>
>>>> On 11/16/2010 02:41 PM, [email protected] wrote:
>>>>>
>>>>> From: Dirk Brandewie<[email protected]>
>>>>>
>>>>> /* .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()
>>>>>
>>>>
>>>> I thought the init.rodata was only for data used by __init things.
>>>> Although the
>>>> current linker scripts do not put it in the section that gets
>>>> recycled as
>>>> usable
>>>> memory.
>>>>
>>>> IIRC the unflattened version of the device tree has pointers to the
>>>> flattened
>>>> data. Since the device tree nodes are live for the entire kernel
>>>> lifecycle,
>>>> shouldn't the device tree blobs be in non-init memory?
>>>>
>>>
>>> The contents of the blob get copied to allocated memory during
>>> unflatten_device_tree() so the blob that is linked in is no longer
>>> needed
>>> after init.
>>
>> Have you written a patch to add this behaviour? The current code
>> doesn't. :-)
>>
>
> I misspoke, my blob gets copied to allocated memory during
> unflatten_device_tree.
> my early_init_dt_alloc_memory_arch() returns the physical address of a
> kmalloc'd
> buffer.
>

Perhaps you should take a look at unflatten_dt_node(), especially the
part where property names and values are assigned. I could be mistaken,
but it appears to me that the memory allocated by
early_init_dt_alloc_memory_arch() is not used to hold the name and value
strings. It is possible that they might be referred to in their
original location in the flattened blob.


> You would want copy the dtb that your platform is going to use to
> non-init memory.
>

... or you might want to locate the dtb somewhere where it would be
unlikely to get clobbered if someone were to arrange for the init.rodata
to be freed for reuse.

David Daney

2010-11-17 18:07:31

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

On Wed, Nov 17, 2010 at 10:27:51AM +0100, Sam Ravnborg wrote:
> On Tue, Nov 16, 2010 at 02:41:36PM -0800, [email protected] wrote:
> > From: Dirk Brandewie <[email protected]>
> >
> > This patch adds support for linking device tree blobs into
> > vmlinux. 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.
> >
> > The DTB's are placed on 32 byte boundries to allow parsing the blob
> > with driver/of/fdt.c during early boot without having to copy the blob
> > to get the structure alignment GCC expects.
> >
> > 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 += foo.dtb.o
> >
> > Signed-off-by: Dirk Brandewie <[email protected]>
> > ---
> > include/asm-generic/vmlinux.lds.h | 19 +++++++++++++++++--
> > scripts/Makefile.lib | 20 ++++++++++++++++++++
> > 2 files changed, 37 insertions(+), 2 deletions(-)
>
> When you touch Makefiles in scripts/* it is always a good idea to cc:
> kbuild maintainer on the patch - I have added Michal.
>
> Support functionality in Makefile.lib is documented in
> Documentation/kbuild/* - please add documentation there.
>
> >
> > 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
>
> It has been discussed in another thread some time ago to move
> to a general 32 byte alignment for everything in vmlinux.lds.h
> So there is not much need for the specific DTB alignment.
>
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index 4c72c11..29db062 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -200,6 +200,26 @@ 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' >> $@
>
>
> This will be noisy during build. Please use proper macors to supress output.
>
>
> > +
> > +DTC = $(objtree)/scripts/dtc/dtc
> > +
> > +quiet_cmd_dtc = DTC $@
> Please avoid tabs in the output - all other uses spaces. (There is a tab between DTC and $@)
>
> > + cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(src)/dts/$*.dts
>
> Looks strange. How about:
> cmd_dtc = $(DTC) -O dtb -o $@ -b 0 $(DTS_FLAGS) $<
>
> Then you avoid the hardcoded path in the rule too.
>
>
> > +
> > +$(obj)/%.dtb: $(src)/dts/%.dts
> > + $(call if_changed,dtc)

The rule should be generic (not depend on the presence of a dts
subdirectory. Basically, the .dtb really should be generated in the
same directory as the .dts file. There is no reason for this rule to
have special behaviour.

>
> This snippet belong in the file that uses this.
> This is how we do for other rules like bzip etc.

This rule is intended to be generic and usable anywhere in the tree.

g.

2010-11-17 20:24:38

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/5] of: Add support for linking device tree blobs into vmlinux

> > > +
> > > +DTC = $(objtree)/scripts/dtc/dtc
> > > +
> > > +quiet_cmd_dtc = DTC $@
> > Please avoid tabs in the output - all other uses spaces. (There is a tab between DTC and $@)
> >
> > > + cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(src)/dts/$*.dts
> >
> > Looks strange. How about:
> > cmd_dtc = $(DTC) -O dtb -o $@ -b 0 $(DTS_FLAGS) $<
> >
> > Then you avoid the hardcoded path in the rule too.
> >
> >
> > > +
> > > +$(obj)/%.dtb: $(src)/dts/%.dts
> > > + $(call if_changed,dtc)
>
> The rule should be generic (not depend on the presence of a dts
> subdirectory. Basically, the .dtb really should be generated in the
> same directory as the .dts file. There is no reason for this rule to
> have special behaviour.
>
> >
> > This snippet belong in the file that uses this.
> > This is how we do for other rules like bzip etc.
>
> This rule is intended to be generic and usable anywhere in the tree.
I understand this.
But only few people will recognize this so they see something like this:

obj-y := foo.dtb.o

And they look for a file named foo.dtb.S or foo.dtb.c.

If we spell it out then we have a better chance to allow
people to understand the relation ships between the files.


If we really want to hide this in scipts/Makefile.* then Makefile.build
is the logical places to do so.
Makefile.lib is supposed to be stuff that is relavent for more than
one Makefile in scripts/* but it has unfortunately also grown
some of the boot support stuff.

Today there is a single rule related to _shipped files.
We should not fill it up with additional rules - put them
Makefile.build if we really want to avoid them in boot/Makefile.

Sam

2010-12-02 16:32:13

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 0/4] V2 Add ability to link device blob(s) into vmlinux

From: Dirk Brandewie <[email protected]>

Resending using the correct To: address sorry for the spam on the CC: lists

This patch set adds the ability to link device tree blobs into
vmlinux.

Patch 1 implements the changes to include/asm-generic/vmlinux.lds.h and
adds a generic rule for generating DTB objects to be linked vmlinux.

Patch 2 implements linking a DTB into an x86 image.

Patch 3-4 move {powerpc,microblaze}/boot/Makefile to use the dtc rule
in patch 1.

This patch set has been tested on x86.

Powerpc and Microblaze have been compile tested with and without patch
3 and 4 applied.

Changes from V1:

Documentation added for dtc command in Makefile.lib to
Documentation/kbuild/makefiles.txt
Separate DTB_ALIGNMENT define removed.
FORCE removed from dtc rule.
Removed hardcoded path to dts files from dtc command.
Moved %.dtb: %.dts rule to arch specific makefiles.

Patch for adding kernel command line option to pass in dtb_compat
string dropped from this set will be submitted seperately.

Dirk Brandewie (4):
of: Add support for linking device tree blobs into vmlinux
x86/of: Add building device tree blob(s) into image.
of/powerpc: Use generic rule to build dtb's
microblaze/of: Use generic rule to build dtb's

Documentation/kbuild/makefiles.txt | 15 +++++++++++++++
arch/microblaze/boot/Makefile | 10 ++--------
arch/powerpc/boot/Makefile | 8 +++-----
arch/x86/platform/ce4100/Makefile | 10 ++++++++++
include/asm-generic/vmlinux.lds.h | 15 ++++++++++++---
scripts/Makefile.lib | 21 ++++++++++++++++++++-
6 files changed, 62 insertions(+), 17 deletions(-)

--
1.7.2.3

2010-12-02 16:32:21

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 3/4] of/powerpc: Use generic rule to build dtb's

From: Dirk Brandewie <[email protected]>

Modify arch/powerpc/boot/Makefile to use dtc command in
scripts/Makefile.lib

Signed-off-by: Dirk Brandewie <[email protected]>
---
arch/powerpc/boot/Makefile | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index fae8192..3afb33a 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -35,7 +35,7 @@ endif

BOOTCFLAGS += -I$(obj) -I$(srctree)/$(obj)

-DTS_FLAGS ?= -p 1024
+DTC_FLAGS ?= -p 1024

$(obj)/4xx.o: BOOTCFLAGS += -mcpu=405
$(obj)/ebony.o: BOOTCFLAGS += -mcpu=405
@@ -332,10 +332,8 @@ $(obj)/treeImage.%: vmlinux $(obj)/%.dtb $(wrapperbits)
$(call if_changed,wrap,treeboot-$*,,$(obj)/$*.dtb)

# Rule to build device tree blobs
-DTC = $(objtree)/scripts/dtc/dtc
-
-$(obj)/%.dtb: $(dtstree)/%.dts
- $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 $(DTS_FLAGS) $(dtstree)/$*.dts
+$(obj)/%.dtb: $(src)/dts/%.dts
+ $(call if_changed,dtc)

# If there isn't a platform selected then just strip the vmlinux.
ifeq (,$(image-y))
--
1.7.2.3

2010-12-02 16:32:25

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 4/4] microblaze/of: Use generic rule to build dtb's

From: Dirk Brandewie <[email protected]>

Modify arch/powerpc/boot/Makefile to use dtc command in
scripts/Makefile.lib

Signed-off-by: Dirk Brandewie <[email protected]>
---
arch/microblaze/boot/Makefile | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
index be01d78..52430e5 100644
--- a/arch/microblaze/boot/Makefile
+++ b/arch/microblaze/boot/Makefile
@@ -10,9 +10,6 @@ targets := linux.bin linux.bin.gz simpleImage.%

OBJCOPYFLAGS := -O binary

-# Where the DTS files live
-dtstree := $(srctree)/$(src)/dts
-
# Ensure system.dtb exists
$(obj)/linked_dtb.o: $(obj)/system.dtb

@@ -51,14 +48,11 @@ $(obj)/simpleImage.%: vmlinux FORCE
$(call if_changed,strip)
@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'

-# Rule to build device tree blobs
-DTC = $(objtree)/scripts/dtc/dtc

# Rule to build device tree blobs
-quiet_cmd_dtc = DTC $@
- cmd_dtc = $(DTC) -O dtb -o $(obj)/$*.dtb -b 0 -p 1024 $(dtstree)/$*.dts
+DTC_FLAGS := -p 1024

-$(obj)/%.dtb: $(dtstree)/%.dts FORCE
+$(obj)/%.dtb: $(src)/dts/%.dts FORCE
$(call if_changed,dtc)

clean-files += *.dtb simpleImage.*.unstrip linux.bin.ub
--
1.7.2.3

2010-12-02 16:32:18

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 1/4] of: Add support for linking device tree blobs into vmlinux

From: Dirk Brandewie <[email protected]>

This patch adds support for linking device tree blob(s) into
vmlinux. Modifies asm-generic/vmlinux.lds.h to add linking
.dtb sections into vmlinux. To maintain compatiblity with the of/fdt
driver code platforms MUST copy the blob to a non-init memory location
before the kernel frees the .init.* sections in the 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.

STRUCT_ALIGNMENT is defined in vmlinux.lds.h for use in the rule to
create wrapper objects for the dtb in Makefile.lib. The
STRUCT_ALIGN() macro in vmlinux.lds.h is modified to use the
STRUCT_ALIGNMENT definition.

The DTB's are placed on 32 byte boundries to allow parsing the blob
with driver/of/fdt.c during early boot without having to copy the blob
to get the structure alignment GCC expects.

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 += foo.dtb.o

Signed-off-by: Dirk Brandewie <[email protected]>
---
Documentation/kbuild/makefiles.txt | 15 +++++++++++++++
include/asm-generic/vmlinux.lds.h | 15 ++++++++++++---
scripts/Makefile.lib | 21 ++++++++++++++++++++-
3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
index 0ef00bd..fc18bb1 100644
--- a/Documentation/kbuild/makefiles.txt
+++ b/Documentation/kbuild/makefiles.txt
@@ -1136,6 +1136,21 @@ When kbuild executes, the following steps are followed (roughly):
resulting in the target file being recompiled for no
obvious reason.

+ dtc
+ Create flattend device tree blob object suitable for linking
+ into vmlinux. Device tree blobs linked into vmlinux are placed
+ in an init section in the image. Platform code *must* copy the
+ blob to non-init memory prior to calling unflatten_device_tree().
+
+ Example:
+ #arch/x86/platform/ce4100/Makefile
+ clean-files := *dtb.S
+
+ DTC_FLAGS := -p 1024
+ obj-y += foo.dtb.o
+
+ $(obj)/%.dtb: $(src)/%.dts
+ $(call if_changed,dtc)

--- 6.7 Custom kbuild commands

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index bd69d79..024d3b9 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -23,7 +23,7 @@
* _etext = .;
*
* _sdata = .;
- * RO_DATA_SECTION(PAGE_SIZE)
+* RO_DATA_SECTION(PAGE_SIZE)
* RW_DATA_SECTION(...)
* _edata = .;
*
@@ -67,7 +67,8 @@
* 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)

/* The actual configuration determine if the init/exit sections
* are handled as text/data or they can be discarded (which
@@ -146,6 +147,13 @@
#define TRACE_SYSCALLS()
#endif

+
+#define KERNEL_DTB() \
+ STRUCT_ALIGN(); \
+ VMLINUX_SYMBOL(__dtb_start) = .; \
+ *(.dtb.init.rodata) \
+ VMLINUX_SYMBOL(__dtb_end) = .;
+
/* .data section */
#define DATA_DATA \
*(.data) \
@@ -468,7 +476,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..937eabbb 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -200,7 +200,26 @@ quiet_cmd_gzip = GZIP $@
cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
(rm -f $@ ; false)

-
+# DTC
+# ---------------------------------------------------------------------------
+
+# Generate an assembly file to wrap the output of the device tree compiler
+$(obj)/%.dtb.S: $(obj)/%.dtb
+ @echo '#include <asm-generic/vmlinux.lds.h>' > $@
+ @echo '.section .dtb.init.rodata,"a"' >> $@
+ @echo '.balign STRUCT_ALIGNMENT' >> $@
+ @echo '.global __dtb_$(*F)_begin' >> $@
+ @echo '__dtb_$(*F)_begin:' >> $@
+ @echo '.incbin "$<" ' >> $@
+ @echo '__dtb_$(*F)_end:' >> $@
+ @echo '.global __dtb_$(*F)_end' >> $@
+ @echo '.balign STRUCT_ALIGNMENT' >> $@
+
+DTC = $(objtree)/scripts/dtc/dtc
+
+quiet_cmd_dtc = DTC $@
+ cmd_dtc = $(DTC) -O dtb -o $@ -b 0 $(DTC_FLAGS) $<
+ooo
# Bzip2
# ---------------------------------------------------------------------------

--
1.7.2.3

2010-12-02 16:32:54

by Dirk Brandewie

[permalink] [raw]
Subject: [PATCH 2/4] x86/of: Add building device tree blob(s) into image.

From: Dirk Brandewie <[email protected]>

This patch adds linking device tree blob into vmlinux. DTB's are
added by adding the blob object name to list of objects to be linked
into the image.

Signed-off-by: Dirk Brandewie <[email protected]>
---
arch/x86/platform/ce4100/Makefile | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/x86/platform/ce4100/Makefile b/arch/x86/platform/ce4100/Makefile
index 91fc929..3b49187 100644
--- a/arch/x86/platform/ce4100/Makefile
+++ b/arch/x86/platform/ce4100/Makefile
@@ -1 +1,11 @@
obj-$(CONFIG_X86_INTEL_CE) += ce4100.o
+clean-files := *dtb.S
+
+ifdef CONFIG_X86_OF
+###
+# device tree blob
+obj-$(CONFIG_X86_INTEL_CE) += ce4100.dtb.o
+
+$(obj)/%.dtb: $(src)/%.dts
+ $(call if_changed,dtc)
+endif
--
1.7.2.3