diff mbox

[v3,3/4] mtd: nand: omap: optimize chip->ecc.hwctl() for H/W ECC schemes

Message ID 1383385576-26095-4-git-send-email-pekon@ti.com
State New, archived
Headers show

Commit Message

pekon gupta Nov. 2, 2013, 9:46 a.m. UTC 
chip->ecc.hwctl() is used for preparing the H/W controller before read/write
NAND accesses (like assigning data-buf, enabling ECC scheme configs, etc.)

Though all ECC schemes in OMAP NAND driver use GPMC controller for generating
ECC syndrome (for both Read/Write accesses). But but in current code
HAM1_ECC and BCHx_ECC schemes implement individual function to achieve this.
This patch merges the GPMC configuration code for all ECC schemes into
single omap_enable_hwecc(), thus adding scalability for future ECC schemes.

 omap_enable_hwecc() + omap3_enable_hwecc_bch() -> omap_enable_hwecc()

Signed-off-by: Pekon Gupta <pekon@ti.com>
---
 drivers/mtd/nand/omap2.c | 212 +++++++++++++++++------------------------------
 1 file changed, 74 insertions(+), 138 deletions(-)

Comments

Brian Norris Nov. 13, 2013, 10:03 p.m. UTC | #1 
On Sat, Nov 02, 2013 at 03:16:15PM +0530, Pekon Gupta wrote:
> chip->ecc.hwctl() is used for preparing the H/W controller before read/write
> NAND accesses (like assigning data-buf, enabling ECC scheme configs, etc.)
> 
> Though all ECC schemes in OMAP NAND driver use GPMC controller for generating
> ECC syndrome (for both Read/Write accesses). But but in current code

s/But but/but/

:)

> HAM1_ECC and BCHx_ECC schemes implement individual function to achieve this.
> This patch merges the GPMC configuration code for all ECC schemes into
> single omap_enable_hwecc(), thus adding scalability for future ECC schemes.
> 
>  omap_enable_hwecc() + omap3_enable_hwecc_bch() -> omap_enable_hwecc()
> 
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
>  drivers/mtd/nand/omap2.c | 212 +++++++++++++++++------------------------------
>  1 file changed, 74 insertions(+), 138 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 1f59505..3a99e29 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -33,6 +33,10 @@
>  #define	DRIVER_NAME	"omap2-nand"
>  #define	OMAP_NAND_TIMEOUT_MS	5000
>  
> +#define GPMC_ECC_READ		0 /* Reset Hardware ECC for read */
> +#define GPMC_ECC_WRITE		1 /* Reset Hardware ECC for write */
> +#define GPMC_ECC_READSYN	2 /* Reset before syndrom is read back */
> +
>  #define NAND_Ecc_P1e		(1 << 0)
>  #define NAND_Ecc_P2e		(1 << 1)
>  #define NAND_Ecc_P4e		(1 << 2)
> @@ -101,13 +105,9 @@
>  #define P4o_s(a)	(TF(a & NAND_Ecc_P4o)		<< 1)
>  
>  #define	PREFETCH_CONFIG1_CS_SHIFT	24
> -#define	ECC_CONFIG_CS_SHIFT		1

You stopped using this macro in a different patch, so this removal is
unrelated to this patch. But I'm not sure the code is better without
these macros anyway... I'll leave it to your discretion, I guess.

>  #define	CS_MASK				0x7
>  #define	ENABLE_PREFETCH			(0x1 << 7)
>  #define	DMA_MPU_MODE_SHIFT		2
> -#define	ECCSIZE0_SHIFT			12
> -#define	ECCSIZE1_SHIFT			22
> -#define	ECC1RESULTSIZE			0x1
>  #define	ECCCLEAR			0x100
>  #define	ECC1				0x1
>  #define	PREFETCH_FIFOTHRESHOLD_MAX	0x40

...

The rest looks OK but very hard for me to verify. Do we have any
testers? Any platforms to check for regressions?

Brian
Ezequiel Garcia Nov. 14, 2013, 7:01 p.m. UTC | #2 
On Wed, Nov 13, 2013 at 02:03:30PM -0800, Brian Norris wrote:
> On Sat, Nov 02, 2013 at 03:16:15PM +0530, Pekon Gupta wrote:
> > chip->ecc.hwctl() is used for preparing the H/W controller before read/write
> > NAND accesses (like assigning data-buf, enabling ECC scheme configs, etc.)
> > 
> > Though all ECC schemes in OMAP NAND driver use GPMC controller for generating
> > ECC syndrome (for both Read/Write accesses). But but in current code
> 
> s/But but/but/
> 
> :)
> 
> > HAM1_ECC and BCHx_ECC schemes implement individual function to achieve this.
> > This patch merges the GPMC configuration code for all ECC schemes into
> > single omap_enable_hwecc(), thus adding scalability for future ECC schemes.
> > 
> >  omap_enable_hwecc() + omap3_enable_hwecc_bch() -> omap_enable_hwecc()
> > 
> > Signed-off-by: Pekon Gupta <pekon@ti.com>
> > ---
> >  drivers/mtd/nand/omap2.c | 212 +++++++++++++++++------------------------------
> >  1 file changed, 74 insertions(+), 138 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> > index 1f59505..3a99e29 100644
> > --- a/drivers/mtd/nand/omap2.c
> > +++ b/drivers/mtd/nand/omap2.c
> > @@ -33,6 +33,10 @@
> >  #define	DRIVER_NAME	"omap2-nand"
> >  #define	OMAP_NAND_TIMEOUT_MS	5000
> >  
> > +#define GPMC_ECC_READ		0 /* Reset Hardware ECC for read */
> > +#define GPMC_ECC_WRITE		1 /* Reset Hardware ECC for write */
> > +#define GPMC_ECC_READSYN	2 /* Reset before syndrom is read back */
> > +
> >  #define NAND_Ecc_P1e		(1 << 0)
> >  #define NAND_Ecc_P2e		(1 << 1)
> >  #define NAND_Ecc_P4e		(1 << 2)
> > @@ -101,13 +105,9 @@
> >  #define P4o_s(a)	(TF(a & NAND_Ecc_P4o)		<< 1)
> >  
> >  #define	PREFETCH_CONFIG1_CS_SHIFT	24
> > -#define	ECC_CONFIG_CS_SHIFT		1
> 
> You stopped using this macro in a different patch, so this removal is
> unrelated to this patch. But I'm not sure the code is better without
> these macros anyway... I'll leave it to your discretion, I guess.
> 
> >  #define	CS_MASK				0x7
> >  #define	ENABLE_PREFETCH			(0x1 << 7)
> >  #define	DMA_MPU_MODE_SHIFT		2
> > -#define	ECCSIZE0_SHIFT			12
> > -#define	ECCSIZE1_SHIFT			22
> > -#define	ECC1RESULTSIZE			0x1
> >  #define	ECCCLEAR			0x100
> >  #define	ECC1				0x1
> >  #define	PREFETCH_FIFOTHRESHOLD_MAX	0x40
> 
> ...
> 
> The rest looks OK but very hard for me to verify. Do we have any
> testers? Any platforms to check for regressions?
> 

I have 8-bit and 16-bit NAND modules to test this.

Pekon: Are you sending a new version of the whole patchset?
Please Cc me and I'll see about testing it.
diff mbox

Patch

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 1f59505..3a99e29 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -33,6 +33,10 @@ 
 #define	DRIVER_NAME	"omap2-nand"
 #define	OMAP_NAND_TIMEOUT_MS	5000
 
+#define GPMC_ECC_READ		0 /* Reset Hardware ECC for read */
+#define GPMC_ECC_WRITE		1 /* Reset Hardware ECC for write */
+#define GPMC_ECC_READSYN	2 /* Reset before syndrom is read back */
+
 #define NAND_Ecc_P1e		(1 << 0)
 #define NAND_Ecc_P2e		(1 << 1)
 #define NAND_Ecc_P4e		(1 << 2)
@@ -101,13 +105,9 @@ 
 #define P4o_s(a)	(TF(a & NAND_Ecc_P4o)		<< 1)
 
 #define	PREFETCH_CONFIG1_CS_SHIFT	24
-#define	ECC_CONFIG_CS_SHIFT		1
 #define	CS_MASK				0x7
 #define	ENABLE_PREFETCH			(0x1 << 7)
 #define	DMA_MPU_MODE_SHIFT		2
-#define	ECCSIZE0_SHIFT			12
-#define	ECCSIZE1_SHIFT			22
-#define	ECC1RESULTSIZE			0x1
 #define	ECCCLEAR			0x100
 #define	ECC1				0x1
 #define	PREFETCH_FIFOTHRESHOLD_MAX	0x40
@@ -118,26 +118,9 @@ 
 
 #define OMAP24XX_DMA_GPMC		4
 
-#define BCH8_MAX_ERROR		8	/* upto 8 bit correctable */
-#define BCH4_MAX_ERROR		4	/* upto 4 bit correctable */
-
 #define SECTOR_BYTES		512
 /* 4 bit padding to make byte aligned, 56 = 52 + 4 */
 #define BCH4_BIT_PAD		4
-#define BCH8_ECC_MAX		((SECTOR_BYTES + BCH8_ECC_OOB_BYTES) * 8)
-#define BCH4_ECC_MAX		((SECTOR_BYTES + BCH4_ECC_OOB_BYTES) * 8)
-
-/* GPMC ecc engine settings for read */
-#define BCH_WRAPMODE_1		1	/* BCH wrap mode 1 */
-#define BCH8R_ECC_SIZE0		0x1a	/* ecc_size0 = 26 */
-#define BCH8R_ECC_SIZE1		0x2	/* ecc_size1 = 2 */
-#define BCH4R_ECC_SIZE0		0xd	/* ecc_size0 = 13 */
-#define BCH4R_ECC_SIZE1		0x3	/* ecc_size1 = 3 */
-
-/* GPMC ecc engine settings for write */
-#define BCH_WRAPMODE_6		6	/* BCH wrap mode 6 */
-#define BCH_ECC_SIZE0		0x0	/* ecc_size0 = 0, no oob protection */
-#define BCH_ECC_SIZE1		0x20	/* ecc_size1 = 32 */
 
 #define BADBLOCK_MARKER_LENGTH		2
 
@@ -177,7 +160,6 @@  struct omap_nand_info {
 	int					buf_len;
 	struct gpmc_nand_regs		reg;
 	/* fields specific for BCHx_HW ECC scheme */
-	bool				is_elm_used;
 	struct device			*elm_dev;
 	struct device_node		*of_node;
 };
@@ -938,7 +920,7 @@  static int omap_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 	u32 val;
 
 	val = readl(info->reg.gpmc_ecc_config);
