[U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
Add "%ll" modifier support for tiny printf.
- Tested on ARM32 and ARM64 systems. - Tested "%lld", "%llu", "%llx" and "%p" format with minimum and maximum ranges. Compared tiny printf output with full printf.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com --- lib/tiny-printf.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index ebef92fc9f..692430efac 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -33,8 +33,8 @@ static void out_dgt(struct printf_info *info, char dgt) info->zs = 1; }
-static void div_out(struct printf_info *info, unsigned long *num, - unsigned long div) +static void div_out(struct printf_info *info, unsigned long long *num, + unsigned long long div) { unsigned char dgt = 0;
@@ -160,8 +160,8 @@ static void ip4_addr_string(struct printf_info *info, u8 *addr) static void pointer(struct printf_info *info, const char *fmt, void *ptr) { #ifdef DEBUG - unsigned long num = (uintptr_t)ptr; - unsigned long div; + unsigned long long num = (uintptr_t)ptr; + unsigned long long div; #endif
switch (*fmt) { @@ -199,9 +199,9 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va) { char ch; char *p; - unsigned long num; - char buf[12]; - unsigned long div; + unsigned long long num; + char buf[22]; + unsigned long long div;
while ((ch = *(fmt++))) { if (ch != '%') { @@ -210,6 +210,7 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va) bool lz = false; int width = 0; bool islong = false; + bool islonglong = false;
ch = *(fmt++); if (ch == '-') @@ -229,7 +230,13 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va) } if (ch == 'l') { ch = *(fmt++); - islong = true; + if (ch == 'l') { + islonglong = true; + ch = *(fmt++); + } else { + islong = true; + } + }
info->bf = buf; @@ -242,7 +249,10 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va) case 'u': case 'd': div = 1000000000; - if (islong) { + if (islonglong) { + num = va_arg(va, unsigned long long); + div *= div * 10; + } else if (islong) { num = va_arg(va, unsigned long); if (sizeof(long) > 4) div *= div * 10; @@ -251,10 +261,13 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va) }
if (ch == 'd') { - if (islong && (long)num < 0) { + if (islonglong && (long long)num < 0) { + num = -(long long)num; + out(info, '-'); + } else if (islong && (long)num < 0) { num = -(long)num; out(info, '-'); - } else if (!islong && (int)num < 0) { + } else if ((!islong && !islonglong) && (int)num < 0) { num = -(int)num; out(info, '-'); } @@ -267,9 +280,12 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va) } break; case 'x': - if (islong) { + if (islonglong) { + num = va_arg(va, unsigned long long); + div = 1ULL << (sizeof(long long) * 8 - 4); + } else if (islong) { num = va_arg(va, unsigned long); - div = 1UL << (sizeof(long) * 8 - 4); + div = 1ULL << (sizeof(long) * 8 - 4); } else { num = va_arg(va, unsigned int); div = 0x10000000;
On Fri, Mar 22, 2019 at 01:12:14AM +0800, Ley Foon Tan wrote:
Add "%ll" modifier support for tiny printf.
- Tested on ARM32 and ARM64 systems.
- Tested "%lld", "%llu", "%llx" and "%p" format with minimum and maximum ranges. Compared tiny printf output with full printf.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
lib/tiny-printf.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
What's the use case for this, how much does it grow the size, and can the code in question be changed to use a different format modifier or be debug() instead? Tiny printf isn't intended to cover all formats but rather still allow some amount of printf on constrained systems. Thanks!
On Thu, Mar 21, 2019 at 7:22 PM Tom Rini trini@konsulko.com wrote:
On Fri, Mar 22, 2019 at 01:12:14AM +0800, Ley Foon Tan wrote:
Add "%ll" modifier support for tiny printf.
- Tested on ARM32 and ARM64 systems.
- Tested "%lld", "%llu", "%llx" and "%p" format with minimum and maximum ranges. Compared tiny printf output with full printf.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
lib/tiny-printf.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
What's the use case for this, how much does it grow the size, and can the code in question be changed to use a different format modifier or be debug() instead? Tiny printf isn't intended to cover all formats but rather still allow some amount of printf on constrained systems. Thanks!
-- Tom
This is to support printf %lld, %llu and %llx and 64-bit %p when CONFIG_USE_TINY_PRINTF=y is enabled. In 64-bit system, phys_size_t and phys_addr_t are unsigned long long. Printf these variables will see rubbish in existing tiny printf.
Example %ll printf; printf("9223372036854775807 ==> %lld \n", (long long)9223372036854775807); printf("0xffffffffffffffff ==> 0x%llx \n", (unsigned long long)0xffffffffffffffff);
There are few issues I've noticed with original tiny printf: - Some common codes use %ll format - %p in tiny printf only support 32-bit, it can't support 64-bit address.
If DEBUG is defined and with tiny printf. You can see all rubbish x for %llx printf. The most serious issue is it cause system hang at line below (printf from common code). addr=x level=0 idx=x PTE at level 16384: x Creating table for virt 0xx Setting 4000 to addr=5000 addr=x level=0 idx=x PTE at level 16384: x idx=x PTE at level 20480: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5000 to block virt=x addr=x level=1073741824 idx=x PTE at level 16384: x addr=x level=1073741824 idx=x PTE at level 16384: x idx=x PTE 1 at level 20488: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5008 to block virt=x addr=x level=
Example output after apply this patch: idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003 Checking if pte fits for virt=c0600000 size=1fa00000 blocksize=40000000 addr=c0600000 level=2 idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003
ARM64: ---------- It increase 200 bytes.
Before: text data bss dec hex filename 69618 5296 232 75146 1258a spl/u-boot-spl
After: text data bss dec hex filename 69818 5296 232 75346 12652 spl/u-boot-spl
ARM32: ---------- It increase 644 bytes.
Before: text data bss dec hex filename 31825 2968 132 34925 886d spl/u-boot-spl
After: text data bss dec hex filename 32469 2968 132 35569 8af1 spl/u-boot-spl
Regards Ley Foon
On Fri, Mar 22, 2019 at 09:28:21AM +0800, Ley Foon Tan wrote:
On Thu, Mar 21, 2019 at 7:22 PM Tom Rini trini@konsulko.com wrote:
On Fri, Mar 22, 2019 at 01:12:14AM +0800, Ley Foon Tan wrote:
Add "%ll" modifier support for tiny printf.
- Tested on ARM32 and ARM64 systems.
- Tested "%lld", "%llu", "%llx" and "%p" format with minimum and maximum ranges. Compared tiny printf output with full printf.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
lib/tiny-printf.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
What's the use case for this, how much does it grow the size, and can the code in question be changed to use a different format modifier or be debug() instead? Tiny printf isn't intended to cover all formats but rather still allow some amount of printf on constrained systems. Thanks!
-- Tom
This is to support printf %lld, %llu and %llx and 64-bit %p when CONFIG_USE_TINY_PRINTF=y is enabled. In 64-bit system, phys_size_t and phys_addr_t are unsigned long long. Printf these variables will see rubbish in existing tiny printf.
Example %ll printf; printf("9223372036854775807 ==> %lld \n", (long long)9223372036854775807); printf("0xffffffffffffffff ==> 0x%llx \n", (unsigned long long)0xffffffffffffffff);
There are few issues I've noticed with original tiny printf:
- Some common codes use %ll format
- %p in tiny printf only support 32-bit, it can't support 64-bit address.
If DEBUG is defined and with tiny printf. You can see all rubbish x for %llx printf. The most serious issue is it cause system hang at line below (printf from common code). addr=x level=0 idx=x PTE at level 16384: x Creating table for virt 0xx Setting 4000 to addr=5000 addr=x level=0 idx=x PTE at level 16384: x idx=x PTE at level 20480: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5000 to block virt=x addr=x level=1073741824 idx=x PTE at level 16384: x addr=x level=1073741824 idx=x PTE at level 16384: x idx=x PTE 1 at level 20488: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5008 to block virt=x addr=x level=
Example output after apply this patch: idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003 Checking if pte fits for virt=c0600000 size=1fa00000 blocksize=40000000 addr=c0600000 level=2 idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003
ARM64:
It increase 200 bytes.
Before: text data bss dec hex filename 69618 5296 232 75146 1258a spl/u-boot-spl
After: text data bss dec hex filename 69818 5296 232 75346 12652 spl/u-boot-spl
ARM32:
It increase 644 bytes.
Before: text data bss dec hex filename 31825 2968 132 34925 886d spl/u-boot-spl
After: text data bss dec hex filename 32469 2968 132 35569 8af1 spl/u-boot-spl
That's a lot, especially since we have tiny printf platforms that are really on the edge. Can you just not use TINY_PRINTF when using DEBUG? Maybe we need a Kconfig symbol for some debug stuff. But I don't think we can just add this.
On Fri, Mar 22, 2019 at 9:31 AM Tom Rini trini@konsulko.com wrote:
On Fri, Mar 22, 2019 at 09:28:21AM +0800, Ley Foon Tan wrote:
On Thu, Mar 21, 2019 at 7:22 PM Tom Rini trini@konsulko.com wrote:
On Fri, Mar 22, 2019 at 01:12:14AM +0800, Ley Foon Tan wrote:
Add "%ll" modifier support for tiny printf.
- Tested on ARM32 and ARM64 systems.
- Tested "%lld", "%llu", "%llx" and "%p" format with minimum and maximum ranges. Compared tiny printf output with full printf.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
lib/tiny-printf.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
What's the use case for this, how much does it grow the size, and can the code in question be changed to use a different format modifier or be debug() instead? Tiny printf isn't intended to cover all formats but rather still allow some amount of printf on constrained systems. Thanks!
-- Tom
This is to support printf %lld, %llu and %llx and 64-bit %p when CONFIG_USE_TINY_PRINTF=y is enabled. In 64-bit system, phys_size_t and phys_addr_t are unsigned long long. Printf these variables will see rubbish in existing tiny printf.
Example %ll printf; printf("9223372036854775807 ==> %lld \n", (long long)9223372036854775807); printf("0xffffffffffffffff ==> 0x%llx \n", (unsigned long long)0xffffffffffffffff);
There are few issues I've noticed with original tiny printf:
- Some common codes use %ll format
- %p in tiny printf only support 32-bit, it can't support 64-bit address.
If DEBUG is defined and with tiny printf. You can see all rubbish x for %llx printf. The most serious issue is it cause system hang at line below (printf from common code). addr=x level=0 idx=x PTE at level 16384: x Creating table for virt 0xx Setting 4000 to addr=5000 addr=x level=0 idx=x PTE at level 16384: x idx=x PTE at level 20480: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5000 to block virt=x addr=x level=1073741824 idx=x PTE at level 16384: x addr=x level=1073741824 idx=x PTE at level 16384: x idx=x PTE 1 at level 20488: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5008 to block virt=x addr=x level=
Example output after apply this patch: idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003 Checking if pte fits for virt=c0600000 size=1fa00000 blocksize=40000000 addr=c0600000 level=2 idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003
ARM64:
It increase 200 bytes.
Before: text data bss dec hex filename 69618 5296 232 75146 1258a spl/u-boot-spl
After: text data bss dec hex filename 69818 5296 232 75346 12652 spl/u-boot-spl
ARM32:
It increase 644 bytes.
Before: text data bss dec hex filename 31825 2968 132 34925 886d spl/u-boot-spl
After: text data bss dec hex filename 32469 2968 132 35569 8af1 spl/u-boot-spl
That's a lot, especially since we have tiny printf platforms that are really on the edge. Can you just not use TINY_PRINTF when using DEBUG?
Yes, we can. Our platform still have space for this.
Maybe we need a Kconfig symbol for some debug stuff. But I don't think we can just add this.
Regards Ley Foon
On Fri, Mar 22, 2019 at 05:01:55PM +0800, Ley Foon Tan wrote:
On Fri, Mar 22, 2019 at 9:31 AM Tom Rini trini@konsulko.com wrote:
On Fri, Mar 22, 2019 at 09:28:21AM +0800, Ley Foon Tan wrote:
On Thu, Mar 21, 2019 at 7:22 PM Tom Rini trini@konsulko.com wrote:
On Fri, Mar 22, 2019 at 01:12:14AM +0800, Ley Foon Tan wrote:
Add "%ll" modifier support for tiny printf.
- Tested on ARM32 and ARM64 systems.
- Tested "%lld", "%llu", "%llx" and "%p" format with minimum and maximum ranges. Compared tiny printf output with full printf.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
lib/tiny-printf.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
What's the use case for this, how much does it grow the size, and can the code in question be changed to use a different format modifier or be debug() instead? Tiny printf isn't intended to cover all formats but rather still allow some amount of printf on constrained systems. Thanks!
-- Tom
This is to support printf %lld, %llu and %llx and 64-bit %p when CONFIG_USE_TINY_PRINTF=y is enabled. In 64-bit system, phys_size_t and phys_addr_t are unsigned long long. Printf these variables will see rubbish in existing tiny printf.
Example %ll printf; printf("9223372036854775807 ==> %lld \n", (long long)9223372036854775807); printf("0xffffffffffffffff ==> 0x%llx \n", (unsigned long long)0xffffffffffffffff);
There are few issues I've noticed with original tiny printf:
- Some common codes use %ll format
- %p in tiny printf only support 32-bit, it can't support 64-bit address.
If DEBUG is defined and with tiny printf. You can see all rubbish x for %llx printf. The most serious issue is it cause system hang at line below (printf from common code). addr=x level=0 idx=x PTE at level 16384: x Creating table for virt 0xx Setting 4000 to addr=5000 addr=x level=0 idx=x PTE at level 16384: x idx=x PTE at level 20480: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5000 to block virt=x addr=x level=1073741824 idx=x PTE at level 16384: x addr=x level=1073741824 idx=x PTE at level 16384: x idx=x PTE 1 at level 20488: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5008 to block virt=x addr=x level=
Example output after apply this patch: idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003 Checking if pte fits for virt=c0600000 size=1fa00000 blocksize=40000000 addr=c0600000 level=2 idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003
ARM64:
It increase 200 bytes.
Before: text data bss dec hex filename 69618 5296 232 75146 1258a spl/u-boot-spl
After: text data bss dec hex filename 69818 5296 232 75346 12652 spl/u-boot-spl
ARM32:
It increase 644 bytes.
Before: text data bss dec hex filename 31825 2968 132 34925 886d spl/u-boot-spl
After: text data bss dec hex filename 32469 2968 132 35569 8af1 spl/u-boot-spl
That's a lot, especially since we have tiny printf platforms that are really on the edge. Can you just not use TINY_PRINTF when using DEBUG?
Yes, we can. Our platform still have space for this.
OK, then lets just defer this, thanks!
On 3/22/19 2:31 AM, Tom Rini wrote:
On Fri, Mar 22, 2019 at 09:28:21AM +0800, Ley Foon Tan wrote:
On Thu, Mar 21, 2019 at 7:22 PM Tom Rini trini@konsulko.com wrote:
On Fri, Mar 22, 2019 at 01:12:14AM +0800, Ley Foon Tan wrote:
Add "%ll" modifier support for tiny printf.
- Tested on ARM32 and ARM64 systems.
- Tested "%lld", "%llu", "%llx" and "%p" format with minimum and maximum ranges. Compared tiny printf output with full printf.
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
lib/tiny-printf.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
What's the use case for this, how much does it grow the size, and can the code in question be changed to use a different format modifier or be debug() instead? Tiny printf isn't intended to cover all formats but rather still allow some amount of printf on constrained systems. Thanks!
-- Tom
This is to support printf %lld, %llu and %llx and 64-bit %p when CONFIG_USE_TINY_PRINTF=y is enabled. In 64-bit system, phys_size_t and phys_addr_t are unsigned long long. Printf these variables will see rubbish in existing tiny printf.
Example %ll printf; printf("9223372036854775807 ==> %lld \n", (long long)9223372036854775807); printf("0xffffffffffffffff ==> 0x%llx \n", (unsigned long long)0xffffffffffffffff);
There are few issues I've noticed with original tiny printf:
- Some common codes use %ll format
- %p in tiny printf only support 32-bit, it can't support 64-bit address.
If DEBUG is defined and with tiny printf. You can see all rubbish x for %llx printf. The most serious issue is it cause system hang at line below (printf from common code). addr=x level=0 idx=x PTE at level 16384: x Creating table for virt 0xx Setting 4000 to addr=5000 addr=x level=0 idx=x PTE at level 16384: x idx=x PTE at level 20480: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5000 to block virt=x addr=x level=1073741824 idx=x PTE at level 16384: x addr=x level=1073741824 idx=x PTE at level 16384: x idx=x PTE 1 at level 20488: x Checking if pte fits for virt=x size=x blocksize=x Setting PTE 5008 to block virt=x addr=x level=
Example output after apply this patch: idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003 Checking if pte fits for virt=c0600000 size=1fa00000 blocksize=40000000 addr=c0600000 level=2 idx=0 PTE 8000 at level 0: 9003 idx=3 PTE 9018 at level 1: a003
ARM64:
It increase 200 bytes.
Before: text data bss dec hex filename 69618 5296 232 75146 1258a spl/u-boot-spl
After: text data bss dec hex filename 69818 5296 232 75346 12652 spl/u-boot-spl
ARM32:
It increase 644 bytes.
Before: text data bss dec hex filename 31825 2968 132 34925 886d spl/u-boot-spl
After: text data bss dec hex filename 32469 2968 132 35569 8af1 spl/u-boot-spl
That's a lot, especially since we have tiny printf platforms that are really on the edge. Can you just not use TINY_PRINTF when using DEBUG? Maybe we need a Kconfig symbol for some debug stuff. But I don't think we can just add this.
I wonder if this could be implemented in some way which doesn't add so much. There will be more and more 64bit systems which use this eventually.
participants (4)
-
Ley Foon Tan -
Ley Foon Tan -
Marek Vasut -
Tom Rini