[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Timur Tabi]

The device tree (fdt) must always exist in within the bootmap (usually the
first 16MB of RAM).  If it doesn't, then boot_relocate_fdt() will allocate an
LMB region in the bootmap and copy the fdt into that region.  It will also
increase the size of the fdt.

If the fdt is already in the bootmap, then previously the memory was just
reserved.  There was no contingency if the reservation failed, however.

By always allocating an lmb region and copying/resizing the fdt into that
region, the code is simplified and the memory region is always allocated
properly.

Also change the types of some variables to avoid some typecasts.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 common/image.c |   83 +++++++++++++++++++-------------------------------------
 1 files changed, 28 insertions(+), 55 deletions(-)

diff --git a/common/image.c b/common/image.c
index 8d4be14..e19732c 100644
--- a/common/image.c
+++ b/common/image.c
@@ -1170,8 +1170,10 @@ static int fit_check_fdt (const void *fit, int fdt_noffset, int verify)
  * @of_flat_tree: pointer to a char* variable, will hold fdt start address
  * @of_size: pointer to a ulong variable, will hold fdt length
  *
- * boot_relocate_fdt() determines if the of_flat_tree address is within
- * the bootmap and if not relocates it into that region
+ * boot_relocate_fdt() allocates a region of memory within the bootmap and
+ * relocates the of_flat_tree into that region, even if the fdt is already in
+ * the bootmap.  It also expands the size of the fdt by CONFIG_SYS_FDT_PAD
+ * bytes.
  *
  * of_flat_tree and of_size are set to final (after relocation) values
  *
@@ -1182,9 +1184,10 @@ static int fit_check_fdt (const void *fit, int fdt_noffset, int verify)
 int boot_relocate_fdt (struct lmb *lmb, ulong bootmap_base,
 		char **of_flat_tree, ulong *of_size)
 {
-	char	*fdt_blob = *of_flat_tree;
-	ulong	relocate = 0;
+	void	*fdt_blob = *of_flat_tree;
+	void	*of_start = 0;
 	ulong	of_len = 0;
+	int	err;
 
 	/* nothing to do */
 	if (*of_size == 0)
@@ -1195,62 +1198,32 @@ int boot_relocate_fdt (struct lmb *lmb, ulong bootmap_base,
 		goto error;
 	}
 
-#ifndef CONFIG_SYS_NO_FLASH
-	/* move the blob if it is in flash (set relocate) */
-	if (addr2info ((ulong)fdt_blob) != NULL)
-		relocate = 1;
-#endif
-
-	/*
-	 * The blob needs to be inside the boot mapping.
-	 */
-	if (fdt_blob < (char *)bootmap_base)
-		relocate = 1;
-
-	if ((fdt_blob + *of_size + CONFIG_SYS_FDT_PAD) >=
-			((char *)CONFIG_SYS_BOOTMAPSZ + bootmap_base))
-		relocate = 1;
-
-	/* move flattend device tree if needed */
-	if (relocate) {
-		int err;
-		ulong of_start = 0;
-
-		/* position on a 4K boundary before the alloc_current */
-		/* Pad the FDT by a specified amount */
-		of_len = *of_size + CONFIG_SYS_FDT_PAD;
-		of_start = (unsigned long)lmb_alloc_base(lmb, of_len, 0x1000,
-				(CONFIG_SYS_BOOTMAPSZ + bootmap_base));
-
-		if (of_start == 0) {
-			puts("device tree - allocation error\n");
-			goto error;
-		}
+	/* position on a 4K boundary before the alloc_current */
+	/* Pad the FDT by a specified amount */
+	of_len = *of_size + CONFIG_SYS_FDT_PAD;
+	of_start = (void *)lmb_alloc_base(lmb, of_len, 0x1000,
+			(CONFIG_SYS_BOOTMAPSZ + bootmap_base));
 
-		debug ("## device tree at 0x%08lX ... 0x%08lX (len=%ld=0x%lX)\n",
-			(ulong)fdt_blob, (ulong)fdt_blob + *of_size - 1,
-			of_len, of_len);
-
-		printf ("   Loading Device Tree to %08lx, end %08lx ... ",
-			of_start, of_start + of_len - 1);
+	if (of_start == 0) {
+		puts("device tree - allocation error\n");
+		goto error;
+	}
 
-		err = fdt_open_into (fdt_blob, (void *)of_start, of_len);
-		if (err != 0) {
-			fdt_error ("fdt move failed");
-			goto error;
-		}
-		puts ("OK\n");
+	debug ("## device tree at %p ... %p (len=%ld [0x%lX])\n",
+		fdt_blob, fdt_blob + *of_size - 1, of_len, of_len);
 
-		*of_flat_tree = (char *)of_start;
-		*of_size = of_len;
-	} else {
-		*of_flat_tree = fdt_blob;
-		of_len = (CONFIG_SYS_BOOTMAPSZ + bootmap_base) - (ulong)fdt_blob;
-		lmb_reserve(lmb, (ulong)fdt_blob, of_len);
-		fdt_set_totalsize(*of_flat_tree, of_len);
+	printf ("   Loading Device Tree to %p, end %p ... ",
+		of_start, of_start + of_len - 1);
 
-		*of_size = of_len;
+	err = fdt_open_into (fdt_blob, of_start, of_len);
+	if (err != 0) {
+		fdt_error ("fdt move failed");
+		goto error;
 	}
+	puts ("OK\n");
+
+	*of_flat_tree = of_start;
+	*of_size = of_len;
 
 	set_working_fdt_addr(*of_flat_tree);
 	return 0;
-- 
1.6.5

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Jerry Van Baren]

Hi Timur,

On 05/24/2010 04:10 PM, Timur Tabi wrote:
> The device tree (fdt) must always exist in within the bootmap (usually the
> first 16MB of RAM).  If it doesn't, then boot_relocate_fdt() will allocate an
> LMB region in the bootmap and copy the fdt into that region.  It will also
> increase the size of the fdt.
>
> If the fdt is already in the bootmap, then previously the memory was just
> reserved.  There was no contingency if the reservation failed, however.
>
> By always allocating an lmb region and copying/resizing the fdt into that
> region, the code is simplified and the memory region is always allocated
> properly.
>
> Also change the types of some variables to avoid some typecasts.
>
> Signed-off-by: Timur Tabi<timur@freescale.com>
                            ^
Need a space in your SOB line?

> ---
>   common/image.c |   83 +++++++++++++++++++-------------------------------------
>   1 files changed, 28 insertions(+), 55 deletions(-)

I assume this is a "live" patch, and replaces the patch with the subject 
"libfdt: make fdt_increase_size() available to	everyone".  It looks like 
a good improvement to me and nobody threw any stones at it, so...

Acked-by: Gerald Van Baren <vanbaren@cideas.com>

[snip]

Best regards,
gvb

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Tabi Timur-B04825]

