On Thu, 21 Jun 2018 22:33:28 +1200 Chris Packham <[email protected]> wrote:
> Some Micron NAND chips have on-die ECC forceably enabled. The detect > these based on chip ID as there seems to be no other way of > distinguishing these chips from those that have optional support for > on-die ECC. > > When a chip with mandatory on-die ECC is detected change the current ECC > mode to on-die. > > Signed-off-by: Chris Packham <[email protected]> > --- > I'm not convinced that just changing chip->ecc.mode is sensible. An > alternative I considered was only proceeding if the ecc.mode is set to > ON_DIE. > > Changes in v4: > - New > > drivers/mtd/nand/raw/nand_micron.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_micron.c > b/drivers/mtd/nand/raw/nand_micron.c > index f1ecd4986b50..9ba9007a9a06 100644 > --- a/drivers/mtd/nand/raw/nand_micron.c > +++ b/drivers/mtd/nand/raw/nand_micron.c > @@ -227,6 +227,14 @@ enum { > MICRON_ON_DIE_MANDATORY, > }; > > +/* > + * These parts are known to have on-die ECC forceably enabled > + */ > +static u8 micron_on_die_ecc[] = { > + 0xd1, /* MT29F1G08ABAFA */ > + 0xa1, /* MT29F1G08ABBFA */ > +}; > + > /* > * Try to detect if the NAND support on-die ECC. To do this, we enable > * the feature, and read back if it has been enabled as expected. We > @@ -241,6 +249,11 @@ static int micron_supports_on_die_ecc(struct nand_chip > *chip) > { > u8 feature[ONFI_SUBFEATURE_PARAM_LEN] = { 0, }; > int ret; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(micron_on_die_ecc); i++) > + if (chip->id.data[1] == micron_on_die_ecc[i]) > + return MICRON_ON_DIE_MANDATORY; > > if (!chip->parameters.onfi.version) > return MICRON_ON_DIE_UNSUPPORTED; > @@ -289,8 +302,8 @@ static int micron_nand_init(struct nand_chip *chip) > ondie = micron_supports_on_die_ecc(chip); > > if (ondie == MICRON_ON_DIE_MANDATORY) { > - pr_err("On-die ECC forcefully enabled, not supported\n"); > - return -EINVAL; > + pr_info("On-die ECC forcefully enabled\n"); > + chip->ecc.mode = NAND_ECC_ON_DIE; No, you can't do that, because most NAND drivers simply don't support NAND_ECC_ON_DIE, and will override chip->ecc.mode without even checking its value. You should just fail if chip->ecc.mode != NAND_ECC_ON_DIE. if (chip->ecc.mode != NAND_ECC_ON_DIE) { pr_err("On-die ECC can't be disabled\n"); return -EINVAL; } > } > > if (chip->ecc.mode == NAND_ECC_ON_DIE) {
