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....@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....@freescale.com>
>> Cc: Simon Glass <s...@chromium.org>
>> Cc: Joe Hershberger <joe.hershber...@ni.com>
>> Cc: Max Krummenacher <max.krummenac...@toradex.com>
>> Cc: Marek Vasut <ma...@denx.de>
>> Cc: Suriyan Ramasami <suriya...@gmail.com>
>> Cc: Paul Kocialkowski <cont...@paulk.fr>
>> Cc: Tom Rini <tr...@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".
>
>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.
Thanks,
Peng.
>
>Regards,
>Simon
--
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
