Re: [PATCH v5 17/28] mtd: rawnand: Define a unique reset data interface

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Rickard Andersson <rickaran@axis.com>,
	Richard Weinberger <richard@nod.at>,
	linux-mtd@lists.infradead.org,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>
Subject: Re: [PATCH v5 17/28] mtd: rawnand: Define a unique reset data interface
Date: Tue, 26 May 2020 23:23:38 +0200	[thread overview]
Message-ID: <20200526232338.7d061be9@collabora.com> (raw)
In-Reply-To: <20200526191725.7591-18-miquel.raynal@bootlin.com>

On Tue, 26 May 2020 21:17:14 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> All NAND chips will always use the same data interface for reset and
> at startup: SDR mode 0. Instead of copying around the data interface
> timings, let's just have a default reset data interface for that.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/nand_base.c    | 9 +++------
>  drivers/mtd/nand/raw/nand_timings.c | 3 +++
>  include/linux/mtd/rawnand.h         | 3 +++
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 89f10a8c1d62..1005035c233a 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -927,9 +927,8 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
>  	 * timings to timing mode 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);
> +							  nand_reset_data_iface);
>  	if (ret)
>  		pr_err("Failed to configure data interface to SDR timing mode 0\n");
>  
> @@ -2484,7 +2483,6 @@ EXPORT_SYMBOL_GPL(nand_subop_get_data_len);
>   */
>  int nand_reset(struct nand_chip *chip, int chipnr)
>  {
> -	struct nand_data_interface saved_data_intf = chip->data_interface;
>  	int ret;
>  
>  	ret = nand_reset_data_interface(chip, chipnr);
> @@ -2509,11 +2507,10 @@ int nand_reset(struct nand_chip *chip, int chipnr)
>  	 * nand_setup_data_interface() uses ->set/get_features() which would
>  	 * fail anyway as the parameter page is not available yet.
>  	 */
> -	if (!memcmp(&chip->data_interface, &saved_data_intf,
> -		    sizeof(saved_data_intf)))
> +	if (!memcmp(&chip->data_interface, nand_reset_data_iface,
> +		    sizeof(*nand_reset_data_iface)))
>  		return 0;
>  
> -	chip->data_interface = saved_data_intf;
>  	ret = nand_setup_data_interface(chip, chipnr);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index 52ee83e75646..343284c43e81 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -285,6 +285,9 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
>  	},
>  };
>  
> +/* All NAND chips share the same reset data interface: SDR mode 0 */
> +const struct nand_data_interface *nand_reset_data_iface = &onfi_sdr_timings[0];

Can we make that one a function?

const struct nand_data_interface *nand_get_reset_data_interface(void)
{
	return &onfi_sdr_timings[0];
}

> +
>  /**
>   * onfi_find_closest_sdr_mode - Derive the closest ONFI SDR timing mode given a
>   *                              set of timings
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 622da6527a36..6c334cca3977 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1203,6 +1203,9 @@ static inline struct device_node *nand_get_flash_node(struct nand_chip *chip)
>  	return mtd_get_of_node(nand_to_mtd(chip));
>  }
>  
> +/* Default/reset data interface */
> +extern const struct nand_data_interface *nand_reset_data_iface;
> +
>  /*
>   * A helper for defining older NAND chips where the second ID byte fully
>   * defined the chip, including the geometry (chip size, eraseblock size, page

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