From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.89 #1 (Red Hat Linux)) id 1ehuYv-0002Zi-RJ for linux-mtd@lists.infradead.org; Sat, 03 Feb 2018 09:56:49 +0000 From: Miquel Raynal To: Boris Brezillon , Richard Weinberger , David Woodhouse , Brian Norris , Marek Vasut , Cyrille Pitchen Cc: linux-mtd@lists.infradead.org, Miquel Raynal Subject: [PATCH 2/6] mtd: nand: Use wrappers to call onfi GET/SET_FEATURES Date: Sat, 3 Feb 2018 10:55:40 +0100 Message-Id: <20180203095544.9855-3-miquel.raynal@bootlin.com> In-Reply-To: <20180203095544.9855-1-miquel.raynal@bootlin.com> References: <20180203095544.9855-1-miquel.raynal@bootlin.com> List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: 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 ->onfi_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 --- drivers/mtd/nand/nand_base.c | 145 ++++++++++++++++++++++++++--------------- drivers/mtd/nand/nand_micron.c | 16 ++--- include/linux/mtd/rawnand.h | 3 + 3 files changed, 104 insertions(+), 60 deletions(-) diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 00111e669c11..8d2c37011979 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/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->onfi_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->onfi_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->onfi_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->onfi_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); @@ -4905,38 +4954,30 @@ 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_set_features_default - [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_set_features_default(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); } /** - * nand_onfi_get_features- [REPLACEABLE] get features for ONFI nand + * nand_get_features_default - [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_get_features_default(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); } @@ -5015,9 +5056,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_set_features_default; if (!chip->onfi_get_features) - chip->onfi_get_features = nand_onfi_get_features; + chip->onfi_get_features = nand_get_features_default; /* 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/nand_micron.c b/drivers/mtd/nand/nand_micron.c index 02e109ae73f1..48847b441ef7 100644 --- a/drivers/mtd/nand/nand_micron.c +++ b/drivers/mtd/nand/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->onfi_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->onfi_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,9 @@ 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); + 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 +227,9 @@ 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); + 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 469dc724f5df..3244f2594b6b 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_onfi_get_set_features_notsupp(struct mtd_info *mtd, struct nand_chip *chip, int addr, -- 2.14.1