[U-Boot] [PATCH] nand_spl_simple: store ecc data on the stack

[U-Boot] [PATCH] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
which is likely to contain already loaded data.
The patch saves the oob data and the ecc on the stack replacing
the fixed address in RAM.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Ilya Yanok <yanok@emcraft.com>
CC: Scott Wood <scottwood@freescale.com>
CC: Tom Rini <tom.rini@gmail.com>
CC: Simon Schwarz <simonschwarzcor@googlemail.com>
CC: Wolfgang Denk <wd@denx.de>
CC: Heiko Schocher <hs@denx.de>
---

Note: Ilya has already submitted a patch to fix this issue:

http://patchwork.ozlabs.org/patch/128018/

after discussing on ML about booting linux from SPL,
we agree about putting the data on the stack.

drivers/mtd/nand/nand_spl_simple.c |   27 ++++++---------------------
 1 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
index ed821f2..a3d1af0 100644
--- a/drivers/mtd/nand/nand_spl_simple.c
+++ b/drivers/mtd/nand/nand_spl_simple.c
@@ -145,9 +145,9 @@ static int nand_is_bad_block(int block)
 static int nand_read_page(int block, int page, uchar *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
+	u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
@@ -155,14 +155,6 @@ static int nand_read_page(int block, int page, uchar *dst)
 	uint8_t *p = dst;
 	int stat;
 
-	/*
-	 * No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	nand_command(block, page, 0, NAND_CMD_READOOB);
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 	nand_command(block, page, 0, NAND_CMD_READ0);
@@ -185,9 +177,9 @@ static int nand_read_page(int block, int page, uchar *dst)
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
+	u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
@@ -197,13 +189,6 @@ static int nand_read_page(int block, int page, void *dst)
 
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
-	/* No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		if (this->ecc.mode != NAND_ECC_SOFT)
 			this->ecc.hwctl(&mtd, NAND_ECC_READ);
-- 
1.7.5.4

[U-Boot] [PATCH] nand_spl_simple: store ecc data on the stack

[Ilya Yanok]

Hi Stefano,

thanks for posting this. There is a couple of comments below.

On 11.12.2011 21:22, Stefano Babic wrote:
> diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
> index ed821f2..a3d1af0 100644
> --- a/drivers/mtd/nand/nand_spl_simple.c
> +++ b/drivers/mtd/nand/nand_spl_simple.c
> @@ -145,9 +145,9 @@ static int nand_is_bad_block(int block)
>  static int nand_read_page(int block, int page, uchar *dst)
>  {
>  	struct nand_chip *this = mtd.priv;
> -	u_char *ecc_calc;
> -	u_char *ecc_code;
> -	u_char *oob_data;
> +	u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
> +	u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL];

Doesn't ECCTOTAL always equal to ECCSTEPS * ECCBYTES?

> +	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];

I think we can save a few bytes by placing ecc_code and oob_data into a
union. We only need oob_data to get ecc_code...

Regards, Ilya.

[U-Boot] [PATCH] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

On 12/12/2011 01:08, Ilya Yanok wrote:
> Hi Stefano,
> 
> thanks for posting this. There is a couple of comments below.
> 
> On 11.12.2011 21:22, Stefano Babic wrote:
>> diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
>> index ed821f2..a3d1af0 100644
>> --- a/drivers/mtd/nand/nand_spl_simple.c
>> +++ b/drivers/mtd/nand/nand_spl_simple.c
>> @@ -145,9 +145,9 @@ static int nand_is_bad_block(int block)
>>  static int nand_read_page(int block, int page, uchar *dst)
>>  {
>>  	struct nand_chip *this = mtd.priv;
>> -	u_char *ecc_calc;
>> -	u_char *ecc_code;
>> -	u_char *oob_data;
>> +	u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
>> +	u_char ecc_code[CONFIG_SYS_NAND_ECCTOTAL];
> 
> Doesn't ECCTOTAL always equal to ECCSTEPS * ECCBYTES?

I see. Then we can also remove CONFIG_SYS_NAND_ECCTOTAL from the
configuration files, because it is not useful. I will do in V2.

By grepping the code, the same issue is present in nand_spl/nand_boot.c,
too. This is part of the old spl code, I do not know if we have also to
fix this one. IMHO we can only fix nand_spl_simple.c, because the other
one will become obsolete and there is not probelms with current boards.

> 
>> +	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
> 
> I think we can save a few bytes by placing ecc_code and oob_data into a
> union. We only need oob_data to get ecc_code...

Checking the real values I see that ECCTOTAL is small - 8 bytes on the
TI we are currently testing, and it has not a big value on other
architectures. So I think we can avoid the union, also because this can
generate overlapping when the oob data is copied: I mean in case
nand_ecc_pos[i] is smaller as CONFIG_SYS_NAND_ECCTOTAL at:

 for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
                ecc_code[i] = oob_data[nand_ecc_pos[i]];



Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de
=====================================================================

[U-Boot] [PATCH V2] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
which is likely to contain already loaded data.
The patch saves the oob data and the ecc on the stack replacing
the fixed address in RAM.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Ilya Yanok <yanok@emcraft.com>
CC: Scott Wood <scottwood@freescale.com>
CC: Tom Rini <tom.rini@gmail.com>
CC: Simon Schwarz <simonschwarzcor@googlemail.com>
CC: Wolfgang Denk <wd@denx.de>
---

V2:
 - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
 - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c

 drivers/mtd/nand/nand_spl_simple.c     |   33 +++++++++----------------------
 include/configs/am3517_crane.h         |    2 -
 include/configs/am3517_evm.h           |    2 -
 include/configs/devkit8000.h           |    2 -
 include/configs/hawkboard.h            |    3 +-
 include/configs/omap3_beagle.h         |    2 -
 include/configs/omap3_evm.h            |    2 -
 include/configs/omap3_evm_quick_nand.h |    2 -
 8 files changed, 11 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
index ed821f2..fbc0fce 100644
--- a/drivers/mtd/nand/nand_spl_simple.c
+++ b/drivers/mtd/nand/nand_spl_simple.c
@@ -145,9 +145,9 @@ static int nand_is_bad_block(int block)
 static int nand_read_page(int block, int page, uchar *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
+	u_char ecc_code[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
@@ -155,20 +155,13 @@ static int nand_read_page(int block, int page, uchar *dst)
 	uint8_t *p = dst;
 	int stat;
 
-	/*
-	 * No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	nand_command(block, page, 0, NAND_CMD_READOOB);
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0;
+		i < (CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES); i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
 
@@ -185,9 +178,9 @@ static int nand_read_page(int block, int page, uchar *dst)
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
+	u_char ecc_code[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
@@ -197,13 +190,6 @@ static int nand_read_page(int block, int page, void *dst)
 
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
-	/* No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		if (this->ecc.mode != NAND_ECC_SOFT)
 			this->ecc.hwctl(&mtd, NAND_ECC_READ);
@@ -213,7 +199,8 @@ static int nand_read_page(int block, int page, void *dst)
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0;
+		i < (CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES); i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
 	eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 0a0c261..3558dfb 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -361,8 +361,6 @@
 #define CONFIG_SYS_NAND_ECCBYTES	3
 #define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
 						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index d44eeec..8ddfaae 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -362,8 +362,6 @@
 #define CONFIG_SYS_NAND_ECCBYTES	3
 #define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
 						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index fc7f0cd..58140df 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -346,8 +346,6 @@
 
 #define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
 						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 
diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h
index 12acb27..0084fc3 100644
--- a/include/configs/hawkboard.h
+++ b/include/configs/hawkboard.h
@@ -141,8 +141,7 @@
 #define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE /	 \
 					 CONFIG_SYS_NAND_ECCSIZE)
 #define CONFIG_SYS_NAND_OOBSIZE		64
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES *	\
-					CONFIG_SYS_NAND_ECCSTEPS)
+
 #endif /* CONFIG_SYS_USE_NAND */
 
 /*
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 970810c..2736f2a 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -428,8 +428,6 @@
 #define CONFIG_SYS_NAND_ECCBYTES	3
 #define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
 						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index 2ce3959..a82c71e 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -123,8 +123,6 @@
 #define CONFIG_SYS_NAND_ECCBYTES	3
 #define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
 						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h
index 2f879c0..caa55a4 100644
--- a/include/configs/omap3_evm_quick_nand.h
+++ b/include/configs/omap3_evm_quick_nand.h
@@ -93,8 +93,6 @@
 #define CONFIG_SYS_NAND_ECCBYTES	3
 #define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
 						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
-- 
1.7.5.4

[U-Boot] [PATCH V2] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Tue, Dec 13, 2011 at 4:30 AM, Stefano Babic <sbabic@denx.de> wrote:
> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
> which is likely to contain already loaded data.
> The patch saves the oob data and the ecc on the stack replacing
> the fixed address in RAM.

OK, I think we need some convenience defines in nand_spl_simple.c:

> + ? ? ? u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];

This is long and used in a lot of places.  Furthermore...

> ?#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?CONFIG_SYS_NAND_ECCSIZE)

Is what everyone does.  So if we replace ECCSTEPS with
(PAGE_SIZE/ECCSIZE) it gets eve longer.  We should probably #define
SYS_NAND_ECCSTEPS, SYS_NAND_ECCTOTAL up top and go with it.

-- 
Tom

[U-Boot] [PATCH V2] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

On 13/12/2011 16:58, Tom Rini wrote:
> On Tue, Dec 13, 2011 at 4:30 AM, Stefano Babic <sbabic@denx.de> wrote:
>> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
>> which is likely to contain already loaded data.
>> The patch saves the oob data and the ecc on the stack replacing
>> the fixed address in RAM.
> 
> OK, I think we need some convenience defines in nand_spl_simple.c:
> 
>> +       u_char ecc_calc[CONFIG_SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES];
> 
> This is long and used in a lot of places.  Furthermore...
> 
>>  #define CONFIG_SYS_NAND_ECCSTEPS       (CONFIG_SYS_NAND_PAGE_SIZE / \
>>                                                CONFIG_SYS_NAND_ECCSIZE)
> 
> Is what everyone does.

Right, I've missed this one - we can then drop CONFIG_SYS_NAND_ECCSTEPS
also from configuration files.

>  So if we replace ECCSTEPS with
> (PAGE_SIZE/ECCSIZE) it gets eve longer.  We should probably #define
> SYS_NAND_ECCSTEPS, SYS_NAND_ECCTOTAL up top and go with it.

I'll do in V3.

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
which is likely to contain already loaded data.
The patch saves the oob data and the ecc on the stack replacing
the fixed address in RAM.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Ilya Yanok <yanok@emcraft.com>
CC: Scott Wood <scottwood@freescale.com>
CC: Tom Rini <tom.rini@gmail.com>
CC: Simon Schwarz <simonschwarzcor@googlemail.com>
CC: Wolfgang Denk <wd@denx.de>
---
V3:
 - use local defines for CONFIG_SYS_NAND_ECCSTEPS and
   CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
 - drop CONFIG_SYS_NAND_ECCSTEPS from board config files

V2:
 - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
 - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c

 drivers/mtd/nand/nand_spl_simple.c     |   42 ++++++++++++-------------------
 include/configs/am3517_crane.h         |    4 ---
 include/configs/am3517_evm.h           |    4 ---
 include/configs/devkit8000.h           |    5 ----
 include/configs/hawkboard.h            |    5 +---
 include/configs/omap3_beagle.h         |    4 ---
 include/configs/omap3_evm.h            |    4 ---
 include/configs/omap3_evm_quick_nand.h |    4 ---
 8 files changed, 17 insertions(+), 55 deletions(-)

diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
index ed821f2..ef34991 100644
--- a/drivers/mtd/nand/nand_spl_simple.c
+++ b/drivers/mtd/nand/nand_spl_simple.c
@@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS;
 static nand_info_t mtd;
 static struct nand_chip nand_chip;
 
+#define SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
+					CONFIG_SYS_NAND_ECCSIZE)
+#define SYS_NAND_ECCTOTAL	(SYS_NAND_ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES)
+
+
 #if (CONFIG_SYS_NAND_PAGE_SIZE <= 512)
 /*
  * NAND command for small page NAND devices (512)
@@ -145,30 +150,22 @@ static int nand_is_bad_block(int block)
 static int nand_read_page(int block, int page, uchar *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[SYS_NAND_ECCTOTAL];
+	u_char ecc_code[SYS_NAND_ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
-	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	int eccsteps = SYS_NAND_ECCSTEPS;
 	uint8_t *p = dst;
 	int stat;
 
-	/*
-	 * No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	nand_command(block, page, 0, NAND_CMD_READOOB);
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0; i < (SYS_NAND_ECCTOTAL); i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
 
@@ -185,25 +182,18 @@ static int nand_read_page(int block, int page, uchar *dst)
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[SYS_NAND_ECCTOTAL];
+	u_char ecc_code[SYS_NAND_ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
-	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	int eccsteps = SYS_NAND_ECCSTEPS;
 	uint8_t *p = dst;
 	int stat;
 
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
-	/* No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		if (this->ecc.mode != NAND_ECC_SOFT)
 			this->ecc.hwctl(&mtd, NAND_ECC_READ);
@@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst)
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0; i < (SYS_NAND_ECCTOTAL); i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
-	eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	eccsteps = SYS_NAND_ECCSTEPS;
 	p = dst;
 
 	for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 0a0c261..b0dd2f0 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -359,10 +359,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index d44eeec..f797f3f 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -360,10 +360,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index fc7f0cd..6ef6a87 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -344,11 +344,6 @@
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
 
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
-
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h
index 12acb27..65b3b78 100644
--- a/include/configs/hawkboard.h
+++ b/include/configs/hawkboard.h
@@ -138,11 +138,8 @@
 #define CONFIG_SYS_NAND_BAD_BLOCK_POS	0
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	10
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE /	 \
-					 CONFIG_SYS_NAND_ECCSIZE)
 #define CONFIG_SYS_NAND_OOBSIZE		64
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES *	\
-					CONFIG_SYS_NAND_ECCSTEPS)
+
 #endif /* CONFIG_SYS_USE_NAND */
 
 /*
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 970810c..7a240bb 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -426,10 +426,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index 2ce3959..1fcb7af 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -121,10 +121,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h
index 2f879c0..362fa1d 100644
--- a/include/configs/omap3_evm_quick_nand.h
+++ b/include/configs/omap3_evm_quick_nand.h
@@ -91,10 +91,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
-- 
1.7.5.4

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Tue, Dec 13, 2011 at 10:50 AM, Stefano Babic <sbabic@denx.de> wrote:
> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
> which is likely to contain already loaded data.
> The patch saves the oob data and the ecc on the stack replacing
> the fixed address in RAM.
>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ilya Yanok <yanok@emcraft.com>
> CC: Scott Wood <scottwood@freescale.com>
> CC: Tom Rini <tom.rini@gmail.com>
> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
> CC: Wolfgang Denk <wd@denx.de>

Acked-by: Tom Rini <trini@ti.com>

Which covers the omap config file changes.  Assuming Wolfgang doesn't
see a problem with using SYS_... in defines, Scott, do you want this
via your tree (in /next) since it's NAND or in mine since it's TI
boards?  Assuming you don't see any problems of course.  Thanks!

-- 
Tom

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Wolfgang Denk]

Dear Tom,

In message <CA+M6bX=j+=UBkZ-VGoAJgFtfap+sLp4i2JypDSjJ_z-bCbw6sQ@mail.gmail.com> you wrote:
>
> Which covers the omap config file changes.  Assuming Wolfgang doesn't
> see a problem with using SYS_... in defines, Scott, do you want this
> via your tree (in /next) since it's NAND or in mine since it's TI
> boards?  Assuming you don't see any problems of course.  Thanks!

Indeed.

Stefano, what is this SYS_NAND_* supposed to mean?  Either it is a
config option, then you should name it CONFIG_SYS_NAND_* (or
CONFIG_NAND_), or it is something else, in which case the "SYS_" is
difficult to swallow for me (unless you have a really good
explanation?)


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
Real computer scientists don't comment their  code.  The  identifiers
are so long they can't afford the disk space.

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Tue, Dec 13, 2011 at 11:18 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Tom,
>
> In message <CA+M6bX=j+=UBkZ-VGoAJgFtfap+sLp4i2JypDSjJ_z-bCbw6sQ@mail.gmail.com> you wrote:
>>
>> Which covers the omap config file changes. ?Assuming Wolfgang doesn't
>> see a problem with using SYS_... in defines, Scott, do you want this
>> via your tree (in /next) since it's NAND or in mine since it's TI
>> boards? ?Assuming you don't see any problems of course. ?Thanks!
>
> Indeed.
>
> Stefano, what is this SYS_NAND_* supposed to mean? ?Either it is a
> config option, then you should name it CONFIG_SYS_NAND_* (or
> CONFIG_NAND_), or it is something else, in which case the "SYS_" is
> difficult to swallow for me (unless you have a really good
> explanation?)

So, we have various characteristics of the NAND chip that need to be
described for SPL.  Today we do CONFIG_SYS_NAND... but they're
unchanged in every implementation and are calculated from other
per-board values.  So to avoid duplication in each config file, we
drop them from there and use them only locally #define calculate them
in the file that needs them.  Since they're still system specific
values, I had suggested calling them SYS_... but I wasn't sure what
the right namespace is.  I had a recollection that using CONFIG_
outside of config files was a bad idea.

-- 
Tom

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Scott Wood]

On 12/13/2011 12:33 PM, Tom Rini wrote:
> So, we have various characteristics of the NAND chip that need to be
> described for SPL.  Today we do CONFIG_SYS_NAND... but they're
> unchanged in every implementation and are calculated from other
> per-board values.  So to avoid duplication in each config file, we
> drop them from there and use them only locally #define calculate them
> in the file that needs them.  Since they're still system specific
> values, I had suggested calling them SYS_... but I wasn't sure what
> the right namespace is.  I had a recollection that using CONFIG_
> outside of config files was a bad idea.

These symbols are not in a header file -- how about just plain ECCSTEPS
and ECCTOTAL?

-Scott

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Scott Wood]

On 12/13/2011 11:50 AM, Stefano Babic wrote:
>  	/* Pick the ECC bytes out of the oob data */
> -	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
> +	for (i = 0; i < (SYS_NAND_ECCTOTAL); i++)
>  		ecc_code[i] = oob_data[nand_ecc_pos[i]];
[snip]
> @@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst)
>  	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
>  
>  	/* Pick the ECC bytes out of the oob data */
> -	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
> +	for (i = 0; i < (SYS_NAND_ECCTOTAL); i++)

