All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
  2019-03-21 17:12 [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support Ley Foon Tan
@ 2019-03-21 11:22 ` Tom Rini
  2019-03-22  1:28   ` Ley Foon Tan
  0 siblings, 1 reply; 7+ messages in thread
From: Tom Rini @ 2019-03-21 11:22 UTC (permalink / raw)
  To: u-boot

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190321/4ee8bacb/attachment.sig>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
@ 2019-03-21 17:12 Ley Foon Tan
  2019-03-21 11:22 ` Tom Rini
  0 siblings, 1 reply; 7+ messages in thread
From: Ley Foon Tan @ 2019-03-21 17:12 UTC (permalink / raw)
  To: u-boot

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;
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
  2019-03-21 11:22 ` Tom Rini
@ 2019-03-22  1:28   ` Ley Foon Tan
  2019-03-22  1:31     ` Tom Rini
  0 siblings, 1 reply; 7+ messages in thread
From: Ley Foon Tan @ 2019-03-22  1:28 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
  2019-03-22  1:28   ` Ley Foon Tan
@ 2019-03-22  1:31     ` Tom Rini
  2019-03-22  9:01       ` Ley Foon Tan
  2019-03-22 10:53       ` Marek Vasut
  0 siblings, 2 replies; 7+ messages in thread
From: Tom Rini @ 2019-03-22  1:31 UTC (permalink / raw)
  To: u-boot

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.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
  2019-03-22  1:31     ` Tom Rini
@ 2019-03-22  9:01       ` Ley Foon Tan
  2019-03-22 11:47         ` Tom Rini
  2019-03-22 10:53       ` Marek Vasut
  1 sibling, 1 reply; 7+ messages in thread
From: Ley Foon Tan @ 2019-03-22  9:01 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
  2019-03-22  1:31     ` Tom Rini
  2019-03-22  9:01       ` Ley Foon Tan
@ 2019-03-22 10:53       ` Marek Vasut
  1 sibling, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2019-03-22 10:53 UTC (permalink / raw)
  To: u-boot

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.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support
  2019-03-22  9:01       ` Ley Foon Tan
@ 2019-03-22 11:47         ` Tom Rini
  0 siblings, 0 replies; 7+ messages in thread
From: Tom Rini @ 2019-03-22 11:47 UTC (permalink / raw)
  To: u-boot

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!

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-03-22 11:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-21 17:12 [U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support Ley Foon Tan
2019-03-21 11:22 ` Tom Rini
2019-03-22  1:28   ` Ley Foon Tan
2019-03-22  1:31     ` Tom Rini
2019-03-22  9:01       ` Ley Foon Tan
2019-03-22 11:47         ` Tom Rini
2019-03-22 10:53       ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.