Simon, On 22/04/19 8:15 PM, Tom Rini wrote: > On Thu, Apr 18, 2019 at 08:23:45AM +0200, Simon Goldschmidt wrote: >> On Thu, Apr 18, 2019 at 8:12 AM Lokesh Vutla <lokeshvu...@ti.com> wrote: >>> >>> >>> >>> On 18/04/19 12:46 AM, Simon Goldschmidt wrote: >>>> [This is a follow-up to https://patchwork.ozlabs.org/patch/1033584/ right?] >>> >>> Right. >>> >>>> >>>> Am 16.04.2019 um 10:43 schrieb Lokesh Vutla: >>>>> SPL while copying u-boot and dtb it does the following: >>>>> - Copy u-boot >>>>> - Copy right dtb. >>>>> - mark dtb location as reserved in dtb. >>>> >>>> Hmm, why does it do that? Reserving space when U-Boot starts Linux *might* >>>> make >>>> sense, but why should SPL add this reservation when starting U-Boot? These >>>> steps >>>> are for U-Boot in a fit-image, right? Because I can't see this for U-Boot >>>> as >>>> uImage. >>>> >>>> Am I correct that 'fdt_shrink_to_minimum()' adds this reservation? The >>>> name of >>>> that function doesn't suggest that to me. Would it make more sense to let >>>> this >>>> funtion only *change* the reservation, i.e. only add it if it is removed >>>> at the >>>> beginning of said function? Would that fix your issue? >>>> >>>> Something like this: >>>> >>>> diff --git a/common/fdt_support.c b/common/fdt_support.c >>>> index ab08a0114f..4e7cf6ebe9 100644 >>>> --- a/common/fdt_support.c >>>> +++ b/common/fdt_support.c >>>> @@ -597,6 +597,7 @@ int fdt_shrink_to_minimum(void *blob, uint extrasize) >>>> uint64_t addr, size; >>>> int total, ret; >>>> uint actualsize; >>>> + int fdt_memrsv = 0; >>>> >>>> if (!blob) >>>> return 0; >>>> @@ -606,6 +607,7 @@ int fdt_shrink_to_minimum(void *blob, uint extrasize) >>>> fdt_get_mem_rsv(blob, i, &addr, &size); >>>> if (addr == (uintptr_t)blob) { >>>> fdt_del_mem_rsv(blob, i); >>>> + fdt_memrsv = 1; >>>> break; >>>> } >>>> } >>>> @@ -627,10 +629,12 @@ int fdt_shrink_to_minimum(void *blob, uint extrasize) >>>> /* Change the fdt header to reflect the correct size */ >>>> fdt_set_totalsize(blob, actualsize); >>>> >>>> - /* Add the new reservation */ >>>> - ret = fdt_add_mem_rsv(blob, map_to_sysmem(blob), actualsize); >>>> - if (ret < 0) >>>> - return ret; >>>> + if (fdt_memrsv) { >>>> + /* Add the new reservation */ >>>> + ret = fdt_add_mem_rsv(blob, map_to_sysmem(blob), >>>> actualsize); >>>> + if (ret < 0) >>>> + return ret; >>>> + } >>> >>> Agreed. This works and more appropriate place to fix it. Will you post a >>> separate patch or do you want me to post it on your behalf? >> >> While it's good to know this fixes your issue, I'm not sure this doesn't >> break >> anything else. >> >> Tom, Simon, can you add anything here? Why is the dtb memory added as >> reserved to itself? It seems to me this is redundant, as the code using the >> dtb should well know the pointer to it... >> >> Lokesh, I can send a patch once this is discussed. > > OK, so the history of that call is really what's seen in: > commit 41c5eaa7253ed82bbae1eda5667755872c615164 > Author: Andy Fleming <aflem...@freescale.com> > Date: Mon Jun 16 13:58:56 2008 -0500 > > Resize device tree to allow space for board changes and the chosen node > > Current code requires that a compiled device tree have space added to the > end to > leave room for extra nodes added by board code (and the chosen node). > This > requires that device tree creators anticipate how much space U-Boot will > add to > the tree, which is absurd. Ideally, the code would resize and/or > relocate the > tree when it needed more space, but this would require a systemic change > to the > fdt code, which is non-trivial. Instead, we resize the tree inside > boot_relocate_fdt, reserving either the remainder of the bootmap (in the > case > where the fdt is inside the bootmap), or adding CFG_FDT_PAD bytes to the > size. > > Signed-off-by: Andy Fleming <aflem...@freescale.com> > > Which is all about preparing things for passing the device tree we > loaded specifically for Linux. In: > commit 3082d2348c8e13342f5fdd10e9b3f7408062dbf9 > Author: Kumar Gala <ga...@kernel.crashing.org> > Date: Fri Aug 15 08:24:42 2008 -0500 > > fdt: refactor fdt resize code > > Move the fdt resizing code out of ppc specific boot code and into > common fdt support code. > > Signed-off-by: Kumar Gala <ga...@kernel.crashing.org> > > The code was moved to a more common area. So the intent on "reserve" > was to make sure that we had enough space set aside for the dtb to be > able to be updated at runtime, by U-Boot, for various needs and that we > wouldn't give that memory away to something else / allow it to be > clobbered. >
Any update on this? Any chance you will be posting your changes? Thanks and regards, Lokesh _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot