[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

This is a resend of those patches with fixups after the latest changes
in mainline.

[PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
This one is new and trivial.

[PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
Mostly John Bonesio's version with some adjustments and cleanups.

[PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
New, simpler alternative to Tony Lindgren's version.


Nicolas

[PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size

[Nicolas Pitre]

This is needed for proper alignment when the DTB appending feature
is used.

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/boot/compressed/vmlinux.lds.in |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/compressed/vmlinux.lds.in b/arch/arm/boot/compressed/vmlinux.lds.in
index ea80abe..6c02db1 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.in
+++ b/arch/arm/boot/compressed/vmlinux.lds.in
@@ -47,6 +47,9 @@ SECTIONS
   .got			: { *(.got) }
   _got_end = .;
   .got.plt		: { *(.got.plt) }
+
+  /* ensure the zImage file size is always a multiple of 64 bits */
+  .pad			: { BYTE(0); . = ALIGN(8); }
   _edata = .;
 
   . = BSS_START;
-- 
1.7.4

[PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary

[Nicolas Pitre]

From: John Bonesio <bones@secretlab.ca>

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>
[nico: adjusted to latest zImage changes plus additional cleanups]
Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/Kconfig                |    8 ++++
 arch/arm/boot/compressed/head.S |   70 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9adc278..66b7d1e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1727,6 +1727,14 @@ config ZBOOT_ROM_MMCIF
 	  which in turn loads the rest the kernel image to RAM using the
 	  MMCIF hardware block.
 
+config ARM_APPENDED_DTB
+	bool "Use appended device tree blob to zImage"
+	depends on OF && !ZBOOT_ROM
+	help
+	  With this option, the boot code will look for a device tree binary
+	  (dtb) appended to zImage
+	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
+
 config CMDLINE
 	string "Default kernel command string"
 	default ""
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index f9da419..4bc009c 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -216,6 +216,59 @@ restart:	adr	r0, LC0
 		mov	r10, r6
 #endif
 
+		mov	r5, #0			@ init dtb size to 0
+#ifdef CONFIG_ARM_APPENDED_DTB
+/*
+ *   r0  = delta
+ *   r2  = BSS start
+ *   r3  = BSS end
+ *   r4  = final kernel address
+ *   r5  = appended dtb size (still unknown)
+ *   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
+ *   sp  = stack pointer
+ *
+ * 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	lr, [r6, #0]
+#ifndef __ARMEB__
+		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
+#else
+		ldr	r1, =0xd00dfeed
+#endif
+		cmp	lr, r1
+		bne	dtb_check_done		@ not found
+
+		mov	r8, r6			@ use the appended device tree
+
+		/* Get the dtb's size */
+		ldr	r5, [r6, #4]
+#ifndef __ARMEB__
+		/* convert lr (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
+#endif
+
+		/* preserve 64-bit alignment */
+		add	r5, r5, #7
+		bic	r5, r5, #7
+
+		/* relocate some pointers past the appended dtb */
+		add	r6, r6, r5
+		add	r10, r10, r5
+		add	sp, sp, r5
+dtb_check_done:
+#endif
+
 /*
  * Check to see if we will overwrite ourselves.
  *   r4  = final kernel address
@@ -285,14 +338,16 @@ wont_overwrite:
  *   r2  = BSS start
  *   r3  = BSS end
  *   r4  = kernel execution address
+ *   r5  = appended dtb size (0 if not present)
  *   r7  = architecture ID
  *   r8  = atags pointer
  *   r11 = GOT start
  *   r12 = GOT end
  *   sp  = stack pointer
  */
-		teq	r0, #0
+		orrs	r1, r0, r5
 		beq	not_relocated
+
 		add	r11, r11, r0
 		add	r12, r12, r0
 
@@ -307,12 +362,21 @@ wont_overwrite:
 
 		/*
 		 * Relocate all entries in the GOT table.
+		 * Bump bss entries to _edata + dtb size
 		 */
 1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
-		add	r1, r1, r0		@ table.  This fixes up the
-		str	r1, [r11], #4		@ C references.
+		add	r1, r1, r0		@ This fixes up C references
+		cmp	r1, r2			@ if entry >= bss_start &&
+		cmphs	r3, r1			@       bss_end > entry
+		addhi	r1, r1, r5		@    entry += dtb size
+		str	r1, [r11], #4		@ next entry
 		cmp	r11, r12
 		blo	1b
+
+		/* bump our bss pointers too */
+		add	r2, r2, r5
+		add	r3, r3, r5
+
 #else
 
 		/*
-- 
1.7.4

[PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss

[Nicolas Pitre]

The appended DTB gets relocated with the decompressor code to get out
of the way of the decompressed kernel.  However the .bss section may
be larger than the relocated code and data, therefore overwriting the
DTB.  Let's make sure the relocation takes care of moving zImage so
no conflict with .bss occurs.

Thanks to Tony Lindgren <tony@atomide.com> for figuring out this issue.

While at it, let's clean up the code a bit so that the wont_overwrite
symbol is used while determining if a conflict exists, making the above
change more precise as well as eliminating some ARM/THUMB alternates.

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/boot/compressed/Makefile |    3 +++
 arch/arm/boot/compressed/head.S   |   19 +++++++++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 23aad07..48bead9 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -98,6 +98,9 @@ endif
 ccflags-y := -fpic -fno-builtin
 asflags-y := -Wa,-march=all
 
+# Supply kernel BSS size to the decompressor via a linker symbol.
+KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
+LDFLAGS_vmlinux = --defsym _kernel_bss_size=$(KBSS_SZ)
 # Supply ZRELADDR to the decompressor via a linker symbol.
 ifneq ($(CONFIG_AUTO_ZRELADDR),y)
 LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 4bc009c..a9e97c9 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -248,6 +248,18 @@ restart:	adr	r0, LC0
 
 		mov	r8, r6			@ use the appended device tree
 
+		/*
+		 * Make sure that the DTB doesn't end up in the final
+		 * kernel's .bss area. To do so, we adjust the decompressed
+		 * kernel size to compensate if that .bss size is larger
+		 * than the relocated code.
+		 */
+		ldr	r5, =_kernel_bss_size
+		adr	r1, wont_overwrite
+		sub	r1, r6, r1
+		subs	r1, r5, r1
+		addhi	r9, r9, r1
+
 		/* Get the dtb's size */
 		ldr	r5, [r6, #4]
 #ifndef __ARMEB__
@@ -276,15 +288,14 @@ dtb_check_done:
  *   r10 = end of this image, including  bss/stack/malloc space if non XIP
  * We basically want:
  *   r4 - 16k page directory >= r10 -> OK
- *   r4 + image length <= current position (pc) -> OK
+ *   r4 + image length <= address of wont_overwrite -> OK
  */
 		add	r10, r10, #16384
 		cmp	r4, r10
 		bhs	wont_overwrite
 		add	r10, r4, r9
-   ARM(		cmp	r10, pc		)
- THUMB(		mov	lr, pc		)
- THUMB(		cmp	r10, lr		)
+		adr	r9, wont_overwrite
+		cmp	r10, r9
 		bls	wont_overwrite
 
 /*
-- 
1.7.4

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 02:06:37AM -0400, Nicolas Pitre wrote:
> This is a resend of those patches with fixups after the latest changes
> in mainline.
> 
> [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> This one is new and trivial.
> 
> [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> Mostly John Bonesio's version with some adjustments and cleanups.
> 
> [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> New, simpler alternative to Tony Lindgren's version.

One thing which has been bugging me for some time is that the DT stuff
completely overrides the ATAGs.  This is wrong with solutions like this.

We have a set of perfectly good boot loaders which provide correct
information to the kernel via ATAGs.  If we start moving everything
over to DT, then we run into a problem because the ATAGs are ignored -
stuff such as the RAM size and command line passed from the boot loader
will be entirely ignored, instead these having to be encoded into the
kernel at build time.

This is clearly not the right thing to be doing, and this will _prevent_
certain platforms being converted over (I'm going to refuse to convert
the ones which I have an interest in with this restriction in place.)

This restriction needs to be fixed without forcing people to rewrite
their boot loaders.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Shawn Guo]

On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote:
> On Sun, Jun 12, 2011 at 02:06:37AM -0400, Nicolas Pitre wrote:
> > This is a resend of those patches with fixups after the latest changes
> > in mainline.
> > 
> > [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> > This one is new and trivial.
> > 
> > [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> > Mostly John Bonesio's version with some adjustments and cleanups.
> > 
> > [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> > New, simpler alternative to Tony Lindgren's version.
> 
> One thing which has been bugging me for some time is that the DT stuff
> completely overrides the ATAGs.  This is wrong with solutions like this.
> 
> We have a set of perfectly good boot loaders which provide correct
> information to the kernel via ATAGs.  If we start moving everything
> over to DT, then we run into a problem because the ATAGs are ignored -
> stuff such as the RAM size and command line passed from the boot loader
> will be entirely ignored, instead these having to be encoded into the
> kernel at build time.
> 
What u-boot does right now is replacing the parameters in dtb with
its for those it's interested in, for example command line is one,
before it passes dtb to kernel.

-- 
Regards,
Shawn

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 04:34:15PM +0800, Shawn Guo wrote:
> On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote:
> > On Sun, Jun 12, 2011 at 02:06:37AM -0400, Nicolas Pitre wrote:
> > > This is a resend of those patches with fixups after the latest changes
> > > in mainline.
> > > 
> > > [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> > > This one is new and trivial.
> > > 
> > > [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> > > Mostly John Bonesio's version with some adjustments and cleanups.
> > > 
> > > [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> > > New, simpler alternative to Tony Lindgren's version.
> > 
> > One thing which has been bugging me for some time is that the DT stuff
> > completely overrides the ATAGs.  This is wrong with solutions like this.
> > 
> > We have a set of perfectly good boot loaders which provide correct
> > information to the kernel via ATAGs.  If we start moving everything
> > over to DT, then we run into a problem because the ATAGs are ignored -
> > stuff such as the RAM size and command line passed from the boot loader
> > will be entirely ignored, instead these having to be encoded into the
> > kernel at build time.
> > 
> What u-boot does right now is replacing the parameters in dtb with
> its for those it's interested in, for example command line is one,
> before it passes dtb to kernel.

What if your platform doesn't use uboot?

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Shawn Guo]

On Sun, Jun 12, 2011 at 10:21:31AM +0100, Russell King - ARM Linux wrote:
> On Sun, Jun 12, 2011 at 04:34:15PM +0800, Shawn Guo wrote:
> > On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote:
> > > On Sun, Jun 12, 2011 at 02:06:37AM -0400, Nicolas Pitre wrote:
> > > > This is a resend of those patches with fixups after the latest changes
> > > > in mainline.
> > > > 
> > > > [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> > > > This one is new and trivial.
> > > > 
> > > > [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> > > > Mostly John Bonesio's version with some adjustments and cleanups.
> > > > 
> > > > [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> > > > New, simpler alternative to Tony Lindgren's version.
> > > 
> > > One thing which has been bugging me for some time is that the DT stuff
> > > completely overrides the ATAGs.  This is wrong with solutions like this.
> > > 
> > > We have a set of perfectly good boot loaders which provide correct
> > > information to the kernel via ATAGs.  If we start moving everything
> > > over to DT, then we run into a problem because the ATAGs are ignored -
> > > stuff such as the RAM size and command line passed from the boot loader
> > > will be entirely ignored, instead these having to be encoded into the
> > > kernel at build time.
> > > 
> > What u-boot does right now is replacing the parameters in dtb with
> > its for those it's interested in, for example command line is one,
> > before it passes dtb to kernel.
> 
> What if your platform doesn't use uboot?
> 
Add dtb parsing support with the help from libfdt, I guess.  It is
some amount of work, but it's not a rewrite of bootloader, IMHO.

-- 
Regards,
Shawn

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 05:38:23PM +0800, Shawn Guo wrote:
> On Sun, Jun 12, 2011 at 10:21:31AM +0100, Russell King - ARM Linux wrote:
> > What if your platform doesn't use uboot?
> 
> Add dtb parsing support with the help from libfdt, I guess.  It is
> some amount of work, but it's not a rewrite of bootloader, IMHO.

I guess you're suggesting that this wrapper uses libfdt to merge
the ATAGs with the DT info?

I think this is getting messy - and I think that we've made a design
error with the boot process in thinking that we'll have either ATAGs
_or_ DT.  While we can continue to stuff the early kernel parts like
decompressors and such like with this stuff, it's getting painful and
over complicated.

It would have been far better to have passed both the ATAGs _and_ DT
to the kernel directly and let it sort them out without having to
teach several layers about DT parsing.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 06:42:16PM +0800, Shawn Guo wrote:
> On Sun, Jun 12, 2011 at 10:52:17AM +0100, Russell King - ARM Linux wrote:
> > On Sun, Jun 12, 2011 at 05:38:23PM +0800, Shawn Guo wrote:
> > > On Sun, Jun 12, 2011 at 10:21:31AM +0100, Russell King - ARM Linux wrote:
> > > > What if your platform doesn't use uboot?
> > > 
> > > Add dtb parsing support with the help from libfdt, I guess.  It is
> > > some amount of work, but it's not a rewrite of bootloader, IMHO.
> > 
> > I guess you're suggesting that this wrapper uses libfdt to merge
> > the ATAGs with the DT info?
> > 
> No, ATAGs does not play at all in this case.  For u-boot example
> again, if it boots a dt kernel, dtb will be parsed to get cmdline
> node overwritten as bootargs env value, and then it boots the dt
> kernel with this updated dtb.

You've missed my point entirely.  What you're saying is that we have to
re-build and replace the boot loader in order to pass a command line into
a kernel using the DT wrapper.

I'm saying that you shouldn't have to, and the kernel should accept the
memory size and command line from the ATAGs _in addition_ to the
appended DT blob, and the ATAGs in that case should take precidence.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Shawn Guo]

On Sun, Jun 12, 2011 at 10:52:17AM +0100, Russell King - ARM Linux wrote:
> On Sun, Jun 12, 2011 at 05:38:23PM +0800, Shawn Guo wrote:
> > On Sun, Jun 12, 2011 at 10:21:31AM +0100, Russell King - ARM Linux wrote:
> > > What if your platform doesn't use uboot?
> > 
> > Add dtb parsing support with the help from libfdt, I guess.  It is
> > some amount of work, but it's not a rewrite of bootloader, IMHO.
> 
> I guess you're suggesting that this wrapper uses libfdt to merge
> the ATAGs with the DT info?
> 
No, ATAGs does not play at all in this case.  For u-boot example
again, if it boots a dt kernel, dtb will be parsed to get cmdline
node overwritten as bootargs env value, and then it boots the dt
kernel with this updated dtb.

> I think this is getting messy - and I think that we've made a design
> error with the boot process in thinking that we'll have either ATAGs
> _or_ DT.  While we can continue to stuff the early kernel parts like
> decompressors and such like with this stuff, it's getting painful and
> over complicated.
> 
> It would have been far better to have passed both the ATAGs _and_ DT
> to the kernel directly and let it sort them out without having to
> teach several layers about DT parsing.
> 

-- 
Regards,
Shawn

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Petr Štetiar]

Shawn Guo <shawn.guo@freescale.com> [2011-06-12 16:34:15]:

> On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote:
> > 
> > One thing which has been bugging me for some time is that the DT stuff
> > completely overrides the ATAGs.  This is wrong with solutions like this.
> > 
> > We have a set of perfectly good boot loaders which provide correct
> > information to the kernel via ATAGs.  If we start moving everything
> > over to DT, then we run into a problem because the ATAGs are ignored -
> > stuff such as the RAM size and command line passed from the boot loader
> > will be entirely ignored, instead these having to be encoded into the
> > kernel at build time.
> > 
> What u-boot does right now is replacing the parameters in dtb with
> its for those it's interested in, for example command line is one,
> before it passes dtb to kernel.

If I understand it all correctly, there must be some 'legacy bootloader
support' added to the kernel, layer which would convert the ATAGs to the DT.
Or something like that.

Please note, that there are some boards out there, which use some kind of
proprietary bootloaders, so you can't update or change them easily. You either
don't have source code, you're limited by the available space (2KB eeprom) so
you can't add much more things into that bootloader or you're limited by the
size of the MBR (yes, even such bootloaders exists).

Ok, everything could be reverse engineered, EEPROM could be exchanged for the
bigger one etc., second stage bootloader could be written for the MBR one, but
it's not as easy as you think, at least not as "Add dtb parsing support with
the help from libfdt. It is some amount of work, but it's not a rewrite of
bootloader, IMHO."

-- ynezz

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 01:22:19PM +0200, Petr ?tetiar wrote:
> Shawn Guo <shawn.guo@freescale.com> [2011-06-12 16:34:15]:
> 
> > On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote:
> > > 
> > > One thing which has been bugging me for some time is that the DT stuff
> > > completely overrides the ATAGs.  This is wrong with solutions like this.
> > > 
> > > We have a set of perfectly good boot loaders which provide correct
> > > information to the kernel via ATAGs.  If we start moving everything
> > > over to DT, then we run into a problem because the ATAGs are ignored -
> > > stuff such as the RAM size and command line passed from the boot loader
> > > will be entirely ignored, instead these having to be encoded into the
> > > kernel at build time.
> > > 
> > What u-boot does right now is replacing the parameters in dtb with
> > its for those it's interested in, for example command line is one,
> > before it passes dtb to kernel.
> 
> If I understand it all correctly, there must be some 'legacy bootloader
> support' added to the kernel, layer which would convert the ATAGs to the DT.
> Or something like that.
> 
> Please note, that there are some boards out there, which use some kind of
> proprietary bootloaders, so you can't update or change them easily. You either
> don't have source code, you're limited by the available space (2KB eeprom) so
> you can't add much more things into that bootloader or you're limited by the
> size of the MBR (yes, even such bootloaders exists).

Exactly my point - I have quite a number of platforms here which will
never be able to have a boot loader capable of modifying a DT blob for
the kernel.

One of the points of Nicolas' patch set is to allow existing boot loaders
to boot kernels where the hardware description is contained in a DT blob
encapsulated with the kernel.  That's great but the way things are currently
setup, it means that the boot loader does nothing more than loading and
executing - and we lose the _existing_ flexibility for the boot loader to
pass platform specific information to the kernel.

So, what I'm considering to do is update the boot protocol such that the
base address of the DT blob is provided in r3, separately from the ATAG
pointer in r2.

This means that boot loaders can provide a DT blob (r2 = 0 r3 => DT) or
they can provide ATAGs (r2 => atag, r3 = indeterminant).  We can then
cater for the situation where we have an ATAG boot loader, but a kernel
with an appended DT description (r2 => atag, r3 => DT) and have the ATAG
information override the DT for things like memory layout and the command
line string.

This is the only sensible way of maintaining compatibility with the
existing boot protocol, which is an absolute _must_ if we are going to
convert some of the existing SoCs (like PXA) to DT and get rid of the
legacy static descriptions of the on-board devices (which is the whole
point we're going down this path.)

It's either that or we'll have to refuse to convert existing SoCs to
DT to maintain compatibility for existing boards - which makes DT
support totally pointless.

Let me give a situation where this matters: you have a boot loader which
loads a kernel from disk and executes it.  You have 256MB of RAM fitted
to the machine.  You replace this kernel with a DT-enabled kernel which
has the DT blob appended to the kernel.  The DT blob says you have 256MB
of RAM.

You remove a 128MB DIMM because its gone faulty.  You try to boot.  The
boot loader provides the kernel with an ATAG telling it that there's
128MB of RAM.  However, the kernel ignores the ATAGs and instead looks
at the encapsulated DT information which tells it that there's 256MB
of RAM.  Your kernel OOPSes.  You reboot, and try passing a command
line argument of 'mem=128M'. The kernel again ignores this because its
an ATAG.

The result: you can't boot the platform.

Another case: your flash has become corrupted, and the kernel won't mount
the flash based rootfs.  You want to boot from a root-NFS export to sort
the problem out, but the kernel ignores your new command line telling it
to do so.

The result: you can't boot the platform.

Another case: you have a Thecus N2100 acting as a server, with a pair
of drives setup as raid 1.  You reboot it one day and it refuses to build
the raid 1 rootfs, and so panics at boot.  You want to change the kernel
command line so that it mounts root from somewhere else, but because
you're using a DT based kernel, it ignores you.

The result: you can't boot the platform.

This behaviour from a DT based kernel is just not acceptable, and it's
also not acceptable to expect platforms to update their boot loaders to
cope with DT.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Arnd Bergmann]

On Sunday 12 June 2011 13:58:20 Russell King - ARM Linux wrote:
> Exactly my point - I have quite a number of platforms here which will
> never be able to have a boot loader capable of modifying a DT blob for
> the kernel.
> 
> One of the points of Nicolas' patch set is to allow existing boot loaders
> to boot kernels where the hardware description is contained in a DT blob
> encapsulated with the kernel.  That's great but the way things are currently
> setup, it means that the boot loader does nothing more than loading and
> executing - and we lose the existing flexibility for the boot loader to
> pass platform specific information to the kernel.
> 
> So, what I'm considering to do is update the boot protocol such that the
> base address of the DT blob is provided in r3, separately from the ATAG
> pointer in r2.
>
> This means that boot loaders can provide a DT blob (r2 = 0 r3 => DT) or
> they can provide ATAGs (r2 => atag, r3 = indeterminant).  We can then
> cater for the situation where we have an ATAG boot loader, but a kernel
> with an appended DT description (r2 => atag, r3 => DT) and have the ATAG
> information override the DT for things like memory layout and the command
> line string.

But when you have both atag and DT and the atag overrides the DT, that
means we have incorrect information in the DT, and code might later
rely on that information.

IMHO when we allow passing a DT to a kernel while booting from an
existing boot loader that only knows about atag, the code that loads
the DT should be responsible for updating the DT with the atag information,
not pass two conflicting sets of data into the actual kernel.

> Let me give a situation where this matters: you have a boot loader which
> loads a kernel from disk and executes it.  You have 256MB of RAM fitted
> to the machine.  You replace this kernel with a DT-enabled kernel which
> has the DT blob appended to the kernel.  The DT blob says you have 256MB
> of RAM.
> 
> You remove a 128MB DIMM because its gone faulty.  You try to boot.  The
> boot loader provides the kernel with an ATAG telling it that there's
> 128MB of RAM.  However, the kernel ignores the ATAGs and instead looks
> at the encapsulated DT information which tells it that there's 256MB
> of RAM.  Your kernel OOPSes.  You reboot, and try passing a command
> line argument of 'mem=128M'. The kernel again ignores this because its
> an ATAG.
> 
> The result: you can't boot the platform.
>
> Another case: your flash has become corrupted, and the kernel won't mount
> the flash based rootfs.  You want to boot from a root-NFS export to sort
> the problem out, but the kernel ignores your new command line telling it
> to do so.
> 
> The result: you can't boot the platform.
> 
> Another case: you have a Thecus N2100 acting as a server, with a pair
> of drives setup as raid 1.  You reboot it one day and it refuses to build
> the raid 1 rootfs, and so panics at boot.  You want to change the kernel
> command line so that it mounts root from somewhere else, but because
> you're using a DT based kernel, it ignores you.
> 
> The result: you can't boot the platform.

So we need to at least the command line and the memory layout to be adapted
in the in-memory DT, from the atags. Any other tags?

	Arnd

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 04:15:23PM +0200, Arnd Bergmann wrote:
> On Sunday 12 June 2011 13:58:20 Russell King - ARM Linux wrote:
> > Exactly my point - I have quite a number of platforms here which will
> > never be able to have a boot loader capable of modifying a DT blob for
> > the kernel.
> > 
> > One of the points of Nicolas' patch set is to allow existing boot loaders
> > to boot kernels where the hardware description is contained in a DT blob
> > encapsulated with the kernel.  That's great but the way things are currently
> > setup, it means that the boot loader does nothing more than loading and
> > executing - and we lose the existing flexibility for the boot loader to
> > pass platform specific information to the kernel.
> > 
> > So, what I'm considering to do is update the boot protocol such that the
> > base address of the DT blob is provided in r3, separately from the ATAG
> > pointer in r2.
> >
> > This means that boot loaders can provide a DT blob (r2 = 0 r3 => DT) or
> > they can provide ATAGs (r2 => atag, r3 = indeterminant).  We can then
> > cater for the situation where we have an ATAG boot loader, but a kernel
> > with an appended DT description (r2 => atag, r3 => DT) and have the ATAG
> > information override the DT for things like memory layout and the command
> > line string.
> 
> But when you have both atag and DT and the atag overrides the DT, that
> means we have incorrect information in the DT, and code might later
> rely on that information.

I don't think you're considering real-world usage scenarios, but instead
concentrating on the use issues.  If you only do that you're boxing
yourself into a corner and will cause a world of pain for folk who would
just like the kernel to be usable.

The information in ATAGs which will either never be in DT or which should
override what is in DT is:

- memory parameters
- command line
- initrd location
- system serial number
- system revision number

Things like the serial number will _never_ be in DT (do you want to build
a DT image unique to each platform?)  initrd location will never be in
DT either (it depends where the boot loader placed it.) ... and so forth.

This is the kind of information which should override whatever is in
DT because the DT contained information could well be wrong or outdated,
and its not the kind of information that anything other than core code
should be dealing with in any case.

> IMHO when we allow passing a DT to a kernel while booting from an
> existing boot loader that only knows about atag, the code that loads
> the DT should be responsible for updating the DT with the atag information,
> not pass two conflicting sets of data into the actual kernel.

So, that means Nicolas' patches which allow a DT to be appended to the
image need to have DT and ATAG parsing in them to update the DT stuff
with the ATAG information.

> > Let me give a situation where this matters: you have a boot loader which
> > loads a kernel from disk and executes it.  You have 256MB of RAM fitted
> > to the machine.  You replace this kernel with a DT-enabled kernel which
> > has the DT blob appended to the kernel.  The DT blob says you have 256MB
> > of RAM.
> > 
> > You remove a 128MB DIMM because its gone faulty.  You try to boot.  The
> > boot loader provides the kernel with an ATAG telling it that there's
> > 128MB of RAM.  However, the kernel ignores the ATAGs and instead looks
> > at the encapsulated DT information which tells it that there's 256MB
> > of RAM.  Your kernel OOPSes.  You reboot, and try passing a command
> > line argument of 'mem=128M'. The kernel again ignores this because its
> > an ATAG.
> > 
> > The result: you can't boot the platform.
> >
> > Another case: your flash has become corrupted, and the kernel won't mount
> > the flash based rootfs.  You want to boot from a root-NFS export to sort
> > the problem out, but the kernel ignores your new command line telling it
> > to do so.
> > 
> > The result: you can't boot the platform.
> > 
> > Another case: you have a Thecus N2100 acting as a server, with a pair
> > of drives setup as raid 1.  You reboot it one day and it refuses to build
> > the raid 1 rootfs, and so panics at boot.  You want to change the kernel
> > command line so that it mounts root from somewhere else, but because
> > you're using a DT based kernel, it ignores you.
> > 
> > The result: you can't boot the platform.
> 
> So we need to at least the command line and the memory layout to be adapted
> in the in-memory DT, from the atags. Any other tags?

See above.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Grant Likely]

On Sun, Jun 12, 2011 at 04:15:23PM +0200, Arnd Bergmann wrote:
> On Sunday 12 June 2011 13:58:20 Russell King - ARM Linux wrote:
> > Exactly my point - I have quite a number of platforms here which will
> > never be able to have a boot loader capable of modifying a DT blob for
> > the kernel.
> > 
> > One of the points of Nicolas' patch set is to allow existing boot loaders
> > to boot kernels where the hardware description is contained in a DT blob
> > encapsulated with the kernel.  That's great but the way things are currently
> > setup, it means that the boot loader does nothing more than loading and
> > executing - and we lose the existing flexibility for the boot loader to
> > pass platform specific information to the kernel.
> > 
> > So, what I'm considering to do is update the boot protocol such that the
> > base address of the DT blob is provided in r3, separately from the ATAG
> > pointer in r2.
> >
> > This means that boot loaders can provide a DT blob (r2 = 0 r3 => DT) or
> > they can provide ATAGs (r2 => atag, r3 = indeterminant).  We can then
> > cater for the situation where we have an ATAG boot loader, but a kernel
> > with an appended DT description (r2 => atag, r3 => DT) and have the ATAG
> > information override the DT for things like memory layout and the command
> > line string.
> 
> But when you have both atag and DT and the atag overrides the DT, that
> means we have incorrect information in the DT, and code might later
> rely on that information.
> 
> IMHO when we allow passing a DT to a kernel while booting from an
> existing boot loader that only knows about atag, the code that loads
> the DT should be responsible for updating the DT with the atag information,
> not pass two conflicting sets of data into the actual kernel.

I completely agree here.  I /started/ from the position that ATAGs and
DTB would coexist, and after extensive debate[1] my opinion turned around
to it should be one or the other.  Otherwise there are all kinds of
questions about accuracy of the information and which takes
precedence.

[1]http://lists.ozlabs.org/pipermail/devicetree-discuss/2010-May/002130.html

> > Let me give a situation where this matters: you have a boot loader which
> > loads a kernel from disk and executes it.  You have 256MB of RAM fitted
> > to the machine.  You replace this kernel with a DT-enabled kernel which
> > has the DT blob appended to the kernel.  The DT blob says you have 256MB
> > of RAM.
> > 
> > You remove a 128MB DIMM because its gone faulty.  You try to boot.  The
> > boot loader provides the kernel with an ATAG telling it that there's
> > 128MB of RAM.  However, the kernel ignores the ATAGs and instead looks
> > at the encapsulated DT information which tells it that there's 256MB
> > of RAM.  Your kernel OOPSes.  You reboot, and try passing a command
> > line argument of 'mem=128M'. The kernel again ignores this because its
> > an ATAG.
> > 
> > The result: you can't boot the platform.
> >
> > Another case: your flash has become corrupted, and the kernel won't mount
> > the flash based rootfs.  You want to boot from a root-NFS export to sort
> > the problem out, but the kernel ignores your new command line telling it
> > to do so.
> > 
> > The result: you can't boot the platform.
> > 
> > Another case: you have a Thecus N2100 acting as a server, with a pair
> > of drives setup as raid 1.  You reboot it one day and it refuses to build
> > the raid 1 rootfs, and so panics at boot.  You want to change the kernel
> > command line so that it mounts root from somewhere else, but because
> > you're using a DT based kernel, it ignores you.
> > 
> > The result: you can't boot the platform.
> 
> So we need to at least the command line and the memory layout to be adapted
> in the in-memory DT, from the atags. Any other tags?

initrd location.

There already exists a prototype patch that loads some atag data into
the dt, and it is exactly the mechanism used by powerpc to boot kernels
on non-dt firmware.

g.

[PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary

[Grant Likely]

On Sun, Jun 12, 2011 at 02:06:39AM -0400, Nicolas Pitre wrote:
> From: John Bonesio <bones@secretlab.ca>
> 
> 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>
> [nico: adjusted to latest zImage changes plus additional cleanups]
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> ---
>  arch/arm/Kconfig                |    8 ++++
>  arch/arm/boot/compressed/head.S |   70 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 75 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 9adc278..66b7d1e 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1727,6 +1727,14 @@ config ZBOOT_ROM_MMCIF
>  	  which in turn loads the rest the kernel image to RAM using the
>  	  MMCIF hardware block.
>  
> +config ARM_APPENDED_DTB
> +	bool "Use appended device tree blob to zImage"
> +	depends on OF && !ZBOOT_ROM
> +	help
> +	  With this option, the boot code will look for a device tree binary
> +	  (dtb) appended to zImage
> +	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> +
>  config CMDLINE
>  	string "Default kernel command string"
>  	default ""
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index f9da419..4bc009c 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -216,6 +216,59 @@ restart:	adr	r0, LC0
>  		mov	r10, r6
>  #endif
>  
> +		mov	r5, #0			@ init dtb size to 0
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +/*
> + *   r0  = delta
> + *   r2  = BSS start
> + *   r3  = BSS end
> + *   r4  = final kernel address
> + *   r5  = appended dtb size (still unknown)
> + *   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
> + *   sp  = stack pointer
> + *
> + * 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	lr, [r6, #0]
> +#ifndef __ARMEB__
> +		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
> +#else
> +		ldr	r1, =0xd00dfeed
> +#endif
> +		cmp	lr, r1
> +		bne	dtb_check_done		@ not found
> +
> +		mov	r8, r6			@ use the appended device tree
> +
> +		/* Get the dtb's size */
> +		ldr	r5, [r6, #4]
> +#ifndef __ARMEB__
> +		/* convert lr (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
> +#endif
> +
> +		/* preserve 64-bit alignment */
> +		add	r5, r5, #7
> +		bic	r5, r5, #7
> +
> +		/* relocate some pointers past the appended dtb */
> +		add	r6, r6, r5
> +		add	r10, r10, r5
> +		add	sp, sp, r5
> +dtb_check_done:
> +#endif
> +
>  /*
>   * Check to see if we will overwrite ourselves.
>   *   r4  = final kernel address
> @@ -285,14 +338,16 @@ wont_overwrite:
>   *   r2  = BSS start
>   *   r3  = BSS end
>   *   r4  = kernel execution address
> + *   r5  = appended dtb size (0 if not present)
>   *   r7  = architecture ID
>   *   r8  = atags pointer
>   *   r11 = GOT start
>   *   r12 = GOT end
>   *   sp  = stack pointer
>   */
> -		teq	r0, #0
> +		orrs	r1, r0, r5
>  		beq	not_relocated
> +
>  		add	r11, r11, r0
>  		add	r12, r12, r0
>  
> @@ -307,12 +362,21 @@ wont_overwrite:
>  
>  		/*
>  		 * Relocate all entries in the GOT table.
> +		 * Bump bss entries to _edata + dtb size
>  		 */
>  1:		ldr	r1, [r11, #0]		@ relocate entries in the GOT
> -		add	r1, r1, r0		@ table.  This fixes up the
> -		str	r1, [r11], #4		@ C references.
> +		add	r1, r1, r0		@ This fixes up C references
> +		cmp	r1, r2			@ if entry >= bss_start &&
> +		cmphs	r3, r1			@       bss_end > entry
> +		addhi	r1, r1, r5		@    entry += dtb size
> +		str	r1, [r11], #4		@ next entry
>  		cmp	r11, r12
>  		blo	1b
> +
> +		/* bump our bss pointers too */
> +		add	r2, r2, r5
> +		add	r3, r3, r5
> +
>  #else
>  
>  		/*
> -- 
> 1.7.4
> 

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Arnd Bergmann]

On Sunday 12 June 2011 16:34:28 Russell King - ARM Linux wrote:
> I don't think you're considering real-world usage scenarios, but instead
> concentrating on the use issues.  If you only do that you're boxing
> yourself into a corner and will cause a world of pain for folk who would
> just like the kernel to be usable.
> 
> The information in ATAGs which will either never be in DT or which should
> override what is in DT is:
> 
> - memory parameters
> - command line
> - initrd location
> - system serial number
> - system revision number

With a device-tree aware boot loader, this is exactly the kind of
information that should be put in the device tree by the boot loader,
along with other parts like local-mac-address and actual presence
of optional devices where the kernel might not have a generic way
of probing whether they are there.

> This is the kind of information which should override whatever is in
> DT because the DT contained information could well be wrong or outdated,
> and its not the kind of information that anything other than core code
> should be dealing with in any case.

Of course it should override the device tree, I'm not arguing that.
All I'm saying is that we don't need to special-case this or support
both formats once the kernel is there as long as we move the information
into appropriate DT-representation of the same data.

The attributes you are looking for are for the information you listed:

/memory/*
/chosen/bootargs
/chosen/linux,initrd-{start,end}
/serial-number
/compatible

PowerPC was facing the exact same problem, see arch/powerpc/boot/main.c
for the solution used there. The way that the boot entry point on powerpc
works is to have a boot-protocol specific entry from which all the
necessary information is taken and then put into a flattened device tree
either built from scratch (in case of real OF) or patched into a prebuilt
device tree. From there we pass the flattened device tree with all the
correct information into the actual boot stage. In some cases, the entry
points are identical and we just jump back to the start with the updated
information.

Having a complete and correct device tree is essential for instance to make
kexec work, because the new kernel will only have the information from
the device tree available but not have access to atags, uefi of open
firmware.

It may be a good idea to preserve the atags in a new property in the
device tree, so we are able to inspect them later.

	Arnd

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 08:57:51AM -0600, Grant Likely wrote:
> On Sun, Jun 12, 2011 at 04:15:23PM +0200, Arnd Bergmann wrote:
> > But when you have both atag and DT and the atag overrides the DT, that
> > means we have incorrect information in the DT, and code might later
> > rely on that information.
> > 
> > IMHO when we allow passing a DT to a kernel while booting from an
> > existing boot loader that only knows about atag, the code that loads
> > the DT should be responsible for updating the DT with the atag information,
> > not pass two conflicting sets of data into the actual kernel.
> 
> I completely agree here.  I /started/ from the position that ATAGs and
> DTB would coexist, and after extensive debate[1] my opinion turned around
> to it should be one or the other.  Otherwise there are all kinds of
> questions about accuracy of the information and which takes
> precedence.

And we've ended up with a fucked up situation which is extremely
fragile, and actually makes me _NOT_ want to convert any existing
platforms to use DT in the least.

This I view as a fundamental blocker which needs addressing before
anyone can make use of DT on ARM.

DT is _not_ the authoritive source of information on systems with
ATAGs.

Imagine this situation: you have your PC.  It provides memory information
through the E820 interface.  You convert your kernel to use DT and it
only uses the information passed from the DT blob, which it loaded as
part of your kernel off disk.

However, your RAM size has changed.  Should the kernel continue to believe
the memory information found in the encapsulated DT blob, or should it
continue to get it from the E820 interface?

It's precisely the same problem here.  The E820 interface _has_ to take
precedence because that is the _authoritive_ source of information.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 05:01:22PM +0200, Arnd Bergmann wrote:
> Of course it should override the device tree, I'm not arguing that.
> All I'm saying is that we don't need to special-case this or support
> both formats once the kernel is there as long as we move the information
> into appropriate DT-representation of the same data.

You're arguing for it to be parsed by whatever fragment of assembly code
we have to place around the zImage to merge it into DT.

And yes, it's going to have to be assembly code because we don't have a
C environment at that point which can run in a relocatable way.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:

> One of the points of Nicolas' patch set is to allow existing boot loaders
> to boot kernels where the hardware description is contained in a DT blob
> encapsulated with the kernel.  That's great but the way things are currently
> setup, it means that the boot loader does nothing more than loading and
> executing - and we lose the _existing_ flexibility for the boot loader to
> pass platform specific information to the kernel.

My stance always has been to preserve ATAG support for legacy platforms.  
I think we have enough work already trying to convert modern platforms 
to DT without bothering with the older ones with non-updatable 
bootloaders.  This ability to append a DTB to the kernel is simply a 
convenience to let people use DT with non DT aware bootloaders, or even 
to override the DTB passed by the bootloader which is unavoidably going 
to be broken in some cases.  Of course the restriction in that case is 
that you'll need to recreate that appended DTB whenever you wish to 
change a parameter.  I don't consider this to be the main mode of 
operation though.

> So, what I'm considering to do is update the boot protocol such that the
> base address of the DT blob is provided in r3, separately from the ATAG
> pointer in r2.
> 
> This means that boot loaders can provide a DT blob (r2 = 0 r3 => DT) or
> they can provide ATAGs (r2 => atag, r3 = indeterminant).  We can then
> cater for the situation where we have an ATAG boot loader, but a kernel
> with an appended DT description (r2 => atag, r3 => DT) and have the ATAG
> information override the DT for things like memory layout and the command
> line string.

No, please let's not go there.  Having competing sources of information 
is the best recipe to madness.  Which one is authoritative?  What if we 
need to have the override going in the other direction?  We should be 
fully with ATAGS, or fully with DT, not a mixture of both.

> This is the only sensible way of maintaining compatibility with the
> existing boot protocol, which is an absolute _must_ if we are going to
> convert some of the existing SoCs (like PXA) to DT and get rid of the
> legacy static descriptions of the on-board devices (which is the whole
> point we're going down this path.)

All the newer PXAs are using U-Boot which is now DT aware.  If a legacy 
bootloader must be used, then it is always possible to add a shim to 
zImage translating selected ATAGs into DT records.  A patch for doing 
just that exists already and was posted on the list by Grant.  However I 
prefer if it is used only when absolutely needed and not by default 
otherwise we lose all possible leverage on bootloader authors to move to 
DT (the fact that the DT append patch still doesn't let you change the 
cmdline live is also part of that leverage).

> It's either that or we'll have to refuse to convert existing SoCs to
> DT to maintain compatibility for existing boards - which makes DT
> support totally pointless.

It is not pointless.  Never it was in the cards to convert old platforms 
first.  The most problematic platforms responsible for the high volume 
of changes that has gone in mainline lately are the modern ones, not PXA 
based ones.  So DT is going to be most effective for the new stuff to 
come.  And realistically that's where we might get developers attention, 
not with those platforms being end-of-lived by their manufacturers.

> Let me give a situation where this matters: you have a boot loader which
> loads a kernel from disk and executes it.  You have 256MB of RAM fitted
> to the machine.  You replace this kernel with a DT-enabled kernel which
> has the DT blob appended to the kernel.  The DT blob says you have 256MB
> of RAM.
> 
> You remove a 128MB DIMM because its gone faulty.  You try to boot.  The
> boot loader provides the kernel with an ATAG telling it that there's
> 128MB of RAM.  However, the kernel ignores the ATAGs and instead looks
> at the encapsulated DT information which tells it that there's 256MB
> of RAM.  Your kernel OOPSes.  You reboot, and try passing a command
> line argument of 'mem=128M'. The kernel again ignores this because its
> an ATAG.
> 
> The result: you can't boot the platform.

I don't think the DT append patch is suitable for such a usage scenario, 
unless you are ready to swap the appended DTB with another one in that 
case.  Or you simply can just drop the appended DTB in which case the 
kernel will use the ATAGs instead.

If you really want to use DT on that platform and still benefit from the 
ATAG information passed by the bootloader, then you could use the 
translation shim that exists.  That, of course, is assuming that you do 
have DT support for that platform in the kernel which, like I said, is 
unlikely to happen for old platforms unless someone really enthusiastic 
invest some time in the required work.

> Another case: you have a Thecus N2100 acting as a server, with a pair
> of drives setup as raid 1.  You reboot it one day and it refuses to build
> the raid 1 rootfs, and so panics at boot.  You want to change the kernel
> command line so that it mounts root from somewhere else, but because
> you're using a DT based kernel, it ignores you.

My answer: if it isn't broken, don't fix it.  In other words, I have not 
seen huge development activities around the N2100 for quite a while.  
It is not responsible for a significant increase of kernel code anymore, 
if at all. So there is no point moving it to DT especially in a 
production setup.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:

> On Sun, Jun 12, 2011 at 05:01:22PM +0200, Arnd Bergmann wrote:
> > Of course it should override the device tree, I'm not arguing that.
> > All I'm saying is that we don't need to special-case this or support
> > both formats once the kernel is there as long as we move the information
> > into appropriate DT-representation of the same data.
> 
> You're arguing for it to be parsed by whatever fragment of assembly code
> we have to place around the zImage to merge it into DT.
> 
> And yes, it's going to have to be assembly code because we don't have a
> C environment at that point which can run in a relocatable way.

The prototype patch translating ATAGs into DT defer that processing to C 
code.  Doing that in assembly is of course madness.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:

> On Sun, Jun 12, 2011 at 08:57:51AM -0600, Grant Likely wrote:
> > On Sun, Jun 12, 2011 at 04:15:23PM +0200, Arnd Bergmann wrote:
> > > But when you have both atag and DT and the atag overrides the DT, that
> > > means we have incorrect information in the DT, and code might later
> > > rely on that information.
> > > 
> > > IMHO when we allow passing a DT to a kernel while booting from an
> > > existing boot loader that only knows about atag, the code that loads
> > > the DT should be responsible for updating the DT with the atag information,
> > > not pass two conflicting sets of data into the actual kernel.
> > 
> > I completely agree here.  I /started/ from the position that ATAGs and
> > DTB would coexist, and after extensive debate[1] my opinion turned around
> > to it should be one or the other.  Otherwise there are all kinds of
> > questions about accuracy of the information and which takes
> > precedence.
> 
> And we've ended up with a fucked up situation which is extremely
> fragile, and actually makes me _NOT_ want to convert any existing
> platforms to use DT in the least.

Agreed.  I don't think that anything older than OMAP2 is worth 
converting to DT.  The return on the investment is simply not worth it, 
other than for experimental purposes.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 11:47:59AM -0400, Nicolas Pitre wrote:
> On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:
> > And we've ended up with a fucked up situation which is extremely
> > fragile, and actually makes me _NOT_ want to convert any existing
> > platforms to use DT in the least.
> 
> Agreed.  I don't think that anything older than OMAP2 is worth 
> converting to DT.  The return on the investment is simply not worth it, 
> other than for experimental purposes.

I think you haven't appreciated the situation - let's take PXA as an
example.  PXA has been around for years, and IP in the latest silicon
is present in many of the older silicons too.

There's two issues here:

1. If we port existing drivers over to use DT as a means to shrink the
size of the kernel, we need _all_ PXA using platforms to use DT.

2. If we continue having board support for PXA submitted, we want it to
use DT support.

The result will be a mess of some bits of PXA using DT, other bits using
statically declared stuff.  It may get to the point where on some PXA
platforms DT is used to describe some of the system, and on a different
PXA platform, it describes some other but needs some static stuff.

I don't see this as a sustainable way forward.  If we're going to move a
particular SoC over to DT, we need to move the entire SoC over.  We can't
do this half-heartedly.

And that means we _must_ deal with accepting ATAGs from existing boot
loaders, with that information taking precidence over the DT blob
supplied with the kernel.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:

> On Sun, Jun 12, 2011 at 11:47:59AM -0400, Nicolas Pitre wrote:
> > On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:
> > > And we've ended up with a fucked up situation which is extremely
> > > fragile, and actually makes me _NOT_ want to convert any existing
> > > platforms to use DT in the least.
> > 
> > Agreed.  I don't think that anything older than OMAP2 is worth 
> > converting to DT.  The return on the investment is simply not worth it, 
> > other than for experimental purposes.
> 
> I think you haven't appreciated the situation - let's take PXA as an
> example.  PXA has been around for years, and IP in the latest silicon
> is present in many of the older silicons too.
> 
> There's two issues here:
> 
> 1. If we port existing drivers over to use DT as a means to shrink the
> size of the kernel, we need _all_ PXA using platforms to use DT.
> 
> 2. If we continue having board support for PXA submitted, we want it to
> use DT support.
> 
> The result will be a mess of some bits of PXA using DT, other bits using
> statically declared stuff.  It may get to the point where on some PXA
> platforms DT is used to describe some of the system, and on a different
> PXA platform, it describes some other but needs some static stuff.
> 
> I don't see this as a sustainable way forward.  If we're going to move a
> particular SoC over to DT, we need to move the entire SoC over.  We can't
> do this half-heartedly.
> 
> And that means we _must_ deal with accepting ATAGs from existing boot
> loaders, with that information taking precidence over the DT blob
> supplied with the kernel.

Well... OK.  Let's see how this can be accommodated with the existing 
patch floating around doing that.

@Grant: could you post the patch you have for that?  Let's see how this 
can be made to play nicely with the DTB append patch.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Warner Losh]

On Jun 12, 2011, at 9:59 AM, Russell King - ARM Linux wrote:
> On Sun, Jun 12, 2011 at 11:47:59AM -0400, Nicolas Pitre wrote:
>> On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:
>>> And we've ended up with a fucked up situation which is extremely
>>> fragile, and actually makes me _NOT_ want to convert any existing
>>> platforms to use DT in the least.
>> 
>> Agreed.  I don't think that anything older than OMAP2 is worth 
>> converting to DT.  The return on the investment is simply not worth it, 
>> other than for experimental purposes.
> 
> I think you haven't appreciated the situation - let's take PXA as an
> example.  PXA has been around for years, and IP in the latest silicon
> is present in many of the older silicons too.
> 
> There's two issues here:
> 
> 1. If we port existing drivers over to use DT as a means to shrink the
> size of the kernel, we need _all_ PXA using platforms to use DT.
> 
> 2. If we continue having board support for PXA submitted, we want it to
> use DT support.
> 
> The result will be a mess of some bits of PXA using DT, other bits using
> statically declared stuff.  It may get to the point where on some PXA
> platforms DT is used to describe some of the system, and on a different
> PXA platform, it describes some other but needs some static stuff.
> 
> I don't see this as a sustainable way forward.  If we're going to move a
> particular SoC over to DT, we need to move the entire SoC over.  We can't
> do this half-heartedly.
> 
> And that means we _must_ deal with accepting ATAGs from existing boot
> loaders, with that information taking precidence over the DT blob
> supplied with the kernel.

The only other way out would be to have a small ATAG interpreter that modified/generated the DT so that the rest of the kernel could rely on it completely. This would fatten what is a very slim later right now, but it would mean the whole rest of the kernel could trust the DT blob as gospel.

Warner

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Grant Likely]

On Sun, Jun 12, 2011 at 12:06 AM, Nicolas Pitre
<nicolas.pitre@linaro.org> wrote:
> This is a resend of those patches with fixups after the latest changes
> in mainline.
>
> [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> This one is new and trivial.
>
> [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> Mostly John Bonesio's version with some adjustments and cleanups.
>
> [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> New, simpler alternative to Tony Lindgren's version.

I've gone ahead and added the series to devicetree/test for anyone who
want to play with it.

Thanks,
g.


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

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [110612 11:55]:
> On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:
> 
> > I don't see this as a sustainable way forward.  If we're going to move a
> > particular SoC over to DT, we need to move the entire SoC over.  We can't
> > do this half-heartedly.
> > 
> > And that means we _must_ deal with accepting ATAGs from existing boot
> > loaders, with that information taking precidence over the DT blob
> > supplied with the kernel.
> 
> Well... OK.  Let's see how this can be accommodated with the existing 
> patch floating around doing that.

I agree that we need to parse the user configurable ATAGs to support
existing hardware properly. Otherwise we have edit the .dts for each board
to change the user configurable things, which is not nice for distros.

Regards,

Tony

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [110612 08:55]:
> 
> I don't see this as a sustainable way forward.  If we're going to move a
> particular SoC over to DT, we need to move the entire SoC over.  We can't
> do this half-heartedly.

Agreed. In addition to ATAGs, we need to also consider the large chunks
of data in kernel that Linus originally complained about. We absolutely don't
want to have the same in two places both in .dts files and in platform data
for things like mux, clocks, hwmod data etc.

So we need to make sure the DT append support works transparently for
existing bootloaders so we can just drop the the old data instead of
keeping it around.

Regards,

Tony

[PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [110611 23:02]:
> This is needed for proper alignment when the DTB appending feature
> is used.
> 
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  arch/arm/boot/compressed/vmlinux.lds.in |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.in b/arch/arm/boot/compressed/vmlinux.lds.in
> index ea80abe..6c02db1 100644
> --- a/arch/arm/boot/compressed/vmlinux.lds.in
> +++ b/arch/arm/boot/compressed/vmlinux.lds.in
> @@ -47,6 +47,9 @@ SECTIONS
>    .got			: { *(.got) }
>    _got_end = .;
>    .got.plt		: { *(.got.plt) }
> +
> +  /* ensure the zImage file size is always a multiple of 64 bits */
> +  .pad			: { BYTE(0); . = ALIGN(8); }
>    _edata = .;
>  
>    . = BSS_START;
> -- 
> 1.7.4
> 

[PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [110611 23:03]:
> From: John Bonesio <bones@secretlab.ca>
> 
> 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>
> [nico: adjusted to latest zImage changes plus additional cleanups]
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Looks like this rebased version has two nice improvments: It uses r5
instead of lr, and keeps stack usable early on :)

Acked-by: Tony Lindgren <tony@atomide.com>

[PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [110611 23:03]:
> The appended DTB gets relocated with the decompressor code to get out
> of the way of the decompressed kernel.  However the .bss section may
> be larger than the relocated code and data, therefore overwriting the
> DTB.  Let's make sure the relocation takes care of moving zImage so
> no conflict with .bss occurs.
> 
> Thanks to Tony Lindgren <tony@atomide.com> for figuring out this issue.
> 
> While at it, let's clean up the code a bit so that the wont_overwrite
> symbol is used while determining if a conflict exists, making the above
> change more precise as well as eliminating some ARM/THUMB alternates.
> 
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Great, this simplifies things:

Acked-by: Tony Lindgren <tony@atomide.com>

[PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size

[Russell King - ARM Linux]

On Sun, Jun 12, 2011 at 02:06:38AM -0400, Nicolas Pitre wrote:
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.in b/arch/arm/boot/compressed/vmlinux.lds.in
> index ea80abe..6c02db1 100644
> --- a/arch/arm/boot/compressed/vmlinux.lds.in
> +++ b/arch/arm/boot/compressed/vmlinux.lds.in
> @@ -47,6 +47,9 @@ SECTIONS
>    .got			: { *(.got) }
>    _got_end = .;
>    .got.plt		: { *(.got.plt) }
> +
> +  /* ensure the zImage file size is always a multiple of 64 bits */
> +  .pad			: { BYTE(0); . = ALIGN(8); }

Why add a byte before aligning?  Isn't merely aligning to 64-bit sufficient?

[PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary

[Russell King - ARM Linux]

On Mon, Jun 13, 2011 at 03:46:49AM -0700, Tony Lindgren wrote:
> * Nicolas Pitre <nicolas.pitre@linaro.org> [110611 23:03]:
> > From: John Bonesio <bones@secretlab.ca>
> > 
> > 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>
> > [nico: adjusted to latest zImage changes plus additional cleanups]
> > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> Looks like this rebased version has two nice improvments: It uses r5
> instead of lr, and keeps stack usable early on :)

One of the comments still refers to lr though.

[PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size

[Nicolas Pitre]

On Mon, 13 Jun 2011, Russell King - ARM Linux wrote:

> On Sun, Jun 12, 2011 at 02:06:38AM -0400, Nicolas Pitre wrote:
> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.in b/arch/arm/boot/compressed/vmlinux.lds.in
> > index ea80abe..6c02db1 100644
> > --- a/arch/arm/boot/compressed/vmlinux.lds.in
> > +++ b/arch/arm/boot/compressed/vmlinux.lds.in
> > @@ -47,6 +47,9 @@ SECTIONS
> >    .got			: { *(.got) }
> >    _got_end = .;
> >    .got.plt		: { *(.got.plt) }
> > +
> > +  /* ensure the zImage file size is always a multiple of 64 bits */
> > +  .pad			: { BYTE(0); . = ALIGN(8); }
> 
> Why add a byte before aligning?  Isn't merely aligning to 64-bit sufficient?

No.  If a section is empty, despite the align statement, ld would simply 
omit it entirely and the output file size won't be aligned.


Nicolas

[PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary

[Nicolas Pitre]

On Mon, 13 Jun 2011, Russell King - ARM Linux wrote:

> On Mon, Jun 13, 2011 at 03:46:49AM -0700, Tony Lindgren wrote:
> > * Nicolas Pitre <nicolas.pitre@linaro.org> [110611 23:03]:
> > > From: John Bonesio <bones@secretlab.ca>
> > > 
> > > 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>
> > > [nico: adjusted to latest zImage changes plus additional cleanups]
> > > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > 
> > Looks like this rebased version has two nice improvments: It uses r5
> > instead of lr, and keeps stack usable early on :)
> 
> One of the comments still refers to lr though.

Heh.  Fixed.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Mon, 13 Jun 2011, Tony Lindgren wrote:

> * Nicolas Pitre <nicolas.pitre@linaro.org> [110612 11:55]:
> > On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:
> > 
> > > I don't see this as a sustainable way forward.  If we're going to move a
> > > particular SoC over to DT, we need to move the entire SoC over.  We can't
> > > do this half-heartedly.
> > > 
> > > And that means we _must_ deal with accepting ATAGs from existing boot
> > > loaders, with that information taking precidence over the DT blob
> > > supplied with the kernel.
> > 
> > Well... OK.  Let's see how this can be accommodated with the existing 
> > patch floating around doing that.
> 
> I agree that we need to parse the user configurable ATAGs to support
> existing hardware properly. Otherwise we have edit the .dts for each board
> to change the user configurable things, which is not nice for distros.

You mean "existing bootloaders", right?

Updated bootloaders should translate user configurable information into 
proper DT records and pass the resulting DTB to the kernel separately.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Mon, Jun 13, 2011 at 10:14:07AM -0400, Nicolas Pitre wrote:
> On Mon, 13 Jun 2011, Tony Lindgren wrote:
> 
> > * Nicolas Pitre <nicolas.pitre@linaro.org> [110612 11:55]:
> > > On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:
> > > 
> > > > I don't see this as a sustainable way forward.  If we're going to move a
> > > > particular SoC over to DT, we need to move the entire SoC over.  We can't
> > > > do this half-heartedly.
> > > > 
> > > > And that means we _must_ deal with accepting ATAGs from existing boot
> > > > loaders, with that information taking precidence over the DT blob
> > > > supplied with the kernel.
> > > 
> > > Well... OK.  Let's see how this can be accommodated with the existing 
> > > patch floating around doing that.
> > 
> > I agree that we need to parse the user configurable ATAGs to support
> > existing hardware properly. Otherwise we have edit the .dts for each board
> > to change the user configurable things, which is not nice for distros.
> 
> You mean "existing bootloaders", right?
> 
> Updated bootloaders should translate user configurable information into 
> proper DT records and pass the resulting DTB to the kernel separately.

OMAP is one of the code bases where this really matters - they have a
_lot_ of existing platforms with boot loaders which do the ATAG stuff.
They also have a lot of code in arch/arm that needs to be converted to
a DT representation.

With the current situation where you can have either ATAGs or DT but
not both, they're currently facing either having to break all the
existing platforms by ignoring the ATAGs _or_ keeping two copies of
a considerable amount of data - one in DT form and one in its existing
form.

At present, DT can only be used sensibly on brand new SoCs where there
are no existing platforms with ATAG based boot loaders to worry about.
As things stand at present, even with your patch series, existing SoCs
have no viable path to transition to DT.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [110613 07:16]:
> On Mon, Jun 13, 2011 at 10:14:07AM -0400, Nicolas Pitre wrote:
> > On Mon, 13 Jun 2011, Tony Lindgren wrote:
> > 
> > > * Nicolas Pitre <nicolas.pitre@linaro.org> [110612 11:55]:
> > > > On Sun, 12 Jun 2011, Russell King - ARM Linux wrote:
> > > > 
> > > > > I don't see this as a sustainable way forward.  If we're going to move a
> > > > > particular SoC over to DT, we need to move the entire SoC over.  We can't
> > > > > do this half-heartedly.
> > > > > 
> > > > > And that means we _must_ deal with accepting ATAGs from existing boot
> > > > > loaders, with that information taking precidence over the DT blob
> > > > > supplied with the kernel.
> > > > 
> > > > Well... OK.  Let's see how this can be accommodated with the existing 
> > > > patch floating around doing that.
> > > 
> > > I agree that we need to parse the user configurable ATAGs to support
> > > existing hardware properly. Otherwise we have edit the .dts for each board
> > > to change the user configurable things, which is not nice for distros.
> > 
> > You mean "existing bootloaders", right?

Yes that's correct. There are some bootloaders that will never get updated
as they're proprietary and signed.

> > Updated bootloaders should translate user configurable information into 
> > proper DT records and pass the resulting DTB to the kernel separately.
> 
> OMAP is one of the code bases where this really matters - they have a
> _lot_ of existing platforms with boot loaders which do the ATAG stuff.
> They also have a lot of code in arch/arm that needs to be converted to
> a DT representation.

I think we're OK to start using appended device tree support for mach-omap2
once we have some real data. The only thing missing is the support for
parsing ATAGs for cmdline and other user configurable stuff.

Of course that means that we need to set the DT_APPEND support on in the
omap2plus_defconfig. And I'm assuming that the DT_APPEND support works
also with bootloader passed DT data if the appended data is empty..
 
> With the current situation where you can have either ATAGs or DT but
> not both, they're currently facing either having to break all the
> existing platforms by ignoring the ATAGs _or_ keeping two copies of
> a considerable amount of data - one in DT form and one in its existing
> form.
> 
> At present, DT can only be used sensibly on brand new SoCs where there
> are no existing platforms with ATAG based boot loaders to worry about.
> As things stand at present, even with your patch series, existing SoCs
> have no viable path to transition to DT.

The situation is not actually not that bad, but currently changes to
the .dts file are needed. So some parsing of user configurable ATAGs
is badly needed.

Regards,

Tony

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Mon, 13 Jun 2011, Russell King - ARM Linux wrote:

> On Mon, Jun 13, 2011 at 10:14:07AM -0400, Nicolas Pitre wrote:
> > On Mon, 13 Jun 2011, Tony Lindgren wrote:
> > 
> > > I agree that we need to parse the user configurable ATAGs to support
> > > existing hardware properly. Otherwise we have edit the .dts for each board
> > > to change the user configurable things, which is not nice for distros.
> > 
> > You mean "existing bootloaders", right?
> > 
> > Updated bootloaders should translate user configurable information into 
> > proper DT records and pass the resulting DTB to the kernel separately.
> 
> OMAP is one of the code bases where this really matters - they have a
> _lot_ of existing platforms with boot loaders which do the ATAG stuff.
> They also have a lot of code in arch/arm that needs to be converted to
> a DT representation.

Yes, agreed.  I just wanted to make the situation clear above, so people 
aren't confused in believing that the DT data is always static.  New 
bootloaders should have the same ability to dynamically change some of 
the parameters passed to the kernel.  So the issue is not about existing 
hardware, but rather about existing bootloaders.

> With the current situation where you can have either ATAGs or DT but
> not both, they're currently facing either having to break all the
> existing platforms by ignoring the ATAGs _or_ keeping two copies of
> a considerable amount of data - one in DT form and one in its existing
> form.
> 
> At present, DT can only be used sensibly on brand new SoCs where there
> are no existing platforms with ATAG based boot loaders to worry about.
> As things stand at present, even with your patch series, existing SoCs
> have no viable path to transition to DT.

As I said, I'm now convinced that the patch adding a shim to translate 
ATAGs into DT entries should be added to this series.  I was reluctant 
initially for insentive purposes, but your argument clearly tilted the 
balance the other way.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Grant Likely]

On Mon, Jun 13, 2011 at 9:14 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Mon, 13 Jun 2011, Russell King - ARM Linux wrote:
>
>> On Mon, Jun 13, 2011 at 10:14:07AM -0400, Nicolas Pitre wrote:
>> > On Mon, 13 Jun 2011, Tony Lindgren wrote:
>> >
>> > > I agree that we need to parse the user configurable ATAGs to support
>> > > existing hardware properly. Otherwise we have edit the .dts for each board
>> > > to change the user configurable things, which is not nice for distros.
>> >
>> > You mean "existing bootloaders", right?
>> >
>> > Updated bootloaders should translate user configurable information into
>> > proper DT records and pass the resulting DTB to the kernel separately.
>>
>> OMAP is one of the code bases where this really matters - they have a
>> _lot_ of existing platforms with boot loaders which do the ATAG stuff.
>> They also have a lot of code in arch/arm that needs to be converted to
>> a DT representation.
>
> Yes, agreed. ?I just wanted to make the situation clear above, so people
> aren't confused in believing that the DT data is always static. ?New
> bootloaders should have the same ability to dynamically change some of
> the parameters passed to the kernel. ?So the issue is not about existing
> hardware, but rather about existing bootloaders.
>
>> With the current situation where you can have either ATAGs or DT but
>> not both, they're currently facing either having to break all the
>> existing platforms by ignoring the ATAGs _or_ keeping two copies of
>> a considerable amount of data - one in DT form and one in its existing
>> form.
>>
>> At present, DT can only be used sensibly on brand new SoCs where there
>> are no existing platforms with ATAG based boot loaders to worry about.
>> As things stand at present, even with your patch series, existing SoCs
>> have no viable path to transition to DT.
>
> As I said, I'm now convinced that the patch adding a shim to translate
> ATAGs into DT entries should be added to this series. ?I was reluctant
> initially for insentive purposes, but your argument clearly tilted the
> balance the other way.

John Bonesio was the author of that patch.

John, can you dust of the ATAGs-->DT conversion patch and get it
rebased to my current devicetree/test branch?  I've picked up Nicolas'
3 patch series which includes your dtb append patch into
devicetree/test.

g.

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

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Dmitry Eremin-Solenikov]

Russell King - ARM Linux wrote:

> On Sun, Jun 12, 2011 at 05:01:22PM +0200, Arnd Bergmann wrote:
>> Of course it should override the device tree, I'm not arguing that.
>> All I'm saying is that we don't need to special-case this or support
>> both formats once the kernel is there as long as we move the information
>> into appropriate DT-representation of the same data.
>
> You're arguing for it to be parsed by whatever fragment of assembly code
> we have to place around the zImage to merge it into DT.
>
> And yes, it's going to have to be assembly code because we don't have a
> C environment at that point which can run in a relocatable way.

Please, again look at the cuboot stuff which is used on powerpc. It uses
limited C environment: it's executed before kernel entry, so it can use
stack, etc. as setup by the bootloader.


-- 
With best wishes
Dmitry

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Sun, 12 Jun 2011, Grant Likely wrote:

> On Sun, Jun 12, 2011 at 12:06 AM, Nicolas Pitre
> <nicolas.pitre@linaro.org> wrote:
> > This is a resend of those patches with fixups after the latest changes
> > in mainline.
> >
> > [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> > This one is new and trivial.
> >
> > [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> > Mostly John Bonesio's version with some adjustments and cleanups.
> >
> > [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> > New, simpler alternative to Tony Lindgren's version.
> 
> I've gone ahead and added the series to devicetree/test for anyone who
> want to play with it.

I've published those patches with all the latest fixes and ACKs here:

	git://git.linaro.org/people/nico/linux.git zImage_DTB_append


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Russell King - ARM Linux]

On Mon, Jun 13, 2011 at 08:24:27PM +0000, Dmitry Eremin-Solenikov wrote:
> Russell King - ARM Linux wrote:
> 
> > On Sun, Jun 12, 2011 at 05:01:22PM +0200, Arnd Bergmann wrote:
> >> Of course it should override the device tree, I'm not arguing that.
> >> All I'm saying is that we don't need to special-case this or support
> >> both formats once the kernel is there as long as we move the information
> >> into appropriate DT-representation of the same data.
> >
> > You're arguing for it to be parsed by whatever fragment of assembly code
> > we have to place around the zImage to merge it into DT.
> >
> > And yes, it's going to have to be assembly code because we don't have a
> > C environment at that point which can run in a relocatable way.
> 
> Please, again look at the cuboot stuff which is used on powerpc. It uses
> limited C environment: it's executed before kernel entry, so it can use
> stack, etc. as setup by the bootloader.

Boot loaders don't pass a stack to the kernel - what may be in the
stack pointer may not be valid at the point when the kernel is called -
it may be that it can only be used while the MMU is on, and at this
point the MMU will be off.  So you can't rely on anything from the
boot loader which I haven't documented as being part of the ARM kernel
boot protocol.

Please understand that we absolutely must get this issue resolved before
the next merge window, and have a solution ready to be merged at that
point.  As I see it, this is one of the major issues that requires
addressing.  Why?

At the moment we can't sensibly accept new board support adding several
thousand lines of new code to arch/arm.  Effectively that means that
the ARM platform people are in limbo at present, and have been for the
past three months.

We _desperately_ need to get the DT story sorted and starting to be
usable, with a transition path for existing SoC support so that
the platform people know where they are and how they can proceed.

The cost to the entire community of not having that sorted by the next
merge window will be enormous - I would not be surprised if it isn't
_already_ turning platform people away from Linux.  The longer this
situation continues, the more damage will be done.

So, for our own sakes, we must get the issue of ATAG and DT
interoperability resolved so that SoC people can rest assured in the
knowledge that they can start moving SoC data into DT without breaking
existing platforms.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[David Brown]

On Sun, Jun 12 2011, Russell King - ARM Linux wrote:

> On Sun, Jun 12, 2011 at 06:42:16PM +0800, Shawn Guo wrote:
>> On Sun, Jun 12, 2011 at 10:52:17AM +0100, Russell King - ARM Linux wrote:
>> > On Sun, Jun 12, 2011 at 05:38:23PM +0800, Shawn Guo wrote:
>> > > On Sun, Jun 12, 2011 at 10:21:31AM +0100, Russell King - ARM Linux wrote:
>> > > > What if your platform doesn't use uboot?
>> > > 
>> > > Add dtb parsing support with the help from libfdt, I guess.  It is
>> > > some amount of work, but it's not a rewrite of bootloader, IMHO.
>> > 
>> > I guess you're suggesting that this wrapper uses libfdt to merge
>> > the ATAGs with the DT info?
>> > 
>> No, ATAGs does not play at all in this case.  For u-boot example
>> again, if it boots a dt kernel, dtb will be parsed to get cmdline
>> node overwritten as bootargs env value, and then it boots the dt
>> kernel with this updated dtb.
>
> You've missed my point entirely.  What you're saying is that we have to
> re-build and replace the boot loader in order to pass a command line into
> a kernel using the DT wrapper.
>
> I'm saying that you shouldn't have to, and the kernel should accept the
> memory size and command line from the ATAGs _in addition_ to the
> appended DT blob, and the ATAGs in that case should take precidence.

Having just now booted an MSM target using device tree, I'm going to
agree with Russell here.  I don't really see a clean transition path for
us to hard cut over to DT, at least on existing targets.

The MSMs use a custom bootloader.  It builds ATAGS to pass in memory
information, command line arguments, and the ramdisk address.  Working
with the bootloader team just to do this was difficult; trying to
support DT in the bootloader would be even more difficult.

But, beyond this, we have to still be able to boot our production
kernels (which don't have DT support, and probably won't for quite some
time).

The steps I had to go through to boot a DT kernel were kind of
complicated:

  - Boot kernel in a debugger, and inspect the ATAGS.

  - Edit my DTS file to match the current values.

  - Compile the DTS file.

  - Load the dtb on top of the ATAGS.

Unless I'm missing something, I don't see a clean way of supporting this
that doesn't involve the kernel being able to parse the ATAGS as well.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Mon, 13 Jun 2011, David Brown wrote:

> On Sun, Jun 12 2011, Russell King - ARM Linux wrote:
> 
> > On Sun, Jun 12, 2011 at 06:42:16PM +0800, Shawn Guo wrote:
> >> On Sun, Jun 12, 2011 at 10:52:17AM +0100, Russell King - ARM Linux wrote:
> >> > On Sun, Jun 12, 2011 at 05:38:23PM +0800, Shawn Guo wrote:
> >> > > On Sun, Jun 12, 2011 at 10:21:31AM +0100, Russell King - ARM Linux wrote:
> >> > > > What if your platform doesn't use uboot?
> >> > > 
> >> > > Add dtb parsing support with the help from libfdt, I guess.  It is
> >> > > some amount of work, but it's not a rewrite of bootloader, IMHO.
> >> > 
> >> > I guess you're suggesting that this wrapper uses libfdt to merge
> >> > the ATAGs with the DT info?
> >> > 
> >> No, ATAGs does not play at all in this case.  For u-boot example
> >> again, if it boots a dt kernel, dtb will be parsed to get cmdline
> >> node overwritten as bootargs env value, and then it boots the dt
> >> kernel with this updated dtb.
> >
> > You've missed my point entirely.  What you're saying is that we have to
> > re-build and replace the boot loader in order to pass a command line into
> > a kernel using the DT wrapper.
> >
> > I'm saying that you shouldn't have to, and the kernel should accept the
> > memory size and command line from the ATAGs _in addition_ to the
> > appended DT blob, and the ATAGs in that case should take precidence.
> 
> Having just now booted an MSM target using device tree, I'm going to
> agree with Russell here.  I don't really see a clean transition path for
> us to hard cut over to DT, at least on existing targets.
> 
> The MSMs use a custom bootloader.  It builds ATAGS to pass in memory
> information, command line arguments, and the ramdisk address.  Working
> with the bootloader team just to do this was difficult; trying to
> support DT in the bootloader would be even more difficult.
> 
> But, beyond this, we have to still be able to boot our production
> kernels (which don't have DT support, and probably won't for quite some
> time).
> 
> The steps I had to go through to boot a DT kernel were kind of
> complicated:
> 
>   - Boot kernel in a debugger, and inspect the ATAGS.
> 
>   - Edit my DTS file to match the current values.
> 
>   - Compile the DTS file.
> 
>   - Load the dtb on top of the ATAGS.
> 
> Unless I'm missing something, I don't see a clean way of supporting this
> that doesn't involve the kernel being able to parse the ATAGS as well.

FYI: I've dug up the patch from John Bonesio doing just that.  While the 
patch doesn't apply anymore, it looks trivial enough.  I should have it 
working by tomorrow.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Grant Likely]

On Mon, Jun 13, 2011 at 4:05 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jun 13, 2011 at 08:24:27PM +0000, Dmitry Eremin-Solenikov wrote:
>> Russell King - ARM Linux wrote:
>>
>> > On Sun, Jun 12, 2011 at 05:01:22PM +0200, Arnd Bergmann wrote:
>> >> Of course it should override the device tree, I'm not arguing that.
>> >> All I'm saying is that we don't need to special-case this or support
>> >> both formats once the kernel is there as long as we move the information
>> >> into appropriate DT-representation of the same data.
>> >
>> > You're arguing for it to be parsed by whatever fragment of assembly code
>> > we have to place around the zImage to merge it into DT.
>> >
>> > And yes, it's going to have to be assembly code because we don't have a
>> > C environment at that point which can run in a relocatable way.
>>
>> Please, again look at the cuboot stuff which is used on powerpc. It uses
>> limited C environment: it's executed before kernel entry, so it can use
>> stack, etc. as setup by the bootloader.
>
> Boot loaders don't pass a stack to the kernel - what may be in the
> stack pointer may not be valid at the point when the kernel is called -
> it may be that it can only be used while the MMU is on, and at this
> point the MMU will be off. ?So you can't rely on anything from the
> boot loader which I haven't documented as being part of the ARM kernel
> boot protocol.
>
> Please understand that we absolutely must get this issue resolved before
> the next merge window, and have a solution ready to be merged at that
> point. ?As I see it, this is one of the major issues that requires
> addressing. ?Why?

John Bonesio is working on it this week (starting tomorrow), and will
get an updated patch out ASAP.

g.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[David Brown]

On Sun, Jun 12 2011, Nicolas Pitre wrote:

> It is not pointless.  Never it was in the cards to convert old platforms 
> first.  The most problematic platforms responsible for the high volume 
> of changes that has gone in mainline lately are the modern ones, not PXA 
> based ones.  So DT is going to be most effective for the new stuff to 
> come.  And realistically that's where we might get developers attention, 
> not with those platforms being end-of-lived by their manufacturers.

New stuff doesn't necessarily mean new boot loaders.  I don't have a lot
of control over the bootloaders on MSM.  We can probably move away from
ATAGS, but only after every kernel that could possibly boot on that
target will work with DT.  I don't see that happening for at least a
year.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Mon, 13 Jun 2011, Nicolas Pitre wrote:

> > Unless I'm missing something, I don't see a clean way of supporting this
> > that doesn't involve the kernel being able to parse the ATAGS as well.
> 
> FYI: I've dug up the patch from John Bonesio doing just that.  While the 
> patch doesn't apply anymore, it looks trivial enough.  I should have it 
> working by tomorrow.

Well, here it is.  It compiles, but otherwise completely untested.

This applies on top of the 3 other patches I posted when this thread was 
started.

While this could be cleaned up further, the functionality should all be 
there and usable.

From: Nicolas Pitre <nicolas.pitre@linaro.org>
Date: Tue, 14 Jun 2011 02:40:33 -0400
Subject: [PATCH] ARM: zImage: allow supplementing appended DTB with traditional ATAG data

This is based on an older patch from John Bonesio <bones@secretlab.ca>.

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 66b7d1e..166bd2a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1735,6 +1735,17 @@ config ARM_APPENDED_DTB
 	  (dtb) appended to zImage
 	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
 
+config ARM_ATAG_DTB_COMPAT
+	bool "Supplement the appended DTB with traditional ATAG information"
+	depends on ARM_APPENDED_DTB
+	help
+	  Some old bootloaders can't be updated to a DTB capable one, yet
+	  they provide ATAGs with memory configuration, the ramdisk address,
+	  the kernel cmdline string, etc.  To allow a device tree enabled
+	  kernel to be used with such bootloaders, this option allows
+	  zImage to extract the information from the ATAG list and store it
+	  at run time into the appended DTB.
+
 config CMDLINE
 	string "Default kernel command string"
 	default ""
diff --git a/arch/arm/boot/compressed/.gitignore b/arch/arm/boot/compressed/.gitignore
index c602896..e0936a1 100644
--- a/arch/arm/boot/compressed/.gitignore
+++ b/arch/arm/boot/compressed/.gitignore
@@ -5,3 +5,12 @@ piggy.lzo
 piggy.lzma
 vmlinux
 vmlinux.lds
+
+# borrowed libfdt files
+fdt.c
+fdt.h
+fdt_ro.c
+fdt_rw.c
+fdt_wip.c
+libfdt.h
+libfdt_internal.h
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 48bead9..4b94995 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -83,19 +83,36 @@ suffix_$(CONFIG_KERNEL_GZIP) = gzip
 suffix_$(CONFIG_KERNEL_LZO)  = lzo
 suffix_$(CONFIG_KERNEL_LZMA) = lzma
 
+# libfdt files for the ATAG compatibility mode
+
+libfdt		:= fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
+libfdt_hdrs	:= fdt.h libfdt.h libfdt_internal.h
+
+libfdt_objs	:= $(addsuffix .o, $(basename $(libfdt)))
+
+$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
+	$(call if_changed,shipped)
+
+$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
+	$(addprefix $(obj)/,$(libfdt_hdrs))
+
+ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
+OBJS	+= $(libfdt_objs) atags_to_fdt.o 
+endif
+
 targets       := vmlinux vmlinux.lds \
 		 piggy.$(suffix_y) piggy.$(suffix_y).o \
 		 font.o font.c head.o misc.o $(OBJS)
 
 # Make sure files are removed during clean
-extra-y       += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S
+extra-y       += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S $(libfdt) $(libfdt_hdrs)
 
 ifeq ($(CONFIG_FUNCTION_TRACER),y)
 ORIG_CFLAGS := $(KBUILD_CFLAGS)
 KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
 endif
 
-ccflags-y := -fpic -fno-builtin
+ccflags-y := -fpic -fno-builtin -I$(src)
 asflags-y := -Wa,-march=all
 
 # Supply kernel BSS size to the decompressor via a linker symbol.
@@ -118,7 +135,7 @@ LDFLAGS_vmlinux += -X
 LDFLAGS_vmlinux += -T
 
 # For __aeabi_uidivmod
-lib1funcs = $(obj)/lib1funcs.o
+lib1funcs = $(obj)/lib1funcs.o $(obj)/../../lib/lib.a
 
 $(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE
 	$(call cmd,shipped)
diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c
new file mode 100644
index 0000000..11c1a88
--- /dev/null
+++ b/arch/arm/boot/compressed/atags_to_fdt.c
@@ -0,0 +1,71 @@
+#include <asm/setup.h>
+#include <libfdt.h>
+
+static int setprop(void *fdt, const char *node_path, const char *property,
+		   uint32_t *val_array, int size)
+{
+	int offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+	return fdt_setprop(fdt, offset, property, val_array, size);
+}
+
+static int setprop_string(void *fdt, const char *node_path,
+			  const char *property, const char *string)
+{
+	int offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+	return fdt_setprop_string(fdt, offset, property, string);
+}
+
+static int setprop_cell(void *fdt, const char *node_path,
+			const char *property, uint32_t val)
+{
+	int offset = fdt_path_offset(fdt, node_path);
+	if (offset < 0)
+		return offset;
+	return fdt_setprop_cell(fdt, offset, property, val);
+}
+
+int atags_to_fdt(void *dt,  void *atag_list)
+{
+	struct tag *atag = atag_list;
+
+	/* make sure we've got an aligned pointer */
+	if ((u32)atag_list & 0x3)
+		return -1;
+
+	/* if we get a DTB here we're done already */
+	if (*(u32 *)atag_list == fdt32_to_cpu(FDT_MAGIC))
+	       return 0;	
+
+	/* validate the ATAG */
+	if (atag->hdr.tag != ATAG_CORE ||
+	    (atag->hdr.size != tag_size(tag_core) &&
+	     atag->hdr.size != 2))
+		return -1;
+
+	for_each_tag(atag, atag_list) {
+		if (atag->hdr.tag == ATAG_CMDLINE) {
+			setprop_string(dt, "/chosen", "bootargs",
+			                  atag->u.cmdline.cmdline);
+		} else if (atag->hdr.tag == ATAG_MEM) {
+			uint32_t mem_reg_property[2];
+			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
+			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
+			setprop(dt, "/memory", "reg", mem_reg_property,
+			           sizeof(mem_reg_property));
+		} else if (atag->hdr.tag == ATAG_INITRD2) {
+			uint32_t initrd_start, initrd_size;
+			initrd_start = atag->u.initrd.start;
+			initrd_size = atag->u.initrd.size;
+			setprop_cell(dt, "/chosen", "linux,initrd-start",
+			                initrd_start);
+			setprop_cell(dt, "/chosen", "linux,initrd-end",
+			                initrd_start + initrd_size);
+		}
+	}
+
+	return 0;
+}
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index d4f8db2..5a0a2409 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -246,6 +246,36 @@ restart:	adr	r0, LC0
 		cmp	lr, r1
 		bne	dtb_check_done		@ not found
 
+#ifdef CONFIG_ARM_ATAG_DTB_COMPAT
+		/*
+		 * OK... Let's do some funky business here.
+		 * If we do have a DTB appended to zImage, and we do have
+		 * an ATAG list around, we want the later to be translated
+		 * and folded into the former here.  To be on the safe side,
+		 * let's temporarily move  the stack away into the malloc
+		 * area.  No GOT fixup has occurred yet, but none of the
+		 * code we're about to call uses any global variable.
+		*/
+		add	sp, sp, #0x10000
+		stmfd	sp!, {r0-r3, ip, lr}
+		mov	r0, r6
+		mov	r1, r8
+		bl	atags_to_fdt
+		
+		/*
+		 * If that didn't work (non-zero return), there is no ATAG
+		 * at the location pointed by r8.  Try the typical 0x100
+		 * offset from start of RAM.
+		 */
+		cmp	r0, #0
+		mov	r0, r6
+		sub	r1, r4, #(TEXT_OFFSET - 0x100)
+		blne	atags_to_fdt
+
+		ldmfd	sp!, {r0-r3, ip, lr}
+		sub	sp, sp, #0x10000
+#endif
+
 		mov	r8, r6			@ use the appended device tree
 
 		/*
diff --git a/arch/arm/boot/compressed/libfdt_env.h b/arch/arm/boot/compressed/libfdt_env.h
new file mode 100644
index 0000000..1f4e718
--- /dev/null
+++ b/arch/arm/boot/compressed/libfdt_env.h
@@ -0,0 +1,15 @@
+#ifndef _ARM_LIBFDT_ENV_H
+#define _ARM_LIBFDT_ENV_H
+
+#include <linux/types.h>
+#include <linux/string.h>
+#include <asm/byteorder.h>
+
+#define fdt16_to_cpu(x)		be16_to_cpu(x)
+#define cpu_to_fdt16(x)		cpu_to_be16(x)
+#define fdt32_to_cpu(x)		be32_to_cpu(x)
+#define cpu_to_fdt32(x)		cpu_to_be32(x)
+#define fdt64_to_cpu(x)		be64_to_cpu(x)
+#define cpu_to_fdt64(x)		cpu_to_be64(x)
+
+#endif
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index 832d372..747de6b 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -136,6 +136,55 @@ void *memcpy(void *__dest, __const void *__src, size_t __n)
 	return __dest;
 }
 
+void *memmove(void *__dest, __const void *__src, size_t __n)
+{
+	unsigned char *d = __dest;
+	const unsigned char *s = __src;
+
+	if (__dest <= __src)
+		return memcpy(__dest, __src, __n);
+
+	while (--__n >= 0)
+		d[__n] = s[__n];
+
+	return __dest;
+}
+
+size_t strlen(const char *s)
+{
+	const char *sc;
+
+	for (sc = s; *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+
+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;
+}
+
+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;
+}
+
 /*
  * gzip declarations
  */

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Shawn Guo]

On Tue, Jun 14, 2011 at 03:09:02AM -0400, Nicolas Pitre wrote:
> On Mon, 13 Jun 2011, Nicolas Pitre wrote:
> 
> > > Unless I'm missing something, I don't see a clean way of supporting this
> > > that doesn't involve the kernel being able to parse the ATAGS as well.
> > 
> > FYI: I've dug up the patch from John Bonesio doing just that.  While the 
> > patch doesn't apply anymore, it looks trivial enough.  I should have it 
> > working by tomorrow.
> 
> Well, here it is.  It compiles, but otherwise completely untested.
> 
Just tested the patch on mx51 babbage, and it's working.  Great work!
I have one comment below though.

> This applies on top of the 3 other patches I posted when this thread was 
> started.
> 
> While this could be cleaned up further, the functionality should all be 
> there and usable.
> 
> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> Date: Tue, 14 Jun 2011 02:40:33 -0400
> Subject: [PATCH] ARM: zImage: allow supplementing appended DTB with traditional ATAG data
> 
> This is based on an older patch from John Bonesio <bones@secretlab.ca>.
> 
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 66b7d1e..166bd2a 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1735,6 +1735,17 @@ config ARM_APPENDED_DTB
>  	  (dtb) appended to zImage
>  	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
>  
> +config ARM_ATAG_DTB_COMPAT
> +	bool "Supplement the appended DTB with traditional ATAG information"
> +	depends on ARM_APPENDED_DTB
> +	help
> +	  Some old bootloaders can't be updated to a DTB capable one, yet
> +	  they provide ATAGs with memory configuration, the ramdisk address,
> +	  the kernel cmdline string, etc.  To allow a device tree enabled
> +	  kernel to be used with such bootloaders, this option allows
> +	  zImage to extract the information from the ATAG list and store it
> +	  at run time into the appended DTB.
> +
>  config CMDLINE
>  	string "Default kernel command string"
>  	default ""
> diff --git a/arch/arm/boot/compressed/.gitignore b/arch/arm/boot/compressed/.gitignore
> index c602896..e0936a1 100644
> --- a/arch/arm/boot/compressed/.gitignore
> +++ b/arch/arm/boot/compressed/.gitignore
> @@ -5,3 +5,12 @@ piggy.lzo
>  piggy.lzma
>  vmlinux
>  vmlinux.lds
> +
> +# borrowed libfdt files
> +fdt.c
> +fdt.h
> +fdt_ro.c
> +fdt_rw.c
> +fdt_wip.c
> +libfdt.h
> +libfdt_internal.h

[Note: I copied these files from scripts/dtc/libfdt]

> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 48bead9..4b94995 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -83,19 +83,36 @@ suffix_$(CONFIG_KERNEL_GZIP) = gzip
>  suffix_$(CONFIG_KERNEL_LZO)  = lzo
>  suffix_$(CONFIG_KERNEL_LZMA) = lzma
>  
> +# libfdt files for the ATAG compatibility mode
> +
> +libfdt		:= fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> +libfdt_hdrs	:= fdt.h libfdt.h libfdt_internal.h
> +
> +libfdt_objs	:= $(addsuffix .o, $(basename $(libfdt)))
> +
> +$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
> +	$(call if_changed,shipped)
> +
> +$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
> +	$(addprefix $(obj)/,$(libfdt_hdrs))
> +
> +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> +OBJS	+= $(libfdt_objs) atags_to_fdt.o 
> +endif
> +
>  targets       := vmlinux vmlinux.lds \
>  		 piggy.$(suffix_y) piggy.$(suffix_y).o \
>  		 font.o font.c head.o misc.o $(OBJS)
>  
>  # Make sure files are removed during clean
> -extra-y       += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S
> +extra-y       += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S $(libfdt) $(libfdt_hdrs)
>  
>  ifeq ($(CONFIG_FUNCTION_TRACER),y)
>  ORIG_CFLAGS := $(KBUILD_CFLAGS)
>  KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
>  endif
>  
> -ccflags-y := -fpic -fno-builtin
> +ccflags-y := -fpic -fno-builtin -I$(src)
>  asflags-y := -Wa,-march=all
>  
>  # Supply kernel BSS size to the decompressor via a linker symbol.
> @@ -118,7 +135,7 @@ LDFLAGS_vmlinux += -X
>  LDFLAGS_vmlinux += -T
>  
>  # For __aeabi_uidivmod
> -lib1funcs = $(obj)/lib1funcs.o
> +lib1funcs = $(obj)/lib1funcs.o $(obj)/../../lib/lib.a
>  
>  $(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE
>  	$(call cmd,shipped)
> diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c
> new file mode 100644
> index 0000000..11c1a88
> --- /dev/null
> +++ b/arch/arm/boot/compressed/atags_to_fdt.c
> @@ -0,0 +1,71 @@
> +#include <asm/setup.h>
> +#include <libfdt.h>
> +
> +static int setprop(void *fdt, const char *node_path, const char *property,
> +		   uint32_t *val_array, int size)
> +{
> +	int offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +	return fdt_setprop(fdt, offset, property, val_array, size);
> +}
> +
> +static int setprop_string(void *fdt, const char *node_path,
> +			  const char *property, const char *string)
> +{
> +	int offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +	return fdt_setprop_string(fdt, offset, property, string);
> +}
> +
> +static int setprop_cell(void *fdt, const char *node_path,
> +			const char *property, uint32_t val)
> +{
> +	int offset = fdt_path_offset(fdt, node_path);
> +	if (offset < 0)
> +		return offset;
> +	return fdt_setprop_cell(fdt, offset, property, val);
> +}
> +
> +int atags_to_fdt(void *dt,  void *atag_list)
> +{
> +	struct tag *atag = atag_list;
> +
> +	/* make sure we've got an aligned pointer */
> +	if ((u32)atag_list & 0x3)
> +		return -1;
> +
> +	/* if we get a DTB here we're done already */
> +	if (*(u32 *)atag_list == fdt32_to_cpu(FDT_MAGIC))
> +	       return 0;	
> +
> +	/* validate the ATAG */
> +	if (atag->hdr.tag != ATAG_CORE ||
> +	    (atag->hdr.size != tag_size(tag_core) &&
> +	     atag->hdr.size != 2))
> +		return -1;
> +
> +	for_each_tag(atag, atag_list) {
> +		if (atag->hdr.tag == ATAG_CMDLINE) {
> +			setprop_string(dt, "/chosen", "bootargs",
> +			                  atag->u.cmdline.cmdline);
> +		} else if (atag->hdr.tag == ATAG_MEM) {
> +			uint32_t mem_reg_property[2];
> +			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> +			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> +			setprop(dt, "/memory", "reg", mem_reg_property,
> +			           sizeof(mem_reg_property));
> +		} else if (atag->hdr.tag == ATAG_INITRD2) {
> +			uint32_t initrd_start, initrd_size;
> +			initrd_start = atag->u.initrd.start;
> +			initrd_size = atag->u.initrd.size;
> +			setprop_cell(dt, "/chosen", "linux,initrd-start",
> +			                initrd_start);
> +			setprop_cell(dt, "/chosen", "linux,initrd-end",
> +			                initrd_start + initrd_size);
> +		}
> +	}
> +
> +	return 0;
> +}

This works only when there are corresponding nodes already in dtb.
Otherwise, the atag arguments are not going to be updated into dtb.
I suppose these nodes should be created to accommodate the arguments
from atags if the appended dtb does not have them, no?

-- 
Regards,
Shawn

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
> +
> +	for_each_tag(atag, atag_list) {
> +		if (atag->hdr.tag == ATAG_CMDLINE) {
> +			setprop_string(dt, "/chosen", "bootargs",
> +			                  atag->u.cmdline.cmdline);
> +		} else if (atag->hdr.tag == ATAG_MEM) {
> +			uint32_t mem_reg_property[2];
> +			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> +			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> +			setprop(dt, "/memory", "reg", mem_reg_property,
> +			           sizeof(mem_reg_property));
> +		} else if (atag->hdr.tag == ATAG_INITRD2) {
> +			uint32_t initrd_start, initrd_size;
> +			initrd_start = atag->u.initrd.start;
> +			initrd_size = atag->u.initrd.size;
> +			setprop_cell(dt, "/chosen", "linux,initrd-start",
> +			                initrd_start);
> +			setprop_cell(dt, "/chosen", "linux,initrd-end",
> +			                initrd_start + initrd_size);
> +		}
> +	}

I think Russell posted a complete list of the ATAGs to translate
somewhere, but at least ATAG_REVISION is missing here. That's being
used quite a bit as system_rev to set things dynamically.

Regards,

Tony

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Tue, 14 Jun 2011, Tony Lindgren wrote:

> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
> > +
> > +	for_each_tag(atag, atag_list) {
> > +		if (atag->hdr.tag == ATAG_CMDLINE) {
> > +			setprop_string(dt, "/chosen", "bootargs",
> > +			                  atag->u.cmdline.cmdline);
> > +		} else if (atag->hdr.tag == ATAG_MEM) {
> > +			uint32_t mem_reg_property[2];
> > +			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> > +			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> > +			setprop(dt, "/memory", "reg", mem_reg_property,
> > +			           sizeof(mem_reg_property));
> > +		} else if (atag->hdr.tag == ATAG_INITRD2) {
> > +			uint32_t initrd_start, initrd_size;
> > +			initrd_start = atag->u.initrd.start;
> > +			initrd_size = atag->u.initrd.size;
> > +			setprop_cell(dt, "/chosen", "linux,initrd-start",
> > +			                initrd_start);
> > +			setprop_cell(dt, "/chosen", "linux,initrd-end",
> > +			                initrd_start + initrd_size);
> > +		}
> > +	}
> 
> I think Russell posted a complete list of the ATAGs to translate
> somewhere, but at least ATAG_REVISION is missing here. That's being
> used quite a bit as system_rev to set things dynamically.

No problem.  This is a work in progress.  We still can test the concept 
and fine tune the actual set of ATAGs being translated.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Arnd Bergmann]

On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote:
> On Tue, 14 Jun 2011, Tony Lindgren wrote:
> 
> > * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
> > > +
> > > +   for_each_tag(atag, atag_list) {
> > > +           if (atag->hdr.tag == ATAG_CMDLINE) {
> > > +                   setprop_string(dt, "/chosen", "bootargs",
> > > +                                     atag->u.cmdline.cmdline);
> > > +           } else if (atag->hdr.tag == ATAG_MEM) {
> > > +                   uint32_t mem_reg_property[2];
> > > +                   mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> > > +                   mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> > > +                   setprop(dt, "/memory", "reg", mem_reg_property,
> > > +                              sizeof(mem_reg_property));
> > > +           } else if (atag->hdr.tag == ATAG_INITRD2) {
> > > +                   uint32_t initrd_start, initrd_size;
> > > +                   initrd_start = atag->u.initrd.start;
> > > +                   initrd_size = atag->u.initrd.size;
> > > +                   setprop_cell(dt, "/chosen", "linux,initrd-start",
> > > +                                   initrd_start);
> > > +                   setprop_cell(dt, "/chosen", "linux,initrd-end",
> > > +                                   initrd_start + initrd_size);
> > > +           }
> > > +   }
> > 
> > I think Russell posted a complete list of the ATAGs to translate
> > somewhere, but at least ATAG_REVISION is missing here. That's being
> > used quite a bit as system_rev to set things dynamically.
> 
> No problem.  This is a work in progress.  We still can test the concept 
> and fine tune the actual set of ATAGs being translated.

Let's talk about the revision field now, because it doesn't have a
direct correspondence in existing attributes.

Functionality-wise, this would probably be the "compatible" property
of the root node, with its most specific name to match, but that's not
trivial to generate.

In some cases, the system revisions have significant differences in their
devices (why else would you care about the revision), which means that you
actually need a different device tree per revision anyway. If that's the
common case, we could just ignore it completely or instead have a way
to choose a specific device tree from a list based on the revision.

Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL
into a string and put them into the root "serial-number" property that
is fairly easy to do, but would be harder to parse if you actually rely
on specific values.

	Arnd

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Tue, 14 Jun 2011, Arnd Bergmann wrote:

> On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote:
> > On Tue, 14 Jun 2011, Tony Lindgren wrote:
> > 
> > > * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
> > > > +
> > > > +   for_each_tag(atag, atag_list) {
> > > > +           if (atag->hdr.tag == ATAG_CMDLINE) {
> > > > +                   setprop_string(dt, "/chosen", "bootargs",
> > > > +                                     atag->u.cmdline.cmdline);
> > > > +           } else if (atag->hdr.tag == ATAG_MEM) {
> > > > +                   uint32_t mem_reg_property[2];
> > > > +                   mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> > > > +                   mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> > > > +                   setprop(dt, "/memory", "reg", mem_reg_property,
> > > > +                              sizeof(mem_reg_property));
> > > > +           } else if (atag->hdr.tag == ATAG_INITRD2) {
> > > > +                   uint32_t initrd_start, initrd_size;
> > > > +                   initrd_start = atag->u.initrd.start;
> > > > +                   initrd_size = atag->u.initrd.size;
> > > > +                   setprop_cell(dt, "/chosen", "linux,initrd-start",
> > > > +                                   initrd_start);
> > > > +                   setprop_cell(dt, "/chosen", "linux,initrd-end",
> > > > +                                   initrd_start + initrd_size);
> > > > +           }
> > > > +   }
> > > 
> > > I think Russell posted a complete list of the ATAGs to translate
> > > somewhere, but at least ATAG_REVISION is missing here. That's being
> > > used quite a bit as system_rev to set things dynamically.
> > 
> > No problem.  This is a work in progress.  We still can test the concept 
> > and fine tune the actual set of ATAGs being translated.
> 
> Let's talk about the revision field now, because it doesn't have a
> direct correspondence in existing attributes.
> 
> Functionality-wise, this would probably be the "compatible" property
> of the root node, with its most specific name to match, but that's not
> trivial to generate.
> 
> In some cases, the system revisions have significant differences in their
> devices (why else would you care about the revision), which means that you
> actually need a different device tree per revision anyway. If that's the
> common case, we could just ignore it completely or instead have a way
> to choose a specific device tree from a list based on the revision.

I don't think this is all that common.  Almost half of the cases, the 
system_rev variable is assigned a specific value from direct probing, 
not from the ATAG_REVISION tag.  So there are only a few cases to 
consider.

Yet it is used only for a few things, which doesn't indicate the system 
differences are that significant in practice.

If we were to use this revision number, which appears to be completely 
arbitrary from one board to another, in order to select the appropriate 
DTB amongst many possibilities, then we'll need something in the DTB 
that can be correlated to this revision number in the first place.

And let's remember that this is only about some compatibility 
convenience for legacy systems.  I really don't want this to be 
over engineered.

> Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL
> into a string and put them into the root "serial-number" property that
> is fairly easy to do, but would be harder to parse if you actually rely
> on specific values.

At some point, if some boards do use the ATAG to let the bootloader 
specify the board revision, that must be because this revision number 
can't be probed by software.  If this can be probed by software, then 
let's do it with kernel code instead.

Otherwise, if the revision number is effectively non probable, then I 
would guess it is the device tree's purpose to carry that information 
somehow, right?  Maybe this can be appended to the board name string?


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[David Brown]

On Tue, Jun 14 2011, Nicolas Pitre wrote:

> +		} else if (atag->hdr.tag == ATAG_MEM) {
> +			uint32_t mem_reg_property[2];
> +			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> +			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> +			setprop(dt, "/memory", "reg", mem_reg_property,
> +			           sizeof(mem_reg_property));

Hopefully, I'll be able to get zImages working on MSM8x60 today, and I
can test this.  I think the above might need some work to handle
multiple ATAG_MEM attributes (our memory isn't contiguous).  I can look
at this once I get it working.

Does this mean that merging appended DTB and atags requires using a
compressed kernel?  It seems that several years ago, someone tested the
decompressor on a slow MSM and decided to boot from the uncompressed
image, and nobody has bothered with the compressed image.  I don't think
it is a problem to require zImage for this case, though.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Arnd Bergmann]

On Tuesday 14 June 2011 23:21:52 Nicolas Pitre wrote:
> Otherwise, if the revision number is effectively non probable, then I 
> would guess it is the device tree's purpose to carry that information 
> somehow, right?  Maybe this can be appended to the board name string?

That's what I meant with adding it to "compatible", which is a list of
strings with varying degrees of detailed information, e.g.

  ti,omap7
  ti,omap7-squirrelboard
  ti,omap7-squirrelboard-v3
  ti,omap7-squirrelboard-v3.17b

You can match the list against a specific revision or a less specific
identifier if you just want to know whether you are on a squirrelboard
or a hamsterboard.

	Arnd

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Grant Likely]

On Tue, Jun 14, 2011 at 3:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 14 June 2011 23:21:52 Nicolas Pitre wrote:
>> Otherwise, if the revision number is effectively non probable, then I
>> would guess it is the device tree's purpose to carry that information
>> somehow, right? ?Maybe this can be appended to the board name string?
>
> That's what I meant with adding it to "compatible", which is a list of
> strings with varying degrees of detailed information, e.g.
>
> ?ti,omap7
> ?ti,omap7-squirrelboard
> ?ti,omap7-squirrelboard-v3
> ?ti,omap7-squirrelboard-v3.17b
>
> You can match the list against a specific revision or a less specific
> identifier if you just want to know whether you are on a squirrelboard
> or a hamsterboard.

Yes, that's generally the right way to handle it.  In practice I've
not seen many cases where it is really required, but it doesn't hurt
if somebody wants to include it in the DT for their board.

g.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Rob Herring]

On 06/14/2011 03:32 PM, Arnd Bergmann wrote:
> On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote:
>> On Tue, 14 Jun 2011, Tony Lindgren wrote:
>>
>>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
>>>> +
>>>> +   for_each_tag(atag, atag_list) {
>>>> +           if (atag->hdr.tag == ATAG_CMDLINE) {
>>>> +                   setprop_string(dt, "/chosen", "bootargs",
>>>> +                                     atag->u.cmdline.cmdline);
>>>> +           } else if (atag->hdr.tag == ATAG_MEM) {
>>>> +                   uint32_t mem_reg_property[2];
>>>> +                   mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
>>>> +                   mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
>>>> +                   setprop(dt, "/memory", "reg", mem_reg_property,
>>>> +                              sizeof(mem_reg_property));
>>>> +           } else if (atag->hdr.tag == ATAG_INITRD2) {
>>>> +                   uint32_t initrd_start, initrd_size;
>>>> +                   initrd_start = atag->u.initrd.start;
>>>> +                   initrd_size = atag->u.initrd.size;
>>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-start",
>>>> +                                   initrd_start);
>>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-end",
>>>> +                                   initrd_start + initrd_size);
>>>> +           }
>>>> +   }
>>>
>>> I think Russell posted a complete list of the ATAGs to translate
>>> somewhere, but at least ATAG_REVISION is missing here. That's being
>>> used quite a bit as system_rev to set things dynamically.
>>
>> No problem.  This is a work in progress.  We still can test the concept 
>> and fine tune the actual set of ATAGs being translated.
> 
> Let's talk about the revision field now, because it doesn't have a
> direct correspondence in existing attributes.
> 
> Functionality-wise, this would probably be the "compatible" property
> of the root node, with its most specific name to match, but that's not
> trivial to generate.
> 
> In some cases, the system revisions have significant differences in their
> devices (why else would you care about the revision), which means that you
> actually need a different device tree per revision anyway. If that's the
> common case, we could just ignore it completely or instead have a way
> to choose a specific device tree from a list based on the revision.
> 
> Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL
> into a string and put them into the root "serial-number" property that
> is fairly easy to do, but would be harder to parse if you actually rely
> on specific values.
> 

IIRC, system_rev at least can be specified on the kernel command line,
so you could just append the cmd line. However, if you force the
built-in command line to be used that would not work,

Rob

[PATCH] Support multiple MEM tags with atags->fdt conversion

[David Brown]

Some targets have multiple memory regions.  Parse up to 8 of these
when converting the atags to fdt.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
With this change, I am able to boot MSM8x60 combining ATAGS and my DT.
It seems to work as long as my device tree has placeholders for all of
the properties I need.

It still seems rather clunky, especially that it requires bootimg from
a zImage.

David

 arch/arm/boot/compressed/atags_to_fdt.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c
index 11c1a88..ac93e28 100644
--- a/arch/arm/boot/compressed/atags_to_fdt.c
+++ b/arch/arm/boot/compressed/atags_to_fdt.c
@@ -31,6 +31,8 @@ static int setprop_cell(void *fdt, const char *node_path,
 int atags_to_fdt(void *dt,  void *atag_list)
 {
 	struct tag *atag = atag_list;
+	uint32_t mem_reg_property[16];
+	int memcount = 0;
 
 	/* make sure we've got an aligned pointer */
 	if ((u32)atag_list & 0x3)
@@ -51,11 +53,10 @@ int atags_to_fdt(void *dt,  void *atag_list)
 			setprop_string(dt, "/chosen", "bootargs",
 			                  atag->u.cmdline.cmdline);
 		} else if (atag->hdr.tag == ATAG_MEM) {
-			uint32_t mem_reg_property[2];
-			mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
-			mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
-			setprop(dt, "/memory", "reg", mem_reg_property,
-			           sizeof(mem_reg_property));
+			if (memcount >= sizeof(mem_reg_property)/sizeof(uint32_t))
+				continue;
+			mem_reg_property[memcount++] = cpu_to_fdt32(atag->u.mem.start);
+			mem_reg_property[memcount++] = cpu_to_fdt32(atag->u.mem.size);
 		} else if (atag->hdr.tag == ATAG_INITRD2) {
 			uint32_t initrd_start, initrd_size;
 			initrd_start = atag->u.initrd.start;
@@ -67,5 +68,10 @@ int atags_to_fdt(void *dt,  void *atag_list)
 		}
 	}
 
+	if (memcount) {
+		setprop(dt, "/memory", "reg", mem_reg_property,
+				4*memcount);
+	}
+
 	return 0;
 }
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Tue, 14 Jun 2011, Rob Herring wrote:

> On 06/14/2011 03:32 PM, Arnd Bergmann wrote:
> > On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote:
> >> On Tue, 14 Jun 2011, Tony Lindgren wrote:
> >>
> >>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
> >>>> +
> >>>> +   for_each_tag(atag, atag_list) {
> >>>> +           if (atag->hdr.tag == ATAG_CMDLINE) {
> >>>> +                   setprop_string(dt, "/chosen", "bootargs",
> >>>> +                                     atag->u.cmdline.cmdline);
> >>>> +           } else if (atag->hdr.tag == ATAG_MEM) {
> >>>> +                   uint32_t mem_reg_property[2];
> >>>> +                   mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> >>>> +                   mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> >>>> +                   setprop(dt, "/memory", "reg", mem_reg_property,
> >>>> +                              sizeof(mem_reg_property));
> >>>> +           } else if (atag->hdr.tag == ATAG_INITRD2) {
> >>>> +                   uint32_t initrd_start, initrd_size;
> >>>> +                   initrd_start = atag->u.initrd.start;
> >>>> +                   initrd_size = atag->u.initrd.size;
> >>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-start",
> >>>> +                                   initrd_start);
> >>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-end",
> >>>> +                                   initrd_start + initrd_size);
> >>>> +           }
> >>>> +   }
> >>>
> >>> I think Russell posted a complete list of the ATAGs to translate
> >>> somewhere, but at least ATAG_REVISION is missing here. That's being
> >>> used quite a bit as system_rev to set things dynamically.
> >>
> >> No problem.  This is a work in progress.  We still can test the concept 
> >> and fine tune the actual set of ATAGs being translated.
> > 
> > Let's talk about the revision field now, because it doesn't have a
> > direct correspondence in existing attributes.
> > 
> > Functionality-wise, this would probably be the "compatible" property
> > of the root node, with its most specific name to match, but that's not
> > trivial to generate.
> > 
> > In some cases, the system revisions have significant differences in their
> > devices (why else would you care about the revision), which means that you
> > actually need a different device tree per revision anyway. If that's the
> > common case, we could just ignore it completely or instead have a way
> > to choose a specific device tree from a list based on the revision.
> > 
> > Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL
> > into a string and put them into the root "serial-number" property that
> > is fairly easy to do, but would be harder to parse if you actually rely
> > on specific values.
> > 
> 
> IIRC, system_rev at least can be specified on the kernel command line,
> so you could just append the cmd line. However, if you force the
> built-in command line to be used that would not work,

If you do that then you probably don't care as much about translating 
existing ATAGs from the bootloader and could as well just live with the 
hardcoded values in the appended DTB.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Rob Herring]

On 06/14/2011 06:50 PM, Nicolas Pitre wrote:
> On Tue, 14 Jun 2011, Rob Herring wrote:
> 
>> On 06/14/2011 03:32 PM, Arnd Bergmann wrote:
>>> On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote:
>>>> On Tue, 14 Jun 2011, Tony Lindgren wrote:
>>>>
>>>>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
>>>>>> +
>>>>>> +   for_each_tag(atag, atag_list) {
>>>>>> +           if (atag->hdr.tag == ATAG_CMDLINE) {
>>>>>> +                   setprop_string(dt, "/chosen", "bootargs",
>>>>>> +                                     atag->u.cmdline.cmdline);
>>>>>> +           } else if (atag->hdr.tag == ATAG_MEM) {
>>>>>> +                   uint32_t mem_reg_property[2];
>>>>>> +                   mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
>>>>>> +                   mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
>>>>>> +                   setprop(dt, "/memory", "reg", mem_reg_property,
>>>>>> +                              sizeof(mem_reg_property));
>>>>>> +           } else if (atag->hdr.tag == ATAG_INITRD2) {
>>>>>> +                   uint32_t initrd_start, initrd_size;
>>>>>> +                   initrd_start = atag->u.initrd.start;
>>>>>> +                   initrd_size = atag->u.initrd.size;
>>>>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-start",
>>>>>> +                                   initrd_start);
>>>>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-end",
>>>>>> +                                   initrd_start + initrd_size);
>>>>>> +           }
>>>>>> +   }
>>>>>
>>>>> I think Russell posted a complete list of the ATAGs to translate
>>>>> somewhere, but at least ATAG_REVISION is missing here. That's being
>>>>> used quite a bit as system_rev to set things dynamically.
>>>>
>>>> No problem.  This is a work in progress.  We still can test the concept 
>>>> and fine tune the actual set of ATAGs being translated.
>>>
>>> Let's talk about the revision field now, because it doesn't have a
>>> direct correspondence in existing attributes.
>>>
>>> Functionality-wise, this would probably be the "compatible" property
>>> of the root node, with its most specific name to match, but that's not
>>> trivial to generate.
>>>
>>> In some cases, the system revisions have significant differences in their
>>> devices (why else would you care about the revision), which means that you
>>> actually need a different device tree per revision anyway. If that's the
>>> common case, we could just ignore it completely or instead have a way
>>> to choose a specific device tree from a list based on the revision.
>>>
>>> Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL
>>> into a string and put them into the root "serial-number" property that
>>> is fairly easy to do, but would be harder to parse if you actually rely
>>> on specific values.
>>>
>>
>> IIRC, system_rev at least can be specified on the kernel command line,
>> so you could just append the cmd line. However, if you force the
>> built-in command line to be used that would not work,
> 
> If you do that then you probably don't care as much about translating 
> existing ATAGs from the bootloader and could as well just live with the 
> hardcoded values in the appended DTB.
> 

What I meant was you could translate these ATAGs into the DTB kernel
command-line options rather than come-up with something new. You already
having to mess with the command line.

Rob

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Tue, 14 Jun 2011, Rob Herring wrote:

> On 06/14/2011 06:50 PM, Nicolas Pitre wrote:
> > On Tue, 14 Jun 2011, Rob Herring wrote:
> > 
> >> On 06/14/2011 03:32 PM, Arnd Bergmann wrote:
> >>> On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote:
> >>>> On Tue, 14 Jun 2011, Tony Lindgren wrote:
> >>>>
> >>>>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]:
> >>>>>> +
> >>>>>> +   for_each_tag(atag, atag_list) {
> >>>>>> +           if (atag->hdr.tag == ATAG_CMDLINE) {
> >>>>>> +                   setprop_string(dt, "/chosen", "bootargs",
> >>>>>> +                                     atag->u.cmdline.cmdline);
> >>>>>> +           } else if (atag->hdr.tag == ATAG_MEM) {
> >>>>>> +                   uint32_t mem_reg_property[2];
> >>>>>> +                   mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start);
> >>>>>> +                   mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size);
> >>>>>> +                   setprop(dt, "/memory", "reg", mem_reg_property,
> >>>>>> +                              sizeof(mem_reg_property));
> >>>>>> +           } else if (atag->hdr.tag == ATAG_INITRD2) {
> >>>>>> +                   uint32_t initrd_start, initrd_size;
> >>>>>> +                   initrd_start = atag->u.initrd.start;
> >>>>>> +                   initrd_size = atag->u.initrd.size;
> >>>>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-start",
> >>>>>> +                                   initrd_start);
> >>>>>> +                   setprop_cell(dt, "/chosen", "linux,initrd-end",
> >>>>>> +                                   initrd_start + initrd_size);
> >>>>>> +           }
> >>>>>> +   }
> >>>>>
> >>>>> I think Russell posted a complete list of the ATAGs to translate
> >>>>> somewhere, but at least ATAG_REVISION is missing here. That's being
> >>>>> used quite a bit as system_rev to set things dynamically.
> >>>>
> >>>> No problem.  This is a work in progress.  We still can test the concept 
> >>>> and fine tune the actual set of ATAGs being translated.
> >>>
> >>> Let's talk about the revision field now, because it doesn't have a
> >>> direct correspondence in existing attributes.
> >>>
> >>> Functionality-wise, this would probably be the "compatible" property
> >>> of the root node, with its most specific name to match, but that's not
> >>> trivial to generate.
> >>>
> >>> In some cases, the system revisions have significant differences in their
> >>> devices (why else would you care about the revision), which means that you
> >>> actually need a different device tree per revision anyway. If that's the
> >>> common case, we could just ignore it completely or instead have a way
> >>> to choose a specific device tree from a list based on the revision.
> >>>
> >>> Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL
> >>> into a string and put them into the root "serial-number" property that
> >>> is fairly easy to do, but would be harder to parse if you actually rely
> >>> on specific values.
> >>>
> >>
> >> IIRC, system_rev at least can be specified on the kernel command line,
> >> so you could just append the cmd line. However, if you force the
> >> built-in command line to be used that would not work,
> > 
> > If you do that then you probably don't care as much about translating 
> > existing ATAGs from the bootloader and could as well just live with the 
> > hardcoded values in the appended DTB.
> > 
> 
> What I meant was you could translate these ATAGs into the DTB kernel
> command-line options rather than come-up with something new. You already
> having to mess with the command line.