-	if (((val >> 1) & 0x07) != info->gpmc_cs) {
+	if (((val >> 1) & 0x7) != info->gpmc_cs) {
 		pr_err("%s: invalid ECC configuration for chip-select=%d",
 				DRIVER_NAME, info->gpmc_cs);
 		return -EINVAL;
@@ -954,47 +936,6 @@  static int omap_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 }
 
 /**
- * omap_enable_hwecc - This function enables the hardware ecc functionality
- * @mtd: MTD device structure
- * @mode: Read/Write mode
- */
-static void omap_enable_hwecc(struct mtd_info *mtd, int mode)
-{
-	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
-							mtd);
-	struct nand_chip *chip = mtd->priv;
-	unsigned int dev_width = (chip->options & NAND_BUSWIDTH_16) ? 1 : 0;
-	u32 val;
-
-	/* clear ecc and enable bits */
-	val = ECCCLEAR | ECC1;
-	writel(val, info->reg.gpmc_ecc_control);
-
-	/* program ecc and result sizes */
-	val = ((((info->nand.ecc.size >> 1) - 1) << ECCSIZE1_SHIFT) |
-			 ECC1RESULTSIZE);
-	writel(val, info->reg.gpmc_ecc_size_config);
-
-	switch (mode) {
-	case NAND_ECC_READ:
-	case NAND_ECC_WRITE:
-		writel(ECCCLEAR | ECC1, info->reg.gpmc_ecc_control);
-		break;
-	case NAND_ECC_READSYN:
-		writel(ECCCLEAR, info->reg.gpmc_ecc_control);
-		break;
-	default:
-		dev_info(&info->pdev->dev,
-			"error: unrecognized Mode[%d]!\n", mode);
-		break;
-	}
-
-	/* (ECC 16 or 8 bit col) | ( CS  )  | ECC Enable */
-	val = (dev_width << 7) | (info->gpmc_cs << 1) | (0x1);
-	writel(val, info->reg.gpmc_ecc_config);
-}
-
-/**
  * omap_wait - wait until the command is done
  * @mtd: MTD device structure
  * @chip: NAND Chip structure
@@ -1050,88 +991,85 @@  static int omap_dev_ready(struct mtd_info *mtd)
 	}
 }
 
-#if defined(CONFIG_MTD_NAND_ECC_BCH) || defined(CONFIG_MTD_NAND_OMAP_BCH)
 /**
- * omap3_enable_hwecc_bch - Program OMAP3 GPMC to perform BCH ECC correction
+ * omap_enable_hwecc - Configure OMAP GPMC to perform ECC calculation
  * @mtd: MTD device structure
  * @mode: Read/Write mode
- *
- * When using BCH, sector size is hardcoded to 512 bytes.
- * Using wrapping mode 6 both for reading and writing if ELM module not uses
- * for error correction.
- * On writing,
- * eccsize0 = 0  (no additional protected byte in spare area)
- * eccsize1 = 32 (skip 32 nibbles = 16 bytes per sector in spare area)
+ * Configurations for eccsize0, eccsize1, and bch_wrapmode are based on
+ * GPMC function spec:
+ * Section 4.6.3.2.3: Supported NAND page mappings and ECC schemes
  */
-static void omap3_enable_hwecc_bch(struct mtd_info *mtd, int mode)
+static void omap_enable_hwecc(struct mtd_info *mtd, int mode)
 {
-	int nerrors;
-	unsigned int dev_width, nsectors;
 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
 						   mtd);
 	struct nand_chip *chip = mtd->priv;
