Re: [PATCH 4.13.0-rc6 1] mt29f_spinand: Fix to Enable the ECC for page read on program

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Aug 23, 2017 at 05:08:54PM -0400, Arun Nagendran wrote:
> staging:mt29f_spinand: Enable the ECC for page read on program.

Why is this line here?

> 
> we have to enable the ECC during page read in spinand_program_page function,
> because program page is over-writing only the specific data in the read page.
> at same time, if you turn off the ECC and read the page, we may read corrupted
> data with potential bit flips error.
> I verified this logic with GiGa Device part GD5F2GQ4RCYI, found this data
> corruption problem.
> 
> 
> --- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
> +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
> @@ -496,8 +496,12 @@ static int spinand_program_page(struct spi_device
> *spi_nand,
>         if (!wbuf)
>                 return -ENOMEM;
>  
> -       enable_read_hw_ecc = 0;
> -       spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
> +       enable_read_hw_ecc = 1;
> +       retval = spinand_read_page(spi_nand, page_id, 0, CACHE_BUF, wbuf);
> +       {

Why the { here?

> +               dev_err(&spi_nand->dev, "ecc error on read page!!!\n");
> +               return retval;

So now you always fail no matter what happens?

Please always test your patches :(

> +       }
>  
>         for (i = offset, j = 0; i < len; i++, j++)
>                 wbuf[i] &= buf[j];
> 
> 
> Signed-off-by: Arun Nagendran <arunrasppi@gmail.com>


This has to go up in the changelog area for it to work properly.

thanks,

greg k-h