spl, nand: add 4bit HW ecc oob first nand_read_page function

PATCH Superseded Heiko Schocher 4 participants 15 messages Oct 5, 2011 – Oct 28, 2011

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Heiko Schocher

author

2011-10-05 14:28 UTC #0 lore raw

PATCH v2 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.

Signed-off-by Heiko Schocher <[email protected]>

Cc Scott Wood <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Sandeep Paulraj <[email protected]>

---
 drivers/mtd/nand/nand_spl_simple.c |   43 ++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

drivers/mtd/nand/nand_spl_simple.c +43 -0

index 71491d4..7ab332f 100644
@@ -140,6 +140,47 @@ static int nand_is_bad_block(int block)
 	return 0;
 }
 
+#if defined(CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST)
+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;
+	int i;
+	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
+	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
+	int eccsteps = CONFIG_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++)
+		ecc_code[i] = oob_data[nand_ecc_pos[i]];
+
+
+	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		this->ecc.hwctl(&mtd, NAND_ECC_READ);
+		this->read_buf(&mtd, p, eccsize);
+		this->ecc.calculate(&mtd, p, &ecc_calc[i]);
+		stat = this->ecc.correct(&mtd, p, &ecc_code[i], &ecc_calc[i]);
+	}
+
+	return 0;
+}
+#else
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;

@@ -186,6 +227,7 @@ static int nand_read_page(int block, int page, void *dst)
 
 	return 0;
 }
+#endif
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
 {

@@ -231,6 +273,7 @@ void nand_init(void)
 	nand_chip.IO_ADDR_R = nand_chip.IO_ADDR_W =
 		(void  __iomem *)CONFIG_SYS_NAND_BASE;
 	nand_chip.options = 0;
+	nand_chip.dev_ready = NULL;
 	board_nand_init(&nand_chip);
 
 	if (nand_chip.select_chip)

Signature · 1 line

1.7.6.2

Scott Wood

2011-10-05 21:45 UTC #1 lore raw

PATCH v2 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Quoted 64 lines · Heiko Schocher

similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.

Signed-off-by Heiko Schocher <[email protected]>

Cc Scott Wood <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Sandeep Paulraj <[email protected]>

---
 drivers/mtd/nand/nand_spl_simple.c |   43 ++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

drivers/mtd/nand/nand_spl_simple.c +41 -0

index 71491d4..7ab332f 100644
@@ -140,6 +140,47 @@ static int nand_is_bad_block(int block)
 	return 0;
 }
 
+#if defined(CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST)
+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;
+	int i;
+	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
+	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
+	int eccsteps = CONFIG_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++)
+		ecc_code[i] = oob_data[nand_ecc_pos[i]];
+
+
+	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		this->ecc.hwctl(&mtd, NAND_ECC_READ);
+		this->read_buf(&mtd, p, eccsize);
+		this->ecc.calculate(&mtd, p, &ecc_calc[i]);
+		stat = this->ecc.correct(&mtd, p, &ecc_code[i], &ecc_calc[i]);
+	}
+
+	return 0;
+}
+#else
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;

drivers/mtd/nand/nand_spl_simple.c static int nand_read_page(int block, int page, void *dst) · line 233

Quoted 7 lines

drivers/mtd/nand/nand_spl_simple.c +1 -0

@@ -186,6 +227,7 @@ static int nand_read_page(int block, int page, void *dst)
 
 	return 0;
 }
+#endif
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
 {

ACK this part

drivers/mtd/nand/nand_spl_simple.c void nand_init(void) · line 279

Quoted 8 lines

drivers/mtd/nand/nand_spl_simple.c +1 -0

@@ -231,6 +273,7 @@ void nand_init(void)
 	nand_chip.IO_ADDR_R = nand_chip.IO_ADDR_W =
 		(void  __iomem *)CONFIG_SYS_NAND_BASE;
 	nand_chip.options = 0;
+	nand_chip.dev_ready = NULL;
 	board_nand_init(&nand_chip);
 
 	if (nand_chip.select_chip)

This looks unrelated, and if it makes a difference suggests that the BSS isn't being cleared.

-Scott

Heiko Schocher

author

2011-10-06 05:41 UTC #2 lore raw

PATCH v2 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Hello Scott,

Quoted 16 lines · Scott Wood

Quoted 15 lines · Heiko Schocher
similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.
Signed-off-by Heiko Schocher <[email protected]>
Cc Scott Wood <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Sandeep Paulraj <[email protected]>
---
 drivers/mtd/nand/nand_spl_simple.c |   43 ++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)
