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 |