[PATCH v2] mtd: rawnand: Add support to dedicated function to set timings

[PATCH v2] mtd: rawnand: Add support to dedicated function to set timings

[kory.maincent]

From: Kory Maincent <kory.maincent@bootlin.com>

With the current code if the board has an ONFI compliant NAND without
support to the get and set features, U-boot returns an ENOTSUP error when
trying to tune the timings which prevents the probe of the device.
Indeed onfi_set_features() return ENOTSUP error if set/get features is not
supported. In the case of timings we should not return ENOTSUP because we
can use the default timings. The NAND is already capable of listening at
its highest supported rate, so we assume in this case that it is fine to
skip the operation.

Fix it by adding an intermediate nand_onfi_set_timings() function which
does not error out if set/get feature is not supported.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change since v1:
- Update commit message

 drivers/mtd/nand/raw/nand_base.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 6f81257cf1..e8ece0a4a0 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -974,6 +974,22 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
 	return ret;
 }
 
+static int nand_onfi_set_timings(struct mtd_info *mtd, struct nand_chip *chip)
+{
+	if (!chip->onfi_version ||
+	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
+	      & ONFI_OPT_CMD_SET_GET_FEATURES))
+		return 0;
+
+	u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
+		chip->onfi_timing_mode_default,
+	};
+
+	return chip->onfi_set_features(mtd, chip,
+				       ONFI_FEATURE_ADDR_TIMING_MODE,
+				       tmode_param);
+}
+
 /**
  * nand_setup_data_interface - Setup the best data interface and timings
  * @chip: The NAND chip
@@ -999,17 +1015,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
 	 * Ensure the timing mode has been changed on the chip side
 	 * before changing timings on the controller side.
 	 */
-	if (chip->onfi_version) {
-		u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
-			chip->onfi_timing_mode_default,
-		};
-
-		ret = chip->onfi_set_features(mtd, chip,
-				ONFI_FEATURE_ADDR_TIMING_MODE,
-				tmode_param);
-		if (ret)
-			goto err;
-	}
+	ret = nand_onfi_set_timings(mtd, chip);
+	if (ret)
+		goto err;
 
 	ret = chip->setup_data_interface(mtd, chipnr, chip->data_interface);
 err:
-- 
2.25.1

Re: [PATCH v2] mtd: rawnand: Add support to dedicated function to set timings

[Miquel Raynal]

Hi Köry,

kory.maincent@bootlin.com wrote on Wed, 22 Jun 2022 11:11:45 +0200:

> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> With the current code if the board has an ONFI compliant NAND without
> support to the get and set features, U-boot returns an ENOTSUP error when
> trying to tune the timings which prevents the probe of the device.
> Indeed onfi_set_features() return ENOTSUP error if set/get features is not
> supported. In the case of timings we should not return ENOTSUP because we
> can use the default timings. The NAND is already capable of listening at
> its highest supported rate, so we assume in this case that it is fine to
> skip the operation.
> 
> Fix it by adding an intermediate nand_onfi_set_timings() function which
> does not error out if set/get feature is not supported.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Seems legitimate.

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

> ---
> 
> Change since v1:
> - Update commit message
> 
>  drivers/mtd/nand/raw/nand_base.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 6f81257cf1..e8ece0a4a0 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -974,6 +974,22 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
>  	return ret;
>  }
>  
> +static int nand_onfi_set_timings(struct mtd_info *mtd, struct nand_chip *chip)
> +{
> +	if (!chip->onfi_version ||
> +	    !(le16_to_cpu(chip->onfi_params.opt_cmd)
> +	      & ONFI_OPT_CMD_SET_GET_FEATURES))
> +		return 0;
> +
> +	u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
> +		chip->onfi_timing_mode_default,
> +	};
> +
> +	return chip->onfi_set_features(mtd, chip,
> +				       ONFI_FEATURE_ADDR_TIMING_MODE,
> +				       tmode_param);
> +}
> +
>  /**
>   * nand_setup_data_interface - Setup the best data interface and timings
>   * @chip: The NAND chip
> @@ -999,17 +1015,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
>  	 * Ensure the timing mode has been changed on the chip side
>  	 * before changing timings on the controller side.
>  	 */
> -	if (chip->onfi_version) {
> -		u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
> -			chip->onfi_timing_mode_default,
> -		};
> -
> -		ret = chip->onfi_set_features(mtd, chip,
> -				ONFI_FEATURE_ADDR_TIMING_MODE,
> -				tmode_param);
> -		if (ret)
> -			goto err;
> -	}
> +	ret = nand_onfi_set_timings(mtd, chip);
> +	if (ret)
> +		goto err;
>  
>  	ret = chip->setup_data_interface(mtd, chipnr, chip->data_interface);
>  err:


Thanks,
Miquèl

Re: [PATCH v2] mtd: rawnand: Add support to dedicated function to set timings

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

On Wed, Jun 22, 2022 at 11:11:45AM +0200, kory.maincent@bootlin.com wrote:

> From: Kory Maincent <kory.maincent@bootlin.com>
> 
> With the current code if the board has an ONFI compliant NAND without
> support to the get and set features, U-boot returns an ENOTSUP error when
> trying to tune the timings which prevents the probe of the device.
> Indeed onfi_set_features() return ENOTSUP error if set/get features is not
> supported. In the case of timings we should not return ENOTSUP because we
> can use the default timings. The NAND is already capable of listening at
> its highest supported rate, so we assume in this case that it is fine to
> skip the operation.
> 
> Fix it by adding an intermediate nand_onfi_set_timings() function which
> does not error out if set/get feature is not supported.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]