[U-Boot] [PATCH 1/1] MX5:MX53: support for freescale MX53LOCO board

Wolfgang Denk wd at denx.de
Wed Apr 13 22:37:08 CEST 2011


Dear Jason Liu,

In message <1298283316-3443-1-git-send-email-r64343 at freescale.com> you wrote:
> This patch add initial support for freescale MX53LOCO board.
> Network(FEC),SD/MMC, UART have been supported by this patch.
> 
> Signed-off-by: Jason Liu <r64343 at freescale.com>
...
> +int dram_init(void)
> +{
> +	gd->ram_size = get_ram_size((volatile void *)CONFIG_SYS_SDRAM_BASE,
> +				PHYS_SDRAM_1_SIZE);
> +
> +	return 0;
> +}
> +void dram_init_banksize(void)
> +{
> +	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> +	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> +
> +	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
> +	gd->bd->bi_dram[1].size = PHYS_SDRAM_2_SIZE;
> +}

This is apparently wrong: in dram_init() you run get_ram_size() only
over the first bank of memory, but dram_init_banksize() says you have
two of them ??


> +int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +{
> +	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> +
> +	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
> +		*cd = mxc_gpio_get(77); /*GPIO3_13*/
> +	else
> +		*cd = mxc_gpio_get(75); /*GPIO3_11*/

Is this just data or a device?  If the latter should be the case you
mustuse I/O accessors instead.

...
> +	puts("Board: MX53LOCO [");
> +
> +	cause = src_regs->srsr;
> +	switch (cause) {
> +	case 0x0001:
> +		printf("POR");
> +		break;
> +	case 0x0009:
> +		printf("RST");
> +		break;
> +	case 0x0010:
> +	case 0x0011:
> +		printf("WDOG");
> +		break;
> +	default:
> +		printf("unknown");
> +	}

This has been discussed before: this code should be factored out into
common code.

...
> +#define CONFIG_CMDLINE_TAG		1	/* enable passing of ATAGs */
> +#define CONFIG_REVISION_TAG		1
> +#define CONFIG_SETUP_MEMORY_TAGS	1
> +#define CONFIG_INITRD_TAG		1

Please remove the '1' from all #defines that just enable features,
i.  e. where no specific numeric value is needed.


> +#define CONFIG_SYS_MEMTEST_START       0x70000000
> +#define CONFIG_SYS_MEMTEST_END         0x10000

Has this actually been tested?

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: wd at denx.de
People seldom know what they want until you give them what  they  ask
for.


More information about the U-Boot mailing list