drivers/mtd/nand/nand_spl_simple.c +0 -0
index 71491d4..7ab332f 100644

[...]

Quoted 10 lines

drivers/mtd/nand/nand_spl_simple.c static int nand_read_page(int block, int page, void *dst) · line 233
Quoted 8 lines
drivers/mtd/nand/nand_spl_simple.c +1 -0
@@ -186,6 +227,7 @@ static int nand_read_page(int block, int page, void *dst)
 
 	return 0;
 }
+#endif
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
 {
ACK this part

great!

Quoted 11 lines

drivers/mtd/nand/nand_spl_simple.c void nand_init(void) · line 279
Quoted 8 lines
drivers/mtd/nand/nand_spl_simple.c +1 -0
@@ -231,6 +273,7 @@ void nand_init(void)
 	nand_chip.IO_ADDR_R = nand_chip.IO_ADDR_W =
 		(void  __iomem *)CONFIG_SYS_NAND_BASE;
 	nand_chip.options = 0;
+	nand_chip.dev_ready = NULL;
 	board_nand_init(&nand_chip);
 
 	if (nand_chip.select_chip)
This looks unrelated, and if it makes a difference suggests that the BSS isn't being cleared.

BSS is cleared at this point, so you are right, above two lines are not necessary, remove them.

Thanks!

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Heiko Schocher

author

2011-10-06 05:45 UTC #3 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

In reply to Heiko Schocher

similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.

Signed-off-by Heiko Schocher <[email protected]>

Cc Scott Wood <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Sandeep Paulraj <[email protected]>

---
changes for v3:
- add comment from Scott Wood:
as BSS is cleared, no need for intializing vars with 0 remove this.

 drivers/mtd/nand/nand_spl_simple.c |   43 +++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)

drivers/mtd/nand/nand_spl_simple.c +42 -1

index 71491d4..622a9b5 100644
@@ -140,6 +140,47 @@ static int nand_is_bad_block(int block)
 	return 0;
 }
 
+#if defined(CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST)
+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;
+	int i;
+	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
+	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
+	int eccsteps = CONFIG_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++)
+		ecc_code[i] = oob_data[nand_ecc_pos[i]];
+
+
+	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		this->ecc.hwctl(&mtd, NAND_ECC_READ);
+		this->read_buf(&mtd, p, eccsize);
+		this->ecc.calculate(&mtd, p, &ecc_calc[i]);
+		stat = this->ecc.correct(&mtd, p, &ecc_code[i], &ecc_calc[i]);
+	}
+
+	return 0;
+}
+#else
 static int nand_read_page(int block, int page, void *dst)
 {
 	struct nand_chip *this = mtd.priv;

@@ -186,6 +227,7 @@ static int nand_read_page(int block, int page, void *dst)
 
 	return 0;
 }
+#endif
 
 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
 {

@@ -230,7 +272,6 @@ void nand_init(void)
 	mtd.priv = &nand_chip;
 	nand_chip.IO_ADDR_R = nand_chip.IO_ADDR_W =
 		(void  __iomem *)CONFIG_SYS_NAND_BASE;
-	nand_chip.options = 0;
 	board_nand_init(&nand_chip);
 
 	if (nand_chip.select_chip)

Signature · 1 line

1.7.6.2

Tom Rini

2011-10-06 16:05 UTC #4 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Quoted 8 lines · Heiko Schocher

similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.
Signed-off-by Heiko Schocher <[email protected]>
Cc Scott Wood <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Sandeep Paulraj <[email protected]>

Quoted 5 lines

---
changes for v3:
- add comment from Scott Wood:
?as BSS is cleared, no need for intializing vars with 0 ?remove this.

Scott and I were talking about this on IRC (since I'm adding in a BCH8 SPL platform) and isn't the only difference just where the OOB is? So the config switch should be changed to reflect that since this code would work with any ECC size and OOB first, yes?

Signature · 1 line

Tom

Scott Wood

2011-10-10 21:17 UTC #5 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

In reply to Heiko Schocher

Quoted 22 lines · Heiko Schocher

similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.
Signed-off-by Heiko Schocher <[email protected]>
Cc Scott Wood <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Sandeep Paulraj <[email protected]>
---
changes for v3:
- add comment from Scott Wood:
as BSS is cleared, no need for intializing vars with 0 remove this.
 drivers/mtd/nand/nand_spl_simple.c |   43 +++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)
drivers/mtd/nand/nand_spl_simple.c +0 -0
index 71491d4..622a9b5 100644

drivers/mtd/nand/nand_spl_simple.c static int nand_is_bad_block(int block) · line 143

Quoted 4 lines

drivers/mtd/nand/nand_spl_simple.c +1 -0

@@ -140,6 +140,47 @@ static int nand_is_bad_block(int block)
 	return 0;
 }
 
+#if defined(CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST)

Change the name to not be 4BIT-specific, and add documentation for this config option.

-Scott

Heiko Schocher

author

2011-10-11 05:41 UTC #6 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Hello Scott,

Quoted 26 lines · Scott Wood

Quoted 25 lines · Heiko Schocher
similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.
Signed-off-by Heiko Schocher <[email protected]>
Cc Scott Wood <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Sandeep Paulraj <[email protected]>
---
changes for v3:
- add comment from Scott Wood:
as BSS is cleared, no need for intializing vars with 0 remove this.
 drivers/mtd/nand/nand_spl_simple.c |   43 +++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)
drivers/mtd/nand/nand_spl_simple.c +0 -0
index 71491d4..622a9b5 100644
@@ -140,6 +140,47 @@ static int nand_is_bad_block(int block)
 	return 0;
 }
 

Quoted 4 lines

drivers/mtd/nand/nand_spl_simple.c static int nand_is_bad_block(int block) · line 143
Quoted 1 line
drivers/mtd/nand/nand_spl_simple.c +1 -0
+#if defined(CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST)
Change the name to not be 4BIT-specific, and add documentation for this config option.

Hmm.. this is no new config option, it enables on davinci socs the 4Bit NAND HW ECC generation ... and I need this in spl code too ... Ok, you are right, there is no documentation of this config option. Should I add this documentation in this patch or in a seperate patch?

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Wolfgang Denk

2011-10-11 05:49 UTC #7 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Dear Heiko Schocher,

Quoted 5 lines · In message <[email protected]> you wrote:

Hmm.. this is no new config option, it enables on davinci socs the 4Bit NAND HW ECC generation ... and I need this in spl code too ... Ok, you are right, there is no documentation of this config option. Should I add this documentation in this patch or in a seperate patch?

Separate, please. Thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines

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
"If you are afraid of loneliness, don't marry."             - Chekhov

Scott Wood

2011-10-25 15:24 UTC #8 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

In reply to Heiko Schocher

Quoted 29 lines · Heiko Schocher

Hello Scott,
Quoted 26 lines · Scott Wood
Quoted 25 lines · Heiko Schocher
similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.
Signed-off-by Heiko Schocher <[email protected]>
Cc Scott Wood <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Sandeep Paulraj <[email protected]>
---
changes for v3:
- add comment from Scott Wood:
as BSS is cleared, no need for intializing vars with 0 remove this.
 drivers/mtd/nand/nand_spl_simple.c |   43 +++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)
drivers/mtd/nand/nand_spl_simple.c +0 -0
index 71491d4..622a9b5 100644
@@ -140,6 +140,47 @@ static int nand_is_bad_block(int block)
 	return 0;
 }
 

Quoted 7 lines

Quoted 4 lines
drivers/mtd/nand/nand_spl_simple.c static int nand_is_bad_block(int block) · line 143
Quoted 1 line
drivers/mtd/nand/nand_spl_simple.c +1 -0
+#if defined(CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST)
Change the name to not be 4BIT-specific, and add documentation for this config option.
Hmm.. this is no new config option, it enables on davinci socs the 4Bit NAND HW ECC generation ... and I need this in spl code too ...

It's not new, but it is misnamed (there's nothing 4-bit specific in the #ifdeffed code), and we now have someone that wants this for 8-bit ECC.

-Scott

Heiko Schocher

author

2011-10-26 06:16 UTC #9 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Hello Scott,

Quoted 30 lines · Scott Wood

