Re: [RESEND PATCH] mtd: davinci-nand: correct empty sector bit flips

[Brian Norris <computersforpeace@gmail.com>]

+ Boris

On Mon, Nov 02, 2015 at 10:59:17AM -0600, Mike Scherban wrote:
> Currently empty page bit flips are not corrected and report 0 errors.
> If there happens to be a bit flip, this will cause UBIFS to fail to
> mount with a "corruption in empty space" error. The only way to recover
> from this is to reflash the NAND partition.
> 
> This changes the empty page handling to count the number of bit flips
> and correct them if they are under or equal to the ECC strength. UBI
> will then resolve the mounting error without requiring a reflash.
> 
> Signed-off-by: Mike Scherban <m-scherban@ti.com>
> ---
>  drivers/mtd/nand/davinci_nand.c |   21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
> index feb6d18..8189d8a 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -316,12 +316,27 @@ static int nand_davinci_correct_4bit(struct mtd_info *mtd,
>  	unsigned num_errors, corrected;
>  	unsigned long timeo;
>  
> -	/* All bytes 0xff?  It's an erased page; ignore its ECC. */
> -	for (i = 0; i < 10; i++) {
> +	/* All bytes 0xff?  It's an erased page;
> +	 * Ignore its ECC, but check for bit flips.
> +	 */
> +	for (i = 0; i < info->chip.ecc.bytes; i++) {
>  		if (ecc_code[i] != 0xff)
>  			goto compare;
>  	}
> -	return 0;
> +
> +	/* Count bit flips in empty space. */
> +	for (i = 0, corrected = 0; i < info->chip.ecc.size; i++)
> +		corrected += hweight8(~data[i]);
> +
> +	/* If bit flips are above the ecc strength produce an error */
> +	if (corrected > info->chip.ecc.strength)
> +		return -EIO;
> +
> +	/* If there are bit flips correct the data */
> +	if (corrected)
> +		memset(data, 0xff, info->chip.ecc.size);
> +
> +	return corrected;

NAK. We don't need any more badly-done local hacks (it's not enough to
just check the in-band data bytes; you have to check the spare area
too). Please review/test this instead:

http://lists.infradead.org/pipermail/linux-mtd/2015-September/061617.html
http://thread.gmane.org/gmane.linux.drivers.mtd/61358

Regards,
Brian

>  
>  compare:
>  	/* Unpack ten bytes into eight 10 bit values.  We know we're
> -- 
> 1.7.9.5
>