Re: [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()

[Boris Brezillon <boris.brezillon@collabora.com>]

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

> This helper is here to simplify the life of NAND manufacturer drivers.
> 
> Manufacturers will be allowed to propose their own set of timings and,
> if they want, use this helper to:
> 1/ verify it is supported by the controller,
> 2/ fallback on a supported default ONFI mode, slower but still faster
>    than the default mode 0.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/internals.h |  2 ++
>  drivers/mtd/nand/raw/nand_base.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index ac103d8767be..9af6979257e2 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -89,6 +89,8 @@ int onfi_fill_data_interface(struct nand_chip *chip,
>  			     enum nand_data_interface_type type,
>  			     int timing_mode);
>  unsigned int onfi_find_equivalent_sdr_mode(const struct nand_sdr_timings *vendor_timings);
> +int nand_choose_best_vendor_sdr_iface(struct nand_chip * chip,
> +				      struct nand_data_interface *best_iface);
>  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);
>  int nand_read_page_raw_notsupp(struct nand_chip *chip, u8 *buf,
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 15e10f045c9f..d9fe7795f183 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1078,6 +1078,33 @@ static int nand_choose_data_interface(struct nand_chip *chip)
>  	return nand_choose_best_sdr_iface(chip, &chip->data_interface);
>  }
>  
> +/**
> + * nand_choose_best_vendor_sdr_iface - given a set of timings, find the closest
> + *                                     mode/timings set for this interface
> + *                                     supported by both the NAND controller and
> + *                                     the NAND chip
> + * @chip: the NAND chip
> + * @best_iface: the best data interface (can eventually be updated)
> + */
> +int nand_choose_best_vendor_sdr_iface(struct nand_chip * chip,
> +				      struct nand_data_interface *best_iface)
> +{
> +	int ret;
> +
> +	/* Pick the closest mode */
> +	best_iface->timings.mode = onfi_find_equivalent_sdr_mode(&best_iface->timings.sdr);
> +
> +	/* Find the closest supported data interface */
> +	ret = nand_choose_best_sdr_iface(chip, best_iface);
> +	if (ret)
> +		return ret;
> +
> +	chip->data_interface = *best_iface;
> +
> +	return 0;
> +}

Can't we just merge nand_choose_best_vendor_sdr_iface() and
nand_choose_best_sdr_iface()?

int nand_choose_best_sdr_timings(struct nand_chip * chip,
				 struct nand_data_interface *iface,
				 const struct nand_sdr_timing *spec_timings)
{
	iface->type = SDR;

	if (spec_timings) {
		iface->timings.sdr = spec_timings;
		iface->timings.mode = onfi_find_closest_sdr_mode(spec_timings);
	} else {
		unsigned int best_mode;

		if (chip->parameters.onfi)
			best_mode = fls(chip->parameters.onfi->async_timing_mode) - 1;
		else
			best_mode = chip->default_timing_mode;

		onfi_fill_data_interface(chip, iface,
					  NAND_SDR_IFACE, best_mode);
	}

	/* Verify the controller supports the requested interface */
	ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
					iface);
	if (!ret)
		return ret;

	/* Fallback to slower modes */
	for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
		ret = onfi_fill_data_interface(chip, iface,
					       NAND_SDR_IFACE, mode);
		if (ret)
			continue;

		ret = ops->setup_data_interface(chip,
						NAND_DATA_IFACE_CHECK_ONLY,
						iface);
		if (!ret)
			break;
	}

	return 0;
}

> +EXPORT_SYMBOL_GPL(nand_choose_best_vendor_sdr_iface);
> +
>  /**
>   * nand_fill_column_cycles - fill the column cycles of an address
>   * @chip: The NAND chip


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