| Message ID | 20190417090942.19595-1-gregory.clement@bootlin.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] mtd: nand: raw: Fix CONFIG_SYS_NAND_DRIVER_ECC_LAYOUT behaviour | expand |
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
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
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;
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(-)