* [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments
@ 2015-11-11 14:25 Stefan Roese
2015-11-11 17:04 ` Albert ARIBAUD
2015-11-14 2:04 ` Simon Glass
0 siblings, 2 replies; 6+ messages in thread
From: Stefan Roese @ 2015-11-11 14:25 UTC (permalink / raw)
To: u-boot
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
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"
+ help
+ This 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 version
+ reduces 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:
+ * http://www.sparetimelabs.com/printfrevisited/printfrevisited.php
+ *
+ * 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;
+
+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';
+ 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);
+ 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);
+ 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--;
+ while (w-- > 0)
+ putc(lz ? '0' : ' ');
+ while ((ch = *p++))
+ 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
+
+ /* 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;
+}
+
static void panic_finish(void) __attribute__ ((noreturn));
static void panic_finish(void)
--
2.6.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments
2015-11-11 14:25 [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments Stefan Roese
@ 2015-11-11 17:04 ` Albert ARIBAUD
2015-11-12 3:56 ` Stefan Roese
2015-11-14 2:04 ` Simon Glass
1 sibling, 1 reply; 6+ messages in thread
From: Albert ARIBAUD @ 2015-11-11 17:04 UTC (permalink / raw)
To: u-boot
Hello Stefan,
On Wed, 11 Nov 2015 15:25:09 +0100, 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.
It would be very useful to document CONFIG_USE_TINY_PRINTF in ./README,
and especially to indicate which format specifiers are supported and
which are not.
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments
2015-11-11 17:04 ` Albert ARIBAUD
@ 2015-11-12 3:56 ` Stefan Roese
2015-11-12 7:17 ` Albert ARIBAUD
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2015-11-12 3:56 UTC (permalink / raw)
To: u-boot
Hi Albert,
On 11.11.2015 18:04, Albert ARIBAUD wrote:
> On Wed, 11 Nov 2015 15:25:09 +0100, 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.
>
> It would be very useful to document CONFIG_USE_TINY_PRINTF in ./README,
> and especially to indicate which format specifiers are supported and
> which are not.
We're not documenting the config options in the README any more,
if they are introduced via Kconfig. The help text is in Kconfig
now. But I surely can add this info about the supported format
specifiers, yet in v2. Just waiting for some more review comments...
Thanks,
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments
2015-11-12 3:56 ` Stefan Roese
@ 2015-11-12 7:17 ` Albert ARIBAUD
0 siblings, 0 replies; 6+ messages in thread
From: Albert ARIBAUD @ 2015-11-12 7:17 UTC (permalink / raw)
To: u-boot
Hello Stefan,
On Thu, 12 Nov 2015 04:56:32 +0100, Stefan Roese <sr@denx.de> wrote:
> Hi Albert,
>
> On 11.11.2015 18:04, Albert ARIBAUD wrote:
> > On Wed, 11 Nov 2015 15:25:09 +0100, 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.
> >
> > It would be very useful to document CONFIG_USE_TINY_PRINTF in ./README,
> > and especially to indicate which format specifiers are supported and
> > which are not.
>
> We're not documenting the config options in the README any more,
> if they are introduced via Kconfig.
Sorry, that's me being a dinosaur again. :)
> The help text is in Kconfig
> now. But I surely can add this info about the supported format
> specifiers, yet in v2. Just waiting for some more review comments...
Ok, thanks.
> Thanks,
> Stefan
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments
2015-11-11 14:25 [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments Stefan Roese
2015-11-11 17:04 ` Albert ARIBAUD
@ 2015-11-14 2:04 ` Simon Glass
2015-11-16 10:29 ` Stefan Roese
1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2015-11-14 2:04 UTC (permalink / raw)
To: u-boot
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!
>
> 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"
> + help
> + This 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 version
> + reduces 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:
> + * http://www.sparetimelabs.com/printfrevisited/printfrevisited.php
> + *
> + * 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?
> +
> +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'
> + 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?
> + 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)
> + 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.
> + 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.
> +
> + /* For this to work, printbuffer must be larger than
/*
* For this to work...
*/
> + * anything we ever want to print.
> + */
> + i = vscnprintf(printbuffer, sizeof(printbuffer), fmt, args);
> +
> + /* Print the string */
> + puts(printbuffer);
> + return i;
> +}
> +
> static void panic_finish(void) __attribute__ ((noreturn));
>
> static void panic_finish(void)
> --
> 2.6.3
>
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments
2015-11-14 2:04 ` Simon Glass
@ 2015-11-16 10:29 ` Stefan Roese
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Roese @ 2015-11-16 10:29 UTC (permalink / raw)
To: u-boot
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 at 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 at 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 at 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 at 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 at 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 at 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 at 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"
>> + help
>> + This 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 version
>> + reduces 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:
>> + * http://www.sparetimelabs.com/printfrevisited/printfrevisited.php
>> + *
>> + * 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-16 10:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-11 14:25 [U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments Stefan Roese
2015-11-11 17:04 ` Albert ARIBAUD
2015-11-12 3:56 ` Stefan Roese
2015-11-12 7:17 ` Albert ARIBAUD
2015-11-14 2:04 ` Simon Glass
2015-11-16 10:29 ` Stefan Roese
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox