[U-Boot] [PATCH 1/2] 8xx, kup4: minor configuration changes

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4C348537.4010101 at denx.de> you wrote:
> - nfs-options removed
> - hda->sda changed
> - mtd parts added
> - loadaddress changed
> - cmd-line length increased
> - lcd stuff removed
> - code cleanup
> 
> Signed-off-by: Klaus Heydeck <heydeck at kieback-peter.de>
...
> +	clrbits_be16 (&immap->im_ioport.iop_padat, PA_RS485);
> +	clrbits_be16 (&immap->im_ioport.iop_papar, PA_RS485);
> +	clrbits_be16 (&immap->im_ioport.iop_paodr, PA_RS485);
> +	setbits_be16 (&immap->im_ioport.iop_padir, PA_RS485);

No space after function names; please fix globally.

> -	if (immap->im_ioport.iop_pcdat & (PC_SWITCH1))
> +	clrbits_be16 (&immap->im_ioport.iop_pcpar, PC_SWITCH1);
> +	clrbits_be16 (&immap->im_ioport.iop_pcdir, PC_SWITCH1);
> +	if (in_be16 (&immap->im_ioport.iop_pcdat) & (PC_SWITCH1))

Please insert blank line above the "if" for readability.

>  		setenv ("key1", "off");
>  	else
>  		setenv ("key1", "on");
>  }
> +

Please do not add trailing empty lines.

> +#define PA_8	0x0080
> +#define PA_9	0x0040
> +#define PA_10	0x0020
> +#define PA_11	0x0010
> +#define PA_12	0x0008
> +
> +#define PB_14	0x00020000
> +#define PB_15	0x00010000
> +#define PB_16	0x00008000
> +#define PB_17	0x00004000
> +
> +#define PC_4	0x0800
> +#define PC_5	0x0400
> +#define PC_9	0x0040

It would make sense to add these defines (using a more generic name)
to include/mpc8xx.h (but note that I don't insist on this, as 8xx is
prettymuch dead).

> +extern void poweron_key(void);
>  extern void load_sernum_ethaddr(void);
> 
>  #endif	/* __KUP_H */
> +

No trailing blank lines, please fix globally!

...
> +	if (read_diag())
> +		gd->flags &= ~GD_FLG_SILENT;
> +		printf ("Board: KUP4K Rev %d.%d AK:",rev,mod);

Argh!! Either this is badly indented, or a bug.  Please fix!

> +	/*
> +	 * TI Application report: Before using the IO as an input,
> +	 * a high must be written to the IO first
> +	 */
> +	pcf = 0xFF;
> +	i2c_write (0x21, 0, 0 , &pcf, 1);
> +	if (i2c_read (0x21, 0, 0, &pcf, 1)) {
> +		puts ("n/a\n");
> +	}
> +	else {

Incorrect brace style - make "} else {"

...
> +	/* no refresh yet */
> +	if(rev >= 7)
> +		out_be32 (&memctl->memc_mamr,
> +				 CONFIG_SYS_MAMR_9COL & (~(MAMR_PTAE)));
> +	else
> +		out_be32 (&memctl->memc_mamr,
> +				 CONFIG_SYS_MAMR_8COL & (~(MAMR_PTAE)));

Braces needed for multiline statements.

>  	/* Configure PA8 as output port */
...
> +	setbits_be16 (&immap->im_ioport.iop_padir, 0x80);
> +	setbits_be16 (&immap->im_ioport.iop_paodr, 0x80);
> +	clrbits_be16 (&immap->im_ioport.iop_papar, 0x80);
> +	setbits_be16 (&immap->im_ioport.iop_padat, 0x80); /* turn it off */

Why don't you use the #defines you added above (PA_8) ?  Please check
and fix globally.


...
> +	out_be32 (&memctl->memc_or1, 0xFF000A00);
> +	out_be32 (&memctl->memc_br1, 0x00000081);
> +	out_be32 (&memctl->memc_or2, 0xFE000A00);
> +	out_be32 (&memctl->memc_br2, 0x01000081);
> +	out_be32 (&memctl->memc_or3, 0xFD000A00);
> +	out_be32 (&memctl->memc_br3, 0x02000081);
> +	out_be32 (&memctl->memc_or6, 0xFC000A00);
> +	out_be32 (&memctl->memc_br6, 0x03000081);
>  	udelay (10000);
> -
> -	return (size_b0 + size_b1 + size_b2 + size_b3);
> +	return (4 * 16 * 1024 * 1024);

Normally I insist on using get_ram_size() to auto-size and test the
RAM...


>  #define CONFIG_BOOTCOMMAND  \
> -    "run slot_a_boot;run slot_b_boot;run nfs_boot;run panic_boot"
> +    "run fat_boot; run slot_b_boot;run slot_a_boot;run nfs_boot;run panic_boot"

Are you absolutely sure you want this behaviour?  I would expect you
mean

	"run fat_boot slot_b_boot slot_a_boot nfs_boot panic_boot"

instead?


> +/*-----------------------------------------------------------------------
> + * Dynamic MTD partition support
> + */

Incorrect multiline comment style.


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
Sometimes, too long is too long.                          - Joe Crowe