[U-Boot] [PATCH] mpc512x: Add esd gmbh mecp5123 board support

Wolfgang Denk wd at denx.de
Mon Jun 8 12:38:18 CEST 2009


Dear Stefan Roese,

In message <1244447665-14264-1-git-send-email-sr at denx.de> you wrote:
> MECP5123 is a MPC5121E based module by esd gmbh.
> 
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Reinhard Arlt <reinhard.arlt at esd-electronics.com>

...
> +int eeprom_write_enable(unsigned dev_addr, int state)
> +{
> +	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
> +
> +        if (dev_addr != CONFIG_SYS_I2C_EEPROM_ADDR)
> +	        return -1;
> +
> +        if (state == 0)
> +		setbits_be32(&im->gpio.gpdat, 0x00100000);
> +	else
> +		clrbits_be32(&im->gpio.gpdat, 0x00100000);
> +
> +        return 0;
> +}

Please fix indentation - indentation by TAB only, please.

...
> +int board_early_init_f(void)
> +{
...
> +	/* Initialize DDR Priority Manager */
> +	out_be32(&im->mddrc.prioman_config1, CONFIG_SYS_MDDRCGRP_PM_CFG1);
> +	out_be32(&im->mddrc.prioman_config2, CONFIG_SYS_MDDRCGRP_PM_CFG2);
> +	out_be32(&im->mddrc.hiprio_config, CONFIG_SYS_MDDRCGRP_HIPRIO_CFG);
> +	out_be32(&im->mddrc.lut_table0_main_upper, CONFIG_SYS_MDDRCGRP_LUT0_MU);
> +	out_be32(&im->mddrc.lut_table0_main_lower, CONFIG_SYS_MDDRCGRP_LUT0_ML);
> +	out_be32(&im->mddrc.lut_table1_main_upper, CONFIG_SYS_MDDRCGRP_LUT1_MU);
> +	out_be32(&im->mddrc.lut_table1_main_lower, CONFIG_SYS_MDDRCGRP_LUT1_ML);
> +	out_be32(&im->mddrc.lut_table2_main_upper, CONFIG_SYS_MDDRCGRP_LUT2_MU);
> +	out_be32(&im->mddrc.lut_table2_main_lower, CONFIG_SYS_MDDRCGRP_LUT2_ML);
> +	out_be32(&im->mddrc.lut_table3_main_upper, CONFIG_SYS_MDDRCGRP_LUT3_MU);
> +	out_be32(&im->mddrc.lut_table3_main_lower, CONFIG_SYS_MDDRCGRP_LUT3_ML);
> +	out_be32(&im->mddrc.lut_table4_main_upper, CONFIG_SYS_MDDRCGRP_LUT4_MU);
> +	out_be32(&im->mddrc.lut_table4_main_lower, CONFIG_SYS_MDDRCGRP_LUT4_ML);
> +	out_be32(&im->mddrc.lut_table0_alternate_upper, CONFIG_SYS_MDDRCGRP_LUT0_AU);
> +	out_be32(&im->mddrc.lut_table0_alternate_lower, CONFIG_SYS_MDDRCGRP_LUT0_AL);
> +	out_be32(&im->mddrc.lut_table1_alternate_upper, CONFIG_SYS_MDDRCGRP_LUT1_AU);
> +	out_be32(&im->mddrc.lut_table1_alternate_lower, CONFIG_SYS_MDDRCGRP_LUT1_AL);
> +	out_be32(&im->mddrc.lut_table2_alternate_upper, CONFIG_SYS_MDDRCGRP_LUT2_AU);
> +	out_be32(&im->mddrc.lut_table2_alternate_lower, CONFIG_SYS_MDDRCGRP_LUT2_AL);
> +	out_be32(&im->mddrc.lut_table3_alternate_upper, CONFIG_SYS_MDDRCGRP_LUT3_AU);
> +	out_be32(&im->mddrc.lut_table3_alternate_lower, CONFIG_SYS_MDDRCGRP_LUT3_AL);
> +	out_be32(&im->mddrc.lut_table4_alternate_upper, CONFIG_SYS_MDDRCGRP_LUT4_AU);
> +	out_be32(&im->mddrc.lut_table4_alternate_lower, CONFIG_SYS_MDDRCGRP_LUT4_AL);

Lines too long.

> +phys_size_t initdram(int board_type)
> +{
> +	return fixed_sdram();
> +}

Use auto-sizing?

> +int misc_init_r(void)
> +{
> +	volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR;
> +
> +	clrbits_be32(&im->clk.scfr[0], 0x00003800);
> +	setbits_be32(&im->clk.scfr[0], 0x01 << 11);
> +	clrbits_be32(&im->lpc.cs_dccr, 0x000000ff);
> +	setbits_be32(&im->lpc.cs_dccr, (0x00 << 4) | (0x01 << 0));
> +	clrbits_be32(&im->lpc.cs_hccr, 0x000000ff);
> +	setbits_be32(&im->lpc.cs_hccr, (0x00 << 4) | (0x01 << 0));

Please add some comments what you are doing here, and eventually
#define the magic constants used.

...
> --- /dev/null
> +++ b/include/configs/mecp5123.h
> @@ -0,0 +1,455 @@
> +/*
> + * (C) Copyright 2007, 2008 DENX Software Engineering

2009?

...
> +/*
> + * NOR FLASH on the Local Bus
> + */
> +#define CONFIG_SYS_FLASH_CFI				/* use the Common Flash Interface */
> +#define CONFIG_FLASH_CFI_DRIVER				/* use the CFI driver */
> +
> +#define CONFIG_SYS_FLASH_BASE		0xFFC00000	/* start of FLASH   */
> +#define CONFIG_SYS_FLASH_SIZE		0x00400000	/* max flash size in bytes */

Lines too long (check everywhere!)

...
> +#define	CONFIG_EXTRA_ENV_SETTINGS					\
> +	"u-boot_addr_r=200000\0"					\
> +	"kernel_addr_r=600000\0"					\
> +	"fdt_addr_r=880000\0"						\
> +	"ramdisk_addr_r=900000\0"					\
> +	"u-boot_addr=FFF00000\0"					\
> +	"kernel_addr=FFC40000\0"					\
> +	"fdt_addr=FFEC0000\0"						\
> +	"ramdisk_addr=FC040000\0"					\
> +	"ramdiskfile=/tftpboot/mecp512x/uRamdisk\0"			\
> +	"u-boot=/tftpboot/mecp512x/u-boot.bin\0"			\
> +	"bootfile=/tftpboot/mecp512x/uImage\0"				\
> +	"fdtfile=/tftpboot/mecp512x/mecp512x.dtb\0"			\
> +	"rootpath=/tftpboot/mecp512x/target_root\n"	      		\
> +	"netdev=eth0\0"							\
> +	"consdev=ttyPSC0\0"						\
> +	"nfsargs=setenv bootargs root=/dev/nfs rw "			\
> +		"nfsroot=${serverip}:${rootpath}\0"			\
> +	"ramargs=setenv bootargs root=/dev/ram rw\0"			\
> +	"addip=setenv bootargs ${bootargs} "				\
> +		"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}"	\
> +		":${hostname}:${netdev}:off panic=1\0"			\
> +	"addtty=setenv bootargs ${bootargs} "				\
> +		"console=${consdev},${baudrate}\0"			\
> +	"flash_nfs=run nfsargs addip addtty;"				\
> +		"bootm ${kernel_addr} - ${fdt_addr}\0"			\
> +	"flash_self=run ramargs addip addtty;"				\
> +		"bootm ${kernel_addr} ${ramdisk_addr} ${fdt_addr}\0"	\
> +	"net_nfs=tftp ${kernel_addr_r} ${bootfile};"			\
> +		"tftp ${fdt_addr_r} ${fdtfile};"			\
> +		"run nfsargs addip addtty;"				\
> +		"bootm ${kernel_addr_r} - ${fdt_addr_r}\0"		\
> +	"net_self=tftp ${kernel_addr_r} ${bootfile};"			\
> +		"tftp ${ramdisk_addr_r} ${ramdiskfile};"		\
> +		"tftp ${fdt_addr_r} ${fdtfile};"			\
> +		"run ramargs addip addtty;"				\
> +		"bootm ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}\0"\
> +	"load=tftp ${u-boot_addr_r} ${u-boot}\0"			\
> +	"update=protect off ${u-boot_addr} +${filesize};"		\
> +		"era ${u-boot_addr} +${filesize};"			\
> +		"cp.b ${u-boot_addr_r} ${u-boot_addr} ${filesize}\0"	\
> +	"upd=run load update\0"						\
> +	""

How long does it to boot with such an environment in EEPROM?

> +#define CONFIG_OF_LIBFDT	1
> +#define CONFIG_OF_BOARD_SETUP	1
> +#define CONFIG_OF_SUPPORT_OLD_DEVICE_TREES	1

Do you really need this on a new board?


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
"I'm growing older, but not up."                      - Jimmy Buffett


More information about the U-Boot mailing list