[PATCH v3 00/14] Improve timings handling in the NAND framework

[PATCH v3 00/14] Improve timings handling in the NAND framework

[Miquel Raynal]

Hello,

This series evolves how GET/SET_FEATURES operations are handled by the
NAND core by adding two helpers that should be called instead of calling
directly the ->onfi_get/set_feature() hooks.

Then, the parameter page that was always allocated no matter if the NAND
core could make use of it is dropped. Instead, a much smaller structure
is embedded in the nand_chip structure and just stores the information
that will be useful for the core. Future enhancements of the core that
will need more information will just have to add a new entry in this
structure and fill it during the detection phase. A bitmap is used to
know which optional feature is actually supported.

This give the possibility in vendor code to overload this bitmap and
force the addition/removal of supported features in the list, depending
for instance on the NAND chip currently being used.

This new possibility is effectively used for Macronix NAND chip
MX30LF2G18AC. This chip supports natively the timing mode 5, and its
parameter page indicates that it supports GET/SET_FEATURES on timing
modes, while in reality it does not. Removing this feature from the
supported bitmap makes it usable at high speed instead of unnecessarily
limiting it to timing mode 0.

Thank you,
Miquèl

Changes since v2:
=================
  - Squashed the series with another preparation series not yet merged
    about GPMI timings improvements. Core patches are here now, while a
    second series with only GPMI-related changes will be sent very soon.
  - s/support_setting_features/supports_set_get_features/
  - Used a bitmap for set_features and another for get_features.
  - Removed repetiting setting the timings mode parameters bit in the
    supported features bitmap in Micron driver.
  - Split the commit removing the parameter pages in three (one that
    adds the new structure, one that get rid of JEDEC parameter page,
    one that get rid of ONFI parameter page).
  - Moved the ONFI-specific fields in the nand_parameters structure
    into an onfi_parameters structure within it. It is statically
    allocated today, and will move to dynamic allocation in the near
    future.
  - Removed the __packed attribute on the nand_parameters structure.

Changes since v1:
=================
  - Some changes, a bit unrelated to what we try to achieve here have
    been moved to the series that prepares this one.
  - Change the formula in Macronix code that determines if we are
    dealing with the failing chip.


Miquel Raynal (14):
  mtd: rawnand: rename default ->onfi_get/set_features() implementations
  mtd: rawnand: rename SET/GET FEATURES related functions
  mtd: rawnand: mxc: rename the local ->set/get_features()
    implementation
  mtd: rawnand: move calls to ->select_chip() in
    nand_setup_data_interface()
  mtd: rawnand: check ONFI timings have been acked by the chip
  mtd: rawnand: avoid setting again the timings to mode 0 after a reset
  mtd: rawnand: use wrappers to call onfi GET/SET_FEATURES
  mtd: rawnand: mxc: remove useless checks in GET/SET_FEATURES functions
  mtd: rawnand: prepare the removal of ONFI/JEDEC parameter pages
  mtd: rawnand: prepare the removal of the ONFI parameter page
  mtd: rawnand: allow vendors to declare (un)supported features
  mtd: rawnand: macronix: nack the support of changing timings for one
    chip
  mtd: rawnand: get rid of the JEDEC parameter page in nand_chip
  mtd: rawnand: get rid of the ONFI parameter page in nand_chip

 drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c |   4 +-
 drivers/mtd/nand/raw/cafe_nand.c                 |   4 +-
 drivers/mtd/nand/raw/docg4.c                     |   4 +-
 drivers/mtd/nand/raw/fsl_elbc_nand.c             |   4 +-
 drivers/mtd/nand/raw/fsl_ifc_nand.c              |   4 +-
 drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c        |   8 +-
 drivers/mtd/nand/raw/hisi504_nand.c              |   4 +-
 drivers/mtd/nand/raw/mpc5121_nfc.c               |   4 +-
 drivers/mtd/nand/raw/mxc_nand.c                  |  24 +-
 drivers/mtd/nand/raw/nand_base.c                 | 282 ++++++++++++++++-------
 drivers/mtd/nand/raw/nand_macronix.c             |  10 +
 drivers/mtd/nand/raw/nand_micron.c               |  39 ++--
 drivers/mtd/nand/raw/nand_timings.c              |  10 +-
 drivers/mtd/nand/raw/pxa3xx_nand.c               |   4 +-
 drivers/mtd/nand/raw/qcom_nandc.c                |   4 +-
 drivers/mtd/nand/raw/sh_flctl.c                  |   4 +-
 drivers/mtd/nand/raw/vf610_nfc.c                 |   4 +-
 drivers/staging/mt29f_spinand/mt29f_spinand.c    |   4 +-
 include/linux/mtd/rawnand.h                      |  76 +++---
 19 files changed, 307 insertions(+), 190 deletions(-)

-- 
2.14.1

[PATCH v3 01/14] mtd: rawnand: rename default ->onfi_get/set_features() implementations

[Miquel Raynal]

Prepare future work on the ->onfi_get/set_features() hooks by renaming
the core's implementation as 'default' ones.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index e70ca16a5118..d1d5e2281860 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4769,14 +4769,15 @@ static int nand_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len)
 }
 
 /**
- * nand_onfi_set_features- [REPLACEABLE] set features for ONFI nand
+ * nand_default_onfi_set_features- [REPLACEABLE] set features for ONFI nand
  * @mtd: MTD device structure
  * @chip: nand chip info structure
  * @addr: feature address.
  * @subfeature_param: the subfeature parameters, a four bytes array.
  */
-static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
-			int addr, uint8_t *subfeature_param)
+static int nand_default_onfi_set_features(struct mtd_info *mtd,
+					  struct nand_chip *chip, int addr,
+					  uint8_t *subfeature_param)
 {
 	if (!chip->onfi_version ||
 	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
@@ -4787,14 +4788,15 @@ static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /**
- * nand_onfi_get_features- [REPLACEABLE] get features for ONFI nand
+ * nand_default_onfi_get_features- [REPLACEABLE] get features for ONFI nand
  * @mtd: MTD device structure
  * @chip: nand chip info structure
  * @addr: feature address.
  * @subfeature_param: the subfeature parameters, a four bytes array.
  */
-static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
-			int addr, uint8_t *subfeature_param)
+static int nand_default_onfi_get_features(struct mtd_info *mtd,
+					  struct nand_chip *chip, int addr,
+					  uint8_t *subfeature_param)
 {
 	if (!chip->onfi_version ||
 	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
@@ -4879,9 +4881,9 @@ static void nand_set_defaults(struct nand_chip *chip)
 
 	/* set for ONFI nand */
 	if (!chip->onfi_set_features)
-		chip->onfi_set_features = nand_onfi_set_features;
+		chip->onfi_set_features = nand_default_onfi_set_features;
 	if (!chip->onfi_get_features)
-		chip->onfi_get_features = nand_onfi_get_features;
+		chip->onfi_get_features = nand_default_onfi_get_features;
 
 	/* If called twice, pointers that depend on busw may need to be reset */
 	if (!chip->read_byte || chip->read_byte == nand_read_byte)
-- 
2.14.1

[PATCH v3 02/14] mtd: rawnand: rename SET/GET FEATURES related functions

[Miquel Raynal]

SET/GET FEATURES are flagged ONFI-compliant because of their name. This
is not accurate as non-ONFI NAND chips support it and use it.

Rename the hooks and helpers to remove the "onfi" prefix.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c |  4 +--
 drivers/mtd/nand/raw/cafe_nand.c                 |  4 +--
 drivers/mtd/nand/raw/docg4.c                     |  4 +--
 drivers/mtd/nand/raw/fsl_elbc_nand.c             |  4 +--
 drivers/mtd/nand/raw/fsl_ifc_nand.c              |  4 +--
 drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c        |  8 ++---
 drivers/mtd/nand/raw/hisi504_nand.c              |  4 +--
 drivers/mtd/nand/raw/mpc5121_nfc.c               |  4 +--
 drivers/mtd/nand/raw/mxc_nand.c                  |  4 +--
 drivers/mtd/nand/raw/nand_base.c                 | 42 +++++++++++-------------
 drivers/mtd/nand/raw/nand_micron.c               | 16 ++++-----
 drivers/mtd/nand/raw/pxa3xx_nand.c               |  4 +--
 drivers/mtd/nand/raw/qcom_nandc.c                |  4 +--
 drivers/mtd/nand/raw/sh_flctl.c                  |  4 +--
 drivers/mtd/nand/raw/vf610_nfc.c                 |  4 +--
 drivers/staging/mt29f_spinand/mt29f_spinand.c    |  4 +--
 include/linux/mtd/rawnand.h                      | 17 +++++-----
 17 files changed, 66 insertions(+), 69 deletions(-)

diff --git a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
index 54bac5b73f0a..60874de430eb 100644
--- a/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
+++ b/drivers/mtd/nand/raw/bcm47xxnflash/ops_bcm4706.c
@@ -392,8 +392,8 @@ int bcm47xxnflash_ops_bcm4706_init(struct bcm47xxnflash *b47n)
 	b47n->nand_chip.read_byte = bcm47xxnflash_ops_bcm4706_read_byte;
 	b47n->nand_chip.read_buf = bcm47xxnflash_ops_bcm4706_read_buf;
 	b47n->nand_chip.write_buf = bcm47xxnflash_ops_bcm4706_write_buf;
-	b47n->nand_chip.onfi_set_features = nand_onfi_get_set_features_notsupp;
-	b47n->nand_chip.onfi_get_features = nand_onfi_get_set_features_notsupp;
+	b47n->nand_chip.set_features = nand_get_set_features_notsupp;
+	b47n->nand_chip.get_features = nand_get_set_features_notsupp;
 
 	nand_chip->chip_delay = 50;
 	b47n->nand_chip.bbt_options = NAND_BBT_USE_FLASH;
diff --git a/drivers/mtd/nand/raw/cafe_nand.c b/drivers/mtd/nand/raw/cafe_nand.c
index 567ff972d5fc..b4c4032a2d83 100644
--- a/drivers/mtd/nand/raw/cafe_nand.c
+++ b/drivers/mtd/nand/raw/cafe_nand.c
@@ -645,8 +645,8 @@ static int cafe_nand_probe(struct pci_dev *pdev,
 	cafe->nand.read_buf = cafe_read_buf;
 	cafe->nand.write_buf = cafe_write_buf;
 	cafe->nand.select_chip = cafe_select_chip;
-	cafe->nand.onfi_set_features = nand_onfi_get_set_features_notsupp;
-	cafe->nand.onfi_get_features = nand_onfi_get_set_features_notsupp;
+	cafe->nand.set_features = nand_get_set_features_notsupp;
+	cafe->nand.get_features = nand_get_set_features_notsupp;
 
 	cafe->nand.chip_delay = 0;
 
diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
index 72f1327c4430..1314aa99b9ab 100644
--- a/drivers/mtd/nand/raw/docg4.c
+++ b/drivers/mtd/nand/raw/docg4.c
@@ -1269,8 +1269,8 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
 	nand->read_buf = docg4_read_buf;
 	nand->write_buf = docg4_write_buf16;
 	nand->erase = docg4_erase_block;
-	nand->onfi_set_features = nand_onfi_get_set_features_notsupp;
-	nand->onfi_get_features = nand_onfi_get_set_features_notsupp;
+	nand->set_features = nand_get_set_features_notsupp;
+	nand->get_features = nand_get_set_features_notsupp;
 	nand->ecc.read_page = docg4_read_page;
 	nand->ecc.write_page = docg4_write_page;
 	nand->ecc.read_page_raw = docg4_read_page_raw;
diff --git a/drivers/mtd/nand/raw/fsl_elbc_nand.c b/drivers/mtd/nand/raw/fsl_elbc_nand.c
index 8b6dcd739ecb..d59ec6572548 100644
--- a/drivers/mtd/nand/raw/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_elbc_nand.c
@@ -775,8 +775,8 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 	chip->select_chip = fsl_elbc_select_chip;
 	chip->cmdfunc = fsl_elbc_cmdfunc;
 	chip->waitfunc = fsl_elbc_wait;
-	chip->onfi_set_features = nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features = nand_onfi_get_set_features_notsupp;
+	chip->set_features = nand_get_set_features_notsupp;
+	chip->get_features = nand_get_set_features_notsupp;
 
 	chip->bbt_td = &bbt_main_descr;
 	chip->bbt_md = &bbt_mirror_descr;
diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c
index 4872a7ba6503..7bcd95ff38c9 100644
--- a/drivers/mtd/nand/raw/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c
@@ -838,8 +838,8 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
 	chip->select_chip = fsl_ifc_select_chip;
 	chip->cmdfunc = fsl_ifc_cmdfunc;
 	chip->waitfunc = fsl_ifc_wait;
-	chip->onfi_set_features = nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features = nand_onfi_get_set_features_notsupp;
+	chip->set_features = nand_get_set_features_notsupp;
+	chip->get_features = nand_get_set_features_notsupp;
 
 	chip->bbt_td = &bbt_main_descr;
 	chip->bbt_md = &bbt_mirror_descr;
diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c
index 97787246af41..f78d907ca61e 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-lib.c
@@ -934,15 +934,15 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
 
 	/* [1] send SET FEATURE command to NAND */
 	feature[0] = mode;
-	ret = nand->onfi_set_features(mtd, nand,
-				ONFI_FEATURE_ADDR_TIMING_MODE, feature);
+	ret = nand->set_features(mtd, nand,
+				 ONFI_FEATURE_ADDR_TIMING_MODE, feature);
 	if (ret)
 		goto err_out;
 
 	/* [2] send GET FEATURE command to double-check the timing mode */
 	memset(feature, 0, ONFI_SUBFEATURE_PARAM_LEN);
-	ret = nand->onfi_get_features(mtd, nand,
-				ONFI_FEATURE_ADDR_TIMING_MODE, feature);
+	ret = nand->get_features(mtd, nand,
+				 ONFI_FEATURE_ADDR_TIMING_MODE, feature);
 	if (ret || feature[0] != mode)
 		goto err_out;
 
diff --git a/drivers/mtd/nand/raw/hisi504_nand.c b/drivers/mtd/nand/raw/hisi504_nand.c
index cb862793ab6d..27558a67fa41 100644
--- a/drivers/mtd/nand/raw/hisi504_nand.c
+++ b/drivers/mtd/nand/raw/hisi504_nand.c
@@ -762,8 +762,8 @@ static int hisi_nfc_probe(struct platform_device *pdev)
 	chip->write_buf		= hisi_nfc_write_buf;
 	chip->read_buf		= hisi_nfc_read_buf;
 	chip->chip_delay	= HINFC504_CHIP_DELAY;
-	chip->onfi_set_features	= nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features	= nand_onfi_get_set_features_notsupp;
+	chip->set_features	= nand_get_set_features_notsupp;
+	chip->get_features	= nand_get_set_features_notsupp;
 
 	hisi_nfc_host_init(host);
 
diff --git a/drivers/mtd/nand/raw/mpc5121_nfc.c b/drivers/mtd/nand/raw/mpc5121_nfc.c
index 913b9d1225c6..6d1740d54e0d 100644
--- a/drivers/mtd/nand/raw/mpc5121_nfc.c
+++ b/drivers/mtd/nand/raw/mpc5121_nfc.c
@@ -707,8 +707,8 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	chip->read_buf = mpc5121_nfc_read_buf;
 	chip->write_buf = mpc5121_nfc_write_buf;
 	chip->select_chip = mpc5121_nfc_select_chip;
-	chip->onfi_set_features	= nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features	= nand_onfi_get_set_features_notsupp;
+	chip->set_features	= nand_get_set_features_notsupp;
+	chip->get_features	= nand_get_set_features_notsupp;
 	chip->bbt_options = NAND_BBT_USE_FLASH;
 	chip->ecc.mode = NAND_ECC_SOFT;
 	chip->ecc.algo = NAND_ECC_HAMMING;
diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
index 87b5ee66e501..d331305f45c1 100644
--- a/drivers/mtd/nand/raw/mxc_nand.c
+++ b/drivers/mtd/nand/raw/mxc_nand.c
@@ -1752,8 +1752,8 @@ static int mxcnd_probe(struct platform_device *pdev)
 	this->read_word = mxc_nand_read_word;
 	this->write_buf = mxc_nand_write_buf;
 	this->read_buf = mxc_nand_read_buf;
-	this->onfi_set_features = mxc_nand_onfi_set_features;
-	this->onfi_get_features = mxc_nand_onfi_get_features;
+	this->set_features = mxc_nand_onfi_set_features;
+	this->get_features = mxc_nand_onfi_get_features;
 
 	host->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk))
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d1d5e2281860..802cd9ed44a1 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -349,7 +349,7 @@ static void nand_write_byte16(struct mtd_info *mtd, uint8_t byte)
 	 *    8-bits of the data bus. During address transfers, the host shall
 	 *    set the upper 8-bits of the data bus to 00h.
 	 *
-	 * One user of the write_byte callback is nand_onfi_set_features. The
+	 * One user of the write_byte callback is nand_set_features. The
 	 * four parameters are specified to be written to I/O[7:0], but this is
 	 * neither an address nor a command transfer. Let's assume a 0 on the
 	 * upper I/O lines is OK.
@@ -1231,9 +1231,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
 			chip->onfi_timing_mode_default,
 		};
 
-		ret = chip->onfi_set_features(mtd, chip,
-				ONFI_FEATURE_ADDR_TIMING_MODE,
-				tmode_param);
+		ret = chip->set_features(mtd, chip,
+					 ONFI_FEATURE_ADDR_TIMING_MODE,
+					 tmode_param);
 		if (ret)
 			goto err;
 	}
@@ -4769,15 +4769,15 @@ static int nand_max_bad_blocks(struct mtd_info *mtd, loff_t ofs, size_t len)
 }
 
 /**
- * nand_default_onfi_set_features- [REPLACEABLE] set features for ONFI nand
+ * nand_default_set_features- [REPLACEABLE] set NAND chip features
  * @mtd: MTD device structure
  * @chip: nand chip info structure
  * @addr: feature address.
  * @subfeature_param: the subfeature parameters, a four bytes array.
  */
