[U-Boot] [PATCH v3 06/23] mpc832x: add support for the mpc8321 based suvd3 board

[Heiko Schocher]

Hello Kim,

Kim Phillips wrote:
> On Mon, 21 Mar 2011 08:02:02 +0100
> Heiko Schocher <hs at denx.de> wrote:
> 
>> +++ b/arch/powerpc/cpu/mpc83xx/fdt.c
>> @@ -32,7 +32,8 @@ extern void ft_qe_setup(void *blob);
>>  
>>  DECLARE_GLOBAL_DATA_PTR;
>>  
>> -#if defined(CONFIG_BOOTCOUNT_LIMIT) && defined(CONFIG_MPC8360)
>> +#if defined(CONFIG_BOOTCOUNT_LIMIT) && \
>> +	(defined(CONFIG_MPC832x) || defined(CONFIG_MPC8360))
> 
> please replace 832x and 8360 with just CONFIG_QE

Ok.

>> +++ b/arch/powerpc/lib/bootcount.c
>> @@ -51,7 +51,7 @@
>>  #define CONFIG_SYS_BOOTCOUNT_ADDR	(CONFIG_SYS_IMMR + CPM_BOOTCOUNT_ADDR)
>>  #endif /* defined(CONFIG_MPC8260) */
>>  
>> -#if defined(CONFIG_MPC8360)
>> +#if defined(CONFIG_MPC832x) || defined(CONFIG_MPC8360)
> 
> same here

Ok.

>> diff --git a/include/configs/kmeter1.h b/include/configs/kmeter1.h
>> -#define CONFIG_83XX_CLKIN		66000000
>> -#define CONFIG_SYS_CLK_FREQ		66000000
>> -#define CONFIG_83XX_PCICLK		66000000
>> +#define CONFIG_SYS_SICRH		0x00000006
> 
> I realize this was a pre-existing condition, but please make this
> (SICRH_UC1EOBI | SICRH_UC2E1OBI) instead of 6.

Ok.

>> +#define CONFIG_SYS_SICRL		0x00000000
> 
> nit-pick, but fyi, one can save a raw write here by not defining it at
> all.

removed.

>>  /* PAXE:  icache cacheable, but dcache-inhibit and guarded */
>>  #define CONFIG_SYS_IBAT5L	(CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
>> -				BATL_MEMCOHERENCE)
>> +					BATL_MEMCOHERENCE)
> 
> B in BATL_ should be aligned to fall directly under the C in
> CONFIG_SYS_PAXE_BASE, like this:
> 
> #define CONFIG_SYS_IBAT5L       (CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
> 				 BATL_MEMCOHERENCE)

Ok.

>>  #define CONFIG_SYS_IBAT5U	(CONFIG_SYS_PAXE_BASE | BATU_BL_256M | \
>> -				BATU_VS | BATU_VP)
>> +					BATU_VS | BATU_VP)
> 
> same here, just as the next one is already:

Ok.

> 
>>  #define CONFIG_SYS_DBAT5L	(CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
>>  				 BATL_CACHEINHIBIT | BATL_GUARDEDSTORAGE)
> 
> <snip>
>> diff --git a/include/configs/suvd3.h b/include/configs/suvd3.h
> <snip>
>> +#define CONFIG_SYS_SICRH	0x00000006
> 
> afaict, SICRH doesn't documentally exist on 832x, so omit this line.
> This comment also applies to the following patches in this series:

Ok.

> [PATCH v3 07/23] mpc832x: add support for mpc8321 based tuxa1 board
> [PATCH v3 08/23] mpc832x: add support for mpc8321 based tuda1 board
> [PATCH v3 12/23] keymile, 8321 boards: move common definitions to km8321-common.h
> 
> even though the 12/23 patch seems to be a refactoring of the definition
> added by the others...<sigh>.
> 
> The rest of the 83xx-touching patches in the series look good to me.

Thanks for reviewing.

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany