| Message ID | 20180119135855.10673-2-miquel.raynal@free-electrons.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | Migrate the GPMI driver to use NAND core timings | expand |
On 01/19/2018 07:58 AM, Miquel Raynal wrote: > Choosing ONFI timings when ->onfi_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 ->onfi_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 > been selected. 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@free-electrons.com> > --- > drivers/mtd/nand/nand_base.c | 41 ++++++++++++++++++++++++++++++++++------- > 1 file changed, 34 insertions(+), 7 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 96c97588e1ba..e209a65a17e0 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -1231,15 +1231,47 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) > chip->onfi_timing_mode_default, > }; > > + chip->select_chip(mtd, chipnr); > ret = chip->onfi_set_features(mtd, chip, > ONFI_FEATURE_ADDR_TIMING_MODE, > 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->onfi_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: > + onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0); > + chip->select_chip(mtd, chipnr); > + nand_reset_op(chip); > + chip->select_chip(mtd, -1); > + > return ret; > } > > @@ -2738,10 +2770,8 @@ int nand_reset(struct nand_chip *chip, int chipnr) > if (ret) > return ret; > > - chip->select_chip(mtd, chipnr); > chip->data_interface = saved_data_intf; > ret = nand_setup_data_interface(chip, chipnr); > - chip->select_chip(mtd, -1); > if (ret) > return ret; > > @@ -6571,10 +6601,7 @@ int nand_scan_tail(struct mtd_info *mtd) > > /* Enter fastest possible mode on all dies. */ > for (i = 0; i < chip->numchips; i++) { > - chip->select_chip(mtd, i); > ret = nand_setup_data_interface(chip, i); > - chip->select_chip(mtd, -1); > - > if (ret) > goto err_nand_manuf_cleanup; > } I am happy with the patch set, just don't get time to test them on my side. Will ack your next split patches. Tested-by: Han Xu <han.xu@nxp.com>
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 96c97588e1ba..e209a65a17e0 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -1231,15 +1231,47 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) chip->onfi_timing_mode_default, }; + chip->select_chip(mtd, chipnr); ret = chip->onfi_set_features(mtd, chip, ONFI_FEATURE_ADDR_TIMING_MODE, 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->onfi_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: + onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0); + chip->select_chip(mtd, chipnr); + nand_reset_op(chip); + chip->select_chip(mtd, -1); + return ret; } @@ -2738,10 +2770,8 @@ int nand_reset(struct nand_chip *chip, int chipnr) if (ret) return ret; - chip->select_chip(mtd, chipnr); chip->data_interface = saved_data_intf; ret = nand_setup_data_interface(chip, chipnr); - chip->select_chip(mtd, -1); if (ret) return ret; @@ -6571,10 +6601,7 @@ int nand_scan_tail(struct mtd_info *mtd) /* Enter fastest possible mode on all dies. */ for (i = 0; i < chip->numchips; i++) { - chip->select_chip(mtd, i); ret = nand_setup_data_interface(chip, i); - chip->select_chip(mtd, -1); - if (ret) goto err_nand_manuf_cleanup; }
Choosing ONFI timings when ->onfi_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 ->onfi_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 been selected. 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@free-electrons.com> --- drivers/mtd/nand/nand_base.c | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-)