Agreed.  What I meant is that you're unlikely to use the kernel built-in 
command line if you care about this.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Tony Lindgren]

* Grant Likely <grant.likely@secretlab.ca> [110614 15:02]:
> On Tue, Jun 14, 2011 at 3:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 14 June 2011 23:21:52 Nicolas Pitre wrote:
> >> Otherwise, if the revision number is effectively non probable, then I
> >> would guess it is the device tree's purpose to carry that information
> >> somehow, right? ?Maybe this can be appended to the board name string?
> >
> > That's what I meant with adding it to "compatible", which is a list of
> > strings with varying degrees of detailed information, e.g.
> >
> > ?ti,omap7
> > ?ti,omap7-squirrelboard
> > ?ti,omap7-squirrelboard-v3
> > ?ti,omap7-squirrelboard-v3.17b
> >
> > You can match the list against a specific revision or a less specific
> > identifier if you just want to know whether you are on a squirrelboard
> > or a hamsterboard.
> 
> Yes, that's generally the right way to handle it.  In practice I've
> not seen many cases where it is really required, but it doesn't hurt
> if somebody wants to include it in the DT for their board.

Appending the ATAG_REVISION to "compatible" and also setting system_rev
to ATAG_REVISION like we already do should work.

Just to clarify things abit: We just can't generate .dts files for all
existing hardware. It would require dumping out system_rev and other ATAGs
on tens or hundeds of pieces of hardware.

