Re: [PATCH v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook

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 v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook
Date: Mon, 25 May 2020 21:51:20 +0200	[thread overview]
Message-ID: <20200525215120.57c28588@collabora.com> (raw)
In-Reply-To: <20200525174239.11349-19-miquel.raynal@bootlin.com>

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

> This hook can be overloaded by NAND manufacturer drivers to propose
> alternative timings when not following the main standards.
> 
> Vendors implementing this hook should:

"Manufacturer drivers ..."

> 1- choose the best timings and fill the data interface,
> 2- verify that the controller supports them.

#1 clearly depends on #2, so I'd just say that the manufacturer driver
is responsible for choosing the data interface config that fits both
the controller and chip capabilities (ideally the best one :)).

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/internals.h | 5 +++++
>  drivers/mtd/nand/raw/nand_base.c | 8 ++++++++
>  include/linux/mtd/rawnand.h      | 2 ++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 9af6979257e2..61edbab35068 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -146,6 +146,11 @@ static inline bool nand_controller_has_setup_data_iface(struct nand_chip *chip)
>  	return true;
>  }
>  
> +static inline bool nand_can_choose_data_interface(struct nand_chip *chip)
> +{
> +	return chip->ops.choose_data_interface;
> +}
> +
>  /* BBT functions */
>  int nand_markbad_bbt(struct nand_chip *chip, loff_t offs);
>  int nand_isreserved_bbt(struct nand_chip *chip, loff_t offs);
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index d9fe7795f183..e9df339849d3 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1060,6 +1060,14 @@ static int nand_choose_data_interface(struct nand_chip *chip)
>  	if (!nand_controller_has_setup_data_iface(chip))
>  		return 0;
>  
> +	/*
> +	 * Let the NAND vendor hook identify the best timings.
> +	 * ->choose_data_interface() is expected to update the entire chip's
> +	 * nand_data_interface structure.
> +	 */
> +	if (nand_can_choose_data_interface(chip))
> +		return chip->ops.choose_data_interface(chip);
> +
>  	/*
>  	 * First try to identify the best timings from ONFI parameters and
>  	 * if the NAND does not support ONFI, fallback to the default ONFI
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index a4b68e7b246a..5b8b94521a18 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1034,6 +1034,7 @@ struct nand_legacy {
>   * @lock_area: Lock operation
>   * @unlock_area: Unlock operation
>   * @setup_read_retry: Set the read-retry mode (mostly needed for MLC NANDs)
> + * @choose_data_interface: Choose the best data interface
>   */
>  struct nand_chip_ops {
>  	int (*suspend)(struct nand_chip *chip);
> @@ -1041,6 +1042,7 @@ struct nand_chip_ops {
>  	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>  	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
>  	int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
> +	int (*choose_data_interface)(struct nand_chip *chip);
>  };
>  
>  /**

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