Re: [2/3] mtd: rawnand: micron: Disable ECC earlier in the read path

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Mon, 16 Jul 2018 18:10:57 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> On Mon, 16 Jul 2018 09:00:59 +0000
> Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:
> 
> > Hi Boris,
> > 
> > On 04/07/18 00:20, Boris Brezillon wrote:  
> > > We are about to support extracting the real number of bitflips for
> > > 4-bits ECC when WRITE_RECOMMEND is returned. This requires re-reading
> > > the page in raw mode to compare it to the corrected version, and this
> > > logic will be placed in micron_nand_on_die_ecc_status_4().
> > > 
> > > Moving the micron_nand_on_die_ecc_setup() will allow us to disable
> > > ECC only once.
> > > 
> > > As a result, we have to rework the exit path and add an error path
> > > where the ECC is disabled.
> > > 
> > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>    
> > 
> > As I said on the other thread this appears to cause a problem for me on 
> > the MT29F1G08ABAFAWP-ITE setup I have. I notice we're not able to find 
> > the BBT, not sure if that is symptom or cause.

It's most likely the symptom, not the cause.

> > > 
> > > diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> > > index 63ac98a36ed7..b9cbaf125a98 100644
> > > --- a/drivers/mtd/nand/raw/nand_micron.c
> > > +++ b/drivers/mtd/nand/raw/nand_micron.c
> > > @@ -197,30 +197,37 @@ micron_nand_read_page_on_die_ecc(struct mtd_info *mtd, struct nand_chip *chip,
> > >   
> > >   	ret = nand_read_page_op(chip, page, 0, NULL, 0);
> > >   	if (ret)
> > > -		goto out;
> > > +		goto err_disable_ecc;
> > >   
> > >   	ret = nand_status_op(chip, &status);
> > >   	if (ret)
> > > -		goto out;
> > > +		goto err_disable_ecc;
> > >   
> > >   	ret = nand_exit_status_op(chip);
> > >   	if (ret)
> > > -		goto out;
> > > +		goto err_disable_ecc;
> > >   
> > > -	if (chip->ecc.strength == 4)
> > > -		max_bitflips = micron_nand_on_die_ecc_status_4(chip, status);
> > > -	else
> > > -		max_bitflips = micron_nand_on_die_ecc_status_8(chip, status);
> > > +	micron_nand_on_die_ecc_setup(chip, false);  
> 
> Hm, can you try to move the micron_nand_on_die_ecc_setup(chip, false)
> call just before nand_exit_status_op()?
> 

Just pushed a branch fixing that [1]. Can you test it? If it works,
I'll ask Miquel to drop the initial set of patches and instead pick the
fixed ones so that we don't break bisectibility.

[1]https://github.com/bbrezillon/linux-0day/commits/nand/next