[U-Boot] [PATCH] ppc4xx: Add GDsys CompactCenter board support.

[Wolfgang Denk]

Dear Dirk Eibach,

In message <1247226686-20065-1-git-send-email-eibach at gdsys.de> you wrote:
> Board support for the Guntermann & Drunck CompactCenter and
> DevCon-Center.
> Based on the AMCC Canyonlands board support by Stefan Roese.
> 
> Signed-off-by: Dirk Eibach <eibach at gdsys.de>
...
> diff --git a/board/gdsys/compactcenter/bootstrap.c b/board/gdsys/compactcenter/bootstrap.c
> new file mode 100644
> index 0000000..cd638c2
> --- /dev/null
> +++ b/board/gdsys/compactcenter/bootstrap.c
...
> +/*
> + * Bytes 5,6,8,9,11 change for NAND boot
> + */
> +#if 0
> +/*
> + * Values for 512 page size NAND chips, not used anymore, just
> + * keep them here for reference
> + */
> +static u8 nand_boot[] = {
> +	0x90, 0x01,  0xa0, 0x68, 0x58
> +};
> +#else

Please do not add dead code.

> +/*
> + * Values for 2k page size NAND chips
> + */
> +static u8 nand_boot[] = {
> +	0x90, 0x01,  0xa0, 0xe8, 0x58
> +};
> +#endif
> +
> +static int do_bootstrap(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> +{
...
> +	/* set the nand flag based on provided input */
> +	if ((strcmp(argv[1], "nand") == 0))
> +		b_nand = 1;
> +	else
> +		b_nand = 0;

Consider changing this into:

	if ((strcmp(argv[1], "nand") == 0))
		b_nand = NAND_COMPATIBLE;
	else
		b_nand = NOR_COMPATIBLE;

(and probably chose a diferent name for the variable then).

Then you could simplify this into:

> +	printf("Available configurations: \n\n");
> +
> +	if (b_nand) {
> +		for (x = 0, y = 0; boot_configs[x][0] != 0; x++) {
> +			/* filter on nand compatible */
> +			if (boot_configs[x][0] & NAND_COMPATIBLE) {
> +				printf(" %d - %s\n", (y+1), config_labels[x]);
> +				y++;
> +			}
> +		}
> +	} else {
> +		for (x = 0, y = 0; boot_configs[x][0] != 0; x++) {
> +			/* filter on nor compatible */
> +			if (boot_configs[x][0] & NOR_COMPATIBLE) {
> +				printf(" %d - %s\n", (y+1), config_labels[x]);
> +				y++;
> +			}
> +		}
> +	}

	for (x = 0, y = 0; boot_configs[x][0] != 0; x++) {
		if (boot_configs[x][0] & b_nand) {
			printf(" %d - %s\n", (y+1), config_labels[x]);
			y++;
		}
	}


> +	for (x = 0; boot_configs[x][0] != 0; x++) {
> +		if (b_nand) {
> +			if (boot_configs[x][0] & NAND_COMPATIBLE) {
> +				if (y > 0)
> +					y--;
> +				else if (y < 1)
> +					break;
> +			}
> +		} else {
> +			if (boot_configs[x][0] & NOR_COMPATIBLE) {
> +				if (y > 0)
> +					y--;
> +				else if (y < 1)
> +					break;
> +			}
> +		}
> +	}

Ditto here.

> diff --git a/board/gdsys/compactcenter/init.S b/board/gdsys/compactcenter/init.S
> new file mode 100644
> index 0000000..ab19ca1
> --- /dev/null
> +++ b/board/gdsys/compactcenter/init.S
...
> +tlbtab:
> +	tlbtab_start
> +
> +	/*
> +	 * BOOT_CS (FLASH) must be first. Before relocation SA_I can be off to
> +	 * use the speed up boot process. It is patched after relocation to
> +	 * enable SA_I
> +	 */
> +	tlbentry(CONFIG_SYS_BOOT_BASE_ADDR, SZ_16M, CONFIG_SYS_BOOT_BASE_ADDR, 4, AC_R|AC_W|AC_X|SA_G) /* TLB 0 */

Line way too long.

> +	/*
> +	 * TLB entries for SDRAM are not needed on this platform.
> +	 * They are dynamically generated in the SPD DDR(2) detection
> +	 * routine.
> +	 */
> +
> +#ifdef CONFIG_SYS_INIT_RAM_DCACHE
> +	/* TLB-entry for init-ram in dcache (SA_I must be turned off!) */
> +	tlbentry(CONFIG_SYS_INIT_RAM_ADDR, SZ_4K, CONFIG_SYS_INIT_RAM_ADDR, 0, AC_R|AC_W|AC_X|SA_G)

Ditto. Please fix everywhere.

> diff --git a/include/configs/compactcenter.h b/include/configs/compactcenter.h
> new file mode 100644
> index 0000000..ebee2c7
> --- /dev/null
> +++ b/include/configs/compactcenter.h
...
> +/*-----------------------------------------------------------------------
> + * Base addresses -- Note these are effective addresses where the
> + * actual resources get mapped (not physical addresses)
> + *----------------------------------------------------------------------*/
> +#define CONFIG_SYS_PCI_MEMBASE		0x80000000	/* mapped PCI memory	*/

More too long lines in this file.


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
Everyting looks interesting until you do it. Then you find it's  just
another job.                     - Terry Pratchett, _Moving Pictures_