Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com>
To:     Boris Brezillon <boris.brezillon@collabora.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: [EXT] Re: [PATCH v3 04/12] mtd: rawnand: introduce struct
 onfi_helper
Thread-Topic: [EXT] Re: [PATCH v3 04/12] mtd: rawnand: introduce struct
 onfi_helper
Thread-Index: AdUaBIJVYblkvvsDT0y1heXJZCvR1gACIb2AACiX1UA=
Date:   Tue, 4 Jun 2019 12:02:28 +0000
Message-ID: <MN2PR08MB595131826D34BFD773DF1251B8150@MN2PR08MB5951.namprd08.prod.outlook.com>
References: <MN2PR08MB5951E35FED92DD502F57B590B8140@MN2PR08MB5951.namprd08.prod.outlook.com>
 <20190603150537.3ca5ca8a@collabora.com>
In-Reply-To: <20190603150537.3ca5ca8a@collabora.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: micron.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: a0bed46e-1543-4f8a-97e8-08d6e8e4843a
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Jun 2019 12:02:28.5965
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: f38a5ecd-2813-4862-b11b-ac1d563c806f
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: sshivamurthy@micron.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR08MB5741
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Boris,

> > 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 implement=
ed
> 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;
>=20
> 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.
>=20

In that case, I am thinking to design as follows, which splits into generic=
 independent code.
Let me know, if you have any concerns or inputs.

I will parsing code from nand_onfi_detect function and move it to mtd/nand/=
onfi.c.
Also, I will move functions like sanitize_string, nand_bit_wise_majority, o=
nfi_crc16, and=20
any other generic info to mtd/nand/onfi.c.

Thanks,
Shiva