Re: [U-Boot] CFG_64BIT_xxx and friends
Jerry Van Baren [EMAIL PROTECTED] wrote: Haavard Skinnemoen wrote: Haavard Skinnemoen [EMAIL PROTECTED] wrote: That's a bit more than expected. Is this with or without --gc-sections? Linking with --gc-sections should make simple_strtoull() go away unless it's actually used. That's assuming the fdt and image code doesn't interpret CFG_64BIT_VSPRINTF as CFG_BLOAT_ME_HARDER, which it does. So enabling CFG_64BIT_VSPRINTF does increase the code size even with --gc-sections. I think fdt and common/image.c should stop abusing CFG_64BIT_VSPRINTF and get its own symbol instead, e.g. CFG_64BIT_PHYS_ADDR, and perhaps a nice str_to_addr() wrapper which selects between strtoul and strtoull based on this symbol. Hi Haavard, fdt and common.image.c don't use CFG_64BIT_VSPRINTF: $ find . -name *.c | xargs grep CFG_64BIT_VSPRINTF ./disk/part.c:#if defined(CFG_64BIT_LBA) defined(CFG_64BIT_VSPRINTF) ./common/cmd_ide.c:#if defined(CFG_64BIT_LBA) defined(CFG_64BIT_VSPRINTF) ./common/cmd_ide.c:#if defined(CFG_64BIT_LBA) defined(CFG_64BIT_VSPRINTF) ./lib_generic/vsprintf.c:#ifdef CFG_64BIT_VSPRINTF ./lib_generic/vsprintf.c:#ifdef CFG_64BIT_VSPRINTF ./lib_generic/vsprintf.c:#ifdef CFG_64BIT_VSPRINTF ./lib_generic/vsprintf.c:#ifdef CFG_64BIT_VSPRINTF ...they use CFG_64BIT_STRTOUL. Ah, sorry. I meant CFG_64BIT_STRTOUL. If a config defines CFG_64BIT_STRTOUL, it is reasonable that the code uses it. I don't see any disadvantage of this vs. creating a new CFG_64BIT_PHYS_ADDR (although I would not object to that being created). Just because a 64-bit strtoul exists doesn't mean you _have_ to use it. Only a select set of PowerPC targets actually define CFG_64BIT_STRTOUL: $ find . -name *.[ch] | xargs grep CFG_64BIT_STRTOUL ./cpu/mpc85xx/mp.c:#ifdef CFG_64BIT_STRTOUL ./include/configs/MPC8540ADS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8572DS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8536DS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8548CDS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8568MDS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8541CDS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8610HPCD.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8641HPCN.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/sbc8641d.h:#define CFG_64BIT_STRTOUL1 ./include/configs/MPC8555CDS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8560ADS.h:#define CFG_64BIT_STRTOUL 1 ./include/configs/MPC8544DS.h:#define CFG_64BIT_STRTOUL 1 ./include/ppc4xx.h:#define CFG_64BIT_STRTOUL ./common/cmd_fdt.c:#ifdef CFG_64BIT_STRTOUL ./common/cmd_fdt.c:#ifdef CFG_64BIT_STRTOUL ./common/image.c:#ifdef CFG_64BIT_STRTOUL ./lib_generic/vsprintf.c:#ifdef CFG_64BIT_STRTOUL ./lib_generic/vsprintf.c:#endif /* CFG_64BIT_STRTOUL */ Ok, so CFG_64BIT_STRTOUL effectively means I want 64-bit physical addresses today. It's a bit unintuitive, but I guess we have bigger problems than that. I must say it was a bit surprising that my u-boot image instantly grew by 200 bytes when I thought I just made a new function available, and --gc-sections was being used. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CFG_64BIT_xxx and friends
On Sunday 07 September 2008, Wolfgang Denk wrote: 5) Delete al references to CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL and unconditionally enable it for all boards. Any takers to submit a patch? If possible not because it will increase the size of u-boot for board which not need it. When we will have kconfig it will be simple to activated it for ppc and other board automaticly. You try to beat me on being the guard dog of memory footprint? That'll be a hard job ;-) Seriously: How much of code size are we talking about? And activating / deactivation the feature is not so trivial as it affects the printf() format specifiers we have to use. I'm with Wolfgang here and think it would be best to unconditionally support the 64bit printf format too. Best regards, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [EMAIL PROTECTED] = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CFG_64BIT_xxx and friends
Matthias Fuchs [EMAIL PROTECTED] wrote: Here is the U-Boot size for the PLU405 board (405EP-based) with and without #define CFG_64BIT_VSPRINTF #define CFG_64BIT_STRTOUL . without: # ppc_4xx-size u-boot textdata bss dec hex filename 289568 17532 301312 608412 9489c u-boot with 64bit format handling: # ppc_4xx-size u-boot textdata bss dec hex filename 291368 17532 301312 610212 94fa4 u-boot So the difference is 1800 bytes on this architecture. Thanks. That's a bit more than expected. Is this with or without --gc-sections? Linking with --gc-sections should make simple_strtoull() go away unless it's actually used. Another thing that might hurt is that lib_generic/vsprintf.c reinvents do_div() without the out-of-line __div_64_32() bit. Converting it to use do_div() from include/div64.h should help. In fact, enabling CFG_64BIT_VSPRINTF unconditionally without fixing this first will probably break all architectures that don't link with libgcc. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CFG_64BIT_xxx and friends
Haavard Skinnemoen [EMAIL PROTECTED] wrote:
That's a bit more than expected. Is this with or without --gc-sections?
Linking with --gc-sections should make simple_strtoull() go away unless
it's actually used.
That's assuming the fdt and image code doesn't interpret
CFG_64BIT_VSPRINTF as CFG_BLOAT_ME_HARDER, which it does. So enabling
CFG_64BIT_VSPRINTF does increase the code size even with --gc-sections.
I think fdt and common/image.c should stop abusing CFG_64BIT_VSPRINTF
and get its own symbol instead, e.g. CFG_64BIT_PHYS_ADDR, and perhaps a
nice str_to_addr() wrapper which selects between strtoul and strtoull
based on this symbol.
Another thing that might hurt is that lib_generic/vsprintf.c reinvents
do_div() without the out-of-line __div_64_32() bit. Converting it to
use do_div() from include/div64.h should help.
It does. A lot. Here are some numbers from avr32:
vanilla:
textdata bss dec hex filename
962327528 216208 319968 4e1e0 ./u-boot
with CFG_64BIT_VSPRINTF:
textdata bss dec hex filename
981007528 216208 321836 4e92c ./u-boot
with CFG_64BIT_VSPRINTF and generic do_div():
textdata bss dec hex filename
963967528 216208 320132 4e284 ./u-boot
So I highly recommend applying the patch below before killing
CFG_64BIT_VSPRINTF.
In fact, enabling CFG_64BIT_VSPRINTF unconditionally without fixing
this first will probably break all architectures that don't link with
libgcc.
I was sort of expecting avr32 to break because of this, but it didn't.
Apparently, the top-level Makefile adds -lgcc unconditionally...which
makes it quite hard to catch undesired bloat like this.
Haavard
From: Haavard Skinnemoen [EMAIL PROTECTED]
Subject: vsprintf: Use generic do_div()
lib_generic/vsprintf.c uses its own reimplementation of do_div().
Switch it over to use the generic implementation from include/div64.h.
This saves 2K of .text on avr32 with CFG_64BIT_VSPRINTF enabled.
Signed-off-by: Haavard Skinnemoen [EMAIL PROTECTED]
diff --git a/lib_generic/vsprintf.c b/lib_generic/vsprintf.c
index 7c9cfe1..a4f8a83 100644
--- a/lib_generic/vsprintf.c
+++ b/lib_generic/vsprintf.c
@@ -15,6 +15,7 @@
#include linux/ctype.h
#include common.h
+#include div64.h
#if !defined (CONFIG_PANIC_HANG)
#include command.h
/*cmd_boot.c*/
@@ -106,22 +107,6 @@ static int skip_atoi(const char **s)
#define LARGE 64 /* use 'ABCDEF' instead of 'abcdef' */
#ifdef CFG_64BIT_VSPRINTF
-#define do_div(n,base) ({ \
- unsigned int __res; \
- __res = ((unsigned long long) n) % base; \
- n = ((unsigned long long) n) / base; \
- __res; \
-})
-#else
-#define do_div(n,base) ({ \
- int __res; \
- __res = ((unsigned long) n) % base; \
- n = ((unsigned long) n) / base; \
- __res; \
-})
-#endif
-
-#ifdef CFG_64BIT_VSPRINTF
static char * number(char * str, long long num, unsigned int base, int size,
int precision ,int type)
#else
static char * number(char * str, long num, unsigned int base, int size, int
precision ,int type)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CFG_64BIT_xxx and friends
Here is the U-Boot size for the PLU405 board (405EP-based) with and without #define CFG_64BIT_VSPRINTF #define CFG_64BIT_STRTOUL . without: # ppc_4xx-size u-boot textdata bss dec hex filename 289568 17532 301312 608412 9489c u-boot with 64bit format handling: # ppc_4xx-size u-boot textdata bss dec hex filename 291368 17532 301312 610212 94fa4 u-boot So the difference is 1800 bytes on this architecture. Matthias On Monday 08 September 2008 13:00, Haavard Skinnemoen wrote: Stefan Roese [EMAIL PROTECTED] wrote: Seriously: How much of code size are we talking about? And activating / deactivation the feature is not so trivial as it affects the printf() format specifiers we have to use. I'm with Wolfgang here and think it would be best to unconditionally support the 64bit printf format too. Would be nice to see some numbers first though. I suspect there won't be much difference, but it could be the printf() implementation does something stupid, and it's much easier to tell when the config symbol is still in place. Haavard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CFG_64BIT_xxx and friends
Dear Matthias, in message [EMAIL PROTECTED] you wrote: after testing the recent U-Boot code on a couple of 405EP boards I noticed, that the memsize in the output of the bdinfo command is always 0x. This is caused by using 64 types and format directives in printf that only work when CFG_64BIT_VSPRINTF is defined. Yeah, that's one more of these ugly bugs. So what's the best way to fix this? Here are four solutions. My favorite is no. 2. 1) Define CFG_64BIT_STRTOUL for all effected board. Currently all 405 boards have memsize output as 0 in bdinfo. 2) Define CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL for all 4xx boards in include/ppc4xx.h: ... 3) Generally define CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL for all boards. 4) Use an (ugly) workaround in common/cmd_bdinfo.c: I vote for # 5: 5) Delete al references to CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL and unconditionally enable it for all boards. Any takers to submit a patch? 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: [EMAIL PROTECTED] One day, said a dull voice from down below, I'm going to be back in form again and you're going to be very sorry you said that. For a very long time. I might even go so far as to make even more Time just for you to be sorry in. - Terry Pratchett, _Small Gods_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CFG_64BIT_xxx and friends
On 01:12 Sun 07 Sep , Wolfgang Denk wrote: Dear Matthias, in message [EMAIL PROTECTED] you wrote: after testing the recent U-Boot code on a couple of 405EP boards I noticed, that the memsize in the output of the bdinfo command is always 0x. This is caused by using 64 types and format directives in printf that only work when CFG_64BIT_VSPRINTF is defined. Yeah, that's one more of these ugly bugs. So what's the best way to fix this? Here are four solutions. My favorite is no. 2. 1) Define CFG_64BIT_STRTOUL for all effected board. Currently all 405 boards have memsize output as 0 in bdinfo. 2) Define CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL for all 4xx boards in include/ppc4xx.h: ... 3) Generally define CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL for all boards. 4) Use an (ugly) workaround in common/cmd_bdinfo.c: I vote for # 5: 5) Delete al references to CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL and unconditionally enable it for all boards. Any takers to submit a patch? If possible not because it will increase the size of u-boot for board which not need it. When we will have kconfig it will be simple to activated it for ppc and other board automaticly. Best Regards, J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] CFG_64BIT_xxx and friends
Dear Jean-Christophe PLAGNIOL-VILLARD, In message [EMAIL PROTECTED] you wrote: 5) Delete al references to CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL and unconditionally enable it for all boards. Any takers to submit a patch? If possible not because it will increase the size of u-boot for board which not need it. When we will have kconfig it will be simple to activated it for ppc and other board automaticly. You try to beat me on being the guard dog of memory footprint? That'll be a hard job ;-) Seriously: How much of code size are we talking about? And activating / deactivation the feature is not so trivial as it affects the printf() format specifiers we have to use. 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: [EMAIL PROTECTED] We're all sorry for the other guy when he loses his job to a machine. But when it comes to your job -- that's different. And it always will be different. -- McCoy, The Ultimate Computer, stardate 4729.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] CFG_64BIT_xxx and friends
Hi all,
after testing the recent U-Boot code on a couple of 405EP boards I noticed,
that the memsize in the output of the bdinfo command is always 0x.
This is caused by using 64 types and format directives in printf that only
work when CFG_64BIT_VSPRINTF is defined.
So what's the best way to fix this?
Here are four solutions. My favorite is no. 2.
1) Define CFG_64BIT_STRTOUL for all effected board.
Currently all 405 boards have memsize output as 0 in bdinfo.
2) Define CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL for all 4xx boards in
include/ppc4xx.h:
diff --git a/include/ppc4xx.h b/include/ppc4xx.h
index 59a3b06..f0dfa38 100644
--- a/include/ppc4xx.h
+++ b/include/ppc4xx.h
@@ -102,13 +102,14 @@
#endif /* 440EP/EPX 440GR/GRX 440SP/SPE 460EX/GT/SX 405EX*/
-#if defined(CONFIG_440)
/*
* Enable long long (%ll ...) printf format on 440 PPC's since most of
* them support 36bit physical addressing
*/
#define CFG_64BIT_VSPRINTF
#define CFG_64BIT_STRTOUL
+
+#if defined(CONFIG_440)
#include ppc440.h
#else
#include ppc405.h
3) Generally define CFG_64BIT_VSPRINTF and CFG_64BIT_STRTOUL for all boards.
4) Use an (ugly) workaround in common/cmd_bdinfo.c:
static void print_lnum(const char *name, u64 value)
{
#ifdef CFG_64BIT_VSPRINTF
printf (%-12s= 0x%.8llX\n, name, value);
#else
u32 *value32 = (u32*)value;
if (value32[0])
printf (%-12s= 0x%lX%.8lX\n, name, value32[0], value32[1]);
else
printf (%-12s= 0x%.8lX\n, name, value32[1]);
#endif
}
So what do you think?
Matthias
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
