Hi Wolfgang, On Wed, Jan 26, 2011 at 5:32 AM, Wolfgang Denk <w...@denx.de> wrote: > Dear Lei Wen, > > In message <1294632087-8025-3-git-send-email-lei...@marvell.com> you wrote: >> Pantheon Family processors are highly integrated SoCs >> based on Sheeva_88SV331x-v5 PJ1 cpu core. >> Ref: >> http://www.marvell.com/products/processors/communications/marvell_pantheon_910_920_pb.pdf >> >> SoC versions Supported: >> 1) PANTHEON920 (TD) >> 2) PANTHEON910 (TTC) >> >> Signed-off-by: Lei Wen <lei...@marvell.com> > ... >> +int dram_init(void) >> +{ > ... >> + for (; i < CONFIG_NR_DRAM_BANKS; i++) { >> + /* If above loop terminated prematurely, we need to set >> + * remaining banks' start address & size as 0. Otherwise other >> + * u-boot functions and Linux kernel gets wrong values which >> + * could result in crash */ > > Incorrect multiline comment style. >
This already fix in the v6 patch... http://patchwork.ozlabs.org/patch/80305/ >> +/* For preventing risk of instability in reading counter value, >> + * first set read request to register cvwr and then read same >> + * register after it captures counter value. >> + */ > > Ditto. Please fix globally. > >> + writel(COUNT_RD_REQ, &panthtimers->cvwr); >> + while (loop--); > > Please write: > > while (loop--) > ; Fixed... > > But then - are you sure the compiler does not optimize this out? You > probably want to use __udelay() instead. From the practice, we think this loop is enough to make timer stablize... Involve the __udelay() may not suitable for the timer functions... > > ... >> --- /dev/null >> +++ b/arch/arm/include/asm/arch-pantheon/config.h >> @@ -0,0 +1,44 @@ > ... >> +/* >> + * There is no internal RAM in ARMADA100, using DRAM >> + * TBD: dcache to be used for this >> + */ >> +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_TEXT_BASE - >> 0x00200000) >> +#define CONFIG_NR_DRAM_BANKS_MAX 2 > > This looks like board specific code that should not be here. Yep... I would update the patch for it. For V7... > > ... >> +struct panthmpmu_registers { >> + u8 pad0[0x0024]; >> + u32 ccgr; /*0x0024*/ >> + u8 pad1[0x0200 - 0x024 - 4]; >> + u32 wdtpcr; /*0x0200*/ >> + u8 pad2[0x1020 - 0x200 - 4]; >> + u32 aprr; /*0x1020*/ >> + u32 acgr; /*0x1024*/ >> +}; > > Please use TAB for vertical alignment of variable names. Please fix > globally. In V6 patch , I think I already do like using tab. :) Best regards, Lei _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
