Re: [PATCH v4 08/19] mtd: rawnand: Rename onfi_timing_mode_default

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Mon, 25 May 2020 19:42:28 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> This parameter would better be named default_timing_mode as we are
> opening the way to non-ONFI timings.

We should patch the toshiba driver to implement the interface you
introduce in this patch series. Along with the suggestion to allocate
the 'ideal' data_interface instead of resetting the interface to mode 0,
you shouldn't need this field anymore.

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
>  drivers/mtd/nand/raw/nand_base.c    | 20 +++++++++-----------
>  drivers/mtd/nand/raw/nand_toshiba.c |  2 +-
>  include/linux/mtd/rawnand.h         |  8 ++++----
>  3 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 7567c973964b..adbc12580e2e 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -952,7 +952,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
>  static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
>  {
>  	u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
> -		chip->onfi_timing_mode_default,
> +		chip->default_timing_mode,
>  	};
>  	int ret;
>  
> @@ -987,9 +987,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
>  	if (ret)
>  		goto err_reset_chip;
>  
> -	if (tmode_param[0] != chip->onfi_timing_mode_default) {
> +	if (tmode_param[0] != chip->default_timing_mode) {
>  		pr_warn("timing mode %d not acknowledged by the NAND chip\n",
> -			chip->onfi_timing_mode_default);
> +			chip->default_timing_mode);
>  		goto err_reset_chip;
>  	}
>  
> @@ -1016,9 +1016,8 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
>   * and the driver.
>   * First tries to retrieve supported timing modes from ONFI information,
>   * and if the NAND chip does not support ONFI, relies on the
> - * ->onfi_timing_mode_default specified in the nand_ids table. After this
> - * function nand_chip->data_interface is initialized with the best timing mode
> - * available.
> + * ->default_timing_mode specified in the nand_ids table. After this function
> + * nand_chip->data_interface is initialized with the best timing mode available.
>   *
>   * Returns 0 for success or negative error code otherwise.
>   */
> @@ -1037,10 +1036,10 @@ static int nand_init_data_interface(struct nand_chip *chip)
>  	if (chip->parameters.onfi) {
>  		modes = chip->parameters.onfi->async_timing_mode;
>  	} else {
> -		if (!chip->onfi_timing_mode_default)
> +		if (!chip->default_timing_mode)
>  			return 0;
>  
> -		modes = GENMASK(chip->onfi_timing_mode_default, 0);
> +		modes = GENMASK(chip->default_timing_mode, 0);
>  	}
>  
>  	for (mode = fls(modes) - 1; mode >= 0; mode--) {
> @@ -1056,7 +1055,7 @@ static int nand_init_data_interface(struct nand_chip *chip)
>  						 NAND_DATA_IFACE_CHECK_ONLY,
>  						 &chip->data_interface);
>  		if (!ret) {
> -			chip->onfi_timing_mode_default = mode;
> +			chip->default_timing_mode = mode;
>  			break;
>  		}
>  	}
> @@ -4814,8 +4813,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
>  		chip->options |= type->options;
>  		chip->base.eccreq.strength = NAND_ECC_STRENGTH(type);
>  		chip->base.eccreq.step_size = NAND_ECC_STEP(type);
> -		chip->onfi_timing_mode_default =
> -					type->onfi_timing_mode_default;
> +		chip->default_timing_mode = type->onfi_timing_mode_default;
>  
>  		chip->parameters.model = kstrdup(type->name, GFP_KERNEL);
>  		if (!chip->parameters.model)
> diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
> index ae069905d7e4..b6efaf5195bb 100644
> --- a/drivers/mtd/nand/raw/nand_toshiba.c
> +++ b/drivers/mtd/nand/raw/nand_toshiba.c
> @@ -198,7 +198,7 @@ static int tc58teg5dclta00_init(struct nand_chip *chip)
>  {
>  	struct mtd_info *mtd = nand_to_mtd(chip);
>  
> -	chip->onfi_timing_mode_default = 5;
> +	chip->default_timing_mode = 5;
>  	chip->options |= NAND_NEED_SCRAMBLING;
>  	mtd_set_pairing_scheme(mtd, &dist3_pairing_scheme);
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 622da6527a36..a4b68e7b246a 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1067,9 +1067,9 @@ struct nand_manufacturer {
>   * @options: Various chip options. They can partly be set to inform nand_scan
>   *           about special functionality. See the defines for further
>   *           explanation.
> - * @onfi_timing_mode_default: Default ONFI timing mode. This field is set to the
> - *			      actually used ONFI mode if the chip is ONFI
> - *			      compliant or deduced from the datasheet otherwise
> + * @default_timing_mode: Default timing mode. This field is set to the actually
> + *                       used ONFI mode if the chip is ONFI compliant or deduced
> + *                       from the datasheet otherwise
>   * @data_interface: NAND interface timing information
>   * @bbt_erase_shift: Number of address bits in a bbt entry
>   * @bbt_options: Bad block table specific options. All options used here must
> @@ -1117,7 +1117,7 @@ struct nand_chip {
>  	unsigned int options;
>  
>  	/* Data interface */
> -	int onfi_timing_mode_default;
> +	int default_timing_mode;
>  	struct nand_data_interface data_interface;
>  
>  	/* Bad block information */


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/