[U-Boot] [PATCH] mtd: nand: raw: Fix CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT behaviour

[U-Boot] [PATCH] mtd: nand: raw: Fix CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT behaviour

[Gregory CLEMENT]

The purpose of "mtd: nand: raw: allow to disable unneeded ECC layouts"
was to allow disabling the default ECC layouts if a driver is known to
provide its own ECC layout. However, this commit did the opposite and
disbaled the default layout whqant it was _not_ selectec.

It breaks all the driver nand that not providing their ecc layout, this
patch fix this sitution.

It was tested with the lpc32xx_nand_slc driver.

Fixes: a38c3af868 ("mtd: nand: raw: allow to disable unneeded ECC layouts")
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 6d2ff58d86..dc4576a3a1 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -47,7 +47,7 @@
 #include <linux/errno.h>
 
 /* Define default oob placement schemes for large and small page devices */
-#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
+#ifndef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
 static struct nand_ecclayout nand_oob_8 = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
@@ -4341,7 +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
+#ifndef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
 		case 8:
 			ecc->layout = &nand_oob_8;
 			break;
-- 
2.20.1

[U-Boot] [PATCH] mtd: nand: raw: Fix CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT behaviour

[Miquel Raynal]

Hi Gregory,

Gregory CLEMENT <gregory.clement@bootlin.com> wrote on Wed, 17 Apr 2019
11:09:42 +0200:

> The purpose of "mtd: nand: raw: allow to disable unneeded ECC layouts"
> was to allow disabling the default ECC layouts if a driver is known to
> provide its own ECC layout. However, this commit did the opposite and
> disbaled the default layout whqant it was _not_ selectec.

                              when                 selected

> 
> It breaks all the driver nand that not providing their ecc layout, this

What about "It breaks all the NAND drivers not providing their own ECC
layout".

> patch fix this sitution.

                 situation

> 
> It was tested with the lpc32xx_nand_slc driver.
> 
> Fixes: a38c3af868 ("mtd: nand: raw: allow to disable unneeded ECC layouts")
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---

I missed that mistake in my original review. Thanks for fixing it.

With above typos fixed:

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

>  drivers/mtd/nand/raw/nand_base.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 6d2ff58d86..dc4576a3a1 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -47,7 +47,7 @@
>  #include <linux/errno.h>
>  
>  /* Define default oob placement schemes for large and small page devices */
> -#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
> +#ifndef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>  static struct nand_ecclayout nand_oob_8 = {
>  	.eccbytes = 3,
>  	.eccpos = {0, 1, 2},
> @@ -4341,7 +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
> +#ifndef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>  		case 8:
>  			ecc->layout = &nand_oob_8;
>  			break;

Thanks,
Miquèl

[U-Boot] [PATCH] mtd: nand: raw: Fix CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT behaviour

[Gregory CLEMENT]

Miquel Raynal <miquel.raynal@bootlin.com> writes:

Hi Miquel,

> Hi Gregory,
>
> Gregory CLEMENT <gregory.clement@bootlin.com> wrote on Wed, 17 Apr 2019
> 11:09:42 +0200:
>
>> The purpose of "mtd: nand: raw: allow to disable unneeded ECC layouts"
>> was to allow disabling the default ECC layouts if a driver is known to
>> provide its own ECC layout. However, this commit did the opposite and
>> disbaled the default layout whqant it was _not_ selectec.
>
>                               when                 selected
>
>> 
>> It breaks all the driver nand that not providing their ecc layout, this
>
> What about "It breaks all the NAND drivers not providing their own ECC
> layout".
>
>> patch fix this sitution.
>
>                  situation
>
>> 
>> It was tested with the lpc32xx_nand_slc driver.
>> 
>> Fixes: a38c3af868 ("mtd: nand: raw: allow to disable unneeded ECC layouts")
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>
> I missed that mistake in my original review. Thanks for fixing it.
>
> With above typos fixed:
>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks I've just sent a v2 with the typo fixed and the new wording as
well as a another typo fix in the topic.

Thanks,

Gregory


>
>>  drivers/mtd/nand/raw/nand_base.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
>> index 6d2ff58d86..dc4576a3a1 100644
>> --- a/drivers/mtd/nand/raw/nand_base.c
>> +++ b/drivers/mtd/nand/raw/nand_base.c
>> @@ -47,7 +47,7 @@
>>  #include <linux/errno.h>
>>  
>>  /* Define default oob placement schemes for large and small page devices */
>> -#ifdef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>> +#ifndef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>>  static struct nand_ecclayout nand_oob_8 = {
>>  	.eccbytes = 3,
>>  	.eccpos = {0, 1, 2},
>> @@ -4341,7 +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
>> +#ifndef CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT
>>  		case 8:
>>  			ecc->layout = &nand_oob_8;
>>  			break;
>
> Thanks,
> Miquèl

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com