-static int nand_default_onfi_set_features(struct mtd_info *mtd,
-					  struct nand_chip *chip, int addr,
-					  uint8_t *subfeature_param)
+static int nand_default_set_features(struct mtd_info *mtd,
+				     struct nand_chip *chip, int addr,
+				     uint8_t *subfeature_param)
 {
 	if (!chip->onfi_version ||
 	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
@@ -4788,15 +4788,15 @@ static int nand_default_onfi_set_features(struct mtd_info *mtd,
 }
 
 /**
- * nand_default_onfi_get_features- [REPLACEABLE] get features for ONFI nand
+ * nand_default_get_features- [REPLACEABLE] get NAND chip features
  * @mtd: MTD device structure
  * @chip: nand chip info structure
  * @addr: feature address.
  * @subfeature_param: the subfeature parameters, a four bytes array.
  */
-static int nand_default_onfi_get_features(struct mtd_info *mtd,
-					  struct nand_chip *chip, int addr,
-					  uint8_t *subfeature_param)
+static int nand_default_get_features(struct mtd_info *mtd,
+				     struct nand_chip *chip, int addr,
+				     uint8_t *subfeature_param)
 {
 	if (!chip->onfi_version ||
 	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
@@ -4807,8 +4807,7 @@ static int nand_default_onfi_get_features(struct mtd_info *mtd,
 }
 
 /**
- * nand_onfi_get_set_features_notsupp - set/get features stub returning
- *					-ENOTSUPP
+ * nand_get_set_features_notsupp - set/get features stub returning -ENOTSUPP
  * @mtd: MTD device structure
  * @chip: nand chip info structure
  * @addr: feature address.
@@ -4817,13 +4816,12 @@ static int nand_default_onfi_get_features(struct mtd_info *mtd,
  * Should be used by NAND controller drivers that do not support the SET/GET
  * FEATURES operations.
  */
-int nand_onfi_get_set_features_notsupp(struct mtd_info *mtd,
-				       struct nand_chip *chip, int addr,
-				       u8 *subfeature_param)
+int nand_get_set_features_notsupp(struct mtd_info *mtd, struct nand_chip *chip,
+				  int addr, u8 *subfeature_param)
 {
 	return -ENOTSUPP;
 }
-EXPORT_SYMBOL(nand_onfi_get_set_features_notsupp);
+EXPORT_SYMBOL(nand_get_set_features_notsupp);
 
 /**
  * nand_suspend - [MTD Interface] Suspend the NAND flash
@@ -4880,10 +4878,10 @@ static void nand_set_defaults(struct nand_chip *chip)
 		chip->select_chip = nand_select_chip;
 
 	/* set for ONFI nand */
-	if (!chip->onfi_set_features)
-		chip->onfi_set_features = nand_default_onfi_set_features;
-	if (!chip->onfi_get_features)
-		chip->onfi_get_features = nand_default_onfi_get_features;
+	if (!chip->set_features)
+		chip->set_features = nand_default_set_features;
+	if (!chip->get_features)
+		chip->get_features = nand_default_get_features;
 
 	/* If called twice, pointers that depend on busw may need to be reset */
 	if (!chip->read_byte || chip->read_byte == nand_read_byte)
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 02e109ae73f1..ab3e3a1a5212 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -48,8 +48,8 @@ static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	u8 feature[ONFI_SUBFEATURE_PARAM_LEN] = {retry_mode};
 
-	return chip->onfi_set_features(mtd, chip, ONFI_FEATURE_ADDR_READ_RETRY,
-				       feature);
+	return chip->set_features(mtd, chip, ONFI_FEATURE_ADDR_READ_RETRY,
+				  feature);
 }
 
 /*
@@ -108,8 +108,8 @@ static int micron_nand_on_die_ecc_setup(struct nand_chip *chip, bool enable)
 	if (enable)
 		feature[0] |= ONFI_FEATURE_ON_DIE_ECC_EN;
 
-	return chip->onfi_set_features(nand_to_mtd(chip), chip,
-				       ONFI_FEATURE_ON_DIE_ECC, feature);
+	return chip->set_features(nand_to_mtd(chip), chip,
+				  ONFI_FEATURE_ON_DIE_ECC, feature);
 }
 
 static int
@@ -219,8 +219,8 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	if (ret)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
-	chip->onfi_get_features(nand_to_mtd(chip), chip,
-				ONFI_FEATURE_ON_DIE_ECC, feature);
+	chip->get_features(nand_to_mtd(chip), chip,
+			   ONFI_FEATURE_ON_DIE_ECC, feature);
 	if ((feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN) == 0)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
@@ -228,8 +228,8 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	if (ret)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
-	chip->onfi_get_features(nand_to_mtd(chip), chip,
-				ONFI_FEATURE_ON_DIE_ECC, feature);
+	chip->get_features(nand_to_mtd(chip), chip,
+			   ONFI_FEATURE_ON_DIE_ECC, feature);
 	if (feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN)
 		return MICRON_ON_DIE_MANDATORY;
 
diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c b/drivers/mtd/nand/raw/pxa3xx_nand.c
index d75f30263d21..9f22b48e38dc 100644
--- a/drivers/mtd/nand/raw/pxa3xx_nand.c
+++ b/drivers/mtd/nand/raw/pxa3xx_nand.c
@@ -1824,8 +1824,8 @@ static int alloc_nand_resource(struct platform_device *pdev)
 		chip->write_buf		= pxa3xx_nand_write_buf;
 		chip->options		|= NAND_NO_SUBPAGE_WRITE;
 		chip->cmdfunc		= nand_cmdfunc;
-		chip->onfi_set_features	= nand_onfi_get_set_features_notsupp;
-		chip->onfi_get_features	= nand_onfi_get_set_features_notsupp;
+		chip->set_features	= nand_get_set_features_notsupp;
+		chip->get_features	= nand_get_set_features_notsupp;
 	}
 
 	nand_hw_control_init(chip->controller);
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 563b759ffca6..b554fb6e609c 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2651,8 +2651,8 @@ static int qcom_nand_host_init(struct qcom_nand_controller *nandc,
 	chip->read_byte		= qcom_nandc_read_byte;
 	chip->read_buf		= qcom_nandc_read_buf;
 	chip->write_buf		= qcom_nandc_write_buf;
-	chip->onfi_set_features	= nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features	= nand_onfi_get_set_features_notsupp;
+	chip->set_features	= nand_get_set_features_notsupp;
+	chip->get_features	= nand_get_set_features_notsupp;
 
 	/*
 	 * the bad block marker is readable only when we read the last codeword
diff --git a/drivers/mtd/nand/raw/sh_flctl.c b/drivers/mtd/nand/raw/sh_flctl.c
index c4e7755448e6..9eb3cf9db111 100644
--- a/drivers/mtd/nand/raw/sh_flctl.c
+++ b/drivers/mtd/nand/raw/sh_flctl.c
@@ -1180,8 +1180,8 @@ static int flctl_probe(struct platform_device *pdev)
 	nand->read_buf = flctl_read_buf;
 	nand->select_chip = flctl_select_chip;
 	nand->cmdfunc = flctl_cmdfunc;
-	nand->onfi_set_features = nand_onfi_get_set_features_notsupp;
-	nand->onfi_get_features = nand_onfi_get_set_features_notsupp;
+	nand->set_features = nand_get_set_features_notsupp;
+	nand->get_features = nand_get_set_features_notsupp;
 
 	if (pdata->flcmncr_val & SEL_16BIT)
 		nand->options |= NAND_BUSWIDTH_16;
diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
index 5d7a1f8f580f..7c20915f2165 100644
--- a/drivers/mtd/nand/raw/vf610_nfc.c
+++ b/drivers/mtd/nand/raw/vf610_nfc.c
@@ -687,8 +687,8 @@ static int vf610_nfc_probe(struct platform_device *pdev)
 	chip->read_buf = vf610_nfc_read_buf;
 	chip->write_buf = vf610_nfc_write_buf;
 	chip->select_chip = vf610_nfc_select_chip;
-	chip->onfi_set_features = nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features = nand_onfi_get_set_features_notsupp;
+	chip->set_features = nand_get_set_features_notsupp;
+	chip->get_features = nand_get_set_features_notsupp;
 
 	chip->options |= NAND_NO_SUBPAGE_WRITE;
 
diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index 264ad362d858..6819dd2c1117 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -918,8 +918,8 @@ static int spinand_probe(struct spi_device *spi_nand)
 	chip->waitfunc	= spinand_wait;
 	chip->options	|= NAND_CACHEPRG;
 	chip->select_chip = spinand_select_chip;
-	chip->onfi_set_features = nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features = nand_onfi_get_set_features_notsupp;
+	chip->set_features = nand_get_set_features_notsupp;
+	chip->get_features = nand_get_set_features_notsupp;
 
 	mtd = nand_to_mtd(chip);
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 56c5570aadbe..fb2e288ef8b1 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1170,8 +1170,8 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
  * @blocks_per_die:	[INTERN] The number of PEBs in a die
  * @data_interface:	[INTERN] NAND interface timing information
  * @read_retries:	[INTERN] the number of read retry modes supported
- * @onfi_set_features:	[REPLACEABLE] set the features for ONFI nand
- * @onfi_get_features:	[REPLACEABLE] get the features for ONFI nand
+ * @set_features:	[REPLACEABLE] set the NAND chip features
+ * @get_features:	[REPLACEABLE] get the NAND chip features
  * @setup_data_interface: [OPTIONAL] setup the data interface and timing. If
  *			  chipnr is set to %NAND_DATA_IFACE_CHECK_ONLY this
  *			  means the configuration should not be applied but
@@ -1212,10 +1212,10 @@ struct nand_chip {
 		       bool check_only);
 	int (*erase)(struct mtd_info *mtd, int page);
 	int (*scan_bbt)(struct mtd_info *mtd);
-	int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip *chip,
-			int feature_addr, uint8_t *subfeature_para);
-	int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,
-			int feature_addr, uint8_t *subfeature_para);
+	int (*set_features)(struct mtd_info *mtd, struct nand_chip *chip,
+			    int feature_addr, uint8_t *subfeature_para);
+	int (*get_features)(struct mtd_info *mtd, struct nand_chip *chip,
+			    int feature_addr, uint8_t *subfeature_para);
 	int (*setup_read_retry)(struct mtd_info *mtd, int retry_mode);
 	int (*setup_data_interface)(struct mtd_info *mtd, int chipnr,
 				    const struct nand_data_interface *conf);
@@ -1630,9 +1630,8 @@ int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 			   int page);
 
 /* Stub used by drivers that do not support GET/SET FEATURES operations */
-int nand_onfi_get_set_features_notsupp(struct mtd_info *mtd,
-				       struct nand_chip *chip, int addr,
-				       u8 *subfeature_param);
+int nand_get_set_features_notsupp(struct mtd_info *mtd, struct nand_chip *chip,
+				  int addr, u8 *subfeature_param);
 
 /* Default read_page_raw implementation */
 int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-- 
2.14.1

[PATCH v3 03/14] mtd: rawnand: mxc: rename the local ->set/get_features() implementation

[Miquel Raynal]

MXC NAND driver embeds its own implementation of the
->set/get_features() hooks. These hooks were once flagged "onfi" but a
lot of not-ONFI compliant NAND chips use it so they have been renamed
to remove the "onfi" relationship. Rename these functions the same way
for consistency.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/mxc_nand.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
index d331305f45c1..61501654e708 100644
--- a/drivers/mtd/nand/raw/mxc_nand.c
+++ b/drivers/mtd/nand/raw/mxc_nand.c
@@ -1421,9 +1421,8 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 	}
 }
 
-static int mxc_nand_onfi_set_features(struct mtd_info *mtd,
-				      struct nand_chip *chip, int addr,
-				      u8 *subfeature_param)
+static int mxc_nand_set_features(struct mtd_info *mtd, struct nand_chip *chip,
+				 int addr, u8 *subfeature_param)
 {
 	struct nand_chip *nand_chip = mtd_to_nand(mtd);
 	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
@@ -1447,9 +1446,8 @@ static int mxc_nand_onfi_set_features(struct mtd_info *mtd,
 	return 0;
 }
 
-static int mxc_nand_onfi_get_features(struct mtd_info *mtd,
-				      struct nand_chip *chip, int addr,
-				      u8 *subfeature_param)
+static int mxc_nand_get_features(struct mtd_info *mtd, struct nand_chip *chip,
+				 int addr, u8 *subfeature_param)
 {
 	struct nand_chip *nand_chip = mtd_to_nand(mtd);
 	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
@@ -1752,8 +1750,8 @@ static int mxcnd_probe(struct platform_device *pdev)
 	this->read_word = mxc_nand_read_word;
 	this->write_buf = mxc_nand_write_buf;
 	this->read_buf = mxc_nand_read_buf;
-	this->set_features = mxc_nand_onfi_set_features;
-	this->get_features = mxc_nand_onfi_get_features;
+	this->set_features = mxc_nand_set_features;
+	this->get_features = mxc_nand_get_features;
 
 	host->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(host->clk))
-- 
2.14.1

[PATCH v3 04/14] mtd: rawnand: move calls to ->select_chip() in nand_setup_data_interface()

[Miquel Raynal]

After a ->set_features(TIMINGS), the chip is supposed to be working at a
new speed. In order for all the transactions to be perperly handled, the
NAND controller should also be configured to this same speed. Calling
->setup_data_interface() is not enough and the chip should be
de-asserted/re-asserted through calls to ->select_chip().

Prepare the next change in nand_setup_data_interface() where timings
will be checked after being applied. Because assertions of the CS pin
will be needed from within this function, move the calls to
->select_chip() inside nand_setup_data_interface() for later
consistency.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 802cd9ed44a1..8e48f08d8496 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1231,9 +1231,11 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
 			chip->onfi_timing_mode_default,
 		};
 
+		chip->select_chip(mtd, chipnr);
 		ret = chip->set_features(mtd, chip,
 					 ONFI_FEATURE_ADDR_TIMING_MODE,
 					 tmode_param);
+		chip->select_chip(mtd, -1);
 		if (ret)
 			goto err;
 	}
@@ -2739,10 +2741,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;
 
@@ -6474,10 +6474,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;
 	}
-- 
2.14.1

[PATCH v3 05/14] mtd: rawnand: check ONFI timings have been acked by the chip

[Miquel Raynal]

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
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@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;
 }
 
-- 
2.14.1

[PATCH v3 06/14] mtd: rawnand: avoid setting again the timings to mode 0 after a reset

[Miquel Raynal]

After a nand_reset_data_interface(), both the NAND chip and the NAND
controller use timing mode 0. The previously defined data interface for
this chip has been saved and is supposed to be restored after that.
However, if the saved data interface also refers to timing mode 0, there
is no need to re-apply them again.

Also, as nand_setup_data_interface() uses ->set/get_features(), it could
lead to issues when doing the reset at probe time as the parameter page
is not available yet to know if these functions are supported or not.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 47f77e846edc..70b59672362c 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2772,6 +2772,16 @@ int nand_reset(struct nand_chip *chip, int chipnr)
 	if (ret)
 		return ret;
 
+	/*
+	 * A nand_reset_data_interface() put both the NAND chip and the NAND
+	 * controller in timings mode 0. If the default mode for this chip is
+	 * also 0, no need to proceed to the change again. Plus, at probe time,
+	 * nand_setup_data_interface() uses ->set/get_features() which would
+	 * fail anyway as the parameter page is not available yet.
+	 */
+	if (!chip->onfi_timing_mode_default)
+		return 0;
+
 	chip->data_interface = saved_data_intf;
 	ret = nand_setup_data_interface(chip, chipnr);
 	if (ret)
-- 
2.14.1

[PATCH v3 07/14] mtd: rawnand: use wrappers to call onfi GET/SET_FEATURES

[Miquel Raynal]

Prepare the fact that some features managed by GET/SET_FEATURES could be
overloaded by vendor code. To handle this logic, use new wrappers
instead of directly call the ->get/set_features() hooks.

Also take into account that not having the proper SET/GET_FEATURES
available is not a reason to return an error. The wrappers that are
created here handle this case by returning a special error code:
-ENOTSUPP. More logic in the calling function of the new helpers
(nand_setup_data_interface()) is added to handle this case).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c   | 127 ++++++++++++++++++++++++-------------
 drivers/mtd/nand/raw/nand_micron.c |  18 +++---
 include/linux/mtd/rawnand.h        |   3 +
 3 files changed, 96 insertions(+), 52 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 70b59672362c..e5bcfbf7c7f6 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1160,6 +1160,52 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
 	return status;
 }
 
+/**
+ * nand_get_features - wrapper to perform a GET_FEATURE
+ * @chip: NAND chip info structure
+ * @addr: feature address
+ * @subfeature_param: the subfeature parameters, a four bytes array
+ *
+ * Returns 0 for success, a negative error otherwise. Returns -ENOTSUPP if the
+ * operation cannot be handled.
+ */
+int nand_get_features(struct nand_chip *chip, int addr,
+		      u8 *subfeature_param)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	if (!chip->onfi_version ||
+	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
+	      & ONFI_OPT_CMD_SET_GET_FEATURES))
+		return -ENOTSUPP;
+
+	return chip->get_features(mtd, chip, addr, subfeature_param);
+}
+EXPORT_SYMBOL_GPL(nand_get_features);
+
+/**
+ * nand_set_features - wrapper to perform a SET_FEATURE
+ * @chip: NAND chip info structure
+ * @addr: feature address
+ * @subfeature_param: the subfeature parameters, a four bytes array
+ *
+ * Returns 0 for success, a negative error otherwise. Returns -ENOTSUPP if the
+ * operation cannot be handled.
+ */
+int nand_set_features(struct nand_chip *chip, int addr,
+		      u8 *subfeature_param)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+
+	if (!chip->onfi_version ||
+	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
+	      & ONFI_OPT_CMD_SET_GET_FEATURES))
+		return -ENOTSUPP;
+
+	return chip->set_features(mtd, chip, addr, subfeature_param);
+}
+EXPORT_SYMBOL_GPL(nand_set_features);
+
 /**
  * nand_reset_data_interface - Reset data interface and timings
  * @chip: The NAND chip
@@ -1215,59 +1261,62 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
 static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
+		chip->onfi_timing_mode_default,
+	};
 	int ret;
 
 	if (!chip->setup_data_interface)
 		return 0;
 
+	/* Change the mode on the chip side */
+	chip->select_chip(mtd, chipnr);
+	ret = nand_set_features(chip, ONFI_FEATURE_ADDR_TIMING_MODE,
+				tmode_param);
+	chip->select_chip(mtd, -1);
+
 	/*
-	 * Ensure the timing mode has been changed on the chip side
-	 * before changing timings on the controller side.
+	 * Do not fail because the mode cannot explicitly be set. If the NAND
+	 * chip claims it supports it, let's just apply the timings on the
+	 * controller side.
 	 */
-	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->onfi_timing_mode_default,
-		};
-
-		chip->select_chip(mtd, chipnr);
-		ret = chip->set_features(mtd, chip,
-					 ONFI_FEATURE_ADDR_TIMING_MODE,
-					 tmode_param);
-		chip->select_chip(mtd, -1);
-		if (ret)
-			return ret;
-	}
+	if (ret && ret != -ENOTSUPP)
+		return ret;
 
+	/* Change the mode on the controller side */
 	ret = chip->setup_data_interface(mtd, chipnr, &chip->data_interface);
 	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] = {};
+	/* Check the mode has been accepted by the chip */
+	memset(tmode_param, 0, ONFI_SUBFEATURE_PARAM_LEN);
+	chip->select_chip(mtd, chipnr);
+	ret = nand_get_features(chip, ONFI_FEATURE_ADDR_TIMING_MODE,
+				tmode_param);
+	chip->select_chip(mtd, -1);
+	if (ret && ret != -ENOTSUPP)
+		goto err_reset_chip;
 
-		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;
+	/*
+	 * Do not fail because the mode could not be checked. However, skip the
+	 * comparison block that would probably raise an error.
+	 */
+	if (ret == -ENOTSUPP)
+		return 0;
 
-		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;
-		}
+	if (tmode_param[0] != chip->onfi_timing_mode_default) {
+		pr_warn("timing 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 */
+	/*
+	 * Fallback to mode 0 if the chip explicitly did not ack the chosen
+	 * timing mode.
+	 */
 	nand_reset_data_interface(chip, chipnr);
 	chip->select_chip(mtd, chipnr);
 	nand_reset_op(chip);
@@ -4820,11 +4869,6 @@ static int nand_default_set_features(struct mtd_info *mtd,
 				     struct nand_chip *chip, int addr,
 				     uint8_t *subfeature_param)
 {
-	if (!chip->onfi_version ||
-	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
-	      & ONFI_OPT_CMD_SET_GET_FEATURES))
-		return -EINVAL;
-
 	return nand_set_features_op(chip, addr, subfeature_param);
 }
 
@@ -4839,11 +4883,6 @@ static int nand_default_get_features(struct mtd_info *mtd,
 				     struct nand_chip *chip, int addr,
 				     uint8_t *subfeature_param)
 {
-	if (!chip->onfi_version ||
-	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
-	      & ONFI_OPT_CMD_SET_GET_FEATURES))
-		return -EINVAL;
-
 	return nand_get_features_op(chip, addr, subfeature_param);
 }
 
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index ab3e3a1a5212..b825656f6284 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -48,8 +48,7 @@ static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	u8 feature[ONFI_SUBFEATURE_PARAM_LEN] = {retry_mode};
 
-	return chip->set_features(mtd, chip, ONFI_FEATURE_ADDR_READ_RETRY,
-				  feature);
+	return nand_set_features(chip, ONFI_FEATURE_ADDR_READ_RETRY, feature);
 }
 
 /*
@@ -108,8 +107,7 @@ static int micron_nand_on_die_ecc_setup(struct nand_chip *chip, bool enable)
 	if (enable)
 		feature[0] |= ONFI_FEATURE_ON_DIE_ECC_EN;
 
-	return chip->set_features(nand_to_mtd(chip), chip,
-				  ONFI_FEATURE_ON_DIE_ECC, feature);
+	return nand_set_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
 }
 
 static int
@@ -219,8 +217,10 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	if (ret)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
-	chip->get_features(nand_to_mtd(chip), chip,
-			   ONFI_FEATURE_ON_DIE_ECC, feature);
+	ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
+	if (ret < 0)
+		return ret;
+
 	if ((feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN) == 0)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
@@ -228,8 +228,10 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	if (ret)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
-	chip->get_features(nand_to_mtd(chip), chip,
-			   ONFI_FEATURE_ON_DIE_ECC, feature);
+	ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
+	if (ret < 0)
+		return ret;
+
 	if (feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN)
 		return MICRON_ON_DIE_MANDATORY;
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index fb2e288ef8b1..3cc2a3435b20 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1629,6 +1629,9 @@ int nand_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page);
 int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 			   int page);
 
+/* Wrapper to use in order for controllers/vendors to GET/SET FEATURES */
+int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
+int nand_set_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
 /* Stub used by drivers that do not support GET/SET FEATURES operations */
 int nand_get_set_features_notsupp(struct mtd_info *mtd, struct nand_chip *chip,
 				  int addr, u8 *subfeature_param);
-- 
2.14.1

[PATCH v3 08/14] mtd: rawnand: mxc: remove useless checks in GET/SET_FEATURES functions

[Miquel Raynal]

