[U-Boot] [PATCH] lib: tiny-printf: Add "%ll" modifier support

[Tom Rini]

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 at 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 at 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>