* [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
@ 2014-07-23 0:08 Chris Packham
2014-07-23 10:27 ` Simon Glass
0 siblings, 1 reply; 12+ messages in thread
From: Chris Packham @ 2014-07-23 0:08 UTC (permalink / raw)
To: u-boot
file_size was being calculated using back-ticks but map_size uses
$(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
The back ticks didn't work in my environment (GNU Make 3.81). Updating
to use $(shell ...) makes sense from a consistency view even if the
problem is my environment.
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index ca212b5..f6a0f68 100644
--- a/Makefile
+++ b/Makefile
@@ -786,7 +786,7 @@ u-boot.hex u-boot.srec: u-boot FORCE
OBJCOPYFLAGS_u-boot.bin := -O binary
binary_size_check: u-boot.bin System.map FORCE
- @file_size=`stat -c %s u-boot.bin` ; \
+ file_size=$(shell stat -c %s u-boot.bin) ; \
map_size=$(shell cat System.map | \
awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
| bc); \
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
2014-07-23 0:08 [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size Chris Packham
@ 2014-07-23 10:27 ` Simon Glass
2014-07-23 18:03 ` Tom Rini
2014-07-24 5:14 ` Chris Packham
0 siblings, 2 replies; 12+ messages in thread
From: Simon Glass @ 2014-07-23 10:27 UTC (permalink / raw)
To: u-boot
On 22 July 2014 18:08, Chris Packham <judge.packham@gmail.com> wrote:
> file_size was being calculated using back-ticks but map_size uses
> $(shell ...). Update the file_size calculation to use $(shell ...).
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
Acked-by: Simon Glass <sjg@chromium.org>
But you might want to look at this.
http://patchwork.ozlabs.org/patch/371936/
Regards,
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
2014-07-23 10:27 ` Simon Glass
@ 2014-07-23 18:03 ` Tom Rini
2014-07-23 19:24 ` Jeroen Hofstee
2014-07-24 5:14 ` Chris Packham
1 sibling, 1 reply; 12+ messages in thread
From: Tom Rini @ 2014-07-23 18:03 UTC (permalink / raw)
To: u-boot
On Wed, Jul 23, 2014 at 04:27:36AM -0600, Simon Glass wrote:
> On 22 July 2014 18:08, Chris Packham <judge.packham@gmail.com> wrote:
> > file_size was being calculated using back-ticks but map_size uses
> > $(shell ...). Update the file_size calculation to use $(shell ...).
> >
> > Signed-off-by: Chris Packham <judge.packham@gmail.com>
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> But you might want to look at this.
>
> http://patchwork.ozlabs.org/patch/371936/
Yeah, Jeroen, can we get a v2 of your patch that uses $(shell ... )?
Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140723/b9d9fd8c/attachment.pgp>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
2014-07-23 18:03 ` Tom Rini
@ 2014-07-23 19:24 ` Jeroen Hofstee
2014-07-23 20:07 ` Tom Rini
0 siblings, 1 reply; 12+ messages in thread
From: Jeroen Hofstee @ 2014-07-23 19:24 UTC (permalink / raw)
To: u-boot
Hi,
On 23-07-14 20:03, Tom Rini wrote:
> On Wed, Jul 23, 2014 at 04:27:36AM -0600, Simon Glass wrote:
>> On 22 July 2014 18:08, Chris Packham <judge.packham@gmail.com> wrote:
>>> file_size was being calculated using back-ticks but map_size uses
>>> $(shell ...). Update the file_size calculation to use $(shell ...).
>>>
>>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> But you might want to look at this.
>>
>> http://patchwork.ozlabs.org/patch/371936/
> Yeah, Jeroen, can we get a v2 of your patch that uses $(shell ... )?
> Thanks!
>
>
no problem. For the record I have not seen any issues with
mentioned gmake version. But using $(shell .. ) seem like a sane
thing to do. v2 is on its way, running MAKEALL for arm (can we keep it?).
Chris: blackfin targets seem to rely on the same construct btw...
Regards,
Jeroen
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
2014-07-23 19:24 ` Jeroen Hofstee
@ 2014-07-23 20:07 ` Tom Rini
0 siblings, 0 replies; 12+ messages in thread
From: Tom Rini @ 2014-07-23 20:07 UTC (permalink / raw)
To: u-boot
On Wed, Jul 23, 2014 at 09:24:00PM +0200, Jeroen Hofstee wrote:
> Hi,
>
> On 23-07-14 20:03, Tom Rini wrote:
> >On Wed, Jul 23, 2014 at 04:27:36AM -0600, Simon Glass wrote:
> >>On 22 July 2014 18:08, Chris Packham <judge.packham@gmail.com> wrote:
> >>>file_size was being calculated using back-ticks but map_size uses
> >>>$(shell ...). Update the file_size calculation to use $(shell ...).
> >>>
> >>>Signed-off-by: Chris Packham <judge.packham@gmail.com>
> >>Acked-by: Simon Glass <sjg@chromium.org>
> >>
> >>But you might want to look at this.
> >>
> >>http://patchwork.ozlabs.org/patch/371936/
> >Yeah, Jeroen, can we get a v2 of your patch that uses $(shell ... )?
> >Thanks!
> >
> >
> no problem. For the record I have not seen any issues with
> mentioned gmake version. But using $(shell .. ) seem like a sane
> thing to do. v2 is on its way, running MAKEALL for arm (can we keep it?).
MAKEALL isn't going away for v2014.10 but please reply to Simon's RFC
about it. I do have to admit that switching all of my stuff to buildman
is still on my TODO list.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140723/64798183/attachment.pgp>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
2014-07-23 10:27 ` Simon Glass
2014-07-23 18:03 ` Tom Rini
@ 2014-07-24 5:14 ` Chris Packham
2014-07-24 5:27 ` [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check Chris Packham
2014-07-25 14:14 ` [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size Simon Glass
1 sibling, 2 replies; 12+ messages in thread
From: Chris Packham @ 2014-07-24 5:14 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Wed, Jul 23, 2014 at 10:27 PM, Simon Glass <sjg@chromium.org> wrote:
> On 22 July 2014 18:08, Chris Packham <judge.packham@gmail.com> wrote:
>> file_size was being calculated using back-ticks but map_size uses
>> $(shell ...). Update the file_size calculation to use $(shell ...).
>>
>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> But you might want to look at this.
>
> http://patchwork.ozlabs.org/patch/371936/
>
Thanks. I've re-submitted a version with Jeroen's change included and
it seems a v2 of his patch is imminent too. Either way I'm not fussed.
But this has highlighted another issue I'm experiencing related to
this code. Originally I thought it was because I was doing something a
bit weird with a custom u-boot.lds but actually the problem is my
u-boot is setup so that it finishes exactly at the end of a 32bit
address space so _image_binary_end is just off the end of it which
screws up the size calculation.
$ grep _image_binary_end u-boot.map
0x0000000100000000 _image_binary_end = .
$ grep _image_binary_end System.map
00000000 A _image_binary_end
$ make binary_size_check
...
System.map shows a binary size of -4294180864
but u-boot.bin shows 786432
make: *** [binary_size_check] Error 1
I think it should be possible to change binary_size_check to use
u-boot.map instead of System.map but would that be OK for all
architectures?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check
2014-07-24 5:14 ` Chris Packham
@ 2014-07-24 5:27 ` Chris Packham
2014-07-24 8:31 ` Wolfgang Denk
2014-08-10 22:22 ` [U-Boot] " Tom Rini
2014-07-25 14:14 ` [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size Simon Glass
1 sibling, 2 replies; 12+ messages in thread
From: Chris Packham @ 2014-07-24 5:27 UTC (permalink / raw)
To: u-boot
u-boot.map is generated automatically by the compiler and more
importantly can handle addresses >4GB.
---
On Thu, Jul 24, 2014 at 5:14 PM, Chris Packham <judge.packham@gmail.com> wrote:
> Hi Simon,
>
> On Wed, Jul 23, 2014 at 10:27 PM, Simon Glass <sjg@chromium.org> wrote:
>> On 22 July 2014 18:08, Chris Packham <judge.packham@gmail.com> wrote:
>>> file_size was being calculated using back-ticks but map_size uses
>>> $(shell ...). Update the file_size calculation to use $(shell ...).
>>>
>>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> But you might want to look at this.
>>
>> http://patchwork.ozlabs.org/patch/371936/
>>
>
> Thanks. I've re-submitted a version with Jeroen's change included and
> it seems a v2 of his patch is imminent too. Either way I'm not fussed.
>
> But this has highlighted another issue I'm experiencing related to
> this code. Originally I thought it was because I was doing something a
> bit weird with a custom u-boot.lds but actually the problem is my
> u-boot is setup so that it finishes exactly at the end of a 32bit
> address space so _image_binary_end is just off the end of it which
> screws up the size calculation.
>
> $ grep _image_binary_end u-boot.map
> 0x0000000100000000 _image_binary_end = .
>
> $ grep _image_binary_end System.map
> 00000000 A _image_binary_end
>
> $ make binary_size_check
> ...
> System.map shows a binary size of -4294180864
> but u-boot.bin shows 786432
> make: *** [binary_size_check] Error 1
>
> I think it should be possible to change binary_size_check to use
> u-boot.map instead of System.map but would that be OK for all
> architectures?
Something like this works for me but maybe there is a way to get nm to handle
addresses >4GB.
Makefile | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index b98a80a..3ed02a4 100644
--- a/Makefile
+++ b/Makefile
@@ -785,14 +785,15 @@ u-boot.hex u-boot.srec: u-boot FORCE
OBJCOPYFLAGS_u-boot.bin := -O binary
-binary_size_check: u-boot.bin System.map FORCE
+binary_size_check: u-boot.bin FORCE
@file_size=$(shell wc -c u-boot.bin | awk '{print $$1}') ; \
- map_size=$(shell cat System.map | \
+ map_size=$(shell cat u-boot.map | \
awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
+ | sed 's/0X//g' \
| bc); \
if [ "" != "$$map_size" ]; then \
if test $$map_size -ne $$file_size; then \
- echo "System.map shows a binary size of $$map_size" >&2 ; \
+ echo "u-boot.map shows a binary size of $$map_size" >&2 ; \
echo " but u-boot.bin shows $$file_size" >&2 ; \
exit 1; \
fi \
--
1.7.9.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check
2014-07-24 5:27 ` [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check Chris Packham
@ 2014-07-24 8:31 ` Wolfgang Denk
2014-07-24 13:50 ` Tom Rini
2014-08-10 22:22 ` [U-Boot] " Tom Rini
1 sibling, 1 reply; 12+ messages in thread
From: Wolfgang Denk @ 2014-07-24 8:31 UTC (permalink / raw)
To: u-boot
Dear Chris Packham,
In message <1406179627-9496-1-git-send-email-judge.packham@gmail.com> you wrote:
> u-boot.map is generated automatically by the compiler and more
> importantly can handle addresses >4GB.
...
> > I think it should be possible to change binary_size_check to use
> > u-boot.map instead of System.map but would that be OK for all
> > architectures?
...
> Something like this works for me but maybe there is a way to get nm to handle
> addresses >4GB.
...
> + map_size=$(shell cat u-boot.map | \
> awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
> + | sed 's/0X//g' \
Do we _really_ need all of this?
This looks very much like creaping featurism to me...
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Remember, Information is not knowledge; Knowledge is not Wisdom;
Wisdom is not truth; Truth is not beauty; Beauty is not love; Love is
not music; Music is the best." - Frank Zappa
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check
2014-07-24 8:31 ` Wolfgang Denk
@ 2014-07-24 13:50 ` Tom Rini
2014-07-24 21:08 ` Chris Packham
0 siblings, 1 reply; 12+ messages in thread
From: Tom Rini @ 2014-07-24 13:50 UTC (permalink / raw)
To: u-boot
On Thu, Jul 24, 2014 at 10:31:14AM +0200, Wolfgang Denk wrote:
> Dear Chris Packham,
>
> In message <1406179627-9496-1-git-send-email-judge.packham@gmail.com> you wrote:
> > u-boot.map is generated automatically by the compiler and more
> > importantly can handle addresses >4GB.
> ...
> > > I think it should be possible to change binary_size_check to use
> > > u-boot.map instead of System.map but would that be OK for all
> > > architectures?
> ...
> > Something like this works for me but maybe there is a way to get nm to handle
> > addresses >4GB.
> ...
> > + map_size=$(shell cat u-boot.map | \
> > awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
> > + | sed 's/0X//g' \
>
>
> Do we _really_ need all of this?
>
> This looks very much like creaping featurism to me...
So it's a safetey check. The various linker script clean-ups we've been
doing have introduced a problem from time to time and binary_check_size
gives a sanity check. This just fixes a corner case in the test.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140724/89be7428/attachment.pgp>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check
2014-07-24 13:50 ` Tom Rini
@ 2014-07-24 21:08 ` Chris Packham
0 siblings, 0 replies; 12+ messages in thread
From: Chris Packham @ 2014-07-24 21:08 UTC (permalink / raw)
To: u-boot
Hi Wolfgang, Tom,
On Fri, Jul 25, 2014 at 1:50 AM, Tom Rini <trini@ti.com> wrote:
> On Thu, Jul 24, 2014 at 10:31:14AM +0200, Wolfgang Denk wrote:
>> Dear Chris Packham,
>>
>> In message <1406179627-9496-1-git-send-email-judge.packham@gmail.com> you wrote:
>> > u-boot.map is generated automatically by the compiler and more
>> > importantly can handle addresses >4GB.
>> ...
>> > > I think it should be possible to change binary_size_check to use
>> > > u-boot.map instead of System.map but would that be OK for all
>> > > architectures?
>> ...
>> > Something like this works for me but maybe there is a way to get nm to handle
>> > addresses >4GB.
>> ...
>> > + map_size=$(shell cat u-boot.map | \
>> > awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
>> > + | sed 's/0X//g' \
>>
>>
>> Do we _really_ need all of this?
>>
>> This looks very much like creaping featurism to me...
Do we need it? Maybe not I'd defer that judgement to you.
My problem is binary_size_check is on by default and my particular
platform (an out of tree ARM board) fails to build because of it. If
there was a way of opting out I'd happily use it.
>
> So it's a safetey check. The various linker script clean-ups we've been
> doing have introduced a problem from time to time and binary_check_size
> gives a sanity check. This just fixes a corner case in the test.
I wouldn't say this is a corner case. Certainly for the ARM target I'm
building it is unusual but I know plenty of powerpc boards would hit
the same problem becuase they have a jump instruction at the top of
their address space.
>
> --
> Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
2014-07-24 5:14 ` Chris Packham
2014-07-24 5:27 ` [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check Chris Packham
@ 2014-07-25 14:14 ` Simon Glass
1 sibling, 0 replies; 12+ messages in thread
From: Simon Glass @ 2014-07-25 14:14 UTC (permalink / raw)
To: u-boot
Hi Chris,
On 24 July 2014 06:14, Chris Packham <judge.packham@gmail.com> wrote:
> Hi Simon,
>
> On Wed, Jul 23, 2014 at 10:27 PM, Simon Glass <sjg@chromium.org> wrote:
>> On 22 July 2014 18:08, Chris Packham <judge.packham@gmail.com> wrote:
>>> file_size was being calculated using back-ticks but map_size uses
>>> $(shell ...). Update the file_size calculation to use $(shell ...).
>>>
>>> Signed-off-by: Chris Packham <judge.packham@gmail.com>
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> But you might want to look at this.
>>
>> http://patchwork.ozlabs.org/patch/371936/
>>
>
> Thanks. I've re-submitted a version with Jeroen's change included and
> it seems a v2 of his patch is imminent too. Either way I'm not fussed.
>
> But this has highlighted another issue I'm experiencing related to
> this code. Originally I thought it was because I was doing something a
> bit weird with a custom u-boot.lds but actually the problem is my
> u-boot is setup so that it finishes exactly at the end of a 32bit
> address space so _image_binary_end is just off the end of it which
> screws up the size calculation.
>
> $ grep _image_binary_end u-boot.map
> 0x0000000100000000 _image_binary_end = .
>
> $ grep _image_binary_end System.map
> 00000000 A _image_binary_end
>
> $ make binary_size_check
> ...
> System.map shows a binary size of -4294180864
> but u-boot.bin shows 786432
> make: *** [binary_size_check] Error 1
>
> I think it should be possible to change binary_size_check to use
> u-boot.map instead of System.map but would that be OK for all
> architectures?
That's unfortunate - it's just a little check. It does protect again a
breakage but we need to keep it simple. Maybe in this case we can mask
with 0xffffffff and get the right answer?
Regards,
Simon
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] Makefile: use u-boot.map for binary_size_check
2014-07-24 5:27 ` [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check Chris Packham
2014-07-24 8:31 ` Wolfgang Denk
@ 2014-08-10 22:22 ` Tom Rini
1 sibling, 0 replies; 12+ messages in thread
From: Tom Rini @ 2014-08-10 22:22 UTC (permalink / raw)
To: u-boot
On Thu, Jul 24, 2014 at 05:27:07PM +1200, Chris Packham wrote:
> u-boot.map is generated automatically by the compiler and more
> importantly can handle addresses >4GB.
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140810/8427baea/attachment.pgp>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-08-10 22:22 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-23 0:08 [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size Chris Packham
2014-07-23 10:27 ` Simon Glass
2014-07-23 18:03 ` Tom Rini
2014-07-23 19:24 ` Jeroen Hofstee
2014-07-23 20:07 ` Tom Rini
2014-07-24 5:14 ` Chris Packham
2014-07-24 5:27 ` [U-Boot] [PATCH] Makefile: use u-boot.map for binary_size_check Chris Packham
2014-07-24 8:31 ` Wolfgang Denk
2014-07-24 13:50 ` Tom Rini
2014-07-24 21:08 ` Chris Packham
2014-08-10 22:22 ` [U-Boot] " Tom Rini
2014-07-25 14:14 ` [U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.