All the calls to the chip's hooks ->get/set_features() go through
the core's wrappers nand_get/set_features() that already do the
necessary checks about feature support. Remove these
checks from the mxc's functions.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/mxc_nand.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
index 61501654e708..944ecf03d3b1 100644
--- a/drivers/mtd/nand/raw/mxc_nand.c
+++ b/drivers/mtd/nand/raw/mxc_nand.c
@@ -1428,11 +1428,6 @@ static int mxc_nand_set_features(struct mtd_info *mtd, struct nand_chip *chip,
 	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
 	int i;
 
-	if (!chip->onfi_version ||
-	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
-	      & ONFI_OPT_CMD_SET_GET_FEATURES))
-		return -EINVAL;
-
 	host->buf_start = 0;
 
 	for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
@@ -1453,11 +1448,6 @@ static int mxc_nand_get_features(struct mtd_info *mtd, struct nand_chip *chip,
 	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
 	int i;
 
-	if (!chip->onfi_version ||
-	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
-	      & ONFI_OPT_CMD_SET_GET_FEATURES))
-		return -EINVAL;
-
 	host->devtype_data->send_cmd(host, NAND_CMD_GET_FEATURES, false);
 	mxc_do_addr_cycle(mtd, addr, -1);
 	host->devtype_data->send_page(mtd, NFC_OUTPUT);
-- 
2.14.1

[PATCH v3 09/14] mtd: rawnand: prepare the removal of ONFI/JEDEC parameter pages

[Miquel Raynal]

The NAND chip parameter page is statically allocated within the
nand_chip structure, which reserves a lot of space. Even not ONFI nor
JEDEC chips have it embedded. Also, only a few parameters are actually
read from the parameter page after the detection.

To prepare to the removal of such huge structure, a small NAND parameter
structure is allocated statically and contains only very few members
that are generic to all chips and actually used elsewhere in the code.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 32 +++++++++++++-------------------
 include/linux/mtd/rawnand.h      |  6 ++++++
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index e5bcfbf7c7f6..30364f60dc4d 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1174,9 +1174,7 @@ int nand_get_features(struct nand_chip *chip, int addr,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 
-	if (!chip->onfi_version ||
-	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
-	      & ONFI_OPT_CMD_SET_GET_FEATURES))
+	if (!chip->parameters.supports_set_get_features)
 		return -ENOTSUPP;
 
 	return chip->get_features(mtd, chip, addr, subfeature_param);
@@ -1197,9 +1195,7 @@ int nand_set_features(struct nand_chip *chip, int addr,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 
-	if (!chip->onfi_version ||
-	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
-	      & ONFI_OPT_CMD_SET_GET_FEATURES))
+	if (!chip->parameters.supports_set_get_features)
 		return -ENOTSUPP;
 
 	return chip->set_features(mtd, chip, addr, subfeature_param);
@@ -5150,8 +5146,9 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 
 	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
 	sanitize_string(p->model, sizeof(p->model));
+	memcpy(chip->parameters.model, p->model, sizeof(p->model));
 	if (!mtd->name)
-		mtd->name = p->model;
+		mtd->name = chip->parameters.model;
 
 	mtd->writesize = le32_to_cpu(p->byte_per_page);
 
@@ -5198,6 +5195,10 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 		pr_warn("Could not retrieve ONFI ECC requirements\n");
 	}
 
+	/* Save some parameters from the parameter page for future use */
+	if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_SET_GET_FEATURES)
+		chip->parameters.supports_set_get_features = true;
+
 	return 1;
 }
 
@@ -5250,8 +5251,9 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
 
 	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
 	sanitize_string(p->model, sizeof(p->model));
+	memcpy(chip->parameters.model, p->model, sizeof(p->model));
 	if (!mtd->name)
-		mtd->name = p->model;
+		mtd->name = chip->parameters.model;
 
 	mtd->writesize = le32_to_cpu(p->byte_per_page);
 
@@ -5652,17 +5654,9 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 
 	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
 		maf_id, dev_id);
-
-	if (chip->onfi_version)
-		pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
-			chip->onfi_params.model);
-	else if (chip->jedec_version)
-		pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
-			chip->jedec_params.model);
-	else
-		pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
-			type->name);
-
+	pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
+		(chip->onfi_version || chip->jedec_version) ?
+		chip->parameters.model : type->name);
 	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
 		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
 		mtd->erasesize >> 10, mtd->writesize, mtd->oobsize);
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 3cc2a3435b20..1af0bff58ff4 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -429,6 +429,11 @@ struct nand_jedec_params {
 	__le16 crc;
 } __packed;
 
+struct nand_parameters {
+	char model[100];
+	bool supports_set_get_features;
+};
+
 /* The maximum expected count of bytes in the NAND ID sequence */
 #define NAND_MAX_ID_LEN 8
 
@@ -1249,6 +1254,7 @@ struct nand_chip {
 		struct nand_onfi_params	onfi_params;
 		struct nand_jedec_params jedec_params;
 	};
+	struct nand_parameters parameters;
 	u16 max_bb_per_die;
 	u32 blocks_per_die;
 
-- 
2.14.1

[PATCH v3 10/14] mtd: rawnand: prepare the removal of the ONFI parameter page

[Miquel Raynal]

The NAND chip parameter page is statically allocated within the
nand_chip structure, which reserves a lot of space. Even not ONFI nor
JEDEC chips have it embedded. Also, only a few parameters are actually
read from the parameter page after the detection.

ONFI-related parameters that will be used outside from the
identification function are stored in a separate onfi_parameters
structure embedded in nand_parameters, this small structure that
already hold generic parameters.

For now, the onfi_parameters structure is allocated statically. However,
after some deep rework in the NAND framework, it will be possible to do
dynamic allocations from the NAND identification phase, and this
strcuture will then be dynamically allocated when needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c    | 14 ++++++++++++--
 drivers/mtd/nand/raw/nand_micron.c  | 17 +++++++----------
 drivers/mtd/nand/raw/nand_timings.c | 10 +++++-----
 include/linux/mtd/rawnand.h         | 29 +++++++++++++++--------------
 4 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 30364f60dc4d..f92e2b8fb2d9 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5170,14 +5170,14 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 	chip->max_bb_per_die = le16_to_cpu(p->bb_per_lun);
 	chip->blocks_per_die = le32_to_cpu(p->blocks_per_lun);
 
-	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
+	if (le16_to_cpu(p->features) & ONFI_FEATURE_16_BIT_BUS)
 		chip->options |= NAND_BUSWIDTH_16;
 
 	if (p->ecc_bits != 0xff) {
 		chip->ecc_strength_ds = p->ecc_bits;
 		chip->ecc_step_ds = 512;
 	} else if (chip->onfi_version >= 21 &&
-		(onfi_feature(chip) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
+		(le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
 
 		/*
 		 * The nand_flash_detect_ext_param_page() uses the
@@ -5198,6 +5198,16 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 	/* Save some parameters from the parameter page for future use */
 	if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_SET_GET_FEATURES)
 		chip->parameters.supports_set_get_features = true;
+	chip->parameters.onfi_params.t_prog = le16_to_cpu(p->t_prog);
+	chip->parameters.onfi_params.t_bers = le16_to_cpu(p->t_bers);
+	chip->parameters.onfi_params.t_r = le16_to_cpu(p->t_r);
+	chip->parameters.onfi_params.t_ccs = le16_to_cpu(p->t_ccs);
+	chip->parameters.onfi_params.async_timing_mode =
+		le16_to_cpu(p->async_timing_mode);
+	chip->parameters.onfi_params.vendor_revision =
+		le16_to_cpu(p->vendor_revision);
+	memcpy(chip->parameters.onfi_params.vendor, p->vendor,
+	       sizeof(p->vendor));
 
 	return 1;
 }
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index b825656f6284..becfda0a28cc 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -56,17 +56,14 @@ static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
  */
 static int micron_nand_onfi_init(struct nand_chip *chip)
 {
-	struct nand_onfi_params *p = &chip->onfi_params;
-	struct nand_onfi_vendor_micron *micron = (void *)p->vendor;
+	struct nand_parameters *p = &chip->parameters;
+	struct nand_onfi_vendor_micron *micron = (void *)p->onfi_params.vendor;
 
-	if (!chip->onfi_version)
-		return 0;
+	if (chip->onfi_version && p->onfi_params.vendor_revision) {
+		chip->read_retries = micron->read_retry_options;
+		chip->setup_read_retry = micron_nand_setup_read_retry;
+	}
 
-	if (le16_to_cpu(p->vendor_revision) < 1)
-		return 0;
-
-	chip->read_retries = micron->read_retry_options;
-	chip->setup_read_retry = micron_nand_setup_read_retry;
 
 	return 0;
 }
@@ -239,7 +236,7 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	 * Some Micron NANDs have an on-die ECC of 4/512, some other
 	 * 8/512. We only support the former.
 	 */
-	if (chip->onfi_params.ecc_bits != 4)
+	if (chip->ecc_strength_ds != 4)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
 	return MICRON_ON_DIE_SUPPORTED;
diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index 9400d039ddbd..b97bcf29f75a 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -307,16 +307,16 @@ int onfi_fill_data_interface(struct nand_chip *chip,
 	 * These information are part of the ONFI parameter page.
 	 */
 	if (chip->onfi_version) {
-		struct nand_onfi_params *params = &chip->onfi_params;
+		struct nand_parameters *params = &chip->parameters;
 		struct nand_sdr_timings *timings = &iface->timings.sdr;
 
 		/* microseconds -> picoseconds */
-		timings->tPROG_max = 1000000ULL * le16_to_cpu(params->t_prog);
-		timings->tBERS_max = 1000000ULL * le16_to_cpu(params->t_bers);
-		timings->tR_max = 1000000ULL * le16_to_cpu(params->t_r);
+		timings->tPROG_max = 1000000ULL * params->onfi_params.t_prog;
+		timings->tBERS_max = 1000000ULL * params->onfi_params.t_bers;
+		timings->tR_max = 1000000ULL * params->onfi_params.t_r;
 
 		/* nanoseconds -> picoseconds */
-		timings->tCCS_min = 1000UL * le16_to_cpu(params->t_ccs);
+		timings->tCCS_min = 1000UL * params->onfi_params.t_ccs;
 	}
 
 	return 0;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 1af0bff58ff4..81236b3fbc6f 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -429,9 +429,23 @@ struct nand_jedec_params {
 	__le16 crc;
 } __packed;
 
+struct onfi_params {
+	u16 t_prog;
+	u16 t_bers;
+	u16 t_r;
+	u16 t_ccs;
+	u16 async_timing_mode;
+	u16 vendor_revision;
+	u8 vendor[88];
+};
+
 struct nand_parameters {
+	/* Generic parameters */
 	char model[100];
 	bool supports_set_get_features;
+
+	/* ONFI parameters */
+	struct onfi_params onfi_params;
 };
 
 /* The maximum expected count of bytes in the NAND ID sequence */
@@ -1541,26 +1555,13 @@ struct platform_nand_data {
 	struct platform_nand_ctrl ctrl;
 };
 
-/* return the supported features. */
-static inline int onfi_feature(struct nand_chip *chip)
-{
-	return chip->onfi_version ? le16_to_cpu(chip->onfi_params.features) : 0;
-}
-
 /* return the supported asynchronous timing mode. */
 static inline int onfi_get_async_timing_mode(struct nand_chip *chip)
 {
 	if (!chip->onfi_version)
 		return ONFI_TIMING_MODE_UNKNOWN;
-	return le16_to_cpu(chip->onfi_params.async_timing_mode);
-}
 
-/* return the supported synchronous timing mode. */
-static inline int onfi_get_sync_timing_mode(struct nand_chip *chip)
-{
-	if (!chip->onfi_version)
-		return ONFI_TIMING_MODE_UNKNOWN;
-	return le16_to_cpu(chip->onfi_params.src_sync_timing_mode);
+	return chip->parameters.onfi_params.async_timing_mode;
 }
 
 int onfi_fill_data_interface(struct nand_chip *chip,
-- 
2.14.1

[PATCH v3 11/14] mtd: rawnand: allow vendors to declare (un)supported features

[Miquel Raynal]

If SET/GET_FEATURES is available (from the parameter page), use a
bitmap to declare what feature is actually supported.

Initialize the bitmap in the core to support timing changes (only
feature used by the core), also add support for Micron specific features
used in Micron initialization code (in the init routine).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c   | 13 ++++++++++---
 drivers/mtd/nand/raw/nand_micron.c |  4 ++++
 include/linux/mtd/rawnand.h        |  6 +++++-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index f92e2b8fb2d9..2d8cbe8eb78b 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1174,7 +1174,8 @@ int nand_get_features(struct nand_chip *chip, int addr,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 
-	if (!chip->parameters.supports_set_get_features)
+	if (!chip->parameters.supports_set_get_features ||
+	    !test_bit(addr, chip->parameters.get_feature_list))
 		return -ENOTSUPP;
 
 	return chip->get_features(mtd, chip, addr, subfeature_param);
@@ -1195,7 +1196,8 @@ int nand_set_features(struct nand_chip *chip, int addr,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 
-	if (!chip->parameters.supports_set_get_features)
+	if (!chip->parameters.supports_set_get_features ||
+	    !test_bit(addr, chip->parameters.set_feature_list))
 		return -ENOTSUPP;
 
 	return chip->set_features(mtd, chip, addr, subfeature_param);
@@ -5196,8 +5198,13 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 	}
 
 	/* Save some parameters from the parameter page for future use */
-	if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_SET_GET_FEATURES)
+	if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_SET_GET_FEATURES) {
 		chip->parameters.supports_set_get_features = true;
+		bitmap_set(chip->parameters.get_feature_list,
+			   ONFI_FEATURE_ADDR_TIMING_MODE, 1);
+		bitmap_set(chip->parameters.set_feature_list,
+			   ONFI_FEATURE_ADDR_TIMING_MODE, 1);
+	}
 	chip->parameters.onfi_params.t_prog = le16_to_cpu(p->t_prog);
 	chip->parameters.onfi_params.t_bers = le16_to_cpu(p->t_bers);
 	chip->parameters.onfi_params.t_r = le16_to_cpu(p->t_r);
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index becfda0a28cc..651129023257 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -64,6 +64,10 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
 		chip->setup_read_retry = micron_nand_setup_read_retry;
 	}
 
+	if (p->supports_set_get_features) {
+		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->set_feature_list);
+		set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->get_feature_list);
+	}
 
 	return 0;
 }
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 81236b3fbc6f..d2105352033a 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -21,6 +21,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/flashchip.h>
 #include <linux/mtd/bbm.h>