On Jul 17, 2010, at 10:17 PM, "Jerry Van Baren" <gvb.uboot@gmail.com> wrote:

>> Signed-off-by: Timur Tabi<timur@freescale.com>
>                           ^
> Need a space in your SOB line?

That's your mailer, not me.  Check the archive -- my message is correct.

> I assume this is a "live" patch, and replaces the patch with the subject "libfdt: make fdt_increase_size() available to	everyone".  It looks like a good improvement to me and nobody threw any stones at it, so...

The two patches are not related.  The other one rejected anyway.

> 

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Timur Tabi]

Kumar, do you have any issues with this patch?  I submitted it in May,
but you never picked it up.

On Mon, May 24, 2010 at 3:10 PM, Timur Tabi <timur@freescale.com> wrote:
> The device tree (fdt) must always exist in within the bootmap (usually the
> first 16MB of RAM). ?If it doesn't, then boot_relocate_fdt() will allocate an
> LMB region in the bootmap and copy the fdt into that region. ?It will also
> increase the size of the fdt.
>
> If the fdt is already in the bootmap, then previously the memory was just
> reserved. ?There was no contingency if the reservation failed, however.
>
> By always allocating an lmb region and copying/resizing the fdt into that
> region, the code is simplified and the memory region is always allocated
> properly.
>
> Also change the types of some variables to avoid some typecasts.
>
> Signed-off-by: Timur Tabi

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Kumar Gala]

On Aug 2, 2010, at 10:43 AM, Timur Tabi wrote:

