All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
@ 2009-03-23  9:02 apgmoorthy
  2009-03-23 21:25 ` Scott Wood
  0 siblings, 1 reply; 25+ messages in thread
From: apgmoorthy @ 2009-03-23  9:02 UTC (permalink / raw)
  To: u-boot

Hi Scott, 

>> +	if (page < 2 && (onenand_readw(THIS_ONENAND(ONENAND_SPARERAM)) != 0xffff))

 >Line length.

     - Corrected.
      
>>  	int pagesize = ONENAND_PAGE_SIZE;
>> +	int nblocks = CONFIG_SYS_MONITOR_LEN / ONENAND_BLOCK_SIZE;

 >Shouldn't nblocks be rounded up?

>>  		pagesize <<= 1;
>> +		nblocks = (nblocks + 1) >> 1;
>> +	}

 >Hmm, might be clearer to just do this:

>if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY))) {
>	pagesize = 4096;
>	pageshift = 12;
>} else {
>	pagesize = 2048;
>	pageshift = 11;
>}

>nblocks = (CONFIG_SYS_MONITOR_LEN + pagesize - 1) >> pageshift;

       - Right , But the above line dosent give the nblocks ,
         used erasesize and erase_shift.


>ONENAND_PAGE_SIZE as a constant should go away since it's not always
>true.

        - Removed it.

>> +	for (; block < nblocks; block++) {
>> +		for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
>> +			if (onenand_read_page(block, page, buf + offset, pagesize)) {
>> +				/* This block is bad. Skip it and read next block */
>> +				nblocks++;
>> +				break;
>> +			}
>> +			offset += pagesize;

>If you find a bad block marker in the second page of a block, shouldn't
>you rewind offset to the beginning of the block?

       - Correct , have reverted back.

>BTW, if we're going to have "onenand_readw" and "onenand_writew"
>wrappers, can we make them useful by combining them with THIS_ONENAND?

       - Corrected , Now onenand_readw/onenand_writew conatins THIS_ONENAND.

Please do find the updated patch below.

With Regards
  Moorthy


Currently OneNAND initial program loader (ipl) reads only block 0 ie 128KB.
However, u-boot image for apollon board is 195KB making the board
unbootable with OneNAND.

Fix ipl to read CONFIG_SYS_MONITOR_LEN.
CONFIG_SYS_MONITOR_LEN macro holds the U-Boot image size.

Signed-off-by: Gangheyamoorthy   <moorthy.apg@samsung.com>
Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
---
 
diff --git a/onenand_ipl/onenand_boot.c b/onenand_ipl/onenand_boot.c
index 86428cc..63995ce 100644
--- a/onenand_ipl/onenand_boot.c
+++ b/onenand_ipl/onenand_boot.c
@@ -36,7 +36,7 @@ void start_oneboot(void)
 
 	buf = (uchar *) CONFIG_SYS_LOAD_ADDR;
 
-	onenand_read_block0(buf);
+	onenand_read_block(buf);
 
 	((init_fnc_t *)CONFIG_SYS_LOAD_ADDR)();
 
diff --git a/onenand_ipl/onenand_ipl.h b/onenand_ipl/onenand_ipl.h
index c5a722d..412572a 100644
--- a/onenand_ipl/onenand_ipl.h
+++ b/onenand_ipl/onenand_ipl.h
@@ -31,5 +31,5 @@
 #define READ_INTERRUPT()                                                \
 	onenand_readw(THIS_ONENAND(ONENAND_REG_INTERRUPT))
 
-extern int onenand_read_block0(unsigned char *buf);
+extern int onenand_read_block(unsigned char *buf);
 #endif
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c
index 6d04943..7415c7f 100644
--- a/onenand_ipl/onenand_read.c
+++ b/onenand_ipl/onenand_read.c
@@ -49,20 +49,20 @@ static inline int onenand_read_page(ulong block, ulong page,
 #endif
 
 	onenand_writew(onenand_block_address(block),
-		THIS_ONENAND(ONENAND_REG_START_ADDRESS1));
+			ONENAND_REG_START_ADDRESS1);
 
 	onenand_writew(onenand_bufferram_address(block),
-		THIS_ONENAND(ONENAND_REG_START_ADDRESS2));
+			ONENAND_REG_START_ADDRESS2);
 
 	onenand_writew(onenand_sector_address(page),
-		THIS_ONENAND(ONENAND_REG_START_ADDRESS8));
+			ONENAND_REG_START_ADDRESS8);
 
 	onenand_writew(onenand_buffer_address(),
-		THIS_ONENAND(ONENAND_REG_START_BUFFER));
+			ONENAND_REG_START_BUFFER);
 
-	onenand_writew(ONENAND_INT_CLEAR, THIS_ONENAND(ONENAND_REG_INTERRUPT));
+	onenand_writew(ONENAND_INT_CLEAR, ONENAND_REG_INTERRUPT);
 
-	onenand_writew(ONENAND_CMD_READ, THIS_ONENAND(ONENAND_REG_COMMAND));
+	onenand_writew(ONENAND_CMD_READ, ONENAND_REG_COMMAND);
 
 #ifndef __HAVE_ARCH_MEMCPY32
 	p = (unsigned long *) buf;
@@ -72,6 +72,10 @@ static inline int onenand_read_page(ulong block, ulong page,
 	while (!(READ_INTERRUPT() & ONENAND_INT_READ))
 		continue;
 
+	/* Check for invalid block mark */
+	if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 0xffff))
+		return 1;
+
 #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
  * @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;
+
+	if(onenand_readw(ONENAND_REG_TECHNOLOGY)) {
+		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;
 
 	/* NOTE: you must read page from page 1 of block 0 */
 	/* read the block page by page*/
-	for (page = ONENAND_START_PAGE;
-	    page < ONENAND_PAGES_PER_BLOCK; page++) {
-
-		onenand_read_page(0, page, buf + offset, pagesize);
-		offset += pagesize;
+	for (; block < nblocks; block++) {
+		for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
+			if (onenand_read_page(block, page, buf + offset, pagesize)) {
+				/* This block is bad. Skip it and read next block */
+	 			offset -= page * pagesize;
+				nblocks++;
+				break;
+			}
+			offset += pagesize;
+		}
+		page = 0;
 	}
 
 	return 0;

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-23  9:02 [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN apgmoorthy
@ 2009-03-23 21:25 ` Scott Wood
  2009-03-23 22:09   ` Wolfgang Denk
                     ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Scott Wood @ 2009-03-23 21:25 UTC (permalink / raw)
  To: u-boot

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@samsung.com>
> Signed-off-by: Rohit Hagargundgi <h.rohit@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)@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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-23 21:25 ` Scott Wood
@ 2009-03-23 22:09   ` Wolfgang Denk
  2009-03-23 22:22     ` Scott Wood
  2009-03-25 15:49   ` apgmoorthy
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 25+ messages in thread
From: Wolfgang Denk @ 2009-03-23 22:09 UTC (permalink / raw)
  To: u-boot

Dear Scott Wood,

In message <20090323212514.GA30976@ld0162-tx32.am.freescale.net> you wrote:
>
> > +	/* Check for invalid block mark */
> > +	if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 0xffff))
> > +		return 1;
> 
> Unnecessary parens.

Where? I find them pretty useful. Please keep!

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
I don't want to be young again, I just don't want to get any older.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-23 22:09   ` Wolfgang Denk
@ 2009-03-23 22:22     ` Scott Wood
  2009-03-23 22:46       ` Wolfgang Denk
  0 siblings, 1 reply; 25+ messages in thread
From: Scott Wood @ 2009-03-23 22:22 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> Dear Scott Wood,
> 
> In message <20090323212514.GA30976@ld0162-tx32.am.freescale.net> you wrote:
>>> +	/* Check for invalid block mark */
>>> +	if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 0xffff))
>>> +		return 1;
>> Unnecessary parens.
> 
> Where? I find them pretty useful.

Around the second comparison.  Why "if (a < b && (c != d))" and not "if 
(a < b && c != d)", or if the parens are preferred, "if ((a < b) && (c 
!= d))"?  Is it because "c" is a function call?

>  Please keep!

OK -- I guess this is another of the unwritten points on which U-boot's 
style deviates from that which is typical in Linux.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-23 22:22     ` Scott Wood
@ 2009-03-23 22:46       ` Wolfgang Denk
  2009-03-25  4:13         ` Amit Kumar Sharma
  0 siblings, 1 reply; 25+ messages in thread
From: Wolfgang Denk @ 2009-03-23 22:46 UTC (permalink / raw)
  To: u-boot

Dear Scott,

In message <49C80B99.5010808@freescale.com> you wrote:
>
> >>> +	if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 0xffff))
> >>> +		return 1;
> >> Unnecessary parens.
> > 
> > Where? I find them pretty useful.
> 
> Around the second comparison.  Why "if (a < b && (c != d))" and not "if 
> (a < b && c != d)", or if the parens are preferred, "if ((a < b) && (c 
> != d))"?  Is it because "c" is a function call?

Actually I'd probably write this as

	if ((page < 2) && (onenand_readw(ONENAND_SPARERAM) != 0xffff))

for consistency, but being lazy I guess I might use the same code as
the OP.

> OK -- I guess this is another of the unwritten points on which U-boot's 
> style deviates from that which is typical in Linux.

Actually this is not some formal style (at least no rule that I know
of), but personal taste :-)

When I parse something like

	if (page < 2 && onenand_readw(ONENAND_SPARERAM) != 0xffff)

I can eaisly see the "page < 2" expression because it  is  relatively
short. But I have to look twice for the "onenand_readw(ONENAND_SPARE-
RAM)  !=  0xffff" part because it is long and includes nested parens.
So I feel tempted to make this easier to read by surrounding it  with
parens - like the OP did.

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@denx.de
I'm frequently appalled by the low regard you Earthmen have for life.
	-- Spock, "The Galileo Seven", stardate 2822.3

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-23 22:46       ` Wolfgang Denk
@ 2009-03-25  4:13         ` Amit Kumar Sharma
  0 siblings, 0 replies; 25+ messages in thread
From: Amit Kumar Sharma @ 2009-03-25  4:13 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,
----- Original Message ----- 
From: "Wolfgang Denk" <wd@denx.de>
To: "Scott Wood" <scottwood@freescale.com>
Cc: <u-boot@lists.denx.de>; "apgmoorthy" 
<moorthy.apg@samsung.com>
Sent: Tuesday, March 24, 2009 4:16 AM
Subject: Re: [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read 
CONFIG_SYS_MONITOR_LEN


> Dear Scott,
>
> In message <49C80B99.5010808@freescale.com> you wrote:
>>
>> >>> + if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 
>> >>> 0xffff))
>> >>> + return 1;
>> >> Unnecessary parens.
>> >
>> > Where? I find them pretty useful.
>>
>> Around the second comparison.  Why "if (a < b && (c != 
>> d))" and not "if
>> (a < b && c != d)", or if the parens are preferred, "if 
>> ((a < b) && (c
>> != d))"?  Is it because "c" is a function call?
>
> Actually I'd probably write this as
>
> if ((page < 2) && (onenand_readw(ONENAND_SPARERAM) != 
> 0xffff))
>
> for consistency, but being lazy I guess I might use the 
> same code as
> the OP.
>
>> OK -- I guess this is another of the unwritten points on 
>> which U-boot's
>> style deviates from that which is typical in Linux.
>
> Actually this is not some formal style (at least no rule 
> that I know
> of), but personal taste :-)
>
> When I parse something like
>
> if (page < 2 && onenand_readw(ONENAND_SPARERAM) != 0xffff)
>
> I can eaisly see the "page < 2" expression because it  is 
> relatively
> short. But I have to look twice for the 
> "onenand_readw(ONENAND_SPARE-
> RAM)  !=  0xffff" part because it is long and includes 
> nested parens.
> So I feel tempted to make this easier to read by 
> surrounding it  with
> parens - like the OP did.
I agree with for better readabilty we will chnage and resend 
it today only.
We will try to improve readability of code.

Thanks for suggestion.
Amit
>
> 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
> I'm frequently appalled by the low regard you Earthmen 
> have for life.
> -- Spock, "The Galileo Seven", stardate 2822.3
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-23 21:25 ` Scott Wood
  2009-03-23 22:09   ` Wolfgang Denk
@ 2009-03-25 15:49   ` apgmoorthy
  2009-03-27  9:15   ` apgmoorthy
  2009-04-06  6:05   ` [U-Boot] OneNand support broken for apollon apgmoorthy
  3 siblings, 0 replies; 25+ messages in thread
From: apgmoorthy @ 2009-03-25 15:49 UTC (permalink / raw)
  To: u-boot

Hi Scott,

On Tuesday, March 24, 2009 2:55 AM , Scott Wood wrote

>> 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).

>> Signed-off-by: Gangheyamoorthy   <moorthy.apg@samsung.com>
>> Signed-off-by: Rohit Hagargundgi <h.rohit@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).

    - Thanks, Will take care.

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

>Unnecessary parens.

    - I think , parens make it more readable.

>> - * 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"

    - Changed it in Function Header.

>> +	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".

   - Corrected them.

>> +	} 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/

    - Changed them.

>> +	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?

    - Initialized block in the for. With 'page' we need to start with page 1 in Block 0.
       

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

>Line length.

   - Resolved.

