always relocate fdt into an lmb-allocated memory block

PATCH New Timur Tabi 7 participants 11 messages May 24, 2010 – Oct 20, 2010

Share & tools

Timur Tabi

author

2010-05-24 20:10 UTC #0 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

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 <[email protected]>

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

common/image.c +28 ~~-55~~

index 8d4be14..e19732c 100644
@@ -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;

Signature · 1 line

1.6.5

Jerry Van Baren

2010-07-18 03:17 UTC #1 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

Hi Timur,

Quoted 8 lines · 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.

Quoted 7 lines

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<[email protected]>

Need a space in your SOB line?

Quoted 3 lines

---
  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 <[email protected]>

[snip]

Best regards,
gvb

Tabi Timur-B04825

2010-07-18 04:37 UTC #2 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

Quoted 3 lines · Jerry Van Baren

Quoted 1 line
Signed-off-by Timur Tabi<[email protected]>
                          ^
Need a space in your SOB line?

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

Quoted 1 line

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.

Quoted 1 line

Timur Tabi

2010-08-02 15:43 UTC #3 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

In reply to Timur Tabi

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

Quoted 15 lines · 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

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Kumar Gala

2010-08-02 15:51 UTC #4 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

Quoted 12 lines · Timur Tabi

Kumar, do you have any issues with this patch? I submitted it in May, but you never picked it up.
Quoted 8 lines · 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.

Quoted 11 lines

Quoted 7 lines
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
Signature · 2 lines
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

Wolfgang Denk

2010-08-02 20:57 UTC #5 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

Dear Kumar Gala,

Quoted 3 lines · In message <[email protected]> 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

Signature · 5 lines

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

Wolfgang Denk

2010-08-07 23:36 UTC #6 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

Dear Kumar,

Quoted 10 lines · In message <[email protected]> I wrote:

Quoted 3 lines · In message <[email protected]> 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

Signature · 6 lines

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

Timur Tabi

author

2010-08-20 19:12 UTC #7 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

Quoted 1 line · Wolfgang Denk

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

Kumar, who should ack this patch?

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Timur Tabi

author

2010-10-11 15:02 UTC #8 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

Quoted 5 lines · Timur Tabi

Quoted 1 line · Wolfgang Denk
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

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Timur Tabi

2010-10-18 15:57 UTC #9 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

In reply to Timur Tabi

Quoted 8 lines · 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.

Quoted 8 lines

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 <[email protected]>
---

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.

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Kumar Gala

Applied

2010-10-20 05:25 UTC #10 lore raw

PATCH 1/1 always relocate fdt into an lmb-allocated memory block

In reply to Timur Tabi

Quoted 12 lines · 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.

Quoted 6 lines

Also change the types of some variables to avoid some typecasts.
Signed-off-by Timur Tabi <[email protected]>
---
common/image.c | 83 +++++++++++++++++++-------------------------------------
1 files changed, 28 insertions(+), 55 deletions(-)

applied to 85xx

- k