> Kumar, do you have any issues with this patch?  I submitted it in May,
> but you never picked it up.
> 
> On Mon, May 24, 2010 at 3:10 PM, Timur Tabi <timur@freescale.com> wrote:
>> The device tree (fdt) must always exist in within the bootmap (usually the
>> first 16MB of RAM).  If it doesn't, then boot_relocate_fdt() will allocate an
>> LMB region in the bootmap and copy the fdt into that region.  It will also
>> increase the size of the fdt.
>> 
>> If the fdt is already in the bootmap, then previously the memory was just
>> reserved.  There was no contingency if the reservation failed, however.
>> 
>> By always allocating an lmb region and copying/resizing the fdt into that
>> region, the code is simplified and the memory region is always allocated
>> properly.
>> 
>> Also change the types of some variables to avoid some typecasts.
>> 
>> Signed-off-by: Timur Tabi
> 
> -- 
> Timur Tabi
> Linux kernel developer at Freescale

As this isn't to 8xxx related code I'm NOT going to apply it.  Wolfgang should.

- k

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Wolfgang Denk]

Dear Kumar Gala,

In message <BCBD0D03-A3BB-4AF7-B8FD-2A130E279E87@kernel.crashing.org> you wrote:
> 
> As this isn't to 8xxx related code I'm NOT going to apply it.  Wolfgang
> should.

Which boards except 8xxx actually use that code? I mean, which boards
can be used for testing?

[I don't remember any Tested-by: messages ?]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are always alternatives.
	-- Spock, "The Galileo Seven", stardate 2822.3

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Wolfgang Denk]

Dear Kumar,

In message <20100802205706.56072D3CE46@gemini.denx.de> I wrote:
> 
> In message <BCBD0D03-A3BB-4AF7-B8FD-2A130E279E87@kernel.crashing.org> you wrote:
> > 
> > As this isn't to 8xxx related code I'm NOT going to apply it.  Wolfgang
> > should.
> 
> Which boards except 8xxx actually use that code? I mean, which boards
> can be used for testing?
> 
> [I don't remember any Tested-by: messages ?]

Ping...

I would like to see any ACK or Tested-by: from actual users of this code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Pig: An animal (Porcus omnivorous) closely allied to the  human  race
by  the splendor and vivacity of its appetite, which, however, is in-
ferior in scope, for it balks at pig.                - Ambrose Bierce

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Timur Tabi]

On Sat, Aug 7, 2010 at 6:36 PM, Wolfgang Denk <wd@denx.de> wrote:

> I would like to see any ACK or Tested-by: from actual users of this code.

Kumar, who should ack this patch?

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Timur Tabi]

On Fri, Aug 20, 2010 at 2:12 PM, Timur Tabi <timur@freescale.com> wrote:
> On Sat, Aug 7, 2010 at 6:36 PM, Wolfgang Denk <wd@denx.de> wrote:
>
>> I would like to see any ACK or Tested-by: from actual users of this code.
>
> Kumar, who should ack this patch?

This patch has been tested and acked by Ira Synder:

http://lists.denx.de/pipermail/u-boot/2010-September/076946.html

Kumar, please apply this patch for -next

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Timur Tabi]

On Mon, May 24, 2010 at 3:10 PM, Timur Tabi <timur@freescale.com> wrote:
> The device tree (fdt) must always exist in within the bootmap (usually the
> first 16MB of RAM). ?If it doesn't, then boot_relocate_fdt() will allocate an
> LMB region in the bootmap and copy the fdt into that region. ?It will also
> increase the size of the fdt.
>
> If the fdt is already in the bootmap, then previously the memory was just
> reserved. ?There was no contingency if the reservation failed, however.
>
> By always allocating an lmb region and copying/resizing the fdt into that
> region, the code is simplified and the memory region is always allocated
> properly.
>
> Also change the types of some variables to avoid some typecasts.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---

Why was this patch not merged into U-Boot during the merge window?
I've been waiting since May for this patch to be applied, and it's
been tested and verified.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] always relocate fdt into an lmb-allocated memory block

[Kumar Gala]

On May 24, 2010, at 3:10 PM, Timur Tabi wrote:

> The device tree (fdt) must always exist in within the bootmap (usually the
> first 16MB of RAM).  If it doesn't, then boot_relocate_fdt() will allocate an
> LMB region in the bootmap and copy the fdt into that region.  It will also
> increase the size of the fdt.
> 
> If the fdt is already in the bootmap, then previously the memory was just
> reserved.  There was no contingency if the reservation failed, however.
> 
> By always allocating an lmb region and copying/resizing the fdt into that
> region, the code is simplified and the memory region is always allocated
> properly.
> 
> Also change the types of some variables to avoid some typecasts.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> common/image.c |   83 +++++++++++++++++++-------------------------------------
> 1 files changed, 28 insertions(+), 55 deletions(-)

applied to 85xx

- k