+#include <linux/types.h>
 
 struct mtd_info;
 struct nand_flash_dev;
@@ -235,7 +236,8 @@ struct nand_chip;
 #define ONFI_TIMING_MODE_5		(1 << 5)
 #define ONFI_TIMING_MODE_UNKNOWN	(1 << 6)
 
-/* ONFI feature address */
+/* ONFI feature number/address */
+#define ONFI_FEATURE_NUMBER		256
 #define ONFI_FEATURE_ADDR_TIMING_MODE	0x1
 
 /* Vendor-specific feature address (Micron) */
@@ -443,6 +445,8 @@ struct nand_parameters {
 	/* Generic parameters */
 	char model[100];
 	bool supports_set_get_features;
+	DECLARE_BITMAP(set_feature_list, ONFI_FEATURE_NUMBER);
+	DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
 
 	/* ONFI parameters */
 	struct onfi_params onfi_params;
-- 
2.14.1

[PATCH v3 12/14] mtd: rawnand: macronix: nack the support of changing timings for one chip

[Miquel Raynal]

The MX30LF2G18AC chip declares in its parameter page supporting
SET/GET_FEATURES but when it comes to timings, experience shows that it
is not the case.

Unflag this feature for this particular chip in the nand_parameters
structure to avoid unnecessary errors and downturns.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_macronix.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index d290ff2a6d2f..deae93d590b5 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -22,6 +22,16 @@ static int macronix_nand_init(struct nand_chip *chip)
 	if (nand_is_slc(chip))
 		chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
 
+	/*
+	 * MX30LF2G18AC chip does not support using SET/GET_FEATURES to change
+	 * the timings unlike what is declared in the parameter page. Unflag
+	 * this feature to avoid unnecessary downturns.
+	 */
+	if (chip->parameters.supports_set_get_features &&
+	    !strcmp("MX30LF2G18AC", chip->parameters.model))
+		bitmap_clear(chip->parameters.get_feature_list,
+			     ONFI_FEATURE_ADDR_TIMING_MODE, 1);
+
 	return 0;
 }
 
-- 
2.14.1

[PATCH v3 13/14] mtd: rawnand: get rid of the JEDEC parameter page in nand_chip

[Miquel Raynal]

The NAND chip parameter page is statically allocated within the
nand_chip structure, which reserves a lot of space. Even not ONFI nor
JEDEC chips have it embedded. Also, only a few parameters are actually
read from the parameter page after the detection.

Now that there is a small nand_parameters structure that can held
generic parameters, remove the JEDEC page from the nand_chip structure
by just allocating it during the identification phase and removing it
right after.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 34 ++++++++++++++++++++++++----------
 include/linux/mtd/rawnand.h      | 14 +-------------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 2d8cbe8eb78b..9706908fbdcf 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5225,7 +5225,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 static int nand_flash_detect_jedec(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct nand_jedec_params *p = &chip->jedec_params;
+	struct nand_jedec_params *p;
 	struct jedec_ecc_info *ecc;
 	char id[5];
 	int i, val, ret;
@@ -5235,14 +5235,23 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
 	if (ret || strncmp(id, "JEDEC", sizeof(id)))
 		return 0;
 
+	/* JEDEC chip: allocate a buffer to hold its parameter page */
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
 	ret = nand_read_param_page_op(chip, 0x40, NULL, 0);
-	if (ret)
-		return 0;
+	if (ret) {
+		ret = 0;
+		goto free_jedec_param_page;
+	}
 
 	for (i = 0; i < 3; i++) {
 		ret = nand_read_data_op(chip, p, sizeof(*p), true);
-		if (ret)
-			return 0;
+		if (ret) {
+			ret = 0;
+			goto free_jedec_param_page;
+		}
 
 		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 510) ==
 				le16_to_cpu(p->crc))
@@ -5251,7 +5260,7 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
 
 	if (i == 3) {
 		pr_err("Could not find valid JEDEC parameter page; aborting\n");
-		return 0;
+		goto free_jedec_param_page;
 	}
 
 	/* Check version */
@@ -5263,7 +5272,7 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
 
 	if (!chip->jedec_version) {
 		pr_info("unsupported JEDEC version: %d\n", val);
-		return 0;
+		goto free_jedec_param_page;
 	}
 
 	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
@@ -5285,7 +5294,7 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
 	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
 	chip->bits_per_cell = p->bits_per_cell;
 
-	if (jedec_feature(chip) & JEDEC_FEATURE_16_BIT_BUS)
+	if (le16_to_cpu(p->features) & JEDEC_FEATURE_16_BIT_BUS)
 		chip->options |= NAND_BUSWIDTH_16;
 
 	/* ECC info */
@@ -5298,7 +5307,9 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
 		pr_warn("Invalid codeword size\n");
 	}
 
-	return 1;
+free_jedec_param_page:
+	kfree(p);
+	return ret;
 }
 
 /*
@@ -5604,7 +5615,10 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 			goto ident_done;
 
 		/* Check if the chip is JEDEC compliant */
-		if (nand_flash_detect_jedec(chip))
+		ret = nand_flash_detect_jedec(chip);
+		if (ret < 0)
+			return ret;
+		if (ret)
 			goto ident_done;
 	}
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index d2105352033a..965e6113f661 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1186,8 +1186,6 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
  *			non 0 if JEDEC supported.
  * @onfi_params:	[INTERN] holds the ONFI page parameter when ONFI is
  *			supported, 0 otherwise.
- * @jedec_params:	[INTERN] holds the JEDEC parameter page when JEDEC is
- *			supported, 0 otherwise.
  * @max_bb_per_die:	[INTERN] the max number of bad blocks each die of a
  *			this nand device will encounter their life times.
  * @blocks_per_die:	[INTERN] The number of PEBs in a die
@@ -1268,10 +1266,7 @@ struct nand_chip {
 	struct nand_id id;
 	int onfi_version;
 	int jedec_version;
-	union {
-		struct nand_onfi_params	onfi_params;
-		struct nand_jedec_params jedec_params;
-	};
+	struct nand_onfi_params	onfi_params;
 	struct nand_parameters parameters;
 	u16 max_bb_per_die;
 	u32 blocks_per_die;
@@ -1602,13 +1597,6 @@ static inline int nand_opcode_8bits(unsigned int command)
 	return 0;
 }
 
-/* return the supported JEDEC features. */
-static inline int jedec_feature(struct nand_chip *chip)
-{
-	return chip->jedec_version ? le16_to_cpu(chip->jedec_params.features)
-		: 0;
-}
-
 /* get timing characteristics from ONFI timing mode. */
 const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode);
 
