tiny-printf: Add support for %p format

PATCH New Vignesh R 3 participants 8 messages Apr 10, 2017 – Apr 14, 2017 Library code

Share & tools

Vignesh R

author

2017-04-10 06:53 UTC #0 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

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 <[email protected]>

---

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(+)

lib/tiny-printf.c +154 -0

index 6def8f98aa41..0b04813dc206 100644
@@ -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 */
+			}
+		}
+	}
+	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
+	unsigned long num = (uintptr_t)ptr;
+	unsigned long div;
+#endif
+
+	switch (*fmt) {
+#ifdef DEBUG
+	case 'a':
+
+		switch (fmt[1]) {
+		case 'p':
+		default:
+			num = *(phys_addr_t *)ptr;
+			break;
+		}
+		break;
+#endif
+#ifdef CONFIG_SPL_NET_SUPPORT
+	case 'm':
+		return mac_address_string(info, ptr, false);
+	case 'M':
+		return mac_address_string(info, ptr, true);
+	case 'I':
+		if (fmt[1] == '4')
+			return ip4_addr_string(info, ptr);
+#endif
+	default:
+		break;
+	}
+#ifdef DEBUG
+	div = 1UL << (sizeof(long) * 8 - 4);
+	for (; div; div /= 0x10)
+		div_out(info, &num, div);
+#endif
+}
+
 static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
 {
 	char ch;

@@ -144,6 +293,11 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
 			case 's':
 				p = va_arg(va, char*);
 				break;
+			case 'p':
+				pointer(info, fmt, va_arg(va, void *));
+				while (isalnum(fmt[0]))
+					fmt++;
+				break;
 			case '%':
 				out(info, '%');
 			default:

Signature · 1 line

2.11.0

Tom Rini

2017-04-11 13:18 UTC #1 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

Quoted 25 lines · Vignesh R

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

Quoted 6 lines

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 <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170411/6037e6a6/attachment.sig>

Simon Glass

2017-04-11 13:56 UTC #2 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

In reply to Vignesh R

Quoted 114 lines · Vignesh R

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 <[email protected]>

---

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(+)

lib/tiny-printf.c +59 -0

index 6def8f98aa41..0b04813dc206 100644
@@ -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;

lib/tiny-printf.c static void div_out(struct printf_info *info, unsigned long *num, · line 123

Quoted 10 lines

lib/tiny-printf.c +10 -0

+                       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!

Quoted 35 lines

lib/tiny-printf.c +35 -0

+                       }
+               }
+       }
+       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:
+ *

lib/tiny-printf.c static void div_out(struct printf_info *info, unsigned long *num, · line 168

Quoted 10 lines

lib/tiny-printf.c +10 -0

+ * - '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

Tom Rini

2017-04-11 14:00 UTC #3 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

Quoted 6 lines · Simon Glass

Quoted 5 lines · Vignesh R
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.

[snip]

Quoted 6 lines

lib/tiny-printf.c static void div_out(struct printf_info *info, unsigned long *num, · line 168
Quoted 3 lines
lib/tiny-printf.c +3 -0
+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.

So, %p/%pa/%pa[p] are used in debug() prints, which only matter when DEBUG is set. Doing it this way means we globally bloat by (I think I snipped out..) 25 bytes? instead of ~250 bytes. And since we're in tiny-printf, which we use when every byte counts, I'm happier about only bloating by 25 bytes here.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170411/4ecccc62/attachment.sig>

Simon Glass

2017-04-11 14:03 UTC #4 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

Hi Tom,

Quoted 15 lines · Tom Rini

Quoted 6 lines · Simon Glass
Quoted 5 lines · Vignesh R
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.
[snip]
Quoted 6 lines
lib/tiny-printf.c static void div_out(struct printf_info *info, unsigned long *num, · line 168
Quoted 3 lines
lib/tiny-printf.c +3 -0
+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.

Quoted 5 lines

So, %p/%pa/%pa[p] are used in debug() prints, which only matter when DEBUG is set. Doing it this way means we globally bloat by (I think I snipped out..) 25 bytes? instead of ~250 bytes. And since we're in tiny-printf, which we use when every byte counts, I'm happier about only bloating by 25 bytes here.

What I mean is that typically DEBUG is enabled file by file. So if I want to output something I need to enable DEBUG in this file as well? That seems confusing to me. Perhaps it needs another CONFIG option?

Regards,
Simon

Tom Rini

2017-04-11 14:13 UTC #5 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

Quoted 24 lines · Simon Glass

Hi Tom,
Quoted 15 lines · Tom Rini
Quoted 6 lines · Simon Glass
Quoted 5 lines · Vignesh R
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.
[snip]
Quoted 6 lines
lib/tiny-printf.c static void div_out(struct printf_info *info, unsigned long *num, · line 168
Quoted 3 lines
lib/tiny-printf.c +3 -0
+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.
Quoted 5 lines
So, %p/%pa/%pa[p] are used in debug() prints, which only matter when DEBUG is set. Doing it this way means we globally bloat by (I think I snipped out..) 25 bytes? instead of ~250 bytes. And since we're in tiny-printf, which we use when every byte counts, I'm happier about only bloating by 25 bytes here.

Quoted 3 lines

What I mean is that typically DEBUG is enabled file by file. So if I want to output something I need to enable DEBUG in this file as well? That seems confusing to me. Perhaps it needs another CONFIG option?

I usually end up whacking DEBUG into common.h myself, so I hadn't thought about it that way. Long-term, yeah, we should think about how to handle debug stuff as there's times you want everything on and times you just want a little bit on, and we're talking about the we-need-space case here too.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170411/f606ae4b/attachment.sig>

Simon Glass

2017-04-11 14:28 UTC #6 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

Quoted 29 lines · Tom Rini

Quoted 24 lines · Simon Glass
Hi Tom,
Quoted 15 lines · Tom Rini
Quoted 6 lines · Simon Glass
Quoted 5 lines · Vignesh R
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.
[snip]
Quoted 6 lines
lib/tiny-printf.c static void div_out(struct printf_info *info, unsigned long *num, · line 168
Quoted 3 lines
lib/tiny-printf.c +3 -0
+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.
Quoted 5 lines
So, %p/%pa/%pa[p] are used in debug() prints, which only matter when DEBUG is set. Doing it this way means we globally bloat by (I think I snipped out..) 25 bytes? instead of ~250 bytes. And since we're in tiny-printf, which we use when every byte counts, I'm happier about only bloating by 25 bytes here.
Quoted 3 lines
What I mean is that typically DEBUG is enabled file by file. So if I want to output something I need to enable DEBUG in this file as well? That seems confusing to me. Perhaps it needs another CONFIG option?

Quoted 5 lines

I usually end up whacking DEBUG into common.h myself, so I hadn't thought about it that way. Long-term, yeah, we should think about how to handle debug stuff as there's times you want everything on and times you just want a little bit on, and we're talking about the we-need-space case here too.

OK. Would be a nice little project for someone :-)

Reviewed-by Simon Glass <[email protected]>

Tom Rini

2017-04-14 21:10 UTC #7 lore raw

PATCH v2 1/1 tiny-printf: Add support for %p format

In reply to Vignesh R

Quoted 25 lines · Vignesh R

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

Quoted 8 lines

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 <[email protected]>
Reviewed-by Tom Rini <[email protected]>
Reviewed-by Simon Glass <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170414/835c976e/attachment.sig>