No need for parens around SYS_NAND_ECCTOTAL.

-Scott

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[stefano babic]

Am 13/12/2011 19:18, schrieb Wolfgang Denk:
> Dear Tom,
> 
> In message <CA+M6bX=j+=UBkZ-VGoAJgFtfap+sLp4i2JypDSjJ_z-bCbw6sQ@mail.gmail.com> you wrote:
>>
>> Which covers the omap config file changes.  Assuming Wolfgang doesn't
>> see a problem with using SYS_... in defines, Scott, do you want this
>> via your tree (in /next) since it's NAND or in mine since it's TI
>> boards?  Assuming you don't see any problems of course.  Thanks!
> 
> Indeed.
> 
> Stefano, what is this SYS_NAND_* supposed to mean?  Either it is a
> config option, then you should name it CONFIG_SYS_NAND_* (or
> CONFIG_NAND_), or it is something else, in which case the "SYS_" is
> difficult to swallow for me (unless you have a really good
> explanation?)

As already explained by Tom: we switch to local defines instead of
CONFIG_SYS, and we get rid of CONFIG_SYS_ in board configuration files.
Inside the single file, we have some freedom to use names, and the names
suggested by Tom tells us that they are system specific, but because the
CONFIG_SYS_ is missing, they cannot be confused with U-Boot general
configuration settings. I find the names (for me) quite self explaining,
but if there is some possibility to misunderstand their meaning we can
drop the SYS_ prefix.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Scott Wood]

