From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Wed, 27 Aug 2014 09:24:10 -0600
Subject: [U-Boot] [PATCH v2 01/40] vsprintf: Add modifier for phys_addr_t
In-Reply-To: <20140827073751.GI15640@ulmo>
References: <1409067268-956-1-git-send-email-thierry.reding@gmail.com>
	<1409067268-956-2-git-send-email-thierry.reding@gmail.com>
	<CAPnjgZ11uEtwiH9OHsrHMceU28acTfJuWa96Xq_xTwo7T2frHQ@mail.gmail.com>
	<20140827073751.GI15640@ulmo>
Message-ID: <CAPnjgZ0=uKHUNbDpxEh_gcpdbS6KH9DytGgtr6t6L7i9CR4Pew@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Thierry,

On 27 August 2014 01:37, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Tue, Aug 26, 2014 at 05:14:17PM -0600, Simon Glass wrote:
>> Hi Thierry,
>>
>> On 26 August 2014 09:33, Thierry Reding <thierry.reding@gmail.com> wrote:
>> >
>> > From: Thierry Reding <treding@nvidia.com>
>> >
>> > Provide a new modifier to vsprintf() to print phys_addr_t variables to
>> > avoid having to cast or #ifdef when printing them out. The %pa modifier
>> > is used for this purpose, so phys_addr_t variables need to be passed by
>> > reference, like so:
>> >
>> >         phys_addr_t start = 0;
>> >
>> >         printf("start: %pa\n", &start);
>> >
>> > Depending on the size of phys_addr_t this will print out the address
>> > with 8 or 16 hexadecimal digits following a 0x prefix.
>>
>> Would it be better to use %#pa to get the 0x prefix so we have the
>> option? Hex is the default in U-Boot.
>
> Yeah, that's very confusing since there's no way to tell apart a
> hexadecimal number from a decimal one since there's no decimal
> equivalent of the 0x prefix.
>
> If you insist I can change that (it should be a simple matter of
> dropping the SPECIAL flag in the 'a' case).

%p doesn't imply %#p, this seems like a unilateral change of
behaviour. So I think you should drop the SPECIAL flag.

It might be confusing in some cases but I feel it is quite expected
that an 8- or 16-digit value will be hex in U-Boot.

If you wish to change the default input/output base for U-Boot I feel
that would require a larger patch.

>
>> > Signed-off-by: Thierry Reding <treding@nvidia.com>
>> > ---
>> >  lib/vsprintf.c | 16 ++++++++++++++--
>> >  1 file changed, 14 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>> > index 7ec758e40fc5..044d5551bdd0 100644
>> > --- a/lib/vsprintf.c
>> > +++ b/lib/vsprintf.c
>> > @@ -518,6 +518,8 @@ static char *ip4_addr_string(char *buf, char *end, u8 *addr, int field_width,
>> >  static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>> >                 int field_width, int precision, int flags)
>> >  {
>> > +       u64 num = (uintptr_t)ptr;
>> > +
>>
>> Will this impact code size much? I suppose it is vsprintf() so it
>> doesn't matter too much?
>
> Doing only this change and the corresponding change in the call to
> number() results in the exact sizes:
>
>         $ size build/jetson-tk1.{before,after}/u-boot
>            text    data     bss     dec     hex filename
>          310434   10872  309184  630490   99eda build/jetson-tk1.before/u-boot
>          310434   10872  309184  630490   99eda build/jetson-tk1.after/u-boot
>
> Given that the num parameter of the number() function is already a u64
> the compiler will presumably automatically handle the ptr argument as a
> 64-bit value.

I hadn't picked that up - I though ulong was 32-bit. Still this code
affects most boards.

Regards,
Simon


>
> Thierry