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

[Simon Glass]

Hi Vignesh,

On Tue, 4 Feb 2020 at 00:58, Vignesh Raghavendra <vigneshr at ti.com> wrote:
>
> 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?

Yes.

See also this commit in u-boot-dm/testing

    test: Add a way to check each line of console output

Regards,
Simon