
Dear Aneesh V,
In message 1274769577-29021-3-git-send-email-aneesh@ti.com you wrote:
Adding support for OMAP4430 SDP board based on the TI OMAP4430 SOC.
TI OMAP4430 is an SOC based on ARM Cortex-A9 cpu. OMAP4430 SDP is a reference board based on OMAP4430.
This patch adds minimum support for booting the board. OMAP4430 SDP does not support XIP. U-boot is loaded to SDRAM with the help of another small bootloader running from SRAM. U-boot currently relies on this bootloader for clock, mux, and SDRAM initialization.
This will change when OMAP4430 Configuration Header(CH) is attached to u-boot. CH is a feature by which ROM code can be made to intialize the clocks and SDRAM and to copy u-boot directly into SDRAM from a non-XIP device.
More support such as MMC, ethernet etc will be added in subsequent patches.
Signed-off-by: Aneesh V aneesh@ti.com
In addition to John's comments:
Makefile | 8 +- arch/arm/cpu/armv7/omap4/Makefile | 50 +++++ arch/arm/cpu/armv7/omap4/cache.S | 128 ++++++++++++ arch/arm/cpu/armv7/omap4/lowlevel_init.S | 49 +++++ arch/arm/cpu/armv7/omap4/omap4.c | 97 +++++++++ arch/arm/cpu/armv7/omap4/reset.S | 36 ++++ arch/arm/cpu/armv7/omap4/sys_info.c | 58 ++++++ arch/arm/cpu/armv7/omap4/timer.c | 140 +++++++++++++ arch/arm/include/asm/arch-omap4/cpu.h | 89 +++++++++ arch/arm/include/asm/arch-omap4/omap4.h | 142 ++++++++++++++ arch/arm/include/asm/arch-omap4/sys_proto.h | 36 ++++ board/ti/sdp4430/Makefile | 49 +++++ board/ti/sdp4430/config.mk | 32 +++ board/ti/sdp4430/sdp.c | 56 ++++++ include/configs/omap4_sdp4430.h | 280 +++++++++++++++++++++++++++ 15 files changed, 1249 insertions(+), 1 deletions(-) create mode 100644 arch/arm/cpu/armv7/omap4/Makefile create mode 100644 arch/arm/cpu/armv7/omap4/cache.S create mode 100644 arch/arm/cpu/armv7/omap4/lowlevel_init.S create mode 100644 arch/arm/cpu/armv7/omap4/omap4.c create mode 100644 arch/arm/cpu/armv7/omap4/reset.S create mode 100644 arch/arm/cpu/armv7/omap4/sys_info.c create mode 100644 arch/arm/cpu/armv7/omap4/timer.c create mode 100644 arch/arm/include/asm/arch-omap4/cpu.h create mode 100644 arch/arm/include/asm/arch-omap4/omap4.h create mode 100644 arch/arm/include/asm/arch-omap4/sys_proto.h create mode 100644 board/ti/sdp4430/Makefile create mode 100644 board/ti/sdp4430/config.mk create mode 100644 board/ti/sdp4430/sdp.c create mode 100644 include/configs/omap4_sdp4430.h
Entries to MAKEALL and MAINTAINERS files missing.
+int checkboard (void) +{
- printf ("%s\n", sysinfo.board_string);
- return 0;
+}
Consider using puts() when you don't need any output formatting.
+#endif /* CONFIG_DISPLAY_BOARDINFO */
...
+#ifdef CONFIG_DISPLAY_CPUINFO
These #defines have never been documented. It seems they are being copied around a lot, but I'm not sure if anybody ever does NOT set these.
I think we should at least document these - or rather drop them everywhere. What do you think?
+{
- printf ("CPU : OMAP4430 ES1.0 \n");
puts()?
And: no spaces after function names please (fix globally!).
...
+/*
- High Level Configuration Options
- */
+#define CONFIG_ARMCORTEXA9 1 /* This is an ARM V7 CPU core */ +#define CONFIG_OMAP 1 /* in a TI OMAP core */ +#define CONFIG_OMAP44XX 1 /* which is a 44XX */ +#define CONFIG_OMAP4430 1 /* which is in a 4430 */ +#define CONFIG_4430SDP 1 /* working with SDP */
/*#define CONFIG_FASTBOOT 1*//* Using fastboot interface */
Line too long - please check (and fix) globally.
And please don't add dead code like this.
+#if 0 +/* Enabled commands */ +#define CONFIG_CMD_DHCP /* DHCP Support */ +#define CONFIG_CMD_EXT2 /* EXT2 Support */ +#define CONFIG_CMD_FAT /* FAT support */ +#define CONFIG_CMD_I2C /* I2C serial bus support */ +#define CONFIG_CMD_MMC /* MMC support */ +#endif
... or like this.
...
+/* SDRAM Test range - start at 16 meg boundary -ends at 1Meg -
- a basic sanity check ONLY
- IF you would like to increase coverage, increase the end address
- or run the test with custom options
- */
Incorrect multiline comment style - in addition, the comment ("start at 16 meg boundary -ends at 1Meg") makes no sense to me.
+#define CONFIG_SYS_MEMTEST_START 0x80000000 +#define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + (1 << 20))
Why would you want to test only such a small fration of your system RAM?
+/*
- Although we don't have
- CFI FLASH driver setup
- NOR boot support - single 1 Gbit PF48F6000M0 Strataflash
- */
Can't parse this!
+#if 0
No dead code, please.
Best regards,
Wolfgang Denk