[U-Boot] [PATCH v2 6/7] arm: add a common .lds link script

Albert ARIBAUD albert.u.boot at aribaud.net
Fri Feb 17 12:08:47 CET 2012


Hi Simon,

Le 21/11/2011 21:49, Simon Glass a écrit :

> +	.text :
> +	{
> +		__text_start = .;

This assignment to __text_start does not exist in any of the existing 
u-boot.lds files. What is the point of it?

> +	.u_boot_cmd : { *(.u_boot_cmd) }
> +	__u_boot_cmd_end = .;
> +
> +	. = ALIGN(4);
> +
> +	__image_copy_end = .;

Ditto for __image_copy_end.

These two changes are unexplained in the commit message. Mind you, the 
one about CPUDIR and start.S isn't either... and it should, because once 
the commit is in, there is no indication left that it is part of a set, 
so readers will have a difficulty spotting the changes introduced.

But what bothers me most is that the patch set produces u-boot.bin files 
which are not binary identical to those produced without the patch set; 
if I remove the two assignments, then binary identity is preserved.

Note: I check for binary identity by diff'ing hex dumps of u-boot.bin 
files produced with and without the patch set. If the only difference is 
the build version and date, I deem the files binary identical. The dump 
is done with 'od -t x1z -A x u-boot.bin'.

So unless there is a compelling and strictly unavoidable reason for 
these assignements, please drop them in the V3 patch set.

Amicalement,
-- 
Albert.


More information about the U-Boot mailing list