From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Thu, 31 Mar 2011 08:02:52 +0200 Subject: [U-Boot] [PATCH v3 06/23] mpc832x: add support for the mpc8321 based suvd3 board In-Reply-To: <20110330194306.af03e02a.kim.phillips@freescale.com> References: <1299591018-8944-1-git-send-email-hs@denx.de> <1300690939-31511-1-git-send-email-hs@denx.de> <1300690939-31511-7-git-send-email-hs@denx.de> <20110330194306.af03e02a.kim.phillips@freescale.com> Message-ID: <4D94190C.8060203@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Kim, Kim Phillips wrote: > On Mon, 21 Mar 2011 08:02:02 +0100 > Heiko Schocher 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) > > >> diff --git a/include/configs/suvd3.h b/include/configs/suvd3.h > >> +#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.... > > 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