On 24 November 2015 at 18:12, Peng Fan b51431@freescale.com wrote:
Hi Simon, On Tue, Nov 24, 2015 at 12:04:56PM -0700, Simon Glass wrote:
Hi Peng,
On 24 November 2015 at 01:54, Peng Fan Peng.Fan@freescale.com wrote:
If condition of "(load == image_start || load == image_data)" is true, should use "fdt_addr = load;", but not "fdt_blob = (char *)image_data;", or fdt_blob will be overridden by "fdt_blob = map_sysmem(fdt_addr, 0);" at the end of the switch case.
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Simon Glass sjg@chromium.org Cc: Joe Hershberger joe.hershberger@ni.com Cc: Max Krummenacher max.krummenacher@toradex.com Cc: Marek Vasut marex@denx.de Cc: Suriyan Ramasami suriyan.r@gmail.com Cc: Paul Kocialkowski contact@paulk.fr Cc: Tom Rini trini@konsulko.com
common/image-fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c index 5180a03..5e4e5bd 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -326,7 +326,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
if (load == image_start || load == image_data) {
fdt_blob = (char *)image_data;
fdt_addr = load; break; }Are you sure that should not be:
fdt_addr = image_data
?
Just code inspection.
See the following code piece:
319 image_start = (ulong)fdt_hdr; 320 image_data = (ulong)image_get_data(fdt_hdr); 321 image_end = image_get_image_end(fdt_hdr); 322 323 load = image_get_load(fdt_hdr); 324 load_end = load + image_get_data_size(fdt_hdr); 325 326 if (load == image_start || 327 load == image_data) { 328 fdt_blob = (char *)image_data; 329 break; 330 } 331 332 if ((load < image_end) && (load_end > image_start)) { 333 fdt_error("fdt overwritten"); 334 goto error; 335 } 336 337 debug(" Loading FDT from 0x%08lx to 0x%08lx\n", 338 image_data, load); 339 340 memmove((void *)load, 341 (void *)image_data, 342 image_get_data_size(fdt_hdr)); 343 344 fdt_addr = load; 345 break;
.........[snip code].........386 printf(" Booting using the fdt blob at %#08lx\n", fdt_addr); 387 fdt_blob = map_sysmem(fdt_addr, 0);
Line 387 will override the settings of line 328. To line 328, means we do not need to relocate image_data to load, since they are same. So according to line 344, I use same way "fdt_addr = load".
OK I see.
Reviewed-by: Simon Glass sjg@chromium.org
The idea is to pick up the FDT from inside the image, since the load address indicates that it should not be relocated.
BTW one more thing I noticed:
image_data = (ulong)image_get_data(fdt_hdr);
The cast is confusing, and can be removed.
Yeah. But this patch is to avoid override settings of fdt_blob, line 328 and line 387. This cast can be discarded using another patch.
Yes it should be a separate patch. But since you are in there...
Regards, Simon