* [U-Boot] __FILE__ usage and and SPL limits for SRAM
@ 2017-03-27 21:44 Nishanth Menon
2017-03-28 6:07 ` Lokesh Vutla
2017-03-28 6:29 ` Masahiro Yamada
0 siblings, 2 replies; 16+ messages in thread
From: Nishanth Menon @ 2017-03-27 21:44 UTC (permalink / raw)
To: u-boot
Hi,
we've kind of run into an interesting situation recently, but might be
of interest for various folks trying to reduce the image sizes.
our AM335x device has a limited amount of sram.. and the SPL tries to
fit into it (a bit tricky given the restricted space we have on it on
certain class of devices).
arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored
around this.
Key in this is:
. = ALIGN(4);
.rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram
. = ALIGN(4);
.data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
Now, our jenkins build system happens to use a varied build path and
uses O= path. to simplify the details:
mkdir
/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc
mkdir
/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b
git clone u-boot
cd u-boot
git clean -fdx
make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig
make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all
depending on depth of the path, this would fail.. a little bit of
headscratching later..
when using O= build system uses absolute paths, which translates to
__FILE__ being absolute paths as well..
in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file
path in rodata.
So, depending on how deep the path is rodata size varies and ends up
pushing .data out of sram max range.
we dont really care to put a print of complete absolute path anyways,
and I am not really sure of a clean way to resolve this:
a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in
b) replace usage of __FILE__ with something like __FILENAME__ as
recommended by [1]
What is the suggestion we do?
[1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 16+ messages in thread* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-03-27 21:44 [U-Boot] __FILE__ usage and and SPL limits for SRAM Nishanth Menon @ 2017-03-28 6:07 ` Lokesh Vutla 2017-04-04 19:06 ` Tom Rini 2017-03-28 6:29 ` Masahiro Yamada 1 sibling, 1 reply; 16+ messages in thread From: Lokesh Vutla @ 2017-03-28 6:07 UTC (permalink / raw) To: u-boot + more folks. On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote: > Hi, > > we've kind of run into an interesting situation recently, but might be > of interest for various folks trying to reduce the image sizes. > > our AM335x device has a limited amount of sram.. and the SPL tries to > fit into it (a bit tricky given the restricted space we have on it on > certain class of devices). > > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored > around this. > > Key in this is: > . = ALIGN(4); > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram > > . = ALIGN(4); > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram > > > Now, our jenkins build system happens to use a varied build path and > uses O= path. to simplify the details: > mkdir > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc > > mkdir > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b > > > git clone u-boot > cd u-boot > > git clean -fdx > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all > > depending on depth of the path, this would fail.. a little bit of > headscratching later.. > when using O= build system uses absolute paths, which translates to > __FILE__ being absolute paths as well.. > > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file > path in rodata. > > So, depending on how deep the path is rodata size varies and ends up > pushing .data out of sram max range. > > we dont really care to put a print of complete absolute path anyways, > and I am not really sure of a clean way to resolve this: > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in > b) replace usage of __FILE__ with something like __FILENAME__ as > recommended by [1] > > > What is the suggestion we do? > > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path Any suggestions would be really helpful. Thanks and regards, Lokesh ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-03-28 6:07 ` Lokesh Vutla @ 2017-04-04 19:06 ` Tom Rini 2017-04-05 11:27 ` Wolfgang Denk 2017-04-09 19:27 ` Simon Glass 0 siblings, 2 replies; 16+ messages in thread From: Tom Rini @ 2017-04-04 19:06 UTC (permalink / raw) To: u-boot On Tue, Mar 28, 2017 at 11:37:45AM +0530, Lokesh Vutla wrote: > + more folks. > > On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote: > > Hi, > > > > we've kind of run into an interesting situation recently, but might be > > of interest for various folks trying to reduce the image sizes. > > > > our AM335x device has a limited amount of sram.. and the SPL tries to > > fit into it (a bit tricky given the restricted space we have on it on > > certain class of devices). > > > > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored > > around this. > > > > Key in this is: > > . = ALIGN(4); > > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram > > > > . = ALIGN(4); > > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram > > > > > > Now, our jenkins build system happens to use a varied build path and > > uses O= path. to simplify the details: > > mkdir > > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc > > > > mkdir > > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b > > > > > > git clone u-boot > > cd u-boot > > > > git clean -fdx > > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig > > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all > > > > depending on depth of the path, this would fail.. a little bit of > > headscratching later.. > > when using O= build system uses absolute paths, which translates to > > __FILE__ being absolute paths as well.. > > > > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file > > path in rodata. > > > > So, depending on how deep the path is rodata size varies and ends up > > pushing .data out of sram max range. > > > > we dont really care to put a print of complete absolute path anyways, > > and I am not really sure of a clean way to resolve this: > > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in > > b) replace usage of __FILE__ with something like __FILENAME__ as > > recommended by [1] > > > > > > What is the suggestion we do? > > > > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path > > Any suggestions would be really helpful. So here's what I've come up with, and it's slightly incomplete: diff --git a/include/common.h b/include/common.h index 2cbbd5a60cd3..cdc61ef5b144 100644 --- a/include/common.h +++ b/include/common.h @@ -145,20 +145,19 @@ typedef volatile unsigned char vu_char; * in any case these failing assertions cannot be fixed with a reset (which * may just do the same assertion again). */ -void __assert_fail(const char *assertion, const char *file, unsigned line, - const char *function); +void __assert_fail(const char *assertion, unsigned line, const char *function); #define assert(x) \ ({ if (!(x) && _DEBUG) \ - __assert_fail(#x, __FILE__, __LINE__, __func__); }) + __assert_fail(#x, __LINE__, __func__); }) #define error(fmt, args...) do { \ - printf("ERROR: " pr_fmt(fmt) "\nat %s:%d/%s()\n", \ - ##args, __FILE__, __LINE__, __func__); \ + printf("ERROR: " pr_fmt(fmt) "\nat %s():%d\n", \ + ##args, __func__, __LINE__); \ } while (0) #ifndef BUG #define BUG() do { \ - printf("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \ + printf("BUG: failure at %s():%d!\n", __FUNCTION__, __LINE__); \ panic("BUG!"); \ } while (0) #define BUG_ON(condition) do { if (unlikely((condition)!=0)) BUG(); } while(0) diff --git a/include/image.h b/include/image.h index 237251896029..a52c5355376e 100644 --- a/include/image.h +++ b/include/image.h @@ -1218,12 +1218,12 @@ static inline int fit_image_check_target_arch(const void *fdt, int node) #ifdef CONFIG_FIT_VERBOSE #define fit_unsupported(msg) printf("! %s:%d " \ "FIT images not supported for '%s'\n", \ - __FILE__, __LINE__, (msg)) + __func__, __LINE__, (msg)) #define fit_unsupported_reset(msg) printf("! %s:%d " \ "FIT images not supported for '%s' " \ "- must reset board to recover!\n", \ - __FILE__, __LINE__, (msg)) + __func__, __LINE__, (msg)) #else #define fit_unsupported(msg) #define fit_unsupported_reset(msg) diff --git a/include/linux/compat.h b/include/linux/compat.h index a43e4d66983d..db9fcb6d47c2 100644 --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -100,14 +100,14 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep) #ifndef BUG #define BUG() do { \ - printf("U-Boot BUG at %s:%d!\n", __FILE__, __LINE__); \ + printf("U-Boot BUG at %s:%d!\n", __func__, __LINE__); \ } while (0) #define BUG_ON(condition) do { if (condition) BUG(); } while(0) #endif /* BUG */ #define WARN_ON(x) if (x) {printf("WARNING in %s line %d\n" \ - , __FILE__, __LINE__); } + , __func__, __LINE__); } #define PAGE_SIZE 4096 diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index 6def8f98aa41..b620463174d7 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -228,11 +228,9 @@ int snprintf(char *buf, size_t size, const char *fmt, ...) return ret; } -void __assert_fail(const char *assertion, const char *file, unsigned line, - const char *function) +void __assert_fail(const char *assertion, unsigned line, const char *function) { /* This will not return */ - printf("%s:%u: %s: Assertion `%s' failed.", file, line, function, - assertion); + printf("%s:%u: Assertion `%s' failed.", function, line, assertion); hang(); } diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 874a2951f705..d14cd67b3817 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -722,12 +722,10 @@ int vprintf(const char *fmt, va_list args) } -void __assert_fail(const char *assertion, const char *file, unsigned line, - const char *function) +void __assert_fail(const char *assertion, unsigned line, const char *function) { /* This will not return */ - panic("%s:%u: %s: Assertion `%s' failed.", file, line, function, - assertion); + panic("%s:%u: Assertion `%s' failed.", function, line, assertion); } char *simple_itoa(ulong i) Why? I'm not sure that in most cases __FILE__ is providing any more useful infomration on top of what we have from __func__ and __LINE__. That said, converting __assert_fail is probably not a good idea as it'll lead to cases (probably) where we don't have enough context to know where the problem really was, but with everything else being a macro it should evaluate out as useful as before at least. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170404/468e54c0/attachment.sig> ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-04-04 19:06 ` Tom Rini @ 2017-04-05 11:27 ` Wolfgang Denk 2017-04-09 19:27 ` Simon Glass 1 sibling, 0 replies; 16+ messages in thread From: Wolfgang Denk @ 2017-04-05 11:27 UTC (permalink / raw) To: u-boot Dear Tom, In message <20170404190647.GH19897@bill-the-cat> you wrote: > > Why? I'm not sure that in most cases __FILE__ is providing any more > useful infomration on top of what we have from __func__ and __LINE__. Is there not quite a lot of functions that are implemented in many files? I expect there are several hundrets of files which include an implementation of board_init() , for example. Similar for things like dram_init(), cpu_eth_init(), board_eth_init(), OK, you can argument that you know which board this is so you can pin this down easily, but it basically makes all tool based access (say, grep) very hard or impossible. Having the file name is somthing that is _really_ helpful in identifying where the code comes from. If we cannot find a way to use short file names (relative to the top level source directory or such), maybe we can use some other uniqe identifier for this file? For example, we could use the output of "git hash-object", eventually truncated so some reasonable length - 8 characters? Similar to: -> ID=`git hash-object Makefile | head -c 8` -> echo $ID d44af786 -> git show $ID ? OK, this requires the code to be in a git repository, but should we really care about people who don't use git? ;-) Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk 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 The greatest threat towards future is indifference. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-04-04 19:06 ` Tom Rini 2017-04-05 11:27 ` Wolfgang Denk @ 2017-04-09 19:27 ` Simon Glass 2017-04-22 5:30 ` Masahiro Yamada 1 sibling, 1 reply; 16+ messages in thread From: Simon Glass @ 2017-04-09 19:27 UTC (permalink / raw) To: u-boot Hi Tom, On 4 April 2017 at 13:06, Tom Rini <trini@konsulko.com> wrote: > On Tue, Mar 28, 2017 at 11:37:45AM +0530, Lokesh Vutla wrote: >> + more folks. >> >> On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote: >> > Hi, >> > >> > we've kind of run into an interesting situation recently, but might be >> > of interest for various folks trying to reduce the image sizes. >> > >> > our AM335x device has a limited amount of sram.. and the SPL tries to >> > fit into it (a bit tricky given the restricted space we have on it on >> > certain class of devices). >> > >> > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored >> > around this. >> > >> > Key in this is: >> > . = ALIGN(4); >> > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram >> > >> > . = ALIGN(4); >> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram >> > >> > >> > Now, our jenkins build system happens to use a varied build path and >> > uses O= path. to simplify the details: >> > mkdir >> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc >> > >> > mkdir >> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b >> > >> > >> > git clone u-boot >> > cd u-boot >> > >> > git clean -fdx >> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig >> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all >> > >> > depending on depth of the path, this would fail.. a little bit of >> > headscratching later.. >> > when using O= build system uses absolute paths, which translates to >> > __FILE__ being absolute paths as well.. >> > >> > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file >> > path in rodata. >> > >> > So, depending on how deep the path is rodata size varies and ends up >> > pushing .data out of sram max range. >> > >> > we dont really care to put a print of complete absolute path anyways, >> > and I am not really sure of a clean way to resolve this: >> > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in >> > b) replace usage of __FILE__ with something like __FILENAME__ as >> > recommended by [1] >> > >> > >> > What is the suggestion we do? >> > >> > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path >> >> Any suggestions would be really helpful. > > So here's what I've come up with, and it's slightly incomplete: > diff --git a/include/common.h b/include/common.h > index 2cbbd5a60cd3..cdc61ef5b144 100644 > --- a/include/common.h > +++ b/include/common.h > @@ -145,20 +145,19 @@ typedef volatile unsigned char vu_char; > * in any case these failing assertions cannot be fixed with a reset (which > * may just do the same assertion again). > */ > -void __assert_fail(const char *assertion, const char *file, unsigned line, > - const char *function); > +void __assert_fail(const char *assertion, unsigned line, const char *function); > #define assert(x) \ > ({ if (!(x) && _DEBUG) \ > - __assert_fail(#x, __FILE__, __LINE__, __func__); }) > + __assert_fail(#x, __LINE__, __func__); }) > > #define error(fmt, args...) do { \ > - printf("ERROR: " pr_fmt(fmt) "\nat %s:%d/%s()\n", \ > - ##args, __FILE__, __LINE__, __func__); \ > + printf("ERROR: " pr_fmt(fmt) "\nat %s():%d\n", \ > + ##args, __func__, __LINE__); \ > } while (0) > > #ifndef BUG > #define BUG() do { \ > - printf("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \ > + printf("BUG: failure at %s():%d!\n", __FUNCTION__, __LINE__); \ > panic("BUG!"); \ > } while (0) > #define BUG_ON(condition) do { if (unlikely((condition)!=0)) BUG(); } while(0) > diff --git a/include/image.h b/include/image.h > index 237251896029..a52c5355376e 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -1218,12 +1218,12 @@ static inline int fit_image_check_target_arch(const void *fdt, int node) > #ifdef CONFIG_FIT_VERBOSE > #define fit_unsupported(msg) printf("! %s:%d " \ > "FIT images not supported for '%s'\n", \ > - __FILE__, __LINE__, (msg)) > + __func__, __LINE__, (msg)) > > #define fit_unsupported_reset(msg) printf("! %s:%d " \ > "FIT images not supported for '%s' " \ > "- must reset board to recover!\n", \ > - __FILE__, __LINE__, (msg)) > + __func__, __LINE__, (msg)) > #else > #define fit_unsupported(msg) > #define fit_unsupported_reset(msg) > diff --git a/include/linux/compat.h b/include/linux/compat.h > index a43e4d66983d..db9fcb6d47c2 100644 > --- a/include/linux/compat.h > +++ b/include/linux/compat.h > @@ -100,14 +100,14 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep) > > #ifndef BUG > #define BUG() do { \ > - printf("U-Boot BUG at %s:%d!\n", __FILE__, __LINE__); \ > + printf("U-Boot BUG at %s:%d!\n", __func__, __LINE__); \ > } while (0) > > #define BUG_ON(condition) do { if (condition) BUG(); } while(0) > #endif /* BUG */ > > #define WARN_ON(x) if (x) {printf("WARNING in %s line %d\n" \ > - , __FILE__, __LINE__); } > + , __func__, __LINE__); } > > #define PAGE_SIZE 4096 > > diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c > index 6def8f98aa41..b620463174d7 100644 > --- a/lib/tiny-printf.c > +++ b/lib/tiny-printf.c > @@ -228,11 +228,9 @@ int snprintf(char *buf, size_t size, const char *fmt, ...) > return ret; > } > > -void __assert_fail(const char *assertion, const char *file, unsigned line, > - const char *function) > +void __assert_fail(const char *assertion, unsigned line, const char *function) > { > /* This will not return */ > - printf("%s:%u: %s: Assertion `%s' failed.", file, line, function, > - assertion); > + printf("%s:%u: Assertion `%s' failed.", function, line, assertion); > hang(); > } > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 874a2951f705..d14cd67b3817 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -722,12 +722,10 @@ int vprintf(const char *fmt, va_list args) > } > > > -void __assert_fail(const char *assertion, const char *file, unsigned line, > - const char *function) > +void __assert_fail(const char *assertion, unsigned line, const char *function) > { > /* This will not return */ > - panic("%s:%u: %s: Assertion `%s' failed.", file, line, function, > - assertion); > + panic("%s:%u: Assertion `%s' failed.", function, line, assertion); > } > > char *simple_itoa(ulong i) > > Why? I'm not sure that in most cases __FILE__ is providing any more > useful infomration on top of what we have from __func__ and __LINE__. > That said, converting __assert_fail is probably not a good idea as it'll > lead to cases (probably) where we don't have enough context to know > where the problem really was, but with everything else being a macro it > should evaluate out as useful as before at least. I agree with this - the function name is normally more useful - it provides immediate context as to what the error relates to, assuming function names are sensible and functions are not too long. If we were to use a filename, it would be great if it could be relative to the U-Boot source tree somehow. Regards, Simon ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-04-09 19:27 ` Simon Glass @ 2017-04-22 5:30 ` Masahiro Yamada 2017-05-22 17:23 ` Denys Dmytriyenko 0 siblings, 1 reply; 16+ messages in thread From: Masahiro Yamada @ 2017-04-22 5:30 UTC (permalink / raw) To: u-boot 2017-04-10 4:27 GMT+09:00 Simon Glass <sjg@chromium.org>: > Hi Tom, > > On 4 April 2017 at 13:06, Tom Rini <trini@konsulko.com> wrote: >> On Tue, Mar 28, 2017 at 11:37:45AM +0530, Lokesh Vutla wrote: >>> + more folks. >>> >>> On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote: >>> > Hi, >>> > >>> > we've kind of run into an interesting situation recently, but might be >>> > of interest for various folks trying to reduce the image sizes. >>> > >>> > our AM335x device has a limited amount of sram.. and the SPL tries to >>> > fit into it (a bit tricky given the restricted space we have on it on >>> > certain class of devices). >>> > >>> > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored >>> > around this. >>> > >>> > Key in this is: >>> > . = ALIGN(4); >>> > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram >>> > >>> > . = ALIGN(4); >>> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram >>> > >>> > >>> > Now, our jenkins build system happens to use a varied build path and >>> > uses O= path. to simplify the details: >>> > mkdir >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc >>> > >>> > mkdir >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b >>> > >>> > >>> > git clone u-boot >>> > cd u-boot >>> > >>> > git clean -fdx >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all >>> > >>> > depending on depth of the path, this would fail.. a little bit of >>> > headscratching later.. >>> > when using O= build system uses absolute paths, which translates to >>> > __FILE__ being absolute paths as well.. >>> > >>> > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file >>> > path in rodata. >>> > >>> > So, depending on how deep the path is rodata size varies and ends up >>> > pushing .data out of sram max range. >>> > >>> > we dont really care to put a print of complete absolute path anyways, >>> > and I am not really sure of a clean way to resolve this: >>> > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in >>> > b) replace usage of __FILE__ with something like __FILENAME__ as >>> > recommended by [1] >>> > >>> > >>> > What is the suggestion we do? >>> > >>> > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path >>> >>> Any suggestions would be really helpful. >> >> So here's what I've come up with, and it's slightly incomplete: >> diff --git a/include/common.h b/include/common.h >> index 2cbbd5a60cd3..cdc61ef5b144 100644 >> --- a/include/common.h >> +++ b/include/common.h >> @@ -145,20 +145,19 @@ typedef volatile unsigned char vu_char; >> * in any case these failing assertions cannot be fixed with a reset (which >> * may just do the same assertion again). >> */ >> -void __assert_fail(const char *assertion, const char *file, unsigned line, >> - const char *function); >> +void __assert_fail(const char *assertion, unsigned line, const char *function); >> #define assert(x) \ >> ({ if (!(x) && _DEBUG) \ >> - __assert_fail(#x, __FILE__, __LINE__, __func__); }) >> + __assert_fail(#x, __LINE__, __func__); }) >> >> #define error(fmt, args...) do { \ >> - printf("ERROR: " pr_fmt(fmt) "\nat %s:%d/%s()\n", \ >> - ##args, __FILE__, __LINE__, __func__); \ >> + printf("ERROR: " pr_fmt(fmt) "\nat %s():%d\n", \ >> + ##args, __func__, __LINE__); \ >> } while (0) >> >> #ifndef BUG >> #define BUG() do { \ >> - printf("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \ >> + printf("BUG: failure at %s():%d!\n", __FUNCTION__, __LINE__); \ >> panic("BUG!"); \ >> } while (0) >> #define BUG_ON(condition) do { if (unlikely((condition)!=0)) BUG(); } while(0) >> diff --git a/include/image.h b/include/image.h >> index 237251896029..a52c5355376e 100644 >> --- a/include/image.h >> +++ b/include/image.h >> @@ -1218,12 +1218,12 @@ static inline int fit_image_check_target_arch(const void *fdt, int node) >> #ifdef CONFIG_FIT_VERBOSE >> #define fit_unsupported(msg) printf("! %s:%d " \ >> "FIT images not supported for '%s'\n", \ >> - __FILE__, __LINE__, (msg)) >> + __func__, __LINE__, (msg)) >> >> #define fit_unsupported_reset(msg) printf("! %s:%d " \ >> "FIT images not supported for '%s' " \ >> "- must reset board to recover!\n", \ >> - __FILE__, __LINE__, (msg)) >> + __func__, __LINE__, (msg)) >> #else >> #define fit_unsupported(msg) >> #define fit_unsupported_reset(msg) >> diff --git a/include/linux/compat.h b/include/linux/compat.h >> index a43e4d66983d..db9fcb6d47c2 100644 >> --- a/include/linux/compat.h >> +++ b/include/linux/compat.h >> @@ -100,14 +100,14 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep) >> >> #ifndef BUG >> #define BUG() do { \ >> - printf("U-Boot BUG at %s:%d!\n", __FILE__, __LINE__); \ >> + printf("U-Boot BUG at %s:%d!\n", __func__, __LINE__); \ >> } while (0) >> >> #define BUG_ON(condition) do { if (condition) BUG(); } while(0) >> #endif /* BUG */ >> >> #define WARN_ON(x) if (x) {printf("WARNING in %s line %d\n" \ >> - , __FILE__, __LINE__); } >> + , __func__, __LINE__); } >> >> #define PAGE_SIZE 4096 >> >> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c >> index 6def8f98aa41..b620463174d7 100644 >> --- a/lib/tiny-printf.c >> +++ b/lib/tiny-printf.c >> @@ -228,11 +228,9 @@ int snprintf(char *buf, size_t size, const char *fmt, ...) >> return ret; >> } >> >> -void __assert_fail(const char *assertion, const char *file, unsigned line, >> - const char *function) >> +void __assert_fail(const char *assertion, unsigned line, const char *function) >> { >> /* This will not return */ >> - printf("%s:%u: %s: Assertion `%s' failed.", file, line, function, >> - assertion); >> + printf("%s:%u: Assertion `%s' failed.", function, line, assertion); >> hang(); >> } >> diff --git a/lib/vsprintf.c b/lib/vsprintf.c >> index 874a2951f705..d14cd67b3817 100644 >> --- a/lib/vsprintf.c >> +++ b/lib/vsprintf.c >> @@ -722,12 +722,10 @@ int vprintf(const char *fmt, va_list args) >> } >> >> >> -void __assert_fail(const char *assertion, const char *file, unsigned line, >> - const char *function) >> +void __assert_fail(const char *assertion, unsigned line, const char *function) >> { >> /* This will not return */ >> - panic("%s:%u: %s: Assertion `%s' failed.", file, line, function, >> - assertion); >> + panic("%s:%u: Assertion `%s' failed.", function, line, assertion); >> } >> >> char *simple_itoa(ulong i) >> >> Why? I'm not sure that in most cases __FILE__ is providing any more >> useful infomration on top of what we have from __func__ and __LINE__. >> That said, converting __assert_fail is probably not a good idea as it'll >> lead to cases (probably) where we don't have enough context to know >> where the problem really was, but with everything else being a macro it >> should evaluate out as useful as before at least. > > I agree with this - the function name is normally more useful - it > provides immediate context as to what the error relates to, assuming > function names are sensible and functions are not too long. > > If we were to use a filename, it would be great if it could be > relative to the U-Boot source tree somehow. > > Regards, > Simon I can do this, but I'd like to move forward synced with Linux. Yesterday, I took some time to write patches and sent them to Linux ML. Plan A: https://lkml.org/lkml/2017/4/21/623 https://patchwork.kernel.org/patch/9693559/ https://patchwork.kernel.org/patch/9693563/ Plan B: https://patchwork.kernel.org/patch/9693623/ Let's see how it goes. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-04-22 5:30 ` Masahiro Yamada @ 2017-05-22 17:23 ` Denys Dmytriyenko 2017-05-23 0:57 ` Masahiro Yamada 0 siblings, 1 reply; 16+ messages in thread From: Denys Dmytriyenko @ 2017-05-22 17:23 UTC (permalink / raw) To: u-boot On Sat, Apr 22, 2017 at 02:30:09PM +0900, Masahiro Yamada wrote: > >>> On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote: > >>> > Hi, > >>> > > >>> > we've kind of run into an interesting situation recently, but might be > >>> > of interest for various folks trying to reduce the image sizes. > >>> > > >>> > our AM335x device has a limited amount of sram.. and the SPL tries to > >>> > fit into it (a bit tricky given the restricted space we have on it on > >>> > certain class of devices). > >>> > > >>> > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored > >>> > around this. > >>> > > >>> > Key in this is: > >>> > . = ALIGN(4); > >>> > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram > >>> > > >>> > . = ALIGN(4); > >>> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram > >>> > > >>> > > >>> > Now, our jenkins build system happens to use a varied build path and > >>> > uses O= path. to simplify the details: > >>> > mkdir > >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc > >>> > > >>> > mkdir > >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b > >>> > > >>> > > >>> > git clone u-boot > >>> > cd u-boot > >>> > > >>> > git clean -fdx > >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig > >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all > >>> > > >>> > depending on depth of the path, this would fail.. a little bit of > >>> > headscratching later.. > >>> > when using O= build system uses absolute paths, which translates to > >>> > __FILE__ being absolute paths as well.. > >>> > > >>> > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file > >>> > path in rodata. > >>> > > >>> > So, depending on how deep the path is rodata size varies and ends up > >>> > pushing .data out of sram max range. > >>> > > >>> > we dont really care to put a print of complete absolute path anyways, > >>> > and I am not really sure of a clean way to resolve this: > >>> > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in > >>> > b) replace usage of __FILE__ with something like __FILENAME__ as > >>> > recommended by [1] > >>> > > >>> > > >>> > What is the suggestion we do? > >>> > > >>> > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path <snip> > I can do this, but > I'd like to move forward synced with Linux. > > > Yesterday, I took some time to write patches > and sent them to Linux ML. > > > Plan A: > https://lkml.org/lkml/2017/4/21/623 > https://patchwork.kernel.org/patch/9693559/ > https://patchwork.kernel.org/patch/9693563/ > > > Plan B: > https://patchwork.kernel.org/patch/9693623/ FWIW: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268 -- Denys ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-05-22 17:23 ` Denys Dmytriyenko @ 2017-05-23 0:57 ` Masahiro Yamada 2017-05-23 1:27 ` Tom Rini 0 siblings, 1 reply; 16+ messages in thread From: Masahiro Yamada @ 2017-05-23 0:57 UTC (permalink / raw) To: u-boot Hi Denys, 2017-05-23 2:23 GMT+09:00 Denys Dmytriyenko <denis@denix.org>: > On Sat, Apr 22, 2017 at 02:30:09PM +0900, Masahiro Yamada wrote: >> >>> On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote: >> >>> > Hi, >> >>> > >> >>> > we've kind of run into an interesting situation recently, but might be >> >>> > of interest for various folks trying to reduce the image sizes. >> >>> > >> >>> > our AM335x device has a limited amount of sram.. and the SPL tries to >> >>> > fit into it (a bit tricky given the restricted space we have on it on >> >>> > certain class of devices). >> >>> > >> >>> > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored >> >>> > around this. >> >>> > >> >>> > Key in this is: >> >>> > . = ALIGN(4); >> >>> > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram >> >>> > >> >>> > . = ALIGN(4); >> >>> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram >> >>> > >> >>> > >> >>> > Now, our jenkins build system happens to use a varied build path and >> >>> > uses O= path. to simplify the details: >> >>> > mkdir >> >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc >> >>> > >> >>> > mkdir >> >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b >> >>> > >> >>> > >> >>> > git clone u-boot >> >>> > cd u-boot >> >>> > >> >>> > git clean -fdx >> >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig >> >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all >> >>> > >> >>> > depending on depth of the path, this would fail.. a little bit of >> >>> > headscratching later.. >> >>> > when using O= build system uses absolute paths, which translates to >> >>> > __FILE__ being absolute paths as well.. >> >>> > >> >>> > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file >> >>> > path in rodata. >> >>> > >> >>> > So, depending on how deep the path is rodata size varies and ends up >> >>> > pushing .data out of sram max range. >> >>> > >> >>> > we dont really care to put a print of complete absolute path anyways, >> >>> > and I am not really sure of a clean way to resolve this: >> >>> > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in >> >>> > b) replace usage of __FILE__ with something like __FILENAME__ as >> >>> > recommended by [1] >> >>> > >> >>> > >> >>> > What is the suggestion we do? >> >>> > >> >>> > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path > > <snip> > >> I can do this, but >> I'd like to move forward synced with Linux. >> >> >> Yesterday, I took some time to write patches >> and sent them to Linux ML. >> >> >> Plan A: >> https://lkml.org/lkml/2017/4/21/623 >> https://patchwork.kernel.org/patch/9693559/ >> https://patchwork.kernel.org/patch/9693563/ >> >> >> Plan B: >> https://patchwork.kernel.org/patch/9693623/ > > FWIW: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268 > > -- > Denys Thanks for you pointer! If merged, it will reduce the image size, and also helpful for reproducible build. (We can not save users of old GCC versions, though...) -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-05-23 0:57 ` Masahiro Yamada @ 2017-05-23 1:27 ` Tom Rini 2017-05-31 4:51 ` Masahiro Yamada 0 siblings, 1 reply; 16+ messages in thread From: Tom Rini @ 2017-05-23 1:27 UTC (permalink / raw) To: u-boot On Tue, May 23, 2017 at 09:57:10AM +0900, Masahiro Yamada wrote: > Hi Denys, > > > 2017-05-23 2:23 GMT+09:00 Denys Dmytriyenko <denis@denix.org>: > > On Sat, Apr 22, 2017 at 02:30:09PM +0900, Masahiro Yamada wrote: > >> >>> On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote: > >> >>> > Hi, > >> >>> > > >> >>> > we've kind of run into an interesting situation recently, but might be > >> >>> > of interest for various folks trying to reduce the image sizes. > >> >>> > > >> >>> > our AM335x device has a limited amount of sram.. and the SPL tries to > >> >>> > fit into it (a bit tricky given the restricted space we have on it on > >> >>> > certain class of devices). > >> >>> > > >> >>> > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored > >> >>> > around this. > >> >>> > > >> >>> > Key in this is: > >> >>> > . = ALIGN(4); > >> >>> > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram > >> >>> > > >> >>> > . = ALIGN(4); > >> >>> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram > >> >>> > > >> >>> > > >> >>> > Now, our jenkins build system happens to use a varied build path and > >> >>> > uses O= path. to simplify the details: > >> >>> > mkdir > >> >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc > >> >>> > > >> >>> > mkdir > >> >>> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b > >> >>> > > >> >>> > > >> >>> > git clone u-boot > >> >>> > cd u-boot > >> >>> > > >> >>> > git clean -fdx > >> >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig > >> >>> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all > >> >>> > > >> >>> > depending on depth of the path, this would fail.. a little bit of > >> >>> > headscratching later.. > >> >>> > when using O= build system uses absolute paths, which translates to > >> >>> > __FILE__ being absolute paths as well.. > >> >>> > > >> >>> > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file > >> >>> > path in rodata. > >> >>> > > >> >>> > So, depending on how deep the path is rodata size varies and ends up > >> >>> > pushing .data out of sram max range. > >> >>> > > >> >>> > we dont really care to put a print of complete absolute path anyways, > >> >>> > and I am not really sure of a clean way to resolve this: > >> >>> > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in > >> >>> > b) replace usage of __FILE__ with something like __FILENAME__ as > >> >>> > recommended by [1] > >> >>> > > >> >>> > > >> >>> > What is the suggestion we do? > >> >>> > > >> >>> > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path > > > > <snip> > > > >> I can do this, but > >> I'd like to move forward synced with Linux. > >> > >> > >> Yesterday, I took some time to write patches > >> and sent them to Linux ML. > >> > >> > >> Plan A: > >> https://lkml.org/lkml/2017/4/21/623 > >> https://patchwork.kernel.org/patch/9693559/ > >> https://patchwork.kernel.org/patch/9693563/ > >> > >> > >> Plan B: > >> https://patchwork.kernel.org/patch/9693623/ > > > > FWIW: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268 > > Thanks for you pointer! > > If merged, it will reduce the image size, and also helpful for > reproducible build. > > (We can not save users of old GCC versions, though...) Did the kernel side of this die out? Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170522/273b2169/attachment.sig> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-05-23 1:27 ` Tom Rini @ 2017-05-31 4:51 ` Masahiro Yamada 2017-05-31 11:32 ` Tom Rini 0 siblings, 1 reply; 16+ messages in thread From: Masahiro Yamada @ 2017-05-31 4:51 UTC (permalink / raw) To: u-boot Hi Tom 2017-05-23 10:27 GMT+09:00 Tom Rini <trini@konsulko.com>: >> > >> > <snip> >> > >> >> I can do this, but >> >> I'd like to move forward synced with Linux. >> >> >> >> >> >> Yesterday, I took some time to write patches >> >> and sent them to Linux ML. >> >> >> >> >> >> Plan A: >> >> https://lkml.org/lkml/2017/4/21/623 >> >> https://patchwork.kernel.org/patch/9693559/ >> >> https://patchwork.kernel.org/patch/9693563/ >> >> >> >> >> >> Plan B: >> >> https://patchwork.kernel.org/patch/9693623/ >> > >> > FWIW: >> > >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268 >> >> Thanks for you pointer! >> >> If merged, it will reduce the image size, and also helpful for >> reproducible build. >> >> (We can not save users of old GCC versions, though...) > > Did the kernel side of this die out? Thanks! It did not. At least, I got some positive comments, but I am still wondering what the best approach would be. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-05-31 4:51 ` Masahiro Yamada @ 2017-05-31 11:32 ` Tom Rini 0 siblings, 0 replies; 16+ messages in thread From: Tom Rini @ 2017-05-31 11:32 UTC (permalink / raw) To: u-boot On Wed, May 31, 2017 at 01:51:30PM +0900, Masahiro Yamada wrote: > Hi Tom > > 2017-05-23 10:27 GMT+09:00 Tom Rini <trini@konsulko.com>: > > >> > > >> > <snip> > >> > > >> >> I can do this, but > >> >> I'd like to move forward synced with Linux. > >> >> > >> >> > >> >> Yesterday, I took some time to write patches > >> >> and sent them to Linux ML. > >> >> > >> >> > >> >> Plan A: > >> >> https://lkml.org/lkml/2017/4/21/623 > >> >> https://patchwork.kernel.org/patch/9693559/ > >> >> https://patchwork.kernel.org/patch/9693563/ > >> >> > >> >> > >> >> Plan B: > >> >> https://patchwork.kernel.org/patch/9693623/ > >> > > >> > FWIW: > >> > > >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268 > >> > >> Thanks for you pointer! > >> > >> If merged, it will reduce the image size, and also helpful for > >> reproducible build. > >> > >> (We can not save users of old GCC versions, though...) > > > > Did the kernel side of this die out? Thanks! > > > It did not. > At least, I got some positive comments, > but I am still wondering what the best approach would be. Plan A got the positive feedback. I'd be happy to have U-Boot trial that out so you can post a non-RFC to the kernel and say we've used it and ironed out any issues, if some arise. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170531/a6ed590e/attachment.sig> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-03-27 21:44 [U-Boot] __FILE__ usage and and SPL limits for SRAM Nishanth Menon 2017-03-28 6:07 ` Lokesh Vutla @ 2017-03-28 6:29 ` Masahiro Yamada 2017-03-28 11:20 ` Nishanth Menon 1 sibling, 1 reply; 16+ messages in thread From: Masahiro Yamada @ 2017-03-28 6:29 UTC (permalink / raw) To: u-boot Hi Nishanth, 2017-03-28 6:44 GMT+09:00 Nishanth Menon <nm@ti.com>: > Hi, > > we've kind of run into an interesting situation recently, but might be of > interest for various folks trying to reduce the image sizes. > > our AM335x device has a limited amount of sram.. and the SPL tries to fit > into it (a bit tricky given the restricted space we have on it on certain > class of devices). > > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored around > this. > > Key in this is: > . = ALIGN(4); > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram > > . = ALIGN(4); > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram > > > Now, our jenkins build system happens to use a varied build path and uses O= > path. to simplify the details: > mkdir > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc > mkdir > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b > > git clone u-boot > cd u-boot > > git clean -fdx > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all > > depending on depth of the path, this would fail.. a little bit of > headscratching later.. > when using O= build system uses absolute paths, which translates to __FILE__ > being absolute paths as well.. > > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file path > in rodata. > > So, depending on how deep the path is rodata size varies and ends up pushing > .data out of sram max range. > > we dont really care to put a print of complete absolute path anyways, and I > am not really sure of a clean way to resolve this: > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in > b) replace usage of __FILE__ with something like __FILENAME__ as recommended > by [1] > > > What is the suggestion we do? > > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path When O= is given, the build system runs in the object tree, not in the source tree. (This is the same as Linux.) If you see the top Makefile: ifeq ($(KBUILD_SRC),) # building in the source tree srctree := . else ifeq ($(KBUILD_SRC)/,$(dir $(CURDIR))) # building in a subdirectory of the source tree srctree := .. else srctree := $(KBUILD_SRC) endif endif If O= points to a sub-directory of the source tree, the relative path "srctree := .." is used. Otherwise, the absolute path srctree := $(KBUILD_SRC) is used. In your case, "O=../b" means the source tree and the obj tree are siblings. So, absolute path. If you want to see a short relative path for __FILE__, I'd recommend to use a sub-directory for O=. For example, your source tree is located at ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot, create a directory ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot/foo, then give O=foo Masahiro ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-03-28 6:29 ` Masahiro Yamada @ 2017-03-28 11:20 ` Nishanth Menon 2017-03-28 11:47 ` Felipe Balbi 0 siblings, 1 reply; 16+ messages in thread From: Nishanth Menon @ 2017-03-28 11:20 UTC (permalink / raw) To: u-boot Hi Masahiro-san, On Tue, Mar 28, 2017 at 1:29 AM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: [...] > > When O= is given, the build system runs in the object tree, > not in the source tree. > (This is the same as Linux.) > > If you see the top Makefile: > > ifeq ($(KBUILD_SRC),) > # building in the source tree > srctree := . > else > ifeq ($(KBUILD_SRC)/,$(dir $(CURDIR))) > # building in a subdirectory of the source tree > srctree := .. > else > srctree := $(KBUILD_SRC) > endif > endif > > > > > If O= points to a sub-directory of the source tree, > the relative path "srctree := .." is used. > > Otherwise, the absolute path srctree := $(KBUILD_SRC) is used. > In your case, "O=../b" means the source tree and the obj tree > are siblings. So, absolute path. I did simplify this a bit with O=../b to highlight exactly what you mentioned above. in our case, the O=path points to a completely different directory path. > If you want to see a short relative path for __FILE__, > I'd recommend to use a sub-directory for O=. > > For example, your source tree is located at > ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot, > create a directory ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot/foo, > then give O=foo Typical product build such as OE recipes used (in our case) does not build into the source folder, instead, all binary builds are pointed to either a temporary location or a package build location. So, while O=source_path/build would aliviate the problem, it still does'nt really solve the root of the issue. -- --- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-03-28 11:20 ` Nishanth Menon @ 2017-03-28 11:47 ` Felipe Balbi 2017-03-28 11:59 ` Nishanth Menon 2017-03-29 10:57 ` Masahiro Yamada 0 siblings, 2 replies; 16+ messages in thread From: Felipe Balbi @ 2017-03-28 11:47 UTC (permalink / raw) To: u-boot Hi, Nishanth Menon <nm@ti.com> writes: > Hi Masahiro-san, > > On Tue, Mar 28, 2017 at 1:29 AM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > [...] >> >> When O= is given, the build system runs in the object tree, >> not in the source tree. >> (This is the same as Linux.) >> >> If you see the top Makefile: >> >> ifeq ($(KBUILD_SRC),) >> # building in the source tree >> srctree := . >> else >> ifeq ($(KBUILD_SRC)/,$(dir $(CURDIR))) >> # building in a subdirectory of the source tree >> srctree := .. >> else >> srctree := $(KBUILD_SRC) >> endif >> endif >> >> >> >> >> If O= points to a sub-directory of the source tree, >> the relative path "srctree := .." is used. >> >> Otherwise, the absolute path srctree := $(KBUILD_SRC) is used. >> In your case, "O=../b" means the source tree and the obj tree >> are siblings. So, absolute path. > > I did simplify this a bit with O=../b to highlight exactly what you > mentioned above. in our case, the O=path points to a completely > different directory path. > >> If you want to see a short relative path for __FILE__, >> I'd recommend to use a sub-directory for O=. >> >> For example, your source tree is located at >> ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot, >> create a directory ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot/foo, >> then give O=foo > > Typical product build such as OE recipes used (in our case) does not > build into the source folder, instead, all binary builds are pointed > to either a temporary location or a package build location. So, while > O=source_path/build would aliviate the problem, it still does'nt > really solve the root of the issue. have you tried using __BASE_FILE__ instead of __FILE__? You could also redefine __FILE__ or __BASE_FILE__ while building SPL. -- balbi ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-03-28 11:47 ` Felipe Balbi @ 2017-03-28 11:59 ` Nishanth Menon 2017-03-29 10:57 ` Masahiro Yamada 1 sibling, 0 replies; 16+ messages in thread From: Nishanth Menon @ 2017-03-28 11:59 UTC (permalink / raw) To: u-boot On Tue, Mar 28, 2017 at 6:47 AM, Felipe Balbi <felipe.balbi@linux.intel.com> wrote: > > have you tried using __BASE_FILE__ instead of __FILE__? You could also https://gcc.gnu.org/onlinedocs/gcc-4.1.2/cpp/Common-Predefined-Macros.html __BASE_FILE__This macro expands to the name of the main input file, in the form of a C string constant. This is the source file that was specified on the command line of the preprocessor or C compiler. __FILE__ and __BASE_FILE__ are'nt the same. and further, __FILE__ is used in many places in source unfortunately. > redefine __FILE__ or __BASE_FILE__ while building SPL. As I mentioned in my original email: a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in If we decide to deprecate usage of __FILE__ , it wont be intutive, the option i can think of is to override __FILE__ and somehow just squash warning just for __FILE__ (I have'nt investigated how).. and get a build time macro to define relative path which is provided as a replacement for __FILE__ [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path -- --- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] __FILE__ usage and and SPL limits for SRAM 2017-03-28 11:47 ` Felipe Balbi 2017-03-28 11:59 ` Nishanth Menon @ 2017-03-29 10:57 ` Masahiro Yamada 1 sibling, 0 replies; 16+ messages in thread From: Masahiro Yamada @ 2017-03-29 10:57 UTC (permalink / raw) To: u-boot 2017-03-28 20:47 GMT+09:00 Felipe Balbi <felipe.balbi@linux.intel.com>: > > Hi, > > Nishanth Menon <nm@ti.com> writes: >> Hi Masahiro-san, >> >> On Tue, Mar 28, 2017 at 1:29 AM, Masahiro Yamada >> <yamada.masahiro@socionext.com> wrote: >> [...] >>> >>> When O= is given, the build system runs in the object tree, >>> not in the source tree. >>> (This is the same as Linux.) >>> >>> If you see the top Makefile: >>> >>> ifeq ($(KBUILD_SRC),) >>> # building in the source tree >>> srctree := . >>> else >>> ifeq ($(KBUILD_SRC)/,$(dir $(CURDIR))) >>> # building in a subdirectory of the source tree >>> srctree := .. >>> else >>> srctree := $(KBUILD_SRC) >>> endif >>> endif >>> >>> >>> >>> >>> If O= points to a sub-directory of the source tree, >>> the relative path "srctree := .." is used. >>> >>> Otherwise, the absolute path srctree := $(KBUILD_SRC) is used. >>> In your case, "O=../b" means the source tree and the obj tree >>> are siblings. So, absolute path. >> >> I did simplify this a bit with O=../b to highlight exactly what you >> mentioned above. in our case, the O=path points to a completely >> different directory path. >> >>> If you want to see a short relative path for __FILE__, >>> I'd recommend to use a sub-directory for O=. >>> >>> For example, your source tree is located at >>> ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot, >>> create a directory ~/aaaaaaaaa/bbbbbbb/cccccccc/u-boot/foo, >>> then give O=foo >> >> Typical product build such as OE recipes used (in our case) does not >> build into the source folder, instead, all binary builds are pointed >> to either a temporary location or a package build location. So, while >> O=source_path/build would aliviate the problem, it still does'nt >> really solve the root of the issue. > > have you tried using __BASE_FILE__ instead of __FILE__? You could also > redefine __FILE__ or __BASE_FILE__ while building SPL. __BASE_FILE__ and __FILE__ are the same in the sense that both of them points to the path to the file, right? In Nishanth's case, both are abs path. We see difference when they are used in a header file. For example, in foo.h static inline void foo(void) { printf("this header file is %s\n", __FILE__); printf("this header is included from %s\n", __BASE_FILE__); } __FILE__ is a path to foo.h On the other hand, __BASE_FILE__ is a path to a C file that includes foo.h -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-05-31 11:32 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-27 21:44 [U-Boot] __FILE__ usage and and SPL limits for SRAM Nishanth Menon 2017-03-28 6:07 ` Lokesh Vutla 2017-04-04 19:06 ` Tom Rini 2017-04-05 11:27 ` Wolfgang Denk 2017-04-09 19:27 ` Simon Glass 2017-04-22 5:30 ` Masahiro Yamada 2017-05-22 17:23 ` Denys Dmytriyenko 2017-05-23 0:57 ` Masahiro Yamada 2017-05-23 1:27 ` Tom Rini 2017-05-31 4:51 ` Masahiro Yamada 2017-05-31 11:32 ` Tom Rini 2017-03-28 6:29 ` Masahiro Yamada 2017-03-28 11:20 ` Nishanth Menon 2017-03-28 11:47 ` Felipe Balbi 2017-03-28 11:59 ` Nishanth Menon 2017-03-29 10:57 ` Masahiro Yamada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox