Patchwork [U-Boot,v2,2/3] mtd: nand: omap: add support for BCH16_ECC - NAND driver updates

login
register
mail settings
Submitter pekon gupta
Date Sept. 10, 2013, 7:25 a.m.
Message ID <1378797908-22567-3-git-send-email-pekon@ti.com>
Download mbox | patch
Permalink /patch/273769/
State Not Applicable
Delegated to: Scott Wood
Headers show

Comments

pekon gupta - Sept. 10, 2013, 7:25 a.m.
With increase in NAND flash densities occurence of bit-flips has increased.
Thus stronger ECC schemes are required for detecting and correcting multiple
simultaneous bit-flips in same NAND page. But stronger ECC schemes have large
ECC syndrome which require more space in OOB/Spare.
This patch add support for BCH16_ECC:
(a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
(b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.

Due to (b) this scheme can only be used with NAND devices which have enough
OOB to satisfy following equation:
OOBsize per page >= 26 * (page-size / 512)

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
 arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
 drivers/mtd/nand/omap_gpmc.c                 | 87 +++++++++++++++++++++++-----
 include/mtd/mtd-abi.h                        |  3 +-
 4 files changed, 90 insertions(+), 19 deletions(-)
Scott Wood - Nov. 14, 2013, 12:40 a.m.
On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> With increase in NAND flash densities occurence of bit-flips has increased.
> Thus stronger ECC schemes are required for detecting and correcting multiple
> simultaneous bit-flips in same NAND page. But stronger ECC schemes have large
> ECC syndrome which require more space in OOB/Spare.
> This patch add support for BCH16_ECC:
> (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> 
> Due to (b) this scheme can only be used with NAND devices which have enough
> OOB to satisfy following equation:
> OOBsize per page >= 26 * (page-size / 512)
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> 
> ---
> arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
>  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
>  drivers/mtd/nand/omap_gpmc.c                 | 87 +++++++++++++++++++++++-----
>  include/mtd/mtd-abi.h                        |  3 +-
>  4 files changed, 90 insertions(+), 19 deletions(-)

This doesn't apply cleanly.

> diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h
> index 10b56e0..1de92e6 100644
> --- a/arch/arm/include/asm/arch-am33xx/cpu.h
> +++ b/arch/arm/include/asm/arch-am33xx/cpu.h
> @@ -63,7 +63,16 @@ struct gpmc_cs {
>  };
>  
>  struct bch_res_0_3 {
> -	u32 bch_result_x[4];
> +	u32 bch_result0;
> +	u32 bch_result1;
> +	u32 bch_result2;
> +	u32 bch_result3;
> +};

Is this really an improvement?

It would also be nice if headers for things in drivers/mtd/nand weren't
in arch.

-Scott
pekon gupta - Nov. 19, 2013, 1:21 p.m.
> From: Scott Wood [mailto:scottwood@freescale.com]
> > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> > With increase in NAND flash densities occurence of bit-flips has increased.
> > Thus stronger ECC schemes are required for detecting and correcting
> multiple
> > simultaneous bit-flips in same NAND page. But stronger ECC schemes have
> large
> > ECC syndrome which require more space in OOB/Spare.
> > This patch add support for BCH16_ECC:
> > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> >
> > Due to (b) this scheme can only be used with NAND devices which have
> enough
> > OOB to satisfy following equation:
> > OOBsize per page >= 26 * (page-size / 512)
> >
> > Signed-off-by: Pekon Gupta <pekon@ti.com>
> >
> > ---
> > arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
> >  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
> >  drivers/mtd/nand/omap_gpmc.c                 | 87
> +++++++++++++++++++++++-----
> >  include/mtd/mtd-abi.h                        |  3 +-
> >  4 files changed, 90 insertions(+), 19 deletions(-)
> 
> This doesn't apply cleanly.
> 
This one is on top of previous patch series like..
[Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html
[Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html

So once the above series are cleaned and accepted. I'll rebase and re-send this
series for BCH16 ecc-scheme again..


> > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h
> b/arch/arm/include/asm/arch-am33xx/cpu.h
> > index 10b56e0..1de92e6 100644
> > --- a/arch/arm/include/asm/arch-am33xx/cpu.h
> > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h
> > @@ -63,7 +63,16 @@ struct gpmc_cs {
> >  };
> >
> >  struct bch_res_0_3 {
> > -	u32 bch_result_x[4];
> > +	u32 bch_result0;
> > +	u32 bch_result1;
> > +	u32 bch_result2;
> > +	u32 bch_result3;
> > +};
> 
> Is this really an improvement?
> 
Good for readability to match it to actual IP spec.. 
Each of bch_resultx map to individual 32-bit registers named accordingly
in IP spec.


> It would also be nice if headers for things in drivers/mtd/nand weren't
> in arch.
> 
I agree.. but I would take that separately, as I need to test all other
platforms also for this change .. (like am335x, am43xx, omap3.. )


with regards, pekon
Scott Wood - Nov. 19, 2013, 6:26 p.m.
On Tue, 2013-11-19 at 13:21 +0000, Gupta, Pekon wrote:
> > From: Scott Wood [mailto:scottwood@freescale.com]
> > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> > > With increase in NAND flash densities occurence of bit-flips has increased.
> > > Thus stronger ECC schemes are required for detecting and correcting
> > multiple
> > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have
> > large
> > > ECC syndrome which require more space in OOB/Spare.
> > > This patch add support for BCH16_ECC:
> > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> > >
> > > Due to (b) this scheme can only be used with NAND devices which have
> > enough
> > > OOB to satisfy following equation:
> > > OOBsize per page >= 26 * (page-size / 512)
> > >
> > > Signed-off-by: Pekon Gupta <pekon@ti.com>
> > >
> > > ---
> > > arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
> > >  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
> > >  drivers/mtd/nand/omap_gpmc.c                 | 87
> > +++++++++++++++++++++++-----
> > >  include/mtd/mtd-abi.h                        |  3 +-
> > >  4 files changed, 90 insertions(+), 19 deletions(-)
> > 
> > This doesn't apply cleanly.
> > 
> This one is on top of previous patch series like..
> [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html
> [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html
> 
> So once the above series are cleaned and accepted. I'll rebase and re-send this
> series for BCH16 ecc-scheme again..
> 
> 
> > > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h
> > b/arch/arm/include/asm/arch-am33xx/cpu.h
> > > index 10b56e0..1de92e6 100644
> > > --- a/arch/arm/include/asm/arch-am33xx/cpu.h
> > > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h
> > > @@ -63,7 +63,16 @@ struct gpmc_cs {
> > >  };
> > >
> > >  struct bch_res_0_3 {
> > > -	u32 bch_result_x[4];
> > > +	u32 bch_result0;
> > > +	u32 bch_result1;
> > > +	u32 bch_result2;
> > > +	u32 bch_result3;
> > > +};
> > 
> > Is this really an improvement?
> > 
> Good for readability to match it to actual IP spec.. 
> Each of bch_resultx map to individual 32-bit registers named accordingly
> in IP spec.

Hardware often names things "FOO1, FOO2, etc", but in C it's often
better to express as foo[n] rather than foo1, foo2, etc.

> > It would also be nice if headers for things in drivers/mtd/nand weren't
> > in arch.
> > 
> I agree.. but I would take that separately, as I need to test all other
> platforms also for this change .. (like am335x, am43xx, omap3.. )

Yes, of course it would be separate.

-Scott
Scott Wood - Nov. 19, 2013, 11:50 p.m.
On Tue, 2013-11-19 at 13:21 +0000, Gupta, Pekon wrote:
> > From: Scott Wood [mailto:scottwood@freescale.com]
> > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote:
> > > With increase in NAND flash densities occurence of bit-flips has increased.
> > > Thus stronger ECC schemes are required for detecting and correcting
> > multiple
> > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have
> > large
> > > ECC syndrome which require more space in OOB/Spare.
> > > This patch add support for BCH16_ECC:
> > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data.
> > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B.
> > >
> > > Due to (b) this scheme can only be used with NAND devices which have
> > enough
> > > OOB to satisfy following equation:
> > > OOBsize per page >= 26 * (page-size / 512)
> > >
> > > Signed-off-by: Pekon Gupta <pekon@ti.com>
> > >
> > > ---
> > > arch/arm/include/asm/arch-am33xx/cpu.h       | 15 ++++-
> > >  arch/arm/include/asm/arch-am33xx/omap_gpmc.h |  4 +-
> > >  drivers/mtd/nand/omap_gpmc.c                 | 87
> > +++++++++++++++++++++++-----
> > >  include/mtd/mtd-abi.h                        |  3 +-
> > >  4 files changed, 90 insertions(+), 19 deletions(-)
> > 
> > This doesn't apply cleanly.
> > 
> This one is on top of previous patch series like..
> [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html
> [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html
> 
> So once the above series are cleaned and accepted. I'll rebase and re-send this
> series for BCH16 ecc-scheme again..

Please be sure to mention such dependencies in the future.

-Scott

Patch

diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h
index 10b56e0..1de92e6 100644
--- a/arch/arm/include/asm/arch-am33xx/cpu.h
+++ b/arch/arm/include/asm/arch-am33xx/cpu.h
@@ -63,7 +63,16 @@  struct gpmc_cs {
 };
 
 struct bch_res_0_3 {
-	u32 bch_result_x[4];
+	u32 bch_result0;
+	u32 bch_result1;
+	u32 bch_result2;
+	u32 bch_result3;
+};
+
+struct bch_res_4_6 {
+	u32 bch_result4;
+	u32 bch_result5;
+	u32 bch_result6;
 };
 
 struct gpmc {
@@ -95,7 +104,9 @@  struct gpmc {
 	u8 res7[12];		/* 0x224 */
 	u32 testmomde_ctrl;	/* 0x230 */
 	u8 res8[12];		/* 0x234 */
-	struct bch_res_0_3 bch_result_0_3[2];	/* 0x240 */
+	struct bch_res_0_3 bch_result_0_3[8];	/* 0x240 - 0x2BF */
+	u8 res9[16 * 4];	/* 0x2C0 - 0x2FF */
+	struct bch_res_4_6 bch_result_4_6[8];	/* 0x300 - 0x37F */
 };
 
 /* Used for board specific gpmc initialization */
diff --git a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
index 69982c1..ade0ca4 100644
--- a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
+++ b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h
@@ -16,7 +16,9 @@  enum omap_ecc {
 	/* 8-bit  ECC calculation by GPMC, Error detection by Software */
 	OMAP_ECC_BCH8_CODE_HW_DETECTION_SW,
 	/* 8-bit  ECC calculation by GPMC, Error detection by ELM */
-	OMAP_ECC_BCH8_CODE_HW
+	OMAP_ECC_BCH8_CODE_HW,
+	/* 16-bit  ECC calculation by GPMC, Error detection by ELM */
+	OMAP_ECC_BCH16_CODE_HW,
 };
 
 #endif /* __ASM_ARCH_OMAP_GPMC_H */
diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c
index c1a7317..b044f00 100644
--- a/drivers/mtd/nand/omap_gpmc.c
+++ b/drivers/mtd/nand/omap_gpmc.c
@@ -154,21 +154,10 @@  static int __maybe_unused omap_correct_data(struct mtd_info *mtd, uint8_t *dat,
 struct nand_bch_priv {
 	uint8_t mode;
 	uint8_t type;
-	uint8_t nibbles;
 	struct bch_control *control;
 	enum omap_ecc ecc_scheme;
 };
 
-/* bch types */
-#define ECC_BCH4	0
-#define ECC_BCH8	1
-#define ECC_BCH16	2
-
-/* BCH nibbles for diff bch levels */
-#define ECC_BCH4_NIBBLES	13
-#define ECC_BCH8_NIBBLES	26
-#define ECC_BCH16_NIBBLES	52
-
 /*
  * This can be a single instance cause all current users have only one NAND
  * with nearly the same setup (BCH8, some with ELM and others with sw BCH
@@ -177,7 +166,6 @@  struct nand_bch_priv {
  */
 static __maybe_unused struct nand_bch_priv bch_priv = {
 	.type = ECC_BCH8,
-	.nibbles = ECC_BCH8_NIBBLES,
 	.control = NULL
 };
 
@@ -243,6 +231,19 @@  static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
 			eccsize1 = 2;  /* non-ECC bits in nibbles per sector */
 		}
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		ecc_algo = 0x1;
+		bch_type = 0x2;
+		if (mode == NAND_ECC_WRITE) {
+			bch_wrapmode = 0x01;
+			eccsize0 = 0;  /* extra bits in nibbles per sector */
+			eccsize1 = 52; /* OOB bits in nibbles per sector */
+		} else {
+			bch_wrapmode = 0x01;
+			eccsize0 = 52; /* ECC bits in nibbles per sector */
+			eccsize1 = 0;  /* non-ECC bits in nibbles per sector */
+		}
+		break;
 	default:
 		return;
 	}
@@ -295,7 +296,7 @@  static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 		break;
 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
 	case OMAP_ECC_BCH8_CODE_HW:
-		ptr = &gpmc_cfg->bch_result_0_3[0].bch_result_x[3];
+		ptr = &gpmc_cfg->bch_result_0_3[0].bch_result3;
 		val = readl(ptr);
 		ecc_code[i++] = (val >>  0) & 0xFF;
 		ptr--;
@@ -308,6 +309,27 @@  static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 			ptr--;
 		}
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		ptr = &gpmc_cfg->bch_result_4_6[0].bch_result6;
+		ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+		ecc_code[i++] = (readl(ptr) >>  0) & 0xFF;
+		ptr--;
+		for (j = 0; j < 2; j++) {
+			ecc_code[i++] = (readl(ptr) >> 24) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >> 16) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  0) & 0xFF;
+			ptr--;
+		}
+		ptr = &gpmc_cfg->bch_result_0_3[0].bch_result3;
+		for (j = 0; j < 4; j++) {
+			ecc_code[i++] = (readl(ptr) >> 24) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >> 16) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  8) & 0xFF;
+			ecc_code[i++] = (readl(ptr) >>  0) & 0xFF;
+			ptr--;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -326,6 +348,8 @@  static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
 	case OMAP_ECC_BCH8_CODE_HW:
 		ecc_code[chip->ecc.bytes - 1] = 0x00;
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		break;
 	}
 	return 0;
 }
@@ -376,12 +400,15 @@  static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 	case OMAP_ECC_BCH8_CODE_HW:
 		omap_reverse_list(calc_ecc, eccbytes - 1);
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		omap_reverse_list(calc_ecc, eccbytes);
+		break;
 	default:
 		return -EINVAL;
 	}
 	/* use elm module to check for errors */
-	elm_config((enum bch_level)(bch->type));
-	if (elm_check_error(calc_ecc, bch->nibbles, &error_count, error_loc)) {
+	elm_config(bch->type);
+	if (elm_check_error(calc_ecc, bch->type, &error_count, error_loc)) {
 		printf("nand: error: uncorrectable ECC errors\n");
 		return -EINVAL;
 	}
@@ -392,6 +419,9 @@  static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat,
 			/* 14th byte in ECC is reserved to match ROM layout */
 			error_max = SECTOR_BYTES + (eccbytes - 1);
 			break;
+		case ECC_BCH16:
+			error_max = SECTOR_BYTES + eccbytes;
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -614,6 +644,7 @@  static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme,
 			printf("nand: error: could not init_bch()\n");
 			return -ENODEV;
 		}
+		bch_priv.type		= ECC_BCH8;
 		/* define ecc-layout */
 		ecclayout->eccbytes	= nand->ecc.bytes *
 						(pagesize / SECTOR_BYTES);
@@ -638,6 +669,7 @@  static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme,
 		nand->ecc.read_page	= omap_read_page_bch;
 		/* ELM is used for ECC error detection */
 		elm_init();
+		bch_priv.type		= ECC_BCH8;
 		/* define ecc-layout */
 		ecclayout->eccbytes	= nand->ecc.bytes *
 						(pagesize / SECTOR_BYTES);
@@ -650,6 +682,31 @@  static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme,
 						BADBLOCK_MARKER_LENGTH;
 		bch->ecc_scheme		= OMAP_ECC_BCH8_CODE_HW;
 		break;
+	case OMAP_ECC_BCH16_CODE_HW:
+		debug("nand: using OMAP_ECC_BCH16_CODE_HW\n");
+		nand->ecc.mode		= NAND_ECC_HW;
+		nand->ecc.size		= SECTOR_BYTES;
+		nand->ecc.bytes		= 26;
+		nand->ecc.strength	= 16;
+		nand->ecc.hwctl		= omap_enable_hwecc;
+		nand->ecc.correct	= omap_correct_data_bch;
+		nand->ecc.calculate	= omap_calculate_ecc;
+		nand->ecc.read_page	= omap_read_page_bch;
+		/* ELM is used for ECC error detection */
+		elm_init();
+		bch_priv.type		= ECC_BCH16;
+		/* define ecc-layout */
+		ecclayout->eccbytes	= nand->ecc.bytes *
+						(pagesize / SECTOR_BYTES);
+		for (i = 0; i < ecclayout->eccbytes; i++)
+			ecclayout->eccpos[i] = i +
+						BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree[0].offset = i +
+						BADBLOCK_MARKER_LENGTH;
+		ecclayout->oobfree[0].length = oobsize - nand->ecc.bytes -
+						BADBLOCK_MARKER_LENGTH;
+		bch->ecc_scheme		= OMAP_ECC_BCH16_CODE_HW;
+		break;
 	default:
 		debug("nand: error: ecc scheme not enabled or supported\n");
 		return -EINVAL;
diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h
index d51c1ab..f7040be 100644
--- a/include/mtd/mtd-abi.h
+++ b/include/mtd/mtd-abi.h
@@ -156,13 +156,14 @@  struct nand_oobfree {
 };
 
 #define MTD_MAX_OOBFREE_ENTRIES	8
+#define MTD_MAX_ECCPOS_ENTRIES	256
 /*
  * ECC layout control structure. Exported to userspace for
  * diagnosis and to allow creation of raw images
  */
 struct nand_ecclayout {
 	uint32_t eccbytes;
-	uint32_t eccpos[128];
+	uint32_t eccpos[MTD_MAX_ECCPOS_ENTRIES];
 	uint32_t oobavail;
 	struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
 };