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 <tr...@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....@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
