Re: [PATCH v6 13/30] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Rickard Andersson <rickaran@axis.com>,
	Michal Simek <monstr@monstr.eu>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	Richard Weinberger <richard@nod.at>,
	linux-mtd@lists.infradead.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Naga Sureshkumar Relli <nagasure@xilinx.com>
Subject: Re: [PATCH v6 13/30] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface
Date: Fri, 29 May 2020 09:16:59 +0200	[thread overview]
Message-ID: <20200529091659.7d2ef884@collabora.com> (raw)
In-Reply-To: <20200528231612.8958-14-miquel.raynal@bootlin.com>

On Fri, 29 May 2020 01:15:55 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> We rely be default on the data interface which is in the nand_chip

          ^by

> structure but it should be possible to fill any other data interface.

But even then it sounds weird. How about:

"
Right now the core uses onfi_fill_data_interface() to initialize the
nand_data_interface object embedded in nand_chip, but we are about to
allocate this object dynamically and let manufacturer drivers provide
their own interface config. Let's patch the onfi_fill_data_interface()
so it can initialize an interface config that's not the one
currently attached to the nand_chip.
"

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/internals.h    | 1 +
>  drivers/mtd/nand/raw/nand_base.c    | 7 ++++---
>  drivers/mtd/nand/raw/nand_timings.c | 3 ++-
>  3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index a5e2cec7e301..0f74509abc4c 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -85,6 +85,7 @@ int nand_markbad_bbm(struct nand_chip *chip, loff_t ofs);
>  int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
>  		    int allowbbt);
>  int onfi_fill_data_interface(struct nand_chip *chip,
> +			     struct nand_data_interface *iface,
>  			     enum nand_data_interface_type type,
>  			     int timing_mode);
>  int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 1d9cf02d164b..b4de85794e07 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -927,7 +927,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
>  	 * timings to timing mode 0.
>  	 */
>  
> -	onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
> +	onfi_fill_data_interface(chip, &chip->data_interface, NAND_SDR_IFACE, 0);
>  	ret = chip->controller->ops->setup_data_interface(chip, chipnr,
>  							&chip->data_interface);
>  	if (ret)
> @@ -1040,7 +1040,8 @@ static int nand_choose_data_interface(struct nand_chip *chip)
>  	}
>  
>  	for (mode = fls(modes) - 1; mode >= 0; mode--) {
> -		ret = onfi_fill_data_interface(chip, NAND_SDR_IFACE, mode);
> +		ret = onfi_fill_data_interface(chip, &chip->data_interface,
> +					       NAND_SDR_IFACE, mode);
>  		if (ret)
>  			continue;
>  
> @@ -5182,7 +5183,7 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
>  	mutex_init(&chip->lock);
>  
>  	/* Enforce the right timings for reset/detection */
> -	onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
> +	onfi_fill_data_interface(chip, &chip->data_interface, NAND_SDR_IFACE, 0);
>  
>  	ret = nand_dt_init(chip);
>  	if (ret)
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index a73d934e86f9..ce6bb87db2e8 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -276,14 +276,15 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  /**
>   * onfi_fill_data_interface - Initialize a data interface from a given ONFI mode
>   * @chip: The NAND chip
> + * @iface: The data interface to fill
>   * @type: The data interface type
>   * @timing_mode: The ONFI timing mode
>   */
>  int onfi_fill_data_interface(struct nand_chip *chip,
> +			     struct nand_data_interface *iface,
>  			     enum nand_data_interface_type type,
>  			     int timing_mode)
>  {
> -	struct nand_data_interface *iface = &chip->data_interface;
>  	struct onfi_params *onfi = chip->parameters.onfi;
>  
>  	if (type != NAND_SDR_IFACE)

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