[U-Boot] [PATCH v3 1/3] NAND boot: MPC8536DS support

[Scott Wood]

On Fri, Sep 18, 2009 at 11:35:33AM +0800, Mingkai Hu wrote:
> diff --git a/Makefile b/Makefile
> index 99837a3..4d18a9f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2446,6 +2446,7 @@ vme8349_config:		unconfig
>  ATUM8548_config:	unconfig
>  	@$(MKCONFIG) $(@:_config=) ppc mpc85xx atum8548
>  
> +MPC8536DS_NAND_config \
>  MPC8536DS_36BIT_config \
>  MPC8536DS_config:       unconfig

NAND and 36BIT are orthogonal.

How about changing it to:

# Options: NAND, 36BIT
MPC8536DS_%_config MPC8536DS_config: unconfig

> +#if defined(CONFIG_NAND_BR_PRELIM)  \
> +	&& defined(CONFIG_NAND_OR_PRELIM)
> +	out_be32(&lbc->br0, CONFIG_NAND_BR_PRELIM);
> +	out_be32(&lbc->or0, CONFIG_NAND_OR_PRELIM);
> +	/* for FPGA */
> +	out_be32(&lbc->br3, CONFIG_SYS_BR3_PRELIM);
> +	out_be32(&lbc->or3, CONFIG_SYS_OR3_PRELIM);

Those last two lines should probably be #ifdef CONFIG_SYS_BR3_PRELIM.

> +#if defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SYS_INIT_L2_ADDR)
> +	ccsr_l2cache_t *l2cache = (void *)CONFIG_SYS_MPC85xx_L2_ADDR;
> +	uint l2srbar;
> +	int i;
> +
> +	l2srbar = CONFIG_SYS_INIT_L2_ADDR;
> +	out_be32(&l2cache->l2srbar0, l2srbar);
> +
> +	/* set MBECCDIS=1, SBECCDIS=1 */
> +	out_be32(&l2cache->l2errdis,
> +			(MPC85xx_L2ERRDIS_MBECC |
> +			 MPC85xx_L2ERRDIS_SBECC));
> +
> +	/* set L2E=1 & L2SRAM=001 */
> +	out_be32(&l2cache->l2ctl,
> +			(MPC85xx_L2CTL_L2E |
> +			 MPC85xx_L2CTL_L2SRAM_ENTIRE));
> +
> +	/* Initialize L2 SRAM to zero */
> +	for (i = 0; i < CONFIG_SYS_L2_SIZE; i++)
> +		((char *)l2srbar)[i] = 0;

"uint" is not a valid type for either virtual or physical addresses.

Use a pointer (or uintptr_t if you must) for the former, and phys_addr_t for
the latter.

You're using it as char *, so why not just declare it that way?

> +void board_init_f(ulong bootflag)
> +{
> +	u8 sysclk_ratio;

You're not saving any space over plain int/uint...

> +	uint plat_ratio, bus_clk, sys_clk;
> +	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
> +
> +	/* initialize selected port with appropriate baud rate */
> +	sysclk_ratio = *((volatile unsigned char *)(PIXIS_BASE + PIXIS_SPD));
> +	sysclk_ratio &= 0x7;
> +	switch (sysclk_ratio) {
> +	case 0:
> +		sys_clk = 33333000;
> +		break;
> +	case 1:
> +		sys_clk = 39999600;
> +		break;
> +	case 2:
> +		sys_clk = 49999500;
> +		break;
> +	case 3:
> +		sys_clk = 66666000;
> +		break;
> +	case 4:
> +		sys_clk = 83332500;
> +		break;
> +	case 5:
> +		sys_clk = 99999000;
> +		break;
> +	case 6:
> +		sys_clk = 133332000;
> +		break;
> +	case 7:
> +		sys_clk = 166665000;
> +		break;
> +	default:
> +		sys_clk = 0;

This default: case is impossible to reach.

> +		break;
> +	}

We could save some space by putting this in a table.

> +	plat_ratio = (gur->porpllsr) & 0x0000003e;

Unnecessary parens.

> +	plat_ratio >>= 1;

plat_ratio /= 2 is more readable and should generate identical code.

-Scott