Re: [PATCH v3 04/12] mtd: rawnand: introduce struct onfi_helper

From: Boris Brezillon <boris.brezillon@collabora.com>
To: "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Boris Brezillon <bbrezillon@kernel.org>,
	Yixun Lan <yixun.lan@amlogic.com>, Lucas Stach <dev@lynxeye.de>,
	Anders Roxell <anders.roxell@linaro.org>,
	Stefan Agner <stefan@agner.ch>,
	Marcel Ziswiler <marcel.ziswiler@toradex.com>,
	"Bean Huo (beanhuo)" <beanhuo@micron.com>,
	Paul Cercueil <paul@crapouillou.net>,
	Frieder Schrempf <frieder.schrempf@kontron.de>,
	Chuanhong Guo <gch981213@gmail.com>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 04/12] mtd: rawnand: introduce struct onfi_helper
Date: Mon, 3 Jun 2019 15:05:37 +0200	[thread overview]
Message-ID: <20190603150537.3ca5ca8a@collabora.com> (raw)
In-Reply-To: <MN2PR08MB5951E35FED92DD502F57B590B8140@MN2PR08MB5951.namprd08.prod.outlook.com>

Hi Shivamurthy,

On Mon, 3 Jun 2019 12:43:28 +0000
"Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote:

> Create onfi_helper object. This is base to turn ONFI code to generic.
> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>  include/linux/mtd/nand.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 3cdf06cae8b6..645dde4c5797 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -11,6 +11,7 @@
>  #define __LINUX_MTD_NAND_H
>  
>  #include <linux/mtd/mtd.h>
> +#include <linux/mtd/onfi.h>
>  
>  /**
>   * struct nand_memory_organization - Memory organization structure
> @@ -157,6 +158,24 @@ struct nand_ops {
>  	bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos);
>  };
>  
> +/**
> + * struct onfi_helper - ONFI helper functions that should be implemented by
> + * specialized layers (raw NAND, SPI NAND, etc.)
> + * @page: Page number for ONFI parameter table
> + * @check_revision: Check ONFI revision number
> + * @parameter_page_read: Function to read parameter pages
> + * @init_intf_data: Initialize interface specific data or fixups
> + */
> +struct onfi_helper {
> +	u8 page;
> +	int (*check_revision)(struct nand_device *base,
> +			      struct nand_onfi_params *p, int *onfi_version);
> +	int (*parameter_page_read)(struct nand_device *base, u8 page,
> +				   void *buf, unsigned int len);
> +	int (*init_intf_data)(struct nand_device *base,
> +			      struct nand_onfi_params *p);
> +};
> +
>  /**
>   * struct nand_device - NAND device
>   * @mtd: MTD instance attached to the NAND device
> @@ -165,6 +184,7 @@ struct nand_ops {
>   * @rowconv: position to row address converter
>   * @bbt: bad block table info
>   * @ops: NAND operations attached to the NAND device
> + * @helper: Helper functions to detect and initialize ONFI NAND
>   *
>   * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, OneNAND)
>   * should declare their own NAND object embedding a nand_device struct (that's
> @@ -183,6 +203,7 @@ struct nand_device {
>  	struct nand_row_converter rowconv;
>  	struct nand_bbt bbt;
>  	const struct nand_ops *ops;
> +	struct onfi_helper helper;

Sorry, but I don't think that's the right solution. When I said we
should have ONFI code shared I was thinking about the code that parses
the ONFI struct/data to extract nand_memory_organization bits or other
generic info, not something that would abstract how to retrieve the
ONFI param page. Clearly, the generic NAND layer is not supposed to
handle such protocol/low-level details.

Regards,

Boris