[U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN

[Scott Wood]

On Mon, Mar 23, 2009 at 02:32:20PM +0530, apgmoorthy wrote:
> >nblocks = (CONFIG_SYS_MONITOR_LEN + pagesize - 1) >> pageshift;
> 
>        - Right , But the above line dosent give the nblocks ,
>          used erasesize and erase_shift.

D'oh, right.

> Please do find the updated patch below.

It's easier if updated patches are sent separately from the reply (both
to avoid getting lost, and to avoid having to manually strip discussion
from the changelog).

The patch looks good other than some style nits:

> Signed-off-by: Gangheyamoorthy   <moorthy.apg at samsung.com>
> Signed-off-by: Rohit Hagargundgi <h.rohit at samsung.com>

The signed-off-by lines should go in chronological order of handling;
thus, yours should be at the bottom (as the most recent one to touch or
forward the patch).

> +	/* Check for invalid block mark */
> +	if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 0xffff))
> +		return 1;

Unnecessary parens.

>  #ifdef __HAVE_ARCH_MEMCPY32
>  	/* 32 bytes boundary memory copy */
>  	memcpy32(buf, base, pagesize);
> @@ -89,25 +93,39 @@ static inline int onenand_read_page(ulong block, ulong page,
>  #define ONENAND_PAGES_PER_BLOCK		64
>  
>  /**
> - * onenand_read_block - Read a block data to buf
> + * onenand_read_block - Read First 'n' consecutive Good blocks holding 
> + *                      data to buf

"Read SYS_MONITOR_LEN from beginning of OneNAND, skipping bad blocks"

>   * @return 0 on success
>   */
> -int onenand_read_block0(unsigned char *buf)
> +int onenand_read_block(unsigned char *buf)
>  {
> -	int page, offset = 0;
> -	int pagesize = ONENAND_PAGE_SIZE;
> -
> -	/* MLC OneNAND has 4KiB page size */
> -	if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY)))
> -		pagesize <<= 1;
> +	int block = 0, page = ONENAND_START_PAGE, offset = 0;
> +	int pagesize = 0, erase_shift =0;
> +	int erasesize = 0, nblocks = 0;

s/=0/= 0/

> +
> +	if(onenand_readw(ONENAND_REG_TECHNOLOGY)) {

Space after "if".

> +		pagesize = 4096; /* MLC OneNAND has 4KiB pagesize */
> +		erase_shift = 18;
> +	} else {
> +		pagesize = 2048;
> +		erase_shift = 17;
> +	}
> +	erasesize = ONENAND_PAGES_PER_BLOCK * pagesize;
> +	nblocks = (CONFIG_SYS_MONITOR_LEN + erasesize -1) >> erase_shift;

Blank line after the closing brace at the end of a block (except with a
hanging else, or similar).

s/-1/- 1/

> +	for (; block < nblocks; block++) {
> +		for (; page < ONENAND_PAGES_PER_BLOCK; page++) {

Why not do block = 0 and page = 0 here, rather than at the beginning of
the function and (in the case of page) at the end of the loop?

> +			if (onenand_read_page(block, page, buf + offset, pagesize)) {
> +				/* This block is bad. Skip it and read next block */

Line length.

-Scott