[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Vignesh Raghavendra]

Faiz,

On 31/01/20 11:44 pm, Simon Glass wrote:
> Hi Vignesh,
> 
> On Thu, 30 Jan 2020 at 22:12, Vignesh Raghavendra <vigneshr at ti.com> wrote:
>>
>> Hi Simon,
>>
>> On 31/01/20 7:57 am, Simon Glass wrote:
>>> Hi Faiz,
>>>
>>> On Thu, 30 Jan 2020 at 08:22, Faiz Abbas <faiz_abbas at ti.com> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On 22/10/19 4:56 am, Simon Glass wrote:
>>>>> With a bit of code reordering we can support %p using the existing code
>>>>> for ulong.
>>>>>
>>>>> Move the %p code up and adjust the logic accordingly.
>>>>>
>>
>> [...]
>>>>
>>>> Retry time exceeded; starting again
>>>> Problem booting with BOOTP
>>>> SPL: failed to boot from all boot devices
>>>> ### ERROR ### Please RESET the board ###
>>>>
>>>> Reverting this patch on the latest U-boot master fixes the issue for me.
>>>>
>>>> I'll look into this more deeply tomorrow. Let me know if you see
>>>> something obviously wrong with the patch.
>>>
>>> Well one thing is that eth_env_set_enetaddr() called from the board's
>>> board.c has this:
>>>
>>> sprintf(buf, "%pM", enetaddr);
>>>
>>> which is not supported with tiny-printf.
>>
>> That is not true. %pM is supported when SPL_NET_SUPPORT is enabled. See:
>>
>> https://gitlab.denx.de/u-boot/u-boot/blob/master/lib/tiny-printf.c#L183
>>
>> I added this specifically to support Ethernet Boot usecases on TI platforms
>>
>> But above commit seems to move pointer() function that formats the
>> output under #ifdef DEBUG which definitely breaks %pM
> 
> OK I see. I think it is too confusing to use #ifdef DEBUG in this code.
> 
> One fix would be to change pointer() to return true if it actually
> does something. I'll take a look.
> 
> This code needs tests also. Vignesh, do you feel like writing something?
> 

Is there a testcase for full printf()? I am not sure where to look for.
Is test/print_ut.c the right place to add new test?


-- 
Regards
Vignesh