On 23/11/2011 17:21, Wolfgang Denk wrote:
+#define CONFIG_CMD_I2C /* I2C serial bus support */ +#define CONFIG_CMD_MMC /* MMC support */ +#define CONFIG_CMD_FAT /* FAT support */ +#define CONFIG_CMD_NAND /* NAND support */ +#define CONFIG_CMD_USB +#define CONFIG_CMD_DHCP +#define CONFIG_CMD_PING +#define CONFIG_CMD_CACHE +#define CONFIG_CMD_GPIO
Maybe you want to sort this list. And eventually remove entries that are defined by default?
I will do it
+#undef CONFIG_CMD_FLASH /* flinfo, erase, protect */ +#undef CONFIG_CMD_IMI /* iminfo */ +#undef CONFIG_CMD_IMLS /* List all found images */
Is there any good reason to disable the "iminfo" command?
Yes - the command relies on NOR flash. In fact, in cmd_bootm.c do_imls() needs CONFIG_SYS_MAX_FLASH_BANKS, that has no sense on this SOM, because I have not CFI flash.
Also defining CONFIG_SYS_MAX_FLASH_BANKS does not work, because cfi.h is still included. I prefer disabling IMLS as adding fake CFI values.
+#define CONFIG_SYS_NAND_ADDR NAND_BASE /* physical address */
/* to access nand */+#define CONFIG_SYS_NAND_BASE NAND_BASE /* physical address */
Do we really need this double definitions? Please get rid of NAND_BASE.
NAND_BASE is the address of the controller defined in cpu.h. That iis correct.
CONFIG_SYS_NAND_ADDR seems to me obsolete and not used anymore - a lot of boards define it, I think it should be globally removed - I start dropping from here.
...
+/* memtest works on */ +#define CONFIG_SYS_MEMTEST_START (OMAP34XX_SDRC_CS0) +#define CONFIG_SYS_MEMTEST_END (OMAP34XX_SDRC_CS0 + \
0x01F00000) /* 31MB */Has this been tested?
Yes - the only issue here is that only 31 MB (the SOM has 256 MB RAM) are tested as default. Personally I do not like to set as default value the whole RAM (subtracting the size of the bootloader code, of course !), because the test takes too much time for a fast run. The user can always provide parameters for the mtest command if he wants to test the whole RAM.
+/*-----------------------------------------------------------------------
- Stack sizes
- The stack sizes are set up in start.S using the settings below
- */
Incorrect multiline comment style. Please fix globally.
Thanks, I will fix it.
+#define CONFIG_ENV_IS_IN_NAND +#define SMNAND_ENV_OFFSET 0x180000 /* environment starts here */
+#define CONFIG_SYS_ENV_SECT_SIZE (128 << 10) /* 128 KiB */ +#define CONFIG_ENV_OFFSET SMNAND_ENV_OFFSET +#define CONFIG_ENV_ADDR SMNAND_ENV_OFFSET
Please extend to support redundant environment, plus one or two reserve sectors in case a sector goes bad.
Good point, I will do it.
+/*-----------------------------------------------------------------------
- CFI FLASH driver setup
- */
+/* timeout values are in ticks */ +#define CONFIG_SYS_FLASH_ERASE_TOUT (100 * CONFIG_SYS_HZ) +#define CONFIG_SYS_FLASH_WRITE_TOUT (100 * CONFIG_SYS_HZ)
This seems bogus, as there is no NOR flash on these devices.
This is completely wrong, bad cut&paste ;-(. I will fix it.
+/* Flash banks JFFS2 should use */ +#define CONFIG_SYS_MAX_MTD_BANKS (CONFIG_SYS_MAX_FLASH_BANKS + \
CONFIG_SYS_MAX_NAND_DEVICE)+#define CONFIG_SYS_JFFS2_MEM_NAND +/* use flash_info[2] */ +#define CONFIG_SYS_JFFS2_FIRST_BANK CONFIG_SYS_MAX_FLASH_BANKS +#define CONFIG_SYS_JFFS2_NUM_BANKS 1
All this probably does not work.
Confirmed, it does not work and I will clean up.
+#define CONFIG_SPL_MAX_SIZE 0xB400 /* 45 K */
(45 << 10) would be way easier to read...
ok
Best regards, Stefano Babic