[U-Boot] [PATCH 1/2]: Add support for the muas3001 board

[Wolfgang Denk]

Dear Heiko,

In message <489FDE9F.9020203 at denx.de> you wrote:
> Hello,
> 
> [POWERPC] Added support for the muas3001 board from IAI,
>            based on a MPC8270 CPU.

...
> +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT)
> +/*
> + * update "memory" property in the blob
> + */
> +void ft_blob_update(void *blob, bd_t *bd)
> +{
> +	int ret, nodeoffset = 0;
> +	ulong memory_data[2] = {0};
> +	ulong flash_data[4] = {0};
> +	ulong freq = 0;
> +	ulong	speed = 0;
> +
> +	memory_data[0] = cpu_to_be32(bd->bi_memstart);
> +	memory_data[1] = cpu_to_be32(bd->bi_memsize);
> +
> +	nodeoffset = fdt_path_offset (blob, "/memory");
> +	if (nodeoffset >= 0) {
> +		ret = fdt_setprop(blob, nodeoffset, "reg", memory_data,
> +					sizeof(memory_data));
> +	if (ret < 0)
> +		printf("ft_blob_update): cannot set /memory/reg "
> +			"property err:%s\n", fdt_strerror(ret));
> +	}
> +	else {

Please make this

	} else {

here and everywhere else.

> +	}
> +	else {
> +		/* memory node is required in dts */
> +		printf("ft_blob_update(): cannot find /localbus node "
> +		"err:%s\n", fdt_strerror(nodeoffset));

Indent the last line please.

> +	}
> +	/* MAC Adresse */
> +	nodeoffset = fdt_path_offset (blob, "/soc/cpm/ethernet");
> +	if (nodeoffset >= 0) {
> +		ret = fdt_setprop(blob, nodeoffset, "mac-address", bd->bi_enetaddr,
> +					sizeof(uchar) * 6);
> +	if (ret < 0)
> +		printf("ft_blob_update): cannot set /soc/cpm/ethernet/mac-address "
> +			"property err:%s\n", fdt_strerror(ret));
> +	}
> +	else {
> +		/* memory node is required in dts */
> +		printf("ft_blob_update(): cannot find /soc/cpm/ethernet node "
> +		"err:%s\n", fdt_strerror(nodeoffset));

Ditto.

> +
> +	/* brg clock */
> +	nodeoffset = fdt_path_offset (blob, "/soc/cpm/brg");
> +	if (nodeoffset >= 0) {
> +		freq = cpu_to_be32(bd->bi_brgfreq);
> +		ret = fdt_setprop(blob, nodeoffset, "clock-frequency", &freq,
> +					sizeof(unsigned long));
> +	if (ret < 0)
> +		printf("ft_blob_update): cannot set /soc/cpm/brg/clock-frequency "
> +			"property err:%s\n", fdt_strerror(ret));
> +	}
> +	else {
> +		/* memory node is required in dts */
> +		printf("ft_blob_update(): cannot find /soc/cpm/brg/clock-frequency node "
> +		"err:%s\n", fdt_strerror(nodeoffset));

Ditto.

> +
> +	/* baudrate */
> +	nodeoffset = fdt_path_offset (blob, "/soc/cpm/serial");
------------------------------------^^ Space
> +	if (nodeoffset >= 0) {
> +		speed = cpu_to_be32(bd->bi_baudrate);
----------------------------------^^ No Space

Your coding style is inconsistent - either do a space between function
name, or don't, but please do not mix styles.

> +#endif /* defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT) */
> diff --git a/include/configs/muas3001.h b/include/configs/muas3001.h
> new file mode 100644
> index 0000000..f915e55
> --- /dev/null
> +++ b/include/configs/muas3001.h
...
> +//# define CFG_CPMFCR_RAMTYPE	(CPMFCR_DTB | CPMFCR_BDB)

C++ Comments are not allowed.

> +#define CONFIG_EXTRA_ENV_SETTINGS						\
> +	"netdev=eth0\0"								\
> +	"u-boot_addr=100000\0"							\
> +	"kernel_addr=200000\0"							\
> +	"fdt_addr=400000\0"							\
> +	"rootpath=/opt/eldk-4.2-2008-04-01/ppc_82xx\0"				\

Does the code work only with this specific version of the ELDK? I
don't think so. I suggest to change this into /opt/eldk/ppc_6xx
instead.

> +	"u-boot=u-boot.bin\0"						\
> +	"bootfile=uImage\0"						\
> +	"fdt_file=muas3001.dtb\0"					\

Standard is to have a board directory, i. e. "muas3001/u-boot.bin"
etc.

> +	"kernel_file=uImage\0"					\

What's the difference between kernel_file and bootfile ?

> +	"ramdisk_file=uRamdisk\0"					\
> +	"load=tftp ${u-boot_addr} ${u-boot}\0"					\
> +	"update=prot off ff000000 ff03ffff; era ff000000 ff03ffff; "		\
> +		"cp.b ${u-boot_addr} ff000000 ${filesize};"			\
> +		"prot on ff000000 ff03ffff\0"					\
> +	"ramargs=setenv bootargs root=/dev/ram rw\0"				\
> +	"nfsargs=setenv bootargs root=/dev/nfs rw "				\
> +		"nfsroot=${serverip}:${rootpath}\0"				\
> +	"addcons=setenv bootargs ${bootargs} console=ttyCPM0,${baudrate}\0"	\
> +	"addmtd=setenv bootargs ${bootargs} ${mtdparts}\0"			\
> +	"addip=setenv bootargs ${bootargs} "					\
> +		"ip=${ipaddr}:${serverip}:${gatewayip}:"			\
> +		"${netmask}:${hostname}:${netdev}:off panic=1\0"		\
> +	"net_nfs=tftp ${kernel_addr} ${bootfile}; "				\
> +		"tftp ${fdt_addr} ${fdt_file}; run nfsargs addip addcons;"	\
> +		"bootm ${kernel_addr} - ${fdt_addr}\0"				\
> +	"net_self=tftp ${kernel_addr} ${bootfile}; "				\
> +		"tftp ${fdt_addr} ${fdt_file}; "				\
> +		"tftp ${ramdisk_addr} ${ramdisk_file}; "			\
> +		"run ramargs addip; "						\
> +		"bootm ${kernel_addr} ${ramdisk_addr} ${fdt_addr}\0"		\
> +	"ramdisk_addr=ff210000\0"						\
> +	"kernel_addr_self=ff050000\0"						\
> +	"fdt_addr_self=ff200000\0"						\
> +	"flash_self=run ramargs addip addcons;bootm ${kernel_addr_self}"	\
> +	" ${ramdisk_addr} ${fdt_addr_self}\0"					\

"Standard" names are "kernel_addr", "ramdisk_addr" and "fdt_addr"  in
flash, and "kernel_addr_r", "ramdisk_addr_r" and "fdt_addr_r" in RAM.
See for example TQM5200.

> +	"updateramdisk=era ${ramdisk_addr} +1f0000;tftpb ${kernel_addr} ${ramdisk_file};"	\
> +	"cp.b ${kernel_addr} ${ramdisk_addr} ${filesize}\0"					\
> +	"updatekernel=era ${kernel_addr_self} +1b0000;tftpb ${kernel_addr} ${kernel_file};"	\
> +	"cp.b ${kernel_addr} ${kernel_addr_self} ${filesize}\0"					\
> +	"updatefdt=era ${fdt_addr_self} +10000;tftpb ${fdt_addr} ${fdt_file};"		\
> +	"cp.b ${fdt_addr} ${fdt_addr_self} ${filesize}\0"					\
> +	""

Maximum line length exceeded.


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
As far as the laws of mathematics refer to reality, they are not cer-
tain, and as far as they are certain, they do not refer  to  reality.
                                                   -- Albert Einstein