All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Support for device-tree appended to zImage
@ 2011-02-28 23:33 ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: glikely-s3s/WqlpOiPyB63q8FvJNQ

The following series support for device tree binary blobs (dtb) that are
appended to the compressed kernel image (zImage).

Having appended dtb data is not required to boot with a device tree. If no dtb
data is appended to zImage, the kernel will boot using prior methods.

---

John Bonesio (4):
      ARM:boot:device tree: Allow the device tree binary to be appended to zImage
      ARM:boot:device tree: Merge specific atags into the device tree
      ARM:boot:device tree: Add puthex routine for use in the boot wrapper
      ARM:boot:device tree: Allow multiple device trees to be appended to zImage


 arch/arm/Kconfig                  |    7 ++
 arch/arm/boot/compressed/Makefile |   31 +++++++-
 arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++
 arch/arm/boot/compressed/head.S   |  136 +++++++++++++++++++++++++++++++++++--
 arch/arm/boot/compressed/misc.c   |  134 ++++++++++++++++++++++++++++++++++++
 5 files changed, 365 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm/boot/compressed/atags.c

-- 
Signature

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/4] Support for device-tree appended to zImage
@ 2011-02-28 23:33 ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: linux-arm-kernel

The following series support for device tree binary blobs (dtb) that are
appended to the compressed kernel image (zImage).

Having appended dtb data is not required to boot with a device tree. If no dtb
data is appended to zImage, the kernel will boot using prior methods.

---

John Bonesio (4):
      ARM:boot:device tree: Allow the device tree binary to be appended to zImage
      ARM:boot:device tree: Merge specific atags into the device tree
      ARM:boot:device tree: Add puthex routine for use in the boot wrapper
      ARM:boot:device tree: Allow multiple device trees to be appended to zImage


 arch/arm/Kconfig                  |    7 ++
 arch/arm/boot/compressed/Makefile |   31 +++++++-
 arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++
 arch/arm/boot/compressed/head.S   |  136 +++++++++++++++++++++++++++++++++++--
 arch/arm/boot/compressed/misc.c   |  134 ++++++++++++++++++++++++++++++++++++
 5 files changed, 365 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm/boot/compressed/atags.c

-- 
Signature

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
  2011-02-28 23:33 ` John Bonesio
@ 2011-02-28 23:33   ` John Bonesio
  -1 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: glikely-s3s/WqlpOiPyB63q8FvJNQ

This patch provides the ability to boot using a device tree that is appended
to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 arch/arm/Kconfig                |    7 +++
 arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index d8a330f..68fc640 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1593,6 +1593,13 @@ config USE_OF
 	help
 	  Include support for flattened device tree machine descriptions.
 
+config ARM_APPENDED_DTB
+       bool "Use appended device tree blob" if OF
+       default n
+       help
+         With this option, the boot code will look for a dtb bianry
+         appended to zImage.
+
 # Compressed boot loader in ROM.  Yes, we really want to ask about
 # TEXT and BSS so we preserve their values in the config files.
 config ZBOOT_ROM_TEXT
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 200625c..ae9f8c6 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -210,6 +210,46 @@ restart:	adr	r0, LC0
 		 */
 		mov	r10, r6
 #endif
