2020-05-04 09:33:34

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH 1/2] nand: brcmnand: improve hamming oob layout

The current code generates 8 oob sections:
S1 1-5
ECC 6-8
S2 9-15
S3 16-21
ECC 22-24
S4 25-31
S5 32-37
ECC 38-40
S6 41-47
S7 48-53
ECC 54-56
S8 57-63

Change it by merging continuous sections:
S1 1-5
ECC 6-8
S2 9-21
ECC 22-24
S3 25-37
ECC 38-40
S4 41-53
ECC 54-56
S5 57-63

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 28 +++++++++---------------
1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index e4e3ceeac38f..1bba309c7684 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1100,29 +1100,21 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
struct brcmnand_cfg *cfg = &host->hwcfg;
int sas = cfg->spare_area_size << cfg->sector_size_1k;
int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
+ u32 next;

- if (section >= sectors * 2)
+ if (section > sectors)
return -ERANGE;

- oobregion->offset = (section / 2) * sas;
+ next = (section * sas);
+ if (section < sectors)
+ next += 6;

- if (section & 1) {
- oobregion->offset += 9;
- oobregion->length = 7;
- } else {
- oobregion->length = 6;
+ if (section)
+ oobregion->offset = ((section - 1) * sas) + 9;
+ else
+ oobregion->offset = 1; /* BBI */

- /* First sector of each page may have BBI */
- if (!section) {
- /*
- * Small-page NAND use byte 6 for BBI while large-page
- * NAND use byte 0.
- */
- if (cfg->page_size > 512)
- oobregion->offset++;
- oobregion->length--;
- }
- }
+ oobregion->length = next - oobregion->offset;

return 0;
}
--
2.26.2


2020-05-04 09:33:38

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH 2/2] nand: brcmnand: fix BBI in hamming oob layout

Small Page NAND uses byte 6 for BBI and Large Page NAND uses first 2 bytes.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 1bba309c7684..59c3241f4ea5 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1109,10 +1109,18 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
if (section < sectors)
next += 6;

- if (section)
+ if (section) {
oobregion->offset = ((section - 1) * sas) + 9;
- else
- oobregion->offset = 1; /* BBI */
+ } else {
+ if (cfg->page_size == 512) {
+ /* small page uses byte 6 for BBI */
+ oobregion->offset = 0;
+ next--;
+ } else {
+ /* large page uses first 2 bytes for BBI */
+ oobregion->offset = 2;
+ }
+ }

oobregion->length = next - oobregion->offset;

--
2.26.2

2020-05-04 19:01:45

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v2 1/2] nand: brcmnand: improve hamming oob layout

The current code generates 8 oob sections:
S1 1-5
ECC 6-8
S2 9-15
S3 16-21
ECC 22-24
S4 25-31
S5 32-37
ECC 38-40
S6 41-47
S7 48-53
ECC 54-56
S8 57-63

Change it by merging continuous sections:
S1 1-5
ECC 6-8
S2 9-21
ECC 22-24
S3 25-37
ECC 38-40
S4 41-53
ECC 54-56
S5 57-63

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
v2: keep original comment and fix correctly skip byte 6 for small-page nand

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 34 +++++++++++++-----------
1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index e4e3ceeac38f..767343e0e6e7 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1100,30 +1100,32 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
struct brcmnand_cfg *cfg = &host->hwcfg;
int sas = cfg->spare_area_size << cfg->sector_size_1k;
int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
+ u32 next;

- if (section >= sectors * 2)
+ if (section > sectors)
return -ERANGE;

- oobregion->offset = (section / 2) * sas;
+ next = (section * sas);
+ if (section < sectors)
+ next += 6;

- if (section & 1) {
- oobregion->offset += 9;
- oobregion->length = 7;
+ if (section) {
+ oobregion->offset = ((section - 1) * sas) + 9;
} else {
- oobregion->length = 6;
-
- /* First sector of each page may have BBI */
- if (!section) {
- /*
- * Small-page NAND use byte 6 for BBI while large-page
- * NAND use byte 0.
- */
- if (cfg->page_size > 512)
- oobregion->offset++;
- oobregion->length--;
+ /*
+ * Small-page NAND use byte 6 for BBI while large-page
+ * NAND use byte 0.
+ */
+ if (cfg->page_size > 512) {
+ oobregion->offset = 1;
+ } else {
+ oobregion->offset = 0;
+ next--;
}
}

+ oobregion->length = next - oobregion->offset;
+
return 0;
}

--
2.26.2

2020-05-04 19:01:59

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v2 2/2] nand: brcmnand: fix hamming oob layout

