[U-Boot] [PATCH] Introduce CONFIG_4xx_HAS_OPB

[Stefan Roese <sr@denx.de>]

Josh,

On Thursday 09 October 2008, Josh Boyer wrote:
> The lib_ppc/board.c file will fill in the bi_opbfreq variable
> in the bd_t structure for PowerPC 4xx platforms.  However, it
> currently seems to be coupled together with the bi_pci_busfreq
> variable under a series of ifdefs for particular CPU types.
> As a result, it is rather easy to miss getting bi_opbfreq
> populated on boards when doing a board port.  This is the
> case for CONFIG_405EZ for example.
>
> This patch introduces a CONFIG_4xx_HAS_OPB option that is
> set to indicate that the platform uses that variable in the
> bd_t structure.  It decouples this from the PCI bus setting
> and sets is properly for CPUs that have this defined.

I thought about it a little more. Some comments below...

> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
>
> diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h
> index 54ac01d..6673cd4 100644
> --- a/include/asm-ppc/u-boot.h
> +++ b/include/asm-ppc/u-boot.h
> @@ -112,12 +112,14 @@ typedef struct bd_info {
>  	unsigned char   bi_enet3addr[6];
>  #endif
>
> +#if defined(CONFIG_4xx_HAS_OPB)
> +	unsigned int	bi_opbfreq;		/* OPB clock in Hz */
> +#endif

It just occurred to me that this change break Linux compatibility. Usually you 
can just copy this file to the include/asm-ppc directory in Linux to have the 
same bd_info as in U-Boot (that's old arch/ppc of course). But Linux doesn't 
know about this CONFIG_4xx_HAS_OPB define. So this will not work.

You should probably better leave this file untouched as the 405EZ is correct.

>  #if defined(CONFIG_405GP) || defined(CONFIG_405EP) || \
>      defined(CONFIG_405EZ) || defined(CONFIG_440GX) || \
>      defined(CONFIG_440EP) || defined(CONFIG_440GR) || \
>      defined(CONFIG_440EPX) || defined(CONFIG_440GRX) || \
>      defined(CONFIG_460EX) || defined(CONFIG_460GT)
> -	unsigned int	bi_opbfreq;		/* OPB clock in Hz */
>  	int		bi_iic_fast[2];		/* Use fast i2c mode */
>  #endif
>  #if defined(CONFIG_NX823)
> diff --git a/include/ppc4xx.h b/include/ppc4xx.h
> index e216663..be6696d 100644
> --- a/include/ppc4xx.h
> +++ b/include/ppc4xx.h
> @@ -46,6 +46,15 @@
>  #define CONFIG_SDRAM_PPC4xx_IBM_DDR2	/* IBM DDR(2) controller */
>  #endif
>
> +/* Configure the OPB variable */
> +#if defined(CONFIG_405GP) || defined(CONFIG_405EP) || \
> +    defined(CONFIG_405EZ) || defined(CONFIG_440GX) || \
> +    defined(CONFIG_440EP) || defined(CONFIG_440GR) || \
> +    defined(CONFIG_440EPX) || defined(CONFIG_440GRX) || \
> +    defined(CONFIG_460EX) || defined(CONFIG_460GT)
> +#define CONFIG_4xx_HAS_OPB
> +#endif

Hmmm. Why not something like this:

#if !defined(CONFIG_XILINX_405) && !defined(CONFIG_XILINX_440)
#define CONFIG_4xx_HAS_OPB
#endif

Does this work?

> +
>  /* PLB4 CrossBar Arbiter Core supported across PPC4xx families */
>  #if defined(CONFIG_405EX) || \
>      defined(CONFIG_440EP) || defined(CONFIG_440EPX) || \
> diff --git a/lib_ppc/board.c b/lib_ppc/board.c
> index c02ac62..977fc19 100644
> --- a/lib_ppc/board.c
> +++ b/lib_ppc/board.c
> @@ -611,10 +611,12 @@ void board_init_f (ulong bootflag)
>      defined(CONFIG_440EP) || defined(CONFIG_440GR) || \
>      defined(CONFIG_440EPX) || defined(CONFIG_440GRX)
>  	bd->bi_pci_busfreq = get_PCI_freq ();
> -	bd->bi_opbfreq = get_OPB_freq ();
>  #elif defined(CONFIG_XILINX_405)
>  	bd->bi_pci_busfreq = get_PCI_freq ();
>  #endif
> +#if defined(CONFIG_4xx_HAS_OPB)
> +	bd->bi_opbfreq = get_OPB_freq ();
> +#endif
>  #endif
>
>  	debug ("New Stack Pointer is: %08lx\n", addr_sp);

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
=====================================================================