+#ifdef CONFIG_ARM_APPENDED_DTB
+/*
+ *   r0  = delta
+ *   r2  = BSS start
+ *   r3  = BSS end
+ *   r4  = final kernel address
+ *   r5  = start of this image
+ *   r6  = _edata
+ *   r7  = architecture ID
+ *   r8  = atags/device tree pointer
+ *   r9  = size of decompressed image
+ *   r10 = end of this image, including  bss/stack/malloc space if non XIP
+ *   r11 = GOT start
+ *   r12 = GOT end
+ *
+ * if there are device trees (dtb) appended to zImage, advance r10 so that the
+ * dtb data will get relocated along with the kernel if necessary.
+ */
+
+		ldr	r12, [r6, #0]
+		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
+		cmp	r12, r1
+		bne	dtb_check_done
+
+		/* Get the dtb's size */
+		ldr	r12, [r6, #4]		@ device tree size
+
+		/* convert r12 (dtb size) to little endian */
+		eor	r1, r12, r12, ror #16
+		bic	r1, r1, #0x00ff0000
+		mov	r12, r12, ror #8
+		eor	r12, r12, r1, lsr #8
+
+		add	r10, r10, r12
+		add	r6, r6, r12
+
+dtb_check_done:
+		adr	r1, LC0
+		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
+#endif
 
 /*
  * Check to see if we will overwrite ourselves.
@@ -223,8 +263,8 @@ restart:	adr	r0, LC0
  */
 		cmp	r4, r10
 		bhs	wont_overwrite
-		add	r10, r4, r9
-		cmp	r10, r5
+		add	r1, r4, r9
+		cmp	r1, r5
 		bls	wont_overwrite
 
 /*
@@ -272,8 +312,10 @@ wont_overwrite:
  *   r12 = GOT end
  *   sp  = stack pointer
  */
-		teq	r0, #0
-		beq	not_relocated
+		adr	r1, LC0
+		ldr	r6, [r1, #16]		@ reload _edata value
+
+		add	r6, r6, r0
 		add	r11, r11, r0
 		add	r12, r12, r0
 
@@ -288,12 +330,34 @@ wont_overwrite:
 
 		/*
 		 * Relocate all entries in the GOT table.
+		 * Bump bss entries to past image end (r10)
 		 */
+		sub	r5, r10, r6		@ delta of image end and _edata
+		add	r5, r5, #7		@ ... rounded up to a multiple
+		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
+		             	 		@ ... GOT entry doesn't
+		             	 		@ ... overwrite end of image
+
 1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
 		add	r1, r1, r0		@ table.  This fixes up the
+		cmp	r1, r2
+		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
+		movcs	r9, #1			@      1;
+		cmp	r1, r3
+		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
+		cmp	r9, #0
+		addne	r1, r5			@ entry += r9 ? bss delta : 0;
 		str	r1, [r11], #4		@ C references.
 		cmp	r11, r12
 		blo	1b
+
+		/* bump our bss registers too */
+		add	r2, r2, r5
+		add	r3, r3, r5
+
+		/* bump the stack pinter, if at or above _edata */
+		cmp	sp, r6
+		addcs	sp, sp, r5
 #else
 
 		/*
@@ -309,7 +373,7 @@ wont_overwrite:
 		blo	1b
 #endif
 
-not_relocated:	mov	r0, #0
+		mov	r0, #0
 1:		str	r0, [r2], #4		@ clear bss
 		str	r0, [r2], #4
 		str	r0, [r2], #4
@@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
 		cmp	r2, r3
 		blo	1b
 
+#ifdef CONFIG_ARM_APPENDED_DTB
+/*
+ * The C runtime environment should now be set up sufficiently.
+ *   r4  = kernel execution address
+ *   r6  = _edata
+ *   r7  = architecture ID
+ *   r8  = atags pointer
+ *
+ * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
+ */
+		ldr	r0, [r6, #0]
+		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
+		cmp	r0, r1
+		bne	keep_atags
+
+		mov	r8, r6			@ use the appended device tree
+keep_atags:
+#endif
 /*
- * The C runtime environment should now be setup sufficiently.
  * Set up some pointers, and start decompressing.
  *   r4  = kernel execution address
  *   r7  = architecture ID

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
@ 2011-02-28 23:33   ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch provides the ability to boot using a device tree that is appended
to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).

Signed-off-by: John Bonesio <bones@secretlab.ca>
---

 arch/arm/Kconfig                |    7 +++
 arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index d8a330f..68fc640 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1593,6 +1593,13 @@ config USE_OF
 	help
 	  Include support for flattened device tree machine descriptions.
 
+config ARM_APPENDED_DTB
+       bool "Use appended device tree blob" if OF
+       default n
+       help
+         With this option, the boot code will look for a dtb bianry
+         appended to zImage.
+
 # Compressed boot loader in ROM.  Yes, we really want to ask about
 # TEXT and BSS so we preserve their values in the config files.
 config ZBOOT_ROM_TEXT
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 200625c..ae9f8c6 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -210,6 +210,46 @@ restart:	adr	r0, LC0
 		 */
 		mov	r10, r6
 #endif
+#ifdef CONFIG_ARM_APPENDED_DTB
+/*
+ *   r0  = delta
+ *   r2  = BSS start
+ *   r3  = BSS end
+ *   r4  = final kernel address
+ *   r5  = start of this image
+ *   r6  = _edata
+ *   r7  = architecture ID
+ *   r8  = atags/device tree pointer
+ *   r9  = size of decompressed image
+ *   r10 = end of this image, including  bss/stack/malloc space if non XIP
+ *   r11 = GOT start
+ *   r12 = GOT end
+ *
+ * if there are device trees (dtb) appended to zImage, advance r10 so that the
+ * dtb data will get relocated along with the kernel if necessary.
+ */
+
+		ldr	r12, [r6, #0]
+		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
+		cmp	r12, r1
+		bne	dtb_check_done
+
+		/* Get the dtb's size */
+		ldr	r12, [r6, #4]		@ device tree size
+
+		/* convert r12 (dtb size) to little endian */
+		eor	r1, r12, r12, ror #16
+		bic	r1, r1, #0x00ff0000
+		mov	r12, r12, ror #8
+		eor	r12, r12, r1, lsr #8
+
+		add	r10, r10, r12
+		add	r6, r6, r12
+
+dtb_check_done:
+		adr	r1, LC0
+		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
+#endif
 
 /*
  * Check to see if we will overwrite ourselves.
@@ -223,8 +263,8 @@ restart:	adr	r0, LC0
  */
 		cmp	r4, r10
 		bhs	wont_overwrite
-		add	r10, r4, r9
-		cmp	r10, r5
+		add	r1, r4, r9
+		cmp	r1, r5
 		bls	wont_overwrite
 
 /*
@@ -272,8 +312,10 @@ wont_overwrite:
  *   r12 = GOT end
  *   sp  = stack pointer
  */
-		teq	r0, #0
-		beq	not_relocated
+		adr	r1, LC0
+		ldr	r6, [r1, #16]		@ reload _edata value
+
+		add	r6, r6, r0
 		add	r11, r11, r0
 		add	r12, r12, r0
 
@@ -288,12 +330,34 @@ wont_overwrite:
 
 		/*
 		 * Relocate all entries in the GOT table.
+		 * Bump bss entries to past image end (r10)
 		 */
+		sub	r5, r10, r6		@ delta of image end and _edata
+		add	r5, r5, #7		@ ... rounded up to a multiple
+		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
+		             	 		@ ... GOT entry doesn't
+		             	 		@ ... overwrite end of image
+
 1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
 		add	r1, r1, r0		@ table.  This fixes up the
+		cmp	r1, r2
+		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
+		movcs	r9, #1			@      1;
+		cmp	r1, r3
+		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
+		cmp	r9, #0
+		addne	r1, r5			@ entry += r9 ? bss delta : 0;
 		str	r1, [r11], #4		@ C references.
 		cmp	r11, r12
 		blo	1b
+
+		/* bump our bss registers too */
+		add	r2, r2, r5
+		add	r3, r3, r5
+
+		/* bump the stack pinter, if at or above _edata */
+		cmp	sp, r6
+		addcs	sp, sp, r5
 #else
 
 		/*
@@ -309,7 +373,7 @@ wont_overwrite:
 		blo	1b
 #endif
 
-not_relocated:	mov	r0, #0
+		mov	r0, #0
 1:		str	r0, [r2], #4		@ clear bss
 		str	r0, [r2], #4
 		str	r0, [r2], #4
@@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
 		cmp	r2, r3
 		blo	1b
 
+#ifdef CONFIG_ARM_APPENDED_DTB
+/*
+ * The C runtime environment should now be set up sufficiently.
+ *   r4  = kernel execution address
+ *   r6  = _edata
+ *   r7  = architecture ID
+ *   r8  = atags pointer
+ *
+ * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
+ */
+		ldr	r0, [r6, #0]
+		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
+		cmp	r0, r1
+		bne	keep_atags
+
+		mov	r8, r6			@ use the appended device tree
+keep_atags:
+#endif
 /*
- * The C runtime environment should now be setup sufficiently.
  * Set up some pointers, and start decompressing.
  *   r4  = kernel execution address
  *   r7  = architecture ID

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] ARM:boot:device tree: Merge specific atags into the device tree
  2011-02-28 23:33 ` John Bonesio
@ 2011-02-28 23:33   ` John Bonesio
  -1 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: glikely-s3s/WqlpOiPyB63q8FvJNQ

This patch is to merge in key atags into the appended device tree.  An appended
device tree is where the zImage has a dtb binary appended at the end of it. The
boot code looks for an appended device tree, then looks for a few key atags
passed in by the bootloader.

The bootargs and memory size settings, if they exist, override existing values
in the appended device tree. If these values don't currently exist in the
appended device tree, they are added.

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 arch/arm/boot/compressed/Makefile |   31 ++++++++++++++--
 arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++++++++++++++++++++
 arch/arm/boot/compressed/head.S   |   15 ++++++++
 arch/arm/boot/compressed/misc.c   |   58 ++++++++++++++++++++++++++++++-
 4 files changed, 167 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/boot/compressed/atags.c

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 9d328be..7a2fe22 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -49,6 +49,10 @@ ifeq ($(CONFIG_ARCH_SHMOBILE),y)
 OBJS		+= head-shmobile.o
 endif
 
+ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
+OBJS		+= atags.o libfdt.a 
+endif
+
 #
 # We now have a PIC decompressor implementation.  Decompressors running
 # from RAM should not define ZTEXTADDR.  Decompressors running directly
@@ -80,7 +84,9 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
 KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
 endif
 
-EXTRA_CFLAGS  := -fpic -fno-builtin
+fdttree      := $(srctree)/scripts/dtc/libfdt
+
+EXTRA_CFLAGS  := -fpic -fno-builtin -I$(fdttree) -I$(obj)
 EXTRA_AFLAGS  := -Wa,-march=all
 
 # Provide size of uncompressed kernel to the decompressor via a linker symbol.
@@ -102,13 +108,28 @@ LDFLAGS_vmlinux += -X
 LDFLAGS_vmlinux += -T
 
 # For __aeabi_uidivmod
-lib1funcs = $(obj)/lib1funcs.o
+libfuncs = $(obj)/lib1funcs.o
 
-$(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE
-	$(call cmd,shipped)
+ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
+# For memchr, memmove, etc
+libfuncs += $(obj)/memchr.o $(obj)/strchr.o $(obj)/memmove.o $(obj)/memzero.o
+endif
+
+
+libfdtheader := $(fdttree)/fdt.h $(fdttree)/libfdt.h $(fdttree)/libfdt_internal.h
+libfdtobj    := $(obj)/fdt.o $(obj)/fdt_ro.o $(obj)/fdt_wip.o $(obj)/fdt_sw.o $(obj)/fdt_rw.o $(obj)/fdt_strerror.o
+
+$(libfdtobj): $(obj)/%.o: $(srctree)/scripts/dtc/libfdt/%.c $(libfdtheader)
+	$(call cmd_cc_o_c)
+
+$(obj)/libfdt.a: $(libfdtobj)
+	$(AR) rcs $@ $^
+
+$(libfuncs): $(obj)/%.o: $(srctree)/arch/$(SRCARCH)/lib/%.S
+	$(call cmd_as_o_S)
 
 $(obj)/vmlinux: $(obj)/vmlinux.lds $(obj)/$(HEAD) $(obj)/piggy.$(suffix_y).o \
-	 	$(addprefix $(obj)/, $(OBJS)) $(lib1funcs) FORCE
+	 	$(addprefix $(obj)/, $(OBJS)) $(libfuncs) FORCE
 	$(call if_changed,ld)
 	@:
 
diff --git a/arch/arm/boot/compressed/atags.c b/arch/arm/boot/compressed/atags.c
new file mode 100644
index 0000000..d19d53d
--- /dev/null
+++ b/arch/arm/boot/compressed/atags.c
@@ -0,0 +1,70 @@
+#include <stddef.h>
+#include <asm/byteorder.h>
+#include <asm/setup.h>
+#include <fdt.h>
+#include <libfdt.h>
+
+int dt_setprop(void *fdt, const char *node_path, const char *property,
+               uint32_t *val_array, int size)
+{
+	int offset;
+
+	offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+
+	return fdt_setprop(fdt, offset, property, val_array, size);
+}
+
+int dt_setprop_string(void *fdt, const char *node_path,
+                      const char *property, const char *string)
+{
+	int offset;
+		
+	offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+			
+	return fdt_setprop_string(fdt, offset, property, string);
+}
+
+int dt_setprop_cell(void *fdt, const char *node_path,
+                    const char *property, uint32_t val)
+{
+	int offset;
+
+	offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+
+	return fdt_setprop_cell(fdt, offset, property, val);
+}
+
+void dt_merge_atags(void *dt,  void *ataglist)
+{
+	struct tag *atag = ataglist;
+	uint32_t mem_reg_property[2];
+	uint32_t initrd_start, initrd_size;
+	int rc;
+
+	while (atag && atag->hdr.tag != 0) {
+		if (atag->hdr.tag == ATAG_CMDLINE) {
+			dt_setprop_string(dt, "/chosen", "bootargs",
+			                  atag->u.cmdline.cmdline);
+		} else if (atag->hdr.tag == ATAG_MEM) {
+			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
+			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
+			dt_setprop(dt, "/memory", "reg", mem_reg_property,
+			           sizeof(mem_reg_property));
+		} else if (atag->hdr.tag == ATAG_INITRD2) {
+			initrd_start = atag->u.initrd.start;
+			initrd_size = atag->u.initrd.size;
+			dt_setprop_cell(dt, "/chosen", "linux,initrd-start",
+			                initrd_start);
+			dt_setprop_cell(dt, "/chosen", "linux,initrd-end",
+			                initrd_start + initrd_size);
+		}
+		atag = tag_next(atag);
+	}
+}
+
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index ae9f8c6..c2cdc5f 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -211,6 +211,7 @@ restart:	adr	r0, LC0
 		mov	r10, r6
 #endif
 #ifdef CONFIG_ARM_APPENDED_DTB
+#define MERGE_SPACE	1024
 /*
  *   r0  = delta
  *   r2  = BSS start
@@ -243,6 +244,8 @@ restart:	adr	r0, LC0
 		mov	r12, r12, ror #8
 		eor	r12, r12, r1, lsr #8
 
+		add	r12, r12, #MERGE_SPACE	@ extra space to merge in atags
+
 		add	r10, r10, r12
 		add	r6, r6, r12
 
@@ -338,6 +341,8 @@ wont_overwrite:
 		             	 		@ ... GOT entry doesn't
 		             	 		@ ... overwrite end of image
 
+		add	r5, r5, #MERGE_SPACE	@ extra space to merge in atags
+
 1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
 		add	r1, r1, r0		@ table.  This fixes up the
 		cmp	r1, r2
@@ -396,6 +401,16 @@ wont_overwrite:
 		cmp	r0, r1
 		bne	keep_atags
 
+#ifndef CONFIG_ZBOOT_ROM
+		ldr	r0, [r6, #4]
+		add	r0, r0, #MERGE_SPACE	
+		str	r0, [r6, #4]		@ increase device tree size
+
+		/* merge in some key atags into the dtb */
+		mov	r0, r6
+		mov	r1, r8
+		bl	dt_merge_atags
+#endif
 		mov	r8, r6			@ use the appended device tree
 keep_atags:
 #endif
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index e653a6d..2d4da4c 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -29,7 +29,7 @@ unsigned int __machine_arch_type;
 #include <asm/unaligned.h>
 
 
-static void putstr(const char *ptr);
+void putstr(const char *ptr);
 extern void error(char *x);
 
 #include <mach/uncompress.h>
@@ -100,7 +100,7 @@ static void icedcc_putc(int ch)
 #define putc(ch)	icedcc_putc(ch)
 #endif
 
-static void putstr(const char *ptr)
+void putstr(const char *ptr)
 {
 	char c;
 
@@ -114,6 +114,60 @@ static void putstr(const char *ptr)
 }
 
 
+#ifdef CONFIG_ARM_APPENDED_DTB
+/**
+ * strlen - Find the length of a string
+ * @s: The string to be sized
+ */
+size_t strlen(const char *s)
+{
+        const char *sc;
+
+        for (sc = s; *sc != '\0'; ++sc)
+                /* nothing */;
+        return sc - s;
+}
+
+/**
+ * strcmp - Compare two strings
+ * @cs: One string
+ * @ct: Another string
+ */
+#undef strcmp
+int strcmp(const char *cs, const char *ct)
+{
+	unsigned char c1, c2;
+
+	while (1) {
+		c1 = *cs++;
+		c2 = *ct++;
+		if (c1 != c2)
+			return c1 < c2 ? -1 : 1;
+		if (!c1)
+			break;
+	}
+	return 0;
+}
+
+/**
+ * memcmp - Compare two areas of memory
+ * @cs: One area of memory
+ * @ct: Another area of memory
+ * @count: The size of the area.
+ */
+#undef memcmp
+int memcmp(const void *cs, const void *ct, size_t count)
+{
+	const unsigned char *su1, *su2;
+	int res = 0;
+
+	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
+		if ((res = *su1 - *su2) != 0)
+			break;
+	return res;
+}
+#endif
+
 void *memcpy(void *__dest, __const void *__src, size_t __n)
 {
 	int i = 0;

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] ARM:boot:device tree: Merge specific atags into the device tree
@ 2011-02-28 23:33   ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch is to merge in key atags into the appended device tree.  An appended
device tree is where the zImage has a dtb binary appended at the end of it. The
boot code looks for an appended device tree, then looks for a few key atags
passed in by the bootloader.

The bootargs and memory size settings, if they exist, override existing values
in the appended device tree. If these values don't currently exist in the
appended device tree, they are added.

Signed-off-by: John Bonesio <bones@secretlab.ca>
---

 arch/arm/boot/compressed/Makefile |   31 ++++++++++++++--
 arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++++++++++++++++++++
 arch/arm/boot/compressed/head.S   |   15 ++++++++
 arch/arm/boot/compressed/misc.c   |   58 ++++++++++++++++++++++++++++++-
 4 files changed, 167 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm/boot/compressed/atags.c

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 9d328be..7a2fe22 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -49,6 +49,10 @@ ifeq ($(CONFIG_ARCH_SHMOBILE),y)
 OBJS		+= head-shmobile.o
 endif
 
+ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
+OBJS		+= atags.o libfdt.a 
+endif
+
 #
 # We now have a PIC decompressor implementation.  Decompressors running
 # from RAM should not define ZTEXTADDR.  Decompressors running directly
@@ -80,7 +84,9 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
 KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
 endif
 
-EXTRA_CFLAGS  := -fpic -fno-builtin
+fdttree      := $(srctree)/scripts/dtc/libfdt
+
+EXTRA_CFLAGS  := -fpic -fno-builtin -I$(fdttree) -I$(obj)
 EXTRA_AFLAGS  := -Wa,-march=all
 
 # Provide size of uncompressed kernel to the decompressor via a linker symbol.
@@ -102,13 +108,28 @@ LDFLAGS_vmlinux += -X
 LDFLAGS_vmlinux += -T
 
 # For __aeabi_uidivmod
-lib1funcs = $(obj)/lib1funcs.o
+libfuncs = $(obj)/lib1funcs.o
 
-$(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE
-	$(call cmd,shipped)
+ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
+# For memchr, memmove, etc
+libfuncs += $(obj)/memchr.o $(obj)/strchr.o $(obj)/memmove.o $(obj)/memzero.o
+endif
+
+
+libfdtheader := $(fdttree)/fdt.h $(fdttree)/libfdt.h $(fdttree)/libfdt_internal.h
+libfdtobj    := $(obj)/fdt.o $(obj)/fdt_ro.o $(obj)/fdt_wip.o $(obj)/fdt_sw.o $(obj)/fdt_rw.o $(obj)/fdt_strerror.o
+
+$(libfdtobj): $(obj)/%.o: $(srctree)/scripts/dtc/libfdt/%.c $(libfdtheader)
+	$(call cmd_cc_o_c)
+
+$(obj)/libfdt.a: $(libfdtobj)
+	$(AR) rcs $@ $^
+
+$(libfuncs): $(obj)/%.o: $(srctree)/arch/$(SRCARCH)/lib/%.S
+	$(call cmd_as_o_S)
 
 $(obj)/vmlinux: $(obj)/vmlinux.lds $(obj)/$(HEAD) $(obj)/piggy.$(suffix_y).o \
-	 	$(addprefix $(obj)/, $(OBJS)) $(lib1funcs) FORCE
+	 	$(addprefix $(obj)/, $(OBJS)) $(libfuncs) FORCE
 	$(call if_changed,ld)
 	@:
 
diff --git a/arch/arm/boot/compressed/atags.c b/arch/arm/boot/compressed/atags.c
new file mode 100644
index 0000000..d19d53d
--- /dev/null
+++ b/arch/arm/boot/compressed/atags.c
@@ -0,0 +1,70 @@
+#include <stddef.h>
+#include <asm/byteorder.h>
+#include <asm/setup.h>
+#include <fdt.h>
+#include <libfdt.h>
+
+int dt_setprop(void *fdt, const char *node_path, const char *property,
+               uint32_t *val_array, int size)
+{
+	int offset;
+
+	offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+
+	return fdt_setprop(fdt, offset, property, val_array, size);
+}
+
+int dt_setprop_string(void *fdt, const char *node_path,
+                      const char *property, const char *string)
+{
+	int offset;
+		
+	offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+			
+	return fdt_setprop_string(fdt, offset, property, string);
+}
+
+int dt_setprop_cell(void *fdt, const char *node_path,
+                    const char *property, uint32_t val)
+{
+	int offset;
+
+	offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+
+	return fdt_setprop_cell(fdt, offset, property, val);
+}
+
+void dt_merge_atags(void *dt,  void *ataglist)
+{
+	struct tag *atag = ataglist;
+	uint32_t mem_reg_property[2];
+	uint32_t initrd_start, initrd_size;
+	int rc;
+
+	while (atag && atag->hdr.tag != 0) {
+		if (atag->hdr.tag == ATAG_CMDLINE) {
+			dt_setprop_string(dt, "/chosen", "bootargs",
+			                  atag->u.cmdline.cmdline);
+		} else if (atag->hdr.tag == ATAG_MEM) {
+			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
+			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
+			dt_setprop(dt, "/memory", "reg", mem_reg_property,
+			           sizeof(mem_reg_property));
+		} else if (atag->hdr.tag == ATAG_INITRD2) {
+			initrd_start = atag->u.initrd.start;
+			initrd_size = atag->u.initrd.size;
+			dt_setprop_cell(dt, "/chosen", "linux,initrd-start",
+			                initrd_start);
+			dt_setprop_cell(dt, "/chosen", "linux,initrd-end",
+			                initrd_start + initrd_size);
+		}
+		atag = tag_next(atag);
+	}
+}
+
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index ae9f8c6..c2cdc5f 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -211,6 +211,7 @@ restart:	adr	r0, LC0
 		mov	r10, r6
 #endif
 #ifdef CONFIG_ARM_APPENDED_DTB
+#define MERGE_SPACE	1024
 /*
  *   r0  = delta
  *   r2  = BSS start
@@ -243,6 +244,8 @@ restart:	adr	r0, LC0
 		mov	r12, r12, ror #8
 		eor	r12, r12, r1, lsr #8
 
+		add	r12, r12, #MERGE_SPACE	@ extra space to merge in atags
+
 		add	r10, r10, r12
 		add	r6, r6, r12
 
@@ -338,6 +341,8 @@ wont_overwrite:
 		             	 		@ ... GOT entry doesn't
 		             	 		@ ... overwrite end of image
 
+		add	r5, r5, #MERGE_SPACE	@ extra space to merge in atags
+
 1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
 		add	r1, r1, r0		@ table.  This fixes up the
 		cmp	r1, r2
@@ -396,6 +401,16 @@ wont_overwrite:
 		cmp	r0, r1
 		bne	keep_atags
 
+#ifndef CONFIG_ZBOOT_ROM
+		ldr	r0, [r6, #4]
+		add	r0, r0, #MERGE_SPACE	
+		str	r0, [r6, #4]		@ increase device tree size
+
+		/* merge in some key atags into the dtb */
+		mov	r0, r6
+		mov	r1, r8
+		bl	dt_merge_atags
+#endif
 		mov	r8, r6			@ use the appended device tree
 keep_atags:
 #endif
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index e653a6d..2d4da4c 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -29,7 +29,7 @@ unsigned int __machine_arch_type;
 #include <asm/unaligned.h>
 
 
-static void putstr(const char *ptr);
+void putstr(const char *ptr);
 extern void error(char *x);
 
 #include <mach/uncompress.h>
@@ -100,7 +100,7 @@ static void icedcc_putc(int ch)
 #define putc(ch)	icedcc_putc(ch)
 #endif
 
-static void putstr(const char *ptr)
+void putstr(const char *ptr)
 {
 	char c;
 
@@ -114,6 +114,60 @@ static void putstr(const char *ptr)
 }
 
 
+#ifdef CONFIG_ARM_APPENDED_DTB
+/**
+ * strlen - Find the length of a string
+ * @s: The string to be sized
+ */
+size_t strlen(const char *s)
+{
+        const char *sc;
+
+        for (sc = s; *sc != '\0'; ++sc)
+                /* nothing */;
+        return sc - s;
+}
+
+/**
+ * strcmp - Compare two strings
+ * @cs: One string
+ * @ct: Another string
+ */
+#undef strcmp
+int strcmp(const char *cs, const char *ct)
+{
+	unsigned char c1, c2;
+
+	while (1) {
+		c1 = *cs++;
+		c2 = *ct++;
+		if (c1 != c2)
+			return c1 < c2 ? -1 : 1;
+		if (!c1)
+			break;
+	}
+	return 0;
+}
+
+/**
+ * memcmp - Compare two areas of memory
+ * @cs: One area of memory
+ * @ct: Another area of memory
+ * @count: The size of the area.
+ */
+#undef memcmp
+int memcmp(const void *cs, const void *ct, size_t count)
+{
+	const unsigned char *su1, *su2;
+	int res = 0;
+
+	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
+		if ((res = *su1 - *su2) != 0)
+			break;
+	return res;
+}
+#endif
+
 void *memcpy(void *__dest, __const void *__src, size_t __n)
 {
 	int i = 0;

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] ARM:boot:device tree: Add puthex routine for use in the boot wrapper
  2011-02-28 23:33 ` John Bonesio
@ 2011-02-28 23:33   ` John Bonesio
  -1 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: glikely-s3s/WqlpOiPyB63q8FvJNQ

This patch adds a routine named 'puthex'. This allows the boot wraper code to
display hex numbers.

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 arch/arm/boot/compressed/misc.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index 2d4da4c..9af05ed 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -113,6 +113,31 @@ void putstr(const char *ptr)
 	flush();
 }
 
+void puthex(unsigned i)
+{
+	unsigned rem;
+	char str[32];
+	int str_idx = sizeof(str) - 1;;
+
+	str[str_idx--] = '\0';
+	putstr("0x");
+	if (!i) {
+		putstr("0");
+	} else {
+		while (i) {
+			rem = i % 16;
+
+			if (rem < 10) {
+				str[str_idx--] = rem + '0';
+			} else {
+				str[str_idx--] = (rem - 10) + 'a';
+			}
+			i = i / 16;
+		}
+
+		putstr(&str[str_idx + 1]);
+	}
+}
 
 #ifdef CONFIG_ARM_APPENDED_DTB
 /**

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] ARM:boot:device tree: Add puthex routine for use in the boot wrapper
@ 2011-02-28 23:33   ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds a routine named 'puthex'. This allows the boot wraper code to
display hex numbers.

Signed-off-by: John Bonesio <bones@secretlab.ca>
---

 arch/arm/boot/compressed/misc.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index 2d4da4c..9af05ed 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -113,6 +113,31 @@ void putstr(const char *ptr)
 	flush();
 }
 
+void puthex(unsigned i)
+{
+	unsigned rem;
+	char str[32];
+	int str_idx = sizeof(str) - 1;;
+
+	str[str_idx--] = '\0';
+	putstr("0x");
+	if (!i) {
+		putstr("0");
+	} else {
+		while (i) {
+			rem = i % 16;
+
+			if (rem < 10) {
+				str[str_idx--] = rem + '0';
+			} else {
+				str[str_idx--] = (rem - 10) + 'a';
+			}
+			i = i / 16;
+		}
+
+		putstr(&str[str_idx + 1]);
+	}
+}
 
 #ifdef CONFIG_ARM_APPENDED_DTB
 /**

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] ARM:boot:device tree: Allow multiple device trees to be appended to zImage
  2011-02-28 23:33 ` John Bonesio
@ 2011-02-28 23:33   ` John Bonesio
  -1 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: glikely-s3s/WqlpOiPyB63q8FvJNQ

This patch allows mutliple device tree binaries to be appended to zImage. A
device tree binary is selected when the 'machine_type' property in the root
node in the dtb matches the machine type passed in by the boot loader.

If the device tree binary does not have the 'machine_type' property, the device
tree is assumed to match the machine type. If no device tree binary matches,
the kernel attempts to boot with out a device tree.

Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---

 arch/arm/boot/compressed/head.S |   30 ++++++++++++++++++++++-
 arch/arm/boot/compressed/misc.c |   51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index c2cdc5f..f743431 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -229,7 +229,7 @@ restart:	adr	r0, LC0
  * if there are device trees (dtb) appended to zImage, advance r10 so that the
  * dtb data will get relocated along with the kernel if necessary.
  */
-
+1:
 		ldr	r12, [r6, #0]
 		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
 		cmp	r12, r1
@@ -248,6 +248,7 @@ restart:	adr	r0, LC0
 
 		add	r10, r10, r12
 		add	r6, r6, r12
+		b	1b
 
 dtb_check_done:
 		adr	r1, LC0
@@ -396,11 +397,34 @@ wont_overwrite:
  *
  * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
  */
+		mvn	r9, #0			@ r9 dtb search result
+		   	      			@ ... -1 = no appended dtb
+dt_search:
 		ldr	r0, [r6, #0]
 		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
 		cmp	r0, r1
 		bne	keep_atags
 
+		/* Get the dtb's size */
+		ldr	r5, [r6, #4]		@ device tree size
+
+		/* convert dtb size to little endian */
+		eor	r1, r5, r5, ror #16
+		bic	r1, r1, #0x00ff0000
+		mov	r5, r5, ror #8
+		eor	r5, r5, r1, lsr #8
+
+		mov	r0, r6
+		mov	r1, r7
+		bl	match_dt_machine_type
+		mov	r9, r0			@ save result
+		cmp	r0, #0
+		bne	dt_found
+
+		add	r6, r5
+		b	dt_search
+
+dt_found:
 #ifndef CONFIG_ZBOOT_ROM
 		ldr	r0, [r6, #4]
 		add	r0, r0, #MERGE_SPACE	
@@ -413,6 +437,10 @@ wont_overwrite:
 #endif
 		mov	r8, r6			@ use the appended device tree
 keep_atags:
+
+		mov	r0, r9
+		mov	r1, r7
+		bl	put_dt_match_str
 #endif
 /*
  * Set up some pointers, and start decompressing.
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index 9af05ed..f16ca6a 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -25,6 +25,8 @@ unsigned int __machine_arch_type;
 #include <linux/stddef.h>	/* for NULL */
 #include <linux/linkage.h>
 #include <asm/string.h>
+#include <fdt.h>
+#include <libfdt.h>
 
 #include <asm/unaligned.h>
 
@@ -227,6 +229,55 @@ void *memcpy(void *__dest, __const void *__src, size_t __n)
 	return __dest;
 }
 
+int match_dt_machine_type(void *fdt, unsigned int mach_type)
+{
+	int offset;
+	unsigned dt_mach_type;
+	unsigned *prop;
+	int match;
+
+	offset = fdt_path_offset(fdt, "/");
+	if (offset < 0)
+		return offset;
+
+	prop = fdt_getprop(fdt, offset, "machine-type", NULL);
+
+	if (! prop)
+		return 2;
+
+	dt_mach_type = fdt32_to_cpu(*prop);
+
+	return mach_type == dt_mach_type;
+}
+
+void put_dt_match_str(int match, unsigned int mach_type)
+{
+	switch (match) {
+	case 2:
+		putstr("'machine-type' property not found.");
+		putstr(" Device tree matched by default\n");
+		break;
+	case 1:
+		putstr("Device tree matched (machine type: ");
+		puthex(mach_type);
+		putstr(")\n");
+		break;
+	case 0:
+		putstr("Device tree not matched (machine type: ");
+		puthex(mach_type);
+		putstr(")\n");
+		break;
+	case -1:
+		/*
+		 * -1 means 'no device tree appended' so don't display any
+		 * message
+		 */
+		break;
+	default:
+		putstr("Unknown device tree match type.\n");
+	}
+}
+
 /*
  * gzip delarations
  */

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] ARM:boot:device tree: Allow multiple device trees to be appended to zImage
@ 2011-02-28 23:33   ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-02-28 23:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch allows mutliple device tree binaries to be appended to zImage. A
device tree binary is selected when the 'machine_type' property in the root
node in the dtb matches the machine type passed in by the boot loader.

If the device tree binary does not have the 'machine_type' property, the device
tree is assumed to match the machine type. If no device tree binary matches,
the kernel attempts to boot with out a device tree.

Signed-off-by: John Bonesio <bones@secretlab.ca>
---

 arch/arm/boot/compressed/head.S |   30 ++++++++++++++++++++++-
 arch/arm/boot/compressed/misc.c |   51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index c2cdc5f..f743431 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -229,7 +229,7 @@ restart:	adr	r0, LC0
  * if there are device trees (dtb) appended to zImage, advance r10 so that the
  * dtb data will get relocated along with the kernel if necessary.
  */
-
+1:
 		ldr	r12, [r6, #0]
 		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
 		cmp	r12, r1
@@ -248,6 +248,7 @@ restart:	adr	r0, LC0
 
 		add	r10, r10, r12
 		add	r6, r6, r12
+		b	1b
 
 dtb_check_done:
 		adr	r1, LC0
@@ -396,11 +397,34 @@ wont_overwrite:
  *
  * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
  */
+		mvn	r9, #0			@ r9 dtb search result
+		   	      			@ ... -1 = no appended dtb
+dt_search:
 		ldr	r0, [r6, #0]
 		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
 		cmp	r0, r1
 		bne	keep_atags
 
+		/* Get the dtb's size */
+		ldr	r5, [r6, #4]		@ device tree size
+
+		/* convert dtb size to little endian */
+		eor	r1, r5, r5, ror #16
+		bic	r1, r1, #0x00ff0000
+		mov	r5, r5, ror #8
+		eor	r5, r5, r1, lsr #8
+
+		mov	r0, r6
+		mov	r1, r7
+		bl	match_dt_machine_type
+		mov	r9, r0			@ save result
+		cmp	r0, #0
+		bne	dt_found
+
+		add	r6, r5
+		b	dt_search
+
+dt_found:
 #ifndef CONFIG_ZBOOT_ROM
 		ldr	r0, [r6, #4]
 		add	r0, r0, #MERGE_SPACE	
@@ -413,6 +437,10 @@ wont_overwrite:
 #endif
 		mov	r8, r6			@ use the appended device tree
 keep_atags:
+
+		mov	r0, r9
+		mov	r1, r7
+		bl	put_dt_match_str
 #endif
 /*
  * Set up some pointers, and start decompressing.
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index 9af05ed..f16ca6a 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -25,6 +25,8 @@ unsigned int __machine_arch_type;
 #include <linux/stddef.h>	/* for NULL */
 #include <linux/linkage.h>
 #include <asm/string.h>
+#include <fdt.h>
+#include <libfdt.h>
 
 #include <asm/unaligned.h>
 
@@ -227,6 +229,55 @@ void *memcpy(void *__dest, __const void *__src, size_t __n)
 	return __dest;
 }
 
+int match_dt_machine_type(void *fdt, unsigned int mach_type)
+{
+	int offset;
+	unsigned dt_mach_type;
+	unsigned *prop;
+	int match;
+
+	offset = fdt_path_offset(fdt, "/");
+	if (offset < 0)
+		return offset;
+
+	prop = fdt_getprop(fdt, offset, "machine-type", NULL);
+
+	if (! prop)
+		return 2;
+
+	dt_mach_type = fdt32_to_cpu(*prop);
+
+	return mach_type == dt_mach_type;
+}
+
+void put_dt_match_str(int match, unsigned int mach_type)
+{
+	switch (match) {
+	case 2:
+		putstr("'machine-type' property not found.");
+		putstr(" Device tree matched by default\n");
+		break;
+	case 1:
+		putstr("Device tree matched (machine type: ");
+		puthex(mach_type);
+		putstr(")\n");
+		break;
+	case 0:
+		putstr("Device tree not matched (machine type: ");
+		puthex(mach_type);
+		putstr(")\n");
+		break;
+	case -1:
+		/*
+		 * -1 means 'no device tree appended' so don't display any
+		 * message
+		 */
+		break;
+	default:
+		putstr("Unknown device tree match type.\n");
+	}
+}
+
 /*
  * gzip delarations
  */

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/4] Support for device-tree appended to zImage
  2011-02-28 23:33 ` John Bonesio
@ 2011-03-01  2:58   ` Shawn Guo
  -1 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2011-03-01  2:58 UTC (permalink / raw)
  To: John Bonesio
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi John,

On Mon, Feb 28, 2011 at 03:33:18PM -0800, John Bonesio wrote:
> The following series support for device tree binary blobs (dtb) that are
> appended to the compressed kernel image (zImage).
> 
> Having appended dtb data is not required to boot with a device tree. If no dtb
> data is appended to zImage, the kernel will boot using prior methods.
> 
> ---
> 
> John Bonesio (4):
>       ARM:boot:device tree: Allow the device tree binary to be appended to zImage
>       ARM:boot:device tree: Merge specific atags into the device tree
>       ARM:boot:device tree: Add puthex routine for use in the boot wrapper
>       ARM:boot:device tree: Allow multiple device trees to be appended to zImage
> 
> 
>  arch/arm/Kconfig                  |    7 ++
>  arch/arm/boot/compressed/Makefile |   31 +++++++-
>  arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++
>  arch/arm/boot/compressed/head.S   |  136 +++++++++++++++++++++++++++++++++++--
>  arch/arm/boot/compressed/misc.c   |  134 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 365 insertions(+), 13 deletions(-)
>  create mode 100644 arch/arm/boot/compressed/atags.c
> 
I'm trying to test and use it.  But it does not apply against any tree
that I can find, neither mainline nor Grant's tree.  Need rebasing?

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/4] Support for device-tree appended to zImage
@ 2011-03-01  2:58   ` Shawn Guo
  0 siblings, 0 replies; 30+ messages in thread
From: Shawn Guo @ 2011-03-01  2:58 UTC (permalink / raw)
  To: linux-arm-kernel

Hi John,

On Mon, Feb 28, 2011 at 03:33:18PM -0800, John Bonesio wrote:
> The following series support for device tree binary blobs (dtb) that are
> appended to the compressed kernel image (zImage).
> 
> Having appended dtb data is not required to boot with a device tree. If no dtb
> data is appended to zImage, the kernel will boot using prior methods.
> 
> ---
> 
> John Bonesio (4):
>       ARM:boot:device tree: Allow the device tree binary to be appended to zImage
>       ARM:boot:device tree: Merge specific atags into the device tree
>       ARM:boot:device tree: Add puthex routine for use in the boot wrapper
>       ARM:boot:device tree: Allow multiple device trees to be appended to zImage
> 
> 
>  arch/arm/Kconfig                  |    7 ++
>  arch/arm/boot/compressed/Makefile |   31 +++++++-
>  arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++
>  arch/arm/boot/compressed/head.S   |  136 +++++++++++++++++++++++++++++++++++--
>  arch/arm/boot/compressed/misc.c   |  134 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 365 insertions(+), 13 deletions(-)
>  create mode 100644 arch/arm/boot/compressed/atags.c
> 
I'm trying to test and use it.  But it does not apply against any tree
that I can find, neither mainline nor Grant's tree.  Need rebasing?

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/4] Support for device-tree appended to zImage
  2011-03-01  2:58   ` Shawn Guo
@ 2011-03-01  3:25       ` John Bonesio
  -1 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-03-01  3:25 UTC (permalink / raw)
  To: Shawn Guo
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Shawn,

I'm using Grant's tree with Nicolas Pitre's patch to the compressed
image wrapper. His patches went out to the ARM list and I'm told they
were accepted. These patches may not yet be in Grant's tree. These
patches are titled:

[PATCH 1/2] ARM: improvements to compressed/head.S
[PATCH 2/2] ARM: remove the 4x expansion presumption while decompressing
the kernel

Grant, is there a branch in your tree that people should be using, or
should be they applying both sets of patches?

- John

On 02/28/2011 06:58 PM, Shawn Guo wrote:
> Hi John,
> 
> On Mon, Feb 28, 2011 at 03:33:18PM -0800, John Bonesio wrote:
>> The following series support for device tree binary blobs (dtb) that are
>> appended to the compressed kernel image (zImage).
>>
>> Having appended dtb data is not required to boot with a device tree. If no dtb
>> data is appended to zImage, the kernel will boot using prior methods.
>>
>> ---
>>
>> John Bonesio (4):
>>       ARM:boot:device tree: Allow the device tree binary to be appended to zImage
>>       ARM:boot:device tree: Merge specific atags into the device tree
>>       ARM:boot:device tree: Add puthex routine for use in the boot wrapper
>>       ARM:boot:device tree: Allow multiple device trees to be appended to zImage
>>
>>
>>  arch/arm/Kconfig                  |    7 ++
>>  arch/arm/boot/compressed/Makefile |   31 +++++++-
>>  arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++
>>  arch/arm/boot/compressed/head.S   |  136 +++++++++++++++++++++++++++++++++++--
>>  arch/arm/boot/compressed/misc.c   |  134 ++++++++++++++++++++++++++++++++++++
>>  5 files changed, 365 insertions(+), 13 deletions(-)
>>  create mode 100644 arch/arm/boot/compressed/atags.c
>>
> I'm trying to test and use it.  But it does not apply against any tree
> that I can find, neither mainline nor Grant's tree.  Need rebasing?
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/4] Support for device-tree appended to zImage
@ 2011-03-01  3:25       ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-03-01  3:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Shawn,

I'm using Grant's tree with Nicolas Pitre's patch to the compressed
image wrapper. His patches went out to the ARM list and I'm told they
were accepted. These patches may not yet be in Grant's tree. These
patches are titled:

[PATCH 1/2] ARM: improvements to compressed/head.S
[PATCH 2/2] ARM: remove the 4x expansion presumption while decompressing
the kernel

Grant, is there a branch in your tree that people should be using, or
should be they applying both sets of patches?

- John

On 02/28/2011 06:58 PM, Shawn Guo wrote:
> Hi John,
> 
> On Mon, Feb 28, 2011 at 03:33:18PM -0800, John Bonesio wrote:
>> The following series support for device tree binary blobs (dtb) that are
>> appended to the compressed kernel image (zImage).
>>
>> Having appended dtb data is not required to boot with a device tree. If no dtb
>> data is appended to zImage, the kernel will boot using prior methods.
>>
>> ---
>>
>> John Bonesio (4):
>>       ARM:boot:device tree: Allow the device tree binary to be appended to zImage
>>       ARM:boot:device tree: Merge specific atags into the device tree
>>       ARM:boot:device tree: Add puthex routine for use in the boot wrapper
>>       ARM:boot:device tree: Allow multiple device trees to be appended to zImage
>>
>>
>>  arch/arm/Kconfig                  |    7 ++
>>  arch/arm/boot/compressed/Makefile |   31 +++++++-
>>  arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++
>>  arch/arm/boot/compressed/head.S   |  136 +++++++++++++++++++++++++++++++++++--
>>  arch/arm/boot/compressed/misc.c   |  134 ++++++++++++++++++++++++++++++++++++
>>  5 files changed, 365 insertions(+), 13 deletions(-)
>>  create mode 100644 arch/arm/boot/compressed/atags.c
>>
> I'm trying to test and use it.  But it does not apply against any tree
> that I can find, neither mainline nor Grant's tree.  Need rebasing?
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
  2011-02-28 23:33   ` John Bonesio
@ 2011-03-01  6:39     ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-01  6:39 UTC (permalink / raw)
  To: John Bonesio; +Cc: devicetree-discuss, glikely, linux-arm-kernel

On Mon, 28 Feb 2011, John Bonesio wrote:

> This patch provides the ability to boot using a device tree that is appended
> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> 
> Signed-off-by: John Bonesio <bones@secretlab.ca>

Comments below.

> ---
> 
>  arch/arm/Kconfig                |    7 +++
>  arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
>  2 files changed, 94 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d8a330f..68fc640 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1593,6 +1593,13 @@ config USE_OF
>  	help
>  	  Include support for flattened device tree machine descriptions.
>  
> +config ARM_APPENDED_DTB
> +       bool "Use appended device tree blob" if OF
> +       default n

The default is n by default, so you don't need to mention it.

Also this should depend on OF (CONFIG_OF).

> +       help
> +         With this option, the boot code will look for a dtb bianry

s/bianry/binary/

Since this is an help text for people who might not have a clue about 
"dtb", it would be better to spell it out.

> +         appended to zImage.
> +
>  # Compressed boot loader in ROM.  Yes, we really want to ask about
>  # TEXT and BSS so we preserve their values in the config files.
>  config ZBOOT_ROM_TEXT
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 200625c..ae9f8c6 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -210,6 +210,46 @@ restart:	adr	r0, LC0
>  		 */
>  		mov	r10, r6
>  #endif
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/*
> + *   r0  = delta
> + *   r2  = BSS start
> + *   r3  = BSS end
> + *   r4  = final kernel address
> + *   r5  = start of this image
> + *   r6  = _edata
> + *   r7  = architecture ID
> + *   r8  = atags/device tree pointer
> + *   r9  = size of decompressed image
> + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
> + *   r11 = GOT start
> + *   r12 = GOT end
> + *
> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> + * dtb data will get relocated along with the kernel if necessary.
> + */
> +
> +		ldr	r12, [r6, #0]
> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
> +		cmp	r12, r1
> +		bne	dtb_check_done
> +
> +		/* Get the dtb's size */
> +		ldr	r12, [r6, #4]		@ device tree size
> +
> +		/* convert r12 (dtb size) to little endian */
> +		eor	r1, r12, r12, ror #16
> +		bic	r1, r1, #0x00ff0000
> +		mov	r12, r12, ror #8
> +		eor	r12, r12, r1, lsr #8
> +
> +		add	r10, r10, r12
> +		add	r6, r6, r12
> +
> +dtb_check_done:
> +		adr	r1, LC0
> +		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
> +#endif

Instead of clobbering r12, you could use lr instead.

The byte swap on the size should be done only if __ARMEB__ is not 
defined i.e. #ifndef __ARMEB__ ...

Also the DT signature should be endian aware.

>  /*
>   * Check to see if we will overwrite ourselves.
> @@ -223,8 +263,8 @@ restart:	adr	r0, LC0
>   */
>  		cmp	r4, r10
>  		bhs	wont_overwrite
> -		add	r10, r4, r9
> -		cmp	r10, r5
> +		add	r1, r4, r9
> +		cmp	r1, r5
>  		bls	wont_overwrite
>  
>  /*
> @@ -272,8 +312,10 @@ wont_overwrite:
>   *   r12 = GOT end
>   *   sp  = stack pointer
>   */
> -		teq	r0, #0
> -		beq	not_relocated
> +		adr	r1, LC0
> +		ldr	r6, [r1, #16]		@ reload _edata value

Why?

> +
> +		add	r6, r6, r0
>  		add	r11, r11, r0
>  		add	r12, r12, r0
>  
> @@ -288,12 +330,34 @@ wont_overwrite:
>  
>  		/*
>  		 * Relocate all entries in the GOT table.
> +		 * Bump bss entries to past image end (r10)
>  		 */
> +		sub	r5, r10, r6		@ delta of image end and _edata
> +		add	r5, r5, #7		@ ... rounded up to a multiple
> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
> +		             	 		@ ... GOT entry doesn't
> +		             	 		@ ... overwrite end of image

This is wrong. You are going to displace the .bss pointers even if they 
don't need that in the case where no dtb was found.  And if a dtb was 
found the displacement is going to be the size of the dtb _plus_ the 
size of the .bss_stack instead of only the dtb size.

At this point you should only keep track of the .bss displacement in 
addition to the delta offset in r0.  And if both are equal to zero then 
skip over the fixup loop as before.

>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>  		add	r1, r1, r0		@ table.  This fixes up the
> +		cmp	r1, r2
> +		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
> +		movcs	r9, #1			@      1;
> +		cmp	r1, r3
> +		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
> +		cmp	r9, #0
> +		addne	r1, r5			@ entry += r9 ? bss delta : 0;

The above would be much more elegant if written like this:

		cmp	r1, r2
		cmphs	r3, r1
		addhi	r1, r5

>  		str	r1, [r11], #4		@ C references.
>  		cmp	r11, r12
>  		blo	1b
> +
> +		/* bump our bss registers too */
> +		add	r2, r2, r5
> +		add	r3, r3, r5
> +
> +		/* bump the stack pinter, if at or above _edata */
> +		cmp	sp, r6
> +		addcs	sp, sp, r5

This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.

>  #else
>  
>  		/*
> @@ -309,7 +373,7 @@ wont_overwrite:
>  		blo	1b
>  #endif
>  
> -not_relocated:	mov	r0, #0
> +		mov	r0, #0
>  1:		str	r0, [r2], #4		@ clear bss
>  		str	r0, [r2], #4
>  		str	r0, [r2], #4
> @@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
>  		cmp	r2, r3
>  		blo	1b
>  
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/*
> + * The C runtime environment should now be set up sufficiently.
> + *   r4  = kernel execution address
> + *   r6  = _edata
> + *   r7  = architecture ID
> + *   r8  = atags pointer
> + *
> + * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
> + */
> +		ldr	r0, [r6, #0]
> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
> +		cmp	r0, r1
> +		bne	keep_atags
> +
> +		mov	r8, r6			@ use the appended device tree

You should have updated r8 the moment you knew you have a valid dtb 
earlier instead of duplicating this test here.

> +keep_atags:
> +#endif
>  /*
> - * The C runtime environment should now be setup sufficiently.
>   * Set up some pointers, and start decompressing.
>   *   r4  = kernel execution address
>   *   r7  = architecture ID
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
@ 2011-03-01  6:39     ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-01  6:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 28 Feb 2011, John Bonesio wrote:

> This patch provides the ability to boot using a device tree that is appended
> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> 
> Signed-off-by: John Bonesio <bones@secretlab.ca>

Comments below.

> ---
> 
>  arch/arm/Kconfig                |    7 +++
>  arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
>  2 files changed, 94 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d8a330f..68fc640 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1593,6 +1593,13 @@ config USE_OF
>  	help
>  	  Include support for flattened device tree machine descriptions.
>  
> +config ARM_APPENDED_DTB
> +       bool "Use appended device tree blob" if OF
> +       default n

The default is n by default, so you don't need to mention it.

Also this should depend on OF (CONFIG_OF).

> +       help
> +         With this option, the boot code will look for a dtb bianry

s/bianry/binary/

Since this is an help text for people who might not have a clue about 
"dtb", it would be better to spell it out.

> +         appended to zImage.
> +
>  # Compressed boot loader in ROM.  Yes, we really want to ask about
>  # TEXT and BSS so we preserve their values in the config files.
>  config ZBOOT_ROM_TEXT
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 200625c..ae9f8c6 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -210,6 +210,46 @@ restart:	adr	r0, LC0
>  		 */
>  		mov	r10, r6
>  #endif
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/*
> + *   r0  = delta
> + *   r2  = BSS start
> + *   r3  = BSS end
> + *   r4  = final kernel address
> + *   r5  = start of this image
> + *   r6  = _edata
> + *   r7  = architecture ID
> + *   r8  = atags/device tree pointer
> + *   r9  = size of decompressed image
> + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
> + *   r11 = GOT start
> + *   r12 = GOT end
> + *
> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> + * dtb data will get relocated along with the kernel if necessary.
> + */
> +
> +		ldr	r12, [r6, #0]
> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
> +		cmp	r12, r1
> +		bne	dtb_check_done
> +
> +		/* Get the dtb's size */
> +		ldr	r12, [r6, #4]		@ device tree size
> +
> +		/* convert r12 (dtb size) to little endian */
> +		eor	r1, r12, r12, ror #16
> +		bic	r1, r1, #0x00ff0000
> +		mov	r12, r12, ror #8
> +		eor	r12, r12, r1, lsr #8
> +
> +		add	r10, r10, r12
> +		add	r6, r6, r12
> +
> +dtb_check_done:
> +		adr	r1, LC0
> +		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
> +#endif

Instead of clobbering r12, you could use lr instead.

The byte swap on the size should be done only if __ARMEB__ is not 
defined i.e. #ifndef __ARMEB__ ...

Also the DT signature should be endian aware.

>  /*
>   * Check to see if we will overwrite ourselves.
> @@ -223,8 +263,8 @@ restart:	adr	r0, LC0
>   */
>  		cmp	r4, r10
>  		bhs	wont_overwrite
> -		add	r10, r4, r9
> -		cmp	r10, r5
> +		add	r1, r4, r9
> +		cmp	r1, r5
>  		bls	wont_overwrite
>  
>  /*
> @@ -272,8 +312,10 @@ wont_overwrite:
>   *   r12 = GOT end
>   *   sp  = stack pointer
>   */
> -		teq	r0, #0
> -		beq	not_relocated
> +		adr	r1, LC0
> +		ldr	r6, [r1, #16]		@ reload _edata value

Why?

> +
> +		add	r6, r6, r0
>  		add	r11, r11, r0
>  		add	r12, r12, r0
>  
> @@ -288,12 +330,34 @@ wont_overwrite:
>  
>  		/*
>  		 * Relocate all entries in the GOT table.
> +		 * Bump bss entries to past image end (r10)
>  		 */
> +		sub	r5, r10, r6		@ delta of image end and _edata
> +		add	r5, r5, #7		@ ... rounded up to a multiple
> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
> +		             	 		@ ... GOT entry doesn't
> +		             	 		@ ... overwrite end of image

This is wrong. You are going to displace the .bss pointers even if they 
don't need that in the case where no dtb was found.  And if a dtb was 
found the displacement is going to be the size of the dtb _plus_ the 
size of the .bss_stack instead of only the dtb size.

At this point you should only keep track of the .bss displacement in 
addition to the delta offset in r0.  And if both are equal to zero then 
skip over the fixup loop as before.

>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>  		add	r1, r1, r0		@ table.  This fixes up the
> +		cmp	r1, r2
> +		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
> +		movcs	r9, #1			@      1;
> +		cmp	r1, r3
> +		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
> +		cmp	r9, #0
> +		addne	r1, r5			@ entry += r9 ? bss delta : 0;

The above would be much more elegant if written like this:

		cmp	r1, r2
		cmphs	r3, r1
		addhi	r1, r5

>  		str	r1, [r11], #4		@ C references.
>  		cmp	r11, r12
>  		blo	1b
> +
> +		/* bump our bss registers too */
> +		add	r2, r2, r5
> +		add	r3, r3, r5
> +
> +		/* bump the stack pinter, if at or above _edata */
> +		cmp	sp, r6
> +		addcs	sp, sp, r5

This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.

>  #else
>  
>  		/*
> @@ -309,7 +373,7 @@ wont_overwrite:
>  		blo	1b
>  #endif
>  
> -not_relocated:	mov	r0, #0
> +		mov	r0, #0
>  1:		str	r0, [r2], #4		@ clear bss
>  		str	r0, [r2], #4
>  		str	r0, [r2], #4
> @@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
>  		cmp	r2, r3
>  		blo	1b
>  
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/*
> + * The C runtime environment should now be set up sufficiently.
> + *   r4  = kernel execution address
> + *   r6  = _edata
> + *   r7  = architecture ID
> + *   r8  = atags pointer
> + *
> + * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
> + */
> +		ldr	r0, [r6, #0]
> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
> +		cmp	r0, r1
> +		bne	keep_atags
> +
> +		mov	r8, r6			@ use the appended device tree

You should have updated r8 the moment you knew you have a valid dtb 
earlier instead of duplicating this test here.

> +keep_atags:
> +#endif
>  /*
> - * The C runtime environment should now be setup sufficiently.
>   * Set up some pointers, and start decompressing.
>   *   r4  = kernel execution address
>   *   r7  = architecture ID
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] ARM:boot:device tree: Merge specific atags into the device tree
  2011-02-28 23:33   ` John Bonesio
@ 2011-03-01  6:45     ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-01  6:45 UTC (permalink / raw)
  To: John Bonesio
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, 28 Feb 2011, John Bonesio wrote:

> This patch is to merge in key atags into the appended device tree.  An appended
> device tree is where the zImage has a dtb binary appended at the end of it. The
> boot code looks for an appended device tree, then looks for a few key atags
> passed in by the bootloader.
> 
> The bootargs and memory size settings, if they exist, override existing values
> in the appended device tree. If these values don't currently exist in the
> appended device tree, they are added.
> 
> Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

Is this really useful?

I understand this might be handy for testing purposes.  But if your 
bootloader does ATAG only then your hardware is likely to have support 
in the kernel without device tree support just fine.

IOW I don't think this would be of much value in upstream.


> ---
> 
>  arch/arm/boot/compressed/Makefile |   31 ++++++++++++++--
>  arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/compressed/head.S   |   15 ++++++++
>  arch/arm/boot/compressed/misc.c   |   58 ++++++++++++++++++++++++++++++-
>  4 files changed, 167 insertions(+), 7 deletions(-)
>  create mode 100644 arch/arm/boot/compressed/atags.c
> 
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 9d328be..7a2fe22 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -49,6 +49,10 @@ ifeq ($(CONFIG_ARCH_SHMOBILE),y)
>  OBJS		+= head-shmobile.o
>  endif
>  
> +ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
> +OBJS		+= atags.o libfdt.a 
> +endif
> +
>  #
>  # We now have a PIC decompressor implementation.  Decompressors running
>  # from RAM should not define ZTEXTADDR.  Decompressors running directly
> @@ -80,7 +84,9 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
>  KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
>  endif
>  
> -EXTRA_CFLAGS  := -fpic -fno-builtin
> +fdttree      := $(srctree)/scripts/dtc/libfdt
> +
> +EXTRA_CFLAGS  := -fpic -fno-builtin -I$(fdttree) -I$(obj)
>  EXTRA_AFLAGS  := -Wa,-march=all
>  
>  # Provide size of uncompressed kernel to the decompressor via a linker symbol.
> @@ -102,13 +108,28 @@ LDFLAGS_vmlinux += -X
>  LDFLAGS_vmlinux += -T
>  
>  # For __aeabi_uidivmod
> -lib1funcs = $(obj)/lib1funcs.o
> +libfuncs = $(obj)/lib1funcs.o
>  
> -$(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE
> -	$(call cmd,shipped)
> +ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
> +# For memchr, memmove, etc
> +libfuncs += $(obj)/memchr.o $(obj)/strchr.o $(obj)/memmove.o $(obj)/memzero.o
> +endif
> +
> +
> +libfdtheader := $(fdttree)/fdt.h $(fdttree)/libfdt.h $(fdttree)/libfdt_internal.h
> +libfdtobj    := $(obj)/fdt.o $(obj)/fdt_ro.o $(obj)/fdt_wip.o $(obj)/fdt_sw.o $(obj)/fdt_rw.o $(obj)/fdt_strerror.o
> +
> +$(libfdtobj): $(obj)/%.o: $(srctree)/scripts/dtc/libfdt/%.c $(libfdtheader)
> +	$(call cmd_cc_o_c)
> +
> +$(obj)/libfdt.a: $(libfdtobj)
> +	$(AR) rcs $@ $^
> +
> +$(libfuncs): $(obj)/%.o: $(srctree)/arch/$(SRCARCH)/lib/%.S
> +	$(call cmd_as_o_S)
>  
>  $(obj)/vmlinux: $(obj)/vmlinux.lds $(obj)/$(HEAD) $(obj)/piggy.$(suffix_y).o \
> -	 	$(addprefix $(obj)/, $(OBJS)) $(lib1funcs) FORCE
> +	 	$(addprefix $(obj)/, $(OBJS)) $(libfuncs) FORCE
>  	$(call if_changed,ld)
>  	@:
>  
> diff --git a/arch/arm/boot/compressed/atags.c b/arch/arm/boot/compressed/atags.c
> new file mode 100644
> index 0000000..d19d53d
> --- /dev/null
> +++ b/arch/arm/boot/compressed/atags.c
> @@ -0,0 +1,70 @@
> +#include <stddef.h>
> +#include <asm/byteorder.h>
> +#include <asm/setup.h>
> +#include <fdt.h>
> +#include <libfdt.h>
> +
> +int dt_setprop(void *fdt, const char *node_path, const char *property,
> +               uint32_t *val_array, int size)
> +{
> +	int offset;
> +
> +	offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +
> +	return fdt_setprop(fdt, offset, property, val_array, size);
> +}
> +
> +int dt_setprop_string(void *fdt, const char *node_path,
> +                      const char *property, const char *string)
> +{
> +	int offset;
> +		
> +	offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +			
> +	return fdt_setprop_string(fdt, offset, property, string);
> +}
> +
> +int dt_setprop_cell(void *fdt, const char *node_path,
> +                    const char *property, uint32_t val)
> +{
> +	int offset;
> +
> +	offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +
> +	return fdt_setprop_cell(fdt, offset, property, val);
> +}
> +
> +void dt_merge_atags(void *dt,  void *ataglist)
> +{
> +	struct tag *atag = ataglist;
> +	uint32_t mem_reg_property[2];
> +	uint32_t initrd_start, initrd_size;
> +	int rc;
> +
> +	while (atag && atag->hdr.tag != 0) {
> +		if (atag->hdr.tag == ATAG_CMDLINE) {
> +			dt_setprop_string(dt, "/chosen", "bootargs",
> +			                  atag->u.cmdline.cmdline);
> +		} else if (atag->hdr.tag == ATAG_MEM) {
> +			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> +			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> +			dt_setprop(dt, "/memory", "reg", mem_reg_property,
> +			           sizeof(mem_reg_property));
> +		} else if (atag->hdr.tag == ATAG_INITRD2) {
> +			initrd_start = atag->u.initrd.start;
> +			initrd_size = atag->u.initrd.size;
> +			dt_setprop_cell(dt, "/chosen", "linux,initrd-start",
> +			                initrd_start);
> +			dt_setprop_cell(dt, "/chosen", "linux,initrd-end",
> +			                initrd_start + initrd_size);
> +		}
> +		atag = tag_next(atag);
> +	}
> +}
> +
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index ae9f8c6..c2cdc5f 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -211,6 +211,7 @@ restart:	adr	r0, LC0
>  		mov	r10, r6
>  #endif
>  #ifdef CONFIG_ARM_APPENDED_DTB
> +#define MERGE_SPACE	1024
>  /*
>   *   r0  = delta
>   *   r2  = BSS start
> @@ -243,6 +244,8 @@ restart:	adr	r0, LC0
>  		mov	r12, r12, ror #8
>  		eor	r12, r12, r1, lsr #8
>  
> +		add	r12, r12, #MERGE_SPACE	@ extra space to merge in atags
> +
>  		add	r10, r10, r12
>  		add	r6, r6, r12
>  
> @@ -338,6 +341,8 @@ wont_overwrite:
>  		             	 		@ ... GOT entry doesn't
>  		             	 		@ ... overwrite end of image
>  
> +		add	r5, r5, #MERGE_SPACE	@ extra space to merge in atags
> +
>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>  		add	r1, r1, r0		@ table.  This fixes up the
>  		cmp	r1, r2
> @@ -396,6 +401,16 @@ wont_overwrite:
>  		cmp	r0, r1
>  		bne	keep_atags
>  
> +#ifndef CONFIG_ZBOOT_ROM
> +		ldr	r0, [r6, #4]
> +		add	r0, r0, #MERGE_SPACE	
> +		str	r0, [r6, #4]		@ increase device tree size
> +
> +		/* merge in some key atags into the dtb */
> +		mov	r0, r6
> +		mov	r1, r8
> +		bl	dt_merge_atags
> +#endif
>  		mov	r8, r6			@ use the appended device tree
>  keep_atags:
>  #endif
> diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
> index e653a6d..2d4da4c 100644
> --- a/arch/arm/boot/compressed/misc.c
> +++ b/arch/arm/boot/compressed/misc.c
> @@ -29,7 +29,7 @@ unsigned int __machine_arch_type;
>  #include <asm/unaligned.h>
>  
>  
> -static void putstr(const char *ptr);
> +void putstr(const char *ptr);
>  extern void error(char *x);
>  
>  #include <mach/uncompress.h>
> @@ -100,7 +100,7 @@ static void icedcc_putc(int ch)
>  #define putc(ch)	icedcc_putc(ch)
>  #endif
>  
> -static void putstr(const char *ptr)
> +void putstr(const char *ptr)
>  {
>  	char c;
>  
> @@ -114,6 +114,60 @@ static void putstr(const char *ptr)
>  }
>  
>  
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/**
> + * strlen - Find the length of a string
> + * @s: The string to be sized
> + */
> +size_t strlen(const char *s)
> +{
> +        const char *sc;
> +
> +        for (sc = s; *sc != '\0'; ++sc)
> +                /* nothing */;
> +        return sc - s;
> +}
> +
> +/**
> + * strcmp - Compare two strings
> + * @cs: One string
> + * @ct: Another string
> + */
> +#undef strcmp
> +int strcmp(const char *cs, const char *ct)
> +{
> +	unsigned char c1, c2;
> +
> +	while (1) {
> +		c1 = *cs++;
> +		c2 = *ct++;
> +		if (c1 != c2)
> +			return c1 < c2 ? -1 : 1;
> +		if (!c1)
> +			break;
> +	}
> +	return 0;
> +}
> +
> +/**
> + * memcmp - Compare two areas of memory
> + * @cs: One area of memory
> + * @ct: Another area of memory
> + * @count: The size of the area.
> + */
> +#undef memcmp
> +int memcmp(const void *cs, const void *ct, size_t count)
> +{
> +	const unsigned char *su1, *su2;
> +	int res = 0;
> +
> +	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
> +		if ((res = *su1 - *su2) != 0)
> +			break;
> +	return res;
> +}
> +#endif
> +
>  void *memcpy(void *__dest, __const void *__src, size_t __n)
>  {
>  	int i = 0;
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2/4] ARM:boot:device tree: Merge specific atags into the device tree
@ 2011-03-01  6:45     ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-01  6:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 28 Feb 2011, John Bonesio wrote:

> This patch is to merge in key atags into the appended device tree.  An appended
> device tree is where the zImage has a dtb binary appended at the end of it. The
> boot code looks for an appended device tree, then looks for a few key atags
> passed in by the bootloader.
> 
> The bootargs and memory size settings, if they exist, override existing values
> in the appended device tree. If these values don't currently exist in the
> appended device tree, they are added.
> 
> Signed-off-by: John Bonesio <bones@secretlab.ca>

Is this really useful?

I understand this might be handy for testing purposes.  But if your 
bootloader does ATAG only then your hardware is likely to have support 
in the kernel without device tree support just fine.

IOW I don't think this would be of much value in upstream.


> ---
> 
>  arch/arm/boot/compressed/Makefile |   31 ++++++++++++++--
>  arch/arm/boot/compressed/atags.c  |   70 +++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/compressed/head.S   |   15 ++++++++
>  arch/arm/boot/compressed/misc.c   |   58 ++++++++++++++++++++++++++++++-
>  4 files changed, 167 insertions(+), 7 deletions(-)
>  create mode 100644 arch/arm/boot/compressed/atags.c
> 
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 9d328be..7a2fe22 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -49,6 +49,10 @@ ifeq ($(CONFIG_ARCH_SHMOBILE),y)
>  OBJS		+= head-shmobile.o
>  endif
>  
> +ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
> +OBJS		+= atags.o libfdt.a 
> +endif
> +
>  #
>  # We now have a PIC decompressor implementation.  Decompressors running
>  # from RAM should not define ZTEXTADDR.  Decompressors running directly
> @@ -80,7 +84,9 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
>  KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
>  endif
>  
> -EXTRA_CFLAGS  := -fpic -fno-builtin
> +fdttree      := $(srctree)/scripts/dtc/libfdt
> +
> +EXTRA_CFLAGS  := -fpic -fno-builtin -I$(fdttree) -I$(obj)
>  EXTRA_AFLAGS  := -Wa,-march=all
>  
>  # Provide size of uncompressed kernel to the decompressor via a linker symbol.
> @@ -102,13 +108,28 @@ LDFLAGS_vmlinux += -X
>  LDFLAGS_vmlinux += -T
>  
>  # For __aeabi_uidivmod
> -lib1funcs = $(obj)/lib1funcs.o
> +libfuncs = $(obj)/lib1funcs.o
>  
> -$(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE
> -	$(call cmd,shipped)
> +ifeq ($(CONFIG_ARM_APPENDED_DTB),y)
> +# For memchr, memmove, etc
> +libfuncs += $(obj)/memchr.o $(obj)/strchr.o $(obj)/memmove.o $(obj)/memzero.o
> +endif
> +
> +
> +libfdtheader := $(fdttree)/fdt.h $(fdttree)/libfdt.h $(fdttree)/libfdt_internal.h
> +libfdtobj    := $(obj)/fdt.o $(obj)/fdt_ro.o $(obj)/fdt_wip.o $(obj)/fdt_sw.o $(obj)/fdt_rw.o $(obj)/fdt_strerror.o
> +
> +$(libfdtobj): $(obj)/%.o: $(srctree)/scripts/dtc/libfdt/%.c $(libfdtheader)
> +	$(call cmd_cc_o_c)
> +
> +$(obj)/libfdt.a: $(libfdtobj)
> +	$(AR) rcs $@ $^
> +
> +$(libfuncs): $(obj)/%.o: $(srctree)/arch/$(SRCARCH)/lib/%.S
> +	$(call cmd_as_o_S)
>  
>  $(obj)/vmlinux: $(obj)/vmlinux.lds $(obj)/$(HEAD) $(obj)/piggy.$(suffix_y).o \
> -	 	$(addprefix $(obj)/, $(OBJS)) $(lib1funcs) FORCE
> +	 	$(addprefix $(obj)/, $(OBJS)) $(libfuncs) FORCE
>  	$(call if_changed,ld)
>  	@:
>  
> diff --git a/arch/arm/boot/compressed/atags.c b/arch/arm/boot/compressed/atags.c
> new file mode 100644
> index 0000000..d19d53d
> --- /dev/null
> +++ b/arch/arm/boot/compressed/atags.c
> @@ -0,0 +1,70 @@
> +#include <stddef.h>
> +#include <asm/byteorder.h>
> +#include <asm/setup.h>
> +#include <fdt.h>
> +#include <libfdt.h>
> +
> +int dt_setprop(void *fdt, const char *node_path, const char *property,
> +               uint32_t *val_array, int size)
> +{
> +	int offset;
> +
> +	offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +
> +	return fdt_setprop(fdt, offset, property, val_array, size);
> +}
> +
> +int dt_setprop_string(void *fdt, const char *node_path,
> +                      const char *property, const char *string)
> +{
> +	int offset;
> +		
> +	offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +			
> +	return fdt_setprop_string(fdt, offset, property, string);
> +}
> +
> +int dt_setprop_cell(void *fdt, const char *node_path,
> +                    const char *property, uint32_t val)
> +{
> +	int offset;
> +
> +	offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +
> +	return fdt_setprop_cell(fdt, offset, property, val);
> +}
> +
> +void dt_merge_atags(void *dt,  void *ataglist)
> +{
> +	struct tag *atag = ataglist;
> +	uint32_t mem_reg_property[2];
> +	uint32_t initrd_start, initrd_size;
> +	int rc;
> +
> +	while (atag && atag->hdr.tag != 0) {
> +		if (atag->hdr.tag == ATAG_CMDLINE) {
> +			dt_setprop_string(dt, "/chosen", "bootargs",
> +			                  atag->u.cmdline.cmdline);
> +		} else if (atag->hdr.tag == ATAG_MEM) {
> +			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> +			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> +			dt_setprop(dt, "/memory", "reg", mem_reg_property,
> +			           sizeof(mem_reg_property));
> +		} else if (atag->hdr.tag == ATAG_INITRD2) {
> +			initrd_start = atag->u.initrd.start;
> +			initrd_size = atag->u.initrd.size;
> +			dt_setprop_cell(dt, "/chosen", "linux,initrd-start",
> +			                initrd_start);
> +			dt_setprop_cell(dt, "/chosen", "linux,initrd-end",
> +			                initrd_start + initrd_size);
> +		}
> +		atag = tag_next(atag);
> +	}
> +}
> +
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index ae9f8c6..c2cdc5f 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -211,6 +211,7 @@ restart:	adr	r0, LC0
>  		mov	r10, r6
>  #endif
>  #ifdef CONFIG_ARM_APPENDED_DTB
> +#define MERGE_SPACE	1024
>  /*
>   *   r0  = delta
>   *   r2  = BSS start
> @@ -243,6 +244,8 @@ restart:	adr	r0, LC0
>  		mov	r12, r12, ror #8
>  		eor	r12, r12, r1, lsr #8
>  
> +		add	r12, r12, #MERGE_SPACE	@ extra space to merge in atags
> +
>  		add	r10, r10, r12
>  		add	r6, r6, r12
>  
> @@ -338,6 +341,8 @@ wont_overwrite:
>  		             	 		@ ... GOT entry doesn't
>  		             	 		@ ... overwrite end of image
>  
> +		add	r5, r5, #MERGE_SPACE	@ extra space to merge in atags
> +
>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>  		add	r1, r1, r0		@ table.  This fixes up the
>  		cmp	r1, r2
> @@ -396,6 +401,16 @@ wont_overwrite:
>  		cmp	r0, r1
>  		bne	keep_atags
>  
> +#ifndef CONFIG_ZBOOT_ROM
> +		ldr	r0, [r6, #4]
> +		add	r0, r0, #MERGE_SPACE	
> +		str	r0, [r6, #4]		@ increase device tree size
> +
> +		/* merge in some key atags into the dtb */
> +		mov	r0, r6
> +		mov	r1, r8
> +		bl	dt_merge_atags
> +#endif
>  		mov	r8, r6			@ use the appended device tree
>  keep_atags:
>  #endif
> diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
> index e653a6d..2d4da4c 100644
> --- a/arch/arm/boot/compressed/misc.c
> +++ b/arch/arm/boot/compressed/misc.c
> @@ -29,7 +29,7 @@ unsigned int __machine_arch_type;
>  #include <asm/unaligned.h>
>  
>  
> -static void putstr(const char *ptr);
> +void putstr(const char *ptr);
>  extern void error(char *x);
>  
>  #include <mach/uncompress.h>
> @@ -100,7 +100,7 @@ static void icedcc_putc(int ch)
>  #define putc(ch)	icedcc_putc(ch)
>  #endif
>  
> -static void putstr(const char *ptr)
> +void putstr(const char *ptr)
>  {
>  	char c;
>  
> @@ -114,6 +114,60 @@ static void putstr(const char *ptr)
>  }
>  
>  
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/**
> + * strlen - Find the length of a string
> + * @s: The string to be sized
> + */
> +size_t strlen(const char *s)
> +{
> +        const char *sc;
> +
> +        for (sc = s; *sc != '\0'; ++sc)
> +                /* nothing */;
> +        return sc - s;
> +}
> +
> +/**
> + * strcmp - Compare two strings
> + * @cs: One string
> + * @ct: Another string
> + */
> +#undef strcmp
> +int strcmp(const char *cs, const char *ct)
> +{
> +	unsigned char c1, c2;
> +
> +	while (1) {
> +		c1 = *cs++;
> +		c2 = *ct++;
> +		if (c1 != c2)
> +			return c1 < c2 ? -1 : 1;
> +		if (!c1)
> +			break;
> +	}
> +	return 0;
> +}
> +
> +/**
> + * memcmp - Compare two areas of memory
> + * @cs: One area of memory
> + * @ct: Another area of memory
> + * @count: The size of the area.
> + */
> +#undef memcmp
> +int memcmp(const void *cs, const void *ct, size_t count)
> +{
> +	const unsigned char *su1, *su2;
> +	int res = 0;
> +
> +	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
> +		if ((res = *su1 - *su2) != 0)
> +			break;
> +	return res;
> +}
> +#endif
> +
>  void *memcpy(void *__dest, __const void *__src, size_t __n)
>  {
>  	int i = 0;
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/4] ARM:boot:device tree: Allow multiple device trees to be appended to zImage
  2011-02-28 23:33   ` John Bonesio
@ 2011-03-01  6:48     ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-01  6:48 UTC (permalink / raw)
  To: John Bonesio
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, 28 Feb 2011, John Bonesio wrote:

> This patch allows mutliple device tree binaries to be appended to zImage. 

Huh?  Why would you do that?


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 4/4] ARM:boot:device tree: Allow multiple device trees to be appended to zImage
@ 2011-03-01  6:48     ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-01  6:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 28 Feb 2011, John Bonesio wrote:

> This patch allows mutliple device tree binaries to be appended to zImage. 

Huh?  Why would you do that?


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] ARM:boot:device tree: Merge specific atags into the device tree
  2011-03-01  6:45     ` Nicolas Pitre
@ 2011-03-01 14:56         ` Grant Likely
  -1 siblings, 0 replies; 30+ messages in thread
From: Grant Likely @ 2011-03-01 14:56 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, Feb 28, 2011 at 11:45 PM, Nicolas Pitre <nico-vtqb6HGKxmzR7s880joybQ@public.gmane.org> wrote:
> On Mon, 28 Feb 2011, John Bonesio wrote:
>
>> This patch is to merge in key atags into the appended device tree.  An appended
>> device tree is where the zImage has a dtb binary appended at the end of it. The
>> boot code looks for an appended device tree, then looks for a few key atags
>> passed in by the bootloader.
>>
>> The bootargs and memory size settings, if they exist, override existing values
>> in the appended device tree. If these values don't currently exist in the
>> appended device tree, they are added.
>>
>> Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>
> Is this really useful?
>
> I understand this might be handy for testing purposes.  But if your
> bootloader does ATAG only then your hardware is likely to have support
> in the kernel without device tree support just fine.
>
> IOW I don't think this would be of much value in upstream.

That's fine.  This feature was implemented to meet a client's
requirement and to experiment with manipulating the .dtb in the zimage
wrapper.  We're posting it here for completeness if anyone else wants
to play with it.  Same for the last patch that allows appending
multiple dtbs.

g.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2/4] ARM:boot:device tree: Merge specific atags into the device tree
@ 2011-03-01 14:56         ` Grant Likely
  0 siblings, 0 replies; 30+ messages in thread
From: Grant Likely @ 2011-03-01 14:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 28, 2011 at 11:45 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Mon, 28 Feb 2011, John Bonesio wrote:
>
>> This patch is to merge in key atags into the appended device tree. ?An appended
>> device tree is where the zImage has a dtb binary appended at the end of it. The
>> boot code looks for an appended device tree, then looks for a few key atags
>> passed in by the bootloader.
>>
>> The bootargs and memory size settings, if they exist, override existing values
>> in the appended device tree. If these values don't currently exist in the
>> appended device tree, they are added.
>>
>> Signed-off-by: John Bonesio <bones@secretlab.ca>
>
> Is this really useful?
>
> I understand this might be handy for testing purposes. ?But if your
> bootloader does ATAG only then your hardware is likely to have support
> in the kernel without device tree support just fine.
>
> IOW I don't think this would be of much value in upstream.

That's fine.  This feature was implemented to meet a client's
requirement and to experiment with manipulating the .dtb in the zimage
wrapper.  We're posting it here for completeness if anyone else wants
to play with it.  Same for the last patch that allows appending
multiple dtbs.

g.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
  2011-03-01  6:39     ` Nicolas Pitre
@ 2011-03-02  1:41         ` John Bonesio
  -1 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-03-02  1:41 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

I have looked through, and I'm reworking the patch set. I have just one
comment below.

- John

On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> On Mon, 28 Feb 2011, John Bonesio wrote:
> 
>> This patch provides the ability to boot using a device tree that is appended
>> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
>>
>> Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
> Comments below.
> 
>> ---
>>
>>  arch/arm/Kconfig                |    7 +++
>>  arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 94 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index d8a330f..68fc640 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1593,6 +1593,13 @@ config USE_OF
>>  	help
>>  	  Include support for flattened device tree machine descriptions.
>>  
>> +config ARM_APPENDED_DTB
>> +       bool "Use appended device tree blob" if OF
>> +       default n
> 
> The default is n by default, so you don't need to mention it.
> 
> Also this should depend on OF (CONFIG_OF).
> 
>> +       help
>> +         With this option, the boot code will look for a dtb bianry
> 
> s/bianry/binary/
> 
> Since this is an help text for people who might not have a clue about 
> "dtb", it would be better to spell it out.
> 
>> +         appended to zImage.
>> +
>>  # Compressed boot loader in ROM.  Yes, we really want to ask about
>>  # TEXT and BSS so we preserve their values in the config files.
>>  config ZBOOT_ROM_TEXT
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index 200625c..ae9f8c6 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -210,6 +210,46 @@ restart:	adr	r0, LC0
>>  		 */
>>  		mov	r10, r6
>>  #endif
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + *   r0  = delta
>> + *   r2  = BSS start
>> + *   r3  = BSS end
>> + *   r4  = final kernel address
>> + *   r5  = start of this image
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags/device tree pointer
>> + *   r9  = size of decompressed image
>> + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
>> + *   r11 = GOT start
>> + *   r12 = GOT end
>> + *
>> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
>> + * dtb data will get relocated along with the kernel if necessary.
>> + */
>> +
>> +		ldr	r12, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r12, r1
>> +		bne	dtb_check_done
>> +
>> +		/* Get the dtb's size */
>> +		ldr	r12, [r6, #4]		@ device tree size
>> +
>> +		/* convert r12 (dtb size) to little endian */
>> +		eor	r1, r12, r12, ror #16
>> +		bic	r1, r1, #0x00ff0000
>> +		mov	r12, r12, ror #8
>> +		eor	r12, r12, r1, lsr #8
>> +
>> +		add	r10, r10, r12
>> +		add	r6, r6, r12
>> +
>> +dtb_check_done:
>> +		adr	r1, LC0
>> +		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
>> +#endif
> 
> Instead of clobbering r12, you could use lr instead.
> 
> The byte swap on the size should be done only if __ARMEB__ is not 
> defined i.e. #ifndef __ARMEB__ ...
> 
> Also the DT signature should be endian aware.
> 
>>  /*
>>   * Check to see if we will overwrite ourselves.
>> @@ -223,8 +263,8 @@ restart:	adr	r0, LC0
>>   */
>>  		cmp	r4, r10
>>  		bhs	wont_overwrite
>> -		add	r10, r4, r9
>> -		cmp	r10, r5
>> +		add	r1, r4, r9
>> +		cmp	r1, r5
>>  		bls	wont_overwrite
>>  
>>  /*
>> @@ -272,8 +312,10 @@ wont_overwrite:
>>   *   r12 = GOT end
>>   *   sp  = stack pointer
>>   */
>> -		teq	r0, #0
>> -		beq	not_relocated
>> +		adr	r1, LC0
>> +		ldr	r6, [r1, #16]		@ reload _edata value
> 
> Why?
> 
>> +
>> +		add	r6, r6, r0
>>  		add	r11, r11, r0
>>  		add	r12, r12, r0
>>  
>> @@ -288,12 +330,34 @@ wont_overwrite:
>>  
>>  		/*
>>  		 * Relocate all entries in the GOT table.
>> +		 * Bump bss entries to past image end (r10)
>>  		 */
>> +		sub	r5, r10, r6		@ delta of image end and _edata
>> +		add	r5, r5, #7		@ ... rounded up to a multiple
>> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
>> +		             	 		@ ... GOT entry doesn't
>> +		             	 		@ ... overwrite end of image
> 
> This is wrong. You are going to displace the .bss pointers even if they 
> don't need that in the case where no dtb was found.  And if a dtb was 
> found the displacement is going to be the size of the dtb _plus_ the 
> size of the .bss_stack instead of only the dtb size.
> 
> At this point you should only keep track of the .bss displacement in 
> addition to the delta offset in r0.  And if both are equal to zero then 
> skip over the fixup loop as before.
> 
>>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>>  		add	r1, r1, r0		@ table.  This fixes up the
>> +		cmp	r1, r2
>> +		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
>> +		movcs	r9, #1			@      1;
>> +		cmp	r1, r3
>> +		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
>> +		cmp	r9, #0
>> +		addne	r1, r5			@ entry += r9 ? bss delta : 0;
> 
> The above would be much more elegant if written like this:
> 
> 		cmp	r1, r2
> 		cmphs	r3, r1
> 		addhi	r1, r5
> 
>>  		str	r1, [r11], #4		@ C references.
>>  		cmp	r11, r12
>>  		blo	1b
>> +
>> +		/* bump our bss registers too */
>> +		add	r2, r2, r5
>> +		add	r3, r3, r5
>> +
>> +		/* bump the stack pinter, if at or above _edata */
>> +		cmp	sp, r6
>> +		addcs	sp, sp, r5
> 
> This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.

Right now sp will always be above r6. I thought it might be prudent to
add the test in case the linker script changed and the stack was placed
elsewhere. It might save someone a headache later.

> 
>>  #else
>>  
>>  		/*
>> @@ -309,7 +373,7 @@ wont_overwrite:
>>  		blo	1b
>>  #endif
>>  
>> -not_relocated:	mov	r0, #0
>> +		mov	r0, #0
>>  1:		str	r0, [r2], #4		@ clear bss
>>  		str	r0, [r2], #4
>>  		str	r0, [r2], #4
>> @@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
>>  		cmp	r2, r3
>>  		blo	1b
>>  
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + * The C runtime environment should now be set up sufficiently.
>> + *   r4  = kernel execution address
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags pointer
>> + *
>> + * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
>> + */
>> +		ldr	r0, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r0, r1
>> +		bne	keep_atags
>> +
>> +		mov	r8, r6			@ use the appended device tree
> 
> You should have updated r8 the moment you knew you have a valid dtb 
> earlier instead of duplicating this test here.
> 
>> +keep_atags:
>> +#endif
>>  /*
>> - * The C runtime environment should now be setup sufficiently.
>>   * Set up some pointers, and start decompressing.
>>   *   r4  = kernel execution address
>>   *   r7  = architecture ID
>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
@ 2011-03-02  1:41         ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-03-02  1:41 UTC (permalink / raw)
  To: linux-arm-kernel

I have looked through, and I'm reworking the patch set. I have just one
comment below.

- John

On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> On Mon, 28 Feb 2011, John Bonesio wrote:
> 
>> This patch provides the ability to boot using a device tree that is appended
>> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
>>
>> Signed-off-by: John Bonesio <bones@secretlab.ca>
> 
> Comments below.
> 
>> ---
>>
>>  arch/arm/Kconfig                |    7 +++
>>  arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 94 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index d8a330f..68fc640 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1593,6 +1593,13 @@ config USE_OF
>>  	help
>>  	  Include support for flattened device tree machine descriptions.
>>  
>> +config ARM_APPENDED_DTB
>> +       bool "Use appended device tree blob" if OF
>> +       default n
> 
> The default is n by default, so you don't need to mention it.
> 
> Also this should depend on OF (CONFIG_OF).
> 
>> +       help
>> +         With this option, the boot code will look for a dtb bianry
> 
> s/bianry/binary/
> 
> Since this is an help text for people who might not have a clue about 
> "dtb", it would be better to spell it out.
> 
>> +         appended to zImage.
>> +
>>  # Compressed boot loader in ROM.  Yes, we really want to ask about
>>  # TEXT and BSS so we preserve their values in the config files.
>>  config ZBOOT_ROM_TEXT
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index 200625c..ae9f8c6 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -210,6 +210,46 @@ restart:	adr	r0, LC0
>>  		 */
>>  		mov	r10, r6
>>  #endif
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + *   r0  = delta
>> + *   r2  = BSS start
>> + *   r3  = BSS end
>> + *   r4  = final kernel address
>> + *   r5  = start of this image
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags/device tree pointer
>> + *   r9  = size of decompressed image
>> + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
>> + *   r11 = GOT start
>> + *   r12 = GOT end
>> + *
>> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
>> + * dtb data will get relocated along with the kernel if necessary.
>> + */
>> +
>> +		ldr	r12, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r12, r1
>> +		bne	dtb_check_done
>> +
>> +		/* Get the dtb's size */
>> +		ldr	r12, [r6, #4]		@ device tree size
>> +
>> +		/* convert r12 (dtb size) to little endian */
>> +		eor	r1, r12, r12, ror #16
>> +		bic	r1, r1, #0x00ff0000
>> +		mov	r12, r12, ror #8
>> +		eor	r12, r12, r1, lsr #8
>> +
>> +		add	r10, r10, r12
>> +		add	r6, r6, r12
>> +
>> +dtb_check_done:
>> +		adr	r1, LC0
>> +		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
>> +#endif
> 
> Instead of clobbering r12, you could use lr instead.
> 
> The byte swap on the size should be done only if __ARMEB__ is not 
> defined i.e. #ifndef __ARMEB__ ...
> 
> Also the DT signature should be endian aware.
> 
>>  /*
>>   * Check to see if we will overwrite ourselves.
>> @@ -223,8 +263,8 @@ restart:	adr	r0, LC0
>>   */
>>  		cmp	r4, r10
>>  		bhs	wont_overwrite
>> -		add	r10, r4, r9
>> -		cmp	r10, r5
>> +		add	r1, r4, r9
>> +		cmp	r1, r5
>>  		bls	wont_overwrite
>>  
>>  /*
>> @@ -272,8 +312,10 @@ wont_overwrite:
>>   *   r12 = GOT end
>>   *   sp  = stack pointer
>>   */
>> -		teq	r0, #0
>> -		beq	not_relocated
>> +		adr	r1, LC0
>> +		ldr	r6, [r1, #16]		@ reload _edata value
> 
> Why?
> 
>> +
>> +		add	r6, r6, r0
>>  		add	r11, r11, r0
>>  		add	r12, r12, r0
>>  
>> @@ -288,12 +330,34 @@ wont_overwrite:
>>  
>>  		/*
>>  		 * Relocate all entries in the GOT table.
>> +		 * Bump bss entries to past image end (r10)
>>  		 */
>> +		sub	r5, r10, r6		@ delta of image end and _edata
>> +		add	r5, r5, #7		@ ... rounded up to a multiple
>> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
>> +		             	 		@ ... GOT entry doesn't
>> +		             	 		@ ... overwrite end of image
> 
> This is wrong. You are going to displace the .bss pointers even if they 
> don't need that in the case where no dtb was found.  And if a dtb was 
> found the displacement is going to be the size of the dtb _plus_ the 
> size of the .bss_stack instead of only the dtb size.
> 
> At this point you should only keep track of the .bss displacement in 
> addition to the delta offset in r0.  And if both are equal to zero then 
> skip over the fixup loop as before.
> 
>>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>>  		add	r1, r1, r0		@ table.  This fixes up the
>> +		cmp	r1, r2
>> +		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
>> +		movcs	r9, #1			@      1;
>> +		cmp	r1, r3
>> +		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
>> +		cmp	r9, #0
>> +		addne	r1, r5			@ entry += r9 ? bss delta : 0;
> 
> The above would be much more elegant if written like this:
> 
> 		cmp	r1, r2
> 		cmphs	r3, r1
> 		addhi	r1, r5
> 
>>  		str	r1, [r11], #4		@ C references.
>>  		cmp	r11, r12
>>  		blo	1b
>> +
>> +		/* bump our bss registers too */
>> +		add	r2, r2, r5
>> +		add	r3, r3, r5
>> +
>> +		/* bump the stack pinter, if at or above _edata */
>> +		cmp	sp, r6
>> +		addcs	sp, sp, r5
> 
> This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.

Right now sp will always be above r6. I thought it might be prudent to
add the test in case the linker script changed and the stack was placed
elsewhere. It might save someone a headache later.

> 
>>  #else
>>  
>>  		/*
>> @@ -309,7 +373,7 @@ wont_overwrite:
>>  		blo	1b
>>  #endif
>>  
>> -not_relocated:	mov	r0, #0
>> +		mov	r0, #0
>>  1:		str	r0, [r2], #4		@ clear bss
>>  		str	r0, [r2], #4
>>  		str	r0, [r2], #4
>> @@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
>>  		cmp	r2, r3
>>  		blo	1b
>>  
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + * The C runtime environment should now be set up sufficiently.
>> + *   r4  = kernel execution address
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags pointer
>> + *
>> + * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
>> + */
>> +		ldr	r0, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r0, r1
>> +		bne	keep_atags
>> +
>> +		mov	r8, r6			@ use the appended device tree
> 
> You should have updated r8 the moment you knew you have a valid dtb 
> earlier instead of duplicating this test here.
> 
>> +keep_atags:
>> +#endif
>>  /*
>> - * The C runtime environment should now be setup sufficiently.
>>   * Set up some pointers, and start decompressing.
>>   *   r4  = kernel execution address
>>   *   r7  = architecture ID
>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
  2011-03-01  6:39     ` Nicolas Pitre
@ 2011-03-02  2:13         ` John Bonesio
  -1 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-03-02  2:13 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Comment/question below.

On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> On Mon, 28 Feb 2011, John Bonesio wrote:
> 
>> This patch provides the ability to boot using a device tree that is appended
>> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
>>
>> Signed-off-by: John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
> Comments below.
> 
>> ---
>>
>>  arch/arm/Kconfig                |    7 +++
>>  arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 94 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index d8a330f..68fc640 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1593,6 +1593,13 @@ config USE_OF
>>  	help
>>  	  Include support for flattened device tree machine descriptions.
>>  
>> +config ARM_APPENDED_DTB
>> +       bool "Use appended device tree blob" if OF
>> +       default n
> 
> The default is n by default, so you don't need to mention it.
> 
> Also this should depend on OF (CONFIG_OF).
> 
>> +       help
>> +         With this option, the boot code will look for a dtb bianry
> 
> s/bianry/binary/
> 
> Since this is an help text for people who might not have a clue about 
> "dtb", it would be better to spell it out.
> 
>> +         appended to zImage.
>> +
>>  # Compressed boot loader in ROM.  Yes, we really want to ask about
>>  # TEXT and BSS so we preserve their values in the config files.
>>  config ZBOOT_ROM_TEXT
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index 200625c..ae9f8c6 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -210,6 +210,46 @@ restart:	adr	r0, LC0
>>  		 */
>>  		mov	r10, r6
>>  #endif
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + *   r0  = delta
>> + *   r2  = BSS start
>> + *   r3  = BSS end
>> + *   r4  = final kernel address
>> + *   r5  = start of this image
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags/device tree pointer
>> + *   r9  = size of decompressed image
>> + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
>> + *   r11 = GOT start
>> + *   r12 = GOT end
>> + *
>> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
>> + * dtb data will get relocated along with the kernel if necessary.
>> + */
>> +
>> +		ldr	r12, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r12, r1
>> +		bne	dtb_check_done
>> +
>> +		/* Get the dtb's size */
>> +		ldr	r12, [r6, #4]		@ device tree size
>> +
>> +		/* convert r12 (dtb size) to little endian */
>> +		eor	r1, r12, r12, ror #16
>> +		bic	r1, r1, #0x00ff0000
>> +		mov	r12, r12, ror #8
>> +		eor	r12, r12, r1, lsr #8
>> +
>> +		add	r10, r10, r12
>> +		add	r6, r6, r12
>> +
>> +dtb_check_done:
>> +		adr	r1, LC0
>> +		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
>> +#endif
> 
> Instead of clobbering r12, you could use lr instead.
> 
> The byte swap on the size should be done only if __ARMEB__ is not 
> defined i.e. #ifndef __ARMEB__ ...
> 
> Also the DT signature should be endian aware.
> 
>>  /*
>>   * Check to see if we will overwrite ourselves.
>> @@ -223,8 +263,8 @@ restart:	adr	r0, LC0
>>   */
>>  		cmp	r4, r10
>>  		bhs	wont_overwrite
>> -		add	r10, r4, r9
>> -		cmp	r10, r5
>> +		add	r1, r4, r9
>> +		cmp	r1, r5
>>  		bls	wont_overwrite
>>  
>>  /*
>> @@ -272,8 +312,10 @@ wont_overwrite:
>>   *   r12 = GOT end
>>   *   sp  = stack pointer
>>   */
>> -		teq	r0, #0
>> -		beq	not_relocated
>> +		adr	r1, LC0
>> +		ldr	r6, [r1, #16]		@ reload _edata value
> 
> Why?
> 
>> +
>> +		add	r6, r6, r0
>>  		add	r11, r11, r0
>>  		add	r12, r12, r0
>>  
>> @@ -288,12 +330,34 @@ wont_overwrite:
>>  
>>  		/*
>>  		 * Relocate all entries in the GOT table.
>> +		 * Bump bss entries to past image end (r10)
>>  		 */
>> +		sub	r5, r10, r6		@ delta of image end and _edata
>> +		add	r5, r5, #7		@ ... rounded up to a multiple
>> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
>> +		             	 		@ ... GOT entry doesn't
>> +		             	 		@ ... overwrite end of image
> 
> This is wrong. You are going to displace the .bss pointers even if they 
> don't need that in the case where no dtb was found.  And if a dtb was 
> found the displacement is going to be the size of the dtb _plus_ the 
> size of the .bss_stack instead of only the dtb size.
> 
> At this point you should only keep track of the .bss displacement in 
> addition to the delta offset in r0.  And if both are equal to zero then 
> skip over the fixup loop as before.

Maybe I'm not understanding correctly. I think that if there is an
appended dtb, then there are sections like the code and data that needs
to be adjusted by the old r0 value, while the bss and the stack need to
be adjusted by the old r0 + dtb size.

If my understanding is right, then we can't just add the dtb size to r0
and adjust everything.

Am I missing something?


> 
>>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>>  		add	r1, r1, r0		@ table.  This fixes up the
>> +		cmp	r1, r2
>> +		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
>> +		movcs	r9, #1			@      1;
>> +		cmp	r1, r3
>> +		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
>> +		cmp	r9, #0
>> +		addne	r1, r5			@ entry += r9 ? bss delta : 0;
> 
> The above would be much more elegant if written like this:
> 
> 		cmp	r1, r2
> 		cmphs	r3, r1
> 		addhi	r1, r5
> 
>>  		str	r1, [r11], #4		@ C references.
>>  		cmp	r11, r12
>>  		blo	1b
>> +
>> +		/* bump our bss registers too */
>> +		add	r2, r2, r5
>> +		add	r3, r3, r5
>> +
>> +		/* bump the stack pinter, if at or above _edata */
>> +		cmp	sp, r6
>> +		addcs	sp, sp, r5
> 
> This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.
> 
>>  #else
>>  
>>  		/*
>> @@ -309,7 +373,7 @@ wont_overwrite:
>>  		blo	1b
>>  #endif
>>  
>> -not_relocated:	mov	r0, #0
>> +		mov	r0, #0
>>  1:		str	r0, [r2], #4		@ clear bss
>>  		str	r0, [r2], #4
>>  		str	r0, [r2], #4
>> @@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
>>  		cmp	r2, r3
>>  		blo	1b
>>  
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + * The C runtime environment should now be set up sufficiently.
>> + *   r4  = kernel execution address
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags pointer
>> + *
>> + * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
>> + */
>> +		ldr	r0, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r0, r1
>> +		bne	keep_atags
>> +
>> +		mov	r8, r6			@ use the appended device tree
> 
> You should have updated r8 the moment you knew you have a valid dtb 
> earlier instead of duplicating this test here.
> 
>> +keep_atags:
>> +#endif
>>  /*
>> - * The C runtime environment should now be setup sufficiently.
>>   * Set up some pointers, and start decompressing.
>>   *   r4  = kernel execution address
>>   *   r7  = architecture ID
>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
@ 2011-03-02  2:13         ` John Bonesio
  0 siblings, 0 replies; 30+ messages in thread
From: John Bonesio @ 2011-03-02  2:13 UTC (permalink / raw)
  To: linux-arm-kernel

Comment/question below.

On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> On Mon, 28 Feb 2011, John Bonesio wrote:
> 
>> This patch provides the ability to boot using a device tree that is appended
>> to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
>>
>> Signed-off-by: John Bonesio <bones@secretlab.ca>
> 
> Comments below.
> 
>> ---
>>
>>  arch/arm/Kconfig                |    7 +++
>>  arch/arm/boot/compressed/head.S |   93 ++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 94 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index d8a330f..68fc640 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1593,6 +1593,13 @@ config USE_OF
>>  	help
>>  	  Include support for flattened device tree machine descriptions.
>>  
>> +config ARM_APPENDED_DTB
>> +       bool "Use appended device tree blob" if OF
>> +       default n
> 
> The default is n by default, so you don't need to mention it.
> 
> Also this should depend on OF (CONFIG_OF).
> 
>> +       help
>> +         With this option, the boot code will look for a dtb bianry
> 
> s/bianry/binary/
> 
> Since this is an help text for people who might not have a clue about 
> "dtb", it would be better to spell it out.
> 
>> +         appended to zImage.
>> +
>>  # Compressed boot loader in ROM.  Yes, we really want to ask about
>>  # TEXT and BSS so we preserve their values in the config files.
>>  config ZBOOT_ROM_TEXT
>> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
>> index 200625c..ae9f8c6 100644
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -210,6 +210,46 @@ restart:	adr	r0, LC0
>>  		 */
>>  		mov	r10, r6
>>  #endif
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + *   r0  = delta
>> + *   r2  = BSS start
>> + *   r3  = BSS end
>> + *   r4  = final kernel address
>> + *   r5  = start of this image
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags/device tree pointer
>> + *   r9  = size of decompressed image
>> + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
>> + *   r11 = GOT start
>> + *   r12 = GOT end
>> + *
>> + * if there are device trees (dtb) appended to zImage, advance r10 so that the
>> + * dtb data will get relocated along with the kernel if necessary.
>> + */
>> +
>> +		ldr	r12, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r12, r1
>> +		bne	dtb_check_done
>> +
>> +		/* Get the dtb's size */
>> +		ldr	r12, [r6, #4]		@ device tree size
>> +
>> +		/* convert r12 (dtb size) to little endian */
>> +		eor	r1, r12, r12, ror #16
>> +		bic	r1, r1, #0x00ff0000
>> +		mov	r12, r12, ror #8
>> +		eor	r12, r12, r1, lsr #8
>> +
>> +		add	r10, r10, r12
>> +		add	r6, r6, r12
>> +
>> +dtb_check_done:
>> +		adr	r1, LC0
>> +		ldr	r12, [r1, #28]		@ restore r12 (GOT end)
>> +#endif
> 
> Instead of clobbering r12, you could use lr instead.
> 
> The byte swap on the size should be done only if __ARMEB__ is not 
> defined i.e. #ifndef __ARMEB__ ...
> 
> Also the DT signature should be endian aware.
> 
>>  /*
>>   * Check to see if we will overwrite ourselves.
>> @@ -223,8 +263,8 @@ restart:	adr	r0, LC0
>>   */
>>  		cmp	r4, r10
>>  		bhs	wont_overwrite
>> -		add	r10, r4, r9
>> -		cmp	r10, r5
>> +		add	r1, r4, r9
>> +		cmp	r1, r5
>>  		bls	wont_overwrite
>>  
>>  /*
>> @@ -272,8 +312,10 @@ wont_overwrite:
>>   *   r12 = GOT end
>>   *   sp  = stack pointer
>>   */
>> -		teq	r0, #0
>> -		beq	not_relocated
>> +		adr	r1, LC0
>> +		ldr	r6, [r1, #16]		@ reload _edata value
> 
> Why?
> 
>> +
>> +		add	r6, r6, r0
>>  		add	r11, r11, r0
>>  		add	r12, r12, r0
>>  
>> @@ -288,12 +330,34 @@ wont_overwrite:
>>  
>>  		/*
>>  		 * Relocate all entries in the GOT table.
>> +		 * Bump bss entries to past image end (r10)
>>  		 */
>> +		sub	r5, r10, r6		@ delta of image end and _edata
>> +		add	r5, r5, #7		@ ... rounded up to a multiple
>> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
>> +		             	 		@ ... GOT entry doesn't
>> +		             	 		@ ... overwrite end of image
> 
> This is wrong. You are going to displace the .bss pointers even if they 
> don't need that in the case where no dtb was found.  And if a dtb was 
> found the displacement is going to be the size of the dtb _plus_ the 
> size of the .bss_stack instead of only the dtb size.
> 
> At this point you should only keep track of the .bss displacement in 
> addition to the delta offset in r0.  And if both are equal to zero then 
> skip over the fixup loop as before.

Maybe I'm not understanding correctly. I think that if there is an
appended dtb, then there are sections like the code and data that needs
to be adjusted by the old r0 value, while the bss and the stack need to
be adjusted by the old r0 + dtb size.

If my understanding is right, then we can't just add the dtb size to r0
and adjust everything.

Am I missing something?


> 
>>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
>>  		add	r1, r1, r0		@ table.  This fixes up the
>> +		cmp	r1, r2
>> +		movcc	r9, #0			@ r9 = entry < bss_start ? 0 :
>> +		movcs	r9, #1			@      1;
>> +		cmp	r1, r3
>> +		movcs	r9, #0			@ r9 = entry >= end ? 0 : t1;
>> +		cmp	r9, #0
>> +		addne	r1, r5			@ entry += r9 ? bss delta : 0;
> 
> The above would be much more elegant if written like this:
> 
> 		cmp	r1, r2
> 		cmphs	r3, r1
> 		addhi	r1, r5
> 
>>  		str	r1, [r11], #4		@ C references.
>>  		cmp	r11, r12
>>  		blo	1b
>> +
>> +		/* bump our bss registers too */
>> +		add	r2, r2, r5
>> +		add	r3, r3, r5
>> +
>> +		/* bump the stack pinter, if at or above _edata */
>> +		cmp	sp, r6
>> +		addcs	sp, sp, r5
> 
> This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.
> 
>>  #else
>>  
>>  		/*
>> @@ -309,7 +373,7 @@ wont_overwrite:
>>  		blo	1b
>>  #endif
>>  
>> -not_relocated:	mov	r0, #0
>> +		mov	r0, #0
>>  1:		str	r0, [r2], #4		@ clear bss
>>  		str	r0, [r2], #4
>>  		str	r0, [r2], #4
>> @@ -317,8 +381,25 @@ not_relocated:	mov	r0, #0
>>  		cmp	r2, r3
>>  		blo	1b
>>  
>> +#ifdef CONFIG_ARM_APPENDED_DTB
>> +/*
>> + * The C runtime environment should now be set up sufficiently.
>> + *   r4  = kernel execution address
>> + *   r6  = _edata
>> + *   r7  = architecture ID
>> + *   r8  = atags pointer
>> + *
>> + * if there is a device tree (dtb) appended to zImage, set up to use this dtb.
>> + */
>> +		ldr	r0, [r6, #0]
>> +		ldr	r1, =0xedfe0dd0		@ sig is 0xdoodfeed big endian
>> +		cmp	r0, r1
>> +		bne	keep_atags
>> +
>> +		mov	r8, r6			@ use the appended device tree
> 
> You should have updated r8 the moment you knew you have a valid dtb 
> earlier instead of duplicating this test here.
> 
>> +keep_atags:
>> +#endif
>>  /*
>> - * The C runtime environment should now be setup sufficiently.
>>   * Set up some pointers, and start decompressing.
>>   *   r4  = kernel execution address
>>   *   r7  = architecture ID
>>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
  2011-03-02  1:41         ` John Bonesio
@ 2011-03-02  2:37             ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-02  2:37 UTC (permalink / raw)
  To: John Bonesio
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Tue, 1 Mar 2011, John Bonesio wrote:

> I have looked through, and I'm reworking the patch set. I have just one
> comment below.
> 
> - John
> 
> On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> > On Mon, 28 Feb 2011, John Bonesio wrote:
> > 

[...]

> >> +
> >> +		/* bump our bss registers too */
> >> +		add	r2, r2, r5
> >> +		add	r3, r3, r5
> >> +
> >> +		/* bump the stack pinter, if at or above _edata */
> >> +		cmp	sp, r6
> >> +		addcs	sp, sp, r5
> > 
> > This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.
> 
> Right now sp will always be above r6. I thought it might be prudent to
> add the test in case the linker script changed and the stack was placed
> elsewhere. It might save someone a headache later.

That's what comments are for.  And if someone modifies the linker script 
without understanding the affected code then that someone deserves the 
pain.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
@ 2011-03-02  2:37             ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-02  2:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 1 Mar 2011, John Bonesio wrote:

> I have looked through, and I'm reworking the patch set. I have just one
> comment below.
> 
> - John
> 
> On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> > On Mon, 28 Feb 2011, John Bonesio wrote:
> > 

[...]

> >> +
> >> +		/* bump our bss registers too */
> >> +		add	r2, r2, r5
> >> +		add	r3, r3, r5
> >> +
> >> +		/* bump the stack pinter, if at or above _edata */
> >> +		cmp	sp, r6
> >> +		addcs	sp, sp, r5
> > 
> > This will always be true as this is within #ifndef CONFIG_ZBOOT_ROM.
> 
> Right now sp will always be above r6. I thought it might be prudent to
> add the test in case the linker script changed and the stack was placed
> elsewhere. It might save someone a headache later.

That's what comments are for.  And if someone modifies the linker script 
without understanding the affected code then that someone deserves the 
pain.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
  2011-03-02  2:13         ` John Bonesio
@ 2011-03-02  2:57             ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-02  2:57 UTC (permalink / raw)
  To: John Bonesio
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	glikely-s3s/WqlpOiPyB63q8FvJNQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Tue, 1 Mar 2011, John Bonesio wrote:

> Comment/question below.
> 
> On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> > On Mon, 28 Feb 2011, John Bonesio wrote:
> > 
> >> @@ -288,12 +330,34 @@ wont_overwrite:
> >>  
> >>  		/*
> >>  		 * Relocate all entries in the GOT table.
> >> +		 * Bump bss entries to past image end (r10)
> >>  		 */
> >> +		sub	r5, r10, r6		@ delta of image end and _edata
> >> +		add	r5, r5, #7		@ ... rounded up to a multiple
> >> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
> >> +		             	 		@ ... GOT entry doesn't
> >> +		             	 		@ ... overwrite end of image
> > 
> > This is wrong. You are going to displace the .bss pointers even if they 
> > don't need that in the case where no dtb was found.  And if a dtb was 
> > found the displacement is going to be the size of the dtb _plus_ the 
> > size of the .bss_stack instead of only the dtb size.
> > 
> > At this point you should only keep track of the .bss displacement in 
> > addition to the delta offset in r0.  And if both are equal to zero then 
> > skip over the fixup loop as before.
> 
> Maybe I'm not understanding correctly. I think that if there is an
> appended dtb, then there are sections like the code and data that needs
> to be adjusted by the old r0 value, while the bss and the stack need to
> be adjusted by the old r0 + dtb size.

Exact.

> If my understanding is right, then we can't just add the dtb size to r0
> and adjust everything.

indeed.

> Am I missing something?

Suppose that no dtb was found.  In that case, r10 is still pointing to 
the top of the stack.  So r5 will contain the size of .bss and the 
stack.  Then you bump .bss pointers in the GOT by that size.  If a dtb 
was found then r5 ends up with the size of the dtb plus .bss plus stack.

What I'm suggesting is that the size of the dtb be kept in a register of 
its own up to this point.  That size can be 0 if none was found.  If
r0 == 0 and dtb_size == 0 then the whole GOT fixup can be skipped as it 
is done at the moment by branching to not_relocated.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
@ 2011-03-02  2:57             ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-03-02  2:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 1 Mar 2011, John Bonesio wrote:

> Comment/question below.
> 
> On 02/28/2011 10:39 PM, Nicolas Pitre wrote:
> > On Mon, 28 Feb 2011, John Bonesio wrote:
> > 
> >> @@ -288,12 +330,34 @@ wont_overwrite:
> >>  
> >>  		/*
> >>  		 * Relocate all entries in the GOT table.
> >> +		 * Bump bss entries to past image end (r10)
> >>  		 */
> >> +		sub	r5, r10, r6		@ delta of image end and _edata
> >> +		add	r5, r5, #7		@ ... rounded up to a multiple
> >> +		bic	r5, r5, #7		@ ... of 8 bytes, so misaligned
> >> +		             	 		@ ... GOT entry doesn't
> >> +		             	 		@ ... overwrite end of image
> > 
> > This is wrong. You are going to displace the .bss pointers even if they 
> > don't need that in the case where no dtb was found.  And if a dtb was 
> > found the displacement is going to be the size of the dtb _plus_ the 
> > size of the .bss_stack instead of only the dtb size.
> > 
> > At this point you should only keep track of the .bss displacement in 
> > addition to the delta offset in r0.  And if both are equal to zero then 
> > skip over the fixup loop as before.
> 
> Maybe I'm not understanding correctly. I think that if there is an
> appended dtb, then there are sections like the code and data that needs
> to be adjusted by the old r0 value, while the bss and the stack need to
> be adjusted by the old r0 + dtb size.

Exact.

> If my understanding is right, then we can't just add the dtb size to r0
> and adjust everything.

indeed.

> Am I missing something?

Suppose that no dtb was found.  In that case, r10 is still pointing to 
the top of the stack.  So r5 will contain the size of .bss and the 
stack.  Then you bump .bss pointers in the GOT by that size.  If a dtb 
was found then r5 ends up with the size of the dtb plus .bss plus stack.

What I'm suggesting is that the size of the dtb be kept in a register of 
its own up to this point.  That size can be 0 if none was found.  If
r0 == 0 and dtb_size == 0 then the whole GOT fixup can be skipped as it 
is done at the moment by branching to not_relocated.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2011-03-02  2:57 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-28 23:33 [PATCH 0/4] Support for device-tree appended to zImage John Bonesio
2011-02-28 23:33 ` John Bonesio
2011-02-28 23:33 ` [PATCH 1/4] ARM:boot:device tree: Allow the device tree binary to be " John Bonesio
2011-02-28 23:33   ` John Bonesio
2011-03-01  6:39   ` Nicolas Pitre
2011-03-01  6:39     ` Nicolas Pitre
     [not found]     ` <alpine.LFD.2.00.1103010019160.10356-QuJgVwGFrdf/9pzu0YdTqQ@public.gmane.org>
2011-03-02  1:41       ` John Bonesio
2011-03-02  1:41         ` John Bonesio
     [not found]         ` <4D6DA062.8010904-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2011-03-02  2:37           ` Nicolas Pitre
2011-03-02  2:37             ` Nicolas Pitre
2011-03-02  2:13       ` John Bonesio
2011-03-02  2:13         ` John Bonesio
     [not found]         ` <4D6DA7C3.4040609-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2011-03-02  2:57           ` Nicolas Pitre
2011-03-02  2:57             ` Nicolas Pitre
2011-02-28 23:33 ` [PATCH 2/4] ARM:boot:device tree: Merge specific atags into the device tree John Bonesio
2011-02-28 23:33   ` John Bonesio
2011-03-01  6:45   ` Nicolas Pitre
2011-03-01  6:45     ` Nicolas Pitre
     [not found]     ` <alpine.LFD.2.00.1103010141140.10356-QuJgVwGFrdf/9pzu0YdTqQ@public.gmane.org>
2011-03-01 14:56       ` Grant Likely
2011-03-01 14:56         ` Grant Likely
2011-02-28 23:33 ` [PATCH 3/4] ARM:boot:device tree: Add puthex routine for use in the boot wrapper John Bonesio
2011-02-28 23:33   ` John Bonesio
2011-02-28 23:33 ` [PATCH 4/4] ARM:boot:device tree: Allow multiple device trees to be appended to zImage John Bonesio
2011-02-28 23:33   ` John Bonesio
2011-03-01  6:48   ` Nicolas Pitre
2011-03-01  6:48     ` Nicolas Pitre
2011-03-01  2:58 ` [PATCH 0/4] Support for device-tree " Shawn Guo
2011-03-01  2:58   ` Shawn Guo
     [not found]   ` <20110301025836.GA3920-+NayF8gZjK2ctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2011-03-01  3:25     ` John Bonesio
2011-03-01  3:25       ` John Bonesio

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.