Regards,

Tony

[PATCH] Support multiple MEM tags with atags->fdt conversion

[Nicolas Pitre]

On Tue, 14 Jun 2011, David Brown wrote:

> Some targets have multiple memory regions.  Parse up to 8 of these
> when converting the atags to fdt.
> 
> Signed-off-by: David Brown <davidb@codeaurora.org>

I've added your patch to my zImage patch queue.

> With this change, I am able to boot MSM8x60 combining ATAGS and my DT.
> It seems to work as long as my device tree has placeholders for all of
> the properties I need.

I think those missing nodes should simply be created if missing.  I 
however don't see any function in the libfdt API to do that -- there is 
fdt_add_subnode() but no fdt_add_node().  Any DT expert please?

Also, should the DTB be enlarged in order to add new nodes, or even 
modify existing ones with larger properties?  In other words, 
should something like fdt_move(fdt, fdt, fdt_totalsize(fdt)+HEADROOM) be 
used beforehand?


Nicolas

[PATCH] Support multiple MEM tags with atags->fdt conversion

[David Brown]

On Wed, Jun 15 2011, Nicolas Pitre wrote:

> On Tue, 14 Jun 2011, David Brown wrote:
>
>> Some targets have multiple memory regions.  Parse up to 8 of these
>> when converting the atags to fdt.
>> 
>> Signed-off-by: David Brown <davidb@codeaurora.org>
>
> I've added your patch to my zImage patch queue.
>
>> With this change, I am able to boot MSM8x60 combining ATAGS and my DT.
>> It seems to work as long as my device tree has placeholders for all of
>> the properties I need.
>
> I think those missing nodes should simply be created if missing.  I 
> however don't see any function in the libfdt API to do that -- there is 
> fdt_add_subnode() but no fdt_add_node().  Any DT expert please?