On 12/13/2011 12:04 PM, Tom Rini wrote:
> On Tue, Dec 13, 2011 at 10:50 AM, Stefano Babic <sbabic@denx.de> wrote:
>> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
>> which is likely to contain already loaded data.
>> The patch saves the oob data and the ecc on the stack replacing
>> the fixed address in RAM.
>>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> CC: Ilya Yanok <yanok@emcraft.com>
>> CC: Scott Wood <scottwood@freescale.com>
>> CC: Tom Rini <tom.rini@gmail.com>
>> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
>> CC: Wolfgang Denk <wd@denx.de>
> 
> Acked-by: Tom Rini <trini@ti.com>
> 
> Which covers the omap config file changes.  Assuming Wolfgang doesn't
> see a problem with using SYS_... in defines, Scott, do you want this
> via your tree (in /next) since it's NAND or in mine since it's TI
> boards?  Assuming you don't see any problems of course.  Thanks!

The board-specific stuff is just removing unneeded NAND config items,
and isn't depending on other non-NAND patches, so I can take it through
my tree -- but if you want me to ACK it (once the minor issues are
fixed) to go via your tree so you can build on top of it sooner, that's
fine...

-Scott

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Tue, Dec 13, 2011 at 11:52 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/13/2011 12:04 PM, Tom Rini wrote:
>> On Tue, Dec 13, 2011 at 10:50 AM, Stefano Babic <sbabic@denx.de> wrote:
>>> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
>>> which is likely to contain already loaded data.
>>> The patch saves the oob data and the ecc on the stack replacing
>>> the fixed address in RAM.
>>>
>>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>>> CC: Ilya Yanok <yanok@emcraft.com>
>>> CC: Scott Wood <scottwood@freescale.com>
>>> CC: Tom Rini <tom.rini@gmail.com>
>>> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
>>> CC: Wolfgang Denk <wd@denx.de>
>>
>> Acked-by: Tom Rini <trini@ti.com>
>>
>> Which covers the omap config file changes. ?Assuming Wolfgang doesn't
>> see a problem with using SYS_... in defines, Scott, do you want this
>> via your tree (in /next) since it's NAND or in mine since it's TI
>> boards? ?Assuming you don't see any problems of course. ?Thanks!
>
> The board-specific stuff is just removing unneeded NAND config items,
> and isn't depending on other non-NAND patches, so I can take it through
> my tree -- but if you want me to ACK it (once the minor issues are
> fixed) to go via your tree so you can build on top of it sooner, that's
> fine...

