Dear Peter Barada,
In message 1324067511-14142-1-git-send-email-peter.barada@logicpd.com you wrote:
This patch adds basic support for OMAP35x/DM37x SOM LV/Torpedo reference boards. It assumes U-boot is loaded to SDRAM with the help of another small bootloader (x-load) running from SRAM.
Signed-off-by: Peter Barada peter.barada@logicpd.com Cc: Tom Rini tom.rini@gmail.com Cc: Igor Grinberg grinberg@compulab.co.il
...
board/logicpd/omap3som/Makefile | 42 +++ board/logicpd/omap3som/omap3logic.c | 523 +++++++++++++++++++++++++++++++++++ board/logicpd/omap3som/omap3logic.h | 35 +++ boards.cfg | 1 + include/configs/omap3_logic.h | 351 +++++++++++++++++++++++ 5 files changed, 952 insertions(+), 0 deletions(-) create mode 100644 board/logicpd/omap3som/Makefile create mode 100644 board/logicpd/omap3som/omap3logic.c create mode 100644 board/logicpd/omap3som/omap3logic.h create mode 100644 include/configs/omap3_logic.h
Entry to MAINTAINERS missing.
+/* two dimensional array of strucures containining board name and Linux
- machine IDs; row it selected based on CPU column is slected based
- on hsusb0_data5 pin having a pulldown resistor */
Incorrect multiline comment style. Please fix globally.
+#ifdef CONFIG_SMC911X +/* GPMC CS1 settings for Logic SOM LV/Torpedo LAN92xx Ethernet chip */ +static const u32 gpmc_lan92xx_config[] = {
- 0x00001000,
- 0x00080801,
- 0x00000000,
- 0x08010801,
- 0x00080a0a,
- 0x03000280,
+};
And what exactly is the meaning of these magic numbers?
+#define CONFIG_HARD_I2C 1 +#define CONFIG_SYS_I2C_SPEED 100000 +#define CONFIG_SYS_I2C_SLAVE 1 +#define CONFIG_SYS_I2C_BUS 0 +#define CONFIG_SYS_I2C_BUS_SELECT 1 +#define CONFIG_I2C_MULTI_BUS 1 +#define CONFIG_DRIVER_OMAP34XX_I2C 1
Please do not define values for macros that only switch on a feature.
...
+#define CONFIG_PREBOOT \
- "echo ======================NOTICE============================;"\
- "echo The u-boot environment is not set. - You are;" \
- "echo required to set a valid display for your LCD panel.;" \
- "echo Valid display options are:;" \
- "echo " 2 == LQ121S1DG31 TFT SVGA (12.1) Sharp";" \
- "echo " 3 == LQ036Q1DA01 TFT QVGA (3.6) Sharp w/ASIC";" \
- "echo " 5 == LQ064D343 TFT VGA (6.4) Sharp";" \
- "echo " 7 == LQ10D368 TFT VGA (10.4) Sharp";" \
- "echo " 15 == LQ043T1DG01 TFT WQVGA (4.3) Sharp (DEFAULT)";" \
- "echo " vga[-16 OR -24] LCD VGA 640x480";" \
- "echo " svga[-16 OR -24] LCD SVGA 800x600";" \
- "echo " xga[-16 OR -24] LCD XGA 1024x768";" \
- "echo " 720p[-16 OR -24] LCD 720P 1280x720";" \
- "echo " sxga[-16 OR -24] LCD SXGA 1280x1024";" \
- "echo " uxga[-16 OR -24] LCD UXGA 1600x1200";" \
- "echo MAKE SURE YOUR DISPLAY VARIABLE IS CORRECTLY ENTERED!;" \
- "setenv display 15;" \
Strange. First you ask the user to set a valid display type, and then you hard-set it to some predefined value. What's the reationale behind that?
- "setenv preboot;" \
- "saveenv;"
Would it not be better to erase the preboot setting only after some display type definition was entered and saved _by_the_user_ ?
+/*-----------------------------------------------------------------------
- Physical Memory Map
- */
Incorrect multiline comment style. Please fix globally.
Best regards,
Wolfgang Denk