Subject: Re: [patch 04/10] 4xx bootwrapper reworks

From: David Gibson <[email-masked]>
Date: Mon, 6 Aug 2007 14:38:55 +1000

On Fri, Aug 03, 2007 at 11:09:04AM -0500, Josh Boyer wrote:
> Make the fixup_memsize function common for all of 4xx as several chips share
> the same SDRAM controller.  Also add functions to reset 40x chips and quiesce
> the ethernet.
> 
> Signed-off-by: Josh Boyer <[email-masked]>
> 
> ---
>  arch/powerpc/boot/44x.h   |    4 ----
>  arch/powerpc/boot/4xx.c   |   36 +++++++++++++++++++++++++++++++-----
>  arch/powerpc/boot/4xx.h   |   20 ++++++++++++++++++++
>  arch/powerpc/boot/dcr.h   |    2 ++
>  arch/powerpc/boot/ebony.c |    3 ++-
>  5 files changed, 55 insertions(+), 10 deletions(-)
> 
> --- linux-2.6.orig/arch/powerpc/boot/44x.h
> +++ linux-2.6/arch/powerpc/boot/44x.h
> @@ -10,10 +10,6 @@
>  #ifndef _PPC_BOOT_44X_H_
>  #define _PPC_BOOT_44X_H_
>  
> -void ibm44x_fixup_memsize(void);
> -void ibm4xx_fixup_ebc_ranges(const char *ebc);
> -
> -void ibm44x_dbcr_reset(void);
>  void ebony_init(void *mac0, void *mac1);
>  
>  #endif /* _PPC_BOOT_44X_H_ */
> --- linux-2.6.orig/arch/powerpc/boot/4xx.c
> +++ linux-2.6/arch/powerpc/boot/4xx.c
> @@ -21,8 +21,8 @@
>  #include "reg.h"
>  #include "dcr.h"
>  
> -/* Read the 44x memory controller to get size of system memory. */
> -void ibm44x_fixup_memsize(void)
> +/* Read the 4xx SDRAM controller to get size of system memory. */
> +void ibm4xx_fixup_memsize(void)
>  {
>  	int i;
>  	unsigned long memsize, bank_config;
> @@ -39,22 +39,48 @@ void ibm44x_fixup_memsize(void)
>  	dt_fixup_memory(0, memsize);
>  }
>  
> -#define SPRN_DBCR0		0x134
> -#define   DBCR0_RST_SYSTEM	0x30000000
> +#define DBCR0_RST_SYSTEM 0x30000000

Rather than just removing these defines and using hardcoded values,
I'd prefer to see separate SPRN_DBCR0_40X and SPRN_DBCR0_44X defines.

[snip]
> +#define EMAC_RESET 0x20000000
> +#define MAL_RESET 0x80000000

I think the MAL_RESET definition should go in the same place as the
DCR number definition.

> +void ibm4xx_quiesce_eth(u32 *emac0, u32 *emac1)
> +{

As I think I said before, I'm not really happy with this being
hardcoded assuming exactly 2 ethernets.

> +	/* Quiesce the MAL and EMAC(s) since PIBS/OpenBIOS don't do this for us */
> +	if (emac0)
> +		*emac0 = EMAC_RESET;
> +	if (emac1)
> +		*emac1 = EMAC_RESET;
> +
> +	mtdcr(DCRN_MAL0_CFG, MAL_RESET);
> +}
[snip]

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson