[U-Boot] Possible bug in NAND driver

If we refer to the following code snippet from nand_util.c
rval = nand_read (nand, offset, &read_length, p_buffer);
if (rval != 0) {
printf ("NAND read from offset %llx failed %d\n",
offset, rval);
*length -= left_to_read;
return rval;
}
The above code will return failure even after ECC errors are corrected.
This is because of the following line of code in nand_base.c
return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
This is in the nand_do_read_ops in nand_bsae.c
I see that changing
if (rval != 0)
to
if (rval != 0 && rval != -EUCLEAN )
solves the problem.
I can submit a patch if required.
Thanks, Sandeep

You are right, this is a bug.
I've already fixed it in our code tree some monthes ago but forgotten to send the patch to the list.
Best regards, Valeriy Glushkov
----- Original Message ----- From: "Paulraj, Sandeep" s-paulraj@ti.com To: u-boot@lists.denx.de Sent: 13 ???? 2009 ?. 22:34 Subject: [U-Boot] Possible bug in NAND driver
If we refer to the following code snippet from nand_util.c
rval = nand_read (nand, offset, &read_length, p_buffer);
if (rval != 0) { printf ("NAND read from offset %llx failed %d\n", offset, rval); *length -= left_to_read; return rval; }
The above code will return failure even after ECC errors are corrected.
This is because of the following line of code in nand_base.c
return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
This is in the nand_do_read_ops in nand_bsae.c
I see that changing
if (rval != 0)
to
if (rval != 0 && rval != -EUCLEAN )
solves the problem.
I can submit a patch if required.
Thanks, Sandeep
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Signed-off-by: Valeriy Glushkov gvv@lstec.com Signed-off-by: Paulraj, Sandeep s-paulraj@ti.com --- drivers/mtd/nand/nand_util.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index fc16282..694ead6 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -567,10 +567,10 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
if (len_incl_bad == *length) { rval = nand_read (nand, offset, length, buffer); - if (rval != 0) - printf ("NAND read from offset %llx failed %d\n", - offset, rval); - + if (!rval || rval == -EUCLEAN) + return 0; + printf ("NAND read from offset %llx failed %d\n", + offset, rval); return rval; }
@@ -591,7 +591,7 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length, read_length = nand->erasesize - block_offset;
rval = nand_read (nand, offset, &read_length, p_buffer); - if (rval != 0) { + if (rval && rval != -EUCLEAN) { printf ("NAND read from offset %llx failed %d\n", offset, rval); *length -= left_to_read;

On Tue, Jul 14, 2009 at 01:51:10PM +0300, Valeriy Glushkov wrote:
Signed-off-by: Valeriy Glushkov gvv@lstec.com Signed-off-by: Paulraj, Sandeep s-paulraj@ti.com
Applied to u-boot-nand-flash.
drivers/mtd/nand/nand_util.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index fc16282..694ead6 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -567,10 +567,10 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
if (len_incl_bad == *length) { rval = nand_read (nand, offset, length, buffer);
if (rval != 0)
printf ("NAND read from offset %llx failed %d\n",
offset, rval);
if (!rval || rval == -EUCLEAN)
return 0;
printf ("NAND read from offset %llx failed %d\n",
offset, rval);
Out of curiosity, why invert the logic from if (error) print; return to if (!error) return; print; return?
-Scott

Hi Scott,
if (len_incl_bad == *length) { rval = nand_read (nand, offset, length, buffer);
- if (rval != 0)
- printf ("NAND read from offset %llx failed %d\n",
- offset, rval);
- if (!rval || rval == -EUCLEAN)
- return 0;
- printf ("NAND read from offset %llx failed %d\n",
- offset, rval);
Out of curiosity, why invert the logic from if (error) print; return to if (!error) return; print; return?
Because it looks a bit better for me than 2 other versions. And saves a line. :) ------- if (!rval || rval == -EUCLEAN) return 0; printf ("NAND read from offset %llx failed %d\n", offset, rval); return rval; -------- if (rval && rval != -EUCLEAN) { printf ("NAND read from offset %llx failed %d\n", offset, rval); return rval; } return 0; -------
if (rval && rval != -EUCLEAN) printf ("NAND read from offset %llx failed %d\n", offset, rval); else rval = 0; return rval; --------
Best regards, Valeriy Glushkov
participants (3)
-
Paulraj, Sandeep
-
Scott Wood
-
Valeriy Glushkov