Hi Simon,
On 14.11.2015 03:04, Simon Glass wrote:
Hi Stefan,
On 11 November 2015 at 07:25, Stefan Roese sr@denx.de wrote:
This patch adds a small printf() version that supports all basic formats. Its intented to be used in U-Boot SPL versions on platforms with very limited internal RAM sizes.
To enable it, just define CONFIG_USE_TINY_PRINTF in your defconfig. This will result in the SPL using this tiny function and the main U-Boot still using the full-blown printf() function.
This code was copied from: http://www.sparetimelabs.com/printfrevisited With mostly only coding style related changes so that its checkpatch clean.
The size reduction is about 2.5KiB. Here a comparison for the db-88f6820-gp (Marvell A38x) SPL:
Without this patch: 108075 13298 2824 124197 1e525 ./spl/u-boot-spl
With this patch: 105398 13298 2852 121548 1dacc ./spl/u-boot-spl
Great!
Thanks. BTW, you might have noticed that I still used MAKEALL for this size outputs here. I somehow failed to get buildman generate these size statistic outputs here. Sometimes buildman starts with the compilation, sometimes it only shows a summary, not really the one that I expected it to show. Here a short log of some experiments:
[stefan@stefan-work u-boot (tiny-printf-v2-2015-11-16)]$ ./tools/buildman/buildman db-mv784mp-gp boards.cfg is up to date. Nothing to do. Building current source for 1 boards (1 thread, 8 jobs per thread) 1 0 0 /1 db-mv784mp-gp [stefan@stefan-work u-boot (tiny-printf-v2-2015-11-16)]$ ./tools/buildman/buildman db-mv784mp-gp -s boards.cfg is up to date. Nothing to do. Summary of current source for 1 boards (1 thread, 8 jobs per thread) (no errors to report) [stefan@stefan-work u-boot (tiny-printf-v2-2015-11-16)]$ ./tools/buildman/buildman db-mv784mp-gp -s -c 2 boards.cfg is up to date. Nothing to do. Summary of current source for 1 boards (1 thread, 8 jobs per thread) (no errors to report) [stefan@stefan-work u-boot (tiny-printf-v2-2015-11-16)]$ ./tools/buildman/buildman db-mv784mp-gp -s -c 5 boards.cfg is up to date. Nothing to do. Summary of current source for 1 boards (1 thread, 8 jobs per thread) (no errors to report) [stefan@stefan-work u-boot (tiny-printf-v2-2015-11-16)]$ ./tools/buildman/buildman db-mv784mp-gp -sS -c 5 boards.cfg is up to date. Nothing to do. Summary of current source for 1 boards (1 thread, 8 jobs per thread) (no errors to report) [stefan@stefan-work u-boot (tiny-printf-v2-2015-11-16)]$ ./tools/buildman/buildman mvebu -sS -c 5 boards.cfg is up to date. Nothing to do. Summary of current source for 4 boards (4 threads, 2 jobs per thread) arm: + clearfog [stefan@stefan-work u-boot (tiny-printf-v2-2015-11-16)]$ ./tools/buildman/buildman mvebu -sS -c 10 boards.cfg is up to date. Nothing to do. Summary of current source for 4 boards (4 threads, 2 jobs per thread) arm: + clearfog
I'm most likely missing something. Perhaps using a branch name, not sure. I also experimented with using -b <branch> and get here:
$ ./tools/buildman/buildman -b tiny-printf-v2-2015-11-16 mvebu Cannot find a suitable upstream for branch 'tiny-printf-v2-2015-11-16'
which makes perfect sense. As this is absolute work-in-progess and I really don't want to push this into some upstream repo at this point.
I've also noticed the comments in the README about this branch usage (e.g. 'git branch --set-upstream-to upstream/master'). But I assume that there has to be the possibility to use buildman without pushing such working branches upstream.
So please let me know what I'm missing here. Thanks!
Some more comments below.
Note: To make it possible to compile tiny-printf.c instead of vsprintf.c when CONFIG_USE_TINY_PRINTF is defined, the functions printf() and vprintf() are moved from common/console.c into vsprintf.c in this patch.
Signed-off-by: Stefan Roese sr@denx.de Cc: Simon Glass sjg@chromium.org Cc: Hans de Goede hdegoede@redhat.com Cc: Tom Rini trini@konsulko.com
common/console.c | 39 ---------------- lib/Kconfig | 8 ++++ lib/Makefile | 7 ++- lib/tiny-printf.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/vsprintf.c | 39 ++++++++++++++++ 5 files changed, 183 insertions(+), 40 deletions(-) create mode 100644 lib/tiny-printf.c
diff --git a/common/console.c b/common/console.c index ace206c..2bfd59f 100644 --- a/common/console.c +++ b/common/console.c @@ -535,45 +535,6 @@ void puts(const char *s) } }
-int printf(const char *fmt, ...) -{
va_list args;uint i;char printbuffer[CONFIG_SYS_PBSIZE];va_start(args, fmt);/* For this to work, printbuffer must be larger than* anything we ever want to print.*/i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);va_end(args);/* Print the string */puts(printbuffer);return i;-}
-int vprintf(const char *fmt, va_list args) -{
uint i;char printbuffer[CONFIG_SYS_PBSIZE];-#if defined(CONFIG_PRE_CONSOLE_BUFFER) && !defined(CONFIG_SANDBOX)
if (!gd->have_console)return 0;-#endif
/* For this to work, printbuffer must be larger than* anything we ever want to print.*/i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);/* Print the string */puts(printbuffer);return i;-}
- /* test if ctrl-c was pressed */ static int ctrlc_disabled = 0; /* see disable_ctrl() */ static int ctrlc_was_pressed = 0;
diff --git a/lib/Kconfig b/lib/Kconfig index 30e84ed..faf3de3 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -36,6 +36,14 @@ config SYS_VSNPRINTF Thumb-2, about 420 bytes). Enable this option for safety when using sprintf() with data you do not control.
+config USE_TINY_PRINTF
bool "Enable tiny printf() version"helpThis option enables a tiny, stripped down printf version.This should only be used in space limited environments,like SPL versions with hard memory limits. This versionreduces the code size by about 2.5KiB on armv7.- config REGEX bool "Enable regular expression support" default y if NET
diff --git a/lib/Makefile b/lib/Makefile index 3eecefa..54b6555 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -79,7 +79,12 @@ obj-y += string.o obj-y += time.o obj-$(CONFIG_TRACE) += trace.o obj-$(CONFIG_LIB_UUID) += uuid.o -obj-y += vsprintf.o obj-$(CONFIG_LIB_RAND) += rand.o
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_USE_TINY_PRINTF),yy) +obj-y += tiny-printf.o +else +obj-y += vsprintf.o +endif
- subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2
diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c new file mode 100644 index 0000000..d743a36 --- /dev/null +++ b/lib/tiny-printf.c @@ -0,0 +1,130 @@ +/*
- Tiny printf version for SPL
- Copied from:
- Copyright (C) 2004,2008 Kustaa Nyholm
- SPDX-License-Identifier: LGPL-2.1+
- */
+#include <common.h> +#include <stdarg.h> +#include <serial.h>
+static char *bf; +static char buf[12]; +static unsigned int num; +static char uc; +static char zs;
Do we need all these variables? Perhaps some of them could become parameters instead?
As mentioned in the commit text, this patch just includes the original code as published on the link above. With only some coding style related changes.
I'll take a closer look at the functional changes later and will perhaps send a follow-up patch for this.
+static void out(char c) +{
*bf++ = c;+}
+static void out_dgt(char dgt) +{
out(dgt + (dgt < 10 ? '0' : (uc ? 'A' : 'a') - 10));zs = 1;+}
+static void div_out(unsigned int div) +{
unsigned char dgt = 0;num &= 0xffff; /* just for testing the code with 32 bit ints */while (num >= div) {num -= div;dgt++;}if (zs || dgt > 0)out_dgt(dgt);+}
+int printf(const char *fmt, ...) +{
va_list va;char ch;char *p;va_start(va, fmt);while ((ch = *(fmt++))) {if (ch != '%') {putc(ch);} else {char lz = 0;char w = 0;ch = *(fmt++);if (ch == '0') {ch = *(fmt++);lz = 1;}if (ch >= '0' && ch <= '9') {w = 0;while (ch >= '0' && ch <= '9') {w = (((w << 2) + w) << 1) + ch - '0';Doesn't the compiler do the right thing if you say:
w = (w * 10) + ch - '0'
I'll test this.
ch = *fmt++;}}bf = buf;p = bf;zs = 0;switch (ch) {case 0:goto abort;case 'u':case 'd':num = va_arg(va, unsigned int);if (ch == 'd' && (int)num < 0) {num = -(int)num;out('-');}div_out(10000);Does this mean it cannot output numbers larger than 99999?
Yes. Seems that the original version only supports numbers up to 0xffff. I'll change this also in a follow-up patch.
div_out(1000);div_out(100);div_out(10);out_dgt(num);break;case 'x':case 'X':uc = ch == 'X';num = va_arg(va, unsigned int);div_out(0x1000);How about:
for (div = 0x1000; div; div >>= 4) div_out(div)
Yes.
div_out(0x100);div_out(0x10);out_dgt(num);break;case 'c':out((char)(va_arg(va, int)));break;case 's':p = va_arg(va, char*);break;case '%':out('%');default:break;}*bf = 0;bf = p;while (*bf++ && w > 0)w--;I wonder if the digits could be written to the buffer in reverse order, thus allowing something like this for the decimal case:
for (div = 10; div <= 10000; div *= 10) div_out(div)
while (w-- > 0)putc(lz ? '0' : ' ');while ((ch = *p++))and here you could work backwards.
I'll also take a look at this.
putc(ch);}}+abort:
va_end(va);return 0;+} diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 4c82837..8cc5b38 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -861,6 +861,45 @@ int sprintf(char *buf, const char *fmt, ...) return i; }
+int printf(const char *fmt, ...) +{
va_list args;uint i;char printbuffer[CONFIG_SYS_PBSIZE];va_start(args, fmt);/* For this to work, printbuffer must be larger than* anything we ever want to print.*/i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);va_end(args);/* Print the string */puts(printbuffer);return i;+}
+int vprintf(const char *fmt, va_list args) +{
uint i;char printbuffer[CONFIG_SYS_PBSIZE];+#if defined(CONFIG_PRE_CONSOLE_BUFFER) && !defined(CONFIG_SANDBOX)
if (!gd->have_console)return 0;+#endif
Hmm, that code in the #if should be removed. I did it for printf() but forgot vprintf(). If you have time you could add a patch for this.
Yes, will do.
/* For this to work, printbuffer must be larger than/*
- For this to work...
*/
Its just a copy of the original code. But I can clean this up as well while moving, yes.
Thanks, Stefan