[U-Boot] [PATCH v2 1/2] APM82xxx: Add CPU and other peripheral support

[Stefan Roese]

Hi Marri,

On Friday 03 September 2010 03:48:44 Tirumala Marri wrote:
> > >  	mtdcr	ISRAM0_SB0CR,r1
> > > 
> > > @@ -752,7 +754,11 @@ _start:
> > >  	mtdcr	ISRAM1_PMEG,r1
> > >  	
> > >  	lis	r1,0x0004		/* BAS = 4_0004_0000 */
> > > 
> > > +#if defined(CONFIG_APM82XXX) /* APM82XXX only has 32KB of OCM */
> > > +	ori     r1,r1,0x0784            /* 32k */
> > > +#else
> > > 
> > >  	ori	r1,r1,0x0984		/* 64k */
> > 
> > Please get rid of these magic numbers and add a #define for a symbolic
> > constant so the code becomes readable, and the selection can be done
> > on a per-CPU base, i. e. without needing #ifdef's here.
> 
> [Marri] I will define CONFIG_SYS_OCM_SIZE . But for now this is only added
> Bluestone.h. I need to remove  "#if defined(CONFIG_APM821XX) I will have
> to
> Add CONFIG_SYS_OCM_SIZE to all the board config files which are based on
> 460Ex and 460GT.
> I think that would be completely different patch.

Since the OCM size is SoC specific, I would prefer not to put such a define in 
each and every board config header, but in a SoC header. There you could 
automatically select the OCM size depending on the SoC type. No change in the 
board config header needed.
 
Cheers,
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