[U-Boot] [PATCH] ppc4xx: Enable support for 64bit printf on all PPC4xx variants

[Stefan Roese]

Hi Wolfgang,

On Monday 06 July 2009 13:17:59 Wolfgang Denk wrote:
> > Until now these defines were only enabled for the PPC440 variants. This
> > patch now defines CONFIG_SYS_64BIT_VSPRINTF and CONFIG_SYS_64BIT_STRTOUL
> > on PPC40x now as well. This removes the compilation warning with the new
> > updated NAND code.
> >
> > Signed-off-by: Stefan Roese <sr at denx.de>
> > ---
> >  include/ppc4xx.h |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/ppc4xx.h b/include/ppc4xx.h
> > index 55ff323..d9f04f7 100644
> > --- a/include/ppc4xx.h
> > +++ b/include/ppc4xx.h
> > @@ -109,13 +109,14 @@
> >
> >  #endif /* 440EP/EPX 440GR/GRX 440SP/SPE 460EX/GT/SX 405EX*/
> >
> > -#if defined(CONFIG_440)
> >  /*
> >   * Enable long long (%ll ...) printf format on 440 PPC's since most of
> >   * them support 36bit physical addressing
> >   */
> >  #define CONFIG_SYS_64BIT_VSPRINTF
> >  #define CONFIG_SYS_64BIT_STRTOUL
> > +
> > +#if defined(CONFIG_440)
> >  #include <ppc440.h>
> >  #else
> >  #include <ppc405.h>
>
> NAK.
>
> This needs to be cleaned up, but differently. Sorry that I did not
> catch this earlier - it seems the related code was introduced by
> commit f2302d44 (which sailed under the innocent title "Fix merge
> problems").
>
> Please move these CONFIG_SYS_* settings out of this file.
>
> For me it is not acceptable to set configuration options in global
> header files like include/ppc4xx.h; CONFIG_* settings are supposed to
> be selected by the board configuration files, and only there.

I don't share this opinion. I think it's perfectly valid to enable CONFIG_* 
settings in global header files (or some other global files). Sometimes there 
are dependencies that can (or even should) be solved this way. One example is 
the 4xx NAND driver which needs to configure/set CONFIG_MTD_NAND_ECC_SMC for 
correct operation. This is currently done directly in 
drivers/mtd/nand/nand_ecc.c. The plan also proposed by Scott Wood is to remove 
this define from this file once we have the Kconfig framework intact. Then 
Kconfig will enable this define as a dependency for PPC4xx. As you know this 
is common praxis in Linux as well.

> I hope we don't have any more such #defines hidden in other header
> files?

I vote for completely removing these defines then (or at least 
CONFIG_SYS_64BIT_VSPRINTF) and by this enabling the 64bit printf format for 
all boards. I myself have hunted problems disguised by incorrect 64bit 
variables/values printed in some %ll formats a few times, before I noticed 
that this 64bit format it not supported at all.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================