Dear Stefano Babic,
In message <1322040416-11751-2-git-send-email-sba...@denx.de> you wrote:
> The TAM3517 is a SOM module that can be used on custom boards.
> The patch add a common configuration file that is included
> by the boards using this module.
>
> Signed-off-by: Stefano Babic <sba...@denx.de>
> CC: Tapani Utrianen <tap...@technexion.com>
> CC: Tom Rini <tom.r...@gmail.com>
> CC: Sandeep Paulraj <s-paul...@ti.com>
> ---
> include/configs/tam3517-common.h | 334
> ++++++++++++++++++++++++++++++++++++++
> 1 files changed, 334 insertions(+), 0 deletions(-)
> create mode 100644 include/configs/tam3517-common.h
...
> +#define CONFIG_CMD_EXT2 /* EXT2 Support */
> +#define CONFIG_CMD_FAT /* FAT support */
> +#define CONFIG_CMD_JFFS2 /* JFFS2 Support */
> +
> +#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?
> +#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?
> +#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.
...
> +/* 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?
> +#define CONFIG_SYS_LOAD_ADDR (OMAP34XX_SDRC_CS0) /* default load */
> + /* address */
Don't we have any exception vectors or similar in low memory?
> +/*-----------------------------------------------------------------------
> + * Stack sizes
> + *
> + * The stack sizes are set up in start.S using the settings below
> + */
Incorrect multiline comment style. Please fix globally.
> +/* Configure the PISMO */
> +#define PISMO1_NAND_SIZE GPMC_SIZE_128M
Don't we auto-detect the size?
> +#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.
> +/*-----------------------------------------------------------------------
> + * 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.
> +/* 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.
> +/*
> + * ethernet support
> + *
Please delete unneeded empty lines like this one.
...
> +#define CONFIG_SPL_NAND_WORKSPACE 0x8f07f000 /* below BSS */
Urghhh... How do you guarantee this really works, no matter how the
image gets linked?
> +#define CONFIG_SPL_MAX_SIZE 0xB400 /* 45 K */
(45 << 10) would be way easier to read...
> +#define CONFIG_SPL_BSS_START_ADDR 0x8f080000 /* end of RAM */
Don't we auto-detect the RAM size?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
You can't evaluate a man by logic alone.
-- McCoy, "I, Mudd", stardate 4513.3
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