-- 
2.14.1

[PATCH v3 14/14] mtd: rawnand: get rid of the ONFI parameter page in nand_chip

[Miquel Raynal]

The NAND chip parameter page is statically allocated within the
nand_chip structure, which reserves a lot of space. Even not ONFI nor
JEDEC chips have it embedded. Also, only a few parameters are actually
read from the parameter page after the detection.

Now that there is a small nand_parameters structure that hold all needed
ONFI parameters, remove the ONFI page from the nand_chip structure by
just allocating it during the identification phase and removing it right
after.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 34 +++++++++++++++++++++++++---------
 include/linux/mtd/rawnand.h      |  3 ---
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 9706908fbdcf..ef0a44a8c3d5 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5099,7 +5099,7 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
 static int nand_flash_detect_onfi(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	struct nand_onfi_params *p = &chip->onfi_params;
+	struct nand_onfi_params *p;
 	char id[4];
 	int i, ret, val;
 
@@ -5108,14 +5108,23 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 	if (ret || strncmp(id, "ONFI", 4))
 		return 0;
 
+	/* ONFI chip: allocate a buffer to hold its parameter page */
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
 	ret = nand_read_param_page_op(chip, 0, NULL, 0);
-	if (ret)
-		return 0;
+	if (ret) {
+		ret = 0;
+		goto free_onfi_param_page;
+	}
 
 	for (i = 0; i < 3; i++) {
 		ret = nand_read_data_op(chip, p, sizeof(*p), true);
-		if (ret)
-			return 0;
+		if (ret) {
+			ret = 0;
+			goto free_onfi_param_page;
+		}
 
 		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==
 				le16_to_cpu(p->crc)) {
@@ -5125,7 +5134,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 
 	if (i == 3) {
 		pr_err("Could not find valid ONFI parameter page; aborting\n");
-		return 0;
+		goto free_onfi_param_page;
 	}
 
 	/* Check version */
@@ -5143,7 +5152,9 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 
 	if (!chip->onfi_version) {
 		pr_info("unsupported ONFI version: %d\n", val);
-		return 0;
+		goto free_onfi_param_page;
+	} else {
+		ret = 1;
 	}
 
 	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
@@ -5216,7 +5227,9 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 	memcpy(chip->parameters.onfi_params.vendor, p->vendor,
 	       sizeof(p->vendor));
 
-	return 1;
+free_onfi_param_page:
+	kfree(p);
+	return ret;
 }
 
 /*
@@ -5611,7 +5624,10 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	chip->onfi_version = 0;
 	if (!type->name || !type->pagesize) {
 		/* Check if the chip is ONFI compliant */
-		if (nand_flash_detect_onfi(chip))
+		ret = nand_flash_detect_onfi(chip);
+		if (ret < 0)
+			return ret;
+		if (ret)
 			goto ident_done;
 
 		/* Check if the chip is JEDEC compliant */
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 965e6113f661..348f2aba0b9e 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1184,8 +1184,6 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
  *			non 0 if ONFI supported.
  * @jedec_version:	[INTERN] holds the chip JEDEC version (BCD encoded),
  *			non 0 if JEDEC supported.
- * @onfi_params:	[INTERN] holds the ONFI page parameter when ONFI is
- *			supported, 0 otherwise.
  * @max_bb_per_die:	[INTERN] the max number of bad blocks each die of a
  *			this nand device will encounter their life times.
  * @blocks_per_die:	[INTERN] The number of PEBs in a die
@@ -1266,7 +1264,6 @@ struct nand_chip {
 	struct nand_id id;
 	int onfi_version;
 	int jedec_version;
-	struct nand_onfi_params	onfi_params;
 	struct nand_parameters parameters;
 	u16 max_bb_per_die;
 	u32 blocks_per_die;
-- 
2.14.1

Re: [PATCH v3 05/14] mtd: rawnand: check ONFI timings have been acked by the chip

[Boris Brezillon]

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;
>  }
>  



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 03/14] mtd: rawnand: mxc: rename the local ->set/get_features() implementation

[Boris Brezillon]

On Fri,  2 Mar 2018 15:24:11 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> MXC NAND driver embeds its own implementation of the
> ->set/get_features() hooks. These hooks were once flagged "onfi" but a  
> lot of not-ONFI compliant NAND chips use it so they have been renamed
> to remove the "onfi" relationship. Rename these functions the same way
> for consistency.

Should be squashed in patch 2 IMO.

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/mxc_nand.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
> index d331305f45c1..61501654e708 100644
> --- a/drivers/mtd/nand/raw/mxc_nand.c
> +++ b/drivers/mtd/nand/raw/mxc_nand.c
> @@ -1421,9 +1421,8 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
>  	}
>  }
>  
> -static int mxc_nand_onfi_set_features(struct mtd_info *mtd,
> -				      struct nand_chip *chip, int addr,
> -				      u8 *subfeature_param)
> +static int mxc_nand_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> +				 int addr, u8 *subfeature_param)
>  {
>  	struct nand_chip *nand_chip = mtd_to_nand(mtd);
>  	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
> @@ -1447,9 +1446,8 @@ static int mxc_nand_onfi_set_features(struct mtd_info *mtd,
>  	return 0;
>  }
>  
> -static int mxc_nand_onfi_get_features(struct mtd_info *mtd,
> -				      struct nand_chip *chip, int addr,
> -				      u8 *subfeature_param)
> +static int mxc_nand_get_features(struct mtd_info *mtd, struct nand_chip *chip,
> +				 int addr, u8 *subfeature_param)
>  {
>  	struct nand_chip *nand_chip = mtd_to_nand(mtd);
>  	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
> @@ -1752,8 +1750,8 @@ static int mxcnd_probe(struct platform_device *pdev)
>  	this->read_word = mxc_nand_read_word;
>  	this->write_buf = mxc_nand_write_buf;
>  	this->read_buf = mxc_nand_read_buf;
> -	this->set_features = mxc_nand_onfi_set_features;
> -	this->get_features = mxc_nand_onfi_get_features;
> +	this->set_features = mxc_nand_set_features;
> +	this->get_features = mxc_nand_get_features;
>  
>  	host->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(host->clk))



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 07/14] mtd: rawnand: use wrappers to call onfi GET/SET_FEATURES

[Boris Brezillon]

On Fri,  2 Mar 2018 15:24:15 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Prepare the fact that some features managed by GET/SET_FEATURES could be
> overloaded by vendor code. To handle this logic, use new wrappers
> instead of directly call the ->get/set_features() hooks.
> 
> Also take into account that not having the proper SET/GET_FEATURES
> available is not a reason to return an error. The wrappers that are
> created here handle this case by returning a special error code:
> -ENOTSUPP. More logic in the calling function of the new helpers
> (nand_setup_data_interface()) is added to handle this case).
> 

