All of lore.kernel.org
 help / color / mirror / Atom feed
* [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-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

* [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

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 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.