From mboxrd@z Thu Jan  1 00:00:00 1970
From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: Fri, 7 Dec 2018 11:31:30 +0100
Subject: [U-Boot] [PATCH v1] mtd: nand: raw: allow to disable unneeded
 ECC layouts
In-Reply-To: <20181207110624.450f3b50@xps13>
References: <20181206135709.8518-1-stefan@agner.ch>
 <20181207110624.450f3b50@xps13>
Message-ID: <20181207113130.39cf2db9@xps13>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
To: u-boot@lists.denx.de

Hello,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Fri, 7 Dec 2018
11:06:24 +0100:

> Hi Stefan,
>=20
> Stefan Agner <stefan@agner.ch> wrote on Thu,  6 Dec 2018 14:57:09 +0100:
>=20
> > From: Stefan Agner <stefan.agner@toradex.com>
> >=20
> > Each ECC layout consumes about 2984 bytes in the .data section. Allow
> > to disable the default ECC layouts if a driver is known to provide its
> > own ECC layout.
> >=20
> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> > ---
> >=20
> >  drivers/mtd/nand/raw/Kconfig     | 7 +++++++
> >  drivers/mtd/nand/raw/nand_base.c | 4 ++++
> >  2 files changed, 11 insertions(+)
> >=20
> > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> > index 008f7b4b4b..b06c45788a 100644
> > --- a/drivers/mtd/nand/raw/Kconfig
> > +++ b/drivers/mtd/nand/raw/Kconfig
> > @@ -9,6 +9,12 @@ config SYS_NAND_SELF_INIT
> >  	  This option, if enabled, provides more flexible and linux-like
> >  	  NAND initialization process.
> > =20
> > +config SYS_NAND_DRIVER_ECC_LAYOUT
> > +	bool
> > +	help
> > +	  Omit standard ECC layouts to safe space. Select this if your driver=
 =20
>=20
> s/safe/save/
>=20
> > +	  is known to provide its own ECC layout. =20
>=20
> layout*s*?
>=20
> > +
> >  config NAND_ATMEL
> >  	bool "Support Atmel NAND controller"
> >  	imply SYS_NAND_USE_FLASH_BBT
> > @@ -81,6 +87,7 @@ config NAND_OMAP_ELM
> >  config NAND_VF610_NFC
> >  	bool "Support for Freescale NFC for VF610"
> >  	select SYS_NAND_SELF_INIT
> > +	select SYS_NAND_DRIVER_ECC_LAYOUT
> >  	imply CMD_NAND
> >  	help
> >  	  Enables support for NAND Flash Controller on some Freescale
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/na=
nd_base.c
> > index 92daebe120..6d2ff58d86 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -47,6 +47,7 @@
> >  #include <linux/errno.h>
> > =20
> >  /* Define default oob placement schemes for large and small page devic=
es */
> > +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
> >  static struct nand_ecclayout nand_oob_8 =3D {
> >  	.eccbytes =3D 3,
> >  	.eccpos =3D {0, 1, 2},
> > @@ -89,6 +90,7 @@ static struct nand_ecclayout nand_oob_128 =3D {
> >  		{.offset =3D 2,
> >  		 .length =3D 78} }
> >  };
> > +#endif
> > =20
> >  static int nand_get_device(struct mtd_info *mtd, int new_state);
> > =20
> > @@ -4339,6 +4341,7 @@ int nand_scan_tail(struct mtd_info *mtd)
> >  	 */
> >  	if (!ecc->layout && (ecc->mode !=3D NAND_ECC_SOFT_BCH)) {
> >  		switch (mtd->oobsize) {
> > +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
> >  		case 8:
> >  			ecc->layout =3D &nand_oob_8;
> >  			break;
> > @@ -4351,6 +4354,7 @@ int nand_scan_tail(struct mtd_info *mtd)
> >  		case 128:
> >  			ecc->layout =3D &nand_oob_128;
> >  			break;
> > +#endif
> >  		default:
> >  			pr_warn("No oob scheme defined for oobsize %d\n",
> >  				   mtd->oobsize); =20
>=20
> I don't like very much the #ifdef approach to compile-out zones in the
> code. I don't have an alternative for now (maybe have the layouts in a
> different file?). I would also like to hear Boris' point of view.

I overlooked the headers, I thought you wanted that in Linux, that's
why I was asking Boris. For U-Boot I agree with the idea and we already
have a lot of #ifdef around there so unless someone else complains:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miqu=C3=A8l