| Message ID | 20180302142422.2543-6-miquel.raynal@bootlin.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | Improve timings handling in the NAND framework | expand |
On Fri, 2 Mar 2018 15:24:13 +0100 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Choosing ONFI timings when ->set/get_features() calls are supported > by the NAND chip is a matter of reading the chip's ONFI parameter page > and telling the chip the chosen mode (between all of the supported ones) > with ->set_feature(). > > Add a check on whether the chip "acked" the timing mode or not. > > This can be a problem for NAND chips that do not follow entirely the > ONFI specification. These chips actually support other modes than > "mode 0", but do not update the parameter page once a timing mode has ^ s/parameter page/timing mode register/ > been selected. To be honest, I don't think this is the problem here. I guess those chips simply don't support the TIMING_MODE feature (so the problem arise at SET_FEATURES time, no GET_FEATURES) and thus don't require users to change the timing mode at all. > This issue will be addressed in another patch that will > add the feature to overwrite NAND chips features within the parameter > page, from the NAND chip driver. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > Tested-by: Han Xu <han.xu@nxp.com> > --- > drivers/mtd/nand/raw/nand_base.c | 35 +++++++++++++++++++++++++++++++++-- > 1 file changed, 33 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 8e48f08d8496..47f77e846edc 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -1237,11 +1237,42 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) > tmode_param); > chip->select_chip(mtd, -1); > if (ret) > - goto err; > + return ret; > } > > ret = chip->setup_data_interface(mtd, chipnr, &chip->data_interface); > -err: > + if (ret) > + return ret; > + > + if (chip->onfi_version && > + (le16_to_cpu(chip->onfi_params.opt_cmd) & > + ONFI_OPT_CMD_SET_GET_FEATURES)) { > + u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {}; > + > + chip->select_chip(mtd, chipnr); > + ret = chip->get_features(mtd, chip, > + ONFI_FEATURE_ADDR_TIMING_MODE, > + tmode_param); > + chip->select_chip(mtd, -1); > + if (ret) > + goto err_reset_chip; > + > + if (tmode_param[0] != chip->onfi_timing_mode_default) { > + pr_warn("timings mode %d not acknowledged by the NAND chip\n", > + chip->onfi_timing_mode_default); > + goto err_reset_chip; > + } > + } > + > + return 0; > + > +err_reset_chip: > + /* Fallback to timing mode 0 */ > + nand_reset_data_interface(chip, chipnr); > + chip->select_chip(mtd, chipnr); > + nand_reset_op(chip); > + chip->select_chip(mtd, -1); > + > return ret; > } >
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 8e48f08d8496..47f77e846edc 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -1237,11 +1237,42 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) tmode_param); chip->select_chip(mtd, -1); if (ret) - goto err; + return ret; } ret = chip->setup_data_interface(mtd, chipnr, &chip->data_interface); -err: + if (ret) + return ret; + + if (chip->onfi_version && + (le16_to_cpu(chip->onfi_params.opt_cmd) & + ONFI_OPT_CMD_SET_GET_FEATURES)) { + u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {}; + + chip->select_chip(mtd, chipnr); + ret = chip->get_features(mtd, chip, + ONFI_FEATURE_ADDR_TIMING_MODE, + tmode_param); + chip->select_chip(mtd, -1); + if (ret) + goto err_reset_chip; + + if (tmode_param[0] != chip->onfi_timing_mode_default) { + pr_warn("timings mode %d not acknowledged by the NAND chip\n", + chip->onfi_timing_mode_default); + goto err_reset_chip; + } + } + + return 0; + +err_reset_chip: + /* Fallback to timing mode 0 */ + nand_reset_data_interface(chip, chipnr); + chip->select_chip(mtd, chipnr); + nand_reset_op(chip); + chip->select_chip(mtd, -1); + return ret; }