Quoted 29 lines · Heiko Schocher
Hello Scott,
Quoted 26 lines · Scott Wood
Quoted 25 lines · Heiko Schocher
similiar to commit dc7cd8e59ba077f3b4c1a4557c9cd86a31b9ab1f, only adapted for the new spl framework.
Signed-off-by Heiko Schocher <[email protected]>
Cc Scott Wood <[email protected]>
Cc Albert ARIBAUD <[email protected]>
Cc Sandeep Paulraj <[email protected]>
---
changes for v3:
- add comment from Scott Wood:
as BSS is cleared, no need for intializing vars with 0 remove this.
 drivers/mtd/nand/nand_spl_simple.c |   43 +++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)
drivers/mtd/nand/nand_spl_simple.c +0 -0
index 71491d4..622a9b5 100644
@@ -140,6 +140,47 @@ static int nand_is_bad_block(int block)
 	return 0;
 }
 

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
drivers/mtd/nand/nand_spl_simple.c static int nand_is_bad_block(int block) · line 143
Quoted 1 line
drivers/mtd/nand/nand_spl_simple.c +1 -0
+#if defined(CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST)
Change the name to not be 4BIT-specific, and add documentation for this config option.
Hmm.. this is no new config option, it enables on davinci socs the 4Bit NAND HW ECC generation ... and I need this in spl code too ...
It's not new, but it is misnamed (there's nothing 4-bit specific in the #ifdeffed code), and we now have someone that wants this for 8-bit ECC.

look at my v4 series of this patchset:

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

I renamed it to: CONFIG_SYS_NAND_HW_ECC_OOBFIRST

In drivers/mtd/nand/davinci_nand.c CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST is used, and there are 4bit specific functions, so this define is also valid, just we need in drivers/mtd/nand/nand_spl_simple.c a more common name, right?

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Scott Wood

2011-10-26 16:58 UTC #10 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Quoted 11 lines · Heiko Schocher

Hello Scott,
Quoted 2 lines · Scott Wood
It's not new, but it is misnamed (there's nothing 4-bit specific in the #ifdeffed code), and we now have someone that wants this for 8-bit ECC.
look at my v4 series of this patchset:
http://patchwork.ozlabs.org/patch/121295/
I renamed it to: CONFIG_SYS_NAND_HW_ECC_OOBFIRST

OK. I just got back from vacation, so I'm still going through an e-mail backlog.

Quoted 4 lines

In drivers/mtd/nand/davinci_nand.c CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST is used, and there are 4bit specific functions, so this define is also valid, just we need in drivers/mtd/nand/nand_spl_simple.c a more common name, right?

Right. Ideally, though, that would either be CONFIG_SYS_DAVINCI_... or #if defined(CONFIG_SYS_NAND_HW_ECC_OOBFIRST) && \
defined(CONFIG_SYS_NAND_HW_ECC_4BIT)

-Scott

Heiko Schocher

author

2011-10-27 05:23 UTC #11 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Hello Scott,

Quoted 6 lines · Scott Wood

Quoted 5 lines · Heiko Schocher
Hello Scott,
Quoted 2 lines · Scott Wood
It's not new, but it is misnamed (there's nothing 4-bit specific in the #ifdeffed code), and we now have someone that wants this for 8-bit ECC.

Quoted 8 lines

Quoted 5 lines
look at my v4 series of this patchset:
http://patchwork.ozlabs.org/patch/121295/
I renamed it to: CONFIG_SYS_NAND_HW_ECC_OOBFIRST
OK. I just got back from vacation, so I'm still going through an e-mail backlog.

No problem, hope you had a nice vacation?

Quoted 8 lines

Quoted 4 lines
In drivers/mtd/nand/davinci_nand.c CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST is used, and there are 4bit specific functions, so this define is also valid, just we need in drivers/mtd/nand/nand_spl_simple.c a more common name, right?
Right. Ideally, though, that would either be CONFIG_SYS_DAVINCI_... or #if defined(CONFIG_SYS_NAND_HW_ECC_OOBFIRST) && \
defined(CONFIG_SYS_NAND_HW_ECC_4BIT)

Hmm.. I thought you meant, this is not davinci nor 4bit specific? Or do you mean to rename the CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST define in drivers/mtd/nand/davinci_nand.c?

Sandeep, what do you think?

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Scott Wood