Just building up my /next tree and don't want this to get lost, so if
you can take v4 once it's out and you're happy, I'd appreciate it.
Thanks!

-- 
Tom

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Tue, Dec 13, 2011 at 11:45 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/13/2011 12:33 PM, Tom Rini wrote:
>> So, we have various characteristics of the NAND chip that need to be
>> described for SPL. ?Today we do CONFIG_SYS_NAND... but they're
>> unchanged in every implementation and are calculated from other
>> per-board values. ?So to avoid duplication in each config file, we
>> drop them from there and use them only locally #define calculate them
>> in the file that needs them. ?Since they're still system specific
>> values, I had suggested calling them SYS_... but I wasn't sure what
>> the right namespace is. ?I had a recollection that using CONFIG_
>> outside of config files was a bad idea.
>
> These symbols are not in a header file -- how about just plain ECCSTEPS
> and ECCTOTAL?

Works for me and Wolfgang (on IRC) so lets go there for a hopefully
last v4 (along with what Scott said in the other email)

-- 
Tom

[U-Boot] [PATCH V3] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

On 13/12/2011 19:45, Scott Wood wrote:
> On 12/13/2011 12:33 PM, Tom Rini wrote:
>> So, we have various characteristics of the NAND chip that need to be
>> described for SPL.  Today we do CONFIG_SYS_NAND... but they're
>> unchanged in every implementation and are calculated from other
>> per-board values.  So to avoid duplication in each config file, we
>> drop them from there and use them only locally #define calculate them
>> in the file that needs them.  Since they're still system specific
>> values, I had suggested calling them SYS_... but I wasn't sure what
>> the right namespace is.  I had a recollection that using CONFIG_
>> outside of config files was a bad idea.
> 
> These symbols are not in a header file -- how about just plain ECCSTEPS
> and ECCTOTAL?

Ok, agree - done in V4.

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
which is likely to contain already loaded data.
The patch saves the oob data and the ecc on the stack replacing
the fixed address in RAM.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Ilya Yanok <yanok@emcraft.com>
CC: Scott Wood <scottwood@freescale.com>
CC: Tom Rini <tom.rini@gmail.com>
CC: Simon Schwarz <simonschwarzcor@googlemail.com>
CC: Wolfgang Denk <wd@denx.de>
---
V4:
 - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
 - drop parenthesis around defines (Scott Wood)

V3:
 - use local defines for CONFIG_SYS_NAND_ECCSTEPS and
   CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
 - drop CONFIG_SYS_NAND_ECCSTEPS from board config files

V2:
 - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
 - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c

 drivers/mtd/nand/nand_spl_simple.c     |   42 ++++++++++++-------------------
 include/configs/am3517_crane.h         |    4 ---
 include/configs/am3517_evm.h           |    4 ---
 include/configs/devkit8000.h           |    5 ----
 include/configs/hawkboard.h            |    5 +---
 include/configs/omap3_beagle.h         |    4 ---
 include/configs/omap3_evm.h            |    4 ---
 include/configs/omap3_evm_quick_nand.h |    4 ---
 8 files changed, 17 insertions(+), 55 deletions(-)

diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
index ed821f2..5a1e1a9 100644
--- a/drivers/mtd/nand/nand_spl_simple.c
+++ b/drivers/mtd/nand/nand_spl_simple.c
@@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS;
 static nand_info_t mtd;
 static struct nand_chip nand_chip;
 
