[U-Boot] [PATCH v2] tiny-printf: Add support for %p format

[Simon Glass]

On 10 April 2017 at 00:53, Vignesh R <vigneshr at ti.com> wrote:
> Add support for %p, %pa[p], %pM, %pm and %pI4 formats to tiny-printf.
> %pM and %pI4 are widely used by SPL networking stack and is required if
> networking support is desired in SPL.
> %p, %pa and %pap are mostly used by debug prints and hence supported
> only when DEBUG is enabled.
>
> Before this patch:
> $ size spl/u-boot-spl
>    text    data     bss     dec     hex filename
>   99325    4899  218584  322808   4ecf8 spl/u-boot-spl
>
> After this patch (with CONFIG_SPL_NET_SUPPORT):
> $ size spl/u-boot-spl
>    text    data     bss     dec     hex filename
>   99666    4899  218584  323149   4ee4d spl/u-boot-spl
>
> So, this patch adds ~350 bytes to code size.
>
> If CONFIG_SPL_NET_SUPPORT is not enabled, this adds ~25 bytes.
>
> If CONFIG_USE_TINY_PRINTF is disabled then:
> $ size spl/u-boot-spl
>   text     data     bss     dec     hex filename
>  101116    4899  218584  324599   4f3f7 spl/u-boot-spl
>
> So, there is still ~1.4K space saved even with support for %pM/%pI4.
>
> Compiler used is to build is:
> arm-linux-gnueabihf-gcc (Linaro GCC 6.2-2016.11) 6.2.1 20161016
>
> Signed-off-by: Vignesh R <vigneshr at ti.com>
> ---
>
> Changes wrt RFC:
> * support %p[ap] only under DEBUG
> * Add comparsion data w/o tiny printf to commit message.
>
>  lib/tiny-printf.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 154 insertions(+)
>
> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> index 6def8f98aa41..0b04813dc206 100644
> --- a/lib/tiny-printf.c
> +++ b/lib/tiny-printf.c
> @@ -12,6 +12,7 @@
>  #include <common.h>
>  #include <stdarg.h>
>  #include <serial.h>
> +#include <linux/ctype.h>
>
>  struct printf_info {
>         char *bf;       /* Digit buffer */
> @@ -52,6 +53,154 @@ static void div_out(struct printf_info *info, unsigned long *num,
>                 out_dgt(info, dgt);
>  }
>
> +#ifdef CONFIG_SPL_NET_SUPPORT
> +static void string(struct printf_info *info, char *s)
> +{
> +       char ch;
> +
> +       while ((ch = *s++))
> +               out(info, ch);
> +}
> +
> +static const char hex_asc[] = "0123456789abcdef";
> +#define hex_asc_lo(x)  hex_asc[((x) & 0x0f)]
> +#define hex_asc_hi(x)  hex_asc[((x) & 0xf0) >> 4]
> +
> +static inline char *pack_hex_byte(char *buf, u8 byte)
> +{
> +       *buf++ = hex_asc_hi(byte);
> +       *buf++ = hex_asc_lo(byte);
> +       return buf;
> +}
> +
> +static void mac_address_string(struct printf_info *info, u8 *addr,
> +                               bool separator)
> +{
> +       /* (6 * 2 hex digits), 5 colons and trailing zero */
> +       char mac_addr[6 * 3];
> +       char *p = mac_addr;
> +       int i;
> +
> +       for (i = 0; i < 6; i++) {
> +               p = pack_hex_byte(p, addr[i]);
> +               if (separator && i != 5)
> +                       *p++ = ':';
> +       }
> +       *p = '\0';
> +
> +       string(info, mac_addr);
> +}
> +
> +static char *put_dec_trunc(char *buf, unsigned int q)
> +{
> +       unsigned int d3, d2, d1, d0;
> +       d1 = (q >> 4) & 0xf;
> +       d2 = (q >> 8) & 0xf;
> +       d3 = (q >> 12);
> +
> +       d0 = 6 * (d3 + d2 + d1) + (q & 0xf);
> +       q = (d0 * 0xcd) >> 11;
> +       d0 = d0 - 10 * q;
> +       *buf++ = d0 + '0'; /* least significant digit */
> +       d1 = q + 9 * d3 + 5 * d2 + d1;
> +       if (d1 != 0) {
> +               q = (d1 * 0xcd) >> 11;
> +               d1 = d1 - 10 * q;
> +               *buf++ = d1 + '0'; /* next digit */
> +
> +               d2 = q + 2 * d2;
> +               if ((d2 != 0) || (d3 != 0)) {
> +                       q = (d2 * 0xd) >> 7;
> +                       d2 = d2 - 10 * q;
> +                       *buf++ = d2 + '0'; /* next digit */
> +
> +                       d3 = q + 4 * d3;
> +                       if (d3 != 0) {
> +                               q = (d3 * 0xcd) >> 11;
> +                               d3 = d3 - 10 * q;
> +                               *buf++ = d3 + '0';  /* next digit */
> +                               if (q != 0)
> +                                       *buf++ = q + '0'; /* most sign. digit */

OMG not the nicest code!

> +                       }
> +               }
> +       }
> +       return buf;
> +}
> +
> +static void ip4_addr_string(struct printf_info *info, u8 *addr)
> +{
> +       /* (4 * 3 decimal digits), 3 dots and trailing zero */
> +       char ip4_addr[4 * 4];
> +       char temp[3];   /* hold each IP quad in reverse order */
> +       char *p = ip4_addr;
> +       int i, digits;
> +
> +       for (i = 0; i < 4; i++) {
> +               digits = put_dec_trunc(temp, addr[i]) - temp;
> +               /* reverse the digits in the quad */
> +               while (digits--)
> +                       *p++ = temp[digits];
> +               if (i != 3)
> +                       *p++ = '.';
> +       }
> +       *p = '\0';
> +
> +       string(info, ip4_addr);
> +}
> +#endif
> +
> +/*
> + * Show a '%p' thing.  A kernel extension is that the '%p' is followed
> + * by an extra set of characters that are extended format
> + * specifiers.
> + *
> + * Right now we handle:
> + *
> + * - 'M' For a 6-byte MAC address, it prints the address in the
> + *       usual colon-separated hex notation.
> + * - 'm' Same as above except there is no colon-separator.
> + * - 'I4'for IPv4 addresses printed in the usual way (dot-separated
> + *       decimal).
> + */
> +
> +static void pointer(struct printf_info *info, const char *fmt, void *ptr)
> +{
> +#ifdef DEBUG

What is the #ifdef DEBUG for? It may not be enabled globally so I
don't think you can do this. You probably need this code always.

Having said that, at some point it would be nice to have a global
debug framework, which understood how to enable/disable debugging at
build-time or run-time.

Regards,
Simon