Hi Marri,
On Thursday 09 December 2010 02:12:07 tmarri@apm.com wrote:
From: Tirumala Marri tmarri@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 tmarri@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: office@denx.de