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? Thanks and regards, Lokesh > > return actualsize; > } > > >> >> U-Boot when copying images at U-Boot prompt, is not able to copy to the >> above dtb location as it sees the region as reserved. But at this stage >> dtb is re located to end of DDR. And the above dtb region is not reserved >> anymore. So delete this reserved region when re locating dtb. >> >> Reported-by: Keerthy <j-keer...@ti.com> >> Signed-off-by: Lokesh Vutla <lokeshvu...@ti.com> >> --- >> common/board_f.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/common/board_f.c b/common/board_f.c >> index 149a7229e8..e4383ae3fa 100644 >> --- a/common/board_f.c >> +++ b/common/board_f.c >> @@ -651,10 +651,23 @@ static int init_post(void) >> static int reloc_fdt(void) >> { >> #ifndef CONFIG_OF_EMBED >> + uint64_t addr, size; >> + int i, cnt, err; >> + >> if (gd->flags & GD_FLG_SKIP_RELOC) >> return 0; >> if (gd->new_fdt) { >> memcpy(gd->new_fdt, gd->fdt_blob, gd->fdt_size); >> + >> + /* Deleting the previously marked FDT reserved region */ >> + cnt = fdt_num_mem_rsv(gd->new_fdt); >> + for (i = 0; i < cnt ; i++) { >> + err = fdt_get_mem_rsv(gd->new_fdt, i, &addr, &size); >> + if (!err && addr == (uintptr_t)gd->fdt_blob) { >> + fdt_del_mem_rsv(gd->new_fdt, i); >> + break; >> + } >> + } > > That code should work, but it's a bit unexpected in this location > >> gd->fdt_blob = gd->new_fdt; >> } >> #endif >> > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot