All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-22 20:16 ` Álvaro Fernández Rojas
  0 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-22 20:16 UTC (permalink / raw)
  To: Brian Norris, Kamal Dasu, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	linux-kernel
  Cc: Álvaro Fernández Rojas

Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
always be done without ECC enabled.
This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
from ff ff ff to 00 00 00, reporting incorrect ECC errors.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 659eaa6f0980..5ff4291380c5 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2688,6 +2688,12 @@ static int brcmnand_attach_chip(struct nand_chip *chip)
 
 	ret = brcmstb_choose_ecc_layout(host);
 
+	/* If OOB is written with ECC enabled it will cause ECC errors */
+	if (is_hamming_ecc(host->ctrl, &host->hwcfg)) {
+		chip->ecc.write_oob = brcmnand_write_oob_raw;
+		chip->ecc.read_oob = brcmnand_read_oob_raw;
+	}
+
 	return ret;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-22 20:16 ` Álvaro Fernández Rojas
  0 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-22 20:16 UTC (permalink / raw)
  To: Brian Norris, Kamal Dasu, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	linux-kernel
  Cc: Álvaro Fernández Rojas

Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
always be done without ECC enabled.
This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
from ff ff ff to 00 00 00, reporting incorrect ECC errors.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 659eaa6f0980..5ff4291380c5 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2688,6 +2688,12 @@ static int brcmnand_attach_chip(struct nand_chip *chip)
 
 	ret = brcmstb_choose_ecc_layout(host);
 
+	/* If OOB is written with ECC enabled it will cause ECC errors */
+	if (is_hamming_ecc(host->ctrl, &host->hwcfg)) {
+		chip->ecc.write_oob = brcmnand_write_oob_raw;
+		chip->ecc.read_oob = brcmnand_read_oob_raw;
+	}
+
 	return ret;
 }
 
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-22 20:16 ` Álvaro Fernández Rojas
@ 2021-02-24  3:46   ` Florian Fainelli
  -1 siblings, 0 replies; 20+ messages in thread
From: Florian Fainelli @ 2021-02-24  3:46 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, Brian Norris, Kamal Dasu,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	linux-mtd, bcm-kernel-feedback-list, linux-kernel



On 2/22/2021 12:16 PM, Álvaro Fernández Rojas wrote:
> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
> always be done without ECC enabled.
> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Should there be a Fixes: tag provided here for back porting to stable trees?
-- 
Florian

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-24  3:46   ` Florian Fainelli
  0 siblings, 0 replies; 20+ messages in thread
From: Florian Fainelli @ 2021-02-24  3:46 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, Brian Norris, Kamal Dasu,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	linux-mtd, bcm-kernel-feedback-list, linux-kernel



On 2/22/2021 12:16 PM, Álvaro Fernández Rojas wrote:
> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
> always be done without ECC enabled.
> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Should there be a Fixes: tag provided here for back porting to stable trees?
-- 
Florian

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-24  3:46   ` Florian Fainelli
@ 2021-02-24  7:16     ` Álvaro Fernández Rojas
  -1 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-24  7:16 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Brian Norris, Kamal Dasu, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	linux-kernel

Hi Florian,

> El 24 feb 2021, a las 4:46, Florian Fainelli <f.fainelli@gmail.com> escribió:
> 
> 
> 
> On 2/22/2021 12:16 PM, Álvaro Fernández Rojas wrote:
>> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
>> always be done without ECC enabled.
>> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
>> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
>> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
>> 
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> 
> Should there be a Fixes: tag provided here for back porting to stable trees?

I think so, but the fixed commit would be the first one, right?
27c5b17cd1b10564fa36f8f51e4b4b41436ecc32

> -- 
> Florian

Best regards,
Álvaro.

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-24  7:16     ` Álvaro Fernández Rojas
  0 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-24  7:16 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Vignesh Raghavendra, Kamal Dasu, Richard Weinberger,
	linux-kernel, linux-mtd, Miquel Raynal, bcm-kernel-feedback-list,
	Brian Norris

Hi Florian,

> El 24 feb 2021, a las 4:46, Florian Fainelli <f.fainelli@gmail.com> escribió:
> 
> 
> 
> On 2/22/2021 12:16 PM, Álvaro Fernández Rojas wrote:
>> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
>> always be done without ECC enabled.
>> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
>> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
>> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
>> 
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> 
> Should there be a Fixes: tag provided here for back porting to stable trees?

I think so, but the fixed commit would be the first one, right?
27c5b17cd1b10564fa36f8f51e4b4b41436ecc32

> -- 
> Florian

Best regards,
Álvaro.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-24  7:16     ` Álvaro Fernández Rojas
@ 2021-02-24  7:38       ` Miquel Raynal
  -1 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-02-24  7:38 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Florian Fainelli, Brian Norris, Kamal Dasu, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	linux-kernel

Hi Álvaro,

Álvaro Fernández Rojas <noltari@gmail.com> wrote on Wed, 24 Feb 2021
08:16:58 +0100:

> Hi Florian,
> 
> > El 24 feb 2021, a las 4:46, Florian Fainelli <f.fainelli@gmail.com> escribió:
> > 
> > 
> > 
> > On 2/22/2021 12:16 PM, Álvaro Fernández Rojas wrote:
> >> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
> >> always be done without ECC enabled.
> >> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
> >> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
> >> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
> >> 
> >> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > 
> > Should there be a Fixes: tag provided here for back porting to stable trees?
> 
> I think so, but the fixed commit would be the first one, right?
> 27c5b17cd1b10564fa36f8f51e4b4b41436ecc32

Yep, shouldn't be a problem.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-24  7:38       ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-02-24  7:38 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Florian Fainelli, Vignesh Raghavendra, Kamal Dasu,
	Richard Weinberger, linux-kernel, linux-mtd,
	bcm-kernel-feedback-list, Brian Norris

Hi Álvaro,

Álvaro Fernández Rojas <noltari@gmail.com> wrote on Wed, 24 Feb 2021
08:16:58 +0100:

> Hi Florian,
> 
> > El 24 feb 2021, a las 4:46, Florian Fainelli <f.fainelli@gmail.com> escribió:
> > 
> > 
> > 
> > On 2/22/2021 12:16 PM, Álvaro Fernández Rojas wrote:
> >> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
> >> always be done without ECC enabled.
> >> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
> >> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
> >> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
> >> 
> >> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > 
> > Should there be a Fixes: tag provided here for back porting to stable trees?
> 
> I think so, but the fixed commit would be the first one, right?
> 27c5b17cd1b10564fa36f8f51e4b4b41436ecc32

Yep, shouldn't be a problem.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-22 20:16 ` Álvaro Fernández Rojas
@ 2021-02-24  8:02   ` Álvaro Fernández Rojas
  -1 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-24  8:02 UTC (permalink / raw)
  To: f.fainelli, Brian Norris, Kamal Dasu, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd,
	bcm-kernel-feedback-list, linux-kernel
  Cc: Álvaro Fernández Rojas

Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
always be done without ECC enabled.
This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
from ff ff ff to 00 00 00, reporting incorrect ECC errors.

Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: Add fixed tag.

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 659eaa6f0980..5ff4291380c5 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2688,6 +2688,12 @@ static int brcmnand_attach_chip(struct nand_chip *chip)
 
 	ret = brcmstb_choose_ecc_layout(host);
 
+	/* If OOB is written with ECC enabled it will cause ECC errors */
+	if (is_hamming_ecc(host->ctrl, &host->hwcfg)) {
+		chip->ecc.write_oob = brcmnand_write_oob_raw;
+		chip->ecc.read_oob = brcmnand_read_oob_raw;
+	}
+
 	return ret;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-24  8:02   ` Álvaro Fernández Rojas
  0 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-24  8:02 UTC (permalink / raw)
  To: f.fainelli, Brian Norris, Kamal Dasu, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd,
	bcm-kernel-feedback-list, linux-kernel
  Cc: Álvaro Fernández Rojas

Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
always be done without ECC enabled.
This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
from ff ff ff to 00 00 00, reporting incorrect ECC errors.

Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: Add fixed tag.

 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 659eaa6f0980..5ff4291380c5 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2688,6 +2688,12 @@ static int brcmnand_attach_chip(struct nand_chip *chip)
 
 	ret = brcmstb_choose_ecc_layout(host);
 
+	/* If OOB is written with ECC enabled it will cause ECC errors */
+	if (is_hamming_ecc(host->ctrl, &host->hwcfg)) {
+		chip->ecc.write_oob = brcmnand_write_oob_raw;
+		chip->ecc.read_oob = brcmnand_read_oob_raw;
+	}
+
 	return ret;
 }
 
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-24  8:02   ` Álvaro Fernández Rojas
@ 2021-02-24 21:01     ` Brian Norris
  -1 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2021-02-24 21:01 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Florian Fainelli, Kamal Dasu, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	Linux Kernel

On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")

FWIW, I could believe this was broken. We weren't testing Hamming ECC
(nor JFFS2) at the time, so it could easily have obvious bugs like
this.

And since I got this far, and I'm still in MAINTAINERS, I guess:

Acked-by: Brian Norris <computersforpeace@gmail.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-24 21:01     ` Brian Norris
  0 siblings, 0 replies; 20+ messages in thread
From: Brian Norris @ 2021-02-24 21:01 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Florian Fainelli, Vignesh Raghavendra, Kamal Dasu,
	Richard Weinberger, Linux Kernel, linux-mtd, Miquel Raynal,
	bcm-kernel-feedback-list

On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")

FWIW, I could believe this was broken. We weren't testing Hamming ECC
(nor JFFS2) at the time, so it could easily have obvious bugs like
this.

And since I got this far, and I'm still in MAINTAINERS, I guess:

Acked-by: Brian Norris <computersforpeace@gmail.com>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-24 21:01     ` Brian Norris
@ 2021-02-25  7:48       ` Miquel Raynal
  -1 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-02-25  7:48 UTC (permalink / raw)
  To: Brian Norris
  Cc: Álvaro Fernández Rojas, Florian Fainelli, Kamal Dasu,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd,
	bcm-kernel-feedback-list, Linux Kernel

Hi Álvaro,

Brian Norris <computersforpeace@gmail.com> wrote on Wed, 24 Feb 2021
13:01:13 -0800:

> On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
> <noltari@gmail.com> wrote:
> > Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")  
> 
> FWIW, I could believe this was broken. We weren't testing Hamming ECC
> (nor JFFS2) at the time, so it could easily have obvious bugs like
> this.

Right, you should probably limit the backport to the time when raw
accessors got introduced/fixed.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-25  7:48       ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-02-25  7:48 UTC (permalink / raw)
  To: Brian Norris
  Cc: Florian Fainelli, Vignesh Raghavendra, Kamal Dasu,
	Richard Weinberger, Linux Kernel,
	Álvaro Fernández Rojas, linux-mtd,
	bcm-kernel-feedback-list

Hi Álvaro,

Brian Norris <computersforpeace@gmail.com> wrote on Wed, 24 Feb 2021
13:01:13 -0800:

> On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
> <noltari@gmail.com> wrote:
> > Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")  
> 
> FWIW, I could believe this was broken. We weren't testing Hamming ECC
> (nor JFFS2) at the time, so it could easily have obvious bugs like
> this.

Right, you should probably limit the backport to the time when raw
accessors got introduced/fixed.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-25  7:48       ` Miquel Raynal
@ 2021-02-25  7:54         ` Álvaro Fernández Rojas
  -1 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-25  7:54 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Brian Norris, Florian Fainelli, Kamal Dasu, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	Linux Kernel

Hi Miquel,

> El 25 feb 2021, a las 8:48, Miquel Raynal <miquel.raynal@bootlin.com> escribió:
> 
> Hi Álvaro,
> 
> Brian Norris <computersforpeace@gmail.com> wrote on Wed, 24 Feb 2021
> 13:01:13 -0800:
> 
>> On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
>> <noltari@gmail.com> wrote:
>>> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")  
>> 
>> FWIW, I could believe this was broken. We weren't testing Hamming ECC
>> (nor JFFS2) at the time, so it could easily have obvious bugs like
>> this.
> 
> Right, you should probably limit the backport to the time when raw
> accessors got introduced/fixed.

What do you mean?
Those accessors have been there since the first commit (27c5b17cd1b10564fa36f8f51e4b4b41436ecc32):
https://github.com/torvalds/linux/blob/27c5b17cd1b10564fa36f8f51e4b4b41436ecc32/drivers/mtd/nand/brcmnand/brcmnand.c#L1896-L1899

> 
> Thanks,
> Miquèl

Best regards,
Álvaro.

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-25  7:54         ` Álvaro Fernández Rojas
  0 siblings, 0 replies; 20+ messages in thread
From: Álvaro Fernández Rojas @ 2021-02-25  7:54 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Florian Fainelli, Vignesh Raghavendra, Kamal Dasu,
	Richard Weinberger, Linux Kernel, linux-mtd,
	bcm-kernel-feedback-list, Brian Norris

Hi Miquel,

> El 25 feb 2021, a las 8:48, Miquel Raynal <miquel.raynal@bootlin.com> escribió:
> 
> Hi Álvaro,
> 
> Brian Norris <computersforpeace@gmail.com> wrote on Wed, 24 Feb 2021
> 13:01:13 -0800:
> 
>> On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
>> <noltari@gmail.com> wrote:
>>> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")  
>> 
>> FWIW, I could believe this was broken. We weren't testing Hamming ECC
>> (nor JFFS2) at the time, so it could easily have obvious bugs like
>> this.
> 
> Right, you should probably limit the backport to the time when raw
> accessors got introduced/fixed.

What do you mean?
Those accessors have been there since the first commit (27c5b17cd1b10564fa36f8f51e4b4b41436ecc32):
https://github.com/torvalds/linux/blob/27c5b17cd1b10564fa36f8f51e4b4b41436ecc32/drivers/mtd/nand/brcmnand/brcmnand.c#L1896-L1899

> 
> Thanks,
> Miquèl

Best regards,
Álvaro.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-25  7:54         ` Álvaro Fernández Rojas
@ 2021-02-25  8:12           ` Miquel Raynal
  -1 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-02-25  8:12 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Brian Norris, Florian Fainelli, Kamal Dasu, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	Linux Kernel

Hi Álvaro,

Álvaro Fernández Rojas <noltari@gmail.com> wrote on Thu, 25 Feb 2021
08:54:09 +0100:

> Hi Miquel,
> 
> > El 25 feb 2021, a las 8:48, Miquel Raynal <miquel.raynal@bootlin.com> escribió:
> > 
> > Hi Álvaro,
> > 
> > Brian Norris <computersforpeace@gmail.com> wrote on Wed, 24 Feb 2021
> > 13:01:13 -0800:
> >   
> >> On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
> >> <noltari@gmail.com> wrote:  
> >>> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")    
> >> 
> >> FWIW, I could believe this was broken. We weren't testing Hamming ECC
> >> (nor JFFS2) at the time, so it could easily have obvious bugs like
> >> this.  
> > 
> > Right, you should probably limit the backport to the time when raw
> > accessors got introduced/fixed.  
> 
> What do you mean?
> Those accessors have been there since the first commit (27c5b17cd1b10564fa36f8f51e4b4b41436ecc32):
> https://github.com/torvalds/linux/blob/27c5b17cd1b10564fa36f8f51e4b4b41436ecc32/drivers/mtd/nand/brcmnand/brcmnand.c#L1896-L1899

I misunderstood Brian's answer. This commit is not that old and looks
legit.

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-02-25  8:12           ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-02-25  8:12 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Florian Fainelli, Vignesh Raghavendra, Kamal Dasu,
	Richard Weinberger, Linux Kernel, linux-mtd,
	bcm-kernel-feedback-list, Brian Norris

Hi Álvaro,

Álvaro Fernández Rojas <noltari@gmail.com> wrote on Thu, 25 Feb 2021
08:54:09 +0100:

> Hi Miquel,
> 
> > El 25 feb 2021, a las 8:48, Miquel Raynal <miquel.raynal@bootlin.com> escribió:
> > 
> > Hi Álvaro,
> > 
> > Brian Norris <computersforpeace@gmail.com> wrote on Wed, 24 Feb 2021
> > 13:01:13 -0800:
> >   
> >> On Wed, Feb 24, 2021 at 12:02 AM Álvaro Fernández Rojas
> >> <noltari@gmail.com> wrote:  
> >>> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")    
> >> 
> >> FWIW, I could believe this was broken. We weren't testing Hamming ECC
> >> (nor JFFS2) at the time, so it could easily have obvious bugs like
> >> this.  
> > 
> > Right, you should probably limit the backport to the time when raw
> > accessors got introduced/fixed.  
> 
> What do you mean?
> Those accessors have been there since the first commit (27c5b17cd1b10564fa36f8f51e4b4b41436ecc32):
> https://github.com/torvalds/linux/blob/27c5b17cd1b10564fa36f8f51e4b4b41436ecc32/drivers/mtd/nand/brcmnand/brcmnand.c#L1896-L1899

I misunderstood Brian's answer. This commit is not that old and looks
legit.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
  2021-02-24  8:02   ` Álvaro Fernández Rojas
@ 2021-03-02 16:32     ` Miquel Raynal
  -1 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-03-02 16:32 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, f.fainelli, Brian Norris,
	Kamal Dasu, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	linux-kernel

On Wed, 2021-02-24 at 08:02:10 UTC, =?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
> always be done without ECC enabled.
> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
> 
> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Acked-by: Brian Norris <computersforpeace@gmail.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v2] nand: brcmnand: fix OOB R/W with Hamming ECC
@ 2021-03-02 16:32     ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2021-03-02 16:32 UTC (permalink / raw)
  To: Álvaro Fernández Rojas, f.fainelli, Brian Norris,
	Kamal Dasu, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, linux-mtd, bcm-kernel-feedback-list,
	linux-kernel

On Wed, 2021-02-24 at 08:02:10 UTC, =?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> Hamming ECC doesn't cover the OOB data, so reading or writing OOB shall
> always be done without ECC enabled.
> This is a problem when adding JFFS2 cleanmarkers to erased blocks. If JFFS2
> clenmarkers are added to the OOB with ECC enabled, OOB bytes will be changed
> from ff ff ff to 00 00 00, reporting incorrect ECC errors.
> 
> Fixes: 27c5b17cd1b1 ("mtd: nand: add NAND driver "library" for Broadcom STB NAND controller")
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Acked-by: Brian Norris <computersforpeace@gmail.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-03-03 15:01 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-22 20:16 [PATCH] nand: brcmnand: fix OOB R/W with Hamming ECC Álvaro Fernández Rojas
2021-02-22 20:16 ` Álvaro Fernández Rojas
2021-02-24  3:46 ` Florian Fainelli
2021-02-24  3:46   ` Florian Fainelli
2021-02-24  7:16   ` Álvaro Fernández Rojas
2021-02-24  7:16     ` Álvaro Fernández Rojas
2021-02-24  7:38     ` Miquel Raynal
2021-02-24  7:38       ` Miquel Raynal
2021-02-24  8:02 ` [PATCH v2] " Álvaro Fernández Rojas
2021-02-24  8:02   ` Álvaro Fernández Rojas
2021-02-24 21:01   ` Brian Norris
2021-02-24 21:01     ` Brian Norris
2021-02-25  7:48     ` Miquel Raynal
2021-02-25  7:48       ` Miquel Raynal
2021-02-25  7:54       ` Álvaro Fernández Rojas
2021-02-25  7:54         ` Álvaro Fernández Rojas
2021-02-25  8:12         ` Miquel Raynal
2021-02-25  8:12           ` Miquel Raynal
2021-03-02 16:32   ` Miquel Raynal
2021-03-02 16:32     ` Miquel Raynal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.