+#define ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
+					CONFIG_SYS_NAND_ECCSIZE)
+#define ECCTOTAL	(ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES)
+
+
 #if (CONFIG_SYS_NAND_PAGE_SIZE <= 512)
 /*
  * NAND command for small page NAND devices (512)
@@ -145,30 +150,22 @@ static int nand_is_bad_block(int block)
 static int nand_read_page(int block, int page, uchar *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[ECCTOTAL];
+	u_char ecc_code[ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
-	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	int eccsteps = ECCSTEPS;
 	uint8_t *p = dst;
 	int stat;
 
-	/*
-	 * No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	nand_command(block, page, 0, NAND_CMD_READOOB);
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0; i < ECCTOTAL; i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
 
@@ -185,25 +182,18 @@ static int nand_read_page(int block, int page, uchar *dst)
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[ECCTOTAL];
+	u_char ecc_code[ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
-	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	int eccsteps = ECCSTEPS;
 	uint8_t *p = dst;
 	int stat;
 
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
-	/* No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		if (this->ecc.mode != NAND_ECC_SOFT)
 			this->ecc.hwctl(&mtd, NAND_ECC_READ);
@@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst)
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0; i < ECCTOTAL; i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
-	eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	eccsteps = ECCSTEPS;
 	p = dst;
 
 	for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 0a0c261..b0dd2f0 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -359,10 +359,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index d44eeec..f797f3f 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -360,10 +360,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index fc7f0cd..6ef6a87 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -344,11 +344,6 @@
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
 
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
-
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h
index 12acb27..65b3b78 100644
--- a/include/configs/hawkboard.h
+++ b/include/configs/hawkboard.h
@@ -138,11 +138,8 @@
 #define CONFIG_SYS_NAND_BAD_BLOCK_POS	0
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	10
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE /	 \
-					 CONFIG_SYS_NAND_ECCSIZE)
 #define CONFIG_SYS_NAND_OOBSIZE		64
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES *	\
-					CONFIG_SYS_NAND_ECCSTEPS)
+
 #endif /* CONFIG_SYS_USE_NAND */
 
 /*
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 970810c..7a240bb 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -426,10 +426,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index 2ce3959..1fcb7af 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -121,10 +121,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h
index 2f879c0..362fa1d 100644
--- a/include/configs/omap3_evm_quick_nand.h
+++ b/include/configs/omap3_evm_quick_nand.h
@@ -91,10 +91,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
-- 
1.7.5.4

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Tue, Dec 13, 2011 at 12:33 PM, Stefano Babic <sbabic@denx.de> wrote:
> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
> which is likely to contain already loaded data.
> The patch saves the oob data and the ecc on the stack replacing
> the fixed address in RAM.
>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ilya Yanok <yanok@emcraft.com>
> CC: Scott Wood <scottwood@freescale.com>
> CC: Tom Rini <tom.rini@gmail.com>
> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
> CC: Wolfgang Denk <wd@denx.de>

Acked-by: Tom Rini <trini@ti.com>

> ---
> V4:
> ?- Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
> ?- drop parenthesis around defines (Scott Wood)
>
> V3:
> ?- use local defines for CONFIG_SYS_NAND_ECCSTEPS and
> ? CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
> ?- drop CONFIG_SYS_NAND_ECCSTEPS from board config files
>
> V2:
> ?- CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
> ?- drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
>
> ?drivers/mtd/nand/nand_spl_simple.c ? ? | ? 42 ++++++++++++-------------------
> ?include/configs/am3517_crane.h ? ? ? ? | ? ?4 ---
> ?include/configs/am3517_evm.h ? ? ? ? ? | ? ?4 ---
> ?include/configs/devkit8000.h ? ? ? ? ? | ? ?5 ----
> ?include/configs/hawkboard.h ? ? ? ? ? ?| ? ?5 +---
> ?include/configs/omap3_beagle.h ? ? ? ? | ? ?4 ---
> ?include/configs/omap3_evm.h ? ? ? ? ? ?| ? ?4 ---
> ?include/configs/omap3_evm_quick_nand.h | ? ?4 ---
> ?8 files changed, 17 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
> index ed821f2..5a1e1a9 100644
> --- a/drivers/mtd/nand/nand_spl_simple.c
> +++ b/drivers/mtd/nand/nand_spl_simple.c
> @@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS;
> ?static nand_info_t mtd;
> ?static struct nand_chip nand_chip;
>
> +#define ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSIZE)
> +#define ECCTOTAL ? ? ? (ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES)
> +
> +
> ?#if (CONFIG_SYS_NAND_PAGE_SIZE <= 512)
> ?/*
> ?* NAND command for small page NAND devices (512)
> @@ -145,30 +150,22 @@ static int nand_is_bad_block(int block)
> ?static int nand_read_page(int block, int page, uchar *dst)
> ?{
> ? ? ? ?struct nand_chip *this = mtd.priv;
> - ? ? ? u_char *ecc_calc;
> - ? ? ? u_char *ecc_code;
> - ? ? ? u_char *oob_data;
> + ? ? ? u_char ecc_calc[ECCTOTAL];
> + ? ? ? u_char ecc_code[ECCTOTAL];
> + ? ? ? u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
> ? ? ? ?int i;
> ? ? ? ?int eccsize = CONFIG_SYS_NAND_ECCSIZE;
> ? ? ? ?int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
> - ? ? ? int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
> + ? ? ? int eccsteps = ECCSTEPS;
> ? ? ? ?uint8_t *p = dst;
> ? ? ? ?int stat;
>
> - ? ? ? /*
> - ? ? ? ?* No malloc available for now, just use some temporary locations
> - ? ? ? ?* in SDRAM
> - ? ? ? ?*/
> - ? ? ? ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
> - ? ? ? ecc_code = ecc_calc + 0x100;
> - ? ? ? oob_data = ecc_calc + 0x200;
> -
> ? ? ? ?nand_command(block, page, 0, NAND_CMD_READOOB);
> ? ? ? ?this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
> ? ? ? ?nand_command(block, page, 0, NAND_CMD_READ0);
>
> ? ? ? ?/* Pick the ECC bytes out of the oob data */
> - ? ? ? for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
> + ? ? ? for (i = 0; i < ECCTOTAL; i++)
> ? ? ? ? ? ? ? ?ecc_code[i] = oob_data[nand_ecc_pos[i]];
>
>
> @@ -185,25 +182,18 @@ static int nand_read_page(int block, int page, uchar *dst)
> ?static int nand_read_page(int block, int page, void *dst)
> ?{
> ? ? ? ?struct nand_chip *this = mtd.priv;
> - ? ? ? u_char *ecc_calc;
> - ? ? ? u_char *ecc_code;
> - ? ? ? u_char *oob_data;
> + ? ? ? u_char ecc_calc[ECCTOTAL];
> + ? ? ? u_char ecc_code[ECCTOTAL];
> + ? ? ? u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
> ? ? ? ?int i;
> ? ? ? ?int eccsize = CONFIG_SYS_NAND_ECCSIZE;
> ? ? ? ?int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
> - ? ? ? int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
> + ? ? ? int eccsteps = ECCSTEPS;
> ? ? ? ?uint8_t *p = dst;
> ? ? ? ?int stat;
>
> ? ? ? ?nand_command(block, page, 0, NAND_CMD_READ0);
>
> - ? ? ? /* No malloc available for now, just use some temporary locations
> - ? ? ? ?* in SDRAM
> - ? ? ? ?*/
> - ? ? ? ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
> - ? ? ? ecc_code = ecc_calc + 0x100;
> - ? ? ? oob_data = ecc_calc + 0x200;
> -
> ? ? ? ?for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> ? ? ? ? ? ? ? ?if (this->ecc.mode != NAND_ECC_SOFT)
> ? ? ? ? ? ? ? ? ? ? ? ?this->ecc.hwctl(&mtd, NAND_ECC_READ);
> @@ -213,10 +203,10 @@ static int nand_read_page(int block, int page, void *dst)
> ? ? ? ?this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
>
> ? ? ? ?/* Pick the ECC bytes out of the oob data */
> - ? ? ? for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
> + ? ? ? for (i = 0; i < ECCTOTAL; i++)
> ? ? ? ? ? ? ? ?ecc_code[i] = oob_data[nand_ecc_pos[i]];
>
> - ? ? ? eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
> + ? ? ? eccsteps = ECCSTEPS;
> ? ? ? ?p = dst;
>
> ? ? ? ?for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
> index 0a0c261..b0dd2f0 100644
> --- a/include/configs/am3517_crane.h
> +++ b/include/configs/am3517_crane.h
> @@ -359,10 +359,6 @@
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?10, 11, 12, 13}
> ?#define CONFIG_SYS_NAND_ECCSIZE ? ? ? ? ? ? ? ?512
> ?#define CONFIG_SYS_NAND_ECCBYTES ? ? ? 3
> -#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSIZE)
> -#define CONFIG_SYS_NAND_ECCTOTAL ? ? ? (CONFIG_SYS_NAND_ECCBYTES * \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSTEPS)
> ?#define CONFIG_SYS_NAND_U_BOOT_START ? CONFIG_SYS_TEXT_BASE
> ?#define CONFIG_SYS_NAND_U_BOOT_OFFS ? ?0x80000
>
> diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
> index d44eeec..f797f3f 100644
> --- a/include/configs/am3517_evm.h
> +++ b/include/configs/am3517_evm.h
> @@ -360,10 +360,6 @@
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?10, 11, 12, 13}
> ?#define CONFIG_SYS_NAND_ECCSIZE ? ? ? ? ? ? ? ?512
> ?#define CONFIG_SYS_NAND_ECCBYTES ? ? ? 3
> -#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSIZE)
> -#define CONFIG_SYS_NAND_ECCTOTAL ? ? ? (CONFIG_SYS_NAND_ECCBYTES * \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSTEPS)
> ?#define CONFIG_SYS_NAND_U_BOOT_START ? CONFIG_SYS_TEXT_BASE
> ?#define CONFIG_SYS_NAND_U_BOOT_OFFS ? ?0x80000
>
> diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
> index fc7f0cd..6ef6a87 100644
> --- a/include/configs/devkit8000.h
> +++ b/include/configs/devkit8000.h
> @@ -344,11 +344,6 @@
> ?#define CONFIG_SYS_NAND_ECCSIZE ? ? ? ? ? ? ? ?512
> ?#define CONFIG_SYS_NAND_ECCBYTES ? ? ? 3
>
> -#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSIZE)
> -#define CONFIG_SYS_NAND_ECCTOTAL ? ? ? (CONFIG_SYS_NAND_ECCBYTES * \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSTEPS)
> -
> ?#define CONFIG_SYS_NAND_U_BOOT_START ? CONFIG_SYS_TEXT_BASE
>
> ?#define CONFIG_SYS_NAND_U_BOOT_OFFS ? ?0x80000
> diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h
> index 12acb27..65b3b78 100644
> --- a/include/configs/hawkboard.h
> +++ b/include/configs/hawkboard.h
> @@ -138,11 +138,8 @@
> ?#define CONFIG_SYS_NAND_BAD_BLOCK_POS ?0
> ?#define CONFIG_SYS_NAND_ECCSIZE ? ? ? ? ? ? ? ?512
> ?#define CONFIG_SYS_NAND_ECCBYTES ? ? ? 10
> -#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / ? ? \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?CONFIG_SYS_NAND_ECCSIZE)
> ?#define CONFIG_SYS_NAND_OOBSIZE ? ? ? ? ? ? ? ?64
> -#define CONFIG_SYS_NAND_ECCTOTAL ? ? ? (CONFIG_SYS_NAND_ECCBYTES * ? ? \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSTEPS)
> +
> ?#endif /* CONFIG_SYS_USE_NAND */
>
> ?/*
> diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> index 970810c..7a240bb 100644
> --- a/include/configs/omap3_beagle.h
> +++ b/include/configs/omap3_beagle.h
> @@ -426,10 +426,6 @@
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?10, 11, 12, 13}
> ?#define CONFIG_SYS_NAND_ECCSIZE ? ? ? ? ? ? ? ?512
> ?#define CONFIG_SYS_NAND_ECCBYTES ? ? ? 3
> -#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSIZE)
> -#define CONFIG_SYS_NAND_ECCTOTAL ? ? ? (CONFIG_SYS_NAND_ECCBYTES * \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSTEPS)
> ?#define CONFIG_SYS_NAND_U_BOOT_START ? CONFIG_SYS_TEXT_BASE
> ?#define CONFIG_SYS_NAND_U_BOOT_OFFS ? ?0x80000
>
> diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
> index 2ce3959..1fcb7af 100644
> --- a/include/configs/omap3_evm.h
> +++ b/include/configs/omap3_evm.h
> @@ -121,10 +121,6 @@
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?10, 11, 12, 13}
> ?#define CONFIG_SYS_NAND_ECCSIZE ? ? ? ? ? ? ? ?512
> ?#define CONFIG_SYS_NAND_ECCBYTES ? ? ? 3
> -#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSIZE)
> -#define CONFIG_SYS_NAND_ECCTOTAL ? ? ? (CONFIG_SYS_NAND_ECCBYTES * \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSTEPS)
> ?#define CONFIG_SYS_NAND_U_BOOT_START ? CONFIG_SYS_TEXT_BASE
> ?#define CONFIG_SYS_NAND_U_BOOT_OFFS ? ?0x80000
>
> diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h
> index 2f879c0..362fa1d 100644
> --- a/include/configs/omap3_evm_quick_nand.h
> +++ b/include/configs/omap3_evm_quick_nand.h
> @@ -91,10 +91,6 @@
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?10, 11, 12, 13}
> ?#define CONFIG_SYS_NAND_ECCSIZE ? ? ? ? ? ? ? ?512
> ?#define CONFIG_SYS_NAND_ECCBYTES ? ? ? 3
> -#define CONFIG_SYS_NAND_ECCSTEPS ? ? ? (CONFIG_SYS_NAND_PAGE_SIZE / \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSIZE)
> -#define CONFIG_SYS_NAND_ECCTOTAL ? ? ? (CONFIG_SYS_NAND_ECCBYTES * \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_NAND_ECCSTEPS)
> ?#define CONFIG_SYS_NAND_U_BOOT_START ? CONFIG_SYS_TEXT_BASE
> ?#define CONFIG_SYS_NAND_U_BOOT_OFFS ? ?0x80000
>
> --
> 1.7.5.4
>



-- 
Tom

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Simon Schwarz]

On 12/13/2011 08:33 PM, Stefano Babic wrote:
> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
> which is likely to contain already loaded data.
> The patch saves the oob data and the ecc on the stack replacing
> the fixed address in RAM.
>
> Signed-off-by: Stefano Babic<sbabic@denx.de>
> CC: Ilya Yanok<yanok@emcraft.com>
> CC: Scott Wood<scottwood@freescale.com>
> CC: Tom Rini<tom.rini@gmail.com>
> CC: Simon Schwarz<simonschwarzcor@googlemail.com>
> CC: Wolfgang Denk<wd@denx.de>
> ---
> V4:
>   - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
>   - drop parenthesis around defines (Scott Wood)
>
> V3:
>   - use local defines for CONFIG_SYS_NAND_ECCSTEPS and
>     CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
>   - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
>
> V2:
>   - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
>   - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
>
[SNIP]

Hi Stefano,

on which repo is this patch based? I tried to test it on u-boot but the 
patch does not apply.

Regards
Simon

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

On 14/12/2011 09:22, Simon Schwarz wrote:
> On 12/13/2011 08:33 PM, Stefano Babic wrote:
>> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
>> which is likely to contain already loaded data.
>> The patch saves the oob data and the ecc on the stack replacing
>> the fixed address in RAM.
>>
>> Signed-off-by: Stefano Babic<sbabic@denx.de>
>> CC: Ilya Yanok<yanok@emcraft.com>
>> CC: Scott Wood<scottwood@freescale.com>
>> CC: Tom Rini<tom.rini@gmail.com>
>> CC: Simon Schwarz<simonschwarzcor@googlemail.com>
>> CC: Wolfgang Denk<wd@denx.de>
>> ---
>> V4:
>>   - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
>>   - drop parenthesis around defines (Scott Wood)
>>
>> V3:
>>   - use local defines for CONFIG_SYS_NAND_ECCSTEPS and
>>     CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
>>   - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
>>
>> V2:
>>   - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
>>   - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
>>
> [SNIP]
> 
> Hi Stefano,
> 
> on which repo is this patch based? I tried to test it on u-boot but the
> patch does not apply.

Because I thought that the patch will be merged by Tom, I rebased the
patch on u-boot-ti. This is also because there is Ilya's patch:

	nand_spl_simple: add support for software ECC

that modifies the same file. The patch can be applied flawlessy on top
of u-boot-ti, I have checked now again.

For testing I have then applied your SPL Linux patches, and tried to
boot linux, as I know there is a conflict with the ECC area.

Tom, it is surely better (I forget to mention) that you merge the patch
into u-boot-ti, because you have already merged Ilya's patch.

Regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

On 15/12/2011 10:03, Simon Schwarz wrote:
> Hi Stefano,
> 
> On 12/14/2011 09:49 AM, Stefano Babic wrote:
> [SNIP]
>> Because I thought that the patch will be merged by Tom, I rebased the
>> patch on u-boot-ti. This is also because there is Ilya's patch:
>>
>>     nand_spl_simple: add support for software ECC
>>
>> that modifies the same file. The patch can be applied flawlessy on top
>> of u-boot-ti, I have checked now again.
>>
>> For testing I have then applied your SPL Linux patches, and tried to
>> boot linux, as I know there is a conflict with the ECC area.
>>
>> Tom, it is surely better (I forget to mention) that you merge the patch
>> into u-boot-ti, because you have already merged Ilya's patch.
>>
>> Regards,
>> Stefano
>>
> 
> hmm. I tried it on u-boot-ti (master and next) and it didn't apply.
> 
> The problem was with drivers/mtd/nand/nand_spl_simple.c:145. The last
> commit touching the file was from Anatolij on 3th december.

I see, maybe it was not already merged when I tested. I send a rebased
patch.

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V5] nand_spl_simple: store ecc data on the stack

[Stefano Babic]

Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
which is likely to contain already loaded data.
The patch saves the oob data and the ecc on the stack replacing
the fixed address in RAM.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Ilya Yanok <yanok@emcraft.com>
CC: Scott Wood <scottwood@freescale.com>
CC: Tom Rini <tom.rini@gmail.com>
CC: Simon Schwarz <simonschwarzcor@googlemail.com>
CC: Wolfgang Denk <wd@denx.de>
---

V5:
  - no changes, rebased on current u-boot-ti

V4:
  - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
  - drop parenthesis around defines (Scott Wood)

V3:
  - use local defines for CONFIG_SYS_NAND_ECCSTEPS and
    CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
  - drop CONFIG_SYS_NAND_ECCSTEPS from board config files

V2:
  - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
  - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c


 drivers/mtd/nand/nand_spl_simple.c     |   42 ++++++++++++-------------------
 include/configs/am3517_crane.h         |    4 ---
 include/configs/am3517_evm.h           |    4 ---
 include/configs/devkit8000.h           |    5 ----
 include/configs/hawkboard.h            |    5 +---
 include/configs/omap3_beagle.h         |    4 ---
 include/configs/omap3_evm.h            |    4 ---
 include/configs/omap3_evm_quick_nand.h |    4 ---
 8 files changed, 17 insertions(+), 55 deletions(-)

diff --git a/drivers/mtd/nand/nand_spl_simple.c b/drivers/mtd/nand/nand_spl_simple.c
index 7eb08a3..4a4d02f 100644
--- a/drivers/mtd/nand/nand_spl_simple.c
+++ b/drivers/mtd/nand/nand_spl_simple.c
@@ -27,6 +27,11 @@ static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS;
 static nand_info_t mtd;
 static struct nand_chip nand_chip;
 
+#define ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
+					CONFIG_SYS_NAND_ECCSIZE)
+#define ECCTOTAL	(ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES)
+
+
 #if (CONFIG_SYS_NAND_PAGE_SIZE <= 512)
 /*
  * NAND command for small page NAND devices (512)
@@ -145,29 +150,21 @@ static int nand_is_bad_block(int block)
 static int nand_read_page(int block, int page, uchar *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[ECCTOTAL];
+	u_char ecc_code[ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
-	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	int eccsteps = ECCSTEPS;
 	uint8_t *p = dst;
 
-	/*
-	 * No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	nand_command(block, page, 0, NAND_CMD_READOOB);
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0; i < ECCTOTAL; i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
 
@@ -184,24 +181,17 @@ static int nand_read_page(int block, int page, uchar *dst)
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;
-	u_char *ecc_calc;
-	u_char *ecc_code;
-	u_char *oob_data;
+	u_char ecc_calc[ECCTOTAL];
+	u_char ecc_code[ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
 	int i;
 	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
 	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
-	int eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	int eccsteps = ECCSTEPS;
 	uint8_t *p = dst;
 
 	nand_command(block, page, 0, NAND_CMD_READ0);
 
-	/* No malloc available for now, just use some temporary locations
-	 * in SDRAM
-	 */
-	ecc_calc = (u_char *)(CONFIG_SYS_SDRAM_BASE + 0x10000);
-	ecc_code = ecc_calc + 0x100;
-	oob_data = ecc_calc + 0x200;
-
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		if (this->ecc.mode != NAND_ECC_SOFT)
 			this->ecc.hwctl(&mtd, NAND_ECC_READ);
