From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gary Jennejohn Date: Sat, 26 Apr 2008 12:18:02 +0200 Subject: [U-Boot-Users] [PATCH V2] Add the Harris QUAD100HD AMCC 405EP-based board. In-Reply-To: <200804260822.02142.sr@denx.de> References: <20080425180412.1eee9b27@peedub.jennejohn.org> <200804260822.02142.sr@denx.de> Message-ID: <20080426121802.2c80657e@peedub.jennejohn.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sat, 26 Apr 2008 08:22:02 +0200 Stefan Roese wrote: Hi Stefan, > Looks much better now. Just some minor issues (nitpicking) left. See comments > below. > > > + /* taken from PPCBoot */ > > + mtdcr(uicsr, 0xFFFFFFFF); /* clear all ints */ > > + mtdcr(uicer, 0x00000000); /* disable all ints */ > > + mtdcr(uiccr, 0x00000000); > > + mtdcr(uicpr, 0xFFFF7FFE); /* set int polarities */ > > + mtdcr(uictr, 0x00000000); /* set int trigger levels */ > > + mtdcr(uicsr, 0xFFFFFFFF); /* clear all ints */ > > + mtdcr(uicvcr, 0x00000001); /* set vect base=0,INT0 highest priority */ > > + > > + mtdcr (CPC0_SRR, 0x00040000); /* Hold PCI bridge in reset */ > > Please use only one coding style in one file. So either: > > func(); > > or > > func (); > > Since this file mostly consists of the func() style I suggest you switch to > this one completely. > Ah yes. I copied parts of this from PPCBoot and failed to notice the different formatting in the various places. Thanks! > > +#define CFG_MONITOR_LEN (256 * 1024) /* Reserve 256 kB for Monitor */ > > +#define CFG_MALLOC_LEN (128 * 1024) /* Reserve 128 kB for malloc() */ > > +#define CFG_MONITOR_BASE 0xFFF80000 > > CFG_MONITOR_BASE at 0xfff80000 makes me think that the U-Boot image has a size > of 512k. But CFG_MONITOR_LEN is 256k. Which one is correct? I would think > that 256k should be enough for this image. > That's a valid point. Actually u-boot.bin is 512kB in length, but only because there's a reset vector at 0xFFFFFFFC. u-boot itself is really only 256kB in size. So now I wonder - what's correct? If CFG_MONITOR_LEN applies to RAM, which it seems to since it only really affects TOTAL_MALLOC_LEN, then 256kB is correct and no change is required. > Starting here > > > +#define CFG_FLASH_WORD_SIZE unsigned short > > +#define CFG_FLASH_ADDR0 0x0555 > > +#define CFG_FLASH_ADDR1 0x02aa > > +#define FLASH_BASE0_PRELIM CFG_FLASH_BASE /* FLASH bank #0 */ > > till here. These defines are most likely not needed for the CFI driver. Please > remove them. > OK, I'll look into it. --- Gary Jennejohn ********************************************************************* 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 *********************************************************************