Hi Marri, On Thursday 09 December 2010 02:12:07 tma...@apm.com wrote: > From: Tirumala Marri <tma...@apm.com> > > Adding Eiger board support for 460SX SoC.
Thanks. Some mostly nitpicking comments below. First typo in the subject: s/Eeigier/Eiger. <snip> > b/include/configs/eiger.h > new file mode 100644 > index 0000000..bc082a6 > --- /dev/null > +++ b/include/configs/eiger.h > @@ -0,0 +1,192 @@ > +/* > + * eiger.h - configuration for Eiger(460SX) Board. > + * > + * Copyright (c) 2010, Applied Micro Circuits Corporation > + * Author: Tirumala R Marri <tma...@apm.com> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > + * MA 02111-1307 USA > + */ > +#ifndef __CONFIG_H > +#define __CONFIG_H > + > +/* > + * High Level Configuration Options > + */ > +#define CONFIG_4xx 1 /* ... PPC4xx family */ > +#define CONFIG_440 1 /* ... PPC460 family */ > +#define CONFIG_460SX 1 /* ... PPC460 family */ > +#define CONFIG_BOARD_EARLY_INIT_F 1 /* Call board_pre_init */ > + > +#define CONFIG_SYS_TEXT_BASE 0xfffb0000 Please use space instead of tab after "#define". > + > +/* > + * Include common defines/options for all AMCC boards > + */ > +#define CONFIG_HOSTNAME eiger > + > +#include "amcc-common.h" > + > +#define CONFIG_SYS_CLK_FREQ 33333333 /* external freq to pll */ > + > +/* > + * Base addresses -- Note these are effective addresses where the > + * actual resources get mapped (not physical addresses) > + */ > +#define CONFIG_SYS_BOOT_BASE_ADDR 0xFF000000 > +#define CONFIG_SYS_FLASH_BASE 0xfff00000 /* start of FLASH */ Again, space after "#define". And use lower- or upper-case hex values consistently in this file. I personally prefer lower-case. > +#define CONFIG_SYS_ISRAM_BASE 0x90000000 /* internal SRAM */ > + > +#define CONFIG_SYS_PCI_BASE 0xd0000000 /* internal PCI regs */ > + > +#define CONFIG_SYS_PCIE_MEMBASE 0x90000000 /* mapped PCIe memory */ > +#define CONFIG_SYS_PCIE0_MEMBASE 0x90000000 /* mapped PCIe memory */ > +#define CONFIG_SYS_PCIE1_MEMBASE 0xa0000000 /* mapped PCIe memory */ > +#define CONFIG_SYS_PCIE_MEMSIZE 0x01000000 > + > +#define CONFIG_SYS_PCIE0_XCFGBASE 0xb0000000 > +#define CONFIG_SYS_PCIE1_XCFGBASE 0xb2000000 > +#define CONFIG_SYS_PCIE2_XCFGBASE 0xb4000000 > +#define CONFIG_SYS_PCIE0_CFGBASE 0xb6000000 > +#define CONFIG_SYS_PCIE1_CFGBASE 0xb8000000 > +#define CONFIG_SYS_PCIE2_CFGBASE 0xba000000 > + > +/* PCIe mapped UTL registers */ > +#define CONFIG_SYS_PCIE0_REGBASE 0xd0000000 > +#define CONFIG_SYS_PCIE1_REGBASE 0xd0010000 > +#define CONFIG_SYS_PCIE2_REGBASE 0xd0020000 > + > +/* System RAM mapped to PCI space */ > +#define CONFIG_PCI_SYS_MEM_BUS CONFIG_SYS_SDRAM_BASE > +#define CONFIG_PCI_SYS_MEM_PHYS CONFIG_SYS_SDRAM_BASE > +#define CONFIG_PCI_SYS_MEM_SIZE (1024 * 1024 * 1024) > + > +#define CONFIG_SYS_OPER_FLASH 0xe7000000 /* SRAM - OPER Flash */ What is this "OPER Flash"? Is it used at all in this code? If not you should better remove it. > +/* > + * Serial Port > + */ > +#define CONFIG_CONS_INDEX 1 /* Use UART0 */ > + > +/* > + * Initial RAM & stack pointer (placed in internal SRAM) > + */ > +#define CONFIG_SYS_TEMP_STACK_OCM 1 > +#define CONFIG_SYS_OCM_DATA_ADDR CONFIG_SYS_ISRAM_BASE > +#define CONFIG_SYS_INIT_RAM_ADDR CONFIG_SYS_ISRAM_BASE /* Initial RAM > address */ +#define CONFIG_SYS_INIT_RAM_SIZE 0x2000 /* Size of used area > in RAM */ + > +#define CONFIG_SYS_GBL_DATA_OFFSET (CONFIG_SYS_INIT_RAM_SIZE - > GENERATED_GBL_DATA_SIZE) +#define > CONFIG_SYS_INIT_SP_OFFSET (CONFIG_SYS_GBL_DATA_OFFSET - 0x4) + > +/* > + * DDR SDRAM > + */ > +#define CONFIG_SPD_EEPROM 1 /* Use SPD EEPROM for setup */ > +#define CONFIG_DDR_ECC 1 /* with ECC support */ > + > +#define CONFIG_SYS_SPD_MAX_DIMMS 2 > + > +/* SPD i2c spd addresses */ > +#define SPD_EEPROM_ADDRESS {IIC0_DIMM0_ADDR, IIC0_DIMM1_ADDR} Indentation with tabs please. And please add space after "{". Otherwise checkpatch will complain: #define SPD_EEPROM_ADDRESS { IIC0_DIMM0_ADDR, IIC0_DIMM1_ADDR } Thanks. 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: off...@denx.de _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot