From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 6/7] arm: add a common .lds link script
Date: Fri, 17 Feb 2012 12:08:47 +0100 [thread overview]
Message-ID: <4F3E353F.9050008@aribaud.net> (raw)
In-Reply-To: <1321908583-4311-7-git-send-email-sjg@chromium.org>
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.
next prev parent reply other threads:[~2012-02-17 11:08 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-21 20:49 [U-Boot] [PATCH v2 0/7] Tidy up ARM link scripts Simon Glass
2011-11-21 20:49 ` [U-Boot] [PATCH v2 1/7] Allow arch directory to contain .lds without requiring Makefile Simon Glass
2011-11-21 20:49 ` [U-Boot] [PATCH v2 2/7] arm: Remove jornada link script Simon Glass
2011-11-21 20:49 ` [U-Boot] [PATCH v2 3/7] arm: Remove zipitz2 " Simon Glass
2011-11-21 20:49 ` [U-Boot] [PATCH v2 4/7] Define CPUDIR for the .lds " Simon Glass
2011-11-21 20:49 ` [U-Boot] [PATCH v2 5/7] arm: Remove unneeded setting of LDCSRIPT Simon Glass
2011-11-21 20:49 ` [U-Boot] [PATCH v2 6/7] arm: add a common .lds link script Simon Glass
2012-02-17 11:08 ` Albert ARIBAUD [this message]
2012-02-21 2:32 ` Simon Glass
2012-02-21 12:34 ` Albert ARIBAUD
2012-02-23 13:28 ` [U-Boot] [PATCH v4 " Simon Glass
2011-11-21 20:49 ` [U-Boot] [PATCH v2 7/7] arm: Use common .lds file where possible Simon Glass
2011-12-03 3:04 ` [U-Boot] [PATCH v2 0/7] Tidy up ARM link scripts Simon Glass
2012-02-17 8:58 ` Albert ARIBAUD
2012-02-17 9:49 ` Marek Vasut
2012-02-17 10:47 ` Albert ARIBAUD
2012-02-21 2:24 ` Simon Glass
2012-02-21 5:53 ` Marek Vasut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F3E353F.9050008@aribaud.net \
--to=albert.u.boot@aribaud.net \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox