[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Rohit Hagargundgi]

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

Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
---
  onenand_ipl/onenand_ipl.h  |    1 +
  onenand_ipl/onenand_read.c |   29 +++++++++++++++++++++--------
  2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/onenand_ipl/onenand_ipl.h b/onenand_ipl/onenand_ipl.h
index 3387998..f96e88a 100644
--- a/onenand_ipl/onenand_ipl.h
+++ b/onenand_ipl/onenand_ipl.h
@@ -24,6 +24,7 @@
  #include <linux/mtd/onenand_regs.h>

  #define ONENAND_BLOCK_SIZE              2048
+#define ONENAND_BOOTLOADER_SIZE		0x40000

  #ifndef CONFIG_SYS_PRINTF
  #define printf(format, args...)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c
index 6d04943..33c66ff 100644
--- a/onenand_ipl/onenand_read.c
+++ b/onenand_ipl/onenand_read.c
@@ -72,6 +72,13 @@ 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) {
+		unsigned int mark = onenand_readw(THIS_ONENAND(ONENAND_SPARERAM));
+		if (mark != 0xffff)
+			return 1;
+	}
+
  #ifdef __HAVE_ARCH_MEMCPY32
  	/* 32 bytes boundary memory copy */
  	memcpy32(buf, base, pagesize);
@@ -94,21 +101,27 @@ 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, offset = 0;
  	int pagesize = ONENAND_PAGE_SIZE;
+	int nblocks = ONENAND_BOOTLOADER_SIZE / (ONENAND_PAGES_PER_BLOCK * ONENAND_PAGE_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 >>= 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 (page = ONENAND_START_PAGE; block < nblocks; page = 0, 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;
+		}

  	return 0;
  }
-- 
1.6.0.6

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Jean-Christophe PLAGNIOL-VILLARD]

On 13:03 Thu 26 Feb     , Rohit Hagargundgi wrote:
> Currently OneNAND initial program loader (ipl) reads only block 0.
> However, u-boot image for apollon board is 195KB making the board
> unbootable with OneNAND.
> Fix ipl to read 256KB.
> 
> Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
> ---
>   onenand_ipl/onenand_ipl.h  |    1 +
>   onenand_ipl/onenand_read.c |   29 +++++++++++++++++++++--------
>   2 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/onenand_ipl/onenand_ipl.h b/onenand_ipl/onenand_ipl.h
> index 3387998..f96e88a 100644
> --- a/onenand_ipl/onenand_ipl.h
> +++ b/onenand_ipl/onenand_ipl.h
> @@ -24,6 +24,7 @@
>   #include <linux/mtd/onenand_regs.h>
> 
>   #define ONENAND_BLOCK_SIZE              2048
> +#define ONENAND_BOOTLOADER_SIZE		0x40000
why hardcoded value?
> 
>   #ifndef CONFIG_SYS_PRINTF
>   #define printf(format, args...)
> diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c
> index 6d04943..33c66ff 100644
> --- a/onenand_ipl/onenand_read.c
> +++ b/onenand_ipl/onenand_read.c
> @@ -72,6 +72,13 @@ 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) {
> +		unsigned int mark = onenand_readw(THIS_ONENAND(ONENAND_SPARERAM));
please add a empty line

and why not do this
	if (page < 2 && (onenand_readw(THIS_ONENAND(ONENAND_SPARERAM)) != 0xffff))
		return 1;

> +		if (mark != 0xffff)
> +			return 1;
> +	}
> +
>   #ifdef __HAVE_ARCH_MEMCPY32
>   	/* 32 bytes boundary memory copy */
>   	memcpy32(buf, base, pagesize);
> @@ -94,21 +101,27 @@ 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, offset = 0;
>   	int pagesize = ONENAND_PAGE_SIZE;
> +	int nblocks = ONENAND_BOOTLOADER_SIZE / (ONENAND_PAGES_PER_BLOCK * ONENAND_PAGE_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 >>= 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 (page = ONENAND_START_PAGE; block < nblocks; page = 0, block++)
please add the {} and move the page = 0 to the second 'for' it will be easier
to read and understand
> +		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;
> +		}

Best Regards,
J.

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Rohit Hagargundgi]

Hi,

Jean-Christophe PLAGNIOL-VILLARD wrote:
>>   #define ONENAND_BLOCK_SIZE              2048
>> +#define ONENAND_BOOTLOADER_SIZE		0x40000
> why hardcoded value?

Is it possible to get image size instead of hard value.
Then we can read exactly u-boot image size on boot up.

>> +	/* Check for invalid block mark*/
>> +	if (page < 2) {
>> +		unsigned int mark = onenand_readw(THIS_ONENAND(ONENAND_SPARERAM));
> please add a empty line
> 
> and why not do this
> 	if (page < 2 && (onenand_readw(THIS_ONENAND(ONENAND_SPARERAM)) != 0xffff))
> 		return 1;

okay.

>> -	for (page = ONENAND_START_PAGE;
>> -	    page < ONENAND_PAGES_PER_BLOCK; page++) {
>> -
>> -		onenand_read_page(0, page, buf + offset, pagesize);
>> -		offset += pagesize;
>> -	}
>> +	for (page = ONENAND_START_PAGE; block < nblocks; page = 0, block++)
> please add the {} and move the page = 0 to the second 'for' it will be easier
> to read and understand

okay.

Thanks,
Rohit

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:01 Thu 26 Feb     , Rohit Hagargundgi wrote:
> Hi,
>
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>   #define ONENAND_BLOCK_SIZE              2048
>>> +#define ONENAND_BOOTLOADER_SIZE		0x40000
>> why hardcoded value?
>
> Is it possible to get image size instead of hard value.
> Then we can read exactly u-boot image size on boot up.
as point out Wolfgang you can use the MONITOR_LEN.

If you really need the u-boot.bin size you will need to built it first and then
on the host calculate it and genereate a .h with its size.

But IMHO the MONITOR_LEN will give enough information evenif it's not the real size
to copy u-boot in memory

Best Regards,
J.

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Wolfgang Denk]

Dear Rohit Hagargundgi,

In message <49A68BA2.80807@samsung.com> you wrote:
> 
> Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>   #define ONENAND_BLOCK_SIZE              2048
> >> +#define ONENAND_BOOTLOADER_SIZE		0x40000
> > why hardcoded value?
> 
> Is it possible to get image size instead of hard value.
> Then we can read exactly u-boot image size on boot up.

Use CONFIG_SYS_MONITOR_LEN as I suggested in my previous message.


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
The philosophy exam was a piece of cake  -  which  was  a  bit  of  a
surprise, actually, because I was expecting some questions on a sheet
of paper.

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Wolfgang Denk]

Dear Rohit Hagargundgi,

In message <49A645E0.9060608@samsung.com> you wrote:
> Currently OneNAND initial program loader (ipl) reads only block 0.
> However, u-boot image for apollon board is 195KB making the board
> unbootable with OneNAND.
> Fix ipl to read 256KB.

What if there is a board that enables additional features and the size
grows above 256 kB?

>   #include <linux/mtd/onenand_regs.h>
> 
>   #define ONENAND_BLOCK_SIZE              2048
> +#define ONENAND_BOOTLOADER_SIZE		0x40000

Why do we need a new #define here. CONFIG_SYS_MONITOR_LEN already
contains the U-Boot image size, and is configurable for each board.

Please drop this #define here and substitute ONENAND_BOOTLOADER_SIZE
by CONFIG_SYS_MONITOR_LEN in the rest of the patch.

Thanks.

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
It's not an optical illusion, it just looks like one.   -- Phil White

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Kyungmin Park]

Hi,

In the previous mail, I also solve this issue. multi-block read for IPL

http://www.mail-archive.com/u-boot at lists.denx.de/msg04641.html

Of course, it's not consider the Flex-OneNAND, please apply this one
first and then fix it for Flex-OneNAND.

How do you think?

Thank you,
Kyungmin Park

On Thu, Feb 26, 2009 at 5:23 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Rohit Hagargundgi,
>
> In message <49A645E0.9060608@samsung.com> you wrote:
>> Currently OneNAND initial program loader (ipl) reads only block 0.
>> However, u-boot image for apollon board is 195KB making the board
>> unbootable with OneNAND.
>> Fix ipl to read 256KB.
>
> What if there is a board that enables additional features and the size
> grows above 256 kB?
>
>> ? #include <linux/mtd/onenand_regs.h>
>>
>> ? #define ONENAND_BLOCK_SIZE ? ? ? ? ? ? ?2048
>> +#define ONENAND_BOOTLOADER_SIZE ? ? ? ? ? ? ?0x40000
>
> Why do we need a new #define here. CONFIG_SYS_MONITOR_LEN already
> contains the U-Boot image size, and is configurable for each board.
>
> Please drop this #define here and substitute ONENAND_BOOTLOADER_SIZE
> by CONFIG_SYS_MONITOR_LEN in the rest of the patch.
>
> Thanks.
>
> 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
> It's not an optical illusion, it just looks like one. ? -- Phil White
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Rohit Hagargundgi]

Hi,

Kyungmin Park wrote:
> Hi,
> 
> In the previous mail, I also solve this issue. multi-block read for IPL
> 
> http://www.mail-archive.com/u-boot at lists.denx.de/msg04641.html

