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

[Simon Glass]

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?


>
> >
> > Before my patch it would probably produce an empty string, but now it
> > will produce garbage. Perhaps need an SPL check there.

Regards,
Simon