@@ -211,10 +201,10 @@ static int nand_read_page(int block, int page, void *dst)
 	this->read_buf(&mtd, oob_data, CONFIG_SYS_NAND_OOBSIZE);
 
 	/* Pick the ECC bytes out of the oob data */
-	for (i = 0; i < CONFIG_SYS_NAND_ECCTOTAL; i++)
+	for (i = 0; i < ECCTOTAL; i++)
 		ecc_code[i] = oob_data[nand_ecc_pos[i]];
 
-	eccsteps = CONFIG_SYS_NAND_ECCSTEPS;
+	eccsteps = ECCSTEPS;
 	p = dst;
 
 	for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 0a0c261..b0dd2f0 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -359,10 +359,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index d44eeec..f797f3f 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -360,10 +360,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index 758326b..2b6a6ee 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -343,11 +343,6 @@
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
 
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
-
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
diff --git a/include/configs/hawkboard.h b/include/configs/hawkboard.h
index 12acb27..65b3b78 100644
--- a/include/configs/hawkboard.h
+++ b/include/configs/hawkboard.h
@@ -138,11 +138,8 @@
 #define CONFIG_SYS_NAND_BAD_BLOCK_POS	0
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	10
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE /	 \
-					 CONFIG_SYS_NAND_ECCSIZE)
 #define CONFIG_SYS_NAND_OOBSIZE		64
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES *	\
-					CONFIG_SYS_NAND_ECCSTEPS)
+
 #endif /* CONFIG_SYS_USE_NAND */
 
 /*
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 91af8a0..4c7a686 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -418,10 +418,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL	(CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index 2ce3959..1fcb7af 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -121,10 +121,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
diff --git a/include/configs/omap3_evm_quick_nand.h b/include/configs/omap3_evm_quick_nand.h
index 2f879c0..362fa1d 100644
--- a/include/configs/omap3_evm_quick_nand.h
+++ b/include/configs/omap3_evm_quick_nand.h
@@ -91,10 +91,6 @@
 						10, 11, 12, 13}
 #define CONFIG_SYS_NAND_ECCSIZE		512
 #define CONFIG_SYS_NAND_ECCBYTES	3
-#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
-						CONFIG_SYS_NAND_ECCSIZE)
-#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
-						CONFIG_SYS_NAND_ECCSTEPS)
 #define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
 
-- 
1.7.5.4

[U-Boot] [PATCH V5] nand_spl_simple: store ecc data on the stack

[Scott Wood]

On 12/15/2011 03:55 AM, Stefano Babic wrote:
> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
> which is likely to contain already loaded data.
> The patch saves the oob data and the ecc on the stack replacing
> the fixed address in RAM.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ilya Yanok <yanok@emcraft.com>
> CC: Scott Wood <scottwood@freescale.com>
> CC: Tom Rini <tom.rini@gmail.com>
> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
> CC: Wolfgang Denk <wd@denx.de>
> ---

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

[U-Boot] [PATCH V5] nand_spl_simple: store ecc data on the stack

[Scott Wood]

On 12/15/2011 03:55 AM, Stefano Babic wrote:
> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
> which is likely to contain already loaded data.
> The patch saves the oob data and the ecc on the stack replacing
> the fixed address in RAM.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ilya Yanok <yanok@emcraft.com>
> CC: Scott Wood <scottwood@freescale.com>
> CC: Tom Rini <tom.rini@gmail.com>
> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
> CC: Wolfgang Denk <wd@denx.de>

Applied to u-boot-nand-flash

-Scott

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Scott Wood]

On 12/13/2011 01:33 PM, Stefano Babic wrote:
> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
> which is likely to contain already loaded data.
> The patch saves the oob data and the ecc on the stack replacing
> the fixed address in RAM.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Ilya Yanok <yanok@emcraft.com>
> CC: Scott Wood <scottwood@freescale.com>
> CC: Tom Rini <tom.rini@gmail.com>
> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
> CC: Wolfgang Denk <wd@denx.de>
> ---
> V4:
>  - Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
>  - drop parenthesis around defines (Scott Wood)
> 
> V3:
>  - use local defines for CONFIG_SYS_NAND_ECCSTEPS and
>    CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
>  - drop CONFIG_SYS_NAND_ECCSTEPS from board config files
> 
> V2:
>  - CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
>  - drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
> 
>  drivers/mtd/nand/nand_spl_simple.c     |   42 ++++++++++++-------------------
>  include/configs/am3517_crane.h         |    4 ---
>  include/configs/am3517_evm.h           |    4 ---
>  include/configs/devkit8000.h           |    5 ----
>  include/configs/hawkboard.h            |    5 +---
>  include/configs/omap3_beagle.h         |    4 ---
>  include/configs/omap3_evm.h            |    4 ---
>  include/configs/omap3_evm_quick_nand.h |    4 ---
>  8 files changed, 17 insertions(+), 55 deletions(-)

After this patch a hawkboard_nand build gives this:

> Configuring for hawkboard_nand - Board: hawkboard, Options: NAND_U_BOOT
> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c: In function 'nand_read_page':
> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: error: 'CONFIG_SYS_NAND_ECCSTEPS' undeclared (first use in this function)
> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: note: each undeclared identifier is reported only once for each function it appears in
> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:164:18: error: 'CONFIG_SYS_NAND_ECCTOTAL' undeclared (first use in this function)
> make[1]: *** [/tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.o] Error 1
> make: *** [nand_spl] Error 2
> make: *** Waiting for unfinished jobs....

Should I drop hawkboard from this patch, or add the change to
nand_spl/nand_boot.c?  Is a hawkboard conversion to the new SPL pending
in some other tree?

-Scott

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Tue, Jan 10, 2012 at 4:01 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/13/2011 01:33 PM, Stefano Babic wrote:
>> Currently nand_spl_simple puts it's temp data at 0x10000 offset in SDRAM
>> which is likely to contain already loaded data.
>> The patch saves the oob data and the ecc on the stack replacing
>> the fixed address in RAM.
>>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> CC: Ilya Yanok <yanok@emcraft.com>
>> CC: Scott Wood <scottwood@freescale.com>
>> CC: Tom Rini <tom.rini@gmail.com>
>> CC: Simon Schwarz <simonschwarzcor@googlemail.com>
>> CC: Wolfgang Denk <wd@denx.de>
>> ---
>> V4:
>> ?- Drop SYS_ from local defines (Wolfgang Denk, Scott Wood)
>> ?- drop parenthesis around defines (Scott Wood)
>>
>> V3:
>> ?- use local defines for CONFIG_SYS_NAND_ECCSTEPS and
>> ? ?CONFIG_SYS_NAND_ECCTOTAL (Tom Rini)
>> ?- drop CONFIG_SYS_NAND_ECCSTEPS from board config files
>>
>> V2:
>> ?- CONFIG_SYS_NAND_ECCTOTAL can always be computed (Ilya Yanok)
>> ?- drop all CONFIG_SYS_NAND_ECCTOTAL in arm boards using nand_simple.c
>>
>> ?drivers/mtd/nand/nand_spl_simple.c ? ? | ? 42 ++++++++++++-------------------
>> ?include/configs/am3517_crane.h ? ? ? ? | ? ?4 ---
>> ?include/configs/am3517_evm.h ? ? ? ? ? | ? ?4 ---
>> ?include/configs/devkit8000.h ? ? ? ? ? | ? ?5 ----
>> ?include/configs/hawkboard.h ? ? ? ? ? ?| ? ?5 +---
>> ?include/configs/omap3_beagle.h ? ? ? ? | ? ?4 ---
>> ?include/configs/omap3_evm.h ? ? ? ? ? ?| ? ?4 ---
>> ?include/configs/omap3_evm_quick_nand.h | ? ?4 ---
>> ?8 files changed, 17 insertions(+), 55 deletions(-)
>
> After this patch a hawkboard_nand build gives this:
>
>> Configuring for hawkboard_nand - Board: hawkboard, Options: NAND_U_BOOT
>> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c: In function 'nand_read_page':
>> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: error: 'CONFIG_SYS_NAND_ECCSTEPS' undeclared (first use in this function)
>> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:148:17: note: each undeclared identifier is reported only once for each function it appears in
>> /tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.c:164:18: error: 'CONFIG_SYS_NAND_ECCTOTAL' undeclared (first use in this function)
>> make[1]: *** [/tmp/u-boot-arm/nand_spl/board/davinci/da8xxevm/nand_boot.o] Error 1
>> make: *** [nand_spl] Error 2
>> make: *** Waiting for unfinished jobs....
>
> Should I drop hawkboard from this patch, or add the change to
> nand_spl/nand_boot.c? ?Is a hawkboard conversion to the new SPL pending
> in some other tree?

A hawkboard conversion is pending Albert ack'ing/commenting on a
generic ARM fixup.

-- 
Tom

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Albert ARIBAUD]

Hi,

I am slowly catching up on my e-mail, so...

Le 11/01/2012 00:24, Tom Rini a ?crit :
> A hawkboard conversion is pending Albert ack'ing/commenting on a
> generic ARM fixup.

Can you point me to this patch series? I'll look it up as soon as possible.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH V4] nand_spl_simple: store ecc data on the stack

[Tom Rini]

On Fri, Feb 3, 2012 at 1:17 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi,
>
> I am slowly catching up on my e-mail, so...
>
> Le 11/01/2012 00:24, Tom Rini a ?crit :
>>
>> A hawkboard conversion is pending Albert ack'ing/commenting on a
>> generic ARM fixup.
>
>
> Can you point me to this patch series? I'll look it up as soon as possible.

Latest version starts at http://patchwork.ozlabs.org/patch/139114/ and
is in my queue for Monday, assuming there's no further feedback.

-- 
Tom