oh, okay. So this merge first.

To Scott, Wolfgang - can you please apply this patch.

Also environment block is affected by larger ipl size.
I will repost as per comments.

Thanks and regards,
Rohit

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Rohit Hagargundgi]

Hi,

Here is the updated patch.

Thanks,
Rohit

Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
---
  include/configs/apollon.h  |    1 +
  onenand_ipl/onenand_read.c |   27 ++++++++++++++++++++-------
  2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/configs/apollon.h b/include/configs/apollon.h
index dff47fc..2e8198f 100644
--- a/include/configs/apollon.h
+++ b/include/configs/apollon.h
@@ -254,6 +254,7 @@

  /* OneNAND boot, OneNAND has CS0, NOR boot ONeNAND has CS2 */
  #define	CONFIG_SYS_ONENAND_BASE	0x00000000
+#define CONFIG_SYS_MONITOR_LEN		SZ_256K	/* U-Boot image size */
  #define	CONFIG_ENV_IS_IN_ONENAND	1
  #define CONFIG_ENV_ADDR		0x00020000

diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c
index 6d04943..6782eac 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);
@@ -94,20 +98,29 @@ 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, offset = 0;
  	int pagesize = ONENAND_PAGE_SIZE;
+	int nblocks = CONFIG_SYS_MONITOR_LEN / (ONENAND_PAGES_PER_BLOCK * ONENAND_PAGE_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;
+	page = ONENAND_START_PAGE;
+	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;
-- 
1.6.0.6

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Kyungmin Park]

On Thu, Mar 5, 2009 at 12:09 AM, Rohit Hagargundgi <h.rohit@samsung.com> wrote:
> Hi,
>
> Here is the updated patch.
>
> Thanks,
> Rohit
>
> Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
> ---
> ?include/configs/apollon.h ?| ? ?1 +
> ?onenand_ipl/onenand_read.c | ? 27 ++++++++++++++++++++-------
> ?2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/include/configs/apollon.h b/include/configs/apollon.h
> index dff47fc..2e8198f 100644
> --- a/include/configs/apollon.h
> +++ b/include/configs/apollon.h
> @@ -254,6 +254,7 @@
>
> ?/* OneNAND boot, OneNAND has CS0, NOR boot ONeNAND has CS2 */
> ?#define ? ? ? CONFIG_SYS_ONENAND_BASE 0x00000000
> +#define CONFIG_SYS_MONITOR_LEN ? ? ? ? SZ_256K /* U-Boot image size */
> ?#define ? ? ? CONFIG_ENV_IS_IN_ONENAND ? ? ? ?1
> ?#define CONFIG_ENV_ADDR ? ? ? ? ? ? ? 0x00020000
>
> diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c
> index 6d04943..6782eac 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;
> +

No need to check invalid block. Note that block 0 is always good
block. no exception.
Now you assume block 1 can be invalid block. If true just skip it.
when update bootloader at u-boot or kernel.
there's bad block at block 1. it will skip write. it means bootloader
are located at block 0 and block 2.

> ?#ifdef __HAVE_ARCH_MEMCPY32
> ? ? ? ?/* 32 bytes boundary memory copy */
> ? ? ? ?memcpy32(buf, base, pagesize);
> @@ -94,20 +98,29 @@ 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, offset = 0;
> ? ? ? ?int pagesize = ONENAND_PAGE_SIZE;
> + ? ? ? int nblocks = CONFIG_SYS_MONITOR_LEN / (ONENAND_PAGES_PER_BLOCK * ONENAND_PAGE_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;
> + ? ? ? page = ONENAND_START_PAGE;
> + ? ? ? 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;

NAK, please use previous one as I sent.
In Flex-OneNAND block 0 has 256KiB. need to handle at here how many
blocks are needed.
I also want to use CONFIG_ONENAND_END_BLOCK since we don't know which
OneNAND or Flex-OneNAND are attached to apollon board.

Thank you,
Kyungmin Park

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Rohit Hagargundgi]

Hi,

Kyungmin Park wrote:
>>
>> +       /* Check for invalid block mark*/
>> +       if (page < 2 && (onenand_readw(THIS_ONENAND(ONENAND_SPARERAM)) != 0xffff))
>> +               return 1;
>> +
> 
> No need to check invalid block. Note that block 0 is always good
> block. no exception.

Correct. block 0 is guaranteed to be good.

> Now you assume block 1 can be invalid block. If true just skip it.
> when update bootloader at u-boot or kernel.
> there's bad block at block 1. it will skip write. it means bootloader
> are located at block 0 and block 2.