First 2 bytes are used in large-page nand.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
v2: extend original comment

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 767343e0e6e7..0a1d76fde37b 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1114,10 +1114,10 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
} else {
/*
* Small-page NAND use byte 6 for BBI while large-page
- * NAND use byte 0.
+ * NAND use bytes 0 and 1.
*/
if (cfg->page_size > 512) {
- oobregion->offset = 1;
+ oobregion->offset = 2;
} else {
oobregion->offset = 0;
next--;
--
2.26.2

2020-05-11 16:36:07

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] nand: brcmnand: improve hamming oob layout

Hi Álvaro,

Álvaro Fernández Rojas <[email protected]> wrote on Mon, 4 May 2020
20:59:44 +0200:

> The current code generates 8 oob sections:
> S1 1-5
> ECC 6-8
> S2 9-15
> S3 16-21
> ECC 22-24
> S4 25-31
> S5 32-37
> ECC 38-40
> S6 41-47
> S7 48-53
> ECC 54-56
> S8 57-63
>
> Change it by merging continuous sections:
> S1 1-5
> ECC 6-8
> S2 9-21
> ECC 22-24
> S3 25-37
> ECC 38-40
> S4 41-53
> ECC 54-56
> S5 57-63
>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
> ---
> v2: keep original comment and fix correctly skip byte 6 for small-page nand
>
> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 34 +++++++++++++-----------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index e4e3ceeac38f..767343e0e6e7 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -1100,30 +1100,32 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
> struct brcmnand_cfg *cfg = &host->hwcfg;
> int sas = cfg->spare_area_size << cfg->sector_size_1k;
> int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
> + u32 next;
>
> - if (section >= sectors * 2)
> + if (section > sectors)
> return -ERANGE;
>
> - oobregion->offset = (section / 2) * sas;
> + next = (section * sas);
> + if (section < sectors)
> + next += 6;
>
> - if (section & 1) {
> - oobregion->offset += 9;
> - oobregion->length = 7;
> + if (section) {
> + oobregion->offset = ((section - 1) * sas) + 9;
> } else {
> - oobregion->length = 6;
> -
> - /* First sector of each page may have BBI */
> - if (!section) {
> - /*
> - * Small-page NAND use byte 6 for BBI while large-page
> - * NAND use byte 0.
> - */
> - if (cfg->page_size > 512)
> - oobregion->offset++;
> - oobregion->length--;
> + /*
> + * Small-page NAND use byte 6 for BBI while large-page
> + * NAND use byte 0.
> + */
> + if (cfg->page_size > 512) {
> + oobregion->offset = 1;
> + } else {
> + oobregion->offset = 0;
> + next--;
> }
> }
>
> + oobregion->length = next - oobregion->offset;
> +
> return 0;
> }
>

I'm fine with the two patches but could you please invert the order and
add Fixes: + Cc: stable tags on both? on "fix hamming oob layout"
please? This way the fix is valid on older versions of the driver an
can be backported easily.

Thanks,
Miquèl

2020-05-12 06:04:37

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v3 0/2] mtd: rawnand: brcmnand: improve hamming oob layout

These patches improve the OOB hamming layout by reducing the number of oob
sections and correctly

v3: invert patch order.
v2: extend original comment and correctly skip byte 6 for small-page.

Álvaro Fernández Rojas (2):
mtd: rawnand: brcmnand: fix hamming oob layout
mtd: rawnand: brcmnand: improve hamming oob layout

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 34 +++++++++++++-----------
1 file changed, 18 insertions(+), 16 deletions(-)

--
2.26.2

2020-05-12 07:59:26

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v4 0/2] mtd: rawnand: brcmnand: improve hamming oob layout

These patches improve the OOB hamming layout by reducing the number of oob
sections and correctly reserving first two bytes for large page NANDs.

v4: clarify small/large pages comment.
v3: invert patch order.
v2: extend original comment and correctly skip byte 6 for small-page.

Álvaro Fernández Rojas (2):
mtd: rawnand: brcmnand: fix hamming oob layout
mtd: rawnand: brcmnand: improve hamming oob layout

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 34 +++++++++++++-----------
1 file changed, 18 insertions(+), 16 deletions(-)

--
2.26.2

2020-05-12 08:01:48

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v4 2/2] mtd: rawnand: brcmnand: improve hamming oob layout

The current code generates 8 oob sections:
S1 1-5
ECC 6-8
S2 9-15
S3 16-21
ECC 22-24
S4 25-31
S5 32-37
ECC 38-40
S6 41-47
S7 48-53
ECC 54-56
S8 57-63

Change it by merging continuous sections:
S1 1-5
ECC 6-8
S2 9-21
ECC 22-24
S3 25-37
ECC 38-40
S4 41-53
ECC 54-56
S5 57-63

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
v4: clarify small/large pages comment
v3: invert patch order
v2: keep original comment and fix correctly skip byte 6 for small-page nand

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 35 +++++++++++-------------
1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 1c1070111ebc..6292fac6cc4f 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1100,33 +1100,30 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
struct brcmnand_cfg *cfg = &host->hwcfg;
int sas = cfg->spare_area_size << cfg->sector_size_1k;
int sectors = cfg->page_size / (512 << cfg->sector_size_1k);
+ u32 next;

- if (section >= sectors * 2)
+ if (section > sectors)
return -ERANGE;

- oobregion->offset = (section / 2) * sas;
+ next = (section * sas);
+ if (section < sectors)
+ next += 6;

- if (section & 1) {
- oobregion->offset += 9;
- oobregion->length = 7;
+ if (section) {
+ oobregion->offset = ((section - 1) * sas) + 9;
} else {
- oobregion->length = 6;
-
- /* First sector of each page may have BBI */
- if (!section) {
- /*
- * Small-page NAND use byte 6 for BBI while large-page
- * NAND use bytes 0 and 1.
- */
- if (cfg->page_size > 512) {
- oobregion->offset += 2;
- oobregion->length -= 2;
- } else {
- oobregion->length--;
- }
+ if (cfg->page_size > 512) {
+ /* Large page NAND uses first 2 bytes for BBI */
+ oobregion->offset = 2;
+ } else {
+ /* Small page NAND uses last byte before ECC for BBI */
+ oobregion->offset = 0;
+ next--;
}
}

+ oobregion->length = next - oobregion->offset;
+
return 0;
}

--
2.26.2

2020-05-12 08:02:34

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH v4 1/2] mtd: rawnand: brcmnand: fix hamming oob layout

First 2 bytes are used in large-page nand.

Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops")
Cc: [email protected]
Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
v4: no changes
v3: invert patch order
v2: extend original comment

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index e4e3ceeac38f..1c1070111ebc 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1116,11 +1116,14 @@ static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section,
if (!section) {
/*
* Small-page NAND use byte 6 for BBI while large-page
- * NAND use byte 0.
+ * NAND use bytes 0 and 1.
*/
- if (cfg->page_size > 512)
- oobregion->offset++;
- oobregion->length--;
+ if (cfg->page_size > 512) {
+ oobregion->offset += 2;
+ oobregion->length -= 2;
+ } else {
+ oobregion->length--;
+ }
}
}

--
2.26.2

2020-05-24 19:19:26

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] mtd: rawnand: brcmnand: improve hamming oob layout

On Tue, 2020-05-12 at 07:57:33 UTC, =?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> The current code generates 8 oob sections:
> S1 1-5
> ECC 6-8
> S2 9-15
> S3 16-21
> ECC 22-24
> S4 25-31
> S5 32-37
> ECC 38-40
> S6 41-47
> S7 48-53
> ECC 54-56
> S8 57-63
>
> Change it by merging continuous sections:
> S1 1-5
> ECC 6-8
> S2 9-21
> ECC 22-24
> S3 25-37
> ECC 38-40
> S4 41-53
> ECC 54-56
> S5 57-63
>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>

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

Miquel

2020-05-24 19:19:33

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] mtd: rawnand: brcmnand: fix hamming oob layout

On Tue, 2020-05-12 at 07:57:32 UTC, =?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote:
> First 2 bytes are used in large-page nand.
>
> Fixes: ef5eeea6e911 ("mtd: nand: brcm: switch to mtd_ooblayout_ops")
> Cc: [email protected]
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>

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

Miquel