[U-Boot] [PATCH] 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 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;

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

On Jul 17, 2010, at 10:17 PM, "Jerry Van Baren" gvb.uboot@gmail.com wrote:
Signed-off-by: Timur Tabitimur@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.

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

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

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

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

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?

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

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.

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
participants (7)
-
Jerry Van Baren
-
Kumar Gala
-
Kumar Gala
-
Tabi Timur-B04825
-
Timur Tabi
-
Timur Tabi
-
Wolfgang Denk