[U-Boot] [PATCH v1] mtd: nand: raw: allow to disable unneeded ECC layouts

[U-Boot] [PATCH v1] mtd: nand: raw: allow to disable unneeded ECC layouts

[Stefan Agner]

From: Stefan Agner <stefan.agner@toradex.com>

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.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
---

 drivers/mtd/nand/raw/Kconfig     | 7 +++++++
 drivers/mtd/nand/raw/nand_base.c | 4 ++++
 2 files changed, 11 insertions(+)

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.
 
+config SYS_NAND_DRIVER_ECC_LAYOUT
+	bool
+	help
+	  Omit standard ECC layouts to safe space. Select this if your driver
+	  is known to provide its own ECC layout.
+
 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/nand_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>
 
 /* Define default oob placement schemes for large and small page devices */
+#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
 static struct nand_ecclayout nand_oob_8 = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
@@ -89,6 +90,7 @@ static struct nand_ecclayout nand_oob_128 = {
 		{.offset = 2,
 		 .length = 78} }
 };
+#endif
 
 static int nand_get_device(struct mtd_info *mtd, int new_state);
 
@@ -4339,6 +4341,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 	 */
 	if (!ecc->layout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
 		switch (mtd->oobsize) {
+#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
 		case 8:
 			ecc->layout = &nand_oob_8;
 			break;
@@ -4351,6 +4354,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 		case 128:
 			ecc->layout = &nand_oob_128;
 			break;
+#endif
 		default:
 			pr_warn("No oob scheme defined for oobsize %d\n",
 				   mtd->oobsize);
-- 
2.19.2

[U-Boot] [PATCH v1] mtd: nand: raw: allow to disable unneeded ECC layouts

[Lukasz Majewski]

On Thu,  6 Dec 2018 14:57:09 +0100
Stefan Agner <stefan@agner.ch> wrote:

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> 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.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---
> 
>  drivers/mtd/nand/raw/Kconfig     | 7 +++++++
>  drivers/mtd/nand/raw/nand_base.c | 4 ++++
>  2 files changed, 11 insertions(+)
> 
> 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.
>  
> +config SYS_NAND_DRIVER_ECC_LAYOUT
> +	bool
> +	help
> +	  Omit standard ECC layouts to safe space. Select this if
> your driver
> +	  is known to provide its own ECC layout.
> +
>  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/nand_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>
>  
>  /* Define default oob placement schemes for large and small page
> devices */ +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>  static struct nand_ecclayout nand_oob_8 = {
>  	.eccbytes = 3,
>  	.eccpos = {0, 1, 2},
> @@ -89,6 +90,7 @@ static struct nand_ecclayout nand_oob_128 = {
>  		{.offset = 2,
>  		 .length = 78} }
>  };
> +#endif
>  
>  static int nand_get_device(struct mtd_info *mtd, int new_state);
>  
> @@ -4339,6 +4341,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	 */
>  	if (!ecc->layout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
>  		switch (mtd->oobsize) {
> +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>  		case 8:
>  			ecc->layout = &nand_oob_8;
>  			break;
> @@ -4351,6 +4354,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>  		case 128:
>  			ecc->layout = &nand_oob_128;
>  			break;
> +#endif
>  		default:
>  			pr_warn("No oob scheme defined for oobsize
> %d\n", mtd->oobsize);

Reviewed-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181206/50d72c8f/attachment.sig>

[U-Boot] [PATCH v1] mtd: nand: raw: allow to disable unneeded ECC layouts

[Miquel Raynal]

Hi Stefan,

Stefan Agner <stefan@agner.ch> wrote on Thu,  6 Dec 2018 14:57:09 +0100:

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> 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.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> ---
> 
>  drivers/mtd/nand/raw/Kconfig     | 7 +++++++
>  drivers/mtd/nand/raw/nand_base.c | 4 ++++
>  2 files changed, 11 insertions(+)
> 
> 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.
>  
> +config SYS_NAND_DRIVER_ECC_LAYOUT
> +	bool
> +	help
> +	  Omit standard ECC layouts to safe space. Select this if your driver

s/safe/save/

> +	  is known to provide its own ECC layout.

layout*s*?

> +
>  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/nand_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>
>  
>  /* Define default oob placement schemes for large and small page devices */
> +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>  static struct nand_ecclayout nand_oob_8 = {
>  	.eccbytes = 3,
>  	.eccpos = {0, 1, 2},
> @@ -89,6 +90,7 @@ static struct nand_ecclayout nand_oob_128 = {
>  		{.offset = 2,
>  		 .length = 78} }
>  };
> +#endif
>  
>  static int nand_get_device(struct mtd_info *mtd, int new_state);
>  
> @@ -4339,6 +4341,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	 */
>  	if (!ecc->layout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
>  		switch (mtd->oobsize) {
> +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>  		case 8:
>  			ecc->layout = &nand_oob_8;
>  			break;
> @@ -4351,6 +4354,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>  		case 128:
>  			ecc->layout = &nand_oob_128;
>  			break;
> +#endif
>  		default:
>  			pr_warn("No oob scheme defined for oobsize %d\n",
>  				   mtd->oobsize);

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.


Thanks,
Miquèl

[U-Boot] [PATCH v1] mtd: nand: raw: allow to disable unneeded ECC layouts

[Miquel Raynal]

Hello,

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

> Hi Stefan,
> 
> Stefan Agner <stefan@agner.ch> wrote on Thu,  6 Dec 2018 14:57:09 +0100:
> 
> > From: Stefan Agner <stefan.agner@toradex.com>
> > 
> > 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.
> > 
> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> > ---
> > 
> >  drivers/mtd/nand/raw/Kconfig     | 7 +++++++
> >  drivers/mtd/nand/raw/nand_base.c | 4 ++++
> >  2 files changed, 11 insertions(+)
> > 
> > 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.
> >  
> > +config SYS_NAND_DRIVER_ECC_LAYOUT
> > +	bool
> > +	help
> > +	  Omit standard ECC layouts to safe space. Select this if your driver  
> 
> s/safe/save/
> 
> > +	  is known to provide its own ECC layout.  
> 
> layout*s*?
> 
> > +
> >  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/nand_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>
> >  
> >  /* Define default oob placement schemes for large and small page devices */
> > +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
> >  static struct nand_ecclayout nand_oob_8 = {
> >  	.eccbytes = 3,
> >  	.eccpos = {0, 1, 2},
> > @@ -89,6 +90,7 @@ static struct nand_ecclayout nand_oob_128 = {
> >  		{.offset = 2,
> >  		 .length = 78} }
> >  };
> > +#endif
> >  
> >  static int nand_get_device(struct mtd_info *mtd, int new_state);
> >  
> > @@ -4339,6 +4341,7 @@ int nand_scan_tail(struct mtd_info *mtd)
> >  	 */
> >  	if (!ecc->layout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
> >  		switch (mtd->oobsize) {
> > +#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
> >  		case 8:
> >  			ecc->layout = &nand_oob_8;
> >  			break;
> > @@ -4351,6 +4354,7 @@ int nand_scan_tail(struct mtd_info *mtd)
> >  		case 128:
> >  			ecc->layout = &nand_oob_128;
> >  			break;
> > +#endif
> >  		default:
> >  			pr_warn("No oob scheme defined for oobsize %d\n",
> >  				   mtd->oobsize);  
> 
> 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èl

[U-Boot] [U-Boot, v1] mtd: nand: raw: allow to disable unneeded ECC layouts

[Tom Rini]

On Thu, Dec 06, 2018 at 02:57:09PM +0100, Stefan Agner wrote:

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> 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.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190117/57904027/attachment.sig>