[PATCH] mtd: nand: docg4: ecc.read_page() returns 0 on uncorrectible errors

[PATCH] mtd: nand: docg4: ecc.read_page() returns 0 on uncorrectible errors

[Mike Dunn]

Currently the docg4's ecc.read_page() method returns -EBADMSG when uncorrectible
bitflips occur.  This is wrong; 0 should be returned in this case.  An error
code should only be returned by this method in the case of a hardware error
(probably -EIO).

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
 drivers/mtd/nand/docg4.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index 793921e..deb718c 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -776,6 +776,8 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
 	}
 
 	writew(0, docptr + DOC_DATAEND);
+	if (bits_corrected == -EBADMSG)	  /* uncorrectible errors */
+		return 0;
 	return bits_corrected;
 }
 
-- 
1.7.3.4

Re: [PATCH] mtd: nand: docg4: ecc.read_page() returns 0 on uncorrectible errors

[Brian Norris]

On Tue, Sep 11, 2012 at 8:50 AM, Mike Dunn <mikedunn@newsguy.com> wrote:
> diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
> index 793921e..deb718c 100644
> --- a/drivers/mtd/nand/docg4.c
> +++ b/drivers/mtd/nand/docg4.c
> @@ -776,6 +776,8 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
>         }
>
>         writew(0, docptr + DOC_DATAEND);
> +       if (bits_corrected == -EBADMSG)   /* uncorrectible errors */

s/uncorrectible/uncorrectable/

> +               return 0;
>         return bits_corrected;
>  }
>

With that change:

Acked-by: Brian Norris <computersforpeace@gmail.com>

Also, not a blocker for this patch, but this made me look elsewhere in docg4.c:

static int __init read_factory_bbt(struct mtd_info *mtd)
{
...
        status = docg4_read_page(mtd, nand, buf, 0, DOCG4_FACTORY_BBT_PAGE);
        if (status)
                goto exit;

This could cause problems if there are ever bitflips on the BBT page.
Can the factory BBT contain bitflips?

Brian

Re: [PATCH] mtd: nand: docg4: ecc.read_page() returns 0 on uncorrectible errors

[Mike Dunn]

Hi Brian, thanks for the Ack.  It seems I was careless with my own driver when I
did the bitflip patches.

BTW, the consequence of this oversight was seen when using mtdchar (e.g.,
mtd-utils 'nanddump') when uncorrectAble bitflips occurred.  The userspace app
just kept repeating the read call, because the prior call indicated that no data
was read.


On 09/14/2012 08:20 PM, Brian Norris wrote:
>> +       if (bits_corrected == -EBADMSG)   /* uncorrectible errors */
> 
> s/uncorrectible/uncorrectable/


Ha!  I actualy looked the word up in the dictionary, and either spelling is
correct.  But yours is more common.


> 
>> +               return 0;
>>         return bits_corrected;
>>  }
>>
> 
> With that change:
> 
> Acked-by: Brian Norris <computersforpeace@gmail.com>
> 
> Also, not a blocker for this patch, but this made me look elsewhere in docg4.c:
> 
> static int __init read_factory_bbt(struct mtd_info *mtd)
> {
> ...
>         status = docg4_read_page(mtd, nand, buf, 0, DOCG4_FACTORY_BBT_PAGE);
>         if (status)
>                 goto exit;
> 
> This could cause problems if there are ever bitflips on the BBT page.
> Can the factory BBT contain bitflips?


Ah, yes, thank you for catching that!!  Overlooked when docg4_read_page() was
changed to return max bitflips; status should be tested for negative value.
Should also try to recover from uncorrectable bitflips (I believe that table is
stored redundantly).

Thanks again,
Mike

Re: [PATCH] mtd: nand: docg4: ecc.read_page() returns 0 on uncorrectible errors

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

On Tue, 2012-09-11 at 08:50 -0700, Mike Dunn wrote:
> Currently the docg4's ecc.read_page() method returns -EBADMSG when uncorrectible
> bitflips occur.  This is wrong; 0 should be returned in this case.  An error
> code should only be returned by this method in the case of a hardware error
> (probably -EIO).
> 
> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>

Pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]