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 -- ===================================================================== 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: [email protected] ===================================================================== _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
