Re: [PATCH v2] mtd: rawnand: micron: handle on-die "ECC-off" devices correctly

[Marco Felsch <m.felsch@xxxxxxxxxxxxxx>]

Hi Boris,

On 19-07-29 09:57, Boris Brezillon wrote:
> On Mon, 29 Jul 2019 09:06:52 +0200
> Marco Felsch <m.felsch@xxxxxxxxxxxxxx> wrote:
> 
> > Some devices are supposed to do not support on-die ECC but experience
> 
> 		^ are not supposed to support

Fixed both, thanks. I will keep you rb-tag okay?

Regards,
  Marco

> > shows that internal ECC machinery can actually be enabled through the
> > "SET FEATURE (EFh)" command, even if a read of the "READ ID Parameter
> > Tables" returns that it is not.
> > 
> > Currently, the driver checks the "READ ID Parameter" field directly
> > after having enabled the feature. If the check fails it returns
> > immediately but leaves the ECC on. When using buggy chips like
> > MT29F2G08ABAGA and MT29F2G08ABBGA, all future read/program cycles will
> > go through the on-die ECC, confusing the host controller which is
> > supposed to be the one handling correction.
> > 
> > To address this in a common way we need to turn off the on-die ECC
> > directly after reading the "READ ID Parameter" and before checking the
> > "ECC status".
> > 
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: dbc44edbf833 ("mtd: rawnand: micron: Fix on-die ECC detection logic")
> > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> > Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> > ---
> > v2:
> > - adapt commit message according Miquel comments
> > - add fixes, stable tags
> > - add Boris rb-tag
> > 
> >  drivers/mtd/nand/raw/nand_micron.c | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> > index 1622d3145587..fb199ad2f1a6 100644
> > --- a/drivers/mtd/nand/raw/nand_micron.c
> > +++ b/drivers/mtd/nand/raw/nand_micron.c
> > @@ -390,6 +390,14 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
> >  	    (chip->id.data[4] & MICRON_ID_INTERNAL_ECC_MASK) != 0x2)
> >  		return MICRON_ON_DIE_UNSUPPORTED;
> >  
> > +	/*
> > +	 * It seems that there are devices which do not support ECC official.
> 
> 								    ^officially.
> 
> > +	 * At least the MT29F2G08ABAGA / MT29F2G08ABBGA devices supports
> > +	 * enabling the ECC feature but don't reflect that to the READ_ID table.
> > +	 * So we have to guarantee that we disable the ECC feature directly
> > +	 * after we did the READ_ID table command. Later we can evaluate the
> > +	 * ECC_ENABLE support.
> > +	 */
> >  	ret = micron_nand_on_die_ecc_setup(chip, true);
> >  	if (ret)
> >  		return MICRON_ON_DIE_UNSUPPORTED;
> > @@ -398,13 +406,13 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
> >  	if (ret)
> >  		return MICRON_ON_DIE_UNSUPPORTED;
> >  
> > -	if (!(id[4] & MICRON_ID_ECC_ENABLED))
> > -		return MICRON_ON_DIE_UNSUPPORTED;
> > -
> >  	ret = micron_nand_on_die_ecc_setup(chip, false);
> >  	if (ret)
> >  		return MICRON_ON_DIE_UNSUPPORTED;
> >  
> > +	if (!(id[4] & MICRON_ID_ECC_ENABLED))
> > +		return MICRON_ON_DIE_UNSUPPORTED;
> > +
> >  	ret = nand_readid_op(chip, 0, id, sizeof(id));
> >  	if (ret)
> >  		return MICRON_ON_DIE_UNSUPPORTED;
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/