It does seem to only work if I already have nodes that are large enough
to hold the properties coming from the ATAG.  For example, I have to
have an array of memory reg values large enough, as well as a command
line large enough to hold the one passed from the bootloader.

If the DTB's command line is smaller than the one from ATAG, it just
silently leaves the DTB one in place.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] Support multiple MEM tags with atags->fdt conversion

[Nicolas Pitre]

On Wed, 15 Jun 2011, David Brown wrote:

> On Wed, Jun 15 2011, Nicolas Pitre wrote:
> 
> > On Tue, 14 Jun 2011, David Brown wrote:
> >
> >> Some targets have multiple memory regions.  Parse up to 8 of these
> >> when converting the atags to fdt.
> >> 
> >> Signed-off-by: David Brown <davidb@codeaurora.org>
> >
> > I've added your patch to my zImage patch queue.
> >
> >> With this change, I am able to boot MSM8x60 combining ATAGS and my DT.
> >> It seems to work as long as my device tree has placeholders for all of
> >> the properties I need.
> >
> > I think those missing nodes should simply be created if missing.  I 
> > however don't see any function in the libfdt API to do that -- there is 
> > fdt_add_subnode() but no fdt_add_node().  Any DT expert please?
> 
> It does seem to only work if I already have nodes that are large enough
> to hold the properties coming from the ATAG.  For example, I have to
> have an array of memory reg values large enough, as well as a command
> line large enough to hold the one passed from the bootloader.
> 
> If the DTB's command line is smaller than the one from ATAG, it just
> silently leaves the DTB one in place.