2011-10-27 18:14 UTC #12 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Quoted 5 lines · Heiko Schocher

Quoted 5 lines
Quoted 4 lines
In drivers/mtd/nand/davinci_nand.c CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST is used, and there are 4bit specific functions, so this define is also valid, just we need in drivers/mtd/nand/nand_spl_simple.c a more common name, right?

Quoted 7 lines

Quoted 3 lines
Right. Ideally, though, that would either be CONFIG_SYS_DAVINCI_... or #if defined(CONFIG_SYS_NAND_HW_ECC_OOBFIRST) && \
defined(CONFIG_SYS_NAND_HW_ECC_4BIT)
Hmm.. I thought you meant, this is not davinci nor 4bit specific? Or do you mean to rename the CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST define in drivers/mtd/nand/davinci_nand.c?

I just mean that "4bit" and "oobfirst" are independent things, so I'd rather not have something that combines the two as part of the configuration of the general NAND interface in the absence of a requirement to enumerate all possibilities. OTOH, the structure of driver's private config can be whatever makes the most sense for that driver, but should be named in a way that it's obviously driver-specific.

-Scott

Heiko Schocher

author

2011-10-28 06:05 UTC #13 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Hello Scott,

Quoted 12 lines · Scott Wood

Quoted 10 lines · Heiko Schocher
Quoted 7 lines
Quoted 4 lines
In drivers/mtd/nand/davinci_nand.c CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST is used, and there are 4bit specific functions, so this define is also valid, just we need in drivers/mtd/nand/nand_spl_simple.c a more common name, right?
Right. Ideally, though, that would either be CONFIG_SYS_DAVINCI_... or #if defined(CONFIG_SYS_NAND_HW_ECC_OOBFIRST) && \
defined(CONFIG_SYS_NAND_HW_ECC_4BIT)
Hmm.. I thought you meant, this is not davinci nor 4bit specific? Or do you mean to rename the CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST define in drivers/mtd/nand/davinci_nand.c?

Quoted 6 lines

I just mean that "4bit" and "oobfirst" are independent things, so I'd rather not have something that combines the two as part of the configuration of the general NAND interface in the absence of a requirement to enumerate all possibilities. OTOH, the structure of driver's private config can be whatever makes the most sense for that driver, but should be named in a way that it's obviously driver-specific.

So my v4 version of this patch is ok? see:
http://patchwork.ozlabs.org/patch/121295/

Maybe we should rename the CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST in drivers/mtd/nand/davinci_nand.c to CONFIG_SYS_DAVINCI_NAND_HW_4BIT in a seperate patch?

Sandeep?

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Scott Wood

2011-10-28 16:03 UTC #14 lore raw

PATCH v3 5/10 spl, nand: add 4bit HW ecc oob first nand_read_page function

Quoted 22 lines · Heiko Schocher

Hello Scott,
Quoted 12 lines · Scott Wood
Quoted 10 lines · Heiko Schocher
Quoted 7 lines
Quoted 4 lines
In drivers/mtd/nand/davinci_nand.c CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST is used, and there are 4bit specific functions, so this define is also valid, just we need in drivers/mtd/nand/nand_spl_simple.c a more common name, right?
Right. Ideally, though, that would either be CONFIG_SYS_DAVINCI_... or #if defined(CONFIG_SYS_NAND_HW_ECC_OOBFIRST) && \
defined(CONFIG_SYS_NAND_HW_ECC_4BIT)
Hmm.. I thought you meant, this is not davinci nor 4bit specific? Or do you mean to rename the CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST define in drivers/mtd/nand/davinci_nand.c?
Quoted 6 lines
I just mean that "4bit" and "oobfirst" are independent things, so I'd rather not have something that combines the two as part of the configuration of the general NAND interface in the absence of a requirement to enumerate all possibilities. OTOH, the structure of driver's private config can be whatever makes the most sense for that driver, but should be named in a way that it's obviously driver-specific.

Quoted 2 lines

So my v4 version of this patch is ok? see:
http://patchwork.ozlabs.org/patch/121295/

Yes, the patch looks good -- ACK sent.

Quoted 3 lines

Maybe we should rename the CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST in drivers/mtd/nand/davinci_nand.c to CONFIG_SYS_DAVINCI_NAND_HW_4BIT in a seperate patch?

Sure, that'd be good.

-Scott