[U-Boot] [RFC PATCH] lib/tiny-printf.c: Add tiny printf function for space limited environments

Simon Glass sjg at chromium.org
Sat Nov 14 03:04:04 CET 2015


Hi Stefan,

On 11 November 2015 at 07:25, Stefan Roese <sr at 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 at denx.de>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Hans de Goede <hdegoede at redhat.com>
> Cc: Tom Rini <trini at 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


More information about the U-Boot mailing list