Re: [PATCH v14 05/11] log: select physical address formatting in a generic way

Abdellatif El Khlifi
2023-07-10T12:15:06Z

Hi Simon,

On Fri, Jul 07, 2023 at 06:34:14PM +0100, Simon Glass wrote:
> Hi Abdellatif,
>
> On Fri, 7 Jul 2023 at 15:44, Abdellatif El Khlifi
> <abdellatif.elkhlifi@arm.com> wrote:
> >
> > sets the log formatting according to the platform (64-bit vs 32-bit)
> >
> > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > ---
> > include/log.h | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/include/log.h b/include/log.h
> > index 3bab40b617..689cef905b 100644
> > --- a/include/log.h
> > +++ b/include/log.h
> > @@ -686,4 +686,12 @@ static inline int log_get_default_format(void)
> > (IS_ENABLED(CONFIG_LOGF_FUNC) ? BIT(LOGF_FUNC) : 0);
> > }
> >
> > +/* Select the right physical address formatting according to the platform */
> > +#ifdef CONFIG_PHYS_64BIT
> > +#define PhysAddrLength "ll"
> > +#else
> > +#define PhysAddrLength ""
>
> Shouldn't this be "l" ? We normally use ulong for addresses.
>

The "ll" is needed by many architectures who declare "phys_addr_t" as "unsigned long long"

Example of architetures doing that: arm, powerpc, ...

Also mips and sandbox use "u64" for "phys_addr_t" , ( "u64" is an "unsigned long long").

Note: When using an "l" for arm, the compiler shows this warning:

cmd/armffa.c:174:18: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘phys_addr_t’ {aka ‘long long unsign
ed int’} [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat=-Wformat=8;;]
174 | log_info("device %s, addr " PHYS_ADDR_LN ", driver %s, ops " PHYS_ADDR_LN "\n",
| ^~~~~~~~~~~~~~~~~~
175 | dev->name,
176 | map_to_sysmem(dev),
| ~~~~~~~~~~~~~~~~~~
| |
| phys_addr_t {aka long long unsigned int}

Cheers,
Abdellatif