[U-Boot] [PATCH] [OneNAND IPL] OneNAND board init support

[Scott Wood]

On Sat, Aug 29, 2009 at 01:00:59PM +0900, Kyungmin Park wrote:
>  #define READ_INTERRUPT()                                                \
> -	onenand_readw(THIS_ONENAND(ONENAND_REG_INTERRUPT))
> +				onenand_readw(ONENAND_REG_INTERRUPT)

You could get rid of the newline now...

> +enum {
> +	ONENAND_USE_DEFAULT,
> +	ONENAND_USE_GENERIC,
> +};

What is this?  Add a comment, and possibly more specific names.

> +extern int (*onenand_read_page)(ulong block, ulong page,
> +				u_char *buf, int pagesize);

Maybe use a weak function instead?  Or an #ifdef
CONFIG_SYS_ONENAND_BOARD_READ_PAGE that will keep the code for the
generic version from being in the image (it'd be nice if we could
optimize out replaced weak functions).  It seems especially odd that you
use one method for init and another for read page.

>  /* read a page with ECC */
> -static inline int onenand_read_page(ulong block, ulong page,
> +static int generic_onenand_read_page(ulong block, ulong page,
>  				u_char * buf, int pagesize)

Is the "generic" code really generic, or is it just one specific
controller?

> +#ifdef CONFIG_ONENAND_BOARD_INIT

This should probably be CONFIG_SYS_ONENAND_BOARD_INIT -- it's not
tweakable by the end user.

How is this different from the existing CONFIG_USE_ONENAND_BOARD_INIT?

> +	onenand_board_init(&page_is_4KiB, &page);
> +#else
> +	onenand_generic_init(&page_is_4KiB, &page);
> +#endif
>  
> -	if (onenand_readw(ONENAND_REG_TECHNOLOGY)) {
> -		pagesize = 4096; /* MLC OneNAND has 4KiB pagesize */
> +	if (page_is_4KiB) {
> +		pagesize = 4096; /* OneNAND has 4KiB pagesize */
>  		erase_shift = 18;
> -	} else {
> -		pagesize = 2048;
> -		erase_shift = 17;
>  	}

I don't understand why you move the pagesize/erase_shift init before
onenand_board_init, suggesting that the init code change it if it needs
changing -- but then leave the page_is_4KiB stuff in the generic code.

This should probably just be filled in by the init code without anything
here.

-Scott