[U-Boot] [PATCH 0/7] Switch rockchip firefly to using tiny-printf
The Rockchip rk3288 SPL was always too close to the 32k limit, either needing gcc 5 or a patched gcc (with some constant string GC fixes) to actually stay (just) below 32k. With recent changes, it unfortunatly went over with common gcc versions.
This serie switches the firefly SPL to use tiny-printf instead of the printf from vsprint, saving around 1800 bytes in the final binary to bring it under the limit with a bit more margin again.
Sjoerd Simons (7): spl: use panic_str instead of panic spl: mmc: Explicitly init mmc struct lib/tiny-printf.c: Implement vprintf lib: Split panic functions out of vsprintf.c lib: split out strtoxxxx functions out of vsprintf.c mmc: mmc: Don't use sprintf when using tiny-printf rockchip: firefly: Use tiny-printf
common/spl/spl.c | 2 +- common/spl/spl_mmc.c | 2 +- configs/firefly-rk3288_defconfig | 1 + drivers/mmc/mmc.c | 4 +- lib/Makefile | 6 +- lib/panic.c | 45 +++++++++ lib/strto.c | 174 +++++++++++++++++++++++++++++++++++ lib/tiny-printf.c | 18 +++- lib/vsprintf.c | 193 --------------------------------------- 9 files changed, 241 insertions(+), 204 deletions(-) create mode 100644 lib/panic.c create mode 100644 lib/strto.c
For a simple static string, use panic_str() which prevents calling printf needlessly.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk ---
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 7a393dc..6e6dee7 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -452,7 +452,7 @@ ulong spl_relocate_stack_gd(void) #ifdef CONFIG_SPL_SYS_MALLOC_SIMPLE if (CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN) { if (!(gd->flags & GD_FLG_SPL_INIT)) - panic("spl_init must be called before heap reloc"); + panic_str("spl_init must be called before heap reloc");
ptr -= CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN; gd->malloc_base = ptr;
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
For a simple static string, use panic_str() which prevents calling printf needlessly.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
On 6 December 2015 at 09:45, Simon Glass sjg@chromium.org wrote:
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
For a simple static string, use panic_str() which prevents calling printf needlessly.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
Acked-by: Simon Glass sjg@chromium.org Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
Applied to u-boot-rockchip, thanks!
gcc fails to work out that the mmc variable will only ever be used if it has been initialized by spl_mmc_get_device_index and throws the following error:
common/spl/spl_mmc.c: In function ‘spl_mmc_load_image’: common/spl/spl_mmc.c:31:24: warning: ‘mmc’ may be used uninitialized in this function [-Wmaybe-uninitialized] count = mmc->block_dev.block_read(0, sector, 1, header); ^ common/spl/spl_mmc.c:251:14: note: ‘mmc’ was declared here struct mmc *mmc;
Prevent this by explicitly initializing the variable.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk ---
common/spl/spl_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index b3c2c64..43748d0 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -248,7 +248,7 @@ int spl_mmc_do_fs_boot(struct mmc *mmc)
int spl_mmc_load_image(u32 boot_device) { - struct mmc *mmc; + struct mmc *mmc = NULL; u32 boot_mode; int err = 0; __maybe_unused int part;
Hi Sjoerd,
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
gcc fails to work out that the mmc variable will only ever be used if it has been initialized by spl_mmc_get_device_index and throws the following error:
common/spl/spl_mmc.c: In function ‘spl_mmc_load_image’: common/spl/spl_mmc.c:31:24: warning: ‘mmc’ may be used uninitialized in this function [-Wmaybe-uninitialized] count = mmc->block_dev.block_read(0, sector, 1, header); ^ common/spl/spl_mmc.c:251:14: note: ‘mmc’ was declared here struct mmc *mmc;
Prevent this by explicitly initializing the variable.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
See:
http://patchwork.ozlabs.org/patch/551653/
Regards, Simon
Implement both printf and vprintf for a bit more flexibility, e.g. allows the panic() function to work with tiny-printf.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk ---
lib/tiny-printf.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index 6766a8f..403b134 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -40,17 +40,14 @@ static void div_out(unsigned int *num, unsigned int div) out_dgt(dgt); }
-int printf(const char *fmt, ...) +int vprintf(const char *fmt, va_list va) { - va_list va; char ch; char *p; unsigned int num; char buf[12]; unsigned int div;
- va_start(va, fmt); - while ((ch = *(fmt++))) { if (ch != '%') { putc(ch); @@ -117,6 +114,17 @@ int printf(const char *fmt, ...) }
abort: - va_end(va); return 0; } + +int printf(const char *fmt, ...) +{ + va_list va; + int ret; + + va_start(va, fmt); + ret = vprintf(fmt, va); + va_end(va); + + return ret; +}
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
Implement both printf and vprintf for a bit more flexibility, e.g. allows the panic() function to work with tiny-printf.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
lib/tiny-printf.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
Acked-by: Simon Glass sjg@chromium.org Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
Applied to u-boot-rockchip, thanks!
To allow panic and panic_str to still be used when using tiny-printf, split them out into their own file which gets build regardless of what printf implementation is used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk ---
lib/Makefile | 6 +++--- lib/panic.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 29 ----------------------------- 3 files changed, 48 insertions(+), 32 deletions(-) create mode 100644 lib/panic.c
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif
subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/panic.c b/lib/panic.c new file mode 100644 index 0000000..e2b8b74 --- /dev/null +++ b/lib/panic.c @@ -0,0 +1,45 @@ +/* + * linux/lib/vsprintf.c + * + * Copyright (C) 1991, 1992 Linus Torvalds + */ + +/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/* + * Wirzenius wrote this portably, Torvalds fucked it up :-) + */ + +#include <common.h> +#if !defined(CONFIG_PANIC_HANG) +#include <command.h> +#endif + +static void panic_finish(void) __attribute__ ((noreturn)); + +static void panic_finish(void) +{ + putc('\n'); +#if defined(CONFIG_PANIC_HANG) + hang(); +#else + udelay(100000); /* allow messages to go out */ + do_reset(NULL, 0, 0, NULL); +#endif + while (1) + ; +} + +void panic_str(const char *str) +{ + puts(str); + panic_finish(); +} + +void panic(const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + vprintf(fmt, args); + va_end(args); + panic_finish(); +} diff --git a/lib/vsprintf.c b/lib/vsprintf.c index dd8380b..bf5fd01 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -897,35 +897,6 @@ int vprintf(const char *fmt, va_list args) return i; }
-static void panic_finish(void) __attribute__ ((noreturn)); - -static void panic_finish(void) -{ - putc('\n'); -#if defined(CONFIG_PANIC_HANG) - hang(); -#else - udelay(100000); /* allow messages to go out */ - do_reset(NULL, 0, 0, NULL); -#endif - while (1) - ; -} - -void panic_str(const char *str) -{ - puts(str); - panic_finish(); -} - -void panic(const char *fmt, ...) -{ - va_list args; - va_start(args, fmt); - vprintf(fmt, args); - va_end(args); - panic_finish(); -}
void __assert_fail(const char *assertion, const char *file, unsigned line, const char *function)
Hi Sjoerd,
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
To allow panic and panic_str to still be used when using tiny-printf, split them out into their own file which gets build regardless of what printf implementation is used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
lib/Makefile | 6 +++--- lib/panic.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 29 ----------------------------- 3 files changed, 48 insertions(+), 32 deletions(-) create mode 100644 lib/panic.c
Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif
Why not just add this outside all the ifdef stuff:
obj-y += panic.o
subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/panic.c b/lib/panic.c new file mode 100644 index 0000000..e2b8b74 --- /dev/null +++ b/lib/panic.c @@ -0,0 +1,45 @@ +/*
- linux/lib/vsprintf.c
nit: can you please drop this line or fix it?
- Copyright (C) 1991, 1992 Linus Torvalds
- */
+/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/*
- Wirzenius wrote this portably, Torvalds fucked it up :-)
- */
Did any of this code actually come from Linux? If not perhaps invent your own copyright?
+#include <common.h> +#if !defined(CONFIG_PANIC_HANG) +#include <command.h> +#endif
+static void panic_finish(void) __attribute__ ((noreturn));
+static void panic_finish(void) +{
putc('\n');+#if defined(CONFIG_PANIC_HANG)
hang();+#else
udelay(100000); /* allow messages to go out */do_reset(NULL, 0, 0, NULL);+#endif
while (1);+}
+void panic_str(const char *str) +{
puts(str);panic_finish();+}
+void panic(const char *fmt, ...) +{
va_list args;va_start(args, fmt);vprintf(fmt, args);va_end(args);panic_finish();+} diff --git a/lib/vsprintf.c b/lib/vsprintf.c index dd8380b..bf5fd01 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -897,35 +897,6 @@ int vprintf(const char *fmt, va_list args) return i; }
-static void panic_finish(void) __attribute__ ((noreturn));
-static void panic_finish(void) -{
putc('\n');-#if defined(CONFIG_PANIC_HANG)
hang();-#else
udelay(100000); /* allow messages to go out */do_reset(NULL, 0, 0, NULL);-#endif
while (1);-}
-void panic_str(const char *str) -{
puts(str);panic_finish();-}
-void panic(const char *fmt, ...) -{
va_list args;va_start(args, fmt);vprintf(fmt, args);va_end(args);panic_finish();-}
void __assert_fail(const char *assertion, const char *file, unsigned line, const char *function) -- 2.6.2
Regards, Simon
On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote:
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif
Why not just add this outside all the ifdef stuff:
obj-y += panic.o
Just keeping the old behaviour, that code was not build for SPL builds without serial support before. Do you see a benefit of just always building it ?
subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/panic.c b/lib/panic.c new file mode 100644 index 0000000..e2b8b74 --- /dev/null +++ b/lib/panic.c @@ -0,0 +1,45 @@ +/*
- * linux/lib/vsprintf.c
nit: can you please drop this line or fix it?
Sure it's pointless anyway
- * Copyright (C) 1991, 1992 Linus Torvalds
- */
+/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/*
- Wirzenius wrote this portably, Torvalds fucked it up :-)
- */
Did any of this code actually come from Linux? If not perhaps invent your own copyright?
I actually went back and checked, vsprintf.c was imported in de180e6daa529dc78668c99bdf17a9cdd440782d which is a helpful commit with the name "Initial revisions".
Most of the code in vsprintf.c is likely to just come from from linux afaik (see lib/vsprintf.c in the linux source) especial in the initial linux git repository. Unfortunate unless you actually want to go trawling back through pre-git linux versions to work out how similar the were at the branching point.
The panic functions don't appear in git versions of linux, but may or may not be there in pre-git versions.
In this case I just took the simple/conservative path and copied the copyright header (assuming correctness which seems reasonable enough) when splitting things up. I'd be fine with adjusting the copyright header _if_ there was more (easily) available historical data about the authors. Given that doesn't seem to be the case, I would prefer to keep the copyright as-is unless someone wants to take the time to do some code archaeology :)
Hi Sjoerd,
On 8 December 2015 at 00:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote:
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif
Why not just add this outside all the ifdef stuff:
obj-y += panic.o
Just keeping the old behaviour, that code was not build for SPL builds without serial support before. Do you see a benefit of just always building it ?
I cannot see a case where you don't build it:
ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o else obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o endif else # Main U-Boot always uses the full printf support obj-y += vsprintf.o panic.o strto.o endif
Every case has panic.o and strto.o. What am I missing?
subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/panic.c b/lib/panic.c new file mode 100644 index 0000000..e2b8b74 --- /dev/null +++ b/lib/panic.c @@ -0,0 +1,45 @@ +/*
- linux/lib/vsprintf.c
nit: can you please drop this line or fix it?
Sure it's pointless anyway
- Copyright (C) 1991, 1992 Linus Torvalds
- */
+/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/*
- Wirzenius wrote this portably, Torvalds fucked it up :-)
- */
Did any of this code actually come from Linux? If not perhaps invent your own copyright?
I actually went back and checked, vsprintf.c was imported in de180e6daa529dc78668c99bdf17a9cdd440782d which is a helpful commit with the name "Initial revisions".
Most of the code in vsprintf.c is likely to just come from from linux afaik (see lib/vsprintf.c in the linux source) especial in the initial linux git repository. Unfortunate unless you actually want to go trawling back through pre-git linux versions to work out how similar the were at the branching point.
The panic functions don't appear in git versions of linux, but may or may not be there in pre-git versions.
In this case I just took the simple/conservative path and copied the copyright header (assuming correctness which seems reasonable enough) when splitting things up. I'd be fine with adjusting the copyright header _if_ there was more (easily) available historical data about the authors. Given that doesn't seem to be the case, I would prefer to keep the copyright as-is unless someone wants to take the time to do some code archaeology :)
-- Sjoerd Simons Collabora Ltd.
Regards, Simon
On Tue, 2015-12-08 at 12:34 -0700, Simon Glass wrote:
Hi Sjoerd,
On 8 December 2015 at 00:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote:
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif
Why not just add this outside all the ifdef stuff:
obj-y += panic.o
Just keeping the old behaviour, that code was not build for SPL builds without serial support before. Do you see a benefit of just always building it ?
I cannot see a case where you don't build it:
ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o else obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o endif else # Main U-Boot always uses the full printf support obj-y += vsprintf.o panic.o strto.o endif
Every case has panic.o and strto.o. What am I missing?
The dependency on CONFIG_SPL_SERIAL_SUPPORT.
-Scott
Hi Scott,
On 8 December 2015 at 12:36, Scott Wood scottwood@freescale.com wrote:
On Tue, 2015-12-08 at 12:34 -0700, Simon Glass wrote:
Hi Sjoerd,
On 8 December 2015 at 00:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote:
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif
Why not just add this outside all the ifdef stuff:
obj-y += panic.o
Just keeping the old behaviour, that code was not build for SPL builds without serial support before. Do you see a benefit of just always building it ?
I cannot see a case where you don't build it:
ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o else obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o endif else # Main U-Boot always uses the full printf support obj-y += vsprintf.o panic.o strto.o endif
Every case has panic.o and strto.o. What am I missing?
The dependency on CONFIG_SPL_SERIAL_SUPPORT.
OK, so how about this:
ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += panic.o strto.o
ifdef CONFIG_USE_TINY_PRINTF
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o
else
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o
endif else # Main U-Boot always uses the full printf support obj-y += vsprintf.o panic.o strto.o endif
Regards, Simon
On 8 December 2015 at 12:38, Simon Glass sjg@chromium.org wrote:
Hi Scott,
On 8 December 2015 at 12:36, Scott Wood scottwood@freescale.com wrote:
On Tue, 2015-12-08 at 12:34 -0700, Simon Glass wrote:
Hi Sjoerd,
On 8 December 2015 at 00:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote:
diff --git a/lib/Makefile b/lib/Makefile index 1f1ff6f..ae84833 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o +obj-y += vsprintf.o panic.o endif
Why not just add this outside all the ifdef stuff:
obj-y += panic.o
Just keeping the old behaviour, that code was not build for SPL builds without serial support before. Do you see a benefit of just always building it ?
I cannot see a case where you don't build it:
ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o else obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o endif else # Main U-Boot always uses the full printf support obj-y += vsprintf.o panic.o strto.o endif
Every case has panic.o and strto.o. What am I missing?
The dependency on CONFIG_SPL_SERIAL_SUPPORT.
OK, so how about this:
ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += panic.o strto.o
ifdef CONFIG_USE_TINY_PRINTF
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o
else
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o
endif else # Main U-Boot always uses the full printf support obj-y += vsprintf.o panic.o strto.o endif
It's just a nit so I'm going to leave it as is for now.
Applied to u-boot-rockchip, thanks!
On Sun, 2015-12-13 at 20:45 -0700, Simon Glass wrote:
On 8 December 2015 at 12:38, Simon Glass sjg@chromium.org wrote:
Hi Scott,
On 8 December 2015 at 12:36, Scott Wood scottwood@freescale.com wrote:
On Tue, 2015-12-08 at 12:34 -0700, Simon Glass wrote:
Hi Sjoerd,
OK, so how about this:
ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += panic.o strto.o
ifdef CONFIG_USE_TINY_PRINTF
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o
else
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o
endif else # Main U-Boot always uses the full printf support obj-y += vsprintf.o panic.o strto.o endif
It's just a nit so I'm going to leave it as is for now.
Applied to u-boot-rockchip, thanks!
Heh, i just did an update series last night to send this morning :) lovely timing.
I'll drop you some patches for these nits later this week while i still remember them :) (I agree your suggestion here is nicer).
To allow the various string to number conversion functions to be used when using tiny-printf,split them out into their own file which gets build regardless of what printf implementation is used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk ---
lib/Makefile | 6 +- lib/strto.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 164 ----------------------------------------------------- 3 files changed, 177 insertions(+), 167 deletions(-) create mode 100644 lib/strto.c
diff --git a/lib/Makefile b/lib/Makefile index ae84833..dd36f25 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o panic.o +obj-y += vsprintf.o panic.o strto.o endif
subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/strto.c b/lib/strto.c new file mode 100644 index 0000000..a6c0157 --- /dev/null +++ b/lib/strto.c @@ -0,0 +1,174 @@ +/* + * linux/lib/vsprintf.c + * + * Copyright (C) 1991, 1992 Linus Torvalds + */ + +/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/* + * Wirzenius wrote this portably, Torvalds fucked it up :-) + */ + +#include <common.h> +#include <errno.h> +#include <linux/ctype.h> + +unsigned long simple_strtoul(const char *cp, char **endp, + unsigned int base) +{ + unsigned long result = 0; + unsigned long value; + + if (*cp == '0') { + cp++; + if ((*cp == 'x') && isxdigit(cp[1])) { + base = 16; + cp++; + } + + if (!base) + base = 8; + } + + if (!base) + base = 10; + + while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp-'0' : (islower(*cp) + ? toupper(*cp) : *cp)-'A'+10) < base) { + result = result*base + value; + cp++; + } + + if (endp) + *endp = (char *)cp; + + return result; +} + +int strict_strtoul(const char *cp, unsigned int base, unsigned long *res) +{ + char *tail; + unsigned long val; + size_t len; + + *res = 0; + len = strlen(cp); + if (len == 0) + return -EINVAL; + + val = simple_strtoul(cp, &tail, base); + if (tail == cp) + return -EINVAL; + + if ((*tail == '\0') || + ((len == (size_t)(tail - cp) + 1) && (*tail == '\n'))) { + *res = val; + return 0; + } + + return -EINVAL; +} + +long simple_strtol(const char *cp, char **endp, unsigned int base) +{ + if (*cp == '-') + return -simple_strtoul(cp + 1, endp, base); + + return simple_strtoul(cp, endp, base); +} + +unsigned long ustrtoul(const char *cp, char **endp, unsigned int base) +{ + unsigned long result = simple_strtoul(cp, endp, base); + switch (**endp) { + case 'G': + result *= 1024; + /* fall through */ + case 'M': + result *= 1024; + /* fall through */ + case 'K': + case 'k': + result *= 1024; + if ((*endp)[1] == 'i') { + if ((*endp)[2] == 'B') + (*endp) += 3; + else + (*endp) += 2; + } + } + return result; +} + +unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base) +{ + unsigned long long result = simple_strtoull(cp, endp, base); + switch (**endp) { + case 'G': + result *= 1024; + /* fall through */ + case 'M': + result *= 1024; + /* fall through */ + case 'K': + case 'k': + result *= 1024; + if ((*endp)[1] == 'i') { + if ((*endp)[2] == 'B') + (*endp) += 3; + else + (*endp) += 2; + } + } + return result; +} + +unsigned long long simple_strtoull(const char *cp, char **endp, + unsigned int base) +{ + unsigned long long result = 0, value; + + if (*cp == '0') { + cp++; + if ((*cp == 'x') && isxdigit(cp[1])) { + base = 16; + cp++; + } + + if (!base) + base = 8; + } + + if (!base) + base = 10; + + while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp - '0' + : (islower(*cp) ? toupper(*cp) : *cp) - 'A' + 10) < base) { + result = result * base + value; + cp++; + } + + if (endp) + *endp = (char *) cp; + + return result; +} + +long trailing_strtoln(const char *str, const char *end) +{ + const char *p; + + if (!end) + end = str + strlen(str); + for (p = end - 1; p > str; p--) { + if (!isdigit(*p)) + return simple_strtoul(p + 1, NULL, 10); + } + + return -1; +} + +long trailing_strtol(const char *str) +{ + return trailing_strtoln(str, NULL); +} diff --git a/lib/vsprintf.c b/lib/vsprintf.c index bf5fd01..24167a1 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -15,176 +15,12 @@ #include <linux/types.h> #include <linux/string.h> #include <linux/ctype.h> -#include <errno.h>
#include <common.h> -#if !defined(CONFIG_PANIC_HANG) -#include <command.h> -#endif
#include <div64.h> #define noinline __attribute__((noinline))
-unsigned long simple_strtoul(const char *cp, char **endp, - unsigned int base) -{ - unsigned long result = 0; - unsigned long value; - - if (*cp == '0') { - cp++; - if ((*cp == 'x') && isxdigit(cp[1])) { - base = 16; - cp++; - } - - if (!base) - base = 8; - } - - if (!base) - base = 10; - - while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp-'0' : (islower(*cp) - ? toupper(*cp) : *cp)-'A'+10) < base) { - result = result*base + value; - cp++; - } - - if (endp) - *endp = (char *)cp; - - return result; -} - -int strict_strtoul(const char *cp, unsigned int base, unsigned long *res) -{ - char *tail; - unsigned long val; - size_t len; - - *res = 0; - len = strlen(cp); - if (len == 0) - return -EINVAL; - - val = simple_strtoul(cp, &tail, base); - if (tail == cp) - return -EINVAL; - - if ((*tail == '\0') || - ((len == (size_t)(tail - cp) + 1) && (*tail == '\n'))) { - *res = val; - return 0; - } - - return -EINVAL; -} - -long simple_strtol(const char *cp, char **endp, unsigned int base) -{ - if (*cp == '-') - return -simple_strtoul(cp + 1, endp, base); - - return simple_strtoul(cp, endp, base); -} - -unsigned long ustrtoul(const char *cp, char **endp, unsigned int base) -{ - unsigned long result = simple_strtoul(cp, endp, base); - switch (**endp) { - case 'G': - result *= 1024; - /* fall through */ - case 'M': - result *= 1024; - /* fall through */ - case 'K': - case 'k': - result *= 1024; - if ((*endp)[1] == 'i') { - if ((*endp)[2] == 'B') - (*endp) += 3; - else - (*endp) += 2; - } - } - return result; -} - -unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base) -{ - unsigned long long result = simple_strtoull(cp, endp, base); - switch (**endp) { - case 'G': - result *= 1024; - /* fall through */ - case 'M': - result *= 1024; - /* fall through */ - case 'K': - case 'k': - result *= 1024; - if ((*endp)[1] == 'i') { - if ((*endp)[2] == 'B') - (*endp) += 3; - else - (*endp) += 2; - } - } - return result; -} - -unsigned long long simple_strtoull(const char *cp, char **endp, - unsigned int base) -{ - unsigned long long result = 0, value; - - if (*cp == '0') { - cp++; - if ((*cp == 'x') && isxdigit(cp[1])) { - base = 16; - cp++; - } - - if (!base) - base = 8; - } - - if (!base) - base = 10; - - while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp - '0' - : (islower(*cp) ? toupper(*cp) : *cp) - 'A' + 10) < base) { - result = result * base + value; - cp++; - } - - if (endp) - *endp = (char *) cp; - - return result; -} - -long trailing_strtoln(const char *str, const char *end) -{ - const char *p; - - if (!end) - end = str + strlen(str); - for (p = end - 1; p > str; p--) { - if (!isdigit(*p)) - return simple_strtoul(p + 1, NULL, 10); - } - - return -1; -} - -long trailing_strtol(const char *str) -{ - return trailing_strtoln(str, NULL); -} - /* we use this so that we can do without the ctype library */ #define is_digit(c) ((c) >= '0' && (c) <= '9')
Hi Sjoerd,
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
To allow the various string to number conversion functions to be used when using tiny-printf,split them out into their own file which gets build regardless of what printf implementation is used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
lib/Makefile | 6 +- lib/strto.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 164 ----------------------------------------------------- 3 files changed, 177 insertions(+), 167 deletions(-) create mode 100644 lib/strto.c
Acked-by: Simon Glass sjg@chromium.org Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
diff --git a/lib/Makefile b/lib/Makefile index ae84833..dd36f25 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o panic.o +obj-y += vsprintf.o panic.o strto.o endif
Can you just add this outside all the ifdef stuff:
obj-y += strto.o
subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/strto.c b/lib/strto.c new file mode 100644 index 0000000..a6c0157 --- /dev/null +++ b/lib/strto.c @@ -0,0 +1,174 @@ +/*
- linux/lib/vsprintf.c
- Copyright (C) 1991, 1992 Linus Torvalds
- */
+/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/*
- Wirzenius wrote this portably, Torvalds fucked it up :-)
- */
Please check this code came from Linux. Probably it did, just want to make sure. The comment does not inspire confidence that it works!
+#include <common.h> +#include <errno.h> +#include <linux/ctype.h>
[snip]
Regards, Simon
On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote:
Hi Sjoerd,
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
To allow the various string to number conversion functions to be used when using tiny-printf,split them out into their own file which gets build regardless of what printf implementation is used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
lib/Makefile | 6 +- lib/strto.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 164 -------------------------------------------
3 files changed, 177 insertions(+), 167 deletions(-) create mode 100644 lib/strto.c
Acked-by: Simon Glass sjg@chromium.org Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
diff --git a/lib/Makefile b/lib/Makefile index ae84833..dd36f25 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o ifdef CONFIG_SPL_BUILD # SPL U-Boot may use full-printf, tiny-printf or none at all ifdef CONFIG_USE_TINY_PRINTF -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o else -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o endif else # Main U-Boot always uses the full printf support -obj-y += vsprintf.o panic.o +obj-y += vsprintf.o panic.o strto.o endif
Can you just add this outside all the ifdef stuff:
obj-y += strto.o
Same as with panic, it wasn't build before on SPLs without serial support so i kept that behaviour. Though as I don't think any of the strto functions depending on print functionality always building it may make sense.
subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2 diff --git a/lib/strto.c b/lib/strto.c new file mode 100644 index 0000000..a6c0157 --- /dev/null +++ b/lib/strto.c @@ -0,0 +1,174 @@ +/*
- * linux/lib/vsprintf.c
- * Copyright (C) 1991, 1992 Linus Torvalds
- */
+/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */ +/*
- Wirzenius wrote this portably, Torvalds fucked it up :-)
- */
Please check this code came from Linux. Probably it did, just want to make sure. The comment does not inspire confidence that it works!
See my reply to the previous patch :). the strtoxxx are more clearly derived from Linux. And yeah, the linux code has the same copyright header, but apparently it works for them... :p
+#include <common.h> +#include <errno.h> +#include <linux/ctype.h>
[snip]
Regards, Simon
Applied to u-boot-rockchip, thanks!
There is no sprintf implementation in tiny-printf, so don't try to use it when tiny-printf if used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk ---
drivers/mmc/mmc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 2a58702..3a34028 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1469,7 +1469,9 @@ static int mmc_startup(struct mmc *mmc) mmc->block_dev.blksz = mmc->read_bl_len; mmc->block_dev.log2blksz = LOG2(mmc->block_dev.blksz); mmc->block_dev.lba = lldiv(mmc->capacity, mmc->read_bl_len); -#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT) +#if !defined(CONFIG_SPL_BUILD) || \ + (defined(CONFIG_SPL_LIBCOMMON_SUPPORT) && \ + !defined(CONFIG_USE_TINY_PRINTF)) sprintf(mmc->block_dev.vendor, "Man %06x Snr %04x%04x", mmc->cid[0] >> 24, (mmc->cid[2] & 0xffff), (mmc->cid[3] >> 16) & 0xffff);
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
There is no sprintf implementation in tiny-printf, so don't try to use it when tiny-printf if used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
drivers/mmc/mmc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
I can't help thinking that sprintf() would not add a lot more code...
- Simon
On Mon, 2015-12-07 at 17:40 -0700, Simon Glass wrote:
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
There is no sprintf implementation in tiny-printf, so don't try to use it when tiny-printf if used.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
drivers/mmc/mmc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
I can't help thinking that sprintf() would not add a lot more code...
Probably not (it's write to buffer rather then write to output, so maybe just passing a function pointer for an out function in would just do the job). More work then I had time for though :/
Though i'm not sure it's worth it though (at least on firefly the spl won't output those fields) :)
Applied to u-boot-rockchip, thanks!
Switch to using tiny-printf for the firefly SPL, this reduces the SPL by around 1800 bytes bringing it back under the 32k limit for both gcc 4.9 and gcc 5.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
---
configs/firefly-rk3288_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/firefly-rk3288_defconfig b/configs/firefly-rk3288_defconfig index b645af5..be4ca88 100644 --- a/configs/firefly-rk3288_defconfig +++ b/configs/firefly-rk3288_defconfig @@ -42,5 +42,6 @@ CONFIG_DEBUG_UART_CLOCK=24000000 CONFIG_DEBUG_UART_SHIFT=2 CONFIG_SYS_NS16550=y CONFIG_USE_PRIVATE_LIBGCC=y +CONFIG_USE_TINY_PRINTF=y CONFIG_CMD_DHRYSTONE=y CONFIG_ERRNO_STR=y
On 4 December 2015 at 15:27, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
Switch to using tiny-printf for the firefly SPL, this reduces the SPL by around 1800 bytes bringing it back under the 32k limit for both gcc 4.9 and gcc 5.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
configs/firefly-rk3288_defconfig | 1 + 1 file changed, 1 insertion(+)
Acked-by: Simon Glass sjg@chromium.org Tested on firefly: Tested-by: Simon Glass sjg@chromium.org
Applied to u-boot-rockchip, thanks!
On 04.12.2015 23:27, Sjoerd Simons wrote:
The Rockchip rk3288 SPL was always too close to the 32k limit, either needing gcc 5 or a patched gcc (with some constant string GC fixes) to actually stay (just) below 32k. With recent changes, it unfortunatly went over with common gcc versions.
This serie switches the firefly SPL to use tiny-printf instead of the printf from vsprint, saving around 1800 bytes in the final binary to bring it under the limit with a bit more margin again.
Thanks for working on this. Whole series:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Hi Tom,
On 5 December 2015 at 03:00, Stefan Roese sr@denx.de wrote:
On 04.12.2015 23:27, Sjoerd Simons wrote:
The Rockchip rk3288 SPL was always too close to the 32k limit, either needing gcc 5 or a patched gcc (with some constant string GC fixes) to actually stay (just) below 32k. With recent changes, it unfortunatly went over with common gcc versions.
This serie switches the firefly SPL to use tiny-printf instead of the printf from vsprint, saving around 1800 bytes in the final binary to bring it under the limit with a bit more margin again.
Thanks for working on this. Whole series:
Reviewed-by: Stefan Roese sr@denx.de
This looks good to me. It does fix Firefly with the bugged gcc.
Tom, would you be OK with me picking this up for this release? I also need to do a proper review...
Regards, Simon
On Mon, Dec 07, 2015 at 02:43:24PM -0700, Simon Glass wrote:
Hi Tom,
On 5 December 2015 at 03:00, Stefan Roese sr@denx.de wrote:
On 04.12.2015 23:27, Sjoerd Simons wrote:
The Rockchip rk3288 SPL was always too close to the 32k limit, either needing gcc 5 or a patched gcc (with some constant string GC fixes) to actually stay (just) below 32k. With recent changes, it unfortunatly went over with common gcc versions.
This serie switches the firefly SPL to use tiny-printf instead of the printf from vsprint, saving around 1800 bytes in the final binary to bring it under the limit with a bit more margin again.
Thanks for working on this. Whole series:
Reviewed-by: Stefan Roese sr@denx.de
This looks good to me. It does fix Firefly with the bugged gcc.
Tom, would you be OK with me picking this up for this release? I also need to do a proper review...
Yes, that's fine, thanks!
participants (5)
-
Scott Wood -
Simon Glass -
Sjoerd Simons -
Stefan Roese -
Tom Rini