With Regards
  Moorthy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-23 21:25 ` Scott Wood
  2009-03-23 22:09   ` Wolfgang Denk
  2009-03-25 15:49   ` apgmoorthy
@ 2009-03-27  9:15   ` apgmoorthy
  2009-03-30 22:33     ` Scott Wood
  2009-03-31 16:08     ` [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN Alessandro Rubini
  2009-04-06  6:05   ` [U-Boot] OneNand support broken for apollon apgmoorthy
  3 siblings, 2 replies; 25+ messages in thread
From: apgmoorthy @ 2009-03-27  9:15 UTC (permalink / raw)
  To: u-boot

Currently OneNAND initial program loader (ipl) reads only block 0 ie 128KB.
However, u-boot image for apollon board is 195KB making the board
unbootable with OneNAND.

Fix ipl to read CONFIG_SYS_MONITOR_LEN.
CONFIG_SYS_MONITOR_LEN macro holds the U-Boot image size.

 Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
 Signed-off-by: Gangheyamoorthy   <moorthy.apg@samsung.com>
---
 onenand_boot.c |    2 -
 onenand_ipl.h  |    8 ++-----
 onenand_read.c |   58 +++++++++++++++++++++++++++++++++++++++------------------
 3 files changed, 44 insertions(+), 24 deletions(-) 
diff --git a/onenand_ipl/onenand_boot.c b/onenand_ipl/onenand_boot.c
index 86428cc..63995ce 100644
--- a/onenand_ipl/onenand_boot.c
+++ b/onenand_ipl/onenand_boot.c
@@ -36,7 +36,7 @@ void start_oneboot(void)
 
 	buf = (uchar *) CONFIG_SYS_LOAD_ADDR;
 
-	onenand_read_block0(buf);
+	onenand_read_block(buf);
 
 	((init_fnc_t *)CONFIG_SYS_LOAD_ADDR)();
 
diff --git a/onenand_ipl/onenand_ipl.h b/onenand_ipl/onenand_ipl.h
index 57e54f5..412572a 100644
--- a/onenand_ipl/onenand_ipl.h
+++ b/onenand_ipl/onenand_ipl.h
@@ -23,15 +23,13 @@
 
 #include <linux/mtd/onenand_regs.h>
 
-#define onenand_readw(a)        readw(a)
-#define onenand_writew(v, a)    writew(v, a)
+#define onenand_readw(a)        readw(THIS_ONENAND(a))
+#define onenand_writew(v, a)    writew(v, THIS_ONENAND(a))
 
 #define THIS_ONENAND(a)         (CONFIG_SYS_ONENAND_BASE + (a))
 
 #define READ_INTERRUPT()                                                \
 	onenand_readw(THIS_ONENAND(ONENAND_REG_INTERRUPT))
 
-#define ONENAND_PAGE_SIZE                       2048
-
-extern int onenand_read_block0(unsigned char *buf);
+extern int onenand_read_block(unsigned char *buf);
 #endif
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c
index 6d04943..7886358 100644
--- a/onenand_ipl/onenand_read.c
+++ b/onenand_ipl/onenand_read.c
@@ -49,20 +49,20 @@ static inline int onenand_read_page(ulong block, ulong page,
 #endif
 
 	onenand_writew(onenand_block_address(block),
-		THIS_ONENAND(ONENAND_REG_START_ADDRESS1));
+			ONENAND_REG_START_ADDRESS1);
 
 	onenand_writew(onenand_bufferram_address(block),
-		THIS_ONENAND(ONENAND_REG_START_ADDRESS2));
+			ONENAND_REG_START_ADDRESS2);
 
 	onenand_writew(onenand_sector_address(page),
-		THIS_ONENAND(ONENAND_REG_START_ADDRESS8));
+			ONENAND_REG_START_ADDRESS8);
 
 	onenand_writew(onenand_buffer_address(),
-		THIS_ONENAND(ONENAND_REG_START_BUFFER));
+			ONENAND_REG_START_BUFFER);
 
-	onenand_writew(ONENAND_INT_CLEAR, THIS_ONENAND(ONENAND_REG_INTERRUPT));
+	onenand_writew(ONENAND_INT_CLEAR, ONENAND_REG_INTERRUPT);
 
-	onenand_writew(ONENAND_CMD_READ, THIS_ONENAND(ONENAND_REG_COMMAND));
+	onenand_writew(ONENAND_CMD_READ, ONENAND_REG_COMMAND);
 
 #ifndef __HAVE_ARCH_MEMCPY32
 	p = (unsigned long *) buf;
@@ -72,6 +72,10 @@ static inline int onenand_read_page(ulong block, ulong page,
 	while (!(READ_INTERRUPT() & ONENAND_INT_READ))
 		continue;
 
+	/* Check for invalid block mark */
+	if (page < 2 && (onenand_readw(ONENAND_SPARERAM) != 0xffff))
+		return 1;
+
 #ifdef __HAVE_ARCH_MEMCPY32
 	/* 32 bytes boundary memory copy */
 	memcpy32(buf, base, pagesize);
@@ -89,25 +93,43 @@ 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 CONFIG_SYS_MONITOR_LEN from begining
+ *                      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;
+	int block;
+	int page = ONENAND_START_PAGE, offset = 0;
+	int pagesize = 0, erase_shift = 0;
+	int erasesize = 0, nblocks = 0;
+
+	if (onenand_readw(ONENAND_REG_TECHNOLOGY)) {
+		pagesize = 4096; /* MLC OneNAND has 4KiB pagesize */
+		erase_shift = 18;
+	} else {
+		pagesize = 2048;
+		erase_shift = 17;
+	}
 
-	/* MLC OneNAND has 4KiB page size */
-	if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY)))
-		pagesize <<= 1;
+	erasesize = ONENAND_PAGES_PER_BLOCK * pagesize;
+	nblocks = (CONFIG_SYS_MONITOR_LEN + erasesize - 1) >> erase_shift;
 
 	/* NOTE: you must read page from page 1 of block 0 */
 	/* read the block page by page*/
-	for (page = ONENAND_START_PAGE;
-	    page < ONENAND_PAGES_PER_BLOCK; page++) {
-
-		onenand_read_page(0, page, buf + offset, pagesize);
-		offset += pagesize;
+	for (block = 0; block < nblocks; block++) {
+		for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
+			if (onenand_read_page(block, page, buf + offset,
+						pagesize)) {
+				/* This block is bad. Skip it 
+				 * and read next block */
+	 			offset -= page * pagesize;
+				nblocks++;
+				break;
+			}
+			offset += pagesize;
+		}
+		page = 0;
 	}
 
 	return 0;

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-27  9:15   ` apgmoorthy
@ 2009-03-30 22:33     ` Scott Wood
  2009-03-31  4:12       ` Amit Kumar Sharma
                         ` (2 more replies)
  2009-03-31 16:08     ` [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN Alessandro Rubini
  1 sibling, 3 replies; 25+ messages in thread
From: Scott Wood @ 2009-03-30 22:33 UTC (permalink / raw)
  To: u-boot

apgmoorthy wrote:
> Currently OneNAND initial program loader (ipl) reads only block 0 ie 128KB.
> However, u-boot image for apollon board is 195KB making the board
> unbootable with OneNAND.
> 
> Fix ipl to read CONFIG_SYS_MONITOR_LEN.
> CONFIG_SYS_MONITOR_LEN macro holds the U-Boot image size.
> 
>  Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
>  Signed-off-by: Gangheyamoorthy   <moorthy.apg@samsung.com>

Applied to u-boot-nand-flash, with the below whitespace errors fixed:

Applying RE: [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read 
CONFIG_SYS_MONITOR_LEN
.dotest/patch:128: trailing whitespace.
                                 /* This block is bad. Skip it
.dotest/patch:130: space before tab in indent.
                                 offset -= page * pagesize;
warning: 2 lines add whitespace errors.

Note that there are a couple of board files (apollon and nmdk8815) that 
use the OneNAND loader that do not define CONFIG_SYS_MONITOR_LEN.  I've 
added the maintainers to the Cc: list.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-30 22:33     ` Scott Wood
@ 2009-03-31  4:12       ` Amit Kumar Sharma
  2009-04-12  7:55       ` apgmoorthy
  2009-04-13 13:04       ` [U-Boot] [PATCH] Bad Block Capable environment for OneNAND apgmoorthy
  2 siblings, 0 replies; 25+ messages in thread
From: Amit Kumar Sharma @ 2009-03-31  4:12 UTC (permalink / raw)
  To: u-boot

Hi Rubini,

AS suggested by scott , Please share your views regarding 
CONFIG_SYS_MONITOR_LEN
because image size getting bigger then 1 block we have to 
use this macro for uboot image size.

----- Original Message ----- 
From: "Scott Wood" <scottwood@freescale.com>
To: "apgmoorthy" <moorthy.apg@samsung.com>
Cc: <u-boot@lists.denx.de>; <rubini@unipv.it>
Sent: Tuesday, March 31, 2009 4:03 AM
Subject: Re: [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read 
CONFIG_SYS_MONITOR_LEN


> apgmoorthy wrote:
>> Currently OneNAND initial program loader (ipl) reads only 
>> block 0 ie 128KB.
>> However, u-boot image for apollon board is 195KB making 
>> the board
>> unbootable with OneNAND.
>>
>> Fix ipl to read CONFIG_SYS_MONITOR_LEN.
>> CONFIG_SYS_MONITOR_LEN macro holds the U-Boot image size.
>>
>>  Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
>>  Signed-off-by: Gangheyamoorthy 
>> <moorthy.apg@samsung.com>
>
> Applied to u-boot-nand-flash, with the below whitespace 
> errors fixed:
>
> Applying RE: [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read
> CONFIG_SYS_MONITOR_LEN
> .dotest/patch:128: trailing whitespace.
>                                 /* This block is bad. Skip 
> it
> .dotest/patch:130: space before tab in indent.
>                                 offset -= page * pagesize;
> warning: 2 lines add whitespace errors.
>
> Note that there are a couple of board files (apollon and 
> nmdk8815) that
> use the OneNAND loader that do not define 
> CONFIG_SYS_MONITOR_LEN.  I've
> added the maintainers to the Cc: list.
>
> -Scott
Thanks
Amit
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-27  9:15   ` apgmoorthy
  2009-03-30 22:33     ` Scott Wood
@ 2009-03-31 16:08     ` Alessandro Rubini
  2009-03-31 16:13       ` Scott Wood
  1 sibling, 1 reply; 25+ messages in thread
From: Alessandro Rubini @ 2009-03-31 16:08 UTC (permalink / raw)
  To: u-boot

Hello.

> Note that there are a couple of board files (apollon and nmdk8815) that 
> use the OneNAND loader that do not define CONFIG_SYS_MONITOR_LEN.  I've 
> added the maintainers to the Cc: list.

Sorry for the delay.

In the nomadik board the OneNAND driver is not yet present, a few init
lines are still missing (I have an error to fix before submitting those
lines).  I'll take care of CONFIG_SYS_MONITOR_LEN together with that.

Note however that on the Nomadik we don't use the IPL, since u-boot is
loaded by other code (a proprietary IPL that onlocks the CPU before
handing control to ours) so u-boot isn't even in the first sector
of OneNAND.

/alessandro

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-31 16:08     ` [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN Alessandro Rubini
@ 2009-03-31 16:13       ` Scott Wood
  2009-04-01  4:10         ` Amit Kumar Sharma
  0 siblings, 1 reply; 25+ messages in thread
From: Scott Wood @ 2009-03-31 16:13 UTC (permalink / raw)
  To: u-boot

Alessandro Rubini wrote:
> Hello.
> 
>> Note that there are a couple of board files (apollon and nmdk8815) that 
>> use the OneNAND loader that do not define CONFIG_SYS_MONITOR_LEN.  I've 
>> added the maintainers to the Cc: list.
> 
> Sorry for the delay.
> 
> In the nomadik board the OneNAND driver is not yet present, a few init
> lines are still missing (I have an error to fix before submitting those
> lines).  I'll take care of CONFIG_SYS_MONITOR_LEN together with that.
> 
> Note however that on the Nomadik we don't use the IPL, since u-boot is
> loaded by other code (a proprietary IPL that onlocks the CPU before
> handing control to ours) so u-boot isn't even in the first sector
> of OneNAND.

OK, sorry about that.  I ran into the issue when building apollon, and 
nmdk8815 looked like it was in a similar situation from grepping config 
files.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-31 16:13       ` Scott Wood
@ 2009-04-01  4:10         ` Amit Kumar Sharma
  2009-04-01 21:55           ` Scott Wood
  0 siblings, 1 reply; 25+ messages in thread
From: Amit Kumar Sharma @ 2009-04-01  4:10 UTC (permalink / raw)
  To: u-boot

Hi Scott,

----- Original Message ----- 
From: "Scott Wood" <scottwood@freescale.com>
To: "Alessandro Rubini" <rubini-list@gnudd.com>
Cc: <u-boot@lists.denx.de>; <moorthy.apg@samsung.com>
Sent: Tuesday, March 31, 2009 9:43 PM
Subject: Re: [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read 
CONFIG_SYS_MONITOR_LEN


> Alessandro Rubini wrote:
>> Hello.
>>
>>> Note that there are a couple of board files (apollon and 
>>> nmdk8815) that
>>> use the OneNAND loader that do not define 
>>> CONFIG_SYS_MONITOR_LEN.  I've
>>> added the maintainers to the Cc: list.
>>
>> Sorry for the delay.
>>
>> In the nomadik board the OneNAND driver is not yet 
>> present, a few init
>> lines are still missing (I have an error to fix before 
>> submitting those
>> lines).  I'll take care of CONFIG_SYS_MONITOR_LEN 
>> together with that.
>>
>> Note however that on the Nomadik we don't use the IPL, 
>> since u-boot is
>> loaded by other code (a proprietary IPL that onlocks the 
>> CPU before
>> handing control to ours) so u-boot isn't even in the 
>> first sector
>> of OneNAND.
>
> OK, sorry about that.  I ran into the issue when building 
> apollon, and
> nmdk8815 looked like it was in a similar situation from 
> grepping config
> files.
>
> -Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-04-01  4:10         ` Amit Kumar Sharma
@ 2009-04-01 21:55           ` Scott Wood
  0 siblings, 0 replies; 25+ messages in thread
From: Scott Wood @ 2009-04-01 21:55 UTC (permalink / raw)
  To: u-boot

Amit Kumar Sharma wrote:
> From Alessandro mail it is claear CONFIG_SYS_MONITOR_LEN 
> only used by apollon
> that can be changed by Apollon users independently. we will 
> check othe rissue of white space and release patch.

No need; I've already fixed the whitespace and applied the patch (as 
noted in the previous e-mail).

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] OneNand support broken for apollon
  2009-03-23 21:25 ` Scott Wood
                     ` (2 preceding siblings ...)
  2009-03-27  9:15   ` apgmoorthy
@ 2009-04-06  6:05   ` apgmoorthy
  3 siblings, 0 replies; 25+ messages in thread
From: apgmoorthy @ 2009-04-06  6:05 UTC (permalink / raw)
  To: u-boot


On Apr 05, 2009 01:40 Jean-Christophe Wrote :

>	Your patch Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
>	brake the appolon
>	Could you take a look?

   - Can you please do let me know , where exactly do you see a problem ?

With Regards
  Moorthy    

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-30 22:33     ` Scott Wood
  2009-03-31  4:12       ` Amit Kumar Sharma
@ 2009-04-12  7:55       ` apgmoorthy
  2009-04-23 22:39         ` Scott Wood
  2009-04-13 13:04       ` [U-Boot] [PATCH] Bad Block Capable environment for OneNAND apgmoorthy
  2 siblings, 1 reply; 25+ messages in thread
From: apgmoorthy @ 2009-04-12  7:55 UTC (permalink / raw)
  To: u-boot


Hi Scott, 

On Tuesday, March 31, 2009 4:04 AM Scott Wood Wrote :

>>Note that there are a couple of board files (apollon and nmdk8815) that use the OneNAND loader 
>>that do not define CONFIG_SYS_MONITOR_LEN.  I've added the maintainers to the Cc: list.

CONFIG_SYS_MONITOR_LEN is not defined in include/configs/apollon.h as of now.

This is done by the Post : [U-Boot] [PATCH 2/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN

What do you feel about getting this one inside u-boot-nand-flash ?

With Regards
  Moorthy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot]  [PATCH] Bad Block Capable environment for OneNAND
  2009-03-30 22:33     ` Scott Wood
  2009-03-31  4:12       ` Amit Kumar Sharma
  2009-04-12  7:55       ` apgmoorthy
@ 2009-04-13 13:04       ` apgmoorthy
  2009-04-28 23:11         ` Scott Wood
  2 siblings, 1 reply; 25+ messages in thread
From: apgmoorthy @ 2009-04-13 13:04 UTC (permalink / raw)
  To: u-boot

Hi,
       With OneNAND ipl reading CONFIG_SYS_MONITOR_LEN , Environment Area should start after CONFIG_SYS_MONITOR_LEN.
Environment is made Bad Block cpapable too. These are done in the patch.
And fix 'onenand test' command to skip Bootloader and Environment Blocks.

With Regards
  Moorthy

Makes Environment to start after CONFIG_SYS_MONITOR_LEN
Environment is made to be Bad Block aware/capable.
Fix 'onenand test' command to skip Bootloader and Environment Blocks.

 Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
 Signed-off-by: Gangheyamoorthy   <moorthy.apg@samsung.com>
---
 common/cmd_onenand.c      |   12 ++++--
 common/env_onenand.c      |   96 +++++++++++++++++++++++++++++++++------------
 include/configs/apollon.h |    2 +-
 include/onenand_uboot.h   |    4 ++
 4 files changed, 84 insertions(+), 30 deletions(-)

diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c
index 5832ff8..c366935 100644
--- a/common/cmd_onenand.c
+++ b/common/cmd_onenand.c
@@ -206,6 +206,7 @@ static int onenand_block_test(u32 start, u32 size)
 	u_char *buf;
 	u_char *verify_buf;
 	int ret;
+	int badblocklen = 0;
 
 	buf = malloc(blocksize);
 	if (!buf) {
@@ -219,13 +220,16 @@ static int onenand_block_test(u32 start, u32 size)
 		return -1;
 	}
 
+	/* Protect boot-loader and environment variables from badblock testing */
+	while (start < CONFIG_SYS_MONITOR_LEN + CONFIG_ENV_SIZE + badblocklen) {
+		if (mtd->block_isbad(&onenand_mtd, start))
+			badblocklen += blocksize;
+		start += blocksize;
+	}
+
 	start_block = start >> this->erase_shift;
 	end_block = (start + size) >> this->erase_shift;
 
-	/* Protect boot-loader from badblock testing */
-	if (start_block < 2)
-		start_block = 2;
-
 	if (end_block > (mtd->size >> this->erase_shift))
 		end_block = mtd->size >> this->erase_shift;
 
diff --git a/common/env_onenand.c b/common/env_onenand.c
index dbccc79..76e3aa8 100644
--- a/common/env_onenand.c
+++ b/common/env_onenand.c
@@ -39,6 +39,16 @@ extern uchar default_environment[];
 
 #define ONENAND_ENV_SIZE(mtd)	(mtd.writesize - ENV_HEADER_SIZE)
 
+/*
+ * User can give many blocks to environment variable partition
+ * through CONFIG_ENV_SIZE macro.
+ * The variables are written to first block in the partition. If this block
+ * goes bad, the successive block is used to store environment variables.
+ *
+ */
+
+#define ONENAND_ENV_END   (CONFIG_SYS_MONITOR_LEN + CONFIG_ENV_SIZE)
+
 char *env_name_spec = "OneNAND";
 
 #ifdef ENV_IS_EMBEDDED
@@ -58,23 +68,31 @@ uchar env_get_char_spec(int index)
 
 void env_relocate_spec(void)
 {
-	unsigned long env_addr;
-	int use_default = 0;
+	unsigned long env_addr = CONFIG_SYS_MONITOR_LEN;
+	int use_default = 1;
 	size_t retlen;
+	int blocksize = onenand_mtd.erasesize;
 
-	env_addr = CONFIG_ENV_ADDR;
+	/* Find environment block. */
+	while (onenand_mtd.writesize && (env_addr < ONENAND_ENV_END)) {
+		if (onenand_block_isbad(&onenand_mtd, env_addr)) {
+			printf("Skip bad block@0x%x\n", (u32)env_addr);
+			env_addr += blocksize;
+			continue;
+		}
 
-	/* Check OneNAND exist */
-	if (onenand_mtd.writesize)
 		/* Ignore read fail */
 		onenand_read(&onenand_mtd, env_addr, onenand_mtd.writesize,
 			     &retlen, (u_char *) env_ptr);
-	else
-		onenand_mtd.writesize = MAX_ONENAND_PAGESIZE;
 
-	if (crc32(0, env_ptr->data, ONENAND_ENV_SIZE(onenand_mtd)) !=
-	    env_ptr->crc)
-		use_default = 1;
+		if (crc32(0, env_ptr->data, ONENAND_ENV_SIZE(onenand_mtd)) ==
+		    env_ptr->crc) {
+			printf("OneNAND: Read environment from 0x%x\n", (u32)env_addr);
+			use_default = 0;
+			break;
+		}
+		env_addr += blocksize;
+	}
 
 	if (use_default) {
 		memcpy(env_ptr->data, default_environment,
@@ -89,31 +107,59 @@ void env_relocate_spec(void)
 
 int saveenv(void)
 {
-	unsigned long env_addr = CONFIG_ENV_ADDR;
+	unsigned long env_addr = 0, badblocklen = 0;
 	struct erase_info instr = {
-		.callback	= NULL,
+		 .callback	= NULL,
 	};
 	size_t retlen;
+	int blocksize = onenand_mtd.erasesize;
 
-	instr.len = CONFIG_ENV_SIZE;
-	instr.addr = env_addr;
-	instr.mtd = &onenand_mtd;
-	if (onenand_erase(&onenand_mtd, &instr)) {
-		printf("OneNAND: erase failed at 0x%08lx\n", env_addr);
+	/* Skip bootloader area. */
+	while (env_addr < CONFIG_SYS_MONITOR_LEN + badblocklen) {
+		if (onenand_block_isbad(&onenand_mtd, env_addr))
+			badblocklen += blocksize;
+		env_addr += blocksize;
+	}
+
+	/* Skip any bad blocks */
+	while (onenand_block_isbad(&onenand_mtd, env_addr)) {
+		printf("Skip bad block@0x%x\n", (u32)env_addr);
+		env_addr += blocksize;
+	}
+
+	 /* update crc */
+	 env_ptr->crc =
+	     crc32(0, env_ptr->data, ONENAND_ENV_SIZE(onenand_mtd));
+
+retry:
+	if (env_addr >= ONENAND_ENV_END) {
+		printf("OneNAND: Saving environment failed\n");
 		return 1;
 	}
 
-	/* update crc */
-	env_ptr->crc =
-	    crc32(0, env_ptr->data, ONENAND_ENV_SIZE(onenand_mtd));
+	instr.len = blocksize;
+	instr.addr = env_addr;
+	instr.mtd = &onenand_mtd;
 
-	if (onenand_write(&onenand_mtd, env_addr, onenand_mtd.writesize, &retlen,
-	     (u_char *) env_ptr)) {
-		printf("OneNAND: write failed at 0x%08x\n", instr.addr);
-		return 2;
+	/* Erase the environment block */
+	while (onenand_erase(&onenand_mtd, &instr)) {
+		onenand_block_markbad(&onenand_mtd, env_addr);
+		env_addr += blocksize;
+		instr.addr += blocksize;
 	}
 
-	return 0;
+	/* Write the environment variables*/
+	if (onenand_write(&onenand_mtd, env_addr, onenand_mtd.writesize,
+	    &retlen, (u_char *) env_ptr)) {
+		/* Mark block bad and try the next one */
+		onenand_block_markbad(&onenand_mtd, env_addr);
+		env_addr += blocksize;
+		goto retry;
+	}
+
+	printf("OneNAND: Saved environment to 0x%x\n", (u32)env_addr);
+
+	 return 0;
 }

 int env_init(void)
diff --git a/include/configs/apollon.h b/include/configs/apollon.h
index 2e8198f..cf41e01 100644
--- a/include/configs/apollon.h
+++ b/include/configs/apollon.h
@@ -75,7 +75,7 @@
 /*
  * Size of malloc() pool
  */
-#define	CONFIG_ENV_SIZE SZ_128K	/* Total Size of Environment Sector */
+#define	CONFIG_ENV_SIZE SZ_256K	/* Total Size of Environment Sector */
 #define	CONFIG_SYS_MALLOC_LEN	(CONFIG_ENV_SIZE + SZ_1M)
 /* bytes reserved for initial data */
 #define	CONFIG_SYS_GBL_DATA_SIZE	128
diff --git a/include/onenand_uboot.h b/include/onenand_uboot.h
index 49da9d0..dbac25b 100644
--- a/include/onenand_uboot.h
+++ b/include/onenand_uboot.h
@@ -38,6 +38,10 @@ extern int onenand_erase(struct mtd_info *mtd, struct erase_info *instr);
 
 extern char *onenand_print_device_info(int device, int version);
 
+extern int onenand_block_isbad(struct mtd_info *mtd, loff_t ofs);
+
+extern int onenand_block_markbad(struct mtd_info *mtd, loff_t ofs);
+
 /* S3C64xx */
 extern void s3c64xx_onenand_init(struct mtd_info *);
 extern void s3c64xx_set_width_regs(struct onenand_chip *);

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-04-12  7:55       ` apgmoorthy
@ 2009-04-23 22:39         ` Scott Wood
  2009-04-24  5:05           ` Amit Kumar Sharma
  0 siblings, 1 reply; 25+ messages in thread
From: Scott Wood @ 2009-04-23 22:39 UTC (permalink / raw)
  To: u-boot

apgmoorthy wrote:
> Hi Scott, 
> 
> On Tuesday, March 31, 2009 4:04 AM Scott Wood Wrote :
> 
>>> Note that there are a couple of board files (apollon and nmdk8815) that use the OneNAND loader 
>>> that do not define CONFIG_SYS_MONITOR_LEN.  I've added the maintainers to the Cc: list.
> 
> CONFIG_SYS_MONITOR_LEN is not defined in include/configs/apollon.h as of now.
> 
> This is done by the Post : [U-Boot] [PATCH 2/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
> 
> What do you feel about getting this one inside u-boot-nand-flash ?

Such a change should go through the board maintainer, who knows better 
what the actual length is.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-04-23 22:39         ` Scott Wood
@ 2009-04-24  5:05           ` Amit Kumar Sharma
  0 siblings, 0 replies; 25+ messages in thread
From: Amit Kumar Sharma @ 2009-04-24  5:05 UTC (permalink / raw)
  To: u-boot

Hi Kyungmin,
----- Original Message ----- 
From: "Scott Wood" <scottwood@freescale.com>
To: "apgmoorthy" <moorthy.apg@samsung.com>
Cc: <u-boot@lists.denx.de>; <rubini@unipv.it>
Sent: Friday, April 24, 2009 4:09 AM
Subject: Re: [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read 
CONFIG_SYS_MONITOR_LEN


> apgmoorthy wrote:
>> Hi Scott,
>>
>> On Tuesday, March 31, 2009 4:04 AM Scott Wood Wrote :
>>
>>>> Note that there are a couple of board files (apollon 
>>>> and nmdk8815) that use the OneNAND loader
>>>> that do not define CONFIG_SYS_MONITOR_LEN.  I've added 
>>>> the maintainers to the Cc: list.
>>
>> CONFIG_SYS_MONITOR_LEN is not defined in 
>> include/configs/apollon.h as of now.
>>
>> This is done by the Post : [U-Boot] [PATCH 2/2] Fix 
>> OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
>>
>> What do you feel about getting this one inside 
>> u-boot-nand-flash ?
>
> Such a change should go through the board maintainer, who 
> knows better
> what the actual length is.
>
> -Scott

Please provide your comments,
Are you using Apollon board now days.

Thanks
Amit 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Bad Block Capable environment for OneNAND
  2009-04-13 13:04       ` [U-Boot] [PATCH] Bad Block Capable environment for OneNAND apgmoorthy
@ 2009-04-28 23:11         ` Scott Wood
  0 siblings, 0 replies; 25+ messages in thread
From: Scott Wood @ 2009-04-28 23:11 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 13, 2009 at 06:34:52PM +0530, apgmoorthy wrote:
> +	/* Protect boot-loader and environment variables from badblock testing */
> +	while (start < CONFIG_SYS_MONITOR_LEN + CONFIG_ENV_SIZE + badblocklen) {
> +		if (mtd->block_isbad(&onenand_mtd, start))
> +			badblocklen += blocksize;
> +		start += blocksize;
> +	}

You need to start at zero to determine the number of bad blocks used by
the boot image -- otherwise, if start points in the middle of the image,
it is assuming that there were no bad blocks prior to that location.

Plus, if "start" was not block-aligned (is that allowed?), this should
round up to the next block rather than jumping to the middle of the next
block.

> +#define ONENAND_ENV_END   (CONFIG_SYS_MONITOR_LEN + CONFIG_ENV_SIZE)
> +
>  char *env_name_spec = "OneNAND";
>  
>  #ifdef ENV_IS_EMBEDDED
> @@ -58,23 +68,31 @@ uchar env_get_char_spec(int index)
>  
>  void env_relocate_spec(void)
>  {
> -	unsigned long env_addr;
> -	int use_default = 0;
> +	unsigned long env_addr = CONFIG_SYS_MONITOR_LEN;

As above, you have to check for bad blocks from the beginning.  You do
this in saveenv...

> +	int use_default = 1;
>  	size_t retlen;
> +	int blocksize = onenand_mtd.erasesize;
>  
> -	env_addr = CONFIG_ENV_ADDR;
> +	/* Find environment block. */
> +	while (onenand_mtd.writesize && (env_addr < ONENAND_ENV_END)) {
> +		if (onenand_block_isbad(&onenand_mtd, env_addr)) {
> +			printf("Skip bad block at 0x%x\n", (u32)env_addr);

Why not just use %lx?

> +	 /* update crc */
> +	 env_ptr->crc =
> +	     crc32(0, env_ptr->data, ONENAND_ENV_SIZE(onenand_mtd));

There's an extra space in the above indent.

> +	 return 0;

Here too.

<record state="broken">
Any consolidation we can do between env_nand and env_onenand would be
appreciated... even if they don't share code yet, let's keep the logic
similar.
</record>

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-04-24 15:21   ` Scott Wood
@ 2009-04-24 21:47     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 25+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-04-24 21:47 UTC (permalink / raw)
  To: u-boot

On 10:21 Fri 24 Apr     , Scott Wood wrote:
> On Fri, Apr 24, 2009 at 02:57:52PM +0900, Kyungmin Park wrote:
> > Actually, I don't like the CONFIG_SYS_MONITOR_LEN approaches, now you
> > are consider the bad block at 1.
> > But we can also consider the bad block 2, if there two consecutive 2
> > bad block at block 1, 2, we should define the CONFIG_SYS_MONITOR_LEN
> > as 128KiB * 4 and reserved block block 4 always even though we use 2
> > blocks.
> 
> There are two separate constants -- the length of data excluding bad
> blocks, and the size of the region set aside for that data including bad
> blocks.  CONFIG_SYS_MONITOR_LEN is the former.
we may need to detect the u-boot.bin size dynamicly instead of staticly

as Scott NACK

Best Regards,
J.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-04-24  5:57 ` [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN Kyungmin Park
@ 2009-04-24 15:21   ` Scott Wood
  2009-04-24 21:47     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 25+ messages in thread
From: Scott Wood @ 2009-04-24 15:21 UTC (permalink / raw)
  To: u-boot

On Fri, Apr 24, 2009 at 02:57:52PM +0900, Kyungmin Park wrote:
> Actually, I don't like the CONFIG_SYS_MONITOR_LEN approaches, now you
> are consider the bad block at 1.
> But we can also consider the bad block 2, if there two consecutive 2
> bad block at block 1, 2, we should define the CONFIG_SYS_MONITOR_LEN
> as 128KiB * 4 and reserved block block 4 always even though we use 2
> blocks.

There are two separate constants -- the length of data excluding bad
blocks, and the size of the region set aside for that data including bad
blocks.  CONFIG_SYS_MONITOR_LEN is the former.

We may need to do something special when writing back the environment to
find its location if there were any bad blocks, though.

> Right, we should consider bad block at IPL, so my recommendation is
> IPL + u-boot should be fit at block 0

We wouldn't be having this discussion if someone didn't have a u-boot
that didn't fit in block 0.

> 128KiB at OneNAND, 256KiB at Flex-OneNAND. In case of apollon. we
> should reduce the size as 128KiB for OneNAND side. If the IPL + u-boot
> size under 128KiB, Flex-OneNAND also can boot it.

For comparison, powerpc u-boots are pretty much never less than 128KiB,
and with the NAND subsystem are typically larger than 256KiB.  We may
need to boot from OneNAND some day.

> For simplicity How about to use block scheme? we always use block 0
> for boot (IPL + u-boot) whatever block size.

NACK.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
       [not found] <22617689.32781240550892435.JavaMail.weblogic@epml08>
@ 2009-04-24  5:57 ` Kyungmin Park
  2009-04-24 15:21   ` Scott Wood
  0 siblings, 1 reply; 25+ messages in thread
From: Kyungmin Park @ 2009-04-24  5:57 UTC (permalink / raw)
  To: u-boot

Hi,

On Fri, Apr 24, 2009 at 2:28 PM, AYYANARPONNUSAMY GANGHEYAMOORTHY
<moorthy.apg@samsung.com> wrote:
> Hi ?Scott,
> On ?Apr 24, 2009 07:39 (GMT+09:00) Scott Wood<scottwood@freescale.com> wrote
>
>>>apgmoorthy wrote:
>>> This is done by the Post : [U-Boot] [PATCH 2/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
>>>
>>> What do you feel about getting this one inside u-boot-nand-flash ?
>
>>Such a change should go through the board maintainer, who knows better
>>what the actual length is.
> ? ? ? ?- Correct ?, I got your point.
>
> Hi Kyungmin,
> ? ? ? Can you please make a call on this , for apollon and get CONFIG_SYS_MONITOR_LEN in.
>

Actually, I don't like the CONFIG_SYS_MONITOR_LEN approaches, now you
are consider the bad block at 1.
But we can also consider the bad block 2, if there two consecutive 2
bad block at block 1, 2, we should define the CONFIG_SYS_MONITOR_LEN
as 128KiB * 4 and reserved block block 4 always even though we use 2
blocks.

Right, we should consider bad block at IPL, so my recommendation is
IPL + u-boot should be fit at block 0
128KiB at OneNAND, 256KiB at Flex-OneNAND. In case of apollon. we
should reduce the size as 128KiB for OneNAND side. If the IPL + u-boot
size under 128KiB, Flex-OneNAND also can boot it.

For simplicity How about to use block scheme? we always use block 0
for boot (IPL + u-boot) whatever block size.

Thank you,
Kyungmin Park

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
  2009-03-09 14:15 Rohit Hagargundgi
@ 2009-03-09 19:42 ` Scott Wood
  0 siblings, 0 replies; 25+ messages in thread
From: Scott Wood @ 2009-03-09 19:42 UTC (permalink / raw)
  To: u-boot

On Mon, Mar 09, 2009 at 07:45:40PM +0530, Rohit Hagargundgi wrote:
> +	/* Check for invalid block mark*/
> +	if (page < 2 && (onenand_readw(THIS_ONENAND(ONENAND_SPARERAM)) != 0xffff))
> +		return 1;

Line length.

> -	int page, offset = 0;
> +	int block = 0, page = ONENAND_START_PAGE, offset = 0;
>  	int pagesize = ONENAND_PAGE_SIZE;
> +	int nblocks = CONFIG_SYS_MONITOR_LEN / ONENAND_BLOCK_SIZE;

Shouldn't nblocks be rounded up?

>  	/* MLC OneNAND has 4KiB page size */
> -	if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY)))
> +	if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY))) {
>  		pagesize <<= 1;
> +		nblocks = (nblocks + 1) >> 1;
> +	}

Hmm, might be clearer to just do this:

if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY))) {
	pagesize = 4096;
	pageshift = 12;
} else {
	pagesize = 2048;
	pageshift = 11;
}

nblocks = (CONFIG_SYS_MONITOR_LEN + pagesize - 1) >> pageshift;

ONENAND_PAGE_SIZE as a constant should go away since it's not always
true.

>  	/* NOTE: you must read page from page 1 of block 0 */
>  	/* read the block page by page*/
> -	for (page = ONENAND_START_PAGE;
> -	    page < ONENAND_PAGES_PER_BLOCK; page++) {
> -
> -		onenand_read_page(0, page, buf + offset, pagesize);
> -		offset += pagesize;
> +	for (; block < nblocks; block++) {
> +		for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
> +			if (onenand_read_page(block, page, buf + offset, pagesize)) {
> +				/* This block is bad. Skip it and read next block */
> +				nblocks++;
> +				break;
> +			}
> +			offset += pagesize;

If you find a bad block marker in the second page of a block, shouldn't
you rewind offset to the beginning of the block?

BTW, if we're going to have "onenand_readw" and "onenand_writew"
wrappers, can we make them useful by combining them with THIS_ONENAND?

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN
@ 2009-03-09 14:15 Rohit Hagargundgi
  2009-03-09 19:42 ` Scott Wood
  0 siblings, 1 reply; 25+ messages in thread
From: Rohit Hagargundgi @ 2009-03-09 14:15 UTC (permalink / raw)
  To: u-boot

Currently OneNAND initial program loader (ipl) reads only block 0 ie 128KB.
However, u-boot image for apollon board is 195KB making the board
unbootable with OneNAND.

Fix ipl to read CONFIG_SYS_MONITOR_LEN.
CONFIG_SYS_MONITOR_LEN macro holds the U-Boot image size.

Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
---
 onenand_ipl/onenand_read.c |   27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c
index 6d04943..c3b63ed 100644
--- a/onenand_ipl/onenand_read.c
+++ b/onenand_ipl/onenand_read.c
@@ -72,6 +72,10 @@ static inline int onenand_read_page(ulong block, ulong page,
 	while (!(READ_INTERRUPT() & ONENAND_INT_READ))
 		continue;
 
+	/* Check for invalid block mark*/
+	if (page < 2 && (onenand_readw(THIS_ONENAND(ONENAND_SPARERAM)) != 0xffff))
+		return 1;
+
 #ifdef __HAVE_ARCH_MEMCPY32
 	/* 32 bytes boundary memory copy */
 	memcpy32(buf, base, pagesize);
@@ -87,6 +91,7 @@ static inline int onenand_read_page(ulong block, ulong page,
 
 #define ONENAND_START_PAGE		1
 #define ONENAND_PAGES_PER_BLOCK		64
+#define ONENAND_BLOCK_SIZE	(ONENAND_PAGES_PER_BLOCK * ONENAND_PAGE_SIZE)
 
 /**
  * onenand_read_block - Read a block data to buf
@@ -94,20 +99,28 @@ static inline int onenand_read_page(ulong block, ulong page,
  */
 int onenand_read_block0(unsigned char *buf)
 {
-	int page, offset = 0;
+	int block = 0, page = ONENAND_START_PAGE, offset = 0;
 	int pagesize = ONENAND_PAGE_SIZE;
+	int nblocks = CONFIG_SYS_MONITOR_LEN / ONENAND_BLOCK_SIZE;
 
 	/* MLC OneNAND has 4KiB page size */
-	if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY)))
+	if (onenand_readw(THIS_ONENAND(ONENAND_REG_TECHNOLOGY))) {
 		pagesize <<= 1;
+		nblocks = (nblocks + 1) >> 1;
+	}
 
 	/* NOTE: you must read page from page 1 of block 0 */
 	/* read the block page by page*/
-	for (page = ONENAND_START_PAGE;
-	    page < ONENAND_PAGES_PER_BLOCK; page++) {
-
-		onenand_read_page(0, page, buf + offset, pagesize);
-		offset += pagesize;
+	for (; block < nblocks; block++) {
+		for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
+			if (onenand_read_page(block, page, buf + offset, pagesize)) {
+				/* This block is bad. Skip it and read next block */
+				nblocks++;
+				break;
+			}
+			offset += pagesize;
+		}
+		page = 0;
 	}
 
 	return 0;

^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2009-04-28 23:11 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-23  9:02 [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN apgmoorthy
2009-03-23 21:25 ` Scott Wood
2009-03-23 22:09   ` Wolfgang Denk
2009-03-23 22:22     ` Scott Wood
2009-03-23 22:46       ` Wolfgang Denk
2009-03-25  4:13         ` Amit Kumar Sharma
2009-03-25 15:49   ` apgmoorthy
2009-03-27  9:15   ` apgmoorthy
2009-03-30 22:33     ` Scott Wood
2009-03-31  4:12       ` Amit Kumar Sharma
2009-04-12  7:55       ` apgmoorthy
2009-04-23 22:39         ` Scott Wood
2009-04-24  5:05           ` Amit Kumar Sharma
2009-04-13 13:04       ` [U-Boot] [PATCH] Bad Block Capable environment for OneNAND apgmoorthy
2009-04-28 23:11         ` Scott Wood
2009-03-31 16:08     ` [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN Alessandro Rubini
2009-03-31 16:13       ` Scott Wood
2009-04-01  4:10         ` Amit Kumar Sharma
2009-04-01 21:55           ` Scott Wood
2009-04-06  6:05   ` [U-Boot] OneNand support broken for apollon apgmoorthy
     [not found] <22617689.32781240550892435.JavaMail.weblogic@epml08>
2009-04-24  5:57 ` [U-Boot] [PATCH 1/2] Fix OneNAND ipl to read CONFIG_SYS_MONITOR_LEN Kyungmin Park
2009-04-24 15:21   ` Scott Wood
2009-04-24 21:47     ` Jean-Christophe PLAGNIOL-VILLARD
  -- strict thread matches above, loose matches on Subject: below --
2009-03-09 14:15 Rohit Hagargundgi
2009-03-09 19:42 ` Scott Wood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.