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

[Miquel Raynal <miquel.raynal@bootlin.com>]

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