yes, block 1 is invalid so block 0 and block 2 store bootloader.
so in ipl, we need to check invalid mark. block 1 is detected bad.
block 1 is skipped and block 2 is read for bootloader.

>> +       int nblocks = CONFIG_SYS_MONITOR_LEN / (ONENAND_PAGES_PER_BLOCK * ONENAND_PAGE_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;
>> +       }

assuming page size of 2KB, nblocks is initialised to 2.
for 4KB paged devices (like Flex-OneNAND), nblocks gets halved ie 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;
>> +       page = ONENAND_START_PAGE;
>> +       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;
> 
> NAK, please use previous one as I sent.
> In Flex-OneNAND block 0 has 256KiB. need to handle at here how many
> blocks are needed.

this is taken care above.

> I also want to use CONFIG_ONENAND_END_BLOCK since we don't know which
> OneNAND or Flex-OneNAND are attached to apollon board.

okay. but why not use a variable instead.

Thanks,
Rohit

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Wolfgang Denk]

Dear Rohit Hagargundgi,

In message <49AE99C0.9040400@samsung.com> you wrote:
> Hi,
> 
> Here is the updated patch.
> 
> Thanks,
> Rohit

This is not an acceptable commit message.

Please include a reasonable commit message that explains what you are
doinbg and why.

> Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
> ---

Comments must go below this "---" line, not above.

>   include/configs/apollon.h  |    1 +
>   onenand_ipl/onenand_read.c |   27 ++++++++++++++++++++-------
>   2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/include/configs/apollon.h b/include/configs/apollon.h
> index dff47fc..2e8198f 100644
> --- a/include/configs/apollon.h
> +++ b/include/configs/apollon.h
> @@ -254,6 +254,7 @@
> 
>   /* OneNAND boot, OneNAND has CS0, NOR boot ONeNAND has CS2 */
>   #define	CONFIG_SYS_ONENAND_BASE	0x00000000
> +#define CONFIG_SYS_MONITOR_LEN		SZ_256K	/* U-Boot image size */
>   #define	CONFIG_ENV_IS_IN_ONENAND	1
>   #define CONFIG_ENV_ADDR		0x00020000

Your patch is white-space corrupted. Please fix your mailer settings.

>   int onenand_read_block0(unsigned char *buf)
>   {
> -	int page, offset = 0;
> +	int block = 0, page, offset = 0;

Please don;t mix uninitialized and initialized variables on one line.

>   	int pagesize = ONENAND_PAGE_SIZE;
> +	int nblocks = CONFIG_SYS_MONITOR_LEN / (ONENAND_PAGES_PER_BLOCK * ONENAND_PAGE_SIZE);

Line too long.

> +	page = ONENAND_START_PAGE;
> +	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;

I always consider it a design problem when the loop limits get changed
within the loop. While legal C, it always gives me the creeps.

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
Politics:  A  strife  of  interests  masquerading  as  a  contest  of
principles. The conduct of public affairs for private advantage.
- Ambrose Bierce

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Rohit Hagargundgi]

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

Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
---
 include/configs/apollon.h  |    1 +
 onenand_ipl/onenand_read.c |   27 ++++++++++++++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/configs/apollon.h b/include/configs/apollon.h
index dff47fc..2e8198f 100644
--- a/include/configs/apollon.h
+++ b/include/configs/apollon.h
@@ -254,6 +254,7 @@
 
 /* OneNAND boot, OneNAND has CS0, NOR boot ONeNAND has CS2 */
 #define	CONFIG_SYS_ONENAND_BASE	0x00000000
+#define CONFIG_SYS_MONITOR_LEN		SZ_256K	/* U-Boot image size */
 #define	CONFIG_ENV_IS_IN_ONENAND	1
 #define CONFIG_ENV_ADDR		0x00020000
 
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;

[U-Boot] [PATCH] Fix OneNAND ipl to read 256KB

[Wolfgang Denk]

Dear Rohit Hagargundgi,

In message <200903091604.45906.h.rohit@samsung.com> you wrote:
> Currently OneNAND initial program loader (ipl) reads only block 0.
> However, u-boot image for apollon board is 195KB making the board
> unbootable with OneNAND.
> Fix ipl to read 256KB.

NAK.

The description is wrong:

The code was not changed to read 256 KB (fixed), but to read whetever
there is defined by the CONFIG_SYS_MONITOR_LEN definition.

Please change the commit messages accordingly.

Also, please split this into two separate patches: the first to
implement the feature (changes to onenand_ipl/onenand_read.c), and a
second one to fix the apollon board (changes to
include/configs/apollon.h).

Thanks.

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
All a hacker needs is a tight PUSHJ, a loose pair of UUOs, and a warm
place to shift.