That I can explain, and that looks easy to fix.


Nicolas

[PATCH] Support multiple MEM tags with atags->fdt conversion

[David Gibson]

On Wed, Jun 15, 2011 at 03:50:37PM -0400, Nicolas Pitre wrote:
> On Tue, 14 Jun 2011, David Brown wrote:
> 
> > Some targets have multiple memory regions.  Parse up to 8 of these
> > when converting the atags to fdt.
> > 
> > Signed-off-by: David Brown <davidb@codeaurora.org>
> 
> I've added your patch to my zImage patch queue.
> 
> > With this change, I am able to boot MSM8x60 combining ATAGS and my DT.
> > It seems to work as long as my device tree has placeholders for all of
> > the properties I need.
> 
> I think those missing nodes should simply be created if missing.  I 
> however don't see any function in the libfdt API to do that -- there is 
> fdt_add_subnode() but no fdt_add_node().  Any DT expert please?

fdt_add_subnode() will create a new node as you require.  The
"subnode" is just supposed to indicate that the parameters are in the
form of the offset of the parent node and the new node's immediate
name, rather than taking a full path name for the new node.

> Also, should the DTB be enlarged in order to add new nodes, or even 
> modify existing ones with larger properties?  In other words, 
> should something like fdt_move(fdt, fdt, fdt_totalsize(fdt)+HEADROOM) be 
> used beforehand?

