[U-Boot] [PATCH] board_f: Do not mark pre-relocated fdt space as reserved

[Simon Goldschmidt]

[This is a follow-up to https://patchwork.ozlabs.org/patch/1033584/ 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;
+       }

         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-keerthy at ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla at 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
>