-	u32 val, wr_mode;
-	unsigned int ecc_size1, ecc_size0;
-
-	/* Using wrapping mode 6 for writing */
-	wr_mode = BCH_WRAPMODE_6;
-
-	/*
-	 * ECC engine enabled for valid ecc_size0 nibbles
-	 * and disabled for ecc_size1 nibbles.
-	 */
-	ecc_size0 = BCH_ECC_SIZE0;
-	ecc_size1 = BCH_ECC_SIZE1;
-
-	/* Perform ecc calculation on 512-byte sector */
-	nsectors = 1;
-
-	/* Update number of error correction */
-	nerrors = info->nand.ecc.strength;
-
-	/* Multi sector reading/writing for NAND flash with page size < 4096 */
-	if (info->is_elm_used && (mtd->writesize <= 4096)) {
-		if (mode == NAND_ECC_READ) {
-			/* Using wrapping mode 1 for reading */
-			wr_mode = BCH_WRAPMODE_1;
-
-			/*
-			 * ECC engine enabled for ecc_size0 nibbles
-			 * and disabled for ecc_size1 nibbles.
-			 */
-			ecc_size0 = (nerrors == 8) ?
-				BCH8R_ECC_SIZE0 : BCH4R_ECC_SIZE0;
-			ecc_size1 = (nerrors == 8) ?
-				BCH8R_ECC_SIZE1 : BCH4R_ECC_SIZE1;
+	unsigned int dev_width = (chip->options & NAND_BUSWIDTH_16) ? 1 : 0;
+	unsigned int nsectors = (mtd->writesize / SECTOR_BYTES);
+	unsigned int ecc_algo = 0;
+	unsigned int bch_type = 0;
+	unsigned int eccsize1 = 0x00, eccsize0 = 0x00, bch_wrapmode = 0x00;
+	u32 ecc_size_config_val = 0;
+	u32 ecc_config_val = 0;
+
+	switch (info->ecc_opt) {
+	case OMAP_ECC_HAM1_CODE_HW:
+		ecc_algo = 0x0;
+		bch_wrapmode = 0x00;
+		eccsize0 = (chip->ecc.size >> 1) - 1;
+		eccsize1 = 0;
+		nsectors = 0;
+		break;
+	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+	case OMAP_ECC_BCH4_CODE_HW:
+		ecc_algo = 0x1;
+		bch_type = 0x0;
+		if (mode == GPMC_ECC_READ) {
+			bch_wrapmode = 0x01;
+			eccsize0 = 13; /* ECC bits in nibbles per sector */
+			eccsize1 = 3;  /* non-ECC bits in nibbles per sector */
+		} else if (mode == GPMC_ECC_WRITE) {
+			eccsize0 = 0;  /* extra bits in nibbles per sector */
+			eccsize1 = 32; /* OOB bits in nibbles per sector */
+			bch_wrapmode = 0x06;
 		}
-
-		/* Perform ecc calculation for one page (< 4096) */
-		nsectors = info->nand.ecc.steps;
+		break;
+	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
+	case OMAP_ECC_BCH8_CODE_HW:
+		ecc_algo = 0x1;
+		bch_type = 0x1;
+		if (mode == GPMC_ECC_READ) {
+			bch_wrapmode = 0x01;
+			eccsize0 = 26; /* ECC bits in nibbles per sector */
+			eccsize1 = 2;  /* non-ECC bits in nibbles per sector */
+		} else if (mode == GPMC_ECC_WRITE) {
+			bch_wrapmode = 0x01;
+			eccsize0 = 0;  /* extra bits in nibbles per sector */
+			eccsize1 = 28; /* OOB bits in nibbles per sector */
+		}
+		break;
+	default:
+		pr_err("selected ECC scheme not supported or not enabled\n");
 	}
-
-	writel(ECC1, info->reg.gpmc_ecc_control);
-
+	/* clear previous ecc result and enable engine */
+	writel(ECCCLEAR | ECC1, info->reg.gpmc_ecc_control);
 	/* Configure ecc size for BCH */
-	val = (ecc_size1 << ECCSIZE1_SHIFT) | (ecc_size0 << ECCSIZE0_SHIFT);
-	writel(val, info->reg.gpmc_ecc_size_config);
-
-	dev_width = (chip->options & NAND_BUSWIDTH_16) ? 1 : 0;
-
-	/* BCH configuration */
-	val = ((1                        << 16) | /* enable BCH */
-	       (((nerrors == 8) ? 1 : 0) << 12) | /* 8 or 4 bits */
-	       (wr_mode                  <<  8) | /* wrap mode */
-	       (dev_width                <<  7) | /* bus width */
-	       (((nsectors-1) & 0x7)     <<  4) | /* number of sectors */
-	       (info->gpmc_cs            <<  1) | /* ECC CS */
-	       (0x1));                            /* enable ECC */
-
-	writel(val, info->reg.gpmc_ecc_config);
-
-	/* Clear ecc and enable bits */
+	ecc_size_config_val = (eccsize1 << 22) | (eccsize0 << 12);
+	writel(ecc_size_config_val, info->reg.gpmc_ecc_size_config);
+	/* Configure device details for BCH engine */
+	ecc_config_val = ((ecc_algo << 16)	| /* HAM1 | BCHx */
+			(bch_type << 12)	| /* BCH4/BCH8/BCH16 */
+			(bch_wrapmode << 8)	| /* wrap mode */
+			(dev_width << 7)	| /* bus width */
+			(((nsectors-1) & 0x7) << 4) | /* number of sectors */
+			(info->gpmc_cs <<  1)	| /* ECC CS */
+			(0x1));			  /* enable ECC */
+	writel(ecc_config_val, info->reg.gpmc_ecc_config);
+	/* Clear ecc and re-enable */
 	writel(ECCCLEAR | ECC1, info->reg.gpmc_ecc_control);
 }
 
+#if defined(CONFIG_MTD_NAND_ECC_BCH) || defined(CONFIG_MTD_NAND_OMAP_BCH)
 /**
  * omap_calculate_ecc_bch - Generate bytes of ECC bytes
  * @mtd:	MTD device structure
@@ -1440,7 +1378,6 @@  static int is_elm_present(struct omap_nand_info *info,
 			struct device_node *elm_node, enum bch_ecc bch_type)
 {
 	struct platform_device *pdev;
-	info->is_elm_used = false;
 	/* check whether elm-id is passed via DT */
 	if (!elm_node) {
 		pr_err("nand: error: ELM DT node not found\n");
@@ -1456,7 +1393,6 @@  static int is_elm_present(struct omap_nand_info *info,
 	info->elm_dev = &pdev->dev;
 	if (elm_config(info->elm_dev, bch_type))
 		return -ENODEV;
-	info->is_elm_used = true;
 	return 0;
 }
 #endif /* CONFIG_MTD_NAND_ECC_BCH */
@@ -1686,7 +1622,7 @@  static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.size		= 512;
 		nand_chip->ecc.bytes		= 7;
 		nand_chip->ecc.strength		= 4;
-		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.hwctl		= omap_enable_hwecc;
 		nand_chip->ecc.correct		= nand_bch_correct_data;
 		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
 		/* define ECC layout */
@@ -1720,7 +1656,7 @@  static int omap_nand_probe(struct platform_device *pdev)
 		/* 14th bit is kept reserved for ROM-code compatibility */
 		nand_chip->ecc.bytes		= 7 + 1;
 		nand_chip->ecc.strength		= 4;
-		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.hwctl		= omap_enable_hwecc;
 		nand_chip->ecc.correct		= omap_elm_correct_data;
 		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
 		nand_chip->ecc.read_page	= omap_read_page_bch;
@@ -1752,7 +1688,7 @@  static int omap_nand_probe(struct platform_device *pdev)
 		nand_chip->ecc.size		= 512;
 		nand_chip->ecc.bytes		= 13;
 		nand_chip->ecc.strength		= 8;
-		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.hwctl		= omap_enable_hwecc;
 		nand_chip->ecc.correct		= nand_bch_correct_data;
 		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
 		/* define ECC layout */
@@ -1787,7 +1723,7 @@  static int omap_nand_probe(struct platform_device *pdev)
 		/* 14th bit is kept reserved for ROM-code compatibility */
 		nand_chip->ecc.bytes		= 13 + 1;
 		nand_chip->ecc.strength		= 8;
-		nand_chip->ecc.hwctl		= omap3_enable_hwecc_bch;
+		nand_chip->ecc.hwctl		= omap_enable_hwecc;
 		nand_chip->ecc.correct		= omap_elm_correct_data;
 		nand_chip->ecc.calculate	= omap_calculate_ecc_bch;
 		nand_chip->ecc.read_page	= omap_read_page_bch;