Yes, you will need to do this, fdt_open_into() is the function you
want.  Without making room first, adding nodes or expanding existing
properties will return -FDT_ERR_NOSPACE.  Once you've made whatever
edits you need, you can use fdt_pack() to collapse the tree back to
its minimum size.

Although this is somewhat awkward, this approach is a deliberate
design decision, to avoid making libfdt dependent on a working general
purpose allocator, which may not be available when libfdt is used in
very limited environments such as a firmware/bootloader.

If you do find things you need to do which libfdt doesn't support, I'm
more than happy to extend it as necessary.

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

[PATCH] Support multiple MEM tags with atags->fdt conversion

[David Brown]

On Wed, Jun 15 2011, Nicolas Pitre wrote:

> I think those missing nodes should simply be created if missing.  I 
> however don't see any function in the libfdt API to do that -- there is 
> fdt_add_subnode() but no fdt_add_node().  Any DT expert please?
>
> Also, should the DTB be enlarged in order to add new nodes, or even 
> modify existing ones with larger properties?  In other words, 
> should something like fdt_move(fdt, fdt, fdt_totalsize(fdt)+HEADROOM) be 
> used beforehand?

I've tried adding some padding to my DTB by passing '-p 1024'.  It does
seem to try and do something, but I'm getting a fault in memmove() from
fdt_splice() that I'm still trying to track down.