Why don't you move this patch just after patch 3 so that you can use
the new wrapper in patch 5?

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c   | 127 ++++++++++++++++++++++++-------------
>  drivers/mtd/nand/raw/nand_micron.c |  18 +++---
>  include/linux/mtd/rawnand.h        |   3 +
>  3 files changed, 96 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 70b59672362c..e5bcfbf7c7f6 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1160,6 +1160,52 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
>  	return status;
>  }
>  
> +/**
> + * nand_get_features - wrapper to perform a GET_FEATURE
> + * @chip: NAND chip info structure
> + * @addr: feature address
> + * @subfeature_param: the subfeature parameters, a four bytes array
> + *
> + * Returns 0 for success, a negative error otherwise. Returns -ENOTSUPP if the
> + * operation cannot be handled.
> + */
> +int nand_get_features(struct nand_chip *chip, int addr,
> +		      u8 *subfeature_param)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +
> +	if (!chip->onfi_version ||
> +	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> +	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> +		return -ENOTSUPP;
> +
> +	return chip->get_features(mtd, chip, addr, subfeature_param);
> +}
> +EXPORT_SYMBOL_GPL(nand_get_features);
> +
> +/**
> + * nand_set_features - wrapper to perform a SET_FEATURE
> + * @chip: NAND chip info structure
> + * @addr: feature address
> + * @subfeature_param: the subfeature parameters, a four bytes array
> + *
> + * Returns 0 for success, a negative error otherwise. Returns -ENOTSUPP if the
> + * operation cannot be handled.
> + */
> +int nand_set_features(struct nand_chip *chip, int addr,
> +		      u8 *subfeature_param)
> +{
> +	struct mtd_info *mtd = nand_to_mtd(chip);
> +
> +	if (!chip->onfi_version ||
> +	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> +	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> +		return -ENOTSUPP;
> +
> +	return chip->set_features(mtd, chip, addr, subfeature_param);
> +}
> +EXPORT_SYMBOL_GPL(nand_set_features);
> +
>  /**
>   * nand_reset_data_interface - Reset data interface and timings
>   * @chip: The NAND chip
> @@ -1215,59 +1261,62 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
>  static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
> +	u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
> +		chip->onfi_timing_mode_default,
> +	};
>  	int ret;
>  
>  	if (!chip->setup_data_interface)
>  		return 0;
>  
> +	/* Change the mode on the chip side */
> +	chip->select_chip(mtd, chipnr);
> +	ret = nand_set_features(chip, ONFI_FEATURE_ADDR_TIMING_MODE,
> +				tmode_param);
> +	chip->select_chip(mtd, -1);
> +
>  	/*
> -	 * Ensure the timing mode has been changed on the chip side
> -	 * before changing timings on the controller side.
> +	 * Do not fail because the mode cannot explicitly be set. If the NAND
> +	 * chip claims it supports it, let's just apply the timings on the
> +	 * controller side.
>  	 */
> -	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->onfi_timing_mode_default,
> -		};
> -
> -		chip->select_chip(mtd, chipnr);
> -		ret = chip->set_features(mtd, chip,
> -					 ONFI_FEATURE_ADDR_TIMING_MODE,
> -					 tmode_param);
> -		chip->select_chip(mtd, -1);
> -		if (ret)
> -			return ret;
> -	}
> +	if (ret && ret != -ENOTSUPP)
> +		return ret;
>  
> +	/* Change the mode on the controller side */
>  	ret = chip->setup_data_interface(mtd, chipnr, &chip->data_interface);
>  	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] = {};
> +	/* Check the mode has been accepted by the chip */
> +	memset(tmode_param, 0, ONFI_SUBFEATURE_PARAM_LEN);
> +	chip->select_chip(mtd, chipnr);
> +	ret = nand_get_features(chip, ONFI_FEATURE_ADDR_TIMING_MODE,
> +				tmode_param);
> +	chip->select_chip(mtd, -1);
> +	if (ret && ret != -ENOTSUPP)
> +		goto err_reset_chip;
>  
> -		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;
> +	/*
> +	 * Do not fail because the mode could not be checked. However, skip the
> +	 * comparison block that would probably raise an error.
> +	 */
> +	if (ret == -ENOTSUPP)
> +		return 0;
>  
> -		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;
> -		}
> +	if (tmode_param[0] != chip->onfi_timing_mode_default) {
> +		pr_warn("timing 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 */
> +	/*
> +	 * Fallback to mode 0 if the chip explicitly did not ack the chosen
> +	 * timing mode.
> +	 */
>  	nand_reset_data_interface(chip, chipnr);
>  	chip->select_chip(mtd, chipnr);
>  	nand_reset_op(chip);
> @@ -4820,11 +4869,6 @@ static int nand_default_set_features(struct mtd_info *mtd,
>  				     struct nand_chip *chip, int addr,
>  				     uint8_t *subfeature_param)
>  {
> -	if (!chip->onfi_version ||
> -	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> -	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> -		return -EINVAL;
> -
>  	return nand_set_features_op(chip, addr, subfeature_param);
>  }
>  
> @@ -4839,11 +4883,6 @@ static int nand_default_get_features(struct mtd_info *mtd,
>  				     struct nand_chip *chip, int addr,
>  				     uint8_t *subfeature_param)
>  {
> -	if (!chip->onfi_version ||
> -	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> -	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> -		return -EINVAL;
> -
>  	return nand_get_features_op(chip, addr, subfeature_param);
>  }
>  
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index ab3e3a1a5212..b825656f6284 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -48,8 +48,7 @@ static int micron_nand_setup_read_retry(struct mtd_info *mtd, int retry_mode)
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  	u8 feature[ONFI_SUBFEATURE_PARAM_LEN] = {retry_mode};
>  
> -	return chip->set_features(mtd, chip, ONFI_FEATURE_ADDR_READ_RETRY,
> -				  feature);
> +	return nand_set_features(chip, ONFI_FEATURE_ADDR_READ_RETRY, feature);
>  }
>  
>  /*
> @@ -108,8 +107,7 @@ static int micron_nand_on_die_ecc_setup(struct nand_chip *chip, bool enable)
>  	if (enable)
>  		feature[0] |= ONFI_FEATURE_ON_DIE_ECC_EN;
>  
> -	return chip->set_features(nand_to_mtd(chip), chip,
> -				  ONFI_FEATURE_ON_DIE_ECC, feature);
> +	return nand_set_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
>  }
>  
>  static int
> @@ -219,8 +217,10 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
>  	if (ret)
>  		return MICRON_ON_DIE_UNSUPPORTED;
>  
> -	chip->get_features(nand_to_mtd(chip), chip,
> -			   ONFI_FEATURE_ON_DIE_ECC, feature);
> +	ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
> +	if (ret < 0)
> +		return ret;
> +
>  	if ((feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN) == 0)
>  		return MICRON_ON_DIE_UNSUPPORTED;
>  
> @@ -228,8 +228,10 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
>  	if (ret)
>  		return MICRON_ON_DIE_UNSUPPORTED;
>  
> -	chip->get_features(nand_to_mtd(chip), chip,
> -			   ONFI_FEATURE_ON_DIE_ECC, feature);
> +	ret = nand_get_features(chip, ONFI_FEATURE_ON_DIE_ECC, feature);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (feature[0] & ONFI_FEATURE_ON_DIE_ECC_EN)
>  		return MICRON_ON_DIE_MANDATORY;
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index fb2e288ef8b1..3cc2a3435b20 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1629,6 +1629,9 @@ int nand_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page);
>  int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>  			   int page);
>  
> +/* Wrapper to use in order for controllers/vendors to GET/SET FEATURES */
> +int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
> +int nand_set_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
>  /* Stub used by drivers that do not support GET/SET FEATURES operations */
>  int nand_get_set_features_notsupp(struct mtd_info *mtd, struct nand_chip *chip,
>  				  int addr, u8 *subfeature_param);



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 09/14] mtd: rawnand: prepare the removal of ONFI/JEDEC parameter pages

[Boris Brezillon]

On Fri,  2 Mar 2018 15:24:17 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> The NAND chip parameter page is statically allocated within the
> nand_chip structure, which reserves a lot of space. Even not ONFI nor
> JEDEC chips have it embedded. Also, only a few parameters are actually
> read from the parameter page after the detection.
> 
> To prepare to the removal of such huge structure, a small NAND parameter
> structure is allocated statically and contains only very few members
> that are generic to all chips and actually used elsewhere in the code.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 32 +++++++++++++-------------------
>  include/linux/mtd/rawnand.h      |  6 ++++++
>  2 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index e5bcfbf7c7f6..30364f60dc4d 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1174,9 +1174,7 @@ int nand_get_features(struct nand_chip *chip, int addr,
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  
> -	if (!chip->onfi_version ||
> -	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> -	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> +	if (!chip->parameters.supports_set_get_features)
>  		return -ENOTSUPP;
>  
>  	return chip->get_features(mtd, chip, addr, subfeature_param);
> @@ -1197,9 +1195,7 @@ int nand_set_features(struct nand_chip *chip, int addr,
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  
> -	if (!chip->onfi_version ||
> -	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> -	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> +	if (!chip->parameters.supports_set_get_features)
>  		return -ENOTSUPP;
>  
>  	return chip->set_features(mtd, chip, addr, subfeature_param);
> @@ -5150,8 +5146,9 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
>  
>  	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
>  	sanitize_string(p->model, sizeof(p->model));
> +	memcpy(chip->parameters.model, p->model, sizeof(p->model));
>  	if (!mtd->name)
> -		mtd->name = p->model;
> +		mtd->name = chip->parameters.model;
>  
>  	mtd->writesize = le32_to_cpu(p->byte_per_page);
>  
> @@ -5198,6 +5195,10 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
>  		pr_warn("Could not retrieve ONFI ECC requirements\n");
>  	}
>  
> +	/* Save some parameters from the parameter page for future use */
> +	if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_SET_GET_FEATURES)
> +		chip->parameters.supports_set_get_features = true;
> +
>  	return 1;
>  }
>  
> @@ -5250,8 +5251,9 @@ static int nand_flash_detect_jedec(struct nand_chip *chip)
>  
>  	sanitize_string(p->manufacturer, sizeof(p->manufacturer));
>  	sanitize_string(p->model, sizeof(p->model));
> +	memcpy(chip->parameters.model, p->model, sizeof(p->model));

It's safe as long as chip->parameters.model is bigger than p->model,
which is the case here, but how about enforcing it, just in case
someone decides to change the chip->parameters.model size.

	strncpy(chip->parameters.model, p->model,
		sizeof(chip->parameters.model) - 1;

>  	if (!mtd->name)
> -		mtd->name = p->model;
> +		mtd->name = chip->parameters.model;
>  
>  	mtd->writesize = le32_to_cpu(p->byte_per_page);
>  
> @@ -5652,17 +5654,9 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
>  
>  	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
>  		maf_id, dev_id);
> -
> -	if (chip->onfi_version)
> -		pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
> -			chip->onfi_params.model);
> -	else if (chip->jedec_version)
> -		pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
> -			chip->jedec_params.model);
> -	else
> -		pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
> -			type->name);
> -
> +	pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
> +		(chip->onfi_version || chip->jedec_version) ?
> +		chip->parameters.model : type->name);
>  	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
>  		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
>  		mtd->erasesize >> 10, mtd->writesize, mtd->oobsize);
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 3cc2a3435b20..1af0bff58ff4 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -429,6 +429,11 @@ struct nand_jedec_params {
>  	__le16 crc;
>  } __packed;
>  
> +struct nand_parameters {
> +	char model[100];
> +	bool supports_set_get_features;
> +};

Can you document this structure with a kernel-doc header?

> +
>  /* The maximum expected count of bytes in the NAND ID sequence */
>  #define NAND_MAX_ID_LEN 8
>  
> @@ -1249,6 +1254,7 @@ struct nand_chip {
>  		struct nand_onfi_params	onfi_params;
>  		struct nand_jedec_params jedec_params;
>  	};
> +	struct nand_parameters parameters;

You forgot to update the kernel-doc header.

>  	u16 max_bb_per_die;
>  	u32 blocks_per_die;
>  



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 10/14] mtd: rawnand: prepare the removal of the ONFI parameter page

[Boris Brezillon]

On Fri,  2 Mar 2018 15:24:18 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 1af0bff58ff4..81236b3fbc6f 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -429,9 +429,23 @@ struct nand_jedec_params {
>  	__le16 crc;
>  } __packed;
>  
> +struct onfi_params {
> +	u16 t_prog;
> +	u16 t_bers;
> +	u16 t_r;
> +	u16 t_ccs;
> +	u16 async_timing_mode;
> +	u16 vendor_revision;
> +	u8 vendor[88];
> +};

Kernel-doc missing here.

> +
>  struct nand_parameters {
> +	/* Generic parameters */
>  	char model[100];
>  	bool supports_set_get_features;
> +
> +	/* ONFI parameters */
> +	struct onfi_params onfi_params;

Why not name the field onfi instead of onfi_params?

>  };


-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 13/14] mtd: rawnand: get rid of the JEDEC parameter page in nand_chip

[Boris Brezillon]

On Fri,  2 Mar 2018 15:24:21 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index d2105352033a..965e6113f661 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1186,8 +1186,6 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
>   *			non 0 if JEDEC supported.
>   * @onfi_params:	[INTERN] holds the ONFI page parameter when ONFI is
>   *			supported, 0 otherwise.

You forgot to remove the onfi_params doc in one of your previous commit.



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 13/14] mtd: rawnand: get rid of the JEDEC parameter page in nand_chip

[Boris Brezillon]

On Mon, 12 Mar 2018 20:23:36 +0100
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> On Fri,  2 Mar 2018 15:24:21 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>   
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index d2105352033a..965e6113f661 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -1186,8 +1186,6 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
> >   *			non 0 if JEDEC supported.
> >   * @onfi_params:	[INTERN] holds the ONFI page parameter when ONFI is
> >   *			supported, 0 otherwise.  
> 
> You forgot to remove the onfi_params doc in one of your previous commit.

Never mind, you remove this field in patch 14.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com