Hi Tom,
On 4 April 2017 at 13:06, Tom Rini <tr...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