It looks like the expansion is possible, though, at least once
everything is right.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH] Support multiple MEM tags with atags->fdt conversion

[Nicolas Pitre]

On Thu, 16 Jun 2011, David Gibson wrote:

> On Wed, Jun 15, 2011 at 03:50:37PM -0400, Nicolas Pitre wrote:
> > On Tue, 14 Jun 2011, David Brown wrote:
> > 
> > > Some targets have multiple memory regions.  Parse up to 8 of these
> > > when converting the atags to fdt.
> > > 
> > > Signed-off-by: David Brown <davidb@codeaurora.org>
> > 
> > I've added your patch to my zImage patch queue.
> > 
> > > With this change, I am able to boot MSM8x60 combining ATAGS and my DT.
> > > It seems to work as long as my device tree has placeholders for all of
> > > the properties I need.
> > 
> > I think those missing nodes should simply be created if missing.  I 
> > however don't see any function in the libfdt API to do that -- there is 
> > fdt_add_subnode() but no fdt_add_node().  Any DT expert please?
> 
> fdt_add_subnode() will create a new node as you require.  The
> "subnode" is just supposed to indicate that the parameters are in the
> form of the offset of the parent node and the new node's immediate
> name, rather than taking a full path name for the new node.

Sure... but I'm still a total nincompoop with regard to FDT.

Let's suppose I do:

	offset = fdt_path_offset(fdt, "/memory");

but that returns -FDT_ERR_NOTFOUND.  Now what?

If I look at the documentation for fdt_add_subnode():

/**
 * fdt_add_subnode - creates a new node
 * @fdt: pointer to the device tree blob
 * @parentoffset: structure block offset of a node
 * @name: name of the subnode to locate
 * [...]

I have no node offset, and can't find the offset for the parent of an 
nonexistent node.  Should I use 0 for parentoffset here?  I'm guessing 
that "/memory" is at the top level so there is just no parent.

> > Also, should the DTB be enlarged in order to add new nodes, or even 
> > modify existing ones with larger properties?  In other words, 
> > should something like fdt_move(fdt, fdt, fdt_totalsize(fdt)+HEADROOM) be 
> > used beforehand?
> 
> Yes, you will need to do this, fdt_open_into() is the function you
> want.  Without making room first, adding nodes or expanding existing
> properties will return -FDT_ERR_NOSPACE.  Once you've made whatever
> edits you need, you can use fdt_pack() to collapse the tree back to
> its minimum size.

Excellent!

FRom a quick code inspection, fdt_open_into() appears to be fine with 
both the source and destination pointers being the same, right?

> Although this is somewhat awkward, this approach is a deliberate
> design decision, to avoid making libfdt dependent on a working general
> purpose allocator, which may not be available when libfdt is used in
> very limited environments such as a firmware/bootloader.

This is perfect.  The environment where I want to use this code is 
fairly limited indeed.


Nicolas

[PATCH] Support multiple MEM tags with atags->fdt conversion

[David Gibson]

On Mon, Jun 20, 2011 at 12:03:21AM -0400, Nicolas Pitre wrote:
> On Thu, 16 Jun 2011, David Gibson wrote:
> 
> > On Wed, Jun 15, 2011 at 03:50:37PM -0400, Nicolas Pitre wrote:
> > > On Tue, 14 Jun 2011, David Brown wrote:
> > > 
> > > > Some targets have multiple memory regions.  Parse up to 8 of these
> > > > when converting the atags to fdt.
> > > > 
> > > > Signed-off-by: David Brown <davidb@codeaurora.org>
> > > 
> > > I've added your patch to my zImage patch queue.
> > > 
> > > > With this change, I am able to boot MSM8x60 combining ATAGS and my DT.
> > > > It seems to work as long as my device tree has placeholders for all of
> > > > the properties I need.
> > > 
> > > I think those missing nodes should simply be created if missing.  I 
> > > however don't see any function in the libfdt API to do that -- there is 
> > > fdt_add_subnode() but no fdt_add_node().  Any DT expert please?
> > 
> > fdt_add_subnode() will create a new node as you require.  The
> > "subnode" is just supposed to indicate that the parameters are in the
> > form of the offset of the parent node and the new node's immediate
> > name, rather than taking a full path name for the new node.
> 
> Sure... but I'm still a total nincompoop with regard to FDT.
> 
> Let's suppose I do:
> 
> 	offset = fdt_path_offset(fdt, "/memory");
> 
> but that returns -FDT_ERR_NOTFOUND.  Now what?
> 
> If I look at the documentation for fdt_add_subnode():
> 
> /**
>  * fdt_add_subnode - creates a new node
>  * @fdt: pointer to the device tree blob
>  * @parentoffset: structure block offset of a node
>  * @name: name of the subnode to locate
>  * [...]
> 
> I have no node offset, and can't find the offset for the parent of an 
> nonexistent node.  Should I use 0 for parentoffset here?  I'm guessing 
> that "/memory" is at the top level so there is just no parent.

Ah! I see the source of your confusion.  In this case the parent node
is the root node, and the root node always has offset 0.  I guess this
needs to be better documented, although I'm not immediately sure where.

> > > Also, should the DTB be enlarged in order to add new nodes, or even 
> > > modify existing ones with larger properties?  In other words, 
> > > should something like fdt_move(fdt, fdt, fdt_totalsize(fdt)+HEADROOM) be 
> > > used beforehand?
> > 
> > Yes, you will need to do this, fdt_open_into() is the function you
> > want.  Without making room first, adding nodes or expanding existing
> > properties will return -FDT_ERR_NOSPACE.  Once you've made whatever
> > edits you need, you can use fdt_pack() to collapse the tree back to
> > its minimum size.
> 
> Excellent!
> 
> FRom a quick code inspection, fdt_open_into() appears to be fine with 
> both the source and destination pointers being the same, right?

Yes, that should be fine.  It should also be fine with the new buffer
partially overlapping the existing tree.

> > Although this is somewhat awkward, this approach is a deliberate
> > design decision, to avoid making libfdt dependent on a working general
> > purpose allocator, which may not be available when libfdt is used in
> > very limited environments such as a firmware/bootloader.
> 
> This is perfect.  The environment where I want to use this code is 
> fairly limited indeed.

*smiles smugly*

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

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[David Gibson]

On Sun, Jun 12, 2011 at 01:22:19PM +0200, Petr ?tetiar wrote:
> Shawn Guo <shawn.guo@freescale.com> [2011-06-12 16:34:15]:
> 
> > On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote:
> > > 
> > > One thing which has been bugging me for some time is that the DT stuff
> > > completely overrides the ATAGs.  This is wrong with solutions like this.
> > > 
> > > We have a set of perfectly good boot loaders which provide correct
> > > information to the kernel via ATAGs.  If we start moving everything
> > > over to DT, then we run into a problem because the ATAGs are ignored -
> > > stuff such as the RAM size and command line passed from the boot loader
> > > will be entirely ignored, instead these having to be encoded into the
> > > kernel at build time.
> > > 
> > What u-boot does right now is replacing the parameters in dtb with
> > its for those it's interested in, for example command line is one,
> > before it passes dtb to kernel.
> 
> If I understand it all correctly, there must be some 'legacy bootloader
> support' added to the kernel, layer which would convert the ATAGs to the DT.
> Or something like that.

More likely combine the ATAGs information with a skeleton DT, but yes,
you probably want something like this.  On powerpc, this role is
usually taken by the zImage wrapper code, sitting between the platform
firmware and the kernel proper.  However, there's no inherent reason
it couldn't be early code in the vmlinux instead.  Just as long as
there is a well defined point after which the DT information is
authoritative.

> Please note, that there are some boards out there, which use some kind of
> proprietary bootloaders, so you can't update or change them easily. You either
> don't have source code, you're limited by the available space (2KB eeprom) so
> you can't add much more things into that bootloader or you're limited by the
> size of the MBR (yes, even such bootloaders exists).

Right.  Sams situation on powerpc - and indeed there are lots of
crappy proprietary bootloaders each with their own whacky way of
conveying the scraps of information they give you.  For this reason we
have a variety of platform-specific bootwrappers which handle these
bootloaders, transforming their information into DT form for the
kernel proper to consume.

No reason you can't do the same thing on ARM, whether the glue
wrappers are wrappers as such, or special early pieces of kernel code.

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

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [110613 13:41]:
> On Sun, 12 Jun 2011, Grant Likely wrote:
> 
> > On Sun, Jun 12, 2011 at 12:06 AM, Nicolas Pitre
> > <nicolas.pitre@linaro.org> wrote:
> > > This is a resend of those patches with fixups after the latest changes
> > > in mainline.
> > >
> > > [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> > > This one is new and trivial.
> > >
> > > [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> > > Mostly John Bonesio's version with some adjustments and cleanups.
> > >
> > > [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> > > New, simpler alternative to Tony Lindgren's version.
> > 
> > I've gone ahead and added the series to devicetree/test for anyone who
> > want to play with it.
> 
> I've published those patches with all the latest fixes and ACKs here:
> 
> 	git://git.linaro.org/people/nico/linux.git zImage_DTB_append

Ping, any news on getting these merged?

Tony

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Nicolas Pitre]

On Mon, 5 Sep 2011, Tony Lindgren wrote:

> * Nicolas Pitre <nicolas.pitre@linaro.org> [110613 13:41]:
> > On Sun, 12 Jun 2011, Grant Likely wrote:
> > 
> > > On Sun, Jun 12, 2011 at 12:06 AM, Nicolas Pitre
> > > <nicolas.pitre@linaro.org> wrote:
> > > > This is a resend of those patches with fixups after the latest changes
> > > > in mainline.
> > > >
> > > > [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
> > > > This one is new and trivial.
> > > >
> > > > [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
> > > > Mostly John Bonesio's version with some adjustments and cleanups.
> > > >
> > > > [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss
> > > > New, simpler alternative to Tony Lindgren's version.
> > > 
> > > I've gone ahead and added the series to devicetree/test for anyone who
> > > want to play with it.
> > 
> > I've published those patches with all the latest fixes and ACKs here:
> > 
> > 	git://git.linaro.org/people/nico/linux.git zImage_DTB_append
> 
> Ping, any news on getting these merged?

I should come around to it once my current memory.h cleanup work is in 
shape. This might depend on some new Kbuild facility from Arnaud Lacombe 
I've yet to test.


Nicolas

[PATCH 0/3] patches to allow DTB to be appended to the ARM zImage

[Linus Walleij]

On Tue, Jun 14, 2011 at 2:13 AM, David Brown <davidb@codeaurora.org> wrote:
> On Sun, Jun 12 2011, Nicolas Pitre wrote:
>
>> It is not pointless. ?Never it was in the cards to convert old platforms
>> first. ?The most problematic platforms responsible for the high volume
>> of changes that has gone in mainline lately are the modern ones, not PXA
>> based ones. ?So DT is going to be most effective for the new stuff to
>> come. ?And realistically that's where we might get developers attention,
>> not with those platforms being end-of-lived by their manufacturers.
>
> New stuff doesn't necessarily mean new boot loaders. ?I don't have a lot
> of control over the bootloaders on MSM. ?We can probably move away from
> ATAGS, but only after every kernel that could possibly boot on that
> target will work with DT. ?I don't see that happening for at least a
> year.

Having now inspected the situation with Ux500 I can second that this
goes for the Ux500 series as well.

The U8500 firstcomer in this series and probably U5500 also is
relying on a 2009-based fork of U-boot. The U-boot for these
platforms just won't be converted to do DT.

In short we also need the ATAGs-overrides-DT-nodes approach
for a